diff --git a/CHANGELOG.md b/CHANGELOG.md index cf47fd423754..fc6557776113 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,10 +55,23 @@ have this method perform a no-op. * Update gov keys to use big endian encoding instead of little endian * (modules) [\#5017](https://github.com/cosmos/cosmos-sdk/pull/5017) The `x/genaccounts` module has been deprecated and all components removed except the `legacy/` package. +* (x/auth) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) Modular `AnteHandler` via composable decorators: + * The `AnteHandler` interface now returns `(newCtx Context, err error)` instead of `(newCtx Context, result sdk.Result, abort bool)` + * The `NewAnteHandler` function returns an `AnteHandler` function that returns the new `AnteHandler` + interface and has been moved into the `auth/ante` directory. + * `ValidateSigCount`, `ValidateMemo`, `ProcessPubKey`, `EnsureSufficientMempoolFee`, and `GetSignBytes` + have all been removed as public functions. + * Invalid Signatures may return `InvalidPubKey` instead of `Unauthorized` error, since the transaction + will first hit `SetPubKeyDecorator` before the `SigVerificationDecorator` runs. + * `StdTx#GetSignatures` will return an array of just signature byte slices `[][]byte` instead of + returning an array of `StdSignature` structs. To replicate the old behavior, use the public field + `StdTx.Signatures` to get back the array of StdSignatures `[]StdSignature`. ### Client Breaking Changes * (rest) [\#4783](https://github.com/cosmos/cosmos-sdk/issues/4783) The balance field in the DelegationResponse type is now sdk.Coin instead of sdk.Int +* (x/auth) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) The gas required to pass the `AnteHandler` has +increased significantly due to modular `AnteHandler` support. Increase GasLimit accordingly. ### Features @@ -79,6 +92,29 @@ upgrade via: `sudo rm -rf /Library/Developer/CommandLineTools; xcode-select --in correct version via: `pkgutil --pkg-info=com.apple.pkg.CLTools_Executables`. * (keys) [\#5097](https://github.com/cosmos/cosmos-sdk/pull/5097) New `keys migrate` command to assist users migrate their keys to the new keyring. +* (x/auth) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) Modular `AnteHandler` via composable decorators: + * The `AnteDecorator` interface has been introduced to allow users to implement modular `AnteHandler` + functionality that can be composed together to create a single `AnteHandler` rather than implementing + a custom `AnteHandler` completely from scratch, where each `AnteDecorator` allows for custom behavior in + tightly defined and logically isolated manner. These custom `AnteDecorator` can then be chained together + with default `AnteDecorator` or third-party `AnteDecorator` to create a modularized `AnteHandler` + which will run each `AnteDecorator` in the order specified in `ChainAnteDecorators`. For details + on the new architecture, refer to the [ADR](docs/architecture/adr-010-modular-antehandler.md). + * `ChainAnteDecorators` function has been introduced to take in a list of `AnteDecorators` and chain + them in sequence and return a single `AnteHandler`: + * `SetUpContextDecorator`: Sets `GasMeter` in context and creates defer clause to recover from any + `OutOfGas` panics in future AnteDecorators and return `OutOfGas` error to `BaseApp`. It MUST be the + first `AnteDecorator` in the chain for any application that uses gas (or another one that sets the gas meter). + * `ValidateBasicDecorator`: Calls tx.ValidateBasic and returns any non-nil error. + * `ValidateMemoDecorator`: Validates tx memo with application parameters and returns any non-nil error. + * `ConsumeGasTxSizeDecorator`: Consumes gas proportional to the tx size based on application parameters. + * `MempoolFeeDecorator`: Checks if fee is above local mempool `minFee` parameter during `CheckTx`. + * `DeductFeeDecorator`: Deducts the `FeeAmount` from first signer of the transaction. + * `SetPubKeyDecorator`: Sets pubkey of account in any account that does not already have pubkey saved in state machine. + * `SigGasConsumeDecorator`: Consume parameter-defined amount of gas for each signature. + * `SigVerificationDecorator`: Verify each signature is valid, return if there is an error. + * `ValidateSigCountDecorator`: Validate the number of signatures in tx based on app-parameters. + * `IncrementSequenceDecorator`: Increments the account sequence for each signer to prevent replay attacks. ### Improvements diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 6053937df3ad..939c4414fca6 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -585,7 +585,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk // performance benefits, but it'll be more difficult to get right. anteCtx, msCache = app.cacheTxContext(ctx, txBytes) - newCtx, result, abort := app.anteHandler(anteCtx, tx, mode == runTxModeSimulate) + newCtx, err := app.anteHandler(anteCtx, tx, mode == runTxModeSimulate) if !newCtx.IsZero() { // At this point, newCtx.MultiStore() is cache-wrapped, or something else // replaced by the ante handler. We want the original multistore, not one @@ -597,10 +597,14 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk ctx = newCtx.WithMultiStore(ms) } - gasWanted = result.GasWanted + // GasMeter expected to be set in AnteHandler + gasWanted = ctx.GasMeter().Limit() - if abort { - return result + if err != nil { + res := sdk.ResultFromError(err) + res.GasWanted = gasWanted + res.GasUsed = ctx.GasMeter().GasConsumed() + return res } msCache.Write() diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 46ac6908298c..c13ab2b28473 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -19,6 +19,7 @@ import ( "github.com/cosmos/cosmos-sdk/store/rootmulti" store "github.com/cosmos/cosmos-sdk/store/types" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) var ( @@ -594,15 +595,18 @@ func testTxDecoder(cdc *codec.Codec) sdk.TxDecoder { } func anteHandlerTxTest(t *testing.T, capKey *sdk.KVStoreKey, storeKey []byte) sdk.AnteHandler { - return func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) { + return func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, err error) { store := ctx.KVStore(capKey) txTest := tx.(txTest) if txTest.FailOnAnte { - return newCtx, sdk.ErrInternal("ante handler failure").Result(), true + return newCtx, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "ante handler failure") } - res = incrementingCounter(t, store, storeKey, txTest.Counter) + res := incrementingCounter(t, store, storeKey, txTest.Counter) + if !res.IsOK() { + err = sdkerrors.ABCIError(string(res.Codespace), uint32(res.Code), res.Log) + } return } } @@ -835,7 +839,7 @@ func TestSimulateTx(t *testing.T) { gasConsumed := uint64(5) anteOpt := func(bapp *BaseApp) { - bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) { + bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, err error) { newCtx = ctx.WithGasMeter(sdk.NewGasMeter(gasConsumed)) return }) @@ -896,7 +900,7 @@ func TestSimulateTx(t *testing.T) { func TestRunInvalidTransaction(t *testing.T) { anteOpt := func(bapp *BaseApp) { - bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) { + bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, err error) { return }) } @@ -980,17 +984,19 @@ func TestRunInvalidTransaction(t *testing.T) { func TestTxGasLimits(t *testing.T) { gasGranted := uint64(10) anteOpt := func(bapp *BaseApp) { - bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) { + bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, err error) { newCtx = ctx.WithGasMeter(sdk.NewGasMeter(gasGranted)) + // AnteHandlers must have their own defer/recover in order for the BaseApp + // to know how much gas was used! This is because the GasMeter is created in + // the AnteHandler, but if it panics the context won't be set properly in + // runTx's recover call. defer func() { if r := recover(); r != nil { switch rType := r.(type) { case sdk.ErrorOutOfGas: log := fmt.Sprintf("out of gas in location: %v", rType.Descriptor) - res = sdk.ErrOutOfGas(log).Result() - res.GasWanted = gasGranted - res.GasUsed = newCtx.GasMeter().GasConsumed() + err = sdkerrors.Wrap(sdkerrors.ErrOutOfGas, log) default: panic(r) } @@ -999,9 +1005,7 @@ func TestTxGasLimits(t *testing.T) { count := tx.(*txTest).Counter newCtx.GasMeter().ConsumeGas(uint64(count), "counter-ante") - res = sdk.Result{ - GasWanted: gasGranted, - } + return }) @@ -1065,17 +1069,14 @@ func TestTxGasLimits(t *testing.T) { func TestMaxBlockGasLimits(t *testing.T) { gasGranted := uint64(10) anteOpt := func(bapp *BaseApp) { - bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) { + bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, err error) { newCtx = ctx.WithGasMeter(sdk.NewGasMeter(gasGranted)) defer func() { if r := recover(); r != nil { switch rType := r.(type) { case sdk.ErrorOutOfGas: - log := fmt.Sprintf("out of gas in location: %v", rType.Descriptor) - res = sdk.ErrOutOfGas(log).Result() - res.GasWanted = gasGranted - res.GasUsed = newCtx.GasMeter().GasConsumed() + err = sdkerrors.Wrapf(sdkerrors.ErrOutOfGas, "out of gas in location: %v", rType.Descriptor) default: panic(r) } @@ -1084,9 +1085,7 @@ func TestMaxBlockGasLimits(t *testing.T) { count := tx.(*txTest).Counter newCtx.GasMeter().ConsumeGas(uint64(count), "counter-ante") - res = sdk.Result{ - GasWanted: gasGranted, - } + return }) @@ -1234,7 +1233,7 @@ func TestBaseAppAnteHandler(t *testing.T) { func TestGasConsumptionBadTx(t *testing.T) { gasWanted := uint64(5) anteOpt := func(bapp *BaseApp) { - bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) { + bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, err error) { newCtx = ctx.WithGasMeter(sdk.NewGasMeter(gasWanted)) defer func() { @@ -1242,9 +1241,7 @@ func TestGasConsumptionBadTx(t *testing.T) { switch rType := r.(type) { case sdk.ErrorOutOfGas: log := fmt.Sprintf("out of gas in location: %v", rType.Descriptor) - res = sdk.ErrOutOfGas(log).Result() - res.GasWanted = gasWanted - res.GasUsed = newCtx.GasMeter().GasConsumed() + err = sdkerrors.Wrap(sdkerrors.ErrOutOfGas, log) default: panic(r) } @@ -1254,12 +1251,9 @@ func TestGasConsumptionBadTx(t *testing.T) { txTest := tx.(txTest) newCtx.GasMeter().ConsumeGas(uint64(txTest.Counter), "counter-ante") if txTest.FailOnAnte { - return newCtx, sdk.ErrInternal("ante handler failure").Result(), true + return newCtx, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "ante handler failure") } - res = sdk.Result{ - GasWanted: gasWanted, - } return }) } @@ -1310,7 +1304,7 @@ func TestGasConsumptionBadTx(t *testing.T) { func TestQuery(t *testing.T) { key, value := []byte("hello"), []byte("goodbye") anteOpt := func(bapp *BaseApp) { - bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) { + bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, err error) { store := ctx.KVStore(capKey1) store.Set(key, value) return diff --git a/docs/architecture/adr-010-modular-antehandler.md b/docs/architecture/adr-010-modular-antehandler.md index 4322dd963666..56b2718b2694 100644 --- a/docs/architecture/adr-010-modular-antehandler.md +++ b/docs/architecture/adr-010-modular-antehandler.md @@ -191,16 +191,16 @@ type AnteDecorator interface { ``` ```go -// ChainDecorators will recursively link all of the AnteDecorators in the chain and return a final AnteHandler function +// ChainAnteDecorators will recursively link all of the AnteDecorators in the chain and return a final AnteHandler function // This is done to preserve the ability to set a single AnteHandler function in the baseapp. -func ChainDecorators(chain ...AnteDecorator) AnteHandler { +func ChainAnteDecorators(chain ...AnteDecorator) AnteHandler { if len(chain) == 1 { return func(ctx Context, tx Tx, simulate bool) { chain[0].AnteHandle(ctx, tx, simulate, nil) } } return func(ctx Context, tx Tx, simulate bool) { - chain[0].AnteHandle(ctx, tx, simulate, ChainDecorators(chain[1:])) + chain[0].AnteHandle(ctx, tx, simulate, ChainAnteDecorators(chain[1:])) } } ``` @@ -211,9 +211,9 @@ Define AnteDecorator functions ```go // Setup GasMeter, catch OutOfGasPanic and handle appropriately -type SetupDecorator struct{} +type SetUpContextDecorator struct{} -func (sud SetupDecorator) AnteHandle(ctx Context, tx Tx, simulate bool, next AnteHandler) (newCtx Context, err error) { +func (sud SetUpContextDecorator) AnteHandle(ctx Context, tx Tx, simulate bool, next AnteHandler) (newCtx Context, err error) { ctx.GasMeter = NewGasMeter(tx.Gas) defer func() { @@ -251,7 +251,7 @@ Link AnteDecorators to create a final AnteHandler. Set this AnteHandler in basea ```go // Create final antehandler by chaining the decorators together -antehandler := ChainDecorators(NewSetupDecorator(), NewSigVerifyDecorator(), NewUserDefinedDecorator()) +antehandler := ChainAnteDecorators(NewSetUpContextDecorator(), NewSigVerifyDecorator(), NewUserDefinedDecorator()) // Set chained Antehandler in the baseapp bapp.SetAnteHandler(antehandler) @@ -264,7 +264,7 @@ Pros: Cons: -1. Decorator pattern may have a deeply nested structure that is hard to understand, this is mitigated by having the decorator order explicitly listed in the `ChainDecorators` function. +1. Decorator pattern may have a deeply nested structure that is hard to understand, this is mitigated by having the decorator order explicitly listed in the `ChainAnteDecorators` function. 2. Does not make use of the ModuleManager design. Since this is already being used for BeginBlocker/EndBlocker, this proposal seems unaligned with that design pattern. ## Status diff --git a/types/handler.go b/types/handler.go index b978e8e51ef4..d0cc41ec46dc 100644 --- a/types/handler.go +++ b/types/handler.go @@ -5,4 +5,62 @@ type Handler func(ctx Context, msg Msg) Result // AnteHandler authenticates transactions, before their internal messages are handled. // If newCtx.IsZero(), ctx is used instead. -type AnteHandler func(ctx Context, tx Tx, simulate bool) (newCtx Context, result Result, abort bool) +type AnteHandler func(ctx Context, tx Tx, simulate bool) (newCtx Context, err error) + +// AnteDecorator wraps the next AnteHandler to perform custom pre- and post-processing. +type AnteDecorator interface { + AnteHandle(ctx Context, tx Tx, simulate bool, next AnteHandler) (newCtx Context, err error) +} + +// ChainDecorator chains AnteDecorators together with each AnteDecorator +// wrapping over the decorators further along chain and returns a single AnteHandler. +// +// NOTE: The first element is outermost decorator, while the last element is innermost +// decorator. Decorator ordering is critical since some decorators will expect +// certain checks and updates to be performed (e.g. the Context) before the decorator +// is run. These expectations should be documented clearly in a CONTRACT docline +// in the decorator's godoc. +// +// NOTE: Any application that uses GasMeter to limit transaction processing cost +// MUST set GasMeter with the FIRST AnteDecorator. Failing to do so will cause +// transactions to be processed with an infinite gasmeter and open a DOS attack vector. +// Use `ante.SetUpContextDecorator` or a custom Decorator with similar functionality. +func ChainAnteDecorators(chain ...AnteDecorator) AnteHandler { + if (chain[len(chain)-1] != Terminator{}) { + chain = append(chain, Terminator{}) + } + + if len(chain) == 1 { + return func(ctx Context, tx Tx, simulate bool) (Context, error) { + return chain[0].AnteHandle(ctx, tx, simulate, nil) + } + } + + return func(ctx Context, tx Tx, simulate bool) (Context, error) { + return chain[0].AnteHandle(ctx, tx, simulate, ChainAnteDecorators(chain[1:]...)) + } +} + +// Terminator AnteDecorator will get added to the chain to simplify decorator code +// Don't need to check if next == nil further up the chain +// ______ +// <((((((\\\ +// / . }\ +// ;--..--._|} +// (\ '--/\--' ) +// \\ | '-' :'| +// \\ . -==- .-| +// \\ \.__.' \--._ +// [\\ __.--| // _/'--. +// \ \\ .'-._ ('-----'/ __/ \ +// \ \\ / __>| | '--. | +// \ \\ | \ | / / / +// \ '\ / \ | | _/ / +// \ \ \ | | / / +// snd \ \ \ / +type Terminator struct{} + +// Simply return provided Context and nil error +func (t Terminator) AnteHandle(ctx Context, _ Tx, _ bool, _ AnteHandler) (Context, error) { + return ctx, nil +} diff --git a/x/auth/alias.go b/x/auth/alias.go index 7fae8d935d4f..9fe6904d169a 100644 --- a/x/auth/alias.go +++ b/x/auth/alias.go @@ -31,14 +31,9 @@ var ( // functions aliases NewAnteHandler = ante.NewAnteHandler GetSignerAcc = ante.GetSignerAcc - ValidateSigCount = ante.ValidateSigCount - ValidateMemo = ante.ValidateMemo - ProcessPubKey = ante.ProcessPubKey DefaultSigVerificationGasConsumer = ante.DefaultSigVerificationGasConsumer DeductFees = ante.DeductFees - EnsureSufficientMempoolFees = ante.EnsureSufficientMempoolFees SetGasMeter = ante.SetGasMeter - GetSignBytes = ante.GetSignBytes NewAccountKeeper = keeper.NewAccountKeeper NewQuerier = keeper.NewQuerier NewBaseAccount = types.NewBaseAccount diff --git a/x/auth/ante/ante.go b/x/auth/ante/ante.go index 457dba893f7b..bdf24823f137 100644 --- a/x/auth/ante/ante.go +++ b/x/auth/ante/ante.go @@ -1,423 +1,26 @@ package ante import ( - "bytes" - "encoding/hex" - "fmt" - - "github.com/tendermint/tendermint/crypto/ed25519" - - "github.com/tendermint/tendermint/crypto" - "github.com/tendermint/tendermint/crypto/multisig" - "github.com/tendermint/tendermint/crypto/secp256k1" - - "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/auth/exported" "github.com/cosmos/cosmos-sdk/x/auth/keeper" "github.com/cosmos/cosmos-sdk/x/auth/types" ) -var ( - // simulation signature values used to estimate gas consumption - simSecp256k1Pubkey secp256k1.PubKeySecp256k1 - simSecp256k1Sig [64]byte -) - -func init() { - // This decodes a valid hex string into a sepc256k1Pubkey for use in transaction simulation - bz, _ := hex.DecodeString("035AD6810A47F073553FF30D2FCC7E0D3B1C0B74B61A1AAA2582344037151E143A") - copy(simSecp256k1Pubkey[:], bz) -} - -// SignatureVerificationGasConsumer is the type of function that is used to both consume gas when verifying signatures -// and also to accept or reject different types of PubKey's. This is where apps can define their own PubKey -type SignatureVerificationGasConsumer = func(meter sdk.GasMeter, sig []byte, pubkey crypto.PubKey, params types.Params) sdk.Result - // NewAnteHandler returns an AnteHandler that checks and increments sequence // numbers, checks signatures & account numbers, and deducts fees from the first // signer. func NewAnteHandler(ak keeper.AccountKeeper, supplyKeeper types.SupplyKeeper, sigGasConsumer SignatureVerificationGasConsumer) sdk.AnteHandler { - return func( - ctx sdk.Context, tx sdk.Tx, simulate bool, - ) (newCtx sdk.Context, res sdk.Result, abort bool) { - - if addr := supplyKeeper.GetModuleAddress(types.FeeCollectorName); addr == nil { - panic(fmt.Sprintf("%s module account has not been set", types.FeeCollectorName)) - } - - // all transactions must be of type auth.StdTx - stdTx, ok := tx.(types.StdTx) - if !ok { - // Set a gas meter with limit 0 as to prevent an infinite gas meter attack - // during runTx. - newCtx = SetGasMeter(simulate, ctx, 0) - return newCtx, sdk.ErrInternal("tx must be StdTx").Result(), true - } - - params := ak.GetParams(ctx) - - // Ensure that the provided fees meet a minimum threshold for the validator, - // if this is a CheckTx. This is only for local mempool purposes, and thus - // is only ran on check tx. - if ctx.IsCheckTx() && !simulate { - res := EnsureSufficientMempoolFees(ctx, stdTx.Fee) - if !res.IsOK() { - return newCtx, res, true - } - } - - newCtx = SetGasMeter(simulate, ctx, stdTx.Fee.Gas) - - // AnteHandlers must have their own defer/recover in order for the BaseApp - // to know how much gas was used! This is because the GasMeter is created in - // the AnteHandler, but if it panics the context won't be set properly in - // runTx's recover call. - defer func() { - if r := recover(); r != nil { - switch rType := r.(type) { - case sdk.ErrorOutOfGas: - log := fmt.Sprintf( - "out of gas in location: %v; gasWanted: %d, gasUsed: %d", - rType.Descriptor, stdTx.Fee.Gas, newCtx.GasMeter().GasConsumed(), - ) - res = sdk.ErrOutOfGas(log).Result() - - res.GasWanted = stdTx.Fee.Gas - res.GasUsed = newCtx.GasMeter().GasConsumed() - abort = true - default: - panic(r) - } - } - }() - - if res := ValidateSigCount(stdTx, params); !res.IsOK() { - return newCtx, res, true - } - - if err := tx.ValidateBasic(); err != nil { - return newCtx, err.Result(), true - } - - newCtx.GasMeter().ConsumeGas(params.TxSizeCostPerByte*sdk.Gas(len(newCtx.TxBytes())), "txSize") - - if res := ValidateMemo(stdTx, params); !res.IsOK() { - return newCtx, res, true - } - - // stdSigs contains the sequence number, account number, and signatures. - // When simulating, this would just be a 0-length slice. - signerAddrs := stdTx.GetSigners() - signerAccs := make([]exported.Account, len(signerAddrs)) - isGenesis := ctx.BlockHeight() == 0 - - // fetch first signer, who's going to pay the fees - signerAccs[0], res = GetSignerAcc(newCtx, ak, signerAddrs[0]) - if !res.IsOK() { - return newCtx, res, true - } - - // deduct the fees - if !stdTx.Fee.Amount.IsZero() { - res = DeductFees(supplyKeeper, newCtx, signerAccs[0], stdTx.Fee.Amount) - if !res.IsOK() { - return newCtx, res, true - } - - // reload the account as fees have been deducted - signerAccs[0] = ak.GetAccount(newCtx, signerAccs[0].GetAddress()) - } - - // stdSigs contains the sequence number, account number, and signatures. - // When simulating, this would just be a 0-length slice. - stdSigs := stdTx.GetSignatures() - - for i := 0; i < len(stdSigs); i++ { - // skip the fee payer, account is cached and fees were deducted already - if i != 0 { - signerAccs[i], res = GetSignerAcc(newCtx, ak, signerAddrs[i]) - if !res.IsOK() { - return newCtx, res, true - } - } - - // check signature, return account with incremented nonce - signBytes := GetSignBytes(newCtx.ChainID(), stdTx, signerAccs[i], isGenesis) - signerAccs[i], res = processSig(newCtx, signerAccs[i], stdSigs[i], signBytes, simulate, params, sigGasConsumer) - if !res.IsOK() { - return newCtx, res, true - } - - ak.SetAccount(newCtx, signerAccs[i]) - } - - return newCtx, sdk.Result{GasWanted: stdTx.Fee.Gas}, false // continue... - } -} - -// GetSignerAcc returns an account for a given address that is expected to sign -// a transaction. -func GetSignerAcc(ctx sdk.Context, ak keeper.AccountKeeper, addr sdk.AccAddress) (exported.Account, sdk.Result) { - if acc := ak.GetAccount(ctx, addr); acc != nil { - return acc, sdk.Result{} - } - return nil, sdk.ErrUnknownAddress(fmt.Sprintf("account %s does not exist", addr)).Result() -} - -// ValidateSigCount validates that the transaction has a valid cumulative total -// amount of signatures. -func ValidateSigCount(stdTx types.StdTx, params types.Params) sdk.Result { - stdSigs := stdTx.GetSignatures() - - sigCount := 0 - for i := 0; i < len(stdSigs); i++ { - sigCount += types.CountSubKeys(stdSigs[i].PubKey) - if uint64(sigCount) > params.TxSigLimit { - return sdk.ErrTooManySignatures( - fmt.Sprintf("signatures: %d, limit: %d", sigCount, params.TxSigLimit), - ).Result() - } - } - - return sdk.Result{} -} - -// ValidateMemo validates the memo size. -func ValidateMemo(stdTx types.StdTx, params types.Params) sdk.Result { - memoLength := len(stdTx.GetMemo()) - if uint64(memoLength) > params.MaxMemoCharacters { - return sdk.ErrMemoTooLarge( - fmt.Sprintf( - "maximum number of characters is %d but received %d characters", - params.MaxMemoCharacters, memoLength, - ), - ).Result() - } - - return sdk.Result{} -} - -// verify the signature and increment the sequence. If the account doesn't have -// a pubkey, set it. -func processSig( - ctx sdk.Context, acc exported.Account, sig types.StdSignature, signBytes []byte, simulate bool, params types.Params, - sigGasConsumer SignatureVerificationGasConsumer, -) (updatedAcc exported.Account, res sdk.Result) { - - pubKey, res := ProcessPubKey(acc, sig, simulate) - if !res.IsOK() { - return nil, res - } - - err := acc.SetPubKey(pubKey) - if err != nil { - return nil, sdk.ErrInternal("setting PubKey on signer's account").Result() - } - - if simulate { - // Simulated txs should not contain a signature and are not required to - // contain a pubkey, so we must account for tx size of including a - // StdSignature (Amino encoding) and simulate gas consumption - // (assuming a SECP256k1 simulation key). - consumeSimSigGas(ctx.GasMeter(), pubKey, sig, params) - } - - if res := sigGasConsumer(ctx.GasMeter(), sig.Signature, pubKey, params); !res.IsOK() { - return nil, res - } - - if !simulate && !pubKey.VerifyBytes(signBytes, sig.Signature) { - return nil, sdk.ErrUnauthorized("signature verification failed; verify correct account sequence and chain-id").Result() - } - - if err := acc.SetSequence(acc.GetSequence() + 1); err != nil { - panic(err) - } - - return acc, res -} - -func consumeSimSigGas(gasmeter sdk.GasMeter, pubkey crypto.PubKey, sig types.StdSignature, params types.Params) { - simSig := types.StdSignature{PubKey: pubkey} - if len(sig.Signature) == 0 { - simSig.Signature = simSecp256k1Sig[:] - } - - sigBz := types.ModuleCdc.MustMarshalBinaryLengthPrefixed(simSig) - cost := sdk.Gas(len(sigBz) + 6) - - // If the pubkey is a multi-signature pubkey, then we estimate for the maximum - // number of signers. - if _, ok := pubkey.(multisig.PubKeyMultisigThreshold); ok { - cost *= params.TxSigLimit - } - - gasmeter.ConsumeGas(params.TxSizeCostPerByte*cost, "txSize") -} - -// ProcessPubKey verifies that the given account address matches that of the -// StdSignature. In addition, it will set the public key of the account if it -// has not been set. -func ProcessPubKey(acc exported.Account, sig types.StdSignature, simulate bool) (crypto.PubKey, sdk.Result) { - // If pubkey is not known for account, set it from the types.StdSignature. - pubKey := acc.GetPubKey() - if simulate { - // In simulate mode the transaction comes with no signatures, thus if the - // account's pubkey is nil, both signature verification and gasKVStore.Set() - // shall consume the largest amount, i.e. it takes more gas to verify - // secp256k1 keys than ed25519 ones. - if pubKey == nil { - return simSecp256k1Pubkey, sdk.Result{} - } - - return pubKey, sdk.Result{} - } - - if pubKey == nil { - pubKey = sig.PubKey - if pubKey == nil { - return nil, sdk.ErrInvalidPubKey("PubKey not found").Result() - } - - if !bytes.Equal(pubKey.Address(), acc.GetAddress()) { - return nil, sdk.ErrInvalidPubKey( - fmt.Sprintf("PubKey does not match Signer address %s", acc.GetAddress())).Result() - } - } - - return pubKey, sdk.Result{} -} - -// DefaultSigVerificationGasConsumer is the default implementation of SignatureVerificationGasConsumer. It consumes gas -// for signature verification based upon the public key type. The cost is fetched from the given params and is matched -// by the concrete type. -func DefaultSigVerificationGasConsumer( - meter sdk.GasMeter, sig []byte, pubkey crypto.PubKey, params types.Params, -) sdk.Result { - switch pubkey := pubkey.(type) { - case ed25519.PubKeyEd25519: - meter.ConsumeGas(params.SigVerifyCostED25519, "ante verify: ed25519") - return sdk.ErrInvalidPubKey("ED25519 public keys are unsupported").Result() - - case secp256k1.PubKeySecp256k1: - meter.ConsumeGas(params.SigVerifyCostSecp256k1, "ante verify: secp256k1") - return sdk.Result{} - - case multisig.PubKeyMultisigThreshold: - var multisignature multisig.Multisignature - codec.Cdc.MustUnmarshalBinaryBare(sig, &multisignature) - - consumeMultisignatureVerificationGas(meter, multisignature, pubkey, params) - return sdk.Result{} - - default: - return sdk.ErrInvalidPubKey(fmt.Sprintf("unrecognized public key type: %T", pubkey)).Result() - } -} - -func consumeMultisignatureVerificationGas(meter sdk.GasMeter, - sig multisig.Multisignature, pubkey multisig.PubKeyMultisigThreshold, - params types.Params) { - - size := sig.BitArray.Size() - sigIndex := 0 - for i := 0; i < size; i++ { - if sig.BitArray.GetIndex(i) { - DefaultSigVerificationGasConsumer(meter, sig.Sigs[sigIndex], pubkey.PubKeys[i], params) - sigIndex++ - } - } -} - -// DeductFees deducts fees from the given account. -// -// NOTE: We could use the CoinKeeper (in addition to the AccountKeeper, because -// the CoinKeeper doesn't give us accounts), but it seems easier to do this. -func DeductFees(supplyKeeper types.SupplyKeeper, ctx sdk.Context, acc exported.Account, fees sdk.Coins) sdk.Result { - blockTime := ctx.BlockHeader().Time - coins := acc.GetCoins() - - if !fees.IsValid() { - return sdk.ErrInsufficientFee(fmt.Sprintf("invalid fee amount: %s", fees)).Result() - } - - // verify the account has enough funds to pay for fees - _, hasNeg := coins.SafeSub(fees) - if hasNeg { - return sdk.ErrInsufficientFunds( - fmt.Sprintf("insufficient funds to pay for fees; %s < %s", coins, fees), - ).Result() - } - - // Validate the account has enough "spendable" coins as this will cover cases - // such as vesting accounts. - spendableCoins := acc.SpendableCoins(blockTime) - if _, hasNeg := spendableCoins.SafeSub(fees); hasNeg { - return sdk.ErrInsufficientFunds( - fmt.Sprintf("insufficient funds to pay for fees; %s < %s", spendableCoins, fees), - ).Result() - } - - err := supplyKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.FeeCollectorName, fees) - if err != nil { - return err.Result() - } - - return sdk.Result{} -} - -// EnsureSufficientMempoolFees verifies that the given transaction has supplied -// enough fees to cover a proposer's minimum fees. A result object is returned -// indicating success or failure. -// -// Contract: This should only be called during CheckTx as it cannot be part of -// consensus. -func EnsureSufficientMempoolFees(ctx sdk.Context, stdFee types.StdFee) sdk.Result { - minGasPrices := ctx.MinGasPrices() - if !minGasPrices.IsZero() { - requiredFees := make(sdk.Coins, len(minGasPrices)) - - // Determine the required fees by multiplying each required minimum gas - // price by the gas limit, where fee = ceil(minGasPrice * gasLimit). - glDec := sdk.NewDec(int64(stdFee.Gas)) - for i, gp := range minGasPrices { - fee := gp.Amount.Mul(glDec) - requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt()) - } - - if !stdFee.Amount.IsAnyGTE(requiredFees) { - return sdk.ErrInsufficientFee( - fmt.Sprintf( - "insufficient fees; got: %q required: %q", stdFee.Amount, requiredFees, - ), - ).Result() - } - } - - return sdk.Result{} -} - -// SetGasMeter returns a new context with a gas meter set from a given context. -func SetGasMeter(simulate bool, ctx sdk.Context, gasLimit uint64) sdk.Context { - // In various cases such as simulation and during the genesis block, we do not - // meter any gas utilization. - if simulate || ctx.BlockHeight() == 0 { - return ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) - } - - return ctx.WithGasMeter(sdk.NewGasMeter(gasLimit)) -} - -// GetSignBytes returns a slice of bytes to sign over for a given transaction -// and an account. -func GetSignBytes(chainID string, stdTx types.StdTx, acc exported.Account, genesis bool) []byte { - var accNum uint64 - if !genesis { - accNum = acc.GetAccountNumber() - } - - return types.StdSignBytes( - chainID, accNum, acc.GetSequence(), stdTx.Fee, stdTx.Msgs, stdTx.Memo, + return sdk.ChainAnteDecorators( + NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first + NewMempoolFeeDecorator(), + NewValidateBasicDecorator(), + NewValidateMemoDecorator(ak), + NewConsumeGasForTxSizeDecorator(ak), + NewSetPubKeyDecorator(ak), // SetPubKeyDecorator must be called before all signature verification decorators + NewValidateSigCountDecorator(ak), + NewDeductFeeDecorator(ak, supplyKeeper), + NewSigGasConsumeDecorator(ak, sigGasConsumer), + NewSigVerificationDecorator(ak), + NewIncrementSequenceDecorator(ak), // innermost AnteDecorator ) } diff --git a/x/auth/ante/ante_test.go b/x/auth/ante/ante_test.go index 982bbec9c7ed..b3d22061b668 100644 --- a/x/auth/ante/ante_test.go +++ b/x/auth/ante/ante_test.go @@ -13,37 +13,26 @@ import ( "github.com/tendermint/tendermint/crypto/secp256k1" sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/auth/ante" - "github.com/cosmos/cosmos-sdk/x/auth/exported" "github.com/cosmos/cosmos-sdk/x/auth/types" ) // run the tx through the anteHandler and ensure its valid func checkValidTx(t *testing.T, anteHandler sdk.AnteHandler, ctx sdk.Context, tx sdk.Tx, simulate bool) { - _, result, abort := anteHandler(ctx, tx, simulate) - require.Equal(t, "", result.Log) - require.False(t, abort) - require.Equal(t, sdk.CodeOK, result.Code) - require.True(t, result.IsOK()) + _, err := anteHandler(ctx, tx, simulate) + require.Nil(t, err) } // run the tx through the anteHandler and ensure it fails with the given code func checkInvalidTx(t *testing.T, anteHandler sdk.AnteHandler, ctx sdk.Context, tx sdk.Tx, simulate bool, code sdk.CodeType) { - newCtx, result, abort := anteHandler(ctx, tx, simulate) - require.True(t, abort) + _, err := anteHandler(ctx, tx, simulate) + require.NotNil(t, err) + + result := sdk.ResultFromError(err) require.Equal(t, code, result.Code, fmt.Sprintf("Expected %v, got %v", code, result)) require.Equal(t, sdk.CodespaceRoot, result.Codespace) - - if code == sdk.CodeOutOfGas { - stdTx, ok := tx.(types.StdTx) - require.True(t, ok, "tx must be in form auth.types.StdTx") - // GasWanted set correctly - require.Equal(t, stdTx.Fee.Gas, result.GasWanted, "Gas wanted not set correctly") - require.True(t, result.GasUsed > result.GasWanted, "GasUsed not greated than GasWanted") - // Check that context is set correctly - require.Equal(t, result.GasUsed, newCtx.GasMeter().GasConsumed(), "Context not updated correctly") - } } // Test various error cases in the AnteHandler control flow. @@ -354,12 +343,12 @@ func TestAnteHandlerMemoGas(t *testing.T) { checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeOutOfGas) // memo too large - fee = types.NewStdFee(9000, sdk.NewCoins(sdk.NewInt64Coin("atom", 0))) + fee = types.NewStdFee(50000, sdk.NewCoins(sdk.NewInt64Coin("atom", 0))) tx = types.NewTestTxWithMemo(ctx, []sdk.Msg{msg}, privs, accnums, seqs, fee, strings.Repeat("01234567890", 500)) checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeMemoTooLarge) // tx with memo has enough gas - fee = types.NewStdFee(9000, sdk.NewCoins(sdk.NewInt64Coin("atom", 0))) + fee = types.NewStdFee(50000, sdk.NewCoins(sdk.NewInt64Coin("atom", 0))) tx = types.NewTestTxWithMemo(ctx, []sdk.Msg{msg}, privs, accnums, seqs, fee, strings.Repeat("0123456789", 10)) checkValidTx(t, anteHandler, ctx, tx, false) } @@ -481,7 +470,7 @@ func TestAnteHandlerBadSignBytes(t *testing.T) { // test wrong signer if public key exist privs, accnums, seqs = []crypto.PrivKey{priv2}, []uint64{0}, []uint64{1} tx = types.NewTestTx(ctx, msgs, privs, accnums, seqs, fee) - checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeUnauthorized) + checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeInvalidPubKey) // test wrong signer if public doesn't exist msg = types.NewTestMsg(addr2) @@ -528,7 +517,7 @@ func TestAnteHandlerSetPubKey(t *testing.T) { msg = types.NewTestMsg(addr2) msgs = []sdk.Msg{msg} tx = types.NewTestTx(ctx, msgs, privs, []uint64{1}, seqs, fee) - sigs := tx.(types.StdTx).GetSignatures() + sigs := tx.(types.StdTx).Signatures sigs[0].PubKey = nil checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeInvalidPubKey) @@ -543,84 +532,6 @@ func TestAnteHandlerSetPubKey(t *testing.T) { require.Nil(t, acc2.GetPubKey()) } -func TestProcessPubKey(t *testing.T) { - app, ctx := createTestApp(true) - - // keys - _, _, addr1 := types.KeyTestPubAddr() - priv2, _, addr2 := types.KeyTestPubAddr() - acc1 := app.AccountKeeper.NewAccountWithAddress(ctx, addr1) - acc2 := app.AccountKeeper.NewAccountWithAddress(ctx, addr2) - - acc2.SetPubKey(priv2.PubKey()) - - type args struct { - acc exported.Account - sig types.StdSignature - simulate bool - } - tests := []struct { - name string - args args - wantErr bool - }{ - {"no sigs, simulate off", args{acc1, types.StdSignature{}, false}, true}, - {"no sigs, simulate on", args{acc1, types.StdSignature{}, true}, false}, - {"no sigs, account with pub, simulate on", args{acc2, types.StdSignature{}, true}, false}, - {"pubkey doesn't match addr, simulate off", args{acc1, types.StdSignature{PubKey: priv2.PubKey()}, false}, true}, - {"pubkey doesn't match addr, simulate on", args{acc1, types.StdSignature{PubKey: priv2.PubKey()}, true}, false}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - _, err := ante.ProcessPubKey(tt.args.acc, tt.args.sig, tt.args.simulate) - require.Equal(t, tt.wantErr, !err.IsOK()) - }) - } -} - -func TestConsumeSignatureVerificationGas(t *testing.T) { - params := types.DefaultParams() - msg := []byte{1, 2, 3, 4} - - pkSet1, sigSet1 := generatePubKeysAndSignatures(5, msg, false) - multisigKey1 := multisig.NewPubKeyMultisigThreshold(2, pkSet1) - multisignature1 := multisig.NewMultisig(len(pkSet1)) - expectedCost1 := expectedGasCostByKeys(pkSet1) - for i := 0; i < len(pkSet1); i++ { - multisignature1.AddSignatureFromPubKey(sigSet1[i], pkSet1[i], pkSet1) - } - - type args struct { - meter sdk.GasMeter - sig []byte - pubkey crypto.PubKey - params types.Params - } - tests := []struct { - name string - args args - gasConsumed uint64 - shouldErr bool - }{ - {"PubKeyEd25519", args{sdk.NewInfiniteGasMeter(), nil, ed25519.GenPrivKey().PubKey(), params}, types.DefaultSigVerifyCostED25519, true}, - {"PubKeySecp256k1", args{sdk.NewInfiniteGasMeter(), nil, secp256k1.GenPrivKey().PubKey(), params}, types.DefaultSigVerifyCostSecp256k1, false}, - {"Multisig", args{sdk.NewInfiniteGasMeter(), multisignature1.Marshal(), multisigKey1, params}, expectedCost1, false}, - {"unknown key", args{sdk.NewInfiniteGasMeter(), nil, nil, params}, 0, true}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - res := ante.DefaultSigVerificationGasConsumer(tt.args.meter, tt.args.sig, tt.args.pubkey, tt.args.params) - - if tt.shouldErr { - require.False(t, res.IsOK()) - } else { - require.True(t, res.IsOK()) - require.Equal(t, tt.gasConsumed, tt.args.meter.GasConsumed(), fmt.Sprintf("%d != %d", tt.gasConsumed, tt.args.meter.GasConsumed())) - } - }) - } -} - func generatePubKeysAndSignatures(n int, msg []byte, keyTypeed25519 bool) (pubkeys []crypto.PubKey, signatures [][]byte) { pubkeys = make([]crypto.PubKey, n) signatures = make([][]byte, n) @@ -702,14 +613,15 @@ func TestAnteHandlerSigLimitExceeded(t *testing.T) { priv7, _, addr7 := types.KeyTestPubAddr() priv8, _, addr8 := types.KeyTestPubAddr() + addrs := []sdk.AccAddress{addr1, addr2, addr3, addr4, addr5, addr6, addr7, addr8} + // set the accounts - acc1 := app.AccountKeeper.NewAccountWithAddress(ctx, addr1) - acc1.SetCoins(types.NewTestCoins()) - app.AccountKeeper.SetAccount(ctx, acc1) - acc2 := app.AccountKeeper.NewAccountWithAddress(ctx, addr2) - acc2.SetCoins(types.NewTestCoins()) - require.NoError(t, acc2.SetAccountNumber(1)) - app.AccountKeeper.SetAccount(ctx, acc2) + for i, addr := range addrs { + acc := app.AccountKeeper.NewAccountWithAddress(ctx, addr) + acc.SetCoins(types.NewTestCoins()) + acc.SetAccountNumber(uint64(i)) + app.AccountKeeper.SetAccount(ctx, acc) + } var tx sdk.Tx msg := types.NewTestMsg(addr1, addr2, addr3, addr4, addr5, addr6, addr7, addr8) @@ -718,75 +630,24 @@ func TestAnteHandlerSigLimitExceeded(t *testing.T) { // test rejection logic privs, accnums, seqs := []crypto.PrivKey{priv1, priv2, priv3, priv4, priv5, priv6, priv7, priv8}, - []uint64{0, 0, 0, 0, 0, 0, 0, 0}, []uint64{0, 0, 0, 0, 0, 0, 0, 0} + []uint64{0, 1, 2, 3, 4, 5, 6, 7}, []uint64{0, 0, 0, 0, 0, 0, 0, 0} tx = types.NewTestTx(ctx, msgs, privs, accnums, seqs, fee) checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeTooManySignatures) } -func TestEnsureSufficientMempoolFees(t *testing.T) { - // setup - _, ctx := createTestApp(true) - ctx = ctx.WithMinGasPrices( - sdk.DecCoins{ - sdk.NewDecCoinFromDec("photino", sdk.NewDecWithPrec(50000000000000, sdk.Precision)), // 0.0001photino - sdk.NewDecCoinFromDec("stake", sdk.NewDecWithPrec(10000000000000, sdk.Precision)), // 0.000001stake - }, - ) - - testCases := []struct { - input types.StdFee - expectedOK bool - }{ - {types.NewStdFee(200000, sdk.Coins{}), false}, - {types.NewStdFee(200000, sdk.NewCoins(sdk.NewInt64Coin("photino", 5))), false}, - {types.NewStdFee(200000, sdk.NewCoins(sdk.NewInt64Coin("stake", 1))), false}, - {types.NewStdFee(200000, sdk.NewCoins(sdk.NewInt64Coin("stake", 2))), true}, - {types.NewStdFee(200000, sdk.NewCoins(sdk.NewInt64Coin("photino", 10))), true}, - { - types.NewStdFee( - 200000, - sdk.NewCoins( - sdk.NewInt64Coin("photino", 10), - sdk.NewInt64Coin("stake", 2), - ), - ), - true, - }, - { - types.NewStdFee( - 200000, - sdk.NewCoins( - sdk.NewInt64Coin("atom", 5), - sdk.NewInt64Coin("photino", 10), - sdk.NewInt64Coin("stake", 2), - ), - ), - true, - }, - } - - for i, tc := range testCases { - res := ante.EnsureSufficientMempoolFees(ctx, tc.input) - require.Equal( - t, tc.expectedOK, res.IsOK(), - "unexpected result; tc #%d, input: %v, log: %v", i, tc.input, res.Log, - ) - } -} - // Test custom SignatureVerificationGasConsumer func TestCustomSignatureVerificationGasConsumer(t *testing.T) { // setup app, ctx := createTestApp(true) ctx = ctx.WithBlockHeight(1) // setup an ante handler that only accepts PubKeyEd25519 - anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, func(meter sdk.GasMeter, sig []byte, pubkey crypto.PubKey, params types.Params) sdk.Result { + anteHandler := ante.NewAnteHandler(app.AccountKeeper, app.SupplyKeeper, func(meter sdk.GasMeter, sig []byte, pubkey crypto.PubKey, params types.Params) error { switch pubkey := pubkey.(type) { case ed25519.PubKeyEd25519: meter.ConsumeGas(params.SigVerifyCostED25519, "ante verify: ed25519") - return sdk.Result{} + return nil default: - return sdk.ErrInvalidPubKey(fmt.Sprintf("unrecognized public key type: %T", pubkey)).Result() + return sdkerrors.Wrapf(sdkerrors.ErrInvalidPubKey, "unrecognized public key type: %T", pubkey) } }) diff --git a/x/auth/ante/basic.go b/x/auth/ante/basic.go new file mode 100644 index 000000000000..b73374a80012 --- /dev/null +++ b/x/auth/ante/basic.go @@ -0,0 +1,87 @@ +package ante + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + err "github.com/cosmos/cosmos-sdk/types/errors" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + + "github.com/cosmos/cosmos-sdk/x/auth/keeper" + "github.com/cosmos/cosmos-sdk/x/auth/types" +) + +var ( + _ TxWithMemo = (*types.StdTx)(nil) // assert StdTx implements TxWithMemo +) + +// ValidateBasicDecorator will call tx.ValidateBasic and return any non-nil error. +// If ValidateBasic passes, decorator calls next AnteHandler in chain. +type ValidateBasicDecorator struct{} + +func NewValidateBasicDecorator() ValidateBasicDecorator { + return ValidateBasicDecorator{} +} + +func (vbd ValidateBasicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { + if err := tx.ValidateBasic(); err != nil { + return ctx, err + } + + return next(ctx, tx, simulate) +} + +// Tx must have GetMemo() method to use ValidateMemoDecorator +type TxWithMemo interface { + sdk.Tx + GetMemo() string +} + +// ValidateMemoDecorator will validate memo given the parameters passed in +// If memo is too large decorator returns with error, otherwise call next AnteHandler +// CONTRACT: Tx must implement TxWithMemo interface +type ValidateMemoDecorator struct { + ak keeper.AccountKeeper +} + +func NewValidateMemoDecorator(ak keeper.AccountKeeper) ValidateMemoDecorator { + return ValidateMemoDecorator{ + ak: ak, + } +} + +func (vmd ValidateMemoDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { + memoTx, ok := tx.(TxWithMemo) + if !ok { + return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type") + } + + params := vmd.ak.GetParams(ctx) + + memoLength := len(memoTx.GetMemo()) + if uint64(memoLength) > params.MaxMemoCharacters { + return ctx, err.Wrapf(err.ErrMemoTooLarge, + "maximum number of characters is %d but received %d characters", + params.MaxMemoCharacters, memoLength, + ) + } + + return next(ctx, tx, simulate) +} + +// ConsumeTxSizeGasDecorator will take in parameters and consume gas proportional to the size of tx +// before calling next AnteHandler +type ConsumeTxSizeGasDecorator struct { + ak keeper.AccountKeeper +} + +func NewConsumeGasForTxSizeDecorator(ak keeper.AccountKeeper) ConsumeTxSizeGasDecorator { + return ConsumeTxSizeGasDecorator{ + ak: ak, + } +} + +func (cgts ConsumeTxSizeGasDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { + params := cgts.ak.GetParams(ctx) + ctx.GasMeter().ConsumeGas(params.TxSizeCostPerByte*sdk.Gas(len(ctx.TxBytes())), "txSize") + + return next(ctx, tx, simulate) +} diff --git a/x/auth/ante/basic_test.go b/x/auth/ante/basic_test.go new file mode 100644 index 000000000000..db9c1a8b31ef --- /dev/null +++ b/x/auth/ante/basic_test.go @@ -0,0 +1,102 @@ +package ante_test + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/crypto" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/auth/ante" + "github.com/cosmos/cosmos-sdk/x/auth/types" +) + +func TestValidateBasic(t *testing.T) { + // setup + _, ctx := createTestApp(true) + + // keys and addresses + priv1, _, addr1 := types.KeyTestPubAddr() + + // msg and signatures + msg1 := types.NewTestMsg(addr1) + fee := types.NewTestStdFee() + + msgs := []sdk.Msg{msg1} + + privs, accNums, seqs := []crypto.PrivKey{}, []uint64{}, []uint64{} + invalidTx := types.NewTestTx(ctx, msgs, privs, accNums, seqs, fee) + + vbd := ante.NewValidateBasicDecorator() + antehandler := sdk.ChainAnteDecorators(vbd) + _, err := antehandler(ctx, invalidTx, false) + + require.NotNil(t, err, "Did not error on invalid tx") + + privs, accNums, seqs = []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} + validTx := types.NewTestTx(ctx, msgs, privs, accNums, seqs, fee) + + _, err = antehandler(ctx, validTx, false) + require.Nil(t, err, "ValidateBasicDecorator returned error on valid tx. err: %v", err) +} + +func TestValidateMemo(t *testing.T) { + // setup + app, ctx := createTestApp(true) + + // keys and addresses + priv1, _, addr1 := types.KeyTestPubAddr() + + // msg and signatures + msg1 := types.NewTestMsg(addr1) + fee := types.NewTestStdFee() + + msgs := []sdk.Msg{msg1} + + privs, accNums, seqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} + invalidTx := types.NewTestTxWithMemo(ctx, msgs, privs, accNums, seqs, fee, strings.Repeat("01234567890", 500)) + + // require that long memos get rejected + vmd := ante.NewValidateMemoDecorator(app.AccountKeeper) + antehandler := sdk.ChainAnteDecorators(vmd) + _, err := antehandler(ctx, invalidTx, false) + + require.NotNil(t, err, "Did not error on tx with high memo") + + validTx := types.NewTestTxWithMemo(ctx, msgs, privs, accNums, seqs, fee, strings.Repeat("01234567890", 10)) + + // require small memos pass ValidateMemo Decorator + _, err = antehandler(ctx, validTx, false) + require.Nil(t, err, "ValidateBasicDecorator returned error on valid tx. err: %v", err) +} + +func TestConsumeGasForTxSize(t *testing.T) { + // setup + app, ctx := createTestApp(true) + + cgtsd := ante.NewConsumeGasForTxSizeDecorator(app.AccountKeeper) + antehandler := sdk.ChainAnteDecorators(cgtsd) + + params := app.AccountKeeper.GetParams(ctx) + txBytes := []byte(strings.Repeat("a", 10)) + expectedGas := sdk.Gas(len(txBytes)) * params.TxSizeCostPerByte + + // Set ctx with TxBytes manually + ctx = ctx.WithTxBytes(txBytes) + + // track how much gas is necessary to retrieve parameters + beforeGas := ctx.GasMeter().GasConsumed() + app.AccountKeeper.GetParams(ctx) + afterGas := ctx.GasMeter().GasConsumed() + expectedGas += afterGas - beforeGas + + // No need to send tx here since this Decorator will do nothing with it + beforeGas = ctx.GasMeter().GasConsumed() + ctx, err := antehandler(ctx, nil, false) + require.Nil(t, err, "ConsumeTxSizeGasDecorator returned error: %v", err) + + // require that decorator consumes expected amount of gas + consumedGas := ctx.GasMeter().GasConsumed() - beforeGas + require.Equal(t, expectedGas, consumedGas, "Decorator did not consume the correct amount of gas") +} diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go new file mode 100644 index 000000000000..d428cb713593 --- /dev/null +++ b/x/auth/ante/fee.go @@ -0,0 +1,144 @@ +package ante + +import ( + "fmt" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/auth/exported" + "github.com/cosmos/cosmos-sdk/x/auth/keeper" + "github.com/cosmos/cosmos-sdk/x/auth/types" + + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" +) + +var ( + _ FeeTx = (*types.StdTx)(nil) // assert StdTx implements FeeTx +) + +// FeeTx defines the interface to be implemented by Tx to use the FeeDecorators +type FeeTx interface { + sdk.Tx + GetGas() uint64 + GetFee() sdk.Coins + FeePayer() sdk.AccAddress +} + +// MempoolFeeDecorator will check if the transaction's fee is at least as large +// as the local validator's minimum gasFee (defined in validator config). +// If fee is too low, decorator returns error and tx is rejected from mempool. +// Note this only applies when ctx.CheckTx = true +// If fee is high enough or not CheckTx, then call next AnteHandler +// CONTRACT: Tx must implement FeeTx to use MempoolFeeDecorator +type MempoolFeeDecorator struct{} + +func NewMempoolFeeDecorator() MempoolFeeDecorator { + return MempoolFeeDecorator{} +} + +func (mfd MempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { + feeTx, ok := tx.(FeeTx) + if !ok { + return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") + } + feeCoins := feeTx.GetFee() + gas := feeTx.GetGas() + + // Ensure that the provided fees meet a minimum threshold for the validator, + // if this is a CheckTx. This is only for local mempool purposes, and thus + // is only ran on check tx. + if ctx.IsCheckTx() && !simulate { + minGasPrices := ctx.MinGasPrices() + if !minGasPrices.IsZero() { + requiredFees := make(sdk.Coins, len(minGasPrices)) + + // Determine the required fees by multiplying each required minimum gas + // price by the gas limit, where fee = ceil(minGasPrice * gasLimit). + glDec := sdk.NewDec(int64(gas)) + for i, gp := range minGasPrices { + fee := gp.Amount.Mul(glDec) + requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt()) + } + + if !feeCoins.IsAnyGTE(requiredFees) { + return ctx, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) + } + } + } + + return next(ctx, tx, simulate) +} + +// DeductFeeDecorator deducts fees from the first signer of the tx +// If the first signer does not have the funds to pay for the fees, return with InsufficientFunds error +// Call next AnteHandler if fees successfully deducted +// CONTRACT: Tx must implement FeeTx interface to use DeductFeeDecorator +type DeductFeeDecorator struct { + ak keeper.AccountKeeper + supplyKeeper types.SupplyKeeper +} + +func NewDeductFeeDecorator(ak keeper.AccountKeeper, sk types.SupplyKeeper) DeductFeeDecorator { + return DeductFeeDecorator{ + ak: ak, + supplyKeeper: sk, + } +} + +func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { + feeTx, ok := tx.(FeeTx) + if !ok { + return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") + } + + if addr := dfd.supplyKeeper.GetModuleAddress(types.FeeCollectorName); addr == nil { + panic(fmt.Sprintf("%s module account has not been set", types.FeeCollectorName)) + } + + feePayer := feeTx.FeePayer() + feePayerAcc := dfd.ak.GetAccount(ctx, feePayer) + + // deduct the fees + if !feeTx.GetFee().IsZero() { + err = DeductFees(dfd.supplyKeeper, ctx, feePayerAcc, feeTx.GetFee()) + if err != nil { + return ctx, err + } + } + + return next(ctx, tx, simulate) +} + +// DeductFees deducts fees from the given account. +// +// NOTE: We could use the BankKeeper (in addition to the AccountKeeper, because +// the BankKeeper doesn't give us accounts), but it seems easier to do this. +func DeductFees(supplyKeeper types.SupplyKeeper, ctx sdk.Context, acc exported.Account, fees sdk.Coins) error { + blockTime := ctx.BlockHeader().Time + coins := acc.GetCoins() + + if !fees.IsValid() { + return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "invalid fee amount: %s", fees) + } + + // verify the account has enough funds to pay for fees + _, hasNeg := coins.SafeSub(fees) + if hasNeg { + return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, + "insufficient funds to pay for fees; %s < %s", coins, fees) + } + + // Validate the account has enough "spendable" coins as this will cover cases + // such as vesting accounts. + spendableCoins := acc.SpendableCoins(blockTime) + if _, hasNeg := spendableCoins.SafeSub(fees); hasNeg { + return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, + "insufficient funds to pay for fees; %s < %s", spendableCoins, fees) + } + + err := supplyKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.FeeCollectorName, fees) + if err != nil { + return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) + } + + return nil +} diff --git a/x/auth/ante/fee_test.go b/x/auth/ante/fee_test.go new file mode 100644 index 000000000000..b088c1961951 --- /dev/null +++ b/x/auth/ante/fee_test.go @@ -0,0 +1,98 @@ +package ante_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/crypto" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/auth/ante" + "github.com/cosmos/cosmos-sdk/x/auth/types" +) + +func TestEnsureMempoolFees(t *testing.T) { + // setup + _, ctx := createTestApp(true) + + mfd := ante.NewMempoolFeeDecorator() + antehandler := sdk.ChainAnteDecorators(mfd) + + // keys and addresses + priv1, _, addr1 := types.KeyTestPubAddr() + + // msg and signatures + msg1 := types.NewTestMsg(addr1) + fee := types.NewTestStdFee() + + msgs := []sdk.Msg{msg1} + + privs, accNums, seqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} + tx := types.NewTestTx(ctx, msgs, privs, accNums, seqs, fee) + + // Set high gas price so standard test fee fails + atomPrice := sdk.NewDecCoinFromDec("atom", sdk.NewDec(200).Quo(sdk.NewDec(100000))) + highGasPrice := []sdk.DecCoin{atomPrice} + ctx = ctx.WithMinGasPrices(highGasPrice) + + // Set IsCheckTx to true + ctx = ctx.WithIsCheckTx(true) + + // antehandler errors with insufficient fees + _, err := antehandler(ctx, tx, false) + require.NotNil(t, err, "Decorator should have errored on too low fee for local gasPrice") + + // Set IsCheckTx to false + ctx = ctx.WithIsCheckTx(false) + + // antehandler should not error since we do not check minGasPrice in DeliverTx + _, err = antehandler(ctx, tx, false) + require.Nil(t, err, "MempoolFeeDecorator returned error in DeliverTx") + + // Set IsCheckTx back to true for testing sufficient mempool fee + ctx = ctx.WithIsCheckTx(true) + + atomPrice = sdk.NewDecCoinFromDec("atom", sdk.NewDec(0).Quo(sdk.NewDec(100000))) + lowGasPrice := []sdk.DecCoin{atomPrice} + ctx = ctx.WithMinGasPrices(lowGasPrice) + + _, err = antehandler(ctx, tx, false) + require.Nil(t, err, "Decorator should not have errored on fee higher than local gasPrice") +} + +func TestDeductFees(t *testing.T) { + // setup + app, ctx := createTestApp(true) + + // keys and addresses + priv1, _, addr1 := types.KeyTestPubAddr() + + // msg and signatures + msg1 := types.NewTestMsg(addr1) + fee := types.NewTestStdFee() + + msgs := []sdk.Msg{msg1} + + privs, accNums, seqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} + tx := types.NewTestTx(ctx, msgs, privs, accNums, seqs, fee) + + // Set account with insufficient funds + acc := app.AccountKeeper.NewAccountWithAddress(ctx, addr1) + acc.SetCoins([]sdk.Coin{sdk.NewCoin("atom", sdk.NewInt(10))}) + app.AccountKeeper.SetAccount(ctx, acc) + + dfd := ante.NewDeductFeeDecorator(app.AccountKeeper, app.SupplyKeeper) + antehandler := sdk.ChainAnteDecorators(dfd) + + _, err := antehandler(ctx, tx, false) + + require.NotNil(t, err, "Tx did not error when fee payer had insufficient funds") + + // Set account with sufficient funds + acc.SetCoins([]sdk.Coin{sdk.NewCoin("atom", sdk.NewInt(200))}) + app.AccountKeeper.SetAccount(ctx, acc) + + _, err = antehandler(ctx, tx, false) + + require.Nil(t, err, "Tx errored after account has been set with sufficient funds") +} diff --git a/x/auth/ante/setup.go b/x/auth/ante/setup.go new file mode 100644 index 000000000000..1e3e8eb68d67 --- /dev/null +++ b/x/auth/ante/setup.go @@ -0,0 +1,76 @@ +package ante + +import ( + "fmt" + + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/cosmos-sdk/x/auth/types" +) + +var ( + _ GasTx = (*types.StdTx)(nil) // assert StdTx implements GasTx +) + +// GasTx defines a Tx with a GetGas() method which is needed to use SetUpContextDecorator +type GasTx interface { + sdk.Tx + GetGas() uint64 +} + +// SetUpContextDecorator sets the GasMeter in the Context and wraps the next AnteHandler with a defer clause +// to recover from any downstream OutOfGas panics in the AnteHandler chain to return an error with information +// on gas provided and gas used. +// CONTRACT: Must be first decorator in the chain +// CONTRACT: Tx must implement GasTx interface +type SetUpContextDecorator struct{} + +func NewSetUpContextDecorator() SetUpContextDecorator { + return SetUpContextDecorator{} +} + +func (sud SetUpContextDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { + // all transactions must implement GasTx + gasTx, ok := tx.(GasTx) + if !ok { + // Set a gas meter with limit 0 as to prevent an infinite gas meter attack + // during runTx. + newCtx = SetGasMeter(simulate, ctx, 0) + return newCtx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be GasTx") + } + + newCtx = SetGasMeter(simulate, ctx, gasTx.GetGas()) + + // Decorator will catch an OutOfGasPanic caused in the next antehandler + // AnteHandlers must have their own defer/recover in order for the BaseApp + // to know how much gas was used! This is because the GasMeter is created in + // the AnteHandler, but if it panics the context won't be set properly in + // runTx's recover call. + defer func() { + if r := recover(); r != nil { + switch rType := r.(type) { + case sdk.ErrorOutOfGas: + log := fmt.Sprintf( + "out of gas in location: %v; gasWanted: %d, gasUsed: %d", + rType.Descriptor, gasTx.GetGas(), newCtx.GasMeter().GasConsumed()) + + err = sdkerrors.Wrap(sdkerrors.ErrOutOfGas, log) + default: + panic(r) + } + } + }() + + return next(newCtx, tx, simulate) +} + +// SetGasMeter returns a new context with a gas meter set from a given context. +func SetGasMeter(simulate bool, ctx sdk.Context, gasLimit uint64) sdk.Context { + // In various cases such as simulation and during the genesis block, we do not + // meter any gas utilization. + if simulate || ctx.BlockHeight() == 0 { + return ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) + } + + return ctx.WithGasMeter(sdk.NewGasMeter(gasLimit)) +} diff --git a/x/auth/ante/setup_test.go b/x/auth/ante/setup_test.go new file mode 100644 index 000000000000..7b7352dbbd26 --- /dev/null +++ b/x/auth/ante/setup_test.go @@ -0,0 +1,97 @@ +package ante_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/crypto" + + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/cosmos-sdk/x/auth/ante" + "github.com/cosmos/cosmos-sdk/x/auth/types" +) + +func TestSetup(t *testing.T) { + // setup + _, ctx := createTestApp(true) + + // keys and addresses + priv1, _, addr1 := types.KeyTestPubAddr() + + // msg and signatures + msg1 := types.NewTestMsg(addr1) + fee := types.NewTestStdFee() + + msgs := []sdk.Msg{msg1} + + privs, accNums, seqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} + tx := types.NewTestTx(ctx, msgs, privs, accNums, seqs, fee) + + sud := ante.NewSetUpContextDecorator() + antehandler := sdk.ChainAnteDecorators(sud) + + // Set height to non-zero value for GasMeter to be set + ctx = ctx.WithBlockHeight(1) + + // Context GasMeter Limit not set + require.Equal(t, uint64(0), ctx.GasMeter().Limit(), "GasMeter set with limit before setup") + + newCtx, err := antehandler(ctx, tx, false) + require.Nil(t, err, "SetUpContextDecorator returned error") + + // Context GasMeter Limit should be set after SetUpContextDecorator runs + require.Equal(t, fee.Gas, newCtx.GasMeter().Limit(), "GasMeter not set correctly") +} + +func TestRecoverPanic(t *testing.T) { + // setup + _, ctx := createTestApp(true) + + // keys and addresses + priv1, _, addr1 := types.KeyTestPubAddr() + + // msg and signatures + msg1 := types.NewTestMsg(addr1) + fee := types.NewTestStdFee() + + msgs := []sdk.Msg{msg1} + + privs, accNums, seqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} + tx := types.NewTestTx(ctx, msgs, privs, accNums, seqs, fee) + + sud := ante.NewSetUpContextDecorator() + antehandler := sdk.ChainAnteDecorators(sud, OutOfGasDecorator{}) + + // Set height to non-zero value for GasMeter to be set + ctx = ctx.WithBlockHeight(1) + + newCtx, err := antehandler(ctx, tx, false) + + require.NotNil(t, err, "Did not return error on OutOfGas panic") + + require.True(t, sdkerrors.ErrOutOfGas.Is(err), "Returned error is not an out of gas error") + require.Equal(t, fee.Gas, newCtx.GasMeter().Limit()) + + antehandler = sdk.ChainAnteDecorators(sud, PanicDecorator{}) + require.Panics(t, func() { antehandler(ctx, tx, false) }, "Recovered from non-Out-of-Gas panic") +} + +type OutOfGasDecorator struct{} + +// AnteDecorator that will throw OutOfGas panic +func (ogd OutOfGasDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { + overLimit := ctx.GasMeter().Limit() + 1 + + // Should panic with outofgas error + ctx.GasMeter().ConsumeGas(overLimit, "test panic") + + // not reached + return next(ctx, tx, simulate) +} + +type PanicDecorator struct{} + +func (pd PanicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { + panic("random error") +} diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go new file mode 100644 index 000000000000..4b340907e1e8 --- /dev/null +++ b/x/auth/ante/sigverify.go @@ -0,0 +1,345 @@ +package ante + +import ( + "bytes" + "encoding/hex" + + "github.com/tendermint/tendermint/crypto" + "github.com/tendermint/tendermint/crypto/ed25519" + "github.com/tendermint/tendermint/crypto/multisig" + "github.com/tendermint/tendermint/crypto/secp256k1" + + "github.com/cosmos/cosmos-sdk/codec" + sdk "github.com/cosmos/cosmos-sdk/types" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/cosmos-sdk/x/auth/exported" + "github.com/cosmos/cosmos-sdk/x/auth/keeper" + "github.com/cosmos/cosmos-sdk/x/auth/types" +) + +var ( + // simulation signature values used to estimate gas consumption + simSecp256k1Pubkey secp256k1.PubKeySecp256k1 + simSecp256k1Sig [64]byte + + _ SigVerifiableTx = (*types.StdTx)(nil) // assert StdTx implements SigVerifiableTx +) + +func init() { + // This decodes a valid hex string into a sepc256k1Pubkey for use in transaction simulation + bz, _ := hex.DecodeString("035AD6810A47F073553FF30D2FCC7E0D3B1C0B74B61A1AAA2582344037151E143A") + copy(simSecp256k1Pubkey[:], bz) +} + +// SignatureVerificationGasConsumer is the type of function that is used to both +// consume gas when verifying signatures and also to accept or reject different types of pubkeys +// This is where apps can define their own PubKey +type SignatureVerificationGasConsumer = func(meter sdk.GasMeter, sig []byte, pubkey crypto.PubKey, params types.Params) error + +// SigVerifiableTx defines a Tx interface for all signature verification decorators +type SigVerifiableTx interface { + sdk.Tx + GetSignatures() [][]byte + GetSigners() []sdk.AccAddress + GetPubKeys() []crypto.PubKey // If signer already has pubkey in context, this list will have nil in its place + GetSignBytes(ctx sdk.Context, acc exported.Account) []byte +} + +// SetPubKeyDecorator sets PubKeys in context for any signer which does not already have pubkey set +// PubKeys must be set in context for all signers before any other sigverify decorators run +// CONTRACT: Tx must implement SigVerifiableTx interface +type SetPubKeyDecorator struct { + ak keeper.AccountKeeper +} + +func NewSetPubKeyDecorator(ak keeper.AccountKeeper) SetPubKeyDecorator { + return SetPubKeyDecorator{ + ak: ak, + } +} + +func (spkd SetPubKeyDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { + if simulate { + return next(ctx, tx, simulate) + } + sigTx, ok := tx.(SigVerifiableTx) + if !ok { + return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "invalid tx type") + } + + pubkeys := sigTx.GetPubKeys() + signers := sigTx.GetSigners() + + for i, pk := range pubkeys { + // PublicKey was omitted from slice since it has already been set in context + if pk == nil { + continue + } + if !bytes.Equal(pk.Address(), signers[i]) { + return ctx, sdkerrors.Wrapf(sdkerrors.ErrInvalidPubKey, + "pubKey does not match signer address %s with signer index: %d", signers[i], i) + } + + acc, err := GetSignerAcc(ctx, spkd.ak, signers[i]) + if err != nil { + return ctx, err + } + err = acc.SetPubKey(pk) + if err != nil { + return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidPubKey, err.Error()) + } + spkd.ak.SetAccount(ctx, acc) + } + + return next(ctx, tx, simulate) +} + +// Consume parameter-defined amount of gas for each signature according to the passed-in SignatureVerificationGasConsumer function +// before calling the next AnteHandler +// CONTRACT: Pubkeys are set in context for all signers before this decorator runs +// CONTRACT: Tx must implement SigVerifiableTx interface +type SigGasConsumeDecorator struct { + ak keeper.AccountKeeper + sigGasConsumer SignatureVerificationGasConsumer +} + +func NewSigGasConsumeDecorator(ak keeper.AccountKeeper, sigGasConsumer SignatureVerificationGasConsumer) SigGasConsumeDecorator { + return SigGasConsumeDecorator{ + ak: ak, + sigGasConsumer: sigGasConsumer, + } +} + +func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { + sigTx, ok := tx.(SigVerifiableTx) + if !ok { + return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type") + } + + params := sgcd.ak.GetParams(ctx) + sigs := sigTx.GetSignatures() + + // stdSigs contains the sequence number, account number, and signatures. + // When simulating, this would just be a 0-length slice. + signerAddrs := sigTx.GetSigners() + + for i, sig := range sigs { + signerAcc, err := GetSignerAcc(ctx, sgcd.ak, signerAddrs[i]) + if err != nil { + return ctx, err + } + pubKey := signerAcc.GetPubKey() + + if simulate { + // Simulated txs should not contain a signature and are not required to + // contain a pubkey, so we must account for tx size of including a + // StdSignature (Amino encoding) and simulate gas consumption + // (assuming a SECP256k1 simulation key). + consumeSimSigGas(ctx.GasMeter(), pubKey, sig, params) + } else { + err = sgcd.sigGasConsumer(ctx.GasMeter(), sig, pubKey, params) + if err != nil { + return ctx, err + } + } + } + + return next(ctx, tx, simulate) +} + +// Verify all signatures for tx and return error if any are invalid +// increment sequence of each signer and set updated account back in store +// Call next AnteHandler +// CONTRACT: Pubkeys are set in context for all signers before this decorator runs +// CONTRACT: Tx must implement SigVerifiableTx interface +type SigVerificationDecorator struct { + ak keeper.AccountKeeper +} + +func NewSigVerificationDecorator(ak keeper.AccountKeeper) SigVerificationDecorator { + return SigVerificationDecorator{ + ak: ak, + } +} + +func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { + sigTx, ok := tx.(SigVerifiableTx) + if !ok { + return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type") + } + + // stdSigs contains the sequence number, account number, and signatures. + // When simulating, this would just be a 0-length slice. + sigs := sigTx.GetSignatures() + + // stdSigs contains the sequence number, account number, and signatures. + // When simulating, this would just be a 0-length slice. + signerAddrs := sigTx.GetSigners() + signerAccs := make([]exported.Account, len(signerAddrs)) + + // check that signer length and signature length are the same + if len(sigs) != len(signerAddrs) { + return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "invalid number of signer; expected: %d, got %d", len(signerAddrs), len(sigs)) + } + + for i, sig := range sigs { + signerAccs[i], err = GetSignerAcc(ctx, svd.ak, signerAddrs[i]) + if err != nil { + return ctx, err + } + + // retrieve signBytes of tx + signBytes := sigTx.GetSignBytes(ctx, signerAccs[i]) + + // retrieve pubkey + pubKey := signerAccs[i].GetPubKey() + if !simulate && pubKey == nil { + return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidPubKey, "pubkey on account is not set") + } + + // verify signature + if !simulate && !pubKey.VerifyBytes(signBytes, sig) { + return ctx, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "signature verification failed; verify correct account sequence and chain-id") + } + } + + return next(ctx, tx, simulate) +} + +// Increments sequences of all signers. +// Use this decorator to prevent replay attacks +// CONTRACT: Tx must implement SigVerifiableTx interface +type IncrementSequenceDecorator struct { + ak keeper.AccountKeeper +} + +func NewIncrementSequenceDecorator(ak keeper.AccountKeeper) IncrementSequenceDecorator { + return IncrementSequenceDecorator{ + ak: ak, + } +} + +func (isd IncrementSequenceDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { + sigTx, ok := tx.(SigVerifiableTx) + if !ok { + return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type") + } + + // increment sequence of all signers + for _, addr := range sigTx.GetSigners() { + acc := isd.ak.GetAccount(ctx, addr) + if err := acc.SetSequence(acc.GetSequence() + 1); err != nil { + panic(err) + } + isd.ak.SetAccount(ctx, acc) + } + + return next(ctx, tx, simulate) +} + +// ValidateSigCountDecorator takes in Params and returns errors if there are too many signatures in the tx for the given params +// otherwise it calls next AnteHandler +// Use this decorator to set parameterized limit on number of signatures in tx +// CONTRACT: Tx must implement SigVerifiableTx interface +type ValidateSigCountDecorator struct { + ak keeper.AccountKeeper +} + +func NewValidateSigCountDecorator(ak keeper.AccountKeeper) ValidateSigCountDecorator { + return ValidateSigCountDecorator{ + ak: ak, + } +} + +func (vscd ValidateSigCountDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { + sigTx, ok := tx.(SigVerifiableTx) + if !ok { + return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a sigTx") + } + + params := vscd.ak.GetParams(ctx) + pubKeys := sigTx.GetPubKeys() + + sigCount := 0 + for _, pk := range pubKeys { + sigCount += types.CountSubKeys(pk) + if uint64(sigCount) > params.TxSigLimit { + return ctx, sdkerrors.Wrapf(sdkerrors.ErrTooManySignatures, + "signatures: %d, limit: %d", sigCount, params.TxSigLimit) + } + } + + return next(ctx, tx, simulate) +} + +// DefaultSigVerificationGasConsumer is the default implementation of SignatureVerificationGasConsumer. It consumes gas +// for signature verification based upon the public key type. The cost is fetched from the given params and is matched +// by the concrete type. +func DefaultSigVerificationGasConsumer( + meter sdk.GasMeter, sig []byte, pubkey crypto.PubKey, params types.Params, +) error { + switch pubkey := pubkey.(type) { + case ed25519.PubKeyEd25519: + meter.ConsumeGas(params.SigVerifyCostED25519, "ante verify: ed25519") + return sdkerrors.Wrap(sdkerrors.ErrInvalidPubKey, "ED25519 public keys are unsupported") + + case secp256k1.PubKeySecp256k1: + meter.ConsumeGas(params.SigVerifyCostSecp256k1, "ante verify: secp256k1") + return nil + + case multisig.PubKeyMultisigThreshold: + var multisignature multisig.Multisignature + codec.Cdc.MustUnmarshalBinaryBare(sig, &multisignature) + + consumeMultisignatureVerificationGas(meter, multisignature, pubkey, params) + return nil + + default: + return sdkerrors.Wrapf(sdkerrors.ErrInvalidPubKey, "unrecognized public key type: %T", pubkey) + } +} + +func consumeMultisignatureVerificationGas(meter sdk.GasMeter, + sig multisig.Multisignature, pubkey multisig.PubKeyMultisigThreshold, + params types.Params) { + size := sig.BitArray.Size() + sigIndex := 0 + for i := 0; i < size; i++ { + if sig.BitArray.GetIndex(i) { + DefaultSigVerificationGasConsumer(meter, sig.Sigs[sigIndex], pubkey.PubKeys[i], params) + sigIndex++ + } + } +} + +// Internal function that simulates gas consumption of signature verification when simulate=true +// TODO: allow users to simulate signatures other than auth.StdSignature +func consumeSimSigGas(gasmeter sdk.GasMeter, pubkey crypto.PubKey, sig []byte, params types.Params) { + simSig := types.StdSignature{ + Signature: sig, + PubKey: pubkey, + } + if len(sig) == 0 { + simSig.Signature = simSecp256k1Sig[:] + } + + sigBz := types.ModuleCdc.MustMarshalBinaryLengthPrefixed(simSig) + cost := sdk.Gas(len(sigBz) + 6) + + // If the pubkey is a multi-signature pubkey, then we estimate for the maximum + // number of signers. + if _, ok := pubkey.(multisig.PubKeyMultisigThreshold); ok { + cost *= params.TxSigLimit + } + + gasmeter.ConsumeGas(params.TxSizeCostPerByte*cost, "txSize") +} + +// GetSignerAcc returns an account for a given address that is expected to sign +// a transaction. +func GetSignerAcc(ctx sdk.Context, ak keeper.AccountKeeper, addr sdk.AccAddress) (exported.Account, error) { + if acc := ak.GetAccount(ctx, addr); acc != nil { + return acc, nil + } + return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownAddress, "account %s does not exist", addr) +} diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go new file mode 100644 index 000000000000..82b9f8f62443 --- /dev/null +++ b/x/auth/ante/sigverify_test.go @@ -0,0 +1,186 @@ +package ante_test + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/crypto" + "github.com/tendermint/tendermint/crypto/ed25519" + "github.com/tendermint/tendermint/crypto/multisig" + "github.com/tendermint/tendermint/crypto/secp256k1" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/auth/ante" + "github.com/cosmos/cosmos-sdk/x/auth/types" +) + +func TestSetPubKey(t *testing.T) { + // setup + app, ctx := createTestApp(true) + + // keys and addresses + priv1, pub1, addr1 := types.KeyTestPubAddr() + priv2, pub2, addr2 := types.KeyTestPubAddr() + priv3, pub3, addr3 := types.KeyTestPubAddr() + + addrs := []sdk.AccAddress{addr1, addr2, addr3} + pubs := []crypto.PubKey{pub1, pub2, pub3} + + msgs := make([]sdk.Msg, len(addrs)) + // set accounts and create msg for each address + for i, addr := range addrs { + acc := app.AccountKeeper.NewAccountWithAddress(ctx, addr) + require.NoError(t, acc.SetAccountNumber(uint64(i))) + app.AccountKeeper.SetAccount(ctx, acc) + msgs[i] = types.NewTestMsg(addr) + } + + fee := types.NewTestStdFee() + + privs, accNums, seqs := []crypto.PrivKey{priv1, priv2, priv3}, []uint64{0, 1, 2}, []uint64{0, 0, 0} + tx := types.NewTestTx(ctx, msgs, privs, accNums, seqs, fee) + + spkd := ante.NewSetPubKeyDecorator(app.AccountKeeper) + antehandler := sdk.ChainAnteDecorators(spkd) + + ctx, err := antehandler(ctx, tx, false) + require.Nil(t, err) + + // Require that all accounts have pubkey set after Decorator runs + for i, addr := range addrs { + pk, err := app.AccountKeeper.GetPubKey(ctx, addr) + require.Nil(t, err, "Error on retrieving pubkey from account") + require.Equal(t, pubs[i], pk, "Pubkey retrieved from account is unexpected") + } +} + +func TestConsumeSignatureVerificationGas(t *testing.T) { + params := types.DefaultParams() + msg := []byte{1, 2, 3, 4} + + pkSet1, sigSet1 := generatePubKeysAndSignatures(5, msg, false) + multisigKey1 := multisig.NewPubKeyMultisigThreshold(2, pkSet1) + multisignature1 := multisig.NewMultisig(len(pkSet1)) + expectedCost1 := expectedGasCostByKeys(pkSet1) + for i := 0; i < len(pkSet1); i++ { + multisignature1.AddSignatureFromPubKey(sigSet1[i], pkSet1[i], pkSet1) + } + + type args struct { + meter sdk.GasMeter + sig []byte + pubkey crypto.PubKey + params types.Params + } + tests := []struct { + name string + args args + gasConsumed uint64 + shouldErr bool + }{ + {"PubKeyEd25519", args{sdk.NewInfiniteGasMeter(), nil, ed25519.GenPrivKey().PubKey(), params}, types.DefaultSigVerifyCostED25519, true}, + {"PubKeySecp256k1", args{sdk.NewInfiniteGasMeter(), nil, secp256k1.GenPrivKey().PubKey(), params}, types.DefaultSigVerifyCostSecp256k1, false}, + {"Multisig", args{sdk.NewInfiniteGasMeter(), multisignature1.Marshal(), multisigKey1, params}, expectedCost1, false}, + {"unknown key", args{sdk.NewInfiniteGasMeter(), nil, nil, params}, 0, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ante.DefaultSigVerificationGasConsumer(tt.args.meter, tt.args.sig, tt.args.pubkey, tt.args.params) + + if tt.shouldErr { + require.NotNil(t, err) + } else { + require.Nil(t, err) + require.Equal(t, tt.gasConsumed, tt.args.meter.GasConsumed(), fmt.Sprintf("%d != %d", tt.gasConsumed, tt.args.meter.GasConsumed())) + } + }) + } +} + +func TestSigVerification(t *testing.T) { + // setup + app, ctx := createTestApp(true) + + // keys and addresses + priv1, _, addr1 := types.KeyTestPubAddr() + priv2, _, addr2 := types.KeyTestPubAddr() + priv3, _, addr3 := types.KeyTestPubAddr() + + addrs := []sdk.AccAddress{addr1, addr2, addr3} + + msgs := make([]sdk.Msg, len(addrs)) + // set accounts and create msg for each address + for i, addr := range addrs { + acc := app.AccountKeeper.NewAccountWithAddress(ctx, addr) + require.NoError(t, acc.SetAccountNumber(uint64(i))) + app.AccountKeeper.SetAccount(ctx, acc) + msgs[i] = types.NewTestMsg(addr) + } + + fee := types.NewTestStdFee() + + privs, accNums, seqs := []crypto.PrivKey{priv3, priv2, priv1}, []uint64{2, 1, 0}, []uint64{0, 0, 0} + tx := types.NewTestTx(ctx, msgs, privs, accNums, seqs, fee) + + spkd := ante.NewSetPubKeyDecorator(app.AccountKeeper) + svd := ante.NewSigVerificationDecorator(app.AccountKeeper) + antehandler := sdk.ChainAnteDecorators(spkd, svd) + + _, err := antehandler(ctx, tx, false) + require.NotNil(t, err) +} + +func TestSigIntegration(t *testing.T) { + // generate private keys + privs := []crypto.PrivKey{secp256k1.GenPrivKey(), secp256k1.GenPrivKey(), secp256k1.GenPrivKey()} + + params := types.DefaultParams() + initialSigCost := params.SigVerifyCostSecp256k1 + initialCost, err := runSigDecorators(t, params, false, privs...) + require.Nil(t, err) + + params.SigVerifyCostSecp256k1 *= 2 + doubleCost, err := runSigDecorators(t, params, false, privs...) + require.Nil(t, err) + + require.Equal(t, initialSigCost*uint64(len(privs)), doubleCost-initialCost) +} + +func runSigDecorators(t *testing.T, params types.Params, multisig bool, privs ...crypto.PrivKey) (sdk.Gas, error) { + // setup + app, ctx := createTestApp(true) + // Make block-height non-zero to include accNum in SignBytes + ctx = ctx.WithBlockHeight(1) + app.AccountKeeper.SetParams(ctx, params) + + msgs := make([]sdk.Msg, len(privs)) + accNums := make([]uint64, len(privs)) + seqs := make([]uint64, len(privs)) + // set accounts and create msg for each address + for i, priv := range privs { + addr := sdk.AccAddress(priv.PubKey().Address()) + acc := app.AccountKeeper.NewAccountWithAddress(ctx, addr) + require.NoError(t, acc.SetAccountNumber(uint64(i))) + app.AccountKeeper.SetAccount(ctx, acc) + msgs[i] = types.NewTestMsg(addr) + accNums[i] = uint64(i) + seqs[i] = uint64(0) + } + + fee := types.NewTestStdFee() + + tx := types.NewTestTx(ctx, msgs, privs, accNums, seqs, fee) + + spkd := ante.NewSetPubKeyDecorator(app.AccountKeeper) + svgc := ante.NewSigGasConsumeDecorator(app.AccountKeeper, ante.DefaultSigVerificationGasConsumer) + svd := ante.NewSigVerificationDecorator(app.AccountKeeper) + antehandler := sdk.ChainAnteDecorators(spkd, svgc, svd) + + // Determine gas consumption of antehandler with default params + before := ctx.GasMeter().GasConsumed() + ctx, err := antehandler(ctx, tx, false) + after := ctx.GasMeter().GasConsumed() + + return after - before, err +} diff --git a/x/auth/client/cli/tx_sign.go b/x/auth/client/cli/tx_sign.go index 95dd0b08d36e..d4af880d3e91 100644 --- a/x/auth/client/cli/tx_sign.go +++ b/x/auth/client/cli/tx_sign.go @@ -196,7 +196,7 @@ func printAndValidateSigs( } success := true - sigs := stdTx.GetSignatures() + sigs := stdTx.Signatures fmt.Println("") fmt.Println("Signatures:") diff --git a/x/auth/types/stdtx.go b/x/auth/types/stdtx.go index 945d9afd8695..fa8b5d100d5f 100644 --- a/x/auth/types/stdtx.go +++ b/x/auth/types/stdtx.go @@ -10,6 +10,7 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/auth/exported" ) var ( @@ -97,7 +98,6 @@ func (tx StdTx) GetSigners() []sdk.AccAddress { // GetMemo returns the memo func (tx StdTx) GetMemo() string { return tx.Memo } -// GetSignatures returns the signature of signers who signed the Msg. // GetSignatures returns the signature of signers who signed the Msg. // CONTRACT: Length returned is same as length of // pubkeys returned from MsgKeySigners, and the order @@ -105,7 +105,53 @@ func (tx StdTx) GetMemo() string { return tx.Memo } // CONTRACT: If the signature is missing (ie the Msg is // invalid), then the corresponding signature is // .Empty(). -func (tx StdTx) GetSignatures() []StdSignature { return tx.Signatures } +func (tx StdTx) GetSignatures() [][]byte { + sigs := make([][]byte, len(tx.Signatures)) + for i, stdSig := range tx.Signatures { + sigs[i] = stdSig.Signature + } + return sigs +} + +// GetPubkeys returns the pubkeys of signers if the pubkey is included in the signature +// If pubkey is not included in the signature, then nil is in the slice instead +func (tx StdTx) GetPubKeys() []crypto.PubKey { + pks := make([]crypto.PubKey, len(tx.Signatures)) + for i, stdSig := range tx.Signatures { + pks[i] = stdSig.PubKey + } + return pks +} + +// GetSignBytes returns the signBytes of the tx for a given signer +func (tx StdTx) GetSignBytes(ctx sdk.Context, acc exported.Account) []byte { + genesis := ctx.BlockHeight() == 0 + chainID := ctx.ChainID() + var accNum uint64 + if !genesis { + accNum = acc.GetAccountNumber() + } + + return StdSignBytes( + chainID, accNum, acc.GetSequence(), tx.Fee, tx.Msgs, tx.Memo, + ) +} + +// GetGas returns the Gas in StdFee +func (tx StdTx) GetGas() uint64 { return tx.Fee.Gas } + +// GetFee returns the FeeAmount in StdFee +func (tx StdTx) GetFee() sdk.Coins { return tx.Fee.Amount } + +// FeePayer returns the address that is responsible for paying fee +// StdTx returns the first signer as the fee payer +// If no signers for tx, return empty address +func (tx StdTx) FeePayer() sdk.AccAddress { + if tx.GetSigners() != nil { + return tx.GetSigners()[0] + } + return sdk.AccAddress{} +} //__________________________________________________________ diff --git a/x/auth/types/stdtx_test.go b/x/auth/types/stdtx_test.go index 661db3fd4752..375a15c49cc4 100644 --- a/x/auth/types/stdtx_test.go +++ b/x/auth/types/stdtx_test.go @@ -27,7 +27,7 @@ func TestStdTx(t *testing.T) { tx := NewStdTx(msgs, fee, sigs, "") require.Equal(t, msgs, tx.GetMsgs()) - require.Equal(t, sigs, tx.GetSignatures()) + require.Equal(t, sigs, tx.Signatures) feePayer := tx.GetSigners()[0] require.Equal(t, addr, feePayer) @@ -49,7 +49,7 @@ func TestStdSignBytes(t *testing.T) { }{ { args{"1234", 3, 6, defaultFee, []sdk.Msg{sdk.NewTestMsg(addr)}, "memo"}, - fmt.Sprintf("{\"account_number\":\"3\",\"chain_id\":\"1234\",\"fee\":{\"amount\":[{\"amount\":\"150\",\"denom\":\"atom\"}],\"gas\":\"50000\"},\"memo\":\"memo\",\"msgs\":[[\"%s\"]],\"sequence\":\"6\"}", addr), + fmt.Sprintf("{\"account_number\":\"3\",\"chain_id\":\"1234\",\"fee\":{\"amount\":[{\"amount\":\"150\",\"denom\":\"atom\"}],\"gas\":\"100000\"},\"memo\":\"memo\",\"msgs\":[[\"%s\"]],\"sequence\":\"6\"}", addr), }, } for i, tc := range tests { diff --git a/x/auth/types/test_common.go b/x/auth/types/test_common.go index 0ab71393ae65..9e2ba20e98d2 100644 --- a/x/auth/types/test_common.go +++ b/x/auth/types/test_common.go @@ -13,7 +13,7 @@ func NewTestMsg(addrs ...sdk.AccAddress) *sdk.TestMsg { } func NewTestStdFee() StdFee { - return NewStdFee(50000, + return NewStdFee(100000, sdk.NewCoins(sdk.NewInt64Coin("atom", 150)), ) } diff --git a/x/auth/types/txbuilder.go b/x/auth/types/txbuilder.go index b29d977fd468..7d60086a137d 100644 --- a/x/auth/types/txbuilder.go +++ b/x/auth/types/txbuilder.go @@ -260,7 +260,7 @@ func (bldr TxBuilder) SignStdTx(name, passphrase string, stdTx StdTx, appendSig return } - sigs := stdTx.GetSignatures() + sigs := stdTx.Signatures if len(sigs) == 0 || !appendSig { sigs = []StdSignature{stdSignature} } else { diff --git a/x/mock/app_test.go b/x/mock/app_test.go index 552bf2a807f7..db5e29522539 100644 --- a/x/mock/app_test.go +++ b/x/mock/app_test.go @@ -77,7 +77,8 @@ func TestCheckAndDeliverGenTx(t *testing.T) { true, false, privKeys[1], ) - require.Equal(t, sdk.CodeUnauthorized, res.Code, res.Log) + // Will fail on SetPubKey decorator + require.Equal(t, sdk.CodeInvalidPubKey, res.Code, res.Log) require.Equal(t, sdk.CodespaceRoot, res.Codespace) // Resigning the tx with the correct privKey should result in an OK result