Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Remove GetSignatures from SigVerifiableTx #6550

Merged
merged 14 commits into from
Sep 2, 2020
Merged

Remove GetSignatures from SigVerifiableTx #6550

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
57a7088
Remove GetSignatures from SigVerifiableTx
aaronc Jun 30, 2020
38c1346
Merge branch 'master' into aaronc/6213-cleanup-sig-api
alexanderbez Jun 30, 2020
691ff6b
Merge branch 'master' of github.com:cosmos/cosmos-sdk into aaronc/621…
sahith-narahari Jul 30, 2020
37b1804
Fix conflicts
sahith-narahari Jul 30, 2020
f310fb9
Merge branch 'master' into aaronc/6213-cleanup-sig-api
sahith-narahari Aug 2, 2020
7be2143
Merge branch 'master' into aaronc/6213-cleanup-sig-api
sahith-narahari Aug 3, 2020
0b4a386
Merge remote-tracking branch 'origin/master' into aaronc/6213-cleanup…
clevinson Aug 31, 2020
af6bca2
update client tests
clevinson Aug 31, 2020
8a3d18e
fix x/auth tests
clevinson Sep 1, 2020
cf2ba63
Merge remote-tracking branch 'origin/master' into aaronc/6213-cleanup…
clevinson Sep 1, 2020
bb5bdbb
add MultiSignatureData test case for ConsumeTxSizeGasDecorator test
clevinson Sep 1, 2020
2cfe6a1
Merge remote-tracking branch 'origin/master' into aaronc/6213-cleanup…
clevinson Sep 1, 2020
3be67fd
Merge branch 'master' into aaronc/6213-cleanup-sig-api
clevinson Sep 2, 2020
9b6a9df
Merge branch 'master' into aaronc/6213-cleanup-sig-api
fedekunze Sep 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
add MultiSignatureData test case for ConsumeTxSizeGasDecorator test
  • Loading branch information
@clevinson
clevinson committed Sep 1, 2020
commit bb5bdbb5dcb02415d49a521a11bf03ad1ed9cb49
4 changes: 3 additions & 1 deletion x/auth/ante/basic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ func (cgts ConsumeTxSizeGasDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, sim
// simulate gas cost for signatures in simulate mode
if simulate {
// in simulate mode, each element should be a nil signature
// TODO: Needs to be able to handle SignaturesV2 which may return nil, Currently this cannot return nil signatures
sigs, err := sigTx.GetSignaturesV2()
if err != nil {
return ctx, err
Expand Down Expand Up @@ -158,6 +157,9 @@ func isIncompleteSignature(data signing.SignatureData) bool {
case *signing.SingleSignatureData:
return len(data.Signature) == 0
case *signing.MultiSignatureData:
if len(data.Signatures) == 0 {
return true
}
for _, s := range data.Signatures {
if isIncompleteSignature(s) {
return true
Expand Down
108 changes: 61 additions & 47 deletions x/auth/ante/basic_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/tendermint/tendermint/crypto"

"github.com/cosmos/cosmos-sdk/crypto/types/multisig"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/auth/ante"
)
Expand Down Expand Up @@ -91,7 +92,6 @@ func (suite *AnteTestSuite) TestValidateMemo() {

func (suite *AnteTestSuite) TestConsumeGasForTxSize() {
suite.SetupTest(true) // setup
suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder()

// keys and addresses
priv1, _, addr1 := testdata.KeyTestPubAddr()
Expand All @@ -100,66 +100,80 @@ func (suite *AnteTestSuite) TestConsumeGasForTxSize() {
msg := testdata.NewTestMsg(addr1)
feeAmount := testdata.NewTestFeeAmount()
gasLimit := testdata.NewTestGasLimit()
suite.Require().NoError(suite.txBuilder.SetMsgs(msg))
suite.txBuilder.SetFeeAmount(feeAmount)
suite.txBuilder.SetGasLimit(gasLimit)
suite.txBuilder.SetMemo(strings.Repeat("01234567890", 10))

privs, accNums, accSeqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0}
tx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID())
suite.Require().NoError(err)

txBytes, err := suite.clientCtx.TxConfig.TxJSONEncoder()(tx)
suite.Require().Nil(err, "Cannot marshal tx: %v", err)

cgtsd := ante.NewConsumeGasForTxSizeDecorator(suite.app.AccountKeeper)
antehandler := sdk.ChainAnteDecorators(cgtsd)

params := suite.app.AccountKeeper.GetParams(suite.ctx)
expectedGas := sdk.Gas(len(txBytes)) * params.TxSizeCostPerByte
testCases := []struct {
name string
sigV2 signing.SignatureV2
}{
{"SingleSignatureData", signing.SignatureV2{PubKey: priv1.PubKey()}},
{"MultiSignatureData", signing.SignatureV2{PubKey: priv1.PubKey(), Data: multisig.NewMultisig(2)}},
}

for _, tc := range testCases {
suite.Run(tc.name, func() {
suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder()
suite.Require().NoError(suite.txBuilder.SetMsgs(msg))
suite.txBuilder.SetFeeAmount(feeAmount)
suite.txBuilder.SetGasLimit(gasLimit)
suite.txBuilder.SetMemo(strings.Repeat("01234567890", 10))

privs, accNums, accSeqs := []crypto.PrivKey{priv1}, []uint64{0}, []uint64{0}
tx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID())
suite.Require().NoError(err)

txBytes, err := suite.clientCtx.TxConfig.TxJSONEncoder()(tx)
suite.Require().Nil(err, "Cannot marshal tx: %v", err)

// Set suite.ctx with TxBytes manually
suite.ctx = suite.ctx.WithTxBytes(txBytes)
params := suite.app.AccountKeeper.GetParams(suite.ctx)
expectedGas := sdk.Gas(len(txBytes)) * params.TxSizeCostPerByte

// track how much gas is necessary to retrieve parameters
beforeGas := suite.ctx.GasMeter().GasConsumed()
suite.app.AccountKeeper.GetParams(suite.ctx)
afterGas := suite.ctx.GasMeter().GasConsumed()
expectedGas += afterGas - beforeGas
// Set suite.ctx with TxBytes manually
suite.ctx = suite.ctx.WithTxBytes(txBytes)

beforeGas = suite.ctx.GasMeter().GasConsumed()
suite.ctx, err = antehandler(suite.ctx, tx, false)
suite.Require().Nil(err, "ConsumeTxSizeGasDecorator returned error: %v", err)
// track how much gas is necessary to retrieve parameters
beforeGas := suite.ctx.GasMeter().GasConsumed()
suite.app.AccountKeeper.GetParams(suite.ctx)
afterGas := suite.ctx.GasMeter().GasConsumed()
expectedGas += afterGas - beforeGas

// require that decorator consumes expected amount of gas
consumedGas := suite.ctx.GasMeter().GasConsumed() - beforeGas
suite.Require().Equal(expectedGas, consumedGas, "Decorator did not consume the correct amount of gas")
beforeGas = suite.ctx.GasMeter().GasConsumed()
suite.ctx, err = antehandler(suite.ctx, tx, false)
suite.Require().Nil(err, "ConsumeTxSizeGasDecorator returned error: %v", err)

// simulation must not underestimate gas of this decorator even with nil signatures
txBuilder, err := suite.clientCtx.TxConfig.WrapTxBuilder(tx)
suite.Require().NoError(err)
suite.Require().NoError(txBuilder.SetSignatures(signing.SignatureV2{
PubKey: priv1.PubKey(),
}))
tx = txBuilder.GetTx()
// require that decorator consumes expected amount of gas
consumedGas := suite.ctx.GasMeter().GasConsumed() - beforeGas
suite.Require().Equal(expectedGas, consumedGas, "Decorator did not consume the correct amount of gas")

// simulation must not underestimate gas of this decorator even with nil signatures
txBuilder, err := suite.clientCtx.TxConfig.WrapTxBuilder(tx)
suite.Require().NoError(err)
suite.Require().NoError(txBuilder.SetSignatures(tc.sigV2))
tx = txBuilder.GetTx()

simTxBytes, err := suite.clientCtx.TxConfig.TxJSONEncoder()(tx)
suite.Require().Nil(err, "Cannot marshal tx: %v", err)
// require that simulated tx is smaller than tx with signatures
suite.Require().True(len(simTxBytes) < len(txBytes), "simulated tx still has signatures")
simTxBytes, err := suite.clientCtx.TxConfig.TxJSONEncoder()(tx)
suite.Require().Nil(err, "Cannot marshal tx: %v", err)
// require that simulated tx is smaller than tx with signatures
suite.Require().True(len(simTxBytes) < len(txBytes), "simulated tx still has signatures")

// Set suite.ctx with smaller simulated TxBytes manually
suite.ctx = suite.ctx.WithTxBytes(simTxBytes)
// Set suite.ctx with smaller simulated TxBytes manually
suite.ctx = suite.ctx.WithTxBytes(simTxBytes)

beforeSimGas := suite.ctx.GasMeter().GasConsumed()
beforeSimGas := suite.ctx.GasMeter().GasConsumed()

// run antehandler with simulate=true
suite.ctx, err = antehandler(suite.ctx, tx, true)
consumedSimGas := suite.ctx.GasMeter().GasConsumed() - beforeSimGas
// run antehandler with simulate=true
suite.ctx, err = antehandler(suite.ctx, tx, true)
consumedSimGas := suite.ctx.GasMeter().GasConsumed() - beforeSimGas

// require that antehandler passes and does not underestimate decorator cost
suite.Require().Nil(err, "ConsumeTxSizeGasDecorator returned error: %v", err)
suite.Require().True(consumedSimGas >= expectedGas, "Simulate mode underestimates gas on AnteDecorator. Simulated cost: %d, expected cost: %d", consumedSimGas, expectedGas)

})
}

// require that antehandler passes and does not underestimate decorator cost
suite.Require().Nil(err, "ConsumeTxSizeGasDecorator returned error: %v", err)
suite.Require().True(consumedSimGas >= expectedGas, "Simulate mode underestimates gas on AnteDecorator. Simulated cost: %d, expected cost: %d", consumedSimGas, expectedGas)
}

func (suite *AnteTestSuite) TestTxHeightTimeoutDecorator() {
Expand Down