From 68ee2ebbeaff67f88525c4784fa2a1b8f9b6e8cb Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Fri, 21 Sep 2018 22:28:44 -0700 Subject: [PATCH 01/11] auth: Don't recalculate mempool fees for every msg signer, misc. cleanup This PR begins improving the godocs for the auth module, and begins cleaning up the Ante handler. Additionally we previously calculated if the fee was sufficient for the tx on every single signer. This is now refactored to be more efficient, and have a better logical flow. No changelog entry as this is new to this release. --- x/auth/account.go | 18 +++++--- x/auth/ante.go | 106 ++++++++++++++++++++++++++++------------------ 2 files changed, 76 insertions(+), 48 deletions(-) diff --git a/x/auth/account.go b/x/auth/account.go index ceadd3951067..4a55a48ea3df 100644 --- a/x/auth/account.go +++ b/x/auth/account.go @@ -8,8 +8,12 @@ import ( "github.com/tendermint/tendermint/crypto" ) -// Account is a standard account using a sequence number for replay protection -// and a pubkey for authentication. +// Account is an interface used to store coins at a given address within state. +// It presumes a notion of sequence numbers for replay protection, +// a notion of account numbers for replay protection for previously pruned accounts, +// and a pubkey for authentication purposes. +// +// Many complex conditions can be used in the concrete struct which implements Account. type Account interface { GetAddress() sdk.AccAddress SetAddress(sdk.AccAddress) error // errors if already set. @@ -35,9 +39,11 @@ type AccountDecoder func(accountBytes []byte) (Account, error) var _ Account = (*BaseAccount)(nil) -// BaseAccount - base account structure. -// Extend this by embedding this in your AppAccount. -// See the examples/basecoin/types/account.go for an example. +// BaseAccount - a base account structure. +// This can be extended by embedding within in your AppAccount. +// There are examples of this in: examples/basecoin/types/account.go. +// However one doesn't have to use BaseAccount as long as your struct +// implements Account. type BaseAccount struct { Address sdk.AccAddress `json:"address"` Coins sdk.Coins `json:"coins"` @@ -118,7 +124,7 @@ func (acc *BaseAccount) SetSequence(seq int64) error { //---------------------------------------- // Wire -// Most users shouldn't use this, but this comes handy for tests. +// Most users shouldn't use this, but this comes in handy for tests. func RegisterBaseAccount(cdc *codec.Codec) { cdc.RegisterInterface((*Account)(nil), nil) cdc.RegisterConcrete(&BaseAccount{}, "cosmos-sdk/BaseAccount", nil) diff --git a/x/auth/ante.go b/x/auth/ante.go index 5773e6b1fe6c..efd2d2c75b49 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -12,20 +12,18 @@ import ( ) const ( - deductFeesCost sdk.Gas = 10 - memoCostPerByte sdk.Gas = 1 - ed25519VerifyCost = 59 - secp256k1VerifyCost = 100 - maxMemoCharacters = 100 - feeDeductionGasFactor = 0.001 + deductFeesCost sdk.Gas = 10 + memoCostPerByte sdk.Gas = 1 + ed25519VerifyCost = 59 + secp256k1VerifyCost = 100 + maxMemoCharacters = 100 + gasPrice = 0.001 ) // NewAnteHandler returns an AnteHandler that checks // and increments sequence numbers, checks signatures & account numbers, // and deducts fees from the first signer. -// nolint: gocyclo func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { - return func( ctx sdk.Context, tx sdk.Tx, simulate bool, ) (newCtx sdk.Context, res sdk.Result, abort bool) { @@ -36,13 +34,17 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { return ctx, sdk.ErrInternal("tx must be StdTx").Result(), true } - // set the gas meter - if simulate { - newCtx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) - } else { - newCtx = ctx.WithGasMeter(sdk.NewGasMeter(stdTx.Fee.Gas)) + // Ensure that the provided fees meet a minimum threshold for the validator, if this is a CheckTx. + // This is for mempool purposes, and thus is only ran on check tx. + if ctx.IsCheckTx() && !simulate { + res := ensureSufficientMempoolFees(ctx, stdTx) + if !res.IsOK() { + return newCtx, res, true + } } + newCtx = setGasMeter(simulate, ctx, stdTx) + // 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, @@ -66,52 +68,37 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { if err != nil { return newCtx, err.Result(), true } + // charge gas for the memo + newCtx.GasMeter().ConsumeGas(memoCostPerByte*sdk.Gas(len(stdTx.GetMemo())), "memo") - sigs := stdTx.GetSignatures() // When simulating, this would just be a 0-length slice. + // stdSigs contains the sequence number, account number, and signatures + stdSigs := stdTx.GetSignatures() // When simulating, this would just be a 0-length slice. signerAddrs := stdTx.GetSigners() - msgs := tx.GetMsgs() - // charge gas for the memo - newCtx.GasMeter().ConsumeGas(memoCostPerByte*sdk.Gas(len(stdTx.GetMemo())), "memo") + // create the list of all sign bytes + signBytesList := getSignBytesList(newCtx.ChainID(), stdTx, stdSigs) // Get the sign bytes (requires all account & sequence numbers and the fee) - sequences := make([]int64, len(sigs)) - accNums := make([]int64, len(sigs)) - for i := 0; i < len(sigs); i++ { - sequences[i] = sigs[i].Sequence - accNums[i] = sigs[i].AccountNumber - } - fee := stdTx.Fee - // Check sig and nonce and collect signer accounts. var signerAccs = make([]Account, len(signerAddrs)) - for i := 0; i < len(sigs); i++ { - signerAddr, sig := signerAddrs[i], sigs[i] + for i := 0; i < len(stdSigs); i++ { + signerAddr, sig := signerAddrs[i], stdSigs[i] // check signature, return account with incremented nonce - signBytes := StdSignBytes(newCtx.ChainID(), accNums[i], sequences[i], fee, msgs, stdTx.GetMemo()) - signerAcc, res := processSig(newCtx, am, signerAddr, sig, signBytes, simulate) + signerAcc, res := processSig(newCtx, am, signerAddr, sig, signBytesList[i], simulate) if !res.IsOK() { return newCtx, res, true } - requiredFees := adjustFeesByGas(ctx.MinimumFees(), fee.Gas) - // fees must be greater than the minimum set by the validator adjusted by gas - if ctx.IsCheckTx() && !simulate && !ctx.MinimumFees().IsZero() && fee.Amount.IsLT(requiredFees) { - // validators reject any tx from the mempool with less than the minimum fee per gas * gas factor - return newCtx, sdk.ErrInsufficientFee(fmt.Sprintf( - "insufficient fee, got: %q required: %q", fee.Amount, requiredFees)).Result(), true - } - // first sig pays the fees - // Can this function be moved outside of the loop? - if i == 0 && !fee.Amount.IsZero() { + // TODO: move outside of the loop + if i == 0 && !stdTx.Fee.Amount.IsZero() { newCtx.GasMeter().ConsumeGas(deductFeesCost, "deductFees") - signerAcc, res = deductFees(signerAcc, fee) + signerAcc, res = deductFees(signerAcc, stdTx.Fee) if !res.IsOK() { return newCtx, res, true } - fck.addCollectedFees(newCtx, fee.Amount) + fck.addCollectedFees(newCtx, stdTx.Fee.Amount) } // Save the account. @@ -153,6 +140,7 @@ func validateBasic(tx StdTx) (err sdk.Error) { // verify the signature and increment the sequence. // if the account doesn't have a pubkey, set it. +// TODO: Change this function to already take in the account func processSig( ctx sdk.Context, am AccountMapper, addr sdk.AccAddress, sig StdSignature, signBytes []byte, simulate bool) ( @@ -245,8 +233,9 @@ func consumeSignatureVerificationGas(meter sdk.GasMeter, pubkey crypto.PubKey) { } func adjustFeesByGas(fees sdk.Coins, gas int64) sdk.Coins { - gasCost := int64(float64(gas) * feeDeductionGasFactor) + gasCost := int64(float64(gas) * gasPrice) gasFees := make(sdk.Coins, len(fees)) + // TODO: Make this not price all coins in the same way for i := 0; i < len(fees); i++ { gasFees[i] = sdk.NewInt64Coin(fees[i].Denom, gasCost) } @@ -273,5 +262,38 @@ func deductFees(acc Account, fee StdFee) (Account, sdk.Result) { return acc, sdk.Result{} } +func ensureSufficientMempoolFees(ctx sdk.Context, stdTx StdTx) sdk.Result { + // currently we use a very primitive gas pricing model with a constant gasPrice. + // adjustFeesByGas handles calculating the amount of fees required based on the provided gas. + // TODO: Make the gasPrice not a constant, and account for tx size. + requiredFees := adjustFeesByGas(ctx.MinimumFees(), stdTx.Fee.Gas) + + if !ctx.MinimumFees().IsZero() && stdTx.Fee.Amount.IsLT(requiredFees) { + // validators reject any tx from the mempool with less than the minimum fee per gas * gas factor + return sdk.ErrInsufficientFee(fmt.Sprintf( + "insufficient fee, got: %q required: %q", stdTx.Fee.Amount, requiredFees)).Result() + } + return sdk.Result{} +} + +func setGasMeter(simulate bool, ctx sdk.Context, stdTx StdTx) sdk.Context { + // set the gas meter + if simulate { + return ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) + } + return ctx.WithGasMeter(sdk.NewGasMeter(stdTx.Fee.Gas)) +} + +func getSignBytesList(chainID string, stdTx StdTx, stdSigs []StdSignature) ( + signatureBytesList [][]byte) { + signatureBytesList = make([][]byte, len(stdSigs)) + for i := 0; i < len(stdSigs); i++ { + signatureBytesList[i] = StdSignBytes(chainID, + stdSigs[i].AccountNumber, stdSigs[i].Sequence, + stdTx.Fee, stdTx.Msgs, stdTx.Memo) + } + return +} + // BurnFeeHandler burns all fees (decreasing total supply) func BurnFeeHandler(_ sdk.Context, _ sdk.Tx, _ sdk.Coins) {} From 8097c52b6a0230721339797dfb0f76e90c3ff385 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Fri, 21 Sep 2018 23:09:31 -0700 Subject: [PATCH 02/11] auth: Fix placement of fee deduction --- x/auth/ante.go | 66 ++++++++++++++++++++++++++------------------------ 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/x/auth/ante.go b/x/auth/ante.go index efd2d2c75b49..d38f1c6b5579 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -77,33 +77,30 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { // create the list of all sign bytes signBytesList := getSignBytesList(newCtx.ChainID(), stdTx, stdSigs) + signerAccs, res := getSignerAccs(newCtx, am, signerAddrs) + if !res.IsOK() { + return newCtx, res, true + } - // Get the sign bytes (requires all account & sequence numbers and the fee) - // Check sig and nonce and collect signer accounts. - var signerAccs = make([]Account, len(signerAddrs)) - for i := 0; i < len(stdSigs); i++ { - signerAddr, sig := signerAddrs[i], stdSigs[i] - - // check signature, return account with incremented nonce - signerAcc, res := processSig(newCtx, am, signerAddr, sig, signBytesList[i], simulate) + // first sig pays the fees + if !stdTx.Fee.Amount.IsZero() { + newCtx.GasMeter().ConsumeGas(deductFeesCost, "deductFees") + signerAccs[0], res = deductFees(signerAccs[0], stdTx.Fee) if !res.IsOK() { return newCtx, res, true } + fck.addCollectedFees(newCtx, stdTx.Fee.Amount) + } - // first sig pays the fees - // TODO: move outside of the loop - if i == 0 && !stdTx.Fee.Amount.IsZero() { - newCtx.GasMeter().ConsumeGas(deductFeesCost, "deductFees") - signerAcc, res = deductFees(signerAcc, stdTx.Fee) - if !res.IsOK() { - return newCtx, res, true - } - fck.addCollectedFees(newCtx, stdTx.Fee.Amount) + for i := 0; i < len(stdSigs); i++ { + // check signature, return account with incremented nonce + res = processSig(newCtx, signerAccs[i], stdSigs[i], signBytesList[i], simulate) + if !res.IsOK() { + return newCtx, res, true } // Save the account. - am.SetAccount(newCtx, signerAcc) - signerAccs[i] = signerAcc + am.SetAccount(newCtx, signerAccs[i]) } // cache the signer accounts in the context @@ -138,31 +135,36 @@ func validateBasic(tx StdTx) (err sdk.Error) { return nil } +func getSignerAccs(ctx sdk.Context, am AccountMapper, addrs []sdk.AccAddress) (accs []Account, res sdk.Result) { + accs = make([]Account, len(addrs)) + for i := 0; i < len(accs); i++ { + accs[i] = am.GetAccount(ctx, addrs[i]) + if accs[i] == nil { + return nil, sdk.ErrUnknownAddress(addrs[i].String()).Result() + } + } + return +} + // verify the signature and increment the sequence. // if the account doesn't have a pubkey, set it. // TODO: Change this function to already take in the account -func processSig( - ctx sdk.Context, am AccountMapper, - addr sdk.AccAddress, sig StdSignature, signBytes []byte, simulate bool) ( - acc Account, res sdk.Result) { +func processSig(ctx sdk.Context, + acc Account, sig StdSignature, signBytes []byte, simulate bool) (res sdk.Result) { // Get the account. - acc = am.GetAccount(ctx, addr) - if acc == nil { - return nil, sdk.ErrUnknownAddress(addr.String()).Result() - } accnum := acc.GetAccountNumber() seq := acc.GetSequence() // Check account number. if accnum != sig.AccountNumber { - return nil, sdk.ErrInvalidSequence( + return sdk.ErrInvalidSequence( fmt.Sprintf("Invalid account number. Got %d, expected %d", sig.AccountNumber, accnum)).Result() } // Check sequence number. if seq != sig.Sequence { - return nil, sdk.ErrInvalidSequence( + return sdk.ErrInvalidSequence( fmt.Sprintf("Invalid sequence. Got %d, expected %d", sig.Sequence, seq)).Result() } err := acc.SetSequence(seq + 1) @@ -172,16 +174,16 @@ func processSig( } pubKey, res := processPubKey(acc, sig, simulate) if !res.IsOK() { - return nil, res + return res } err = acc.SetPubKey(pubKey) if err != nil { - return nil, sdk.ErrInternal("setting PubKey on signer's account").Result() + return sdk.ErrInternal("setting PubKey on signer's account").Result() } consumeSignatureVerificationGas(ctx.GasMeter(), pubKey) if !simulate && !pubKey.VerifyBytes(signBytes, sig.Signature) { - return nil, sdk.ErrUnauthorized("signature verification failed").Result() + return sdk.ErrUnauthorized("signature verification failed").Result() } return From 8f817af3b3cb9f3136e842bb1a823053b84c67e5 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Fri, 21 Sep 2018 23:15:08 -0700 Subject: [PATCH 03/11] Verify all sequences and account numbers before any signature verifications --- x/auth/ante.go | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/x/auth/ante.go b/x/auth/ante.go index d38f1c6b5579..437736334e6a 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -81,6 +81,10 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { if !res.IsOK() { return newCtx, res, true } + res = validateAccNumAndSequence(signerAccs, stdSigs) + if !res.IsOK() { + return newCtx, res, true + } // first sig pays the fees if !stdTx.Fee.Amount.IsZero() { @@ -107,7 +111,6 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { newCtx = WithSigners(newCtx, signerAccs) // TODO: tx tags (?) - return newCtx, sdk.Result{GasWanted: stdTx.Fee.Gas}, false // continue... } } @@ -146,27 +149,32 @@ func getSignerAccs(ctx sdk.Context, am AccountMapper, addrs []sdk.AccAddress) (a return } +func validateAccNumAndSequence(accs []Account, sigs []StdSignature) sdk.Result { + for i := 0; i < len(accs); i++ { + accnum := accs[i].GetAccountNumber() + seq := accs[i].GetSequence() + // Check account number. + if accnum != sigs[i].AccountNumber { + return sdk.ErrInvalidSequence( + fmt.Sprintf("Invalid account number. Got %d, expected %d", sigs[i].AccountNumber, accnum)).Result() + } + + // Check sequence number. + if seq != sigs[i].Sequence { + return sdk.ErrInvalidSequence( + fmt.Sprintf("Invalid sequence. Got %d, expected %d", sigs[i].Sequence, seq)).Result() + } + } + return sdk.Result{} +} + // verify the signature and increment the sequence. // if the account doesn't have a pubkey, set it. // TODO: Change this function to already take in the account func processSig(ctx sdk.Context, acc Account, sig StdSignature, signBytes []byte, simulate bool) (res sdk.Result) { - // Get the account. - - accnum := acc.GetAccountNumber() seq := acc.GetSequence() - // Check account number. - if accnum != sig.AccountNumber { - return sdk.ErrInvalidSequence( - fmt.Sprintf("Invalid account number. Got %d, expected %d", sig.AccountNumber, accnum)).Result() - } - - // Check sequence number. - if seq != sig.Sequence { - return sdk.ErrInvalidSequence( - fmt.Sprintf("Invalid sequence. Got %d, expected %d", sig.Sequence, seq)).Result() - } err := acc.SetSequence(seq + 1) if err != nil { // Handle w/ #870 From 9332e4a5345f76fe8d11199862bd6e36b30bab21 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Fri, 21 Sep 2018 23:27:51 -0700 Subject: [PATCH 04/11] Add changelog entry, plus no longer charge gas for charging fees --- CHANGELOG.md | 2 ++ x/auth/ante.go | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 479f5913a82e..834f81f15322 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -109,6 +109,8 @@ IMPROVEMENTS - [baseapp] Allow any alphanumeric character in route - [tools] Remove `rm -rf vendor/` from `make get_vendor_deps` - [x/auth] Recover ErrorOutOfGas panic in order to set sdk.Result attributes correctly + - [x/auth] No longer runs any signature in a multi-msg, if any account/sequence number is wrong. + - [x/auth] No longer charge gas for subtracting fees - [x/bank] Unit tests are now table-driven - [tests] Add tests to example apps in docs - [tests] Fixes ansible scripts to work with AWS too diff --git a/x/auth/ante.go b/x/auth/ante.go index 437736334e6a..c82a15b42bcc 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -12,7 +12,6 @@ import ( ) const ( - deductFeesCost sdk.Gas = 10 memoCostPerByte sdk.Gas = 1 ed25519VerifyCost = 59 secp256k1VerifyCost = 100 @@ -88,7 +87,6 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { // first sig pays the fees if !stdTx.Fee.Amount.IsZero() { - newCtx.GasMeter().ConsumeGas(deductFeesCost, "deductFees") signerAccs[0], res = deductFees(signerAccs[0], stdTx.Fee) if !res.IsOK() { return newCtx, res, true From 199cfe7a3574c54b11d8bce9ef3eb7be719ac4ae Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Fri, 21 Sep 2018 23:42:16 -0700 Subject: [PATCH 05/11] minor cleanup --- CHANGELOG.md | 4 ++-- x/auth/ante.go | 29 ++++++++++++++--------------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 834f81f15322..bb5d1e639a15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -109,8 +109,8 @@ IMPROVEMENTS - [baseapp] Allow any alphanumeric character in route - [tools] Remove `rm -rf vendor/` from `make get_vendor_deps` - [x/auth] Recover ErrorOutOfGas panic in order to set sdk.Result attributes correctly - - [x/auth] No longer runs any signature in a multi-msg, if any account/sequence number is wrong. - - [x/auth] No longer charge gas for subtracting fees + - [x/auth] \#2376 No longer runs any signature in a multi-msg, if any account/sequence number is wrong. + - [x/auth] \#2376 No longer charge gas for subtracting fees - [x/bank] Unit tests are now table-driven - [tests] Add tests to example apps in docs - [tests] Fixes ansible scripts to work with AWS too diff --git a/x/auth/ante.go b/x/auth/ante.go index c82a15b42bcc..e9e10c2fc728 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -96,7 +96,7 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { for i := 0; i < len(stdSigs); i++ { // check signature, return account with incremented nonce - res = processSig(newCtx, signerAccs[i], stdSigs[i], signBytesList[i], simulate) + signerAccs[i], res = processSig(newCtx, signerAccs[i], stdSigs[i], signBytesList[i], simulate) if !res.IsOK() { return newCtx, res, true } @@ -168,31 +168,30 @@ func validateAccNumAndSequence(accs []Account, sigs []StdSignature) sdk.Result { // verify the signature and increment the sequence. // if the account doesn't have a pubkey, set it. -// TODO: Change this function to already take in the account func processSig(ctx sdk.Context, - acc Account, sig StdSignature, signBytes []byte, simulate bool) (res sdk.Result) { - seq := acc.GetSequence() - - err := acc.SetSequence(seq + 1) - if err != nil { - // Handle w/ #870 - panic(err) - } + acc Account, sig StdSignature, signBytes []byte, simulate bool) (updatedAcc Account, res sdk.Result) { pubKey, res := processPubKey(acc, sig, simulate) if !res.IsOK() { - return res + return nil, res } - err = acc.SetPubKey(pubKey) + err := acc.SetPubKey(pubKey) if err != nil { - return sdk.ErrInternal("setting PubKey on signer's account").Result() + return nil, sdk.ErrInternal("setting PubKey on signer's account").Result() } consumeSignatureVerificationGas(ctx.GasMeter(), pubKey) if !simulate && !pubKey.VerifyBytes(signBytes, sig.Signature) { - return sdk.ErrUnauthorized("signature verification failed").Result() + return nil, sdk.ErrUnauthorized("signature verification failed").Result() } - return + // increment the sequence number + err = acc.SetSequence(acc.GetSequence() + 1) + if err != nil { + // Handle w/ #870 + panic(err) + } + + return acc, res } var dummySecp256k1Pubkey secp256k1.PubKeySecp256k1 From a23d454ef8abe279145c6ffc143a72243c0cc1ab Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Mon, 24 Sep 2018 10:26:42 -0700 Subject: [PATCH 06/11] Make function signature more clear --- x/auth/ante.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x/auth/ante.go b/x/auth/ante.go index e9e10c2fc728..ee9290533904 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -291,8 +291,7 @@ func setGasMeter(simulate bool, ctx sdk.Context, stdTx StdTx) sdk.Context { return ctx.WithGasMeter(sdk.NewGasMeter(stdTx.Fee.Gas)) } -func getSignBytesList(chainID string, stdTx StdTx, stdSigs []StdSignature) ( - signatureBytesList [][]byte) { +func getSignBytesList(chainID string, stdTx StdTx, stdSigs []StdSignature) (signatureBytesList [][]byte) { signatureBytesList = make([][]byte, len(stdSigs)) for i := 0; i < len(stdSigs); i++ { signatureBytesList[i] = StdSignBytes(chainID, From b513a95bbbabfd2c3e8683ad7da66738b3db43e3 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Mon, 24 Sep 2018 10:48:23 -0700 Subject: [PATCH 07/11] Remove stdtx.FeePayer() --- PENDING.md | 1 + docs/sdk/core/app3.md | 5 ++--- x/auth/ante.go | 1 + x/auth/stdtx.go | 9 +-------- x/auth/stdtx_test.go | 2 +- 5 files changed, 6 insertions(+), 12 deletions(-) diff --git a/PENDING.md b/PENDING.md index ab0a42d45da7..4e33847aac61 100644 --- a/PENDING.md +++ b/PENDING.md @@ -88,6 +88,7 @@ FEATURES * [simulation] [\#1924](https://github.com/cosmos/cosmos-sdk/issues/1924) allow operations to specify future operations * [simulation] [\#1924](https://github.com/cosmos/cosmos-sdk/issues/1924) Add benchmarking capabilities, with makefile commands "test_sim_gaia_benchmark, test_sim_gaia_profile" * [simulation] [\#2349](https://github.com/cosmos/cosmos-sdk/issues/2349) Add time-based future scheduled operations to simulator + * [x/auth] \#2376 Remove FeePayer() from StdTx * Tendermint diff --git a/docs/sdk/core/app3.md b/docs/sdk/core/app3.md index b84313adc535..595a3694fac5 100644 --- a/docs/sdk/core/app3.md +++ b/docs/sdk/core/app3.md @@ -133,7 +133,7 @@ Now that we have a native model for accounts, it's time to introduce the native ```go // StdTx is a standard way to wrap a Msg with Fee and Signatures. -// NOTE: the first signature is the FeePayer (Signatures must not be nil). +// NOTE: the first signature is the fee payer (Signatures must not be nil). type StdTx struct { Msgs []sdk.Msg `json:"msg"` Fee StdFee `json:"fee"` @@ -259,8 +259,7 @@ the same message could be executed over and over again. The PubKey is required for signature verification, but it is only required in the StdSignature once. From that point on, it will be stored in the account. -The fee is paid by the first address returned by `msg.GetSigners()` for the first `Msg`, -as provided by the `FeePayer(tx Tx) sdk.AccAddress` function. +The fee is paid by the first address returned by `msg.GetSigners()` for the first `Msg`. ## CoinKeeper diff --git a/x/auth/ante.go b/x/auth/ante.go index ee9290533904..31f0d026f795 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -87,6 +87,7 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { // first sig pays the fees if !stdTx.Fee.Amount.IsZero() { + // signerAccs[0] is the fee payer signerAccs[0], res = deductFees(signerAccs[0], stdTx.Fee) if !res.IsOK() { return newCtx, res, true diff --git a/x/auth/stdtx.go b/x/auth/stdtx.go index e38dc0c7ee24..061d47a713f9 100644 --- a/x/auth/stdtx.go +++ b/x/auth/stdtx.go @@ -11,7 +11,7 @@ import ( var _ sdk.Tx = (*StdTx)(nil) // StdTx is a standard way to wrap a Msg with Fee and Signatures. -// NOTE: the first signature is the FeePayer (Signatures must not be nil). +// NOTE: the first signature is the fee payer (Signatures must not be nil). type StdTx struct { Msgs []sdk.Msg `json:"msg"` Fee StdFee `json:"fee"` @@ -63,13 +63,6 @@ func (tx StdTx) GetMemo() string { return tx.Memo } // .Empty(). func (tx StdTx) GetSignatures() []StdSignature { return tx.Signatures } -// FeePayer returns the address responsible for paying the fees -// for the transactions. It's the first address returned by msg.GetSigners(). -// If GetSigners() is empty, this panics. -func FeePayer(tx sdk.Tx) sdk.AccAddress { - return tx.GetMsgs()[0].GetSigners()[0] -} - //__________________________________________________________ // StdFee includes the amount of coins paid in fees and the maximum diff --git a/x/auth/stdtx_test.go b/x/auth/stdtx_test.go index f6e3f8fc21e8..a2c2818a50e4 100644 --- a/x/auth/stdtx_test.go +++ b/x/auth/stdtx_test.go @@ -21,7 +21,7 @@ func TestStdTx(t *testing.T) { require.Equal(t, msgs, tx.GetMsgs()) require.Equal(t, sigs, tx.GetSignatures()) - feePayer := FeePayer(tx) + feePayer := tx.GetSigners()[0] require.Equal(t, addr, feePayer) } From 1c598076df721c72ffd4c78a66c8a7c718bf521f Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Mon, 24 Sep 2018 21:27:53 -0700 Subject: [PATCH 08/11] Fix lint --- cmd/gaia/app/benchmarks/txsize_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/gaia/app/benchmarks/txsize_test.go b/cmd/gaia/app/benchmarks/txsize_test.go index 83e51c041681..186ad87e5867 100644 --- a/cmd/gaia/app/benchmarks/txsize_test.go +++ b/cmd/gaia/app/benchmarks/txsize_test.go @@ -26,7 +26,7 @@ func ExampleTxSendSize() { Outputs: []bank.Output{bank.NewOutput(addr2, coins)}, } sig, _ := priv1.Sign(msg1.GetSignBytes()) - sigs := []auth.StdSignature{auth.StdSignature{nil, sig, 0, 0}} + sigs := []auth.StdSignature{{nil, sig, 0, 0}} tx := auth.NewStdTx([]sdk.Msg{msg1}, auth.NewStdFee(0, coins...), sigs, "") fmt.Println(len(cdc.MustMarshalBinaryBare([]sdk.Msg{msg1}))) fmt.Println(len(cdc.MustMarshalBinaryBare(tx))) From 543cb7fb1631f1e8a9ceac4bce4bc970b71b67d3 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Mon, 24 Sep 2018 21:56:35 -0700 Subject: [PATCH 09/11] amend comment per Jae's suggestion --- x/auth/ante.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/auth/ante.go b/x/auth/ante.go index 31f0d026f795..e088ee8e6162 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -34,7 +34,7 @@ func NewAnteHandler(am AccountMapper, fck FeeCollectionKeeper) sdk.AnteHandler { } // Ensure that the provided fees meet a minimum threshold for the validator, if this is a CheckTx. - // This is for mempool purposes, and thus is only ran on check tx. + // This is only for local mempool purposes, and thus is only ran on check tx. if ctx.IsCheckTx() && !simulate { res := ensureSufficientMempoolFees(ctx, stdTx) if !res.IsOK() { From 344aab80cbf4dc0b0b2b4a3f398d5627ceb379b0 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Mon, 24 Sep 2018 22:05:13 -0700 Subject: [PATCH 10/11] make gasCost use int64 instead of float --- x/auth/ante.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/x/auth/ante.go b/x/auth/ante.go index e088ee8e6162..9bac3d67e496 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -16,7 +16,8 @@ const ( ed25519VerifyCost = 59 secp256k1VerifyCost = 100 maxMemoCharacters = 100 - gasPrice = 0.001 + // how much gas = 1 atom + gasPrice = 1000 ) // NewAnteHandler returns an AnteHandler that checks @@ -241,7 +242,7 @@ func consumeSignatureVerificationGas(meter sdk.GasMeter, pubkey crypto.PubKey) { } func adjustFeesByGas(fees sdk.Coins, gas int64) sdk.Coins { - gasCost := int64(float64(gas) * gasPrice) + gasCost := gas / gasPrice gasFees := make(sdk.Coins, len(fees)) // TODO: Make this not price all coins in the same way for i := 0; i < len(fees); i++ { From 2d10de35912dc60180d85f0cd6edab6a0a628880 Mon Sep 17 00:00:00 2001 From: ValarDragon Date: Wed, 26 Sep 2018 09:25:14 -0700 Subject: [PATCH 11/11] rename variable --- x/auth/ante.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/auth/ante.go b/x/auth/ante.go index 9bac3d67e496..68f6f65408ee 100644 --- a/x/auth/ante.go +++ b/x/auth/ante.go @@ -17,7 +17,7 @@ const ( secp256k1VerifyCost = 100 maxMemoCharacters = 100 // how much gas = 1 atom - gasPrice = 1000 + gasPerUnitCost = 1000 ) // NewAnteHandler returns an AnteHandler that checks @@ -242,7 +242,7 @@ func consumeSignatureVerificationGas(meter sdk.GasMeter, pubkey crypto.PubKey) { } func adjustFeesByGas(fees sdk.Coins, gas int64) sdk.Coins { - gasCost := gas / gasPrice + gasCost := gas / gasPerUnitCost gasFees := make(sdk.Coins, len(fees)) // TODO: Make this not price all coins in the same way for i := 0; i < len(fees); i++ {