Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: fix several typos in the document #22135

Merged
merged 8 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions docs/architecture/adr-003-dynamic-capability-store.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

Full implementation of the [IBC specification](https://github.com/cosmos/ibc) requires the ability to create and authenticate object-capability keys at runtime (i.e., during transaction execution),
as described in [ICS 5](https://github.com/cosmos/ibc/tree/master/spec/core/ics-005-port-allocation#technical-specification). In the IBC specification, capability keys are created for each newly initialised
port & channel, and are used to authenticate future usage of the port or channel. Since channels and potentially ports can be initialised during transaction execution, the state machine must be able to create
port & channel, and are used to authenticate future usage of the port or channel. Since channels and potential ports can be initialised during transaction execution, the state machine must be able to create
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially ports was good

object-capability keys at this time.

At present, the Cosmos SDK does not have the ability to do this. Object-capability keys are currently pointers (memory addresses) of `StoreKey` structs created at application initialisation in `app.go` ([example](https://github.com/cosmos/gaia/blob/dcbddd9f04b3086c0ad07ee65de16e7adedc7da4/app/app.go#L132))
Expand All @@ -27,15 +27,15 @@ the `CapabilityKeeper` will be hooked up to modules through unique function refe
(by calling `ScopeToModule`, defined below) so that it can identify the calling module when later
invoked.

When the initial state is loaded from disk, the `CapabilityKeeper`'s `Initialise` function will create
When the initial state is loaded from the disk, the `CapabilityKeeper`'s `Initialise` function will create
xiaobei0715 marked this conversation as resolved.
Show resolved Hide resolved
new capability keys for all previously allocated capability identifiers (allocated during execution of
past transactions and assigned to particular modes), and keep them in a memory-only store while the
chain is running.

The `CapabilityKeeper` will include a persistent `KVStore`, a `MemoryStore`, and an in-memory map.
The persistent `KVStore` tracks which capability is owned by which modules.
The `MemoryStore` stores a forward mapping that map from module name, capability tuples to capability names and
a reverse mapping that map from module name, capability name to the capability index.
The `MemoryStore` stores a forward mapping that maps from module name, capability tuples to capability names and
a reverse mapping that maps from module name, capability name to the capability index.
Since we cannot marshal the capability into a `KVStore` and unmarshal without changing the memory location of the capability,
the reverse mapping in the KVStore will simply map to an index. This index can then be used as a key in the ephemeral
go-map to retrieve the capability at the original memory location.
Expand Down Expand Up @@ -277,7 +277,7 @@ ck.InitialiseAndSeal(initialContext)

#### Creating, passing, claiming and using capabilities

Consider the case where `mod1` wants to create a capability, associate it with a resource (e.g. an IBC channel) by name, then pass it to `mod2` which will use it later:
Consider the case where `mod1` wants to create a capability, associate it with a resource (e.g. an IBC channel) by name, and then pass it to `mod2` which will use it later:

Module 1 would have the following code:

Expand Down Expand Up @@ -327,12 +327,12 @@ Proposed.
### Positive

* Dynamic capability support.
* Allows CapabilityKeeper to return same capability pointer from go-map while reverting any writes to the persistent `KVStore` and in-memory `MemoryStore` on tx failure.
* Allows CapabilityKeeper to return the same capability pointer from go-map while reverting any writes to the persistent `KVStore` and in-memory `MemoryStore` on tx failure.

### Negative

* Requires an additional keeper.
* Some overlap with existing `StoreKey` system (in the future they could be combined, since this is a superset functionality-wise).
* Some overlap with the existing `StoreKey` system (in the future they could be combined, since this is a superset functionality-wise).
* Requires an extra level of indirection in the reverse mapping, since MemoryStore must map to index which must then be used as key in a go map to retrieve the actual capability

### Neutral
Expand Down
6 changes: 3 additions & 3 deletions docs/architecture/adr-010-modular-antehandler.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ Pros:
Cons:

1. Improves granularity but still cannot get more granular than a per-module basis. e.g. If auth's `AnteHandle` function is in charge of validating memo and signatures, users cannot swap the signature-checking functionality while keeping the rest of auth's `AnteHandle` functionality.
2. Module AnteHandler are run one after the other. There is no way for one AnteHandler to wrap or "decorate" another.
2. Module AnteHandler is run one after the other. There is no way for one AnteHandler to wrap or "decorate" another.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would put antehandler plural here instead


### Decorator Pattern

Expand Down Expand Up @@ -157,7 +157,7 @@ app.SetAnteHandler(mm.GetAnteHandler())

#### Custom Workflow

This is an example workflow for a user that wants to implement custom antehandler logic. In this example, the user wants to implement custom signature verification and change the order of antehandler so that validate memo runs before signature verification.
This is an example workflow for a user who wants to implement custom antehandler logic. In this example, the user wants to implement custom signature verification and change the order of antehandler so that validate memo runs before signature verification.

##### User Code

Expand Down Expand Up @@ -192,7 +192,7 @@ In addition, this approach will not break any core Cosmos SDK API's. Since we pr

Allow Decorator interface that can be chained together to create a Cosmos SDK AnteHandler.

This allows users to choose between implementing an AnteHandler by themselves and setting it in the baseapp, or use the decorator pattern to chain their custom decorators with the Cosmos SDK provided decorators in the order they wish.
This allows users to choose between implementing an AnteHandler by themselves and setting it in the baseapp, or using the decorator pattern to chain their custom decorators with the Cosmos SDK provided decorators in the order they wish.

```go
// An AnteDecorator wraps an AnteHandler, and can do pre- and post-processing on the next AnteHandler
Expand Down
2 changes: 1 addition & 1 deletion docs/architecture/adr-012-state-accessors.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ value and finally unmarshal. Usually this is done by declaring `Keeper.GetXXX` a
which are repetitive and hard to maintain.

Second, this makes it harder to align with the object capability theorem: the right to access the
state is defined as a `StoreKey`, which gives full access on the entire Merkle tree, so a module cannot
state is defined as a `StoreKey`, which gives full access to the entire Merkle tree, so a module cannot
send the access right to a specific key-value pair (or a set of key-value pairs) to another module safely.

Finally, because the getter/setter functions are defined as methods of a module's `Keeper`, the reviewers
Expand Down
4 changes: 2 additions & 2 deletions docs/architecture/adr-013-metrics.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ Proposed

## Context

Telemetry is paramount into debugging and understanding what the application is doing and how it is
Telemetry is paramount in debugging and understanding what the application is doing and how it is
performing. We aim to expose metrics from modules and other core parts of the Cosmos SDK.

In addition, we should aim to support multiple configurable sinks that an operator may choose from.
Expand Down Expand Up @@ -148,7 +148,7 @@ func (k BaseKeeper) MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins)

### Positive

* Exposure into the performance and behavior of an application
* Exposure to the performance and behavior of an application

### Negative

Expand Down
22 changes: 11 additions & 11 deletions docs/architecture/adr-016-validator-consensus-key-rotation.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

## Context

Validator consensus key rotation feature has been discussed and requested for a long time, for the sake of safer validator key management policy (e.g. https://github.com/tendermint/tendermint/issues/1136). So, we suggest one of the simplest form of validator consensus key rotation implementation mostly onto Cosmos SDK.
Validator consensus key rotation feature has been discussed and requested for a long time, for the sake of safer validator key management policy (e.g. https://github.com/tendermint/tendermint/issues/1136). So, we suggest one of the simplest forms of validator consensus key rotation implementation mostly onto Cosmos SDK.

We don't need to make any update on consensus logic in Tendermint because Tendermint does not have any mapping information of consensus key and validator operator key, meaning that from Tendermint point of view, a consensus key rotation of a validator is simply a replacement of a consensus key to another.

Expand All @@ -21,33 +21,33 @@ Also, it should be noted that this ADR includes only the simplest form of consen
* create and broadcast a transaction with a `MsgRotateConsPubKey` that states the new consensus key is now coupled with the validator operator with signature from the validator's operator key.
* old consensus key becomes unable to participate on consensus immediately after the update of key mapping state on-chain.
* start validating with new consensus key.
* validators using HSM and KMS should update the consensus key in HSM to use the new rotated key after the height `h` when `MsgRotateConsPubKey` committed to the blockchain.
* validators using HSM and KMS should update the consensus key in HSM to use the new rotated key after the height `h` when `MsgRotateConsPubKey` is committed to the blockchain.

### Considerations

* consensus key mapping information management strategy
* store history of each key mapping changes in the kvstore.
* the state machine can search corresponding consensus key paired with given validator operator for any arbitrary height in a recent unbonding period.
* store history of each key mapping change in the kvstore.
* the state machine can search corresponding consensus key paired with the given validator operator for any arbitrary height in a recent unbonding period.
* the state machine does not need any historical mapping information which is past more than unbonding period.
* key rotation costs related to LCD and IBC
* LCD and IBC will have traffic/computation burden when there exists frequent power changes
* In current Tendermint design, consensus key rotations are seen as power changes from LCD or IBC perspective
* Therefore, to minimize unnecessary frequent key rotation behavior, we limited maximum number of rotation in recent unbonding period and also applied exponentially increasing rotation fee
* Therefore, to minimize unnecessary frequent key rotation behavior, we limited the maximum number of rotations in recent unbonding period and also applied an exponentially increasing rotation fee
* limits
* rotations are limited to 1 time in an unbonding window. In future rewrites of the staking module it could be made to happen more times than 1
* parameters can be decided by governance and stored in genesis file.
* parameters can be decided by governance and stored in the genesis file.
* key rotation fee
* a validator should pay `KeyRotationFee` to rotate the consensus key which is calculated as below
* `KeyRotationFee` = (max(`VotingPowerPercentage`, 1)* `InitialKeyRotationFee`) * 2^(number of rotations in `ConsPubKeyRotationHistory` in recent unbonding period)
* evidence module
* evidence module can search corresponding consensus key for any height from slashing keeper so that it can decide which consensus key is supposed to be used for given height.
* evidence module can search corresponding consensus key for any height from slashing keeper so that it can decide which consensus key is supposed to be used for a given height.
* abci.ValidatorUpdate
* tendermint already has ability to change a consensus key by ABCI communication(`ValidatorUpdate`).
* validator consensus key update can be done via creating new + delete old by change the power to zero.
* therefore, we expect we even do not need to change tendermint codebase at all to implement this feature.
* new genesis parameters in `staking` module
* `MaxConsPubKeyRotations` : maximum number of rotation can be executed by a validator in recent unbonding period. default value 10 is suggested(11th key rotation will be rejected)
* `InitialKeyRotationFee` : the initial key rotation fee when no key rotation has happened in recent unbonding period. default value 1atom is suggested(1atom fee for the first key rotation in recent unbonding period)
* `MaxConsPubKeyRotations` : maximum number of rotations can be executed by a validator in the recent unbonding period. default value 10 is suggested(11th key rotation will be rejected)
* `InitialKeyRotationFee` : the initial key rotation fee when no key rotation has happened in the recent unbonding period. default value 1atom is suggested(1atom fee for the first key rotation in recent unbonding period)

### Workflow

Expand All @@ -64,7 +64,7 @@ Also, it should be noted that this ADR includes only the simplest form of consen
3. `handleMsgRotateConsPubKey` gets `MsgRotateConsPubKey`, calls `RotateConsPubKey` with emits event
4. `RotateConsPubKey`
* checks if `NewPubKey` is not duplicated on `ValidatorsByConsAddr`
* checks if the validator is does not exceed parameter `MaxConsPubKeyRotations` by iterating `ConsPubKeyRotationHistory`
* checks if the validator does not exceed parameter `MaxConsPubKeyRotations` by iterating `ConsPubKeyRotationHistory`
* checks if the signing account has enough balance to pay `KeyRotationFee`
* pays `KeyRotationFee` to community fund
* overwrites `NewPubKey` in `validator.ConsPubKey`
Expand All @@ -81,7 +81,7 @@ Also, it should be noted that this ADR includes only the simplest form of consen
}
```

5. `ApplyAndReturnValidatorSetUpdates` checks if there is `ConsPubKeyRotationHistory` with `ConsPubKeyRotationHistory.RotatedHeight == ctx.BlockHeight()` and if so, generates 2 `ValidatorUpdate` , one for a remove validator and one for create new validator
5. `ApplyAndReturnValidatorSetUpdates` checks if there is `ConsPubKeyRotationHistory` with `ConsPubKeyRotationHistory.RotatedHeight == ctx.BlockHeight()` and if so, generates 2 `ValidatorUpdate` , one for a remove validator and one for creating a new validator
xiaobei0715 marked this conversation as resolved.
Show resolved Hide resolved

```go
abci.ValidatorUpdate{
Expand Down
Loading