LaconicNetwork/cosmos-sdk
10
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

refactor(auth): group sig verification ante handlers (SigGasConsume, SigVerify, IncreaseSequence) (#18817)

Browse Source 
Co-authored-by: unknown unknown <unknown@unknown>
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
This commit is contained in:
testinginprod 2023-12-20 19:58:09 +01:00 committed by GitHub
parent 935f4f5118
commit 4753d16ef4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 123 additions and 220 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
UPGRADING.md
View File

@ -14,6 +14,11 @@ Note, always read the **SimApp** section for more information on application wir
In this section we describe the changes made in Cosmos SDK' SimApp.
**These changes are directly applicable to your application wiring.**
#### AnteHandlers
The GasConsumptionDecorator and IncreaseSequenceDecorator have been merged with the SigVerificationDecorator, so you'll
need to remove them both from your app.go code, they will yield to unresolvable symbols when compiling.
#### Client (`root.go`)
The `client` package has been refactored to make use of the address codecs (address, validator address, consensus address, etc.).

2
baseapp/block_gas_test.go
View File

@ -174,7 +174,7 @@ func TestBaseApp_BlockGas(t *testing.T) {
require.Equal(t, []byte("ok"), okValue)
}
// check block gas is always consumed
baseGas := uint64(57504) // baseGas is the gas consumed before tx msg
baseGas := uint64(54436) // baseGas is the gas consumed before tx msg
expGasConsumed := addUint64Saturating(tc.gasToConsume, baseGas)
if expGasConsumed > uint64(simtestutil.DefaultConsensusParams.Block.MaxGas) {
// capped by gasLimit

4
simapp/ante.go
View File

@ -42,9 +42,7 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {
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.NewSigGasConsumeDecorator(options.AccountKeeper, options.SigGasConsumer),
ante.NewSigVerificationDecorator(options.AccountKeeper, options.SignModeHandler),
ante.NewIncrementSequenceDecorator(options.AccountKeeper),
ante.NewSigVerificationDecorator(options.AccountKeeper, options.SignModeHandler, options.SigGasConsumer),
}
return sdk.ChainAnteDecorators(anteDecorators...), nil

4
x/auth/CHANGELOG.md
View File

@ -35,4 +35,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
### API Breaking Changes
### 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.
### Bug Fixes

4
x/auth/ante/ante.go
View File

@ -48,9 +48,7 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {
NewDeductFeeDecorator(options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper, options.TxFeeChecker),
NewSetPubKeyDecorator(options.AccountKeeper), // SetPubKeyDecorator must be called before all signature verification decorators
NewValidateSigCountDecorator(options.AccountKeeper),
NewSigGasConsumeDecorator(options.AccountKeeper, options.SigGasConsumer),
NewSigVerificationDecorator(options.AccountKeeper, options.SignModeHandler),
NewIncrementSequenceDecorator(options.AccountKeeper),
NewSigVerificationDecorator(options.AccountKeeper, options.SignModeHandler, options.SigGasConsumer),
}
return sdk.ChainAnteDecorators(anteDecorators...), nil

247
x/auth/ante/sigverify.go
View File

@ -146,97 +146,23 @@ func (spkd SetPubKeyDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate b
return next(ctx, tx, simulate)
}
// Consume parameter-defined amount of gas for each signature according to the passed-in SignatureVerificationGasConsumer function
// before calling the next AnteHandler
// CONTRACT: Pubkeys are set in context for all signers before this decorator runs
// CONTRACT: Tx must implement SigVerifiableTx interface
type SigGasConsumeDecorator struct {
ak AccountKeeper
sigGasConsumer SignatureVerificationGasConsumer
}
func NewSigGasConsumeDecorator(ak AccountKeeper, sigGasConsumer SignatureVerificationGasConsumer) SigGasConsumeDecorator {
if sigGasConsumer == nil {
sigGasConsumer = DefaultSigVerificationGasConsumer
}
return SigGasConsumeDecorator{
ak: ak,
sigGasConsumer: sigGasConsumer,
}
}
func (sgcd SigGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (newCtx sdk.Context, err error) {
sigTx, ok := tx.(authsigning.SigVerifiableTx)
if !ok {
return ctx, errorsmod.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type")
}
params := sgcd.ak.GetParams(ctx)
sigs, err := sigTx.GetSignaturesV2()
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.
signers, err := sigTx.GetSigners()
if err != nil {
return ctx, err
}
for i, sig := range sigs {
signerAcc, err := GetSignerAcc(ctx, sgcd.ak, signers[i])
if err != nil {
return ctx, err
}
pubKey := signerAcc.GetPubKey()
if !simulate && pubKey == nil {
return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "pubkey on account is not set")
}
if err := verifyIsOnCurve(pubKey); err != nil {
return ctx, err
}
// 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 simulate && pubKey == nil {
pubKey = simSecp256k1Pubkey
}
// make a SignatureV2 with PubKey filled in from above
sig = signing.SignatureV2{
PubKey: pubKey,
Data: sig.Data,
Sequence: sig.Sequence,
}
err = sgcd.sigGasConsumer(ctx.GasMeter(), sig, params)
if err != nil {
return ctx, err
}
}
return next(ctx, tx, simulate)
}
// SigVerificationDecorator verifies all signatures for a tx and return an error if any are invalid. Note,
// the SigVerificationDecorator will not check signatures on ReCheck.
// It will also increase the sequence number, and consume gas for signature verification.
//
// CONTRACT: Pubkeys are set in context for all signers before this decorator runs
// CONTRACT: Tx must implement SigVerifiableTx interface
type SigVerificationDecorator struct {
ak AccountKeeper
signModeHandler *txsigning.HandlerMap
sigGasConsumer SignatureVerificationGasConsumer
}
func NewSigVerificationDecorator(ak AccountKeeper, signModeHandler *txsigning.HandlerMap) SigVerificationDecorator {
func NewSigVerificationDecorator(ak AccountKeeper, signModeHandler *txsigning.HandlerMap, sigGasConsumer SignatureVerificationGasConsumer) SigVerificationDecorator {
return SigVerificationDecorator{
ak: ak,
signModeHandler: signModeHandler,
sigGasConsumer: sigGasConsumer,
}
}
@ -341,6 +267,53 @@ func (svd SigVerificationDecorator) authenticate(ctx sdk.Context, tx sdk.Tx, sim
return err
}
err = svd.consumeSignatureGas(ctx, simulate, acc.GetPubKey(), sig)
if err != nil {
return err
}
err = svd.verifySig(ctx, simulate, tx, acc, sig)
if err != nil {
return err
}
err = svd.increaseSequence(ctx, acc)
if err != nil {
return err
}
return nil
}
// consumeSignatureGas will consume gas according to the pub-key being verified.
func (svd SigVerificationDecorator) consumeSignatureGas(
ctx sdk.Context,
simulate bool,
pubKey cryptotypes.PubKey,
signature signing.SignatureV2,
) error {
if simulate && pubKey == nil {
pubKey = simSecp256k1Pubkey
}
// make a SignatureV2 with PubKey filled in from above
signature = signing.SignatureV2{
PubKey: pubKey,
Data: signature.Data,
Sequence: signature.Sequence,
}
err := svd.sigGasConsumer(ctx.GasMeter(), signature, svd.ak.GetParams(ctx))
if err != nil {
return err
}
return nil
}
// verifySig will verify the signature of the provided signer account.
// it will assess:
// - the pub key is on the curve.
// - verify sig
func (svd SigVerificationDecorator) verifySig(ctx sdk.Context, simulate bool, tx sdk.Tx, acc sdk.AccountI, sig signing.SignatureV2) error {
// retrieve pubkey
pubKey := acc.GetPubKey()
if !simulate && pubKey == nil {
@ -358,6 +331,13 @@ func (svd SigVerificationDecorator) authenticate(ctx sdk.Context, tx sdk.Tx, sim
)
}
// we're in simulation mode, or in ReCheckTx, or context is not
// on sig verify tx, then we do not need to verify the signatures
// in the tx.
if simulate || ctx.IsReCheckTx() || !ctx.IsSigverifyTx() {
return nil
}
// retrieve signer data
genesis := ctx.BlockHeight() == 0
chainID := ctx.ChainID()
@ -366,90 +346,47 @@ func (svd SigVerificationDecorator) authenticate(ctx sdk.Context, tx sdk.Tx, sim
accNum = acc.GetAccountNumber()
}
// no need to verify signatures on recheck tx
if !simulate && !ctx.IsReCheckTx() && ctx.IsSigverifyTx() {
anyPk, _ := codectypes.NewAnyWithValue(pubKey)
anyPk, _ := codectypes.NewAnyWithValue(pubKey)
signerData := txsigning.SignerData{
Address: acc.GetAddress().String(),
ChainID: chainID,
AccountNumber: accNum,
Sequence: acc.GetSequence(),
PubKey: &anypb.Any{
TypeUrl: anyPk.TypeUrl,
Value: anyPk.Value,
},
}
adaptableTx, ok := tx.(authsigning.V2AdaptableTx)
if !ok {
return fmt.Errorf("expected tx to implement V2AdaptableTx, got %T", tx)
}
txData := adaptableTx.GetSigningTxData()
err = authsigning.VerifySignature(ctx, pubKey, signerData, sig.Data, svd.signModeHandler, txData)
if err != nil {
var errMsg string
if OnlyLegacyAminoSigners(sig.Data) {
// If all signers are using SIGN_MODE_LEGACY_AMINO, we rely on VerifySignature to check account sequence number,
// and therefore communicate sequence number as a potential cause of error.
errMsg = fmt.Sprintf("signature verification failed; please verify account number (%d), sequence (%d) and chain-id (%s)", accNum, acc.GetSequence(), chainID)
} else {
errMsg = fmt.Sprintf("signature verification failed; please verify account number (%d) and chain-id (%s): (%s)", accNum, chainID, err.Error())
}
return errorsmod.Wrap(sdkerrors.ErrUnauthorized, errMsg)
}
signerData := txsigning.SignerData{
Address: acc.GetAddress().String(),
ChainID: chainID,
AccountNumber: accNum,
Sequence: acc.GetSequence(),
PubKey: &anypb.Any{
TypeUrl: anyPk.TypeUrl,
Value: anyPk.Value,
},
}
adaptableTx, ok := tx.(authsigning.V2AdaptableTx)
if !ok {
return fmt.Errorf("expected tx to implement V2AdaptableTx, got %T", tx)
}
txData := adaptableTx.GetSigningTxData()
err := authsigning.VerifySignature(ctx, pubKey, signerData, sig.Data, svd.signModeHandler, txData)
if err != nil {
var errMsg string
if OnlyLegacyAminoSigners(sig.Data) {
// If all signers are using SIGN_MODE_LEGACY_AMINO, we rely on VerifySignature to check account sequence number,
// and therefore communicate sequence number as a potential cause of error.
errMsg = fmt.Sprintf("signature verification failed; please verify account number (%d), sequence (%d) and chain-id (%s)", accNum, acc.GetSequence(), chainID)
} else {
errMsg = fmt.Sprintf("signature verification failed; please verify account number (%d) and chain-id (%s): (%s)", accNum, chainID, err.Error())
}
return errorsmod.Wrap(sdkerrors.ErrUnauthorized, errMsg)
}
return nil
}
// IncrementSequenceDecorator handles incrementing sequences of all signers.
// Use the IncrementSequenceDecorator decorator to prevent replay attacks. Note,
// there is need to execute IncrementSequenceDecorator on RecheckTx since
// BaseApp.Commit() will set the check state based on the latest header.
//
// NOTE: Since CheckTx and DeliverTx state are managed separately, subsequent and
// sequential txs originating from the same account cannot be handled correctly in
// a reliable way unless sequence numbers are managed and tracked manually by a
// client. It is recommended to instead use multiple messages in a tx.
type IncrementSequenceDecorator struct {
ak AccountKeeper
}
func NewIncrementSequenceDecorator(ak AccountKeeper) IncrementSequenceDecorator {
return IncrementSequenceDecorator{
ak: ak,
}
}
func (isd IncrementSequenceDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) {
sigTx, ok := tx.(authsigning.SigVerifiableTx)
if !ok {
return ctx, errorsmod.Wrap(sdkerrors.ErrTxDecode, "invalid transaction type")
// increaseSequence will increase the sequence number of the account.
func (svd SigVerificationDecorator) increaseSequence(ctx sdk.Context, acc sdk.AccountI) error {
if err := acc.SetSequence(acc.GetSequence() + 1); err != nil {
return err
}
// increment sequence of all signers
signers, err := sigTx.GetSigners()
if err != nil {
return sdk.Context{}, err
}
for _, signer := range signers {
acc := isd.ak.GetAccount(ctx, signer)
if err := acc.SetSequence(acc.GetSequence() + 1); err != nil {
panic(err)
}
pubKey := acc.GetPubKey()
if !simulate && pubKey == nil {
return ctx, errorsmod.Wrap(sdkerrors.ErrInvalidPubKey, "pubkey on account is not set")
}
if err := verifyIsOnCurve(pubKey); err != nil {
return ctx, err
}
isd.ak.SetAccount(ctx, acc)
}
return next(ctx, tx, simulate)
svd.ak.SetAccount(ctx, acc)
return nil
}
// ValidateSigCountDecorator takes in Params and returns errors if there are too many signatures in the tx for the given params

77
x/auth/ante/sigverify_test.go
View File

@ -178,7 +178,8 @@ func TestSigVerification(t *testing.T) {
txConfigOpts,
)
require.NoError(t, err)
svd := ante.NewSigVerificationDecorator(suite.accountKeeper, anteTxConfig.SignModeHandler())
noOpGasConsume := func(_ storetypes.GasMeter, _ signing.SignatureV2, _ types.Params) error { return nil }
svd := ante.NewSigVerificationDecorator(suite.accountKeeper, anteTxConfig.SignModeHandler(), noOpGasConsume)
antehandler := sdk.ChainAnteDecorators(spkd, svd)
defaultSignMode, err := authsign.APISignModeToInternal(anteTxConfig.SignModeHandler().DefaultMode())
require.NoError(t, err)
@ -209,14 +210,15 @@ func TestSigVerification(t *testing.T) {
for i, tc := range testCases {
for _, signMode := range enabledSignModes {
t.Run(fmt.Sprintf("%s with %s", tc.name, signMode), func(t *testing.T) {
suite.ctx = suite.ctx.WithIsReCheckTx(tc.recheck).WithIsSigverifyTx(tc.sigverify)
ctx, _ := suite.ctx.CacheContext()
ctx = ctx.WithIsReCheckTx(tc.recheck).WithIsSigverifyTx(tc.sigverify)
suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder() // Create new txBuilder for each test
require.NoError(t, suite.txBuilder.SetMsgs(msgs...))
suite.txBuilder.SetFeeAmount(feeAmount)
suite.txBuilder.SetGasLimit(gasLimit)
tx, err := suite.CreateTestTx(suite.ctx, tc.privs, tc.accNums, tc.accSeqs, suite.ctx.ChainID(), signMode)
tx, err := suite.CreateTestTx(ctx, tc.privs, tc.accNums, tc.accSeqs, ctx.ChainID(), signMode)
require.NoError(t, err)
if tc.invalidSigs {
txSigs, _ := tx.GetSignaturesV2()
@ -237,12 +239,21 @@ func TestSigVerification(t *testing.T) {
txBytes, err := suite.clientCtx.TxConfig.TxEncoder()(tx)
require.NoError(t, err)
byteCtx := suite.ctx.WithTxBytes(txBytes)
byteCtx := ctx.WithTxBytes(txBytes)
_, err = antehandler(byteCtx, tx, false)
if tc.shouldErr {
require.NotNil(t, err, "TestCase %d: %s did not error as expected", i, tc.name)
} else {
require.Nil(t, err, "TestCase %d: %s errored unexpectedly. Err: %v", i, tc.name, err)
// check account sequence
signers, err := tx.GetSigners()
require.NoError(t, err)
for i, signer := range signers {
wantSeq := tc.accSeqs[i] + 1
acc, err := suite.accountKeeper.Accounts.Get(ctx, signer)
require.NoError(t, err)
require.Equal(t, int(wantSeq), int(acc.GetSequence()))
}
}
})
}
@ -303,9 +314,8 @@ func runSigDecorators(t *testing.T, params types.Params, _ bool, privs ...crypto
require.NoError(t, err)
spkd := ante.NewSetPubKeyDecorator(suite.accountKeeper)
svgc := ante.NewSigGasConsumeDecorator(suite.accountKeeper, ante.DefaultSigVerificationGasConsumer)
svd := ante.NewSigVerificationDecorator(suite.accountKeeper, suite.clientCtx.TxConfig.SignModeHandler())
antehandler := sdk.ChainAnteDecorators(spkd, svgc, svd)
svd := ante.NewSigVerificationDecorator(suite.accountKeeper, suite.clientCtx.TxConfig.SignModeHandler(), ante.DefaultSigVerificationGasConsumer)
antehandler := sdk.ChainAnteDecorators(spkd, svd)
txBytes, err := suite.clientCtx.TxConfig.TxEncoder()(tx)
require.NoError(t, err)
@ -319,53 +329,6 @@ func runSigDecorators(t *testing.T, params types.Params, _ bool, privs ...crypto
return after - before, err
}
func TestIncrementSequenceDecorator(t *testing.T) {
suite := SetupTestSuite(t, true)
suite.txBuilder = suite.clientCtx.TxConfig.NewTxBuilder()
priv, _, addr := testdata.KeyTestPubAddr()
acc := suite.accountKeeper.NewAccountWithAddress(suite.ctx, addr)
require.NoError(t, acc.SetAccountNumber(uint64(50)))
suite.accountKeeper.SetAccount(suite.ctx, acc)
msgs := []sdk.Msg{testdata.NewTestMsg(addr)}
require.NoError(t, suite.txBuilder.SetMsgs(msgs...))
privs := []cryptotypes.PrivKey{priv}
accNums := []uint64{suite.accountKeeper.GetAccount(suite.ctx, addr).GetAccountNumber()}
accSeqs := []uint64{suite.accountKeeper.GetAccount(suite.ctx, addr).GetSequence()}
feeAmount := testdata.NewTestFeeAmount()
gasLimit := testdata.NewTestGasLimit()
suite.txBuilder.SetFeeAmount(feeAmount)
suite.txBuilder.SetGasLimit(gasLimit)
tx, err := suite.CreateTestTx(suite.ctx, privs, accNums, accSeqs, suite.ctx.ChainID(), signing.SignMode_SIGN_MODE_DIRECT)
require.NoError(t, err)
pubKeyDecorator := ante.NewSetPubKeyDecorator(suite.accountKeeper)
IncrementSequenceDecorator := ante.NewIncrementSequenceDecorator(suite.accountKeeper)
antehandler := sdk.ChainAnteDecorators(pubKeyDecorator, IncrementSequenceDecorator)
testCases := []struct {
ctx sdk.Context
simulate bool
expectedSeq uint64
}{
{suite.ctx.WithIsReCheckTx(true), false, 1},
{suite.ctx.WithIsCheckTx(true).WithIsReCheckTx(false), false, 2},
{suite.ctx.WithIsReCheckTx(true), false, 3},
{suite.ctx.WithIsReCheckTx(true), false, 4},
{suite.ctx.WithIsReCheckTx(true), true, 5},
}
for i, tc := range testCases {
t.Run(fmt.Sprintf("%d test", i), func(t *testing.T) {
_, err = antehandler(tc.ctx, tx, tc.simulate)
require.NoError(t, err, "unexpected error; tc #%d, %v", i, tc)
require.Equal(t, tc.expectedSeq, suite.accountKeeper.GetAccount(suite.ctx, addr).GetSequence())
})
}
}
func TestAnteHandlerChecks(t *testing.T) {
suite := SetupTestSuite(t, true)
suite.txBankKeeper.EXPECT().DenomMetadataV2(gomock.Any(), gomock.Any()).Return(&bankv1beta1.QueryDenomMetadataResponse{}, nil).AnyTimes()
@ -408,11 +371,9 @@ func TestAnteHandlerChecks(t *testing.T) {
}
setPubKeyDecorator := ante.NewSetPubKeyDecorator(suite.accountKeeper)
sigGasConsumeDecorator := ante.NewSigGasConsumeDecorator(suite.accountKeeper, ante.DefaultSigVerificationGasConsumer)
sigVerificationDecorator := ante.NewSigVerificationDecorator(suite.accountKeeper, anteTxConfig.SignModeHandler())
IncrementSequenceDecorator := ante.NewIncrementSequenceDecorator(suite.accountKeeper)
sigVerificationDecorator := ante.NewSigVerificationDecorator(suite.accountKeeper, anteTxConfig.SignModeHandler(), ante.DefaultSigVerificationGasConsumer)
anteHandler := sdk.ChainAnteDecorators(setPubKeyDecorator, sigGasConsumeDecorator, sigVerificationDecorator, IncrementSequenceDecorator)
anteHandler := sdk.ChainAnteDecorators(setPubKeyDecorator, sigVerificationDecorator)
type testCase struct {
name string