Skip to content

Commit

Permalink
chore: x/auth audit changes (#11860)
Browse files Browse the repository at this point in the history
## Description

ref: #11362 



---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
  • Loading branch information
amaury1093 committed May 5, 2022
1 parent 6e18f58 commit 3e3c114
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 51 deletions.
32 changes: 25 additions & 7 deletions proto/cosmos/auth/v1beta1/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -34,17 +34,23 @@ service Query {
option (google.api.http).get = "/cosmos/auth/v1beta1/module_accounts";
}

// Bech32 queries bech32Prefix
// Bech32Prefix queries bech32Prefix
//
// Since: cosmos-sdk 0.46
rpc Bech32Prefix(Bech32PrefixRequest) returns (Bech32PrefixResponse) {
option (google.api.http).get = "/cosmos/auth/v1beta1/bech32";
}

// AddressBytesToString converts Account Address bytes to string
//
// Since: cosmos-sdk 0.46
rpc AddressBytesToString(AddressBytesToStringRequest) returns (AddressBytesToStringResponse) {
option (google.api.http).get = "/cosmos/auth/v1beta1/bech32/{address_bytes}";
}

// AddressStringToBytes converts Address string to bytes
//
// Since: cosmos-sdk 0.46
rpc AddressStringToBytes(AddressStringToBytesRequest) returns (AddressStringToBytesResponse) {
option (google.api.http).get = "/cosmos/auth/v1beta1/bech32/{address_string}";
}
Expand Down Expand Up @@ -101,30 +107,42 @@ message QueryModuleAccountsResponse {
repeated google.protobuf.Any accounts = 1 [(cosmos_proto.accepts_interface) = "ModuleAccountI"];
}

// Bech32PrefixRequest is the request type for Bech32Prefix rpc method
// Bech32PrefixRequest is the request type for Bech32Prefix rpc method.
//
// Since: cosmos-sdk 0.46
message Bech32PrefixRequest {}

// Bech32PrefixResponse is the response type for Bech32Prefix rpc method
// Bech32PrefixResponse is the response type for Bech32Prefix rpc method.
//
// Since: cosmos-sdk 0.46
message Bech32PrefixResponse {
string bech32_prefix = 1;
}

// AddressBytesToStringRequest is the request type for AddressString rpc method
// AddressBytesToStringRequest is the request type for AddressString rpc method.
//
// Since: cosmos-sdk 0.46
message AddressBytesToStringRequest {
bytes address_bytes = 1;
}

// AddressBytesToStringResponse is the response type for AddressString rpc method
// AddressBytesToStringResponse is the response type for AddressString rpc method.
//
// Since: cosmos-sdk 0.46
message AddressBytesToStringResponse {
string address_string = 1;
}

// AddressStringToBytesRequest is the request type for AccountBytes rpc method
// AddressStringToBytesRequest is the request type for AccountBytes rpc method.
//
// Since: cosmos-sdk 0.46
message AddressStringToBytesRequest {
string address_string = 1;
}

// AddressStringToBytesResponse is the response type for AddressBytes rpc method
// AddressStringToBytesResponse is the response type for AddressBytes rpc method.
//
// Since: cosmos-sdk 0.46
message AddressStringToBytesResponse {
bytes address_bytes = 1;
}
5 changes: 5 additions & 0 deletions x/auth/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ func (ak AccountKeeper) ModuleAccounts(c context.Context, req *types.QueryModule
return &types.QueryModuleAccountsResponse{Accounts: modAccounts}, nil
}

// Bech32Prefix returns the keeper internally stored bech32 prefix.
func (ak AccountKeeper) Bech32Prefix(ctx context.Context, req *types.Bech32PrefixRequest) (*types.Bech32PrefixResponse, error) {
bech32Prefix, err := ak.getBech32Prefix()
if err != nil {
Expand All @@ -128,6 +129,8 @@ func (ak AccountKeeper) Bech32Prefix(ctx context.Context, req *types.Bech32Prefi
return &types.Bech32PrefixResponse{Bech32Prefix: bech32Prefix}, nil
}

// AddressBytesToString converts an address from bytes to string, using the
// keeper's bech32 prefix.
func (ak AccountKeeper) AddressBytesToString(ctx context.Context, req *types.AddressBytesToStringRequest) (*types.AddressBytesToStringResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "empty request")
Expand All @@ -145,6 +148,8 @@ func (ak AccountKeeper) AddressBytesToString(ctx context.Context, req *types.Add
return &types.AddressBytesToStringResponse{AddressString: text}, nil
}

// AddressStringToBytes converts an address from string to bytes, using the
// keeper's bech32 prefix.
func (ak AccountKeeper) AddressStringToBytes(ctx context.Context, req *types.AddressStringToBytesRequest) (*types.AddressStringToBytesResponse, error) {
if req == nil {
return nil, status.Error(codes.InvalidArgument, "empty request")
Expand Down
4 changes: 1 addition & 3 deletions x/auth/keeper/keeper.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package keeper

import (
"errors"
"fmt"

gogotypes "github.com/gogo/protobuf/types"
Expand All @@ -11,7 +10,6 @@ import (
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/address"

storetypes "github.com/cosmos/cosmos-sdk/store/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/x/auth/types"
Expand Down Expand Up @@ -249,7 +247,7 @@ func (ak AccountKeeper) GetCodec() codec.BinaryCodec { return ak.cdc }
func (ak AccountKeeper) getBech32Prefix() (string, error) {
bech32Codec, ok := ak.addressCdc.(bech32Codec)
if !ok {
return "", errors.New("unable cast addressCdc to bech32Codec")
return "", fmt.Errorf("unable cast addressCdc to bech32Codec; expected %T got %T", bech32Codec, ak.addressCdc)
}

return bech32Codec.bech32Prefix, nil
Expand Down
2 changes: 1 addition & 1 deletion x/auth/signing/sign_mode_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ type SignerData struct {
// since in SIGN_MODE_DIRECT the account sequence is already in the signer
// info.
//
// In case of multisigs, this should be the multisig account number.
// In case of multisigs, this should be the multisig sequence.
Sequence uint64

// PubKey is the public key of the signer.
Expand Down
40 changes: 0 additions & 40 deletions x/auth/spec/03_antehandlers.md

This file was deleted.

38 changes: 38 additions & 0 deletions x/auth/spec/03_middlewares.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<!--
order: 3
-->

# Middlewares

The `x/auth` module presently has no transaction handlers of its own, but does expose middlewares directly called from BaseApp's `CheckTx` and `DeliverTx`, which can be used for performing any operations on transactions, such as basic validity checks on a transaction such that it could be thrown out of the mempool, or routing the transactions to their `Msg` service to perform state transitions.
The middlewares can be seen as a set of decorators wrapped one on top of the other, that check transactions within the current context, per [ADR-045](https://github.com/cosmos/cosmos-sdk/blob/v0.46.0-beta2/docs/architecture/adr-045-check-delivertx-middlewares.md).

Note that the middlewares are called on both `CheckTx` and `DeliverTx`, as Tendermint proposers presently have the ability to include in their proposed block transactions which fail `CheckTx`.

## List of Middleware

The auth module provides:

- one `tx.Handler`, called `RunMsgsTxHandler`, which routes each `sdk.Msg` from a transaction to the correct module `Msg` service, and runs each `sdk.Msg` to perform state transitions,
- a set of middlewares that are recursively chained together around the base `tx.Handler` in the following order (the first middleware's `pre`-hook is run first, and `post`-hook is run last):

- `NewTxDecoderMiddleware`: Decodes the transaction bytes from ABCI `CheckTx` and `DeliverTx` into the SDK transaction type. This middleware is generally called first, as most middlewares logic rely on a decoded SDK transaction.
- `GasTxMiddleware`: Sets the `GasMeter` in the `Context`.
- `RecoveryTxMiddleware`: Wraps the next middleware with a defer clause to recover from any downstream panics in the middleware chain to return an error with information on gas provided and gas used.
- `RejectExtensionOptionsMiddleware`: Rejects all extension options which can optionally be included in protobuf transactions.
- `IndexEventsTxMiddleware`: Choose which events to index in Tendermint. Make sure no events are emitted outside of this middleware.
- `ValidateBasicMiddleware`: Calls `tx.ValidateBasic` and returns any non-nil error.
- `TxTimeoutHeightMiddleware`: Check for a `tx` height timeout.
- `ValidateMemoMiddleware`: Validates `tx` memo with application parameters and returns any non-nil error.
- `ConsumeGasTxSizeMiddleware`: Consumes gas proportional to the `tx` size based on application parameters.
- `DeductFeeMiddleware`: Deducts the `FeeAmount` from first signer of the `tx`. If the `x/feegrant` module is enabled and a fee granter is set, it deducts fees from the fee granter account.
- `SetPubKeyMiddleware`: Sets the pubkey from a `tx`'s signers that does not already have its corresponding pubkey saved in the state machine and in the current context.
- `ValidateSigCountMiddleware`: Validates the number of signatures in the `tx` based on app-parameters.
- `SigGasConsumeMiddleware`: Consumes parameter-defined amount of gas for each signature. This requires pubkeys to be set in context for all signers as part of `SetPubKeyMiddleware`.
- `SigVerificationMiddleware`: Verifies all signatures are valid. This requires pubkeys to be set in context for all signers as part of `SetPubKeyMiddleware`.
- `IncrementSequenceMiddleware`: Increments the account sequence for each signer to prevent replay attacks.
- `WithBranchedStore`: Creates a new MultiStore branch, discards downstream writes if the downstream returns error.
- `ConsumeBlockGasMiddleware`: Consume block gas.
- `TipMiddleware`: Transfer tips to the fee payer in transactions with tips.

This default list of middlewares can be instantiated using the `NewDefaultTxHandler` function. If a chain wants to tweak the list of middlewares, they can create their own `NewTxHandler` function using the same template as `NewDefaultTxHandler`, and chain new middlewares in the `ComposeMiddleware` function.

0 comments on commit 3e3c114

Please sign in to comment.