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.

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

fix: do not use posthandler to handle failed tx sequence increment #92

Merged
merged 2 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions app/ante/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func NewAnteHandler(options HandlerOptions) (sdk.AnteHandler, error) {
ante.NewValidateSigCountDecorator(options.AccountKeeper),
ante.NewSigGasConsumeDecorator(options.AccountKeeper, sigGasConsumer),
NewSigVerificationDecorator(options.AccountKeeper, options.EVMKeeper, options.SignModeHandler),
NewIncrementSequenceDecorator(options.AccountKeeper),
evmante.NewIncrementSequenceDecorator(options.AccountKeeper),
ibcante.NewRedundantRelayDecorator(options.IBCkeeper),
auctionante.NewAuctionDecorator(options.AuctionKeeper, options.TxEncoder, options.MevLane),
}
Expand All @@ -111,6 +111,6 @@ func CreateAnteHandlerForOPinit(ak ante.AccountKeeper, ek *evmkeeper.Keeper, sig
ante.NewValidateSigCountDecorator(ak),
ante.NewSigGasConsumeDecorator(ak, ante.DefaultSigVerificationGasConsumer),
NewSigVerificationDecorator(ak, ek, signModeHandler),
NewIncrementSequenceDecorator(ak),
evmante.NewIncrementSequenceDecorator(ak),
)
}
85 changes: 0 additions & 85 deletions app/ante/sigverify.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"github.com/cosmos/cosmos-sdk/x/auth/types"

"github.com/initia-labs/initia/crypto/ethsecp256k1"
evmtypes "github.com/initia-labs/minievm/x/evm/types"
)

// SigVerificationDecorator verifies all signatures for a tx and return an error if any are invalid. Note,
Expand Down Expand Up @@ -198,87 +197,3 @@ func consumeMultisignatureVerificationGas(

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 orginating 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.
//
// NOTE: When we execute evm messages, it whould handle sequence number increment internally,
// so we need to decrease sequence number if it is used in EVM.
type IncrementSequenceDecorator struct {
ak authante.AccountKeeper
}

func NewIncrementSequenceDecorator(ak authante.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")
}

// 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)
}

isd.ak.SetAccount(ctx, acc)
}

// decrement sequence of all signers which is used in EVM
// when we execute evm messages, it whould handle sequence number increment.
//
// NOTE: Need to revert it if a tx failed. See ../posthandler/sequence.go
if simulate || ctx.ExecMode() == sdk.ExecModeFinalize {
signerMap := make(map[string]bool)
for _, msg := range tx.GetMsgs() {
var caller string
switch msg := msg.(type) {
case *evmtypes.MsgCreate:
caller = msg.Sender
case *evmtypes.MsgCreate2:
caller = msg.Sender
case *evmtypes.MsgCall:
caller = msg.Sender
default:
continue
}

if _, ok := signerMap[caller]; ok {
continue
}
signerMap[caller] = true

callerAccAddr, err := isd.ak.AddressCodec().StringToBytes(caller)
if err != nil {
return ctx, err
}

acc := isd.ak.GetAccount(ctx, callerAccAddr)
if err := acc.SetSequence(acc.GetSequence() - 1); err != nil {
panic(err)
}

isd.ak.SetAccount(ctx, acc)
}
}

return next(ctx, tx, simulate)
}
97 changes: 0 additions & 97 deletions app/ante/sigverify_test.go

This file was deleted.

1 change: 0 additions & 1 deletion app/posthandler/posthandler.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ func NewPostHandler(
ek EVMKeeper,
) sdk.PostHandler {
return sdk.ChainPostDecorators(
NewSequenceIncrementDecorator(ak),
NewGasRefundDecorator(ek),
)
}
Expand Down
59 changes: 0 additions & 59 deletions app/posthandler/sequence.go

This file was deleted.

1 change: 1 addition & 0 deletions x/evm/ante/context_keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ type contextKey int

const (
ContextKeyGasPrices contextKey = iota
ContextKeySequenceIncremented
)
48 changes: 48 additions & 0 deletions x/evm/ante/sequence.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package ante

import (
errorsmod "cosmossdk.io/errors"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
authante "github.com/cosmos/cosmos-sdk/x/auth/ante"
authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
)

// IncrementSequenceDecorator is an AnteDecorator that increments the sequence number
// of all signers in a transaction. It also sets a flag in the context to indicate
// that the sequence has been incremented in the ante handler.
type IncrementSequenceDecorator struct {
ak authante.AccountKeeper
}

func NewIncrementSequenceDecorator(ak authante.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")
}

Check warning on line 28 in x/evm/ante/sequence.go

View check run for this annotation

Codecov / codecov/patch

x/evm/ante/sequence.go#L27-L28

Added lines #L27 - L28 were not covered by tests

// increment sequence of all signers
signers, err := sigTx.GetSigners()
if err != nil {
return sdk.Context{}, err
}

Check warning on line 34 in x/evm/ante/sequence.go

View check run for this annotation

Codecov / codecov/patch

x/evm/ante/sequence.go#L33-L34

Added lines #L33 - L34 were not covered by tests

for _, signer := range signers {
acc := isd.ak.GetAccount(ctx, signer)
if err := acc.SetSequence(acc.GetSequence() + 1); err != nil {
beer-1 marked this conversation as resolved.
Show resolved Hide resolved
panic(err)

Check warning on line 39 in x/evm/ante/sequence.go

View check run for this annotation

Codecov / codecov/patch

x/evm/ante/sequence.go#L39

Added line #L39 was not covered by tests
}
beer-1 marked this conversation as resolved.
Show resolved Hide resolved

isd.ak.SetAccount(ctx, acc)
}

// set a flag in context to indicate that sequence has been incremented in ante handler
ctx = ctx.WithValue(ContextKeySequenceIncremented, true)
return next(ctx, tx, simulate)
}
56 changes: 56 additions & 0 deletions x/evm/ante/sequence_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package ante_test

import (
cryptotypes "github.com/cosmos/cosmos-sdk/crypto/types"
"github.com/cosmos/cosmos-sdk/testutil/testdata"
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/initia-labs/minievm/x/evm/ante"
)

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

priv, _, addr := testdata.KeyTestPubAddr()
acc := suite.app.AccountKeeper.NewAccountWithAddress(suite.ctx, addr)
suite.NoError(acc.SetAccountNumber(uint64(50)))
suite.app.AccountKeeper.SetAccount(suite.ctx, acc)

msgs := []sdk.Msg{testdata.NewTestMsg(addr)}
suite.NoError(suite.txBuilder.SetMsgs(msgs...))
privs := []cryptotypes.PrivKey{priv}
accNums := []uint64{suite.app.AccountKeeper.GetAccount(suite.ctx, addr).GetAccountNumber()}
accSeqs := []uint64{suite.app.AccountKeeper.GetAccount(suite.ctx, addr).GetSequence()}
feeAmount := testdata.NewTestFeeAmount()
gasLimit := testdata.NewTestGasLimit()
suite.txBuilder.SetFeeAmount(feeAmount)
suite.txBuilder.SetGasLimit(gasLimit)

tx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID())
suite.NoError(err)

isd := ante.NewIncrementSequenceDecorator(suite.app.AccountKeeper)
antehandler := sdk.ChainAnteDecorators(isd)

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 {
ctx, err := antehandler(tc.ctx, tx, tc.simulate)
suite.NoError(err, "unexpected error; tc #%d, %v", i, tc)
suite.Equal(tc.expectedSeq, suite.app.AccountKeeper.GetAccount(suite.ctx, addr).GetSequence())

// the flag should be set in the context
suite.NotNil(ctx.Value(ante.ContextKeySequenceIncremented))
}
beer-1 marked this conversation as resolved.
Show resolved Hide resolved
}
Loading
Loading