Skip to content

Commit

Permalink
refactor(auth): incorporate set pub key decorator into sig verificati…
Browse files Browse the repository at this point in the history
…on decorator (#19093)

Co-authored-by: unknown unknown <unknown@unknown>
Co-authored-by: Aleksandr Bezobchuk <[email protected]>
  • Loading branch information
3 people authored Jan 22, 2024
1 parent a90b55f commit 582bba3
Show file tree
Hide file tree
Showing 9 changed files with 176 additions and 262 deletions.
4 changes: 2 additions & 2 deletions baseapp/block_gas_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,11 +168,11 @@ func TestBaseApp_BlockGas(t *testing.T) {
}
require.Empty(t, okValue)
} else {
require.Equal(t, uint32(0), rsp.TxResults[0].Code)
require.Equal(t, uint32(0), rsp.TxResults[0].Code, "failure", rsp.TxResults[0].Log)
require.Equal(t, []byte("ok"), okValue)
}
// check block gas is always consumed
baseGas := uint64(54436) // baseGas is the gas consumed before tx msg
baseGas := uint64(38798) // baseGas is the gas consumed before tx msg
expGasConsumed := addUint64Saturating(tc.gasToConsume, baseGas)
if expGasConsumed > uint64(simtestutil.DefaultConsensusParams.Block.MaxGas) {
// capped by gasLimit
Expand Down
1 change: 0 additions & 1 deletion simapp/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {
ante.NewValidateMemoDecorator(options.AccountKeeper),
ante.NewConsumeGasForTxSizeDecorator(options.AccountKeeper),
ante.NewDeductFeeDecorator(options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper, options.TxFeeChecker),
ante.NewSetPubKeyDecorator(options.AccountKeeper), // SetPubKeyDecorator must be called before all signature verification decorators
ante.NewValidateSigCountDecorator(options.AccountKeeper),
ante.NewSigVerificationDecorator(options.AccountKeeper, options.SignModeHandler, options.SigGasConsumer),
}
Expand Down
4 changes: 1 addition & 3 deletions tests/integration/bank/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,10 +419,8 @@ func TestMsgSetSendEnabled(t *testing.T) {
},
accSeqs: []uint64{1}, // wrong signer, so this sequence doesn't actually get used.
expInError: []string{
"pubKey does not match signer address",
"cannot be claimed by public key with address",
govAddr,
"with signer index: 0",
"invalid pubkey",
},
},
{
Expand Down
1 change: 1 addition & 0 deletions x/auth/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
### Consensus Breaking Changes

* [#18817](https://github.com/cosmos/cosmos-sdk/pull/18817) SigVerification, GasConsumption, IncreaseSequence ante decorators have all been joined into one SigVerification decorator. Gas consumption during TX validation flow has reduced.
* [#19093](https://github.com/cosmos/cosmos-sdk/pull/19093) SetPubKeyDecorator was merged into SigVerification, gas consumption is almost halved for a simple tx.

### Bug Fixes

Expand Down
1 change: 0 additions & 1 deletion x/auth/ante/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {
NewValidateMemoDecorator(options.AccountKeeper),
NewConsumeGasForTxSizeDecorator(options.AccountKeeper),
NewDeductFeeDecorator(options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper, options.TxFeeChecker),
NewSetPubKeyDecorator(options.AccountKeeper), // SetPubKeyDecorator must be called before all signature verification decorators
NewValidateSigCountDecorator(options.AccountKeeper),
NewSigVerificationDecorator(options.AccountKeeper, options.SignModeHandler, options.SigGasConsumer),
}
Expand Down
77 changes: 1 addition & 76 deletions x/auth/ante/ante_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,25 +166,6 @@ func TestAnteHandlerSigErrors(t *testing.T) {
false,
sdkerrors.ErrUnknownAddress,
},
{
"save the first account, but second is still unrecognized",
func(suite *AnteTestSuite) TestCaseArgs {
suite.accountKeeper.SetAccount(suite.ctx, suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr0))
suite.bankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)

privs, accNums, accSeqs := []cryptotypes.PrivKey{priv0, priv1, priv2}, []uint64{0, 1, 2}, []uint64{0, 0, 0}

return TestCaseArgs{
accNums: accNums,
accSeqs: accSeqs,
msgs: msgs,
privs: privs,
}
},
false,
false,
sdkerrors.ErrUnknownAddress,
},
{
"save all the accounts, should pass",
func(suite *AnteTestSuite) TestCaseArgs {
Expand Down Expand Up @@ -1121,62 +1102,6 @@ func TestAnteHandlerSetPubKey(t *testing.T) {
false,
sdkerrors.ErrWrongSequence,
},
{
"make sure previous public key has been set after wrong signature",
func(suite *AnteTestSuite) TestCaseArgs {
accs := suite.CreateTestAccounts(2)
suite.bankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)

// Make sure public key has not been set from previous test.
acc1 := suite.accountKeeper.GetAccount(suite.ctx, accs[1].acc.GetAddress())
require.Nil(t, acc1.GetPubKey())

privs, accNums, accSeqs := []cryptotypes.PrivKey{accs[1].priv}, []uint64{accs[1].acc.GetAccountNumber()}, []uint64{accs[1].acc.GetSequence()}
msgs := []sdk.Msg{testdata.NewTestMsg(accs[1].acc.GetAddress())}
err := suite.txBuilder.SetMsgs(msgs...)
require.NoError(t, err)
suite.txBuilder.SetFeeAmount(feeAmount)
suite.txBuilder.SetGasLimit(gasLimit)

// Manually create tx, and remove signature.
tx, err := suite.CreateTestTx(suite.ctx, privs, accNums, accSeqs, suite.ctx.ChainID(), signing.SignMode_SIGN_MODE_DIRECT)
require.NoError(t, err)
txBuilder, err := suite.clientCtx.TxConfig.WrapTxBuilder(tx)
require.NoError(t, err)
require.NoError(t, txBuilder.SetSignatures())

// Run anteHandler manually, expect ErrNoSignatures.
_, err = suite.anteHandler(suite.ctx, txBuilder.GetTx(), false)
require.Error(t, err)
require.True(t, errors.Is(err, sdkerrors.ErrNoSignatures))

// Make sure public key has not been set.
acc1 = suite.accountKeeper.GetAccount(suite.ctx, accs[1].acc.GetAddress())
require.Nil(t, acc1.GetPubKey())

// Set incorrect accSeq, to generate incorrect signature.
privs, accNums, accSeqs = []cryptotypes.PrivKey{accs[1].priv}, []uint64{accs[1].acc.GetAccountNumber()}, []uint64{1}

suite.ctx, err = suite.DeliverMsgs(t, privs, msgs, feeAmount, gasLimit, accNums, accSeqs, suite.ctx.ChainID(), false)
require.Error(t, err)

// Make sure public key has been set, as SetPubKeyDecorator
// is called before all signature verification decorators.
acc1 = suite.accountKeeper.GetAccount(suite.ctx, accs[1].acc.GetAddress())
require.Equal(t, acc1.GetPubKey(), accs[1].priv.PubKey())
suite.bankKeeper.EXPECT().SendCoinsFromAccountToModule(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)

return TestCaseArgs{
accNums: accNums,
accSeqs: accSeqs,
msgs: msgs,
privs: privs,
}
},
false,
false,
sdkerrors.ErrWrongSequence,
},
}

for _, tc := range testCases {
Expand Down Expand Up @@ -1206,7 +1131,7 @@ func generatePubKeysAndSignatures(n int, msg []byte, _ bool) (pubkeys []cryptoty
// privkey = ed25519.GenPrivKey()
// } else {
// privkey = secp256k1.GenPrivKey()
//}
// }

pubkeys[i] = privkey.PubKey()
signatures[i], _ = privkey.Sign(msg)
Expand Down
Loading

0 comments on commit 582bba3

Please sign in to comment.