From 196099302b17332621d7a00c4870675a4db26ceb Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 28 Aug 2019 16:00:53 -0700 Subject: [PATCH 01/32] start decorator changes --- baseapp/baseapp.go | 12 ++++++--- types/handler.go | 22 +++++++++++++++- x/auth/ante/setup.go | 46 ++++++++++++++++++++++++++++++++++ x/auth/ante/sigverification.go | 19 ++++++++++++++ 4 files changed, 94 insertions(+), 5 deletions(-) create mode 100644 x/auth/ante/setup.go create mode 100644 x/auth/ante/sigverification.go diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 836eb1dd15fa..45624454e5ca 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -990,7 +990,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 @@ -1002,10 +1002,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/types/handler.go b/types/handler.go index b978e8e51ef4..794b84584eeb 100644 --- a/types/handler.go +++ b/types/handler.go @@ -5,4 +5,24 @@ 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 element +// wrapping over the decorators further along chain and returns a single AnteHandler +// +// First element is outermost decorator, last element is innermost decorator +func ChainDecorators(chain ...AnteDecorator) AnteHandler { + 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, ChainDecorators(chain[1:]...)) + } +} diff --git a/x/auth/ante/setup.go b/x/auth/ante/setup.go new file mode 100644 index 000000000000..166c0426fdd1 --- /dev/null +++ b/x/auth/ante/setup.go @@ -0,0 +1,46 @@ +package ante + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" +) + +func SetUpDecorator(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx Context, error) { + // 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 + } + + ctx = SetGasMeter(simulate, ctx, stdTx.Fee.Gas) + + // 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, stdTx.Fee.Gas, newCtx.GasMeter().GasConsumed(), + ) + res = sdk.ErrOutOfGas(log).Result() + + res.GasWanted = stdTx.Fee.Gas + res.GasUsed = newCtx.GasMeter().GasConsumed() + + // TODO: figure out how to return Context, error so that baseapp can recover gasWanted/gasUsed + default: + panic(r) + } + } + }() + + newCtx, err = next(ctx, tx, simulate) + return +} \ No newline at end of file diff --git a/x/auth/ante/sigverification.go b/x/auth/ante/sigverification.go new file mode 100644 index 000000000000..5d89feeb7285 --- /dev/null +++ b/x/auth/ante/sigverification.go @@ -0,0 +1,19 @@ +package ante + +import ( + "encoding/hex" + + "github.com/tendermint/tendermint/crypto/secp256k1" +) + +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) +} From 12244c5480b2cff36df1a76a33de5e16e0b42bf2 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 30 Aug 2019 14:59:13 -0700 Subject: [PATCH 02/32] start replacing ante logic with decorators --- x/auth/ante/ante.go | 7 +- x/auth/ante/basic.go | 1 + x/auth/ante/fee.go | 110 +++++++++++++++++++++ x/auth/ante/setup.go | 12 +-- x/auth/ante/sigverify.go | 200 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 319 insertions(+), 11 deletions(-) create mode 100644 x/auth/ante/basic.go create mode 100644 x/auth/ante/fee.go create mode 100644 x/auth/ante/sigverify.go diff --git a/x/auth/ante/ante.go b/x/auth/ante/ante.go index 457dba893f7b..820552b792c5 100644 --- a/x/auth/ante/ante.go +++ b/x/auth/ante/ante.go @@ -13,6 +13,7 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" + errs "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" @@ -158,11 +159,11 @@ func NewAnteHandler(ak keeper.AccountKeeper, supplyKeeper types.SupplyKeeper, si // 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) { +func GetSignerAcc(ctx sdk.Context, ak keeper.AccountKeeper, addr sdk.AccAddress) (exported.Account, error) { if acc := ak.GetAccount(ctx, addr); acc != nil { - return acc, sdk.Result{} + return acc, nil } - return nil, sdk.ErrUnknownAddress(fmt.Sprintf("account %s does not exist", addr)).Result() + return nil, errs.Wrapf(errs.ErrUnknownAddress, "account %s does not exist", addr) } // ValidateSigCount validates that the transaction has a valid cumulative total diff --git a/x/auth/ante/basic.go b/x/auth/ante/basic.go new file mode 100644 index 000000000000..2aa954f07058 --- /dev/null +++ b/x/auth/ante/basic.go @@ -0,0 +1 @@ +package ante diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go new file mode 100644 index 000000000000..b67975030544 --- /dev/null +++ b/x/auth/ante/fee.go @@ -0,0 +1,110 @@ +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" + + errs "github.com/cosmos/cosmos-sdk/types/errors" +) + +func MempoolFeeDecorator(ctx Context, tx Tx, simulate bool, next AnteHandler) (newCtx Context, err error) { + stdTx, ok := tx.(types.StdTx) + if !ok { + return ctx, errs.Wrap(errs.ErrInternal, "Tx must be a StdTx") + } + stdFee := stdTx.Fee + + // 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(stdFee.Gas)) + for i, gp := range minGasPrices { + fee := gp.Amount.Mul(glDec) + requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt()) + } + + return ctx, errs.Wrapf(errs.ErrInsufficientFee, "insufficient fees; got: %q required: %q", stdFee.Amount, requiredFees) + } + } + + return next(ctx, tx, simulate) +} + +func NewDeductFeeDecorator(ak keeper.AccountKeeper, supplyKeeper types.SupplyKeeper) { + return func(ctx Context, tx Tx, simulate bool, next AnteHandler) (newCtx Context, err error) { + stdTx, ok := tx.(types.StdTx) + if !ok { + return ctx, errs.Wrap(errs.ErrInternal, "Tx must be a StdTx") + } + + if addr := supplyKeeper.GetModuleAddress(types.FeeCollectorName); addr == nil { + panic(fmt.Sprintf("%s module account has not been set", types.FeeCollectorName)) + } + + // stdSigs contains the sequence number, account number, and signatures. + // When simulating, this would just be a 0-length slice. + signerAddrs := stdTx.GetSigners() + + // fetch first signer, who's going to pay the fees + feePayer, err = GetSignerAcc(newCtx, ak, signerAddrs[0]) + if err != nil { + return ctx, err + } + + // deduct the fees + if !stdTx.Fee.Amount.IsZero() { + err = DeductFees(supplyKeeper, newCtx, feePayer, stdTx.Fee.Amount) + if err != nil { + return ctx, err + } + } + + return next(ctx, tx, simulate) + } +} + +// 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) error { + blockTime := ctx.BlockHeader().Time + coins := acc.GetCoins() + + if !fees.IsValid() { + return errs.Wrapf(errs.ErrInsufficientFee, "invalid fee amount: %s", fees) + } + + // verify the account has enough funds to pay for fees + _, hasNeg := coins.SafeSub(fees) + if hasNeg { + return errs.Wrapf(errs.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 errs.Wrapf(errs.ErrInsufficientFunds, + "insufficient funds to pay for fees; %s < %s", spendableCoins, fees) + } + + err := supplyKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.FeeCollectorName, fees) + if err != nil { + return err.Result() + } + + return sdk.Result{} +} diff --git a/x/auth/ante/setup.go b/x/auth/ante/setup.go index 166c0426fdd1..c34fe02e96cd 100644 --- a/x/auth/ante/setup.go +++ b/x/auth/ante/setup.go @@ -2,6 +2,7 @@ package ante import ( sdk "github.com/cosmos/cosmos-sdk/types" + err "github.com/cosmos/cosmos-sdk/types/errors" ) func SetUpDecorator(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx Context, error) { @@ -14,7 +15,7 @@ func SetUpDecorator(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHand return newCtx, sdk.ErrInternal("tx must be StdTx").Result(), true } - ctx = SetGasMeter(simulate, ctx, stdTx.Fee.Gas) + newCtx = SetGasMeter(simulate, ctx, stdTx.Fee.Gas) // Decorator will catch an OutOfGasPanic caused in the next antehandler // AnteHandlers must have their own defer/recover in order for the BaseApp @@ -28,12 +29,8 @@ func SetUpDecorator(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHand 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() + return ctx, err.Wrap(errs.ErrOutOfGas, log) // TODO: figure out how to return Context, error so that baseapp can recover gasWanted/gasUsed default: panic(r) @@ -41,6 +38,5 @@ func SetUpDecorator(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHand } }() - newCtx, err = next(ctx, tx, simulate) - return + return next(newCtx, tx, simulate) } \ No newline at end of file diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go new file mode 100644 index 000000000000..fc76b082ced3 --- /dev/null +++ b/x/auth/ante/sigverify.go @@ -0,0 +1,200 @@ +package ante + +import ( + "bytes" + + "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" + "github.com/tendermint/tendermint/crypto" + "github.com/tendermint/tendermint/crypto/ed25519" + "github.com/tendermint/tendermint/crypto/multisig" + "github.com/tendermint/tendermint/crypto/secp256k1" +) + +// 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) error + +func SigGasConsumeDecorator(ak keeper.AccountKeeper, sigGasConsumer SignatureVerificationGasConsumer) { + return func(ctx Context, tx Tx, simulate bool, next AnteHandler) { + stdTx, ok := tx.(types.StdTx) + if !ok { + return ctx, errs.Wrap(errs.ErrInternal, "Tx must be a StdTx") + } + + stdSigs := stdTx.GetSignatures() + params := ak.GetParams(ctx) + + // 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)) + + for i, sig := range stdSigs { + pubKey := signerAccs[i].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 := sigGasConsumer(ctx.GasMeter(), sig.Signature, pubKey, params) + if err != nil { + return ctx, err + } + } + } + + return next(ctx, tx, simulate) + } +} + +func SigVerificationDecorator(ak keeper.AccountKeeper) { + return func(ctx Context, tx Tx, simulate bool, next sdk.AnteHandler) (newCtx, error) { + stdTx, ok := tx.(types.StdTx) + if !ok { + return ctx, errs.Wrap(errs.ErrInternal, "Tx must be a StdTx") + } + + // stdSigs contains the sequence number, account number, and signatures. + // When simulating, this would just be a 0-length slice. + stdSigs := stdTx.GetSignatures() + + // 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)) + + for i := 0; i < len(stdSigs); i++ { + signerAccs[i], err = GetSignerAcc(ctx, ak, signerAddrs[i]) + if err != nil { + return ctx, err + } + + // check signature, return account with incremented nonce + signBytes := GetSignBytes(newCtx.ChainID(), stdTx, signerAccs[i], isGenesis) + signerAccs[i], err = processSig(newCtx, signerAccs[i], stdSigs[i], signBytes, simulate) + if err != nil { + return ctx, err + } + + ak.SetAccount(ctx, signerAccs[i]) + } + + 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, +) sdk.Result { + switch pubkey := pubkey.(type) { + case ed25519.PubKeyEd25519: + meter.ConsumeGas(params.SigVerifyCostED25519, "ante verify: ed25519") + return errs.Wrap(errs.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 errs.Wrapf(errs.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++ + } + } +} + +// 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, error) { + // 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, nil + } + + if pubKey == nil { + pubKey = sig.PubKey + if pubKey == nil { + return nil, errs.Wrap(errs.ErrInvalidPubKey, "PubKey not found") + } + + if !bytes.Equal(pubKey.Address(), acc.GetAddress()) { + return nil, errs.Wrapf(errs.ErrInvalidPubKey, + "PubKey does not match Signer address %s", acc.GetAddress()) + } + } + + return pubKey, nil +} + +// 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, +) (updatedAcc exported.Account, res sdk.Result) { + + pubKey, err := ProcessPubKey(acc, sig, simulate) + if err != nil { + return nil, res + } + + err := acc.SetPubKey(pubKey) + if err != nil { + return nil, errs.Wrap(errs.ErrInternal, "setting PubKey on signer's account") + } + + if !simulate && !pubKey.VerifyBytes(signBytes, sig.Signature) { + return nil, errs.Wrap(errs.ErrUnauthorized, "signature verification failed; verify correct account sequence and chain-id") + } + + if err := acc.SetSequence(acc.GetSequence() + 1); err != nil { + panic(err) + } + + return acc, nil +} + +// 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, errs.Wrapf(errs.ErrUnknownAddress, "account %s does not exist", addr) +} From 5a46ceb770baf5ede406ffbc400713023d546d59 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 5 Sep 2019 13:30:16 -0700 Subject: [PATCH 03/32] Fix build errors --- types/handler.go | 10 + x/auth/alias.go | 3 - x/auth/ante/ante.go | 420 +-------------------------------- x/auth/ante/basic.go | 75 ++++++ x/auth/ante/fee.go | 79 ++++--- x/auth/ante/setup.go | 38 ++- x/auth/ante/sigverification.go | 19 -- x/auth/ante/sigverify.go | 227 +++++++++++++----- 8 files changed, 343 insertions(+), 528 deletions(-) delete mode 100644 x/auth/ante/sigverification.go diff --git a/types/handler.go b/types/handler.go index 794b84584eeb..2762c9007257 100644 --- a/types/handler.go +++ b/types/handler.go @@ -17,6 +17,7 @@ type AnteDecorator interface { // // First element is outermost decorator, last element is innermost decorator func ChainDecorators(chain ...AnteDecorator) AnteHandler { + chain = append(chain, Tail{}) if len(chain) == 1 { return func(ctx Context, tx Tx, simulate bool) (Context, error) { return chain[0].AnteHandle(ctx, tx, simulate, nil) @@ -26,3 +27,12 @@ func ChainDecorators(chain ...AnteDecorator) AnteHandler { return chain[0].AnteHandle(ctx, tx, simulate, ChainDecorators(chain[1:]...)) } } + +// Tail AnteDecorator will get added to the chain to simplify decorator code +// Don't need to check if next == nil further up the chain +type Tail struct{} + +// Simply return provided Context and nil error +func (t Tail) AnteHandle(ctx Context, tx Tx, simulate bool, next AnteHandler) (Context, error) { + return ctx, nil +} diff --git a/x/auth/alias.go b/x/auth/alias.go index f6a721497ccc..a61931771f44 100644 --- a/x/auth/alias.go +++ b/x/auth/alias.go @@ -31,12 +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 diff --git a/x/auth/ante/ante.go b/x/auth/ante/ante.go index 820552b792c5..73a472cae4d6 100644 --- a/x/auth/ante/ante.go +++ b/x/auth/ante/ante.go @@ -1,424 +1,24 @@ 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" - errs "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 -) - -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, error) { - if acc := ak.GetAccount(ctx, addr); acc != nil { - return acc, nil - } - return nil, errs.Wrapf(errs.ErrUnknownAddress, "account %s does not exist", addr) -} - -// 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.ChainDecorators( + NewSetupDecorator(), + NewMempoolFeeDecorator(), + NewValidateBasicDecorator(), + NewValidateMemoDecorator(ak), + NewConsumeGasForTxSizeDecorator(ak), + NewValidateSigCountDecorator(ak), + NewDeductFeeDecorator(ak, supplyKeeper), + NewSigGasConsumeDecorator(ak, sigGasConsumer), + NewSigVerificationDecorator(ak), ) } diff --git a/x/auth/ante/basic.go b/x/auth/ante/basic.go index 2aa954f07058..cde03307e372 100644 --- a/x/auth/ante/basic.go +++ b/x/auth/ante/basic.go @@ -1 +1,76 @@ package ante + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + err "github.com/cosmos/cosmos-sdk/types/errors" + errs "github.com/cosmos/cosmos-sdk/types/errors" + + "github.com/cosmos/cosmos-sdk/x/auth/keeper" + "github.com/cosmos/cosmos-sdk/x/auth/types" +) + +// 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) +} + +// ValidateMemoDecorator will validate memo given the parameters passed in +// If memo is too large decorator returns with error, otherwise call next AnteHandler +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) { + stdTx, ok := tx.(types.StdTx) + if !ok { + return ctx, errs.Wrap(errs.ErrTxDecode, "Tx must be a StdTx") + } + + params := vmd.ak.GetParams(ctx) + + memoLength := len(stdTx.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) +} + +// ConsumeGasForTxSizeDecorator will take in parameters and consume gas proportional to the size of tx +// before calling next AnteHandler +type ConsumeGasForTxSizeDecorator struct { + ak keeper.AccountKeeper +} + +func NewConsumeGasForTxSizeDecorator(ak keeper.AccountKeeper) ConsumeGasForTxSizeDecorator { + return ConsumeGasForTxSizeDecorator{ + ak: ak, + } +} + +func (cgts ConsumeGasForTxSizeDecorator) 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/fee.go b/x/auth/ante/fee.go index b67975030544..c14ffa54a1ce 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -11,10 +11,20 @@ import ( errs "github.com/cosmos/cosmos-sdk/types/errors" ) -func MempoolFeeDecorator(ctx Context, tx Tx, simulate bool, next AnteHandler) (newCtx Context, err error) { +// MempoolFeeDecorator will check if the transaction's fee is at least as large 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 +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) { stdTx, ok := tx.(types.StdTx) if !ok { - return ctx, errs.Wrap(errs.ErrInternal, "Tx must be a StdTx") + return ctx, errs.Wrap(errs.ErrTxDecode, "Tx must be a StdTx") } stdFee := stdTx.Fee @@ -41,37 +51,50 @@ func MempoolFeeDecorator(ctx Context, tx Tx, simulate bool, next AnteHandler) (n return next(ctx, tx, simulate) } -func NewDeductFeeDecorator(ak keeper.AccountKeeper, supplyKeeper types.SupplyKeeper) { - return func(ctx Context, tx Tx, simulate bool, next AnteHandler) (newCtx Context, err error) { - stdTx, ok := tx.(types.StdTx) - if !ok { - return ctx, errs.Wrap(errs.ErrInternal, "Tx must be a StdTx") - } +// 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 +type DeductFeeDecorator struct { + ak keeper.AccountKeeper + supplyKeeper types.SupplyKeeper +} - if addr := supplyKeeper.GetModuleAddress(types.FeeCollectorName); addr == nil { - panic(fmt.Sprintf("%s module account has not been set", types.FeeCollectorName)) - } +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) { + stdTx, ok := tx.(types.StdTx) + if !ok { + return ctx, errs.Wrap(errs.ErrTxDecode, "Tx must be a StdTx") + } - // stdSigs contains the sequence number, account number, and signatures. - // When simulating, this would just be a 0-length slice. - signerAddrs := stdTx.GetSigners() + if addr := dfd.supplyKeeper.GetModuleAddress(types.FeeCollectorName); addr == nil { + panic(fmt.Sprintf("%s module account has not been set", types.FeeCollectorName)) + } + + // stdSigs contains the sequence number, account number, and signatures. + // When simulating, this would just be a 0-length slice. + signerAddrs := stdTx.GetSigners() + + // fetch first signer, who's going to pay the fees + feePayer, err := GetSignerAcc(newCtx, dfd.ak, signerAddrs[0]) + if err != nil { + return ctx, err + } - // fetch first signer, who's going to pay the fees - feePayer, err = GetSignerAcc(newCtx, ak, signerAddrs[0]) + // deduct the fees + if !stdTx.Fee.Amount.IsZero() { + err = DeductFees(dfd.supplyKeeper, ctx, feePayer, stdTx.Fee.Amount) if err != nil { return ctx, err } - - // deduct the fees - if !stdTx.Fee.Amount.IsZero() { - err = DeductFees(supplyKeeper, newCtx, feePayer, stdTx.Fee.Amount) - if err != nil { - return ctx, err - } - } - - return next(ctx, tx, simulate) } + + return next(ctx, tx, simulate) } // DeductFees deducts fees from the given account. @@ -103,8 +126,8 @@ func DeductFees(supplyKeeper types.SupplyKeeper, ctx sdk.Context, acc exported.A err := supplyKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.FeeCollectorName, fees) if err != nil { - return err.Result() + return err } - return sdk.Result{} + return nil } diff --git a/x/auth/ante/setup.go b/x/auth/ante/setup.go index c34fe02e96cd..fb4cd256909c 100644 --- a/x/auth/ante/setup.go +++ b/x/auth/ante/setup.go @@ -1,18 +1,31 @@ package ante import ( + "fmt" + sdk "github.com/cosmos/cosmos-sdk/types" - err "github.com/cosmos/cosmos-sdk/types/errors" + errs "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/cosmos-sdk/x/auth/types" ) -func SetUpDecorator(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx Context, error) { +// SetUpDecorator 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. +// Should be the first decorator in the chain +type SetUpDecorator struct{} + +func NewSetupDecorator() SetUpDecorator { + return SetUpDecorator{} +} + +func (sud SetUpDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { // 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 + return newCtx, errs.Wrap(errs.ErrTxDecode, "Tx must be auth.StdTx") } newCtx = SetGasMeter(simulate, ctx, stdTx.Fee.Gas) @@ -28,9 +41,9 @@ func SetUpDecorator(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHand case sdk.ErrorOutOfGas: log := fmt.Sprintf( "out of gas in location: %v; gasWanted: %d, gasUsed: %d", - rType.Descriptor, stdTx.Fee.Gas, newCtx.GasMeter().GasConsumed(), - - return ctx, err.Wrap(errs.ErrOutOfGas, log) + rType.Descriptor, stdTx.Fee.Gas, newCtx.GasMeter().GasConsumed()) + + err = errs.Wrap(errs.ErrOutOfGas, log) // TODO: figure out how to return Context, error so that baseapp can recover gasWanted/gasUsed default: panic(r) @@ -39,4 +52,15 @@ func SetUpDecorator(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHand }() return next(newCtx, tx, simulate) -} \ No newline at end of file +} + +// 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/sigverification.go b/x/auth/ante/sigverification.go deleted file mode 100644 index 5d89feeb7285..000000000000 --- a/x/auth/ante/sigverification.go +++ /dev/null @@ -1,19 +0,0 @@ -package ante - -import ( - "encoding/hex" - - "github.com/tendermint/tendermint/crypto/secp256k1" -) - -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) -} diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index fc76b082ced3..1ef2549700c9 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -2,9 +2,11 @@ package ante import ( "bytes" + "encoding/hex" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" + errs "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" @@ -14,79 +16,151 @@ import ( "github.com/tendermint/tendermint/crypto/secp256k1" ) +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) error -func SigGasConsumeDecorator(ak keeper.AccountKeeper, sigGasConsumer SignatureVerificationGasConsumer) { - return func(ctx Context, tx Tx, simulate bool, next AnteHandler) { - stdTx, ok := tx.(types.StdTx) - if !ok { - return ctx, errs.Wrap(errs.ErrInternal, "Tx must be a StdTx") - } +// Consume parameter-defined amount of gas for each signature according to the passed-in SignatureVerificationGasConsumer function +// before calling the next AnteHandler +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) (sdk.Context, error) { + stdTx, ok := tx.(types.StdTx) + if !ok { + return ctx, errs.Wrap(errs.ErrTxDecode, "Tx must be a StdTx") + } - stdSigs := stdTx.GetSignatures() - params := ak.GetParams(ctx) - - // 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)) - - for i, sig := range stdSigs { - pubKey := signerAccs[i].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 := sigGasConsumer(ctx.GasMeter(), sig.Signature, pubKey, params) - if err != nil { - return ctx, err - } + params := sgcd.ak.GetParams(ctx) + + stdSigs := stdTx.GetSignatures() + + // 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)) + + for i, sig := range stdSigs { + pubKey := signerAccs[i].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.Signature, pubKey, params) + if err != nil { + return ctx, err } } + } - return next(ctx, tx, simulate) + 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 +type SigVerificationDecorator struct { + ak keeper.AccountKeeper +} + +func NewSigVerificationDecorator(ak keeper.AccountKeeper) SigVerificationDecorator { + return SigVerificationDecorator{ + ak: ak, } } -func SigVerificationDecorator(ak keeper.AccountKeeper) { - return func(ctx Context, tx Tx, simulate bool, next sdk.AnteHandler) (newCtx, error) { - stdTx, ok := tx.(types.StdTx) - if !ok { - return ctx, errs.Wrap(errs.ErrInternal, "Tx must be a StdTx") - } +func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { + stdTx, ok := tx.(types.StdTx) + if !ok { + return ctx, errs.Wrap(errs.ErrTxDecode, "Tx must be a StdTx") + } - // stdSigs contains the sequence number, account number, and signatures. - // When simulating, this would just be a 0-length slice. - stdSigs := stdTx.GetSignatures() + isGenesis := ctx.BlockHeight() == 0 - // 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)) + // 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++ { - signerAccs[i], err = GetSignerAcc(ctx, ak, signerAddrs[i]) - if err != nil { - return ctx, err - } + // 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)) - // check signature, return account with incremented nonce - signBytes := GetSignBytes(newCtx.ChainID(), stdTx, signerAccs[i], isGenesis) - signerAccs[i], err = processSig(newCtx, signerAccs[i], stdSigs[i], signBytes, simulate) - if err != nil { - return ctx, err - } + for i := 0; i < len(stdSigs); i++ { + signerAccs[i], err = GetSignerAcc(ctx, svd.ak, signerAddrs[i]) + if err != nil { + return ctx, err + } - ak.SetAccount(ctx, signerAccs[i]) + // check signature, return account with incremented nonce + signBytes := GetSignBytes(newCtx.ChainID(), stdTx, signerAccs[i], isGenesis) + signerAccs[i], err = processSig(newCtx, signerAccs[i], stdSigs[i], signBytes, simulate) + if err != nil { + return ctx, err } - return next(ctx, tx, simulate) + svd.ak.SetAccount(ctx, signerAccs[i]) } + + 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 +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) { + stdTx, ok := tx.(types.StdTx) + if !ok { + return ctx, errs.Wrap(errs.ErrTxDecode, "Tx must be a StdTx") + } + + params := vscd.ak.GetParams(ctx) + + stdSigs := stdTx.GetSignatures() + + sigCount := 0 + for i := 0; i < len(stdSigs); i++ { + sigCount += types.CountSubKeys(stdSigs[i].PubKey) + if uint64(sigCount) > params.TxSigLimit { + return ctx, errs.Wrapf(errs.ErrTooManySignatures, + "signatures: %d, limit: %d", sigCount, params.TxSigLimit) + } + } + + return next(ctx, tx, simulate) } // DefaultSigVerificationGasConsumer is the default implementation of SignatureVerificationGasConsumer. It consumes gas @@ -94,7 +168,7 @@ func SigVerificationDecorator(ak keeper.AccountKeeper) { // by the concrete type. func DefaultSigVerificationGasConsumer( meter sdk.GasMeter, sig []byte, pubkey crypto.PubKey, params types.Params, -) sdk.Result { +) error { switch pubkey := pubkey.(type) { case ed25519.PubKeyEd25519: meter.ConsumeGas(params.SigVerifyCostED25519, "ante verify: ed25519") @@ -130,6 +204,24 @@ func consumeMultisignatureVerificationGas(meter sdk.GasMeter, } } +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. @@ -142,7 +234,7 @@ func ProcessPubKey(acc exported.Account, sig types.StdSignature, simulate bool) // 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 simSecp256k1Pubkey, nil } return pubKey, nil @@ -167,23 +259,23 @@ func ProcessPubKey(acc exported.Account, sig types.StdSignature, simulate bool) // a pubkey, set it. func processSig( ctx sdk.Context, acc exported.Account, sig types.StdSignature, signBytes []byte, simulate bool, -) (updatedAcc exported.Account, res sdk.Result) { +) (updatedAcc exported.Account, err error) { pubKey, err := ProcessPubKey(acc, sig, simulate) if err != nil { - return nil, res + return nil, err } - err := acc.SetPubKey(pubKey) + err = acc.SetPubKey(pubKey) if err != nil { - return nil, errs.Wrap(errs.ErrInternal, "setting PubKey on signer's account") + return nil, errs.Wrap(errs.ErrTxDecode, "setting PubKey on signer's account") } if !simulate && !pubKey.VerifyBytes(signBytes, sig.Signature) { return nil, errs.Wrap(errs.ErrUnauthorized, "signature verification failed; verify correct account sequence and chain-id") } - if err := acc.SetSequence(acc.GetSequence() + 1); err != nil { + if err = acc.SetSequence(acc.GetSequence() + 1); err != nil { panic(err) } @@ -198,3 +290,16 @@ func GetSignerAcc(ctx sdk.Context, ak keeper.AccountKeeper, addr sdk.AccAddress) } return nil, errs.Wrapf(errs.ErrUnknownAddress, "account %s does not exist", addr) } + +// 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, + ) +} From ac19f18e2d2e4a3050153d04fd915185d1718edc Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 6 Sep 2019 16:39:40 -0700 Subject: [PATCH 04/32] fix some tests --- x/auth/ante/ante_test.go | 40 ++++++++++++++++------------------------ x/auth/ante/fee.go | 2 +- x/auth/ante/sigverify.go | 12 +++++++++++- 3 files changed, 28 insertions(+), 26 deletions(-) diff --git a/x/auth/ante/ante_test.go b/x/auth/ante/ante_test.go index 982bbec9c7ed..b0585a5bee71 100644 --- a/x/auth/ante/ante_test.go +++ b/x/auth/ante/ante_test.go @@ -13,6 +13,7 @@ import ( "github.com/tendermint/tendermint/crypto/secp256k1" sdk "github.com/cosmos/cosmos-sdk/types" + errs "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" @@ -20,30 +21,19 @@ import ( // 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. @@ -573,7 +563,7 @@ func TestProcessPubKey(t *testing.T) { 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()) + require.Equal(t, tt.wantErr, err != nil) }) } } @@ -609,12 +599,12 @@ func TestConsumeSignatureVerificationGas(t *testing.T) { } 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) + err := ante.DefaultSigVerificationGasConsumer(tt.args.meter, tt.args.sig, tt.args.pubkey, tt.args.params) if tt.shouldErr { - require.False(t, res.IsOK()) + require.NotNil(t, err) } else { - require.True(t, res.IsOK()) + require.Nil(t, err) require.Equal(t, tt.gasConsumed, tt.args.meter.GasConsumed(), fmt.Sprintf("%d != %d", tt.gasConsumed, tt.args.meter.GasConsumed())) } }) @@ -723,6 +713,7 @@ func TestAnteHandlerSigLimitExceeded(t *testing.T) { checkInvalidTx(t, anteHandler, ctx, tx, false, sdk.CodeTooManySignatures) } +/* func TestEnsureSufficientMempoolFees(t *testing.T) { // setup _, ctx := createTestApp(true) @@ -773,6 +764,7 @@ func TestEnsureSufficientMempoolFees(t *testing.T) { ) } } +*/ // Test custom SignatureVerificationGasConsumer func TestCustomSignatureVerificationGasConsumer(t *testing.T) { @@ -780,13 +772,13 @@ func TestCustomSignatureVerificationGasConsumer(t *testing.T) { 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 errs.Wrapf(errs.ErrInvalidPubKey, "unrecognized public key type: %T", pubkey) } }) diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index c14ffa54a1ce..a2b133151e5d 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -81,7 +81,7 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo signerAddrs := stdTx.GetSigners() // fetch first signer, who's going to pay the fees - feePayer, err := GetSignerAcc(newCtx, dfd.ak, signerAddrs[0]) + feePayer, err := GetSignerAcc(ctx, dfd.ak, signerAddrs[0]) if err != nil { return ctx, err } diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 1ef2549700c9..031635b57d9a 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -46,7 +46,7 @@ func NewSigGasConsumeDecorator(ak keeper.AccountKeeper, sigGasConsumer Signature } } -func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { +func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { stdTx, ok := tx.(types.StdTx) if !ok { return ctx, errs.Wrap(errs.ErrTxDecode, "Tx must be a StdTx") @@ -62,7 +62,17 @@ func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula signerAccs := make([]exported.Account, len(signerAddrs)) for i, sig := range stdSigs { + signerAccs[i], err = GetSignerAcc(ctx, sgcd.ak, signerAddrs[i]) + if err != nil { + return ctx, err + } + + // TODO + pubKey := signerAccs[i].GetPubKey() + if pubKey == nil { + pubKey = sig.PubKey + } 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 From 9a0eac764beb7c0622a96c7b707aa3d978344822 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 9 Sep 2019 11:00:55 -0700 Subject: [PATCH 05/32] fix baseapp tests --- baseapp/baseapp_test.go | 53 +++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 29 deletions(-) diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index eb50fc3ed2c8..3ae8ac1dca98 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" + errs "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, errs.Wrap(errs.ErrUnauthorized, "ante handler failure") } - res = incrementingCounter(t, store, storeKey, txTest.Counter) + res := incrementingCounter(t, store, storeKey, txTest.Counter) + if !res.IsOK() { + err = errs.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,20 @@ 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)) + // 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", rType.Descriptor) - res = sdk.ErrOutOfGas(log).Result() - res.GasWanted = gasGranted - res.GasUsed = newCtx.GasMeter().GasConsumed() + err = errs.Wrap(errs.ErrOutOfGas, log) default: panic(r) } @@ -999,9 +1006,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 +1070,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 = errs.Wrapf(errs.ErrOutOfGas, "out of gas in location: %v", rType.Descriptor) default: panic(r) } @@ -1084,9 +1086,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 +1234,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 +1242,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 = errs.Wrap(errs.ErrOutOfGas, log) default: panic(r) } @@ -1254,12 +1252,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, errs.Wrap(errs.ErrUnauthorized, "ante handler failure") } - res = sdk.Result{ - GasWanted: gasWanted, - } return }) } @@ -1310,7 +1305,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 From 725f5cfcbaa4bdf455a9c7df229f5e658346f91c Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 12 Sep 2019 14:53:26 -0700 Subject: [PATCH 06/32] fix auth tests --- x/auth/ante/ante.go | 1 + x/auth/ante/ante_test.go | 23 ++++++------ x/auth/ante/sigverify.go | 77 +++++++++++++++++++++++++++++----------- 3 files changed, 70 insertions(+), 31 deletions(-) diff --git a/x/auth/ante/ante.go b/x/auth/ante/ante.go index 73a472cae4d6..f21966a57d04 100644 --- a/x/auth/ante/ante.go +++ b/x/auth/ante/ante.go @@ -16,6 +16,7 @@ func NewAnteHandler(ak keeper.AccountKeeper, supplyKeeper types.SupplyKeeper, si NewValidateBasicDecorator(), NewValidateMemoDecorator(ak), NewConsumeGasForTxSizeDecorator(ak), + NewSetPubKeyDecorator(ak), NewValidateSigCountDecorator(ak), NewDeductFeeDecorator(ak, supplyKeeper), NewSigGasConsumeDecorator(ak, sigGasConsumer), diff --git a/x/auth/ante/ante_test.go b/x/auth/ante/ante_test.go index b0585a5bee71..b1ae33f3cf42 100644 --- a/x/auth/ante/ante_test.go +++ b/x/auth/ante/ante_test.go @@ -344,12 +344,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) } @@ -471,7 +471,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) @@ -692,14 +692,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) @@ -708,7 +709,7 @@ 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) } diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 031635b57d9a..ed19f5c264f7 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -67,23 +67,21 @@ func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula return ctx, err } - // TODO - - pubKey := signerAccs[i].GetPubKey() - if pubKey == nil { - pubKey = sig.PubKey + pubKey, err := ProcessPubKey(signerAccs[i], sig, simulate) + if err != nil { + return ctx, err } + 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.Signature, pubKey, params) - if err != nil { - return ctx, err - } + } + err = sgcd.sigGasConsumer(ctx.GasMeter(), sig.Signature, pubKey, params) + if err != nil { + return ctx, err } } @@ -127,8 +125,8 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul } // check signature, return account with incremented nonce - signBytes := GetSignBytes(newCtx.ChainID(), stdTx, signerAccs[i], isGenesis) - signerAccs[i], err = processSig(newCtx, signerAccs[i], stdSigs[i], signBytes, simulate) + signBytes := GetSignBytes(ctx.ChainID(), stdTx, signerAccs[i], isGenesis) + signerAccs[i], err = processSig(ctx, signerAccs[i], stdSigs[i], signBytes, simulate) if err != nil { return ctx, err } @@ -173,6 +171,50 @@ func (vscd ValidateSigCountDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, sim return next(ctx, tx, simulate) } +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) + } + stdTx, ok := tx.(types.StdTx) + if !ok { + return ctx, errs.Wrap(errs.ErrTxDecode, "Tx must be a StdTx") + } + + stdSigs := stdTx.GetSignatures() + signers := stdTx.GetSigners() + + for i, sig := range stdSigs { + if sig.PubKey != nil { + if !bytes.Equal(sig.PubKey.Address(), signers[i]) { + return ctx, errs.Wrapf(errs.ErrInvalidPubKey, + "PubKey does not match Signer address %s", signers[i]) + } + + acc, err := GetSignerAcc(ctx, spkd.ak, signers[i]) + if err != nil { + return ctx, err + } + err = acc.SetPubKey(sig.PubKey) + if err != nil { + return ctx, errs.Wrap(errs.ErrInvalidPubKey, err.Error()) + } + spkd.ak.SetAccount(ctx, acc) + } + } + + 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. @@ -271,14 +313,9 @@ func processSig( ctx sdk.Context, acc exported.Account, sig types.StdSignature, signBytes []byte, simulate bool, ) (updatedAcc exported.Account, err error) { - pubKey, err := ProcessPubKey(acc, sig, simulate) - if err != nil { - return nil, err - } - - err = acc.SetPubKey(pubKey) - if err != nil { - return nil, errs.Wrap(errs.ErrTxDecode, "setting PubKey on signer's account") + pubKey := acc.GetPubKey() + if pubKey == nil { + return nil, errs.Wrap(errs.ErrInvalidPubKey, "pubkey on account is not set") } if !simulate && !pubKey.VerifyBytes(signBytes, sig.Signature) { From ff5f1f797db6d41ba560474d2a53b0c00c29da9f Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Fri, 13 Sep 2019 17:13:27 -0700 Subject: [PATCH 07/32] start individual decorator tests --- baseapp/baseapp_test.go | 1 - x/auth/ante/basic_test.go | 101 ++++++++++++++++++++++++++++++++++ x/auth/ante/fee_test.go | 97 ++++++++++++++++++++++++++++++++ x/auth/ante/setup_test.go | 99 +++++++++++++++++++++++++++++++++ x/auth/ante/sigverify_test.go | 0 x/auth/types/stdtx_test.go | 2 +- x/auth/types/test_common.go | 2 +- 7 files changed, 299 insertions(+), 3 deletions(-) create mode 100644 x/auth/ante/basic_test.go create mode 100644 x/auth/ante/fee_test.go create mode 100644 x/auth/ante/setup_test.go create mode 100644 x/auth/ante/sigverify_test.go diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 3ae8ac1dca98..76ebfb24ac4a 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -987,7 +987,6 @@ func TestTxGasLimits(t *testing.T) { bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, err error) { newCtx = ctx.WithGasMeter(sdk.NewGasMeter(gasGranted)) - // 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 diff --git a/x/auth/ante/basic_test.go b/x/auth/ante/basic_test.go new file mode 100644 index 000000000000..4f9fbb6d52dc --- /dev/null +++ b/x/auth/ante/basic_test.go @@ -0,0 +1,101 @@ +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.ChainDecorators(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)) + + vmd := ante.NewValidateMemoDecorator(app.AccountKeeper) + antehandler := sdk.ChainDecorators(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)) + + _, 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.ChainDecorators(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, "ConsumeGasForTxSizeDecorator returned error: %v", err) + + consumedGas := ctx.GasMeter().GasConsumed() - beforeGas + require.Equal(t, sdk.Gas(expectedGas), consumedGas, "Decorator did not consume the correct amount of gas") +} diff --git a/x/auth/ante/fee_test.go b/x/auth/ante/fee_test.go new file mode 100644 index 000000000000..254652837215 --- /dev/null +++ b/x/auth/ante/fee_test.go @@ -0,0 +1,97 @@ +package ante_test + +import ( + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/auth/ante" + "github.com/cosmos/cosmos-sdk/x/auth/types" + "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/crypto" +) + +func TestEnsureMempoolFees(t *testing.T) { + // setup + _, ctx := createTestApp(true) + + mfd := ante.NewMempoolFeeDecorator() + antehandler := sdk.ChainDecorators(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.ChainDecorators(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_test.go b/x/auth/ante/setup_test.go new file mode 100644 index 000000000000..4196fad42b31 --- /dev/null +++ b/x/auth/ante/setup_test.go @@ -0,0 +1,99 @@ +package ante_test + +import ( + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + errs "github.com/cosmos/cosmos-sdk/types/errors" + "github.com/cosmos/cosmos-sdk/x/auth/ante" + "github.com/cosmos/cosmos-sdk/x/auth/types" + "github.com/stretchr/testify/require" + "github.com/tendermint/tendermint/crypto" +) + +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.NewSetupDecorator() + antehandler := sdk.ChainDecorators(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, "SetupDecorator returned error") + + // Context GasMeter Limit should be set after SetupDecorator 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.NewSetupDecorator() + antehandler := sdk.ChainDecorators(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, errs.ErrOutOfGas.Is(err), "Returned error is not an out of gas error") + require.Equal(t, fee.Gas, newCtx.GasMeter().Limit()) + + antehandler = sdk.ChainDecorators(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) (sdk.Context, error) { + panic("random error") + + // not reached + return ctx, nil +} diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/x/auth/types/stdtx_test.go b/x/auth/types/stdtx_test.go index 661db3fd4752..9322fdb8e80d 100644 --- a/x/auth/types/stdtx_test.go +++ b/x/auth/types/stdtx_test.go @@ -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\":\"70000\"},\"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)), ) } From 73e273919f3a3e0f8d38526b5b7d628adf19cbd2 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 16 Sep 2019 22:58:39 -0700 Subject: [PATCH 08/32] add signature tests --- x/auth/ante/ante_test.go | 43 -------- x/auth/ante/sigverify.go | 18 ++-- x/auth/ante/sigverify_test.go | 181 ++++++++++++++++++++++++++++++++++ 3 files changed, 189 insertions(+), 53 deletions(-) diff --git a/x/auth/ante/ante_test.go b/x/auth/ante/ante_test.go index b1ae33f3cf42..534a1937a55a 100644 --- a/x/auth/ante/ante_test.go +++ b/x/auth/ante/ante_test.go @@ -568,49 +568,6 @@ func TestProcessPubKey(t *testing.T) { } } -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 generatePubKeysAndSignatures(n int, msg []byte, keyTypeed25519 bool) (pubkeys []crypto.PubKey, signatures [][]byte) { pubkeys = make([]crypto.PubKey, n) signatures = make([][]byte, n) diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index ed19f5c264f7..75cf052b569e 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -59,17 +59,15 @@ func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula // 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)) for i, sig := range stdSigs { - signerAccs[i], err = GetSignerAcc(ctx, sgcd.ak, signerAddrs[i]) - if err != nil { - return ctx, err - } - - pubKey, err := ProcessPubKey(signerAccs[i], sig, simulate) - if err != nil { - return ctx, err + pubKey := sig.PubKey + if pubKey == nil { + signerAcc, err := GetSignerAcc(ctx, sgcd.ak, signerAddrs[i]) + if err != nil { + return ctx, err + } + pubKey = signerAcc.GetPubKey() } if simulate { @@ -197,7 +195,7 @@ func (spkd SetPubKeyDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b if sig.PubKey != nil { if !bytes.Equal(sig.PubKey.Address(), signers[i]) { return ctx, errs.Wrapf(errs.ErrInvalidPubKey, - "PubKey does not match Signer address %s", signers[i]) + "PubKey does not match Signer address %s with signer index: %d", signers[i], i) } acc, err := GetSignerAcc(ctx, spkd.ak, signers[i]) diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index e69de29bb2d1..c4715d2402c9 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -0,0 +1,181 @@ +package ante_test + +import ( + "fmt" + "testing" + + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/auth/ante" + "github.com/cosmos/cosmos-sdk/x/auth/types" + "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" +) + +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} + + var msgs []sdk.Msg + // 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 = append(msgs, 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.ChainDecorators(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 TestSigGasConsumerSecp56k1(t *testing.T) { + // generate private keys + privs := []crypto.PrivKey{secp256k1.GenPrivKey(), secp256k1.GenPrivKey(), secp256k1.GenPrivKey()} + + params := types.DefaultParams() + initialSigCost := params.SigVerifyCostSecp256k1 + initialCost := benchmarkGasSigDecorator(t, params, false, privs...) + + params.SigVerifyCostSecp256k1 = params.SigVerifyCostSecp256k1 * 2 + doubleCost := benchmarkGasSigDecorator(t, params, false, privs...) + + require.Equal(t, initialSigCost*uint64(len(privs)), doubleCost-initialCost) +} + +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} + + var msgs []sdk.Msg + // 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 = append(msgs, 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.ChainDecorators(spkd, svd) + + ctx, err := antehandler(ctx, tx, false) + require.NotNil(t, err) +} + +func benchmarkGasSigDecorator(t *testing.T, params types.Params, multisig bool, privs ...crypto.PrivKey) sdk.Gas { + // setup + app, ctx := createTestApp(true) + app.AccountKeeper.SetParams(ctx, params) + + var msgs []sdk.Msg + var accNums []uint64 + var seqs []uint64 + // 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 = append(msgs, types.NewTestMsg(addr)) + accNums = append(accNums, uint64(i)) + seqs = append(seqs, 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) + antehandler := sdk.ChainDecorators(spkd, svgc) + + // Determine gas consumption of antehandler with default params + before := ctx.GasMeter().GasConsumed() + ctx, err := antehandler(ctx, tx, false) + require.Nil(t, err) + after := ctx.GasMeter().GasConsumed() + + return sdk.Gas(after - before) +} From 77bc7c6f0b0c361eda1c4cd186bc0644b54100f6 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 17 Sep 2019 12:05:49 -0700 Subject: [PATCH 09/32] complete sig tests --- x/auth/ante/sigverify_test.go | 40 +++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index c4715d2402c9..efcba42b3126 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -97,20 +97,6 @@ func TestConsumeSignatureVerificationGas(t *testing.T) { } } -func TestSigGasConsumerSecp56k1(t *testing.T) { - // generate private keys - privs := []crypto.PrivKey{secp256k1.GenPrivKey(), secp256k1.GenPrivKey(), secp256k1.GenPrivKey()} - - params := types.DefaultParams() - initialSigCost := params.SigVerifyCostSecp256k1 - initialCost := benchmarkGasSigDecorator(t, params, false, privs...) - - params.SigVerifyCostSecp256k1 = params.SigVerifyCostSecp256k1 * 2 - doubleCost := benchmarkGasSigDecorator(t, params, false, privs...) - - require.Equal(t, initialSigCost*uint64(len(privs)), doubleCost-initialCost) -} - func TestSigVerification(t *testing.T) { // setup app, ctx := createTestApp(true) @@ -144,9 +130,27 @@ func TestSigVerification(t *testing.T) { require.NotNil(t, err) } -func benchmarkGasSigDecorator(t *testing.T, params types.Params, multisig bool, privs ...crypto.PrivKey) sdk.Gas { +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 = 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) var msgs []sdk.Msg @@ -169,13 +173,13 @@ func benchmarkGasSigDecorator(t *testing.T, params types.Params, multisig bool, spkd := ante.NewSetPubKeyDecorator(app.AccountKeeper) svgc := ante.NewSigGasConsumeDecorator(app.AccountKeeper, ante.DefaultSigVerificationGasConsumer) - antehandler := sdk.ChainDecorators(spkd, svgc) + svd := ante.NewSigVerificationDecorator(app.AccountKeeper) + antehandler := sdk.ChainDecorators(spkd, svgc, svd) // Determine gas consumption of antehandler with default params before := ctx.GasMeter().GasConsumed() ctx, err := antehandler(ctx, tx, false) - require.Nil(t, err) after := ctx.GasMeter().GasConsumed() - return sdk.Gas(after - before) + return sdk.Gas(after - before), err } From baf5bfb1ecaf292624d4bf37ad9ab245453ca421 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 17 Sep 2019 15:01:30 -0700 Subject: [PATCH 10/32] fix all test errors --- x/auth/ante/sigverify.go | 4 ++-- x/auth/types/stdtx_test.go | 2 +- x/mock/app_test.go | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 75cf052b569e..66d196653e73 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -297,7 +297,7 @@ func ProcessPubKey(acc exported.Account, sig types.StdSignature, simulate bool) } if !bytes.Equal(pubKey.Address(), acc.GetAddress()) { - return nil, errs.Wrapf(errs.ErrInvalidPubKey, + return nil, errs.Wrapf(errs.ErrUnauthorized, "PubKey does not match Signer address %s", acc.GetAddress()) } } @@ -312,7 +312,7 @@ func processSig( ) (updatedAcc exported.Account, err error) { pubKey := acc.GetPubKey() - if pubKey == nil { + if !simulate && &pubKey == nil { return nil, errs.Wrap(errs.ErrInvalidPubKey, "pubkey on account is not set") } diff --git a/x/auth/types/stdtx_test.go b/x/auth/types/stdtx_test.go index 9322fdb8e80d..0cab5a29e88c 100644 --- a/x/auth/types/stdtx_test.go +++ b/x/auth/types/stdtx_test.go @@ -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\":\"70000\"},\"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/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 From 71e07ce7c544e9a213b3fe3b06358bd03fda4377 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 17 Sep 2019 23:31:26 -0700 Subject: [PATCH 11/32] remove unnecessary & --- x/auth/ante/sigverify.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 66d196653e73..56efd6d81fb3 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -312,7 +312,7 @@ func processSig( ) (updatedAcc exported.Account, err error) { pubKey := acc.GetPubKey() - if !simulate && &pubKey == nil { + if !simulate && pubKey == nil { return nil, errs.Wrap(errs.ErrInvalidPubKey, "pubkey on account is not set") } From 3b3a74f95f6ff6e13bbd727ee569093e5de1c5a9 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 18 Sep 2019 00:03:22 -0700 Subject: [PATCH 12/32] fix linter errors --- x/auth/ante/basic_test.go | 2 +- x/auth/ante/setup_test.go | 5 +---- x/auth/ante/sigverify_test.go | 6 +++--- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/x/auth/ante/basic_test.go b/x/auth/ante/basic_test.go index 4f9fbb6d52dc..446975ab55da 100644 --- a/x/auth/ante/basic_test.go +++ b/x/auth/ante/basic_test.go @@ -97,5 +97,5 @@ func TestConsumeGasForTxSize(t *testing.T) { require.Nil(t, err, "ConsumeGasForTxSizeDecorator returned error: %v", err) consumedGas := ctx.GasMeter().GasConsumed() - beforeGas - require.Equal(t, sdk.Gas(expectedGas), consumedGas, "Decorator did not consume the correct amount of gas") + require.Equal(t, expectedGas, consumedGas, "Decorator did not consume the correct amount of gas") } diff --git a/x/auth/ante/setup_test.go b/x/auth/ante/setup_test.go index 4196fad42b31..bc7d1719b837 100644 --- a/x/auth/ante/setup_test.go +++ b/x/auth/ante/setup_test.go @@ -91,9 +91,6 @@ func (ogd OutOfGasDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate boo type PanicDecorator struct{} -func (pd PanicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { +func (pd PanicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { panic("random error") - - // not reached - return ctx, nil } diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index efcba42b3126..e582a89fd72b 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -126,7 +126,7 @@ func TestSigVerification(t *testing.T) { svd := ante.NewSigVerificationDecorator(app.AccountKeeper) antehandler := sdk.ChainDecorators(spkd, svd) - ctx, err := antehandler(ctx, tx, false) + _, err := antehandler(ctx, tx, false) require.NotNil(t, err) } @@ -139,7 +139,7 @@ func TestSigIntegration(t *testing.T) { initialCost, err := runSigDecorators(t, params, false, privs...) require.Nil(t, err) - params.SigVerifyCostSecp256k1 = params.SigVerifyCostSecp256k1 * 2 + params.SigVerifyCostSecp256k1 *= 2 doubleCost, err := runSigDecorators(t, params, false, privs...) require.Nil(t, err) @@ -181,5 +181,5 @@ func runSigDecorators(t *testing.T, params types.Params, multisig bool, privs .. ctx, err := antehandler(ctx, tx, false) after := ctx.GasMeter().GasConsumed() - return sdk.Gas(after - before), err + return after - before, err } From e52e89db65000921fcaafddf1d84a53d4d35b6c7 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 18 Sep 2019 15:22:14 -0700 Subject: [PATCH 13/32] interface all decorators except sigs --- x/auth/ante/basic.go | 14 ++++++++++---- x/auth/ante/fee.go | 42 ++++++++++++++++++++++++------------------ x/auth/ante/setup.go | 19 ++++++++++++------- x/auth/types/stdtx.go | 12 ++++++++++++ 4 files changed, 58 insertions(+), 29 deletions(-) diff --git a/x/auth/ante/basic.go b/x/auth/ante/basic.go index cde03307e372..5d3b5e215bf3 100644 --- a/x/auth/ante/basic.go +++ b/x/auth/ante/basic.go @@ -6,7 +6,6 @@ import ( errs "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/auth/keeper" - "github.com/cosmos/cosmos-sdk/x/auth/types" ) // ValidateBasicDecorator will call tx.ValidateBasic and return any non-nil error @@ -25,8 +24,15 @@ func (vbd ValidateBasicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulat 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 } @@ -38,14 +44,14 @@ func NewValidateMemoDecorator(ak keeper.AccountKeeper) ValidateMemoDecorator { } func (vmd ValidateMemoDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { - stdTx, ok := tx.(types.StdTx) + memoTx, ok := tx.(TxWithMemo) if !ok { - return ctx, errs.Wrap(errs.ErrTxDecode, "Tx must be a StdTx") + return ctx, errs.Wrap(errs.ErrTxDecode, "Tx must have a GetMemo method") } params := vmd.ak.GetParams(ctx) - memoLength := len(stdTx.GetMemo()) + 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", diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index a2b133151e5d..12fbc04a2d8e 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -11,10 +11,19 @@ import ( errs "github.com/cosmos/cosmos-sdk/types/errors" ) +// Tx must implement FeeTx interface to use the FeeDecorators +type FeeTx interface { + sdk.Tx + Gas() uint64 + FeeCoins() sdk.Coins + FeePayer() sdk.AccAddress +} + // MempoolFeeDecorator will check if the transaction's fee is at least as large 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 { @@ -22,11 +31,12 @@ func NewMempoolFeeDecorator() MempoolFeeDecorator { } func (mfd MempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { - stdTx, ok := tx.(types.StdTx) + feeTx, ok := tx.(FeeTx) if !ok { - return ctx, errs.Wrap(errs.ErrTxDecode, "Tx must be a StdTx") + return ctx, errs.Wrap(errs.ErrTxDecode, "Tx must be a FeeTx") } - stdFee := stdTx.Fee + feeCoins := feeTx.FeeCoins() + gas := feeTx.Gas() // 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 @@ -38,13 +48,15 @@ func (mfd MempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b // 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)) + glDec := sdk.NewDec(int64(gas)) for i, gp := range minGasPrices { fee := gp.Amount.Mul(glDec) requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt()) } - return ctx, errs.Wrapf(errs.ErrInsufficientFee, "insufficient fees; got: %q required: %q", stdFee.Amount, requiredFees) + if !feeCoins.IsAnyGTE(requiredFees) { + return ctx, errs.Wrapf(errs.ErrInsufficientFee, "insufficient fees; got: %q required: %q", feeCoins, requiredFees) + } } } @@ -54,6 +66,7 @@ func (mfd MempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b // 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 @@ -67,28 +80,21 @@ func NewDeductFeeDecorator(ak keeper.AccountKeeper, sk types.SupplyKeeper) Deduc } func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { - stdTx, ok := tx.(types.StdTx) + feeTx, ok := tx.(FeeTx) if !ok { - return ctx, errs.Wrap(errs.ErrTxDecode, "Tx must be a StdTx") + return ctx, errs.Wrap(errs.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)) } - // stdSigs contains the sequence number, account number, and signatures. - // When simulating, this would just be a 0-length slice. - signerAddrs := stdTx.GetSigners() - - // fetch first signer, who's going to pay the fees - feePayer, err := GetSignerAcc(ctx, dfd.ak, signerAddrs[0]) - if err != nil { - return ctx, err - } + feePayer := feeTx.FeePayer() + feePayerAcc := dfd.ak.GetAccount(ctx, feePayer) // deduct the fees - if !stdTx.Fee.Amount.IsZero() { - err = DeductFees(dfd.supplyKeeper, ctx, feePayer, stdTx.Fee.Amount) + if !feeTx.FeeCoins().IsZero() { + err = DeductFees(dfd.supplyKeeper, ctx, feePayerAcc, feeTx.FeeCoins()) if err != nil { return ctx, err } diff --git a/x/auth/ante/setup.go b/x/auth/ante/setup.go index fb4cd256909c..3c971c112375 100644 --- a/x/auth/ante/setup.go +++ b/x/auth/ante/setup.go @@ -5,13 +5,18 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" errs "github.com/cosmos/cosmos-sdk/types/errors" - "github.com/cosmos/cosmos-sdk/x/auth/types" ) +// Tx with a Gas() method is needed to use SetupDecorator +type GasTx interface { + Gas() uint64 +} + // SetUpDecorator 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. -// Should be the first decorator in the chain +// CONTRACT: Must be first decorator in the chain +// CONTRACT: Tx must implement GasTx interface type SetUpDecorator struct{} func NewSetupDecorator() SetUpDecorator { @@ -19,16 +24,16 @@ func NewSetupDecorator() SetUpDecorator { } func (sud SetUpDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { - // all transactions must be of type auth.StdTx - stdTx, ok := tx.(types.StdTx) + // 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, errs.Wrap(errs.ErrTxDecode, "Tx must be auth.StdTx") + return newCtx, errs.Wrap(errs.ErrTxDecode, "Tx must be GasTx") } - newCtx = SetGasMeter(simulate, ctx, stdTx.Fee.Gas) + newCtx = SetGasMeter(simulate, ctx, gasTx.Gas()) // Decorator will catch an OutOfGasPanic caused in the next antehandler // AnteHandlers must have their own defer/recover in order for the BaseApp @@ -41,7 +46,7 @@ func (sud SetUpDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, case sdk.ErrorOutOfGas: log := fmt.Sprintf( "out of gas in location: %v; gasWanted: %d, gasUsed: %d", - rType.Descriptor, stdTx.Fee.Gas, newCtx.GasMeter().GasConsumed()) + rType.Descriptor, gasTx.Gas(), newCtx.GasMeter().GasConsumed()) err = errs.Wrap(errs.ErrOutOfGas, log) // TODO: figure out how to return Context, error so that baseapp can recover gasWanted/gasUsed diff --git a/x/auth/types/stdtx.go b/x/auth/types/stdtx.go index 945d9afd8695..c712bc6e9b78 100644 --- a/x/auth/types/stdtx.go +++ b/x/auth/types/stdtx.go @@ -107,6 +107,18 @@ func (tx StdTx) GetMemo() string { return tx.Memo } // .Empty(). func (tx StdTx) GetSignatures() []StdSignature { return tx.Signatures } +// Gas returns the Gas in StdFee +func (tx StdTx) Gas() uint64 { return tx.Fee.Gas } + +// FeeCoins returns the FeeAmount in StdFee +func (tx StdTx) FeeCoins() 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 +func (tx StdTx) FeePayer() sdk.AccAddress { + return tx.GetMsgs()[0].GetSigners()[0] +} + //__________________________________________________________ // StdFee includes the amount of coins paid in fees and the maximum From ac8209d1478ce897b4b8db8a66b836afbf012864 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 19 Sep 2019 11:34:43 -0700 Subject: [PATCH 14/32] check signer lenght in sigverify --- x/auth/ante/sigverify.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 56efd6d81fb3..5b6478dc083e 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -116,6 +116,11 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul signerAddrs := stdTx.GetSigners() signerAccs := make([]exported.Account, len(signerAddrs)) + // check that signer length and signature length are the same + if len(stdSigs) != len(signerAddrs) { + return ctx, errs.Wrapf(errs.ErrUnauthorized, "Wrong number of signers. Expected: %d, got %d", len(signerAddrs), len(stdSigs)) + } + for i := 0; i < len(stdSigs); i++ { signerAccs[i], err = GetSignerAcc(ctx, svd.ak, signerAddrs[i]) if err != nil { From b93ca362548c0537cc4b70d58a5d8736eb97bdb6 Mon Sep 17 00:00:00 2001 From: Aditya Date: Sun, 22 Sep 2019 19:41:41 +0000 Subject: [PATCH 15/32] Apply suggestions from bez code review Co-Authored-By: Alexander Bezobchuk --- types/handler.go | 2 +- x/auth/ante/basic.go | 6 +++--- x/auth/ante/sigverify.go | 10 ++++------ 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/types/handler.go b/types/handler.go index 2762c9007257..a9c2c2785ece 100644 --- a/types/handler.go +++ b/types/handler.go @@ -13,7 +13,7 @@ type AnteDecorator interface { } // ChainDecorator chains AnteDecorators together with each element -// wrapping over the decorators further along chain and returns a single AnteHandler +// wrapping over the decorators further along chain and returns a single AnteHandler. // // First element is outermost decorator, last element is innermost decorator func ChainDecorators(chain ...AnteDecorator) AnteHandler { diff --git a/x/auth/ante/basic.go b/x/auth/ante/basic.go index 5d3b5e215bf3..44382d0b8b33 100644 --- a/x/auth/ante/basic.go +++ b/x/auth/ante/basic.go @@ -8,8 +8,8 @@ import ( "github.com/cosmos/cosmos-sdk/x/auth/keeper" ) -// ValidateBasicDecorator will call tx.ValidateBasic and return any non-nil error -// If ValidateBasic passes, decorator calls next AnteHandler in chain +// 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 { @@ -46,7 +46,7 @@ func NewValidateMemoDecorator(ak keeper.AccountKeeper) ValidateMemoDecorator { 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, errs.Wrap(errs.ErrTxDecode, "Tx must have a GetMemo method") + return ctx, errs.Wrap(errs.ErrTxDecode, "invalid transaction type") } params := vmd.ak.GetParams(ctx) diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 5b6478dc083e..66d31e36e28c 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -49,11 +49,10 @@ func NewSigGasConsumeDecorator(ak keeper.AccountKeeper, sigGasConsumer Signature func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { stdTx, ok := tx.(types.StdTx) if !ok { - return ctx, errs.Wrap(errs.ErrTxDecode, "Tx must be a StdTx") + return ctx, errs.Wrap(errs.ErrTxDecode, "invalid transaction type") } params := sgcd.ak.GetParams(ctx) - stdSigs := stdTx.GetSignatures() // stdSigs contains the sequence number, account number, and signatures. @@ -102,7 +101,7 @@ func NewSigVerificationDecorator(ak keeper.AccountKeeper) SigVerificationDecorat func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { stdTx, ok := tx.(types.StdTx) if !ok { - return ctx, errs.Wrap(errs.ErrTxDecode, "Tx must be a StdTx") + return ctx, errs.Wrap(errs.ErrTxDecode, "invalid tx type") } isGenesis := ctx.BlockHeight() == 0 @@ -118,7 +117,7 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul // check that signer length and signature length are the same if len(stdSigs) != len(signerAddrs) { - return ctx, errs.Wrapf(errs.ErrUnauthorized, "Wrong number of signers. Expected: %d, got %d", len(signerAddrs), len(stdSigs)) + return ctx, errs.Wrapf(errs.ErrUnauthorized, "invalid number of signer; expected: %d, got %d", len(signerAddrs), len(stdSigs)) } for i := 0; i < len(stdSigs); i++ { @@ -159,7 +158,6 @@ func (vscd ValidateSigCountDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, sim } params := vscd.ak.GetParams(ctx) - stdSigs := stdTx.GetSignatures() sigCount := 0 @@ -200,7 +198,7 @@ func (spkd SetPubKeyDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b if sig.PubKey != nil { if !bytes.Equal(sig.PubKey.Address(), signers[i]) { return ctx, errs.Wrapf(errs.ErrInvalidPubKey, - "PubKey does not match Signer address %s with signer index: %d", signers[i], i) + "pubkey does not match signer address %s with signer index: %d", signers[i], i) } acc, err := GetSignerAcc(ctx, spkd.ak, signers[i]) From 5825e81f7679334af4d4355f4cba4be1c5539129 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Sun, 29 Sep 2019 15:57:04 -0700 Subject: [PATCH 16/32] complete bez suggestions --- baseapp/baseapp_test.go | 14 +++---- .../adr-010-modular-antehandler.md | 10 ++--- types/handler.go | 14 ++++--- x/auth/ante/ante.go | 4 +- x/auth/ante/ante_test.go | 4 +- x/auth/ante/basic.go | 14 +++---- x/auth/ante/basic_test.go | 8 ++-- x/auth/ante/fee.go | 17 +++++---- x/auth/ante/fee_test.go | 4 +- x/auth/ante/setup.go | 7 ++-- x/auth/ante/setup_test.go | 10 ++--- x/auth/ante/sigverify.go | 37 ++++++++++--------- x/auth/ante/sigverify_test.go | 6 +-- 13 files changed, 76 insertions(+), 73 deletions(-) diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index c5e6ed747ac7..c13ab2b28473 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -19,7 +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" - errs "github.com/cosmos/cosmos-sdk/types/errors" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) var ( @@ -600,12 +600,12 @@ func anteHandlerTxTest(t *testing.T, capKey *sdk.KVStoreKey, storeKey []byte) sd txTest := tx.(txTest) if txTest.FailOnAnte { - return newCtx, errs.Wrap(errs.ErrUnauthorized, "ante handler failure") + return newCtx, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "ante handler failure") } res := incrementingCounter(t, store, storeKey, txTest.Counter) if !res.IsOK() { - err = errs.ABCIError(string(res.Codespace), uint32(res.Code), res.Log) + err = sdkerrors.ABCIError(string(res.Codespace), uint32(res.Code), res.Log) } return } @@ -996,7 +996,7 @@ func TestTxGasLimits(t *testing.T) { switch rType := r.(type) { case sdk.ErrorOutOfGas: log := fmt.Sprintf("out of gas in location: %v", rType.Descriptor) - err = errs.Wrap(errs.ErrOutOfGas, log) + err = sdkerrors.Wrap(sdkerrors.ErrOutOfGas, log) default: panic(r) } @@ -1076,7 +1076,7 @@ func TestMaxBlockGasLimits(t *testing.T) { if r := recover(); r != nil { switch rType := r.(type) { case sdk.ErrorOutOfGas: - err = errs.Wrapf(errs.ErrOutOfGas, "out of gas in location: %v", rType.Descriptor) + err = sdkerrors.Wrapf(sdkerrors.ErrOutOfGas, "out of gas in location: %v", rType.Descriptor) default: panic(r) } @@ -1241,7 +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) - err = errs.Wrap(errs.ErrOutOfGas, log) + err = sdkerrors.Wrap(sdkerrors.ErrOutOfGas, log) default: panic(r) } @@ -1251,7 +1251,7 @@ func TestGasConsumptionBadTx(t *testing.T) { txTest := tx.(txTest) newCtx.GasMeter().ConsumeGas(uint64(txTest.Counter), "counter-ante") if txTest.FailOnAnte { - return newCtx, errs.Wrap(errs.ErrUnauthorized, "ante handler failure") + return newCtx, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "ante handler failure") } return diff --git a/docs/architecture/adr-010-modular-antehandler.md b/docs/architecture/adr-010-modular-antehandler.md index 4322dd963666..ce79b64a4945 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:])) } } ``` @@ -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(NewSetupDecorator(), 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 2762c9007257..5ba9da3bbf59 100644 --- a/types/handler.go +++ b/types/handler.go @@ -16,23 +16,25 @@ type AnteDecorator interface { // wrapping over the decorators further along chain and returns a single AnteHandler // // First element is outermost decorator, last element is innermost decorator -func ChainDecorators(chain ...AnteDecorator) AnteHandler { - chain = append(chain, Tail{}) +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, ChainDecorators(chain[1:]...)) + return chain[0].AnteHandle(ctx, tx, simulate, ChainAnteDecorators(chain[1:]...)) } } -// Tail AnteDecorator will get added to the chain to simplify decorator code +// Terminator AnteDecorator will get added to the chain to simplify decorator code // Don't need to check if next == nil further up the chain -type Tail struct{} +type Terminator struct{} // Simply return provided Context and nil error -func (t Tail) AnteHandle(ctx Context, tx Tx, simulate bool, next AnteHandler) (Context, error) { +func (t Terminator) AnteHandle(ctx Context, tx Tx, simulate bool, next AnteHandler) (Context, error) { return ctx, nil } diff --git a/x/auth/ante/ante.go b/x/auth/ante/ante.go index f21966a57d04..1968cc788b6d 100644 --- a/x/auth/ante/ante.go +++ b/x/auth/ante/ante.go @@ -10,15 +10,15 @@ import ( // 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 sdk.ChainDecorators( + return sdk.ChainAnteDecorators( NewSetupDecorator(), NewMempoolFeeDecorator(), NewValidateBasicDecorator(), NewValidateMemoDecorator(ak), NewConsumeGasForTxSizeDecorator(ak), + NewDeductFeeDecorator(ak, supplyKeeper), NewSetPubKeyDecorator(ak), NewValidateSigCountDecorator(ak), - NewDeductFeeDecorator(ak, supplyKeeper), NewSigGasConsumeDecorator(ak, sigGasConsumer), NewSigVerificationDecorator(ak), ) diff --git a/x/auth/ante/ante_test.go b/x/auth/ante/ante_test.go index 534a1937a55a..dee4c0f5ee82 100644 --- a/x/auth/ante/ante_test.go +++ b/x/auth/ante/ante_test.go @@ -13,7 +13,7 @@ import ( "github.com/tendermint/tendermint/crypto/secp256k1" sdk "github.com/cosmos/cosmos-sdk/types" - errs "github.com/cosmos/cosmos-sdk/types/errors" + 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" @@ -736,7 +736,7 @@ func TestCustomSignatureVerificationGasConsumer(t *testing.T) { meter.ConsumeGas(params.SigVerifyCostED25519, "ante verify: ed25519") return nil default: - return errs.Wrapf(errs.ErrInvalidPubKey, "unrecognized public key type: %T", pubkey) + 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 index 5d3b5e215bf3..fd252bff07fd 100644 --- a/x/auth/ante/basic.go +++ b/x/auth/ante/basic.go @@ -3,7 +3,7 @@ package ante import ( sdk "github.com/cosmos/cosmos-sdk/types" err "github.com/cosmos/cosmos-sdk/types/errors" - errs "github.com/cosmos/cosmos-sdk/types/errors" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/auth/keeper" ) @@ -46,7 +46,7 @@ func NewValidateMemoDecorator(ak keeper.AccountKeeper) ValidateMemoDecorator { 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, errs.Wrap(errs.ErrTxDecode, "Tx must have a GetMemo method") + return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must have a GetMemo method") } params := vmd.ak.GetParams(ctx) @@ -62,19 +62,19 @@ func (vmd ValidateMemoDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate return next(ctx, tx, simulate) } -// ConsumeGasForTxSizeDecorator will take in parameters and consume gas proportional to the size of tx +// ConsumeTxSizeGasDecorator will take in parameters and consume gas proportional to the size of tx // before calling next AnteHandler -type ConsumeGasForTxSizeDecorator struct { +type ConsumeTxSizeGasDecorator struct { ak keeper.AccountKeeper } -func NewConsumeGasForTxSizeDecorator(ak keeper.AccountKeeper) ConsumeGasForTxSizeDecorator { - return ConsumeGasForTxSizeDecorator{ +func NewConsumeGasForTxSizeDecorator(ak keeper.AccountKeeper) ConsumeTxSizeGasDecorator { + return ConsumeTxSizeGasDecorator{ ak: ak, } } -func (cgts ConsumeGasForTxSizeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { +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") diff --git a/x/auth/ante/basic_test.go b/x/auth/ante/basic_test.go index 446975ab55da..e1b90d057b76 100644 --- a/x/auth/ante/basic_test.go +++ b/x/auth/ante/basic_test.go @@ -29,7 +29,7 @@ func TestValidateBasic(t *testing.T) { invalidTx := types.NewTestTx(ctx, msgs, privs, accNums, seqs, fee) vbd := ante.NewValidateBasicDecorator() - antehandler := sdk.ChainDecorators(vbd) + antehandler := sdk.ChainAnteDecorators(vbd) _, err := antehandler(ctx, invalidTx, false) require.NotNil(t, err, "Did not error on invalid tx") @@ -58,7 +58,7 @@ func TestValidateMemo(t *testing.T) { invalidTx := types.NewTestTxWithMemo(ctx, msgs, privs, accNums, seqs, fee, strings.Repeat("01234567890", 500)) vmd := ante.NewValidateMemoDecorator(app.AccountKeeper) - antehandler := sdk.ChainDecorators(vmd) + antehandler := sdk.ChainAnteDecorators(vmd) _, err := antehandler(ctx, invalidTx, false) require.NotNil(t, err, "Did not error on tx with high memo") @@ -75,7 +75,7 @@ func TestConsumeGasForTxSize(t *testing.T) { app, ctx := createTestApp(true) cgtsd := ante.NewConsumeGasForTxSizeDecorator(app.AccountKeeper) - antehandler := sdk.ChainDecorators(cgtsd) + antehandler := sdk.ChainAnteDecorators(cgtsd) params := app.AccountKeeper.GetParams(ctx) txBytes := []byte(strings.Repeat("a", 10)) @@ -94,7 +94,7 @@ func TestConsumeGasForTxSize(t *testing.T) { // 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, "ConsumeGasForTxSizeDecorator returned error: %v", err) + require.Nil(t, err, "ConsumeTxSizeGasDecorator returned error: %v", err) 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 index 12fbc04a2d8e..15ea9e5cab64 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -8,7 +8,7 @@ import ( "github.com/cosmos/cosmos-sdk/x/auth/keeper" "github.com/cosmos/cosmos-sdk/x/auth/types" - errs "github.com/cosmos/cosmos-sdk/types/errors" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) // Tx must implement FeeTx interface to use the FeeDecorators @@ -19,7 +19,8 @@ type FeeTx interface { FeePayer() sdk.AccAddress } -// MempoolFeeDecorator will check if the transaction's fee is at least as large the local validator's minimum gasFee (defined in validator config). +// 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 @@ -33,7 +34,7 @@ func NewMempoolFeeDecorator() 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, errs.Wrap(errs.ErrTxDecode, "Tx must be a FeeTx") + return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") } feeCoins := feeTx.FeeCoins() gas := feeTx.Gas() @@ -55,7 +56,7 @@ func (mfd MempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b } if !feeCoins.IsAnyGTE(requiredFees) { - return ctx, errs.Wrapf(errs.ErrInsufficientFee, "insufficient fees; got: %q required: %q", feeCoins, requiredFees) + return ctx, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %q required: %q", feeCoins, requiredFees) } } } @@ -82,7 +83,7 @@ func NewDeductFeeDecorator(ak keeper.AccountKeeper, sk types.SupplyKeeper) Deduc 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, errs.Wrap(errs.ErrTxDecode, "Tx must be a FeeTx") + return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") } if addr := dfd.supplyKeeper.GetModuleAddress(types.FeeCollectorName); addr == nil { @@ -112,13 +113,13 @@ func DeductFees(supplyKeeper types.SupplyKeeper, ctx sdk.Context, acc exported.A coins := acc.GetCoins() if !fees.IsValid() { - return errs.Wrapf(errs.ErrInsufficientFee, "invalid fee amount: %s", fees) + 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 errs.Wrapf(errs.ErrInsufficientFunds, + return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "insufficient funds to pay for fees; %s < %s", coins, fees) } @@ -126,7 +127,7 @@ func DeductFees(supplyKeeper types.SupplyKeeper, ctx sdk.Context, acc exported.A // such as vesting accounts. spendableCoins := acc.SpendableCoins(blockTime) if _, hasNeg := spendableCoins.SafeSub(fees); hasNeg { - return errs.Wrapf(errs.ErrInsufficientFunds, + return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "insufficient funds to pay for fees; %s < %s", spendableCoins, fees) } diff --git a/x/auth/ante/fee_test.go b/x/auth/ante/fee_test.go index 254652837215..c11e4ce938c2 100644 --- a/x/auth/ante/fee_test.go +++ b/x/auth/ante/fee_test.go @@ -15,7 +15,7 @@ func TestEnsureMempoolFees(t *testing.T) { _, ctx := createTestApp(true) mfd := ante.NewMempoolFeeDecorator() - antehandler := sdk.ChainDecorators(mfd) + antehandler := sdk.ChainAnteDecorators(mfd) // keys and addresses priv1, _, addr1 := types.KeyTestPubAddr() @@ -81,7 +81,7 @@ func TestDeductFees(t *testing.T) { app.AccountKeeper.SetAccount(ctx, acc) dfd := ante.NewDeductFeeDecorator(app.AccountKeeper, app.SupplyKeeper) - antehandler := sdk.ChainDecorators(dfd) + antehandler := sdk.ChainAnteDecorators(dfd) _, err := antehandler(ctx, tx, false) diff --git a/x/auth/ante/setup.go b/x/auth/ante/setup.go index 3c971c112375..3da978eff425 100644 --- a/x/auth/ante/setup.go +++ b/x/auth/ante/setup.go @@ -4,7 +4,7 @@ import ( "fmt" sdk "github.com/cosmos/cosmos-sdk/types" - errs "github.com/cosmos/cosmos-sdk/types/errors" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) // Tx with a Gas() method is needed to use SetupDecorator @@ -30,7 +30,7 @@ func (sud SetUpDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, // Set a gas meter with limit 0 as to prevent an infinite gas meter attack // during runTx. newCtx = SetGasMeter(simulate, ctx, 0) - return newCtx, errs.Wrap(errs.ErrTxDecode, "Tx must be GasTx") + return newCtx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be GasTx") } newCtx = SetGasMeter(simulate, ctx, gasTx.Gas()) @@ -48,8 +48,7 @@ func (sud SetUpDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, "out of gas in location: %v; gasWanted: %d, gasUsed: %d", rType.Descriptor, gasTx.Gas(), newCtx.GasMeter().GasConsumed()) - err = errs.Wrap(errs.ErrOutOfGas, log) - // TODO: figure out how to return Context, error so that baseapp can recover gasWanted/gasUsed + err = sdkerrors.Wrap(sdkerrors.ErrOutOfGas, log) default: panic(r) } diff --git a/x/auth/ante/setup_test.go b/x/auth/ante/setup_test.go index bc7d1719b837..f6fd4aca0005 100644 --- a/x/auth/ante/setup_test.go +++ b/x/auth/ante/setup_test.go @@ -4,7 +4,7 @@ import ( "testing" sdk "github.com/cosmos/cosmos-sdk/types" - errs "github.com/cosmos/cosmos-sdk/types/errors" + sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/auth/ante" "github.com/cosmos/cosmos-sdk/x/auth/types" "github.com/stretchr/testify/require" @@ -28,7 +28,7 @@ func TestSetup(t *testing.T) { tx := types.NewTestTx(ctx, msgs, privs, accNums, seqs, fee) sud := ante.NewSetupDecorator() - antehandler := sdk.ChainDecorators(sud) + antehandler := sdk.ChainAnteDecorators(sud) // Set height to non-zero value for GasMeter to be set ctx = ctx.WithBlockHeight(1) @@ -60,7 +60,7 @@ func TestRecoverPanic(t *testing.T) { tx := types.NewTestTx(ctx, msgs, privs, accNums, seqs, fee) sud := ante.NewSetupDecorator() - antehandler := sdk.ChainDecorators(sud, OutOfGasDecorator{}) + antehandler := sdk.ChainAnteDecorators(sud, OutOfGasDecorator{}) // Set height to non-zero value for GasMeter to be set ctx = ctx.WithBlockHeight(1) @@ -69,10 +69,10 @@ func TestRecoverPanic(t *testing.T) { require.NotNil(t, err, "Did not return error on OutOfGas panic") - require.True(t, errs.ErrOutOfGas.Is(err), "Returned error is not an out of gas error") + 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.ChainDecorators(sud, PanicDecorator{}) + antehandler = sdk.ChainAnteDecorators(sud, PanicDecorator{}) require.Panics(t, func() { antehandler(ctx, tx, false) }, "Recovered from non-Out-of-Gas panic") } diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 5b6478dc083e..5d039e8584f6 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -6,7 +6,7 @@ import ( "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - errs "github.com/cosmos/cosmos-sdk/types/errors" + 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" @@ -28,8 +28,9 @@ func init() { 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 +// 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 // Consume parameter-defined amount of gas for each signature according to the passed-in SignatureVerificationGasConsumer function @@ -49,7 +50,7 @@ func NewSigGasConsumeDecorator(ak keeper.AccountKeeper, sigGasConsumer Signature func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { stdTx, ok := tx.(types.StdTx) if !ok { - return ctx, errs.Wrap(errs.ErrTxDecode, "Tx must be a StdTx") + return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a StdTx") } params := sgcd.ak.GetParams(ctx) @@ -102,7 +103,7 @@ func NewSigVerificationDecorator(ak keeper.AccountKeeper) SigVerificationDecorat func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { stdTx, ok := tx.(types.StdTx) if !ok { - return ctx, errs.Wrap(errs.ErrTxDecode, "Tx must be a StdTx") + return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a StdTx") } isGenesis := ctx.BlockHeight() == 0 @@ -118,7 +119,7 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul // check that signer length and signature length are the same if len(stdSigs) != len(signerAddrs) { - return ctx, errs.Wrapf(errs.ErrUnauthorized, "Wrong number of signers. Expected: %d, got %d", len(signerAddrs), len(stdSigs)) + return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "Wrong number of signers. Expected: %d, got %d", len(signerAddrs), len(stdSigs)) } for i := 0; i < len(stdSigs); i++ { @@ -155,7 +156,7 @@ func NewValidateSigCountDecorator(ak keeper.AccountKeeper) ValidateSigCountDecor func (vscd ValidateSigCountDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { stdTx, ok := tx.(types.StdTx) if !ok { - return ctx, errs.Wrap(errs.ErrTxDecode, "Tx must be a StdTx") + return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a StdTx") } params := vscd.ak.GetParams(ctx) @@ -166,7 +167,7 @@ func (vscd ValidateSigCountDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, sim for i := 0; i < len(stdSigs); i++ { sigCount += types.CountSubKeys(stdSigs[i].PubKey) if uint64(sigCount) > params.TxSigLimit { - return ctx, errs.Wrapf(errs.ErrTooManySignatures, + return ctx, sdkerrors.Wrapf(sdkerrors.ErrTooManySignatures, "signatures: %d, limit: %d", sigCount, params.TxSigLimit) } } @@ -190,7 +191,7 @@ func (spkd SetPubKeyDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b } stdTx, ok := tx.(types.StdTx) if !ok { - return ctx, errs.Wrap(errs.ErrTxDecode, "Tx must be a StdTx") + return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a StdTx") } stdSigs := stdTx.GetSignatures() @@ -199,7 +200,7 @@ func (spkd SetPubKeyDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b for i, sig := range stdSigs { if sig.PubKey != nil { if !bytes.Equal(sig.PubKey.Address(), signers[i]) { - return ctx, errs.Wrapf(errs.ErrInvalidPubKey, + return ctx, sdkerrors.Wrapf(sdkerrors.ErrInvalidPubKey, "PubKey does not match Signer address %s with signer index: %d", signers[i], i) } @@ -209,7 +210,7 @@ func (spkd SetPubKeyDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b } err = acc.SetPubKey(sig.PubKey) if err != nil { - return ctx, errs.Wrap(errs.ErrInvalidPubKey, err.Error()) + return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidPubKey, err.Error()) } spkd.ak.SetAccount(ctx, acc) } @@ -227,7 +228,7 @@ func DefaultSigVerificationGasConsumer( switch pubkey := pubkey.(type) { case ed25519.PubKeyEd25519: meter.ConsumeGas(params.SigVerifyCostED25519, "ante verify: ed25519") - return errs.Wrap(errs.ErrInvalidPubKey, "ED25519 public keys are unsupported") + return sdkerrors.Wrap(sdkerrors.ErrInvalidPubKey, "ED25519 public keys are unsupported") case secp256k1.PubKeySecp256k1: meter.ConsumeGas(params.SigVerifyCostSecp256k1, "ante verify: secp256k1") @@ -241,7 +242,7 @@ func DefaultSigVerificationGasConsumer( return nil default: - return errs.Wrapf(errs.ErrInvalidPubKey, "unrecognized public key type: %T", pubkey) + return sdkerrors.Wrapf(sdkerrors.ErrInvalidPubKey, "unrecognized public key type: %T", pubkey) } } @@ -298,11 +299,11 @@ func ProcessPubKey(acc exported.Account, sig types.StdSignature, simulate bool) if pubKey == nil { pubKey = sig.PubKey if pubKey == nil { - return nil, errs.Wrap(errs.ErrInvalidPubKey, "PubKey not found") + return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidPubKey, "PubKey not found") } if !bytes.Equal(pubKey.Address(), acc.GetAddress()) { - return nil, errs.Wrapf(errs.ErrUnauthorized, + return nil, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "PubKey does not match Signer address %s", acc.GetAddress()) } } @@ -318,11 +319,11 @@ func processSig( pubKey := acc.GetPubKey() if !simulate && pubKey == nil { - return nil, errs.Wrap(errs.ErrInvalidPubKey, "pubkey on account is not set") + return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidPubKey, "pubkey on account is not set") } if !simulate && !pubKey.VerifyBytes(signBytes, sig.Signature) { - return nil, errs.Wrap(errs.ErrUnauthorized, "signature verification failed; verify correct account sequence and chain-id") + return nil, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "signature verification failed; verify correct account sequence and chain-id") } if err = acc.SetSequence(acc.GetSequence() + 1); err != nil { @@ -338,7 +339,7 @@ func GetSignerAcc(ctx sdk.Context, ak keeper.AccountKeeper, addr sdk.AccAddress) if acc := ak.GetAccount(ctx, addr); acc != nil { return acc, nil } - return nil, errs.Wrapf(errs.ErrUnknownAddress, "account %s does not exist", addr) + return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownAddress, "account %s does not exist", addr) } // GetSignBytes returns a slice of bytes to sign over for a given transaction diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index e582a89fd72b..88553a9ff6be 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -41,7 +41,7 @@ func TestSetPubKey(t *testing.T) { tx := types.NewTestTx(ctx, msgs, privs, accNums, seqs, fee) spkd := ante.NewSetPubKeyDecorator(app.AccountKeeper) - antehandler := sdk.ChainDecorators(spkd) + antehandler := sdk.ChainAnteDecorators(spkd) ctx, err := antehandler(ctx, tx, false) require.Nil(t, err) @@ -124,7 +124,7 @@ func TestSigVerification(t *testing.T) { spkd := ante.NewSetPubKeyDecorator(app.AccountKeeper) svd := ante.NewSigVerificationDecorator(app.AccountKeeper) - antehandler := sdk.ChainDecorators(spkd, svd) + antehandler := sdk.ChainAnteDecorators(spkd, svd) _, err := antehandler(ctx, tx, false) require.NotNil(t, err) @@ -174,7 +174,7 @@ func runSigDecorators(t *testing.T, params types.Params, multisig bool, privs .. spkd := ante.NewSetPubKeyDecorator(app.AccountKeeper) svgc := ante.NewSigGasConsumeDecorator(app.AccountKeeper, ante.DefaultSigVerificationGasConsumer) svd := ante.NewSigVerificationDecorator(app.AccountKeeper) - antehandler := sdk.ChainDecorators(spkd, svgc, svd) + antehandler := sdk.ChainAnteDecorators(spkd, svgc, svd) // Determine gas consumption of antehandler with default params before := ctx.GasMeter().GasConsumed() From 2f22251adcab5c8bff22d5ab428c1bdf3a14c554 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Mon, 30 Sep 2019 18:10:37 -0700 Subject: [PATCH 17/32] create sigTx interface --- .../adr-010-modular-antehandler.md | 6 +- x/auth/alias.go | 2 - x/auth/ante/ante.go | 5 +- x/auth/ante/ante_test.go | 71 +++-- x/auth/ante/setup.go | 12 +- x/auth/ante/setup_test.go | 8 +- x/auth/ante/sigverify.go | 270 ++++++++---------- x/auth/client/cli/tx_sign.go | 2 +- x/auth/types/stdtx.go | 34 ++- x/auth/types/stdtx_test.go | 2 +- x/auth/types/txbuilder.go | 2 +- 11 files changed, 212 insertions(+), 202 deletions(-) diff --git a/docs/architecture/adr-010-modular-antehandler.md b/docs/architecture/adr-010-modular-antehandler.md index ce79b64a4945..56b2718b2694 100644 --- a/docs/architecture/adr-010-modular-antehandler.md +++ b/docs/architecture/adr-010-modular-antehandler.md @@ -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 := ChainAnteDecorators(NewSetupDecorator(), NewSigVerifyDecorator(), NewUserDefinedDecorator()) +antehandler := ChainAnteDecorators(NewSetUpContextDecorator(), NewSigVerifyDecorator(), NewUserDefinedDecorator()) // Set chained Antehandler in the baseapp bapp.SetAnteHandler(antehandler) diff --git a/x/auth/alias.go b/x/auth/alias.go index 768e4ad8e4b8..9fe6904d169a 100644 --- a/x/auth/alias.go +++ b/x/auth/alias.go @@ -31,11 +31,9 @@ var ( // functions aliases NewAnteHandler = ante.NewAnteHandler GetSignerAcc = ante.GetSignerAcc - ProcessPubKey = ante.ProcessPubKey DefaultSigVerificationGasConsumer = ante.DefaultSigVerificationGasConsumer DeductFees = ante.DeductFees 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 1968cc788b6d..d8b8a650b7a4 100644 --- a/x/auth/ante/ante.go +++ b/x/auth/ante/ante.go @@ -11,15 +11,16 @@ import ( // signer. func NewAnteHandler(ak keeper.AccountKeeper, supplyKeeper types.SupplyKeeper, sigGasConsumer SignatureVerificationGasConsumer) sdk.AnteHandler { return sdk.ChainAnteDecorators( - NewSetupDecorator(), + NewSetUpContextDecorator(), NewMempoolFeeDecorator(), NewValidateBasicDecorator(), NewValidateMemoDecorator(ak), NewConsumeGasForTxSizeDecorator(ak), - NewDeductFeeDecorator(ak, supplyKeeper), NewSetPubKeyDecorator(ak), NewValidateSigCountDecorator(ak), + NewDeductFeeDecorator(ak, supplyKeeper), NewSigGasConsumeDecorator(ak, sigGasConsumer), NewSigVerificationDecorator(ak), + NewIncrementSequenceDecorator(ak), ) } diff --git a/x/auth/ante/ante_test.go b/x/auth/ante/ante_test.go index dee4c0f5ee82..cdf931e3dc3f 100644 --- a/x/auth/ante/ante_test.go +++ b/x/auth/ante/ante_test.go @@ -15,7 +15,6 @@ import ( 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" ) @@ -518,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) @@ -533,40 +532,40 @@ 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 != nil) - }) - } -} +// 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 != nil) +// }) +// } +// } func generatePubKeysAndSignatures(n int, msg []byte, keyTypeed25519 bool) (pubkeys []crypto.PubKey, signatures [][]byte) { pubkeys = make([]crypto.PubKey, n) diff --git a/x/auth/ante/setup.go b/x/auth/ante/setup.go index 3da978eff425..6cc6592646f7 100644 --- a/x/auth/ante/setup.go +++ b/x/auth/ante/setup.go @@ -7,23 +7,23 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) -// Tx with a Gas() method is needed to use SetupDecorator +// Tx with a Gas() method is needed to use SetUpContextDecorator type GasTx interface { Gas() uint64 } -// SetUpDecorator sets the GasMeter in the Context and wraps the next AnteHandler with a defer clause +// 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 SetUpDecorator struct{} +type SetUpContextDecorator struct{} -func NewSetupDecorator() SetUpDecorator { - return SetUpDecorator{} +func NewSetUpContextDecorator() SetUpContextDecorator { + return SetUpContextDecorator{} } -func (sud SetUpDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { +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 { diff --git a/x/auth/ante/setup_test.go b/x/auth/ante/setup_test.go index f6fd4aca0005..694277e4d249 100644 --- a/x/auth/ante/setup_test.go +++ b/x/auth/ante/setup_test.go @@ -27,7 +27,7 @@ func TestSetup(t *testing.T) { privs, accNums, seqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} tx := types.NewTestTx(ctx, msgs, privs, accNums, seqs, fee) - sud := ante.NewSetupDecorator() + sud := ante.NewSetUpContextDecorator() antehandler := sdk.ChainAnteDecorators(sud) // Set height to non-zero value for GasMeter to be set @@ -37,9 +37,9 @@ func TestSetup(t *testing.T) { require.Equal(t, uint64(0), ctx.GasMeter().Limit(), "GasMeter set with limit before setup") newCtx, err := antehandler(ctx, tx, false) - require.Nil(t, err, "SetupDecorator returned error") + require.Nil(t, err, "SetUpContextDecorator returned error") - // Context GasMeter Limit should be set after SetupDecorator runs + // Context GasMeter Limit should be set after SetUpContextDecorator runs require.Equal(t, fee.Gas, newCtx.GasMeter().Limit(), "GasMeter not set correctly") } @@ -59,7 +59,7 @@ func TestRecoverPanic(t *testing.T) { privs, accNums, seqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0} tx := types.NewTestTx(ctx, msgs, privs, accNums, seqs, fee) - sud := ante.NewSetupDecorator() + sud := ante.NewSetUpContextDecorator() antehandler := sdk.ChainAnteDecorators(sud, OutOfGasDecorator{}) // Set height to non-zero value for GasMeter to be set diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index d7f73e8bb9a4..b952e3540c86 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -33,8 +33,64 @@ func init() { // This is where apps can define their own PubKey type SignatureVerificationGasConsumer = func(meter sdk.GasMeter, sig []byte, pubkey crypto.PubKey, params types.Params) error +type SigVerifiableTx interface { + 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 +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 type SigGasConsumeDecorator struct { ak keeper.AccountKeeper sigGasConsumer SignatureVerificationGasConsumer @@ -48,27 +104,24 @@ func NewSigGasConsumeDecorator(ak keeper.AccountKeeper, sigGasConsumer Signature } func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { - stdTx, ok := tx.(types.StdTx) + sigTx, ok := tx.(SigVerifiableTx) if !ok { return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type") } params := sgcd.ak.GetParams(ctx) - stdSigs := stdTx.GetSignatures() + sigs := sigTx.GetSignatures() // stdSigs contains the sequence number, account number, and signatures. // When simulating, this would just be a 0-length slice. - signerAddrs := stdTx.GetSigners() + signerAddrs := sigTx.GetSigners() - for i, sig := range stdSigs { - pubKey := sig.PubKey - if pubKey == nil { - signerAcc, err := GetSignerAcc(ctx, sgcd.ak, signerAddrs[i]) - if err != nil { - return ctx, err - } - pubKey = signerAcc.GetPubKey() + 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 @@ -76,10 +129,11 @@ func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula // StdSignature (Amino encoding) and simulate gas consumption // (assuming a SECP256k1 simulation key). consumeSimSigGas(ctx.GasMeter(), pubKey, sig, params) - } - err = sgcd.sigGasConsumer(ctx.GasMeter(), sig.Signature, pubKey, params) - if err != nil { - return ctx, err + } else { + err = sgcd.sigGasConsumer(ctx.GasMeter(), sig, pubKey, params) + if err != nil { + return ctx, err + } } } @@ -89,6 +143,7 @@ func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula // 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 type SigVerificationDecorator struct { ak keeper.AccountKeeper } @@ -100,117 +155,107 @@ func NewSigVerificationDecorator(ak keeper.AccountKeeper) SigVerificationDecorat } func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) { - stdTx, ok := tx.(types.StdTx) + sigTx, ok := tx.(SigVerifiableTx) if !ok { return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type") } - isGenesis := ctx.BlockHeight() == 0 - // stdSigs contains the sequence number, account number, and signatures. // When simulating, this would just be a 0-length slice. - stdSigs := stdTx.GetSignatures() + sigs := sigTx.GetSignatures() // stdSigs contains the sequence number, account number, and signatures. // When simulating, this would just be a 0-length slice. - signerAddrs := stdTx.GetSigners() + signerAddrs := sigTx.GetSigners() signerAccs := make([]exported.Account, len(signerAddrs)) // check that signer length and signature length are the same - if len(stdSigs) != len(signerAddrs) { - return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, "invalid number of signer; expected: %d, got %d", len(signerAddrs), len(stdSigs)) + 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 := 0; i < len(stdSigs); i++ { + for i, sig := range sigs { signerAccs[i], err = GetSignerAcc(ctx, svd.ak, signerAddrs[i]) if err != nil { return ctx, err } - // check signature, return account with incremented nonce - signBytes := GetSignBytes(ctx.ChainID(), stdTx, signerAccs[i], isGenesis) - signerAccs[i], err = processSig(ctx, signerAccs[i], stdSigs[i], signBytes, simulate) - 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") } - svd.ak.SetAccount(ctx, signerAccs[i]) + // 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) } -// 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 -type ValidateSigCountDecorator struct { +// Increments sequences of all signers. +// Use this decorator to prevent replay attacks +type IncrementSequenceDecorator struct { ak keeper.AccountKeeper } -func NewValidateSigCountDecorator(ak keeper.AccountKeeper) ValidateSigCountDecorator { - return ValidateSigCountDecorator{ +func NewIncrementSequenceDecorator(ak keeper.AccountKeeper) IncrementSequenceDecorator { + return IncrementSequenceDecorator{ ak: ak, } } -func (vscd ValidateSigCountDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { - stdTx, ok := tx.(types.StdTx) +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, "Tx must be a StdTx") + return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type") } - params := vscd.ak.GetParams(ctx) - stdSigs := stdTx.GetSignatures() - - sigCount := 0 - for i := 0; i < len(stdSigs); i++ { - sigCount += types.CountSubKeys(stdSigs[i].PubKey) - if uint64(sigCount) > params.TxSigLimit { - return ctx, sdkerrors.Wrapf(sdkerrors.ErrTooManySignatures, - "signatures: %d, limit: %d", sigCount, params.TxSigLimit) + // 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) } -type SetPubKeyDecorator struct { +// 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 +type ValidateSigCountDecorator struct { ak keeper.AccountKeeper } -func NewSetPubKeyDecorator(ak keeper.AccountKeeper) SetPubKeyDecorator { - return SetPubKeyDecorator{ +func NewValidateSigCountDecorator(ak keeper.AccountKeeper) ValidateSigCountDecorator { + return ValidateSigCountDecorator{ 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) - } - stdTx, ok := tx.(types.StdTx) +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 StdTx") + return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a sigTx") } - stdSigs := stdTx.GetSignatures() - signers := stdTx.GetSigners() - - for i, sig := range stdSigs { - if sig.PubKey != nil { - if !bytes.Equal(sig.PubKey.Address(), signers[i]) { - return ctx, sdkerrors.Wrapf(sdkerrors.ErrInvalidPubKey, - "pubKey does not match signer address %s with signer index: %d", signers[i], i) - } + params := vscd.ak.GetParams(ctx) + pubKeys := sigTx.GetPubKeys() - acc, err := GetSignerAcc(ctx, spkd.ak, signers[i]) - if err != nil { - return ctx, err - } - err = acc.SetPubKey(sig.PubKey) - if err != nil { - return ctx, sdkerrors.Wrap(sdkerrors.ErrInvalidPubKey, err.Error()) - } - spkd.ak.SetAccount(ctx, acc) + 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) } } @@ -258,9 +303,14 @@ func consumeMultisignatureVerificationGas(meter sdk.GasMeter, } } -func consumeSimSigGas(gasmeter sdk.GasMeter, pubkey crypto.PubKey, sig types.StdSignature, params types.Params) { - simSig := types.StdSignature{PubKey: pubkey} - if len(sig.Signature) == 0 { +// 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[:] } @@ -276,61 +326,6 @@ func consumeSimSigGas(gasmeter sdk.GasMeter, pubkey crypto.PubKey, sig types.Std 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, error) { - // 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, nil - } - - return pubKey, nil - } - - if pubKey == nil { - pubKey = sig.PubKey - if pubKey == nil { - return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidPubKey, "PubKey not found") - } - - if !bytes.Equal(pubKey.Address(), acc.GetAddress()) { - return nil, sdkerrors.Wrapf(sdkerrors.ErrUnauthorized, - "PubKey does not match Signer address %s", acc.GetAddress()) - } - } - - return pubKey, nil -} - -// 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, -) (updatedAcc exported.Account, err error) { - - pubKey := acc.GetPubKey() - if !simulate && pubKey == nil { - return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidPubKey, "pubkey on account is not set") - } - - if !simulate && !pubKey.VerifyBytes(signBytes, sig.Signature) { - return nil, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "signature verification failed; verify correct account sequence and chain-id") - } - - if err = acc.SetSequence(acc.GetSequence() + 1); err != nil { - panic(err) - } - - return acc, nil -} - // 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) { @@ -339,16 +334,3 @@ func GetSignerAcc(ctx sdk.Context, ak keeper.AccountKeeper, addr sdk.AccAddress) } return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownAddress, "account %s does not exist", addr) } - -// 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, - ) -} diff --git a/x/auth/client/cli/tx_sign.go b/x/auth/client/cli/tx_sign.go index 8ae87cb40cef..2c7ad1b33ca2 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 c712bc6e9b78..28f346c8081b 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,37 @@ 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 { + var sigs [][]byte + for _, stdSig := range tx.Signatures { + sigs = append(sigs, 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 { + var pks []crypto.PubKey + for _, stdSig := range tx.Signatures { + pks = append(pks, 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, + ) +} // Gas returns the Gas in StdFee func (tx StdTx) Gas() uint64 { return tx.Fee.Gas } diff --git a/x/auth/types/stdtx_test.go b/x/auth/types/stdtx_test.go index 0cab5a29e88c..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) 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 { From 975ad0ed5f106f18aa1f458a1563c902c4ffc132 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 1 Oct 2019 11:35:11 -0700 Subject: [PATCH 18/32] linting --- x/auth/ante/basic_test.go | 1 - x/auth/ante/sigverify_test.go | 20 ++++++++++---------- x/auth/types/stdtx.go | 12 ++++++------ 3 files changed, 16 insertions(+), 17 deletions(-) diff --git a/x/auth/ante/basic_test.go b/x/auth/ante/basic_test.go index e1b90d057b76..d58b411ca801 100644 --- a/x/auth/ante/basic_test.go +++ b/x/auth/ante/basic_test.go @@ -67,7 +67,6 @@ func TestValidateMemo(t *testing.T) { _, err = antehandler(ctx, validTx, false) require.Nil(t, err, "ValidateBasicDecorator returned error on valid tx. err: %v", err) - } func TestConsumeGasForTxSize(t *testing.T) { diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index 88553a9ff6be..22d47c6d93af 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -26,13 +26,13 @@ func TestSetPubKey(t *testing.T) { addrs := []sdk.AccAddress{addr1, addr2, addr3} pubs := []crypto.PubKey{pub1, pub2, pub3} - var msgs []sdk.Msg + 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 = append(msgs, types.NewTestMsg(addr)) + msgs[i] = types.NewTestMsg(addr) } fee := types.NewTestStdFee() @@ -108,13 +108,13 @@ func TestSigVerification(t *testing.T) { addrs := []sdk.AccAddress{addr1, addr2, addr3} - var msgs []sdk.Msg + 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 = append(msgs, types.NewTestMsg(addr)) + msgs[i] = types.NewTestMsg(addr) } fee := types.NewTestStdFee() @@ -153,18 +153,18 @@ func runSigDecorators(t *testing.T, params types.Params, multisig bool, privs .. ctx = ctx.WithBlockHeight(1) app.AccountKeeper.SetParams(ctx, params) - var msgs []sdk.Msg - var accNums []uint64 - var seqs []uint64 + 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 = append(msgs, types.NewTestMsg(addr)) - accNums = append(accNums, uint64(i)) - seqs = append(seqs, uint64(0)) + msgs[i] = types.NewTestMsg(addr) + accNums[i] = uint64(i) + seqs[i] = uint64(0) } fee := types.NewTestStdFee() diff --git a/x/auth/types/stdtx.go b/x/auth/types/stdtx.go index 28f346c8081b..37eea8b456cc 100644 --- a/x/auth/types/stdtx.go +++ b/x/auth/types/stdtx.go @@ -106,9 +106,9 @@ func (tx StdTx) GetMemo() string { return tx.Memo } // invalid), then the corresponding signature is // .Empty(). func (tx StdTx) GetSignatures() [][]byte { - var sigs [][]byte - for _, stdSig := range tx.Signatures { - sigs = append(sigs, stdSig.Signature) + sigs := make([][]byte, len(tx.Signatures)) + for i, stdSig := range tx.Signatures { + sigs[i] = stdSig.Signature } return sigs } @@ -116,9 +116,9 @@ func (tx StdTx) GetSignatures() [][]byte { // 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 { - var pks []crypto.PubKey - for _, stdSig := range tx.Signatures { - pks = append(pks, stdSig.PubKey) + pks := make([]crypto.PubKey, len(tx.Signatures)) + for i, stdSig := range tx.Signatures { + pks[i] = stdSig.PubKey } return pks } From c5ede73a367ce657ebbeb29e8d8244446664270c Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 1 Oct 2019 12:40:32 -0700 Subject: [PATCH 19/32] finish linting except TODO --- x/auth/ante/sigverify.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index b952e3540c86..b2e6770c732f 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -292,7 +292,6 @@ func DefaultSigVerificationGasConsumer( 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++ { From 9a5cb61ccf6153cbd028f21a55a9e6331f853e3d Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 1 Oct 2019 14:36:26 -0700 Subject: [PATCH 20/32] make auth tx interfaces extend sdk.Tx --- x/auth/ante/setup.go | 1 + x/auth/ante/sigverify.go | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/x/auth/ante/setup.go b/x/auth/ante/setup.go index 6cc6592646f7..03d21c04d8c3 100644 --- a/x/auth/ante/setup.go +++ b/x/auth/ante/setup.go @@ -9,6 +9,7 @@ import ( // Tx with a Gas() method is needed to use SetUpContextDecorator type GasTx interface { + sdk.Tx Gas() uint64 } diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index b2e6770c732f..14075569537c 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -33,7 +33,9 @@ func init() { // 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 @@ -42,6 +44,7 @@ type SigVerifiableTx interface { // 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 } @@ -91,6 +94,7 @@ func (spkd SetPubKeyDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b // 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 @@ -144,6 +148,7 @@ func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula // 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 } @@ -200,6 +205,7 @@ func (svd SigVerificationDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simul // Increments sequences of all signers. // Use this decorator to prevent replay attacks +// CONTRACT: Tx must implement SigVerifiableTx interface type IncrementSequenceDecorator struct { ak keeper.AccountKeeper } @@ -231,6 +237,7 @@ func (isd IncrementSequenceDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, sim // 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 } From 634c1aa493a442dceacb96b8f64e50c72d3e48c9 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 2 Oct 2019 12:09:59 -0700 Subject: [PATCH 21/32] test docs, replace FeeCoins with GetFee --- x/auth/ante/ante_test.go | 35 ----------------------------------- x/auth/ante/basic_test.go | 4 +++- x/auth/ante/fee.go | 12 ++++++------ x/auth/ante/setup.go | 8 ++++---- x/auth/types/stdtx.go | 8 ++++---- 5 files changed, 17 insertions(+), 50 deletions(-) diff --git a/x/auth/ante/ante_test.go b/x/auth/ante/ante_test.go index cdf931e3dc3f..b4391c05fd79 100644 --- a/x/auth/ante/ante_test.go +++ b/x/auth/ante/ante_test.go @@ -532,41 +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 != nil) -// }) -// } -// } - func generatePubKeysAndSignatures(n int, msg []byte, keyTypeed25519 bool) (pubkeys []crypto.PubKey, signatures [][]byte) { pubkeys = make([]crypto.PubKey, n) signatures = make([][]byte, n) diff --git a/x/auth/ante/basic_test.go b/x/auth/ante/basic_test.go index d58b411ca801..db9c1a8b31ef 100644 --- a/x/auth/ante/basic_test.go +++ b/x/auth/ante/basic_test.go @@ -57,6 +57,7 @@ func TestValidateMemo(t *testing.T) { 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) @@ -65,6 +66,7 @@ func TestValidateMemo(t *testing.T) { 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) } @@ -84,7 +86,6 @@ func TestConsumeGasForTxSize(t *testing.T) { 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() @@ -95,6 +96,7 @@ func TestConsumeGasForTxSize(t *testing.T) { 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 index 15ea9e5cab64..bb9be84e9d44 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -14,8 +14,8 @@ import ( // Tx must implement FeeTx interface to use the FeeDecorators type FeeTx interface { sdk.Tx - Gas() uint64 - FeeCoins() sdk.Coins + GetGas() uint64 + GetFee() sdk.Coins FeePayer() sdk.AccAddress } @@ -36,8 +36,8 @@ func (mfd MempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b if !ok { return ctx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx") } - feeCoins := feeTx.FeeCoins() - gas := feeTx.Gas() + 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 @@ -94,8 +94,8 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo feePayerAcc := dfd.ak.GetAccount(ctx, feePayer) // deduct the fees - if !feeTx.FeeCoins().IsZero() { - err = DeductFees(dfd.supplyKeeper, ctx, feePayerAcc, feeTx.FeeCoins()) + if !feeTx.GetFee().IsZero() { + err = DeductFees(dfd.supplyKeeper, ctx, feePayerAcc, feeTx.GetFee()) if err != nil { return ctx, err } diff --git a/x/auth/ante/setup.go b/x/auth/ante/setup.go index 03d21c04d8c3..74b166f675fa 100644 --- a/x/auth/ante/setup.go +++ b/x/auth/ante/setup.go @@ -7,10 +7,10 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) -// Tx with a Gas() method is needed to use SetUpContextDecorator +// Tx with a GetGas() method is needed to use SetUpContextDecorator type GasTx interface { sdk.Tx - Gas() uint64 + GetGas() uint64 } // SetUpContextDecorator sets the GasMeter in the Context and wraps the next AnteHandler with a defer clause @@ -34,7 +34,7 @@ func (sud SetUpContextDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate return newCtx, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be GasTx") } - newCtx = SetGasMeter(simulate, ctx, gasTx.Gas()) + 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 @@ -47,7 +47,7 @@ func (sud SetUpContextDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate case sdk.ErrorOutOfGas: log := fmt.Sprintf( "out of gas in location: %v; gasWanted: %d, gasUsed: %d", - rType.Descriptor, gasTx.Gas(), newCtx.GasMeter().GasConsumed()) + rType.Descriptor, gasTx.GetGas(), newCtx.GasMeter().GasConsumed()) err = sdkerrors.Wrap(sdkerrors.ErrOutOfGas, log) default: diff --git a/x/auth/types/stdtx.go b/x/auth/types/stdtx.go index 37eea8b456cc..575f9abd822b 100644 --- a/x/auth/types/stdtx.go +++ b/x/auth/types/stdtx.go @@ -137,11 +137,11 @@ func (tx StdTx) GetSignBytes(ctx sdk.Context, acc exported.Account) []byte { ) } -// Gas returns the Gas in StdFee -func (tx StdTx) Gas() uint64 { return tx.Fee.Gas } +// GetGas returns the Gas in StdFee +func (tx StdTx) GetGas() uint64 { return tx.Fee.Gas } -// FeeCoins returns the FeeAmount in StdFee -func (tx StdTx) FeeCoins() sdk.Coins { return tx.Fee.Amount } +// 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 From 0c39cd9bb046549f40ce4e71cc8b7353e2895e73 Mon Sep 17 00:00:00 2001 From: Aditya Date: Wed, 2 Oct 2019 15:03:07 -0700 Subject: [PATCH 22/32] Apply suggestions from fede code review Co-Authored-By: Federico Kunze <31522760+fedekunze@users.noreply.github.com> --- x/auth/ante/ante.go | 4 ++-- x/auth/ante/fee.go | 6 +++--- x/auth/ante/setup.go | 2 +- x/auth/ante/sigverify.go | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/x/auth/ante/ante.go b/x/auth/ante/ante.go index d8b8a650b7a4..fdf51949e4d6 100644 --- a/x/auth/ante/ante.go +++ b/x/auth/ante/ante.go @@ -11,7 +11,7 @@ import ( // signer. func NewAnteHandler(ak keeper.AccountKeeper, supplyKeeper types.SupplyKeeper, sigGasConsumer SignatureVerificationGasConsumer) sdk.AnteHandler { return sdk.ChainAnteDecorators( - NewSetUpContextDecorator(), + NewSetUpContextDecorator(), // outermost AnteDecorator NewMempoolFeeDecorator(), NewValidateBasicDecorator(), NewValidateMemoDecorator(ak), @@ -21,6 +21,6 @@ func NewAnteHandler(ak keeper.AccountKeeper, supplyKeeper types.SupplyKeeper, si NewDeductFeeDecorator(ak, supplyKeeper), NewSigGasConsumeDecorator(ak, sigGasConsumer), NewSigVerificationDecorator(ak), - NewIncrementSequenceDecorator(ak), + NewIncrementSequenceDecorator(ak), // innermost AnteDecorator ) } diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index bb9be84e9d44..06bd3aa7ad7f 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -11,7 +11,7 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) -// Tx must implement FeeTx interface to use the FeeDecorators +// FeeTx defines the interface to be implemented by Tx to use the FeeDecorators type FeeTx interface { sdk.Tx GetGas() uint64 @@ -106,8 +106,8 @@ func (dfd DeductFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bo // 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. +// 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() diff --git a/x/auth/ante/setup.go b/x/auth/ante/setup.go index 74b166f675fa..1fca15fd7e70 100644 --- a/x/auth/ante/setup.go +++ b/x/auth/ante/setup.go @@ -7,7 +7,7 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" ) -// Tx with a GetGas() method is needed to use SetUpContextDecorator +// GasTx defines a Tx with a GetGas() method which is needed to use SetUpContextDecorator type GasTx interface { sdk.Tx GetGas() uint64 diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 14075569537c..537d3dd18fd2 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -33,7 +33,7 @@ func init() { // 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 +// SigVerifiableTx defines a Tx interface for all signature verification decorators type SigVerifiableTx interface { sdk.Tx GetSignatures() [][]byte From 29e85bfeaebd4b7501a6cca07467221e7807a8c5 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Thu, 3 Oct 2019 10:49:16 -0700 Subject: [PATCH 23/32] address tim comments --- x/auth/ante/ante_test.go | 53 ---------------------------------------- x/auth/ante/fee.go | 2 +- x/auth/types/stdtx.go | 6 ++++- 3 files changed, 6 insertions(+), 55 deletions(-) diff --git a/x/auth/ante/ante_test.go b/x/auth/ante/ante_test.go index b4391c05fd79..b3d22061b668 100644 --- a/x/auth/ante/ante_test.go +++ b/x/auth/ante/ante_test.go @@ -635,59 +635,6 @@ func TestAnteHandlerSigLimitExceeded(t *testing.T) { 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 diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index bb9be84e9d44..4e7ef28fa866 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -133,7 +133,7 @@ func DeductFees(supplyKeeper types.SupplyKeeper, ctx sdk.Context, acc exported.A err := supplyKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.FeeCollectorName, fees) if err != nil { - return err + return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, err.Error()) } return nil diff --git a/x/auth/types/stdtx.go b/x/auth/types/stdtx.go index 575f9abd822b..fa8b5d100d5f 100644 --- a/x/auth/types/stdtx.go +++ b/x/auth/types/stdtx.go @@ -145,8 +145,12 @@ 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 { - return tx.GetMsgs()[0].GetSigners()[0] + if tx.GetSigners() != nil { + return tx.GetSigners()[0] + } + return sdk.AccAddress{} } //__________________________________________________________ From 0fd1234335efbdded572edcee485138a59b8fea7 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 8 Oct 2019 09:53:55 -0700 Subject: [PATCH 24/32] add order comments --- x/auth/ante/ante.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/auth/ante/ante.go b/x/auth/ante/ante.go index fdf51949e4d6..bdf24823f137 100644 --- a/x/auth/ante/ante.go +++ b/x/auth/ante/ante.go @@ -11,12 +11,12 @@ import ( // signer. func NewAnteHandler(ak keeper.AccountKeeper, supplyKeeper types.SupplyKeeper, sigGasConsumer SignatureVerificationGasConsumer) sdk.AnteHandler { return sdk.ChainAnteDecorators( - NewSetUpContextDecorator(), // outermost AnteDecorator + NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first NewMempoolFeeDecorator(), NewValidateBasicDecorator(), NewValidateMemoDecorator(ak), NewConsumeGasForTxSizeDecorator(ak), - NewSetPubKeyDecorator(ak), + NewSetPubKeyDecorator(ak), // SetPubKeyDecorator must be called before all signature verification decorators NewValidateSigCountDecorator(ak), NewDeductFeeDecorator(ak, supplyKeeper), NewSigGasConsumeDecorator(ak, sigGasConsumer), From aad96733e6b2acb7adccc42cd9cfe6b3a7542a44 Mon Sep 17 00:00:00 2001 From: Aditya Date: Tue, 8 Oct 2019 12:00:41 -0700 Subject: [PATCH 25/32] Add Schwarzenegger art Co-Authored-By: frog power 4000 --- types/handler.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/types/handler.go b/types/handler.go index 27874231cc75..97a57baeed44 100644 --- a/types/handler.go +++ b/types/handler.go @@ -32,6 +32,21 @@ func ChainAnteDecorators(chain ...AnteDecorator) AnteHandler { // 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 From 5ce99dae0a981b204f30c580e769f50935d18b7b Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Tue, 8 Oct 2019 12:06:39 -0700 Subject: [PATCH 26/32] add assertions that StdTx implements all necessary decorator interfaces --- x/auth/ante/basic.go | 5 +++++ x/auth/ante/fee.go | 4 ++++ x/auth/ante/setup.go | 5 +++++ x/auth/ante/sigverify.go | 2 ++ 4 files changed, 16 insertions(+) diff --git a/x/auth/ante/basic.go b/x/auth/ante/basic.go index 343dbfe5faf6..b73374a80012 100644 --- a/x/auth/ante/basic.go +++ b/x/auth/ante/basic.go @@ -6,6 +6,11 @@ import ( 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. diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index 141ff304e73a..1ac1b2c5bdac 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -11,6 +11,10 @@ import ( 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 diff --git a/x/auth/ante/setup.go b/x/auth/ante/setup.go index 1fca15fd7e70..1e3e8eb68d67 100644 --- a/x/auth/ante/setup.go +++ b/x/auth/ante/setup.go @@ -5,6 +5,11 @@ import ( 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 diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index 537d3dd18fd2..b05b8c982f1a 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -20,6 +20,8 @@ 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() { From 2f548764279d8ba6aea2db2066964e673cabca61 Mon Sep 17 00:00:00 2001 From: Aditya Sripal Date: Wed, 9 Oct 2019 15:26:30 -0700 Subject: [PATCH 27/32] documentation and CHANGELOG --- CHANGELOG.md | 20 ++++++++++++++++++++ types/handler.go | 10 +++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf47fd423754..862b167d69ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,10 +55,16 @@ 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. +* (types) `AnteHandler` interface now returns `newCtx Context, err error` instead of `newCtx Context, result sdk.Result, abort bool` +* (x/auth) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) `auth.NewAnteHandler` returns a antehandler function that returns the new AnteHandler interface and has been moved into the `auth/ante` subfolder +* (x/auth) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) ValidateSigCount, ValidateMemo, ProcessPubKey, EnsureSufficientMempoolFee, and GetSignBytes have all been removed as public functions +* (x/auth) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) Invalid Signatures may return `InvalidPubKey` instead of `Unauthorized` error, since the transaction will first hit `SetPubKeyDecorator` before the `SigVerificationDecorator` runs. +* (x/auth) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) `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. Increase GasLimit accordingly ### Features @@ -79,6 +85,20 @@ 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. +* (types) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) `AnteDecorator` interface has been introduced to allow users to implement modular antehandler functionality that can be composed together to create an `AnteHandler` Rather implementing a custom AnteHandler completely from scratch, implement the custom behavior in tightly defined, logically isolated AnteDecorators. These custom AnteDecorators can then be chained together with default AnteDecorators or third-party AnteDecorators 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) +* (types) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) `ChainAnteDecorators` function has been introduced to take in a list of `AnteDecorators` and chain them in sequence and return the final `AnteHandler`. +* (x/auth) A series of default `AnteDecorators` have been provided to replace the old AnteHandler functionality: + * `SetUpContextDecorator`: Sets `GasMeter` in context and creates defer clause to recover from any `OutOfGas` panics in future AnteDecorators and return `OutOfGas` error to baseapp. MUST be the first `AnteDecorator` in the chain for any application that uses gas. + * `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/types/handler.go b/types/handler.go index 97a57baeed44..c32bb677cbd7 100644 --- a/types/handler.go +++ b/types/handler.go @@ -15,7 +15,15 @@ type AnteDecorator interface { // ChainDecorator chains AnteDecorators together with each element // wrapping over the decorators further along chain and returns a single AnteHandler. // -// First element is outermost decorator, last element is innermost decorator +// First element is outermost decorator, last element is innermost decorator. +// Decorating ordering is critical since some decorators will expect certain checks +// and updates to be performed before the decorator is run. These expectations should +// be documented clearly in a CONTRACT docline in the decorator godocs. +// +// 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{}) From a4ef3ec2d98d8a11d37920c282d3f8eaece7dcc4 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 9 Oct 2019 19:55:27 -0400 Subject: [PATCH 28/32] Run goimports --- x/auth/ante/fee_test.go | 5 +++-- x/auth/ante/setup_test.go | 5 +++-- x/auth/ante/sigverify.go | 9 +++++---- x/auth/ante/sigverify_test.go | 7 ++++--- 4 files changed, 15 insertions(+), 11 deletions(-) diff --git a/x/auth/ante/fee_test.go b/x/auth/ante/fee_test.go index c11e4ce938c2..b088c1961951 100644 --- a/x/auth/ante/fee_test.go +++ b/x/auth/ante/fee_test.go @@ -3,11 +3,12 @@ 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" - "github.com/stretchr/testify/require" - "github.com/tendermint/tendermint/crypto" ) func TestEnsureMempoolFees(t *testing.T) { diff --git a/x/auth/ante/setup_test.go b/x/auth/ante/setup_test.go index 694277e4d249..7b7352dbbd26 100644 --- a/x/auth/ante/setup_test.go +++ b/x/auth/ante/setup_test.go @@ -3,12 +3,13 @@ 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" - "github.com/stretchr/testify/require" - "github.com/tendermint/tendermint/crypto" ) func TestSetup(t *testing.T) { diff --git a/x/auth/ante/sigverify.go b/x/auth/ante/sigverify.go index b05b8c982f1a..4b340907e1e8 100644 --- a/x/auth/ante/sigverify.go +++ b/x/auth/ante/sigverify.go @@ -4,16 +4,17 @@ 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" - "github.com/tendermint/tendermint/crypto" - "github.com/tendermint/tendermint/crypto/ed25519" - "github.com/tendermint/tendermint/crypto/multisig" - "github.com/tendermint/tendermint/crypto/secp256k1" ) var ( diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index 22d47c6d93af..82b9f8f62443 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -4,14 +4,15 @@ import ( "fmt" "testing" - sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/auth/ante" - "github.com/cosmos/cosmos-sdk/x/auth/types" "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) { From 33cc17acd915a1410be5b1790deac0e979ea8648 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 9 Oct 2019 20:00:19 -0400 Subject: [PATCH 29/32] Update ChainAnteDecorators godoc --- types/handler.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/types/handler.go b/types/handler.go index c32bb677cbd7..d0cc41ec46dc 100644 --- a/types/handler.go +++ b/types/handler.go @@ -12,27 +12,30 @@ type AnteDecorator interface { AnteHandle(ctx Context, tx Tx, simulate bool, next AnteHandler) (newCtx Context, err error) } -// ChainDecorator chains AnteDecorators together with each element +// ChainDecorator chains AnteDecorators together with each AnteDecorator // wrapping over the decorators further along chain and returns a single AnteHandler. // -// First element is outermost decorator, last element is innermost decorator. -// Decorating ordering is critical since some decorators will expect certain checks -// and updates to be performed before the decorator is run. These expectations should -// be documented clearly in a CONTRACT docline in the decorator godocs. +// 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 +// 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 +// 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:]...)) } @@ -58,6 +61,6 @@ func ChainAnteDecorators(chain ...AnteDecorator) AnteHandler { type Terminator struct{} // Simply return provided Context and nil error -func (t Terminator) AnteHandle(ctx Context, tx Tx, simulate bool, next AnteHandler) (Context, error) { +func (t Terminator) AnteHandle(ctx Context, _ Tx, _ bool, _ AnteHandler) (Context, error) { return ctx, nil } From 882fac5d707db8579566a1c128b9c6b50726c280 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 9 Oct 2019 20:09:40 -0400 Subject: [PATCH 30/32] Changelog entries cleanup --- CHANGELOG.md | 41 ++++++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 862b167d69ac..8abdaafdb714 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,7 +64,10 @@ deprecated and all components removed except the `legacy/` package. ### 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. Increase GasLimit accordingly +* (x/auth) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) The gas required to pass the `AnteHandler` has +increased significantly. Increase GasLimit accordingly. +* (x/auth) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) `StdTx#GetSignatures` now returns `[][]byte` instead of +`[]StdSignature`. ### Features @@ -85,20 +88,28 @@ 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. -* (types) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) `AnteDecorator` interface has been introduced to allow users to implement modular antehandler functionality that can be composed together to create an `AnteHandler` Rather implementing a custom AnteHandler completely from scratch, implement the custom behavior in tightly defined, logically isolated AnteDecorators. These custom AnteDecorators can then be chained together with default AnteDecorators or third-party AnteDecorators 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) -* (types) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) `ChainAnteDecorators` function has been introduced to take in a list of `AnteDecorators` and chain them in sequence and return the final `AnteHandler`. -* (x/auth) A series of default `AnteDecorators` have been provided to replace the old AnteHandler functionality: - * `SetUpContextDecorator`: Sets `GasMeter` in context and creates defer clause to recover from any `OutOfGas` panics in future AnteDecorators and return `OutOfGas` error to baseapp. MUST be the first `AnteDecorator` in the chain for any application that uses gas. - * `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 +* (types) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) 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) +* (types) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) `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 From 163da7fdf4a66dbe85e479ee702fd1aab9152543 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Wed, 9 Oct 2019 20:18:04 -0400 Subject: [PATCH 31/32] Changelog entries cleanup --- CHANGELOG.md | 65 ++++++++++++++++++++++++++++------------------------ 1 file changed, 35 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8abdaafdb714..fc6557776113 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,19 +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. -* (types) `AnteHandler` interface now returns `newCtx Context, err error` instead of `newCtx Context, result sdk.Result, abort bool` -* (x/auth) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) `auth.NewAnteHandler` returns a antehandler function that returns the new AnteHandler interface and has been moved into the `auth/ante` subfolder -* (x/auth) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) ValidateSigCount, ValidateMemo, ProcessPubKey, EnsureSufficientMempoolFee, and GetSignBytes have all been removed as public functions -* (x/auth) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) Invalid Signatures may return `InvalidPubKey` instead of `Unauthorized` error, since the transaction will first hit `SetPubKeyDecorator` before the `SigVerificationDecorator` runs. -* (x/auth) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) `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`. +* (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. Increase GasLimit accordingly. -* (x/auth) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) `StdTx#GetSignatures` now returns `[][]byte` instead of -`[]StdSignature`. +increased significantly due to modular `AnteHandler` support. Increase GasLimit accordingly. ### Features @@ -88,28 +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. -* (types) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) 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) -* (types) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) `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. +* (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 From 1b1a779476ffb3c67a0b28d79cabf7323bf64539 Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Thu, 10 Oct 2019 08:36:02 -0400 Subject: [PATCH 32/32] Fix formatter --- x/auth/ante/fee.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/auth/ante/fee.go b/x/auth/ante/fee.go index 1ac1b2c5bdac..d428cb713593 100644 --- a/x/auth/ante/fee.go +++ b/x/auth/ante/fee.go @@ -60,7 +60,7 @@ func (mfd MempoolFeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b } if !feeCoins.IsAnyGTE(requiredFees) { - return ctx, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %q required: %q", feeCoins, requiredFees) + return ctx, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees) } } }