From 68afb1e60424f4254efb125bece04da46742ef5d Mon Sep 17 00:00:00 2001 From: Steven Landers Date: Wed, 17 Apr 2024 22:07:44 -0400 Subject: [PATCH] [EVM] Add more ante evm unit tests (#1561) * add more ante tests * ocd spacing of imports * ocd spacing of imports --- x/evm/ante/basic.go | 77 +++++++++++++++++------------------ x/evm/ante/error_test.go | 7 ++-- x/evm/ante/preprocess_test.go | 54 ++++++++++++++++++++++++ x/evm/ante/router_test.go | 25 ++++++++++++ 4 files changed, 121 insertions(+), 42 deletions(-) diff --git a/x/evm/ante/basic.go b/x/evm/ante/basic.go index e9db85dca..f3f7b45d0 100644 --- a/x/evm/ante/basic.go +++ b/x/evm/ante/basic.go @@ -1,16 +1,14 @@ package ante import ( - "crypto/sha256" "fmt" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" - "github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/core" ethtypes "github.com/ethereum/go-ethereum/core/types" - "github.com/ethereum/go-ethereum/crypto/kzg4844" "github.com/ethereum/go-ethereum/params" + evmtypes "github.com/sei-protocol/sei-chain/x/evm/types" ) @@ -70,39 +68,40 @@ func (gl BasicDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, n return next(ctx, tx, simulate) } -//nolint:deadcode -func validateBlobSidecar(hashes []common.Hash, sidecar *ethtypes.BlobTxSidecar) error { - if len(sidecar.Blobs) != len(hashes) { - return fmt.Errorf("invalid number of %d blobs compared to %d blob hashes", len(sidecar.Blobs), len(hashes)) - } - if len(sidecar.Commitments) != len(hashes) { - return fmt.Errorf("invalid number of %d blob commitments compared to %d blob hashes", len(sidecar.Commitments), len(hashes)) - } - if len(sidecar.Proofs) != len(hashes) { - return fmt.Errorf("invalid number of %d blob proofs compared to %d blob hashes", len(sidecar.Proofs), len(hashes)) - } - // Blob quantities match up, validate that the provers match with the - // transaction hash before getting to the cryptography - hasher := sha256.New() - for i, want := range hashes { - hasher.Write(sidecar.Commitments[i][:]) - hash := hasher.Sum(nil) - hasher.Reset() - - var vhash common.Hash - vhash[0] = params.BlobTxHashVersion - copy(vhash[1:], hash[1:]) - - if vhash != want { - return fmt.Errorf("blob %d: computed hash %#x mismatches transaction one %#x", i, vhash, want) - } - } - // Blob commitments match with the hashes in the transaction, verify the - // blobs themselves via KZG - for i := range sidecar.Blobs { - if err := kzg4844.VerifyBlobProof(sidecar.Blobs[i], sidecar.Commitments[i], sidecar.Proofs[i]); err != nil { - return fmt.Errorf("invalid blob %d: %v", i, err) - } - } - return nil -} +// NOTE: if we re-enable blob support we can put this back. +// this allows code coverage to calculate correctly +//func validateBlobSidecar(hashes []common.Hash, sidecar *ethtypes.BlobTxSidecar) error { +// if len(sidecar.Blobs) != len(hashes) { +// return fmt.Errorf("invalid number of %d blobs compared to %d blob hashes", len(sidecar.Blobs), len(hashes)) +// } +// if len(sidecar.Commitments) != len(hashes) { +// return fmt.Errorf("invalid number of %d blob commitments compared to %d blob hashes", len(sidecar.Commitments), len(hashes)) +// } +// if len(sidecar.Proofs) != len(hashes) { +// return fmt.Errorf("invalid number of %d blob proofs compared to %d blob hashes", len(sidecar.Proofs), len(hashes)) +// } +// // Blob quantities match up, validate that the provers match with the +// // transaction hash before getting to the cryptography +// hasher := sha256.New() +// for i, want := range hashes { +// hasher.Write(sidecar.Commitments[i][:]) +// hash := hasher.Sum(nil) +// hasher.Reset() +// +// var vhash common.Hash +// vhash[0] = params.BlobTxHashVersion +// copy(vhash[1:], hash[1:]) +// +// if vhash != want { +// return fmt.Errorf("blob %d: computed hash %#x mismatches transaction one %#x", i, vhash, want) +// } +// } +// // Blob commitments match with the hashes in the transaction, verify the +// // blobs themselves via KZG +// for i := range sidecar.Blobs { +// if err := kzg4844.VerifyBlobProof(sidecar.Blobs[i], sidecar.Commitments[i], sidecar.Proofs[i]); err != nil { +// return fmt.Errorf("invalid blob %d: %v", i, err) +// } +// } +// return nil +//} diff --git a/x/evm/ante/error_test.go b/x/evm/ante/error_test.go index 401b91565..8b312a64f 100644 --- a/x/evm/ante/error_test.go +++ b/x/evm/ante/error_test.go @@ -98,11 +98,12 @@ func TestAnteErrorHandler_Handle(t *testing.T) { name: "error should not append error if data of tx cannot be decoded (not an evm message)", handlerErr: testErr, txResultCode: code.CodeTypeUnknownError, - tx: &sdk.ABCIMessageLog{ // this isn't a tx, so it'll fail to parse - Log: "string", + tx: &sdk.GasInfo{ // not a valid eth tx, just a random proto so it will fail + GasWanted: 100, + GasUsed: 100, }, assertions: func(t *testing.T, ctx sdk.Context, k *keeper.Keeper, err error) { - require.ErrorIs(t, err, testErr) + require.Error(t, err, "failed to unpack message data") require.Len(t, k.GetEVMTxDeferredInfo(ctx), 0) }, }, diff --git a/x/evm/ante/preprocess_test.go b/x/evm/ante/preprocess_test.go index f687b2c8f..9ec2f23a6 100644 --- a/x/evm/ante/preprocess_test.go +++ b/x/evm/ante/preprocess_test.go @@ -219,3 +219,57 @@ func TestEVMAddressDecorator(t *testing.T) { require.True(t, associated) require.Equal(t, evmAddr, associatedEvmAddr) } + +// MockTxNotSigVerifiable is a simple mock transaction type that implements sdk.Tx but not SigVerifiableTx +type MockTxIncompatible struct { + msgs []sdk.Msg +} + +func (m MockTxIncompatible) GetMsgs() []sdk.Msg { + return m.msgs +} + +func (m MockTxIncompatible) ValidateBasic() error { + return nil +} + +func TestEVMAddressDecoratorContinueDespiteErrors(t *testing.T) { + k, ctx := testkeeper.MockEVMKeeper() + handler := ante.NewEVMAddressDecorator(k, k.AccountKeeper()) + + _, err := handler.AnteHandle(ctx, MockTxIncompatible{}, false, func(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) { + return ctx, nil + }) + require.NotNil(t, err) + require.Contains(t, err.Error(), "invalid tx type") + + // Prepare a SigVerifiableTx with no public key + privKey := testkeeper.MockPrivateKey() + sender, _ := testkeeper.PrivateKeyToAddresses(privKey) + k.AccountKeeper().SetAccount(ctx, authtypes.NewBaseAccount(sender, &secp256k1.PubKey{}, 1, 1)) // deliberately no pubkey set + msg := banktypes.NewMsgSend(sender, sender, sdk.NewCoins(sdk.NewCoin("usei", sdk.OneInt()))) // to self to simplify + ctx, err = handler.AnteHandle(ctx, mockTx{msgs: []sdk.Msg{msg}, signers: []sdk.AccAddress{sender}}, false, func(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) { + return ctx, nil + }) + // Since the handler logs the error but does not stop processing, we expect no error returned + require.Nil(t, err, "Expected no error from AnteHandle despite missing public key") + + k.AccountKeeper().SetAccount(ctx, authtypes.NewBaseAccount(sender, nil, 1, 1)) // deliberately no pubkey set + msg = banktypes.NewMsgSend(sender, sender, sdk.NewCoins(sdk.NewCoin("usei", sdk.OneInt()))) // to self to simplify + ctx, err = handler.AnteHandle(ctx, mockTx{msgs: []sdk.Msg{msg}, signers: []sdk.AccAddress{sender}}, false, func(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) { + return ctx, nil + }) + + // Since the handler logs the error but does not stop processing, we expect no error returned + require.Nil(t, err, "Expected no error from AnteHandle despite nil public key") + + // Prepare a SigVerifiableTx with a pubkey that fails to parse + brokenPubKey := &secp256k1.PubKey{Key: []byte{1, 2, 3}} // deliberately too short to be valid + k.AccountKeeper().SetAccount(ctx, authtypes.NewBaseAccount(sender, brokenPubKey, 1, 1)) + ctx, err = handler.AnteHandle(ctx, mockTx{msgs: []sdk.Msg{msg}, signers: []sdk.AccAddress{sender}}, false, func(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) { + return ctx, nil + }) + + // Since the handler logs the error but does not stop processing, we expect no error returned + require.Nil(t, err, "Expected no error from AnteHandle despite inability to parse public key") +} diff --git a/x/evm/ante/router_test.go b/x/evm/ante/router_test.go index 86e16cca9..433d99675 100644 --- a/x/evm/ante/router_test.go +++ b/x/evm/ante/router_test.go @@ -63,3 +63,28 @@ func TestRouter(t *testing.T) { _, err = router.AnteHandle(sdk.Context{}, mockTx{msgs: []sdk.Msg{evmMsg, bankMsg}}, false) require.NotNil(t, err) } + +func TestEVMRouterDecorator_AnteDeps(t *testing.T) { + bankMsg := &banktypes.MsgSend{} + evmMsg, _ := types.NewMsgEVMTransaction(ðtx.LegacyTx{}) + + // non-EVM message + mockAnte := mockAnteState{} + router := ante.NewEVMRouterDecorator(mockAnte.regularAnteHandler, mockAnte.evmAnteHandler, mockAnte.regularAnteDepGenerator, mockAnte.evmAnteDepGenerator) + txDeps := []sdkacltypes.AccessOperation{{}} + _, err := router.AnteDeps(txDeps, mockTx{msgs: []sdk.Msg{bankMsg}}, 0) + require.Nil(t, err) + require.Equal(t, "regulardep", mockAnte.call) + + // EVM message + mockAnte = mockAnteState{} + _, err = router.AnteDeps(txDeps, mockTx{msgs: []sdk.Msg{evmMsg}}, 0) + require.Nil(t, err) + require.Equal(t, "evmdep", mockAnte.call) + + // mixed messages + mockAnte = mockAnteState{} + _, err = router.AnteDeps(txDeps, mockTx{msgs: []sdk.Msg{evmMsg, bankMsg}}, 0) + require.NotNil(t, err) + require.Equal(t, "", mockAnte.call) +}