Skip to content

Commit

Permalink
Problem: sender address verification is not decoupled from execution (#…
Browse files Browse the repository at this point in the history 
…358)

* Problem: sender address verification is not decoupled from execution

WIP: #355

Solution:
- to decouple sender address verification from execution, we must first enforce user setting the From field in the msg

* changelog

* fix GetSenderLegacy

* cleanup

* fix unit tests

* reduce overhead of debug log

* lazy init the btree in cache store

because many stores are never touched

* fix mutation
  • Loading branch information
@yihuang
yihuang authored Sep 22, 2023
1 parent 9c9f9e6 commit 8e9320e
Show file tree
Hide file tree
Showing 43 changed files with 390 additions and 307 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
- (eip712) [#1746](https://github.com/evmos/ethermint/pull/1746) Add EIP712 support for multiple messages and schemas
- (feemarket) [#1790](https://github.com/evmos/ethermint/pull/1790) Raise error when get invalid consensus params
- (deps) [#1782](https://github.com/evmos/ethermint/pull/1782) Bump Cosmos-SDK to v0.47.3 and ibc-go to v7.1.0.
- (ante) [#358](https://github.com/crypto-org-chain/ethermint/pull/358) enforce user setting the From address in MsgEthereumTx

### Bug Fixes

Expand Down
66 changes: 34 additions & 32 deletions app/ante/ante_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
nil,
nil,
)
signedContractTx.From = addr.Hex()
signedContractTx.From = addr.Bytes()

tx := suite.CreateTestTx(signedContractTx, privKey, 1, false)
return tx
Expand All @@ -116,7 +116,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
nil,
nil,
)
signedContractTx.From = addr.Hex()
signedContractTx.From = addr.Bytes()

tx := suite.CreateTestTx(signedContractTx, privKey, 1, false)
return tx
Expand All @@ -137,7 +137,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
nil,
nil,
)
signedContractTx.From = addr.Hex()
signedContractTx.From = addr.Bytes()

tx := suite.CreateTestTx(signedContractTx, privKey, 1, false)
return tx
Expand All @@ -159,7 +159,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
nil,
nil,
)
signedTx.From = addr.Hex()
signedTx.From = addr.Bytes()

tx := suite.CreateTestTx(signedTx, privKey, 1, false)
return tx
Expand All @@ -181,7 +181,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
nil,
nil,
)
signedTx.From = addr.Hex()
signedTx.From = addr.Bytes()

tx := suite.CreateTestTx(signedTx, privKey, 1, false)
return tx
Expand All @@ -203,7 +203,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
nil,
nil,
)
signedTx.From = addr.Hex()
signedTx.From = addr.Bytes()

tx := suite.CreateTestTx(signedTx, privKey, 1, false)
return tx
Expand All @@ -224,7 +224,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
nil,
nil,
)
signedTx.From = addr.Hex()
signedTx.From = addr.Bytes()

tx := suite.CreateTestTx(signedTx, privKey, 1, false)
return tx
Expand All @@ -234,7 +234,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
"fail - CheckTx (cosmos tx is not valid)",
func() sdk.Tx {
signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 1, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
signedTx.From = addr.Hex()
signedTx.From = addr.Bytes()

txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false)
// bigger than MaxGasWanted
Expand All @@ -246,7 +246,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
"fail - CheckTx (memo too long)",
func() sdk.Tx {
signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 1, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
signedTx.From = addr.Hex()
signedTx.From = addr.Bytes()

txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false)
txBuilder.SetMemo(strings.Repeat("*", 257))
Expand All @@ -257,7 +257,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
"fail - CheckTx (ExtensionOptionsEthereumTx not set)",
func() sdk.Tx {
signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), 1, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
signedTx.From = addr.Hex()
signedTx.From = addr.Bytes()

txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false, true)
return txBuilder.GetTx()
Expand All @@ -271,7 +271,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
nonce, err := suite.app.AccountKeeper.GetSequence(suite.ctx, acc.GetAddress())
suite.Require().NoError(err)
signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), nonce, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
signedTx.From = addr.Hex()
signedTx.From = addr.Bytes()

tx := suite.CreateTestTx(signedTx, privKey, 1, true)
return tx
Expand All @@ -283,7 +283,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
nonce, err := suite.app.AccountKeeper.GetSequence(suite.ctx, acc.GetAddress())
suite.Require().NoError(err)
signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), nonce, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
signedTx.From = addr.Hex()
signedTx.From = addr.Bytes()

txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false)
txBuilder.SetMemo("memo for cosmos tx not allowed")
Expand All @@ -296,7 +296,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
nonce, err := suite.app.AccountKeeper.GetSequence(suite.ctx, acc.GetAddress())
suite.Require().NoError(err)
signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), nonce, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
signedTx.From = addr.Hex()
signedTx.From = addr.Bytes()

txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false)
txBuilder.SetTimeoutHeight(10)
Expand All @@ -309,7 +309,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
nonce, err := suite.app.AccountKeeper.GetSequence(suite.ctx, acc.GetAddress())
suite.Require().NoError(err)
signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), nonce, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
signedTx.From = addr.Hex()
signedTx.From = addr.Bytes()

txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false)

Expand All @@ -329,7 +329,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
nonce, err := suite.app.AccountKeeper.GetSequence(suite.ctx, acc.GetAddress())
suite.Require().NoError(err)
signedTx := evmtypes.NewTx(suite.app.EvmKeeper.ChainID(), nonce, &to, big.NewInt(10), 100000, big.NewInt(1), nil, nil, nil, nil)
signedTx.From = addr.Hex()
signedTx.From = addr.Bytes()

txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false)

Expand Down Expand Up @@ -589,10 +589,12 @@ func (suite AnteTestSuite) TestAnteHandler() {
nil,
nil,
)
msg.From = addr.Hex()
msg.From = addr.Bytes()
tx := suite.CreateTestTx(msg, privKey, 1, false)
msg = tx.GetMsgs()[0].(*evmtypes.MsgEthereumTx)
msg.From = addr.Hex()
msg.From = addr.Bytes()
// arbitrary modify
msg.From[0] = msg.From[0] + 1
return tx
}, true, false, false,
},
Expand Down Expand Up @@ -906,7 +908,7 @@ func (suite AnteTestSuite) TestAnteHandler() {
nil,
nil,
)
ethTx.From = addr.Hex()
ethTx.From = addr.Bytes()

msg := authz.NewMsgExec(
sdk.AccAddress(privKey.PubKey().Address()),
Expand Down Expand Up @@ -976,7 +978,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
nil,
&types.AccessList{},
)
signedContractTx.From = addr.Hex()
signedContractTx.From = addr.Bytes()

tx := suite.CreateTestTx(signedContractTx, privKey, 1, false)
return tx
Expand All @@ -998,7 +1000,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
nil,
&types.AccessList{},
)
signedContractTx.From = addr.Hex()
signedContractTx.From = addr.Bytes()

tx := suite.CreateTestTx(signedContractTx, privKey, 1, false)
return tx
Expand All @@ -1020,7 +1022,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
nil,
&types.AccessList{},
)
signedContractTx.From = addr.Hex()
signedContractTx.From = addr.Bytes()

tx := suite.CreateTestTx(signedContractTx, privKey, 1, false)
return tx
Expand All @@ -1043,7 +1045,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
nil,
&types.AccessList{},
)
signedTx.From = addr.Hex()
signedTx.From = addr.Bytes()

tx := suite.CreateTestTx(signedTx, privKey, 1, false)
return tx
Expand All @@ -1066,7 +1068,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
nil,
&types.AccessList{},
)
signedTx.From = addr.Hex()
signedTx.From = addr.Bytes()

tx := suite.CreateTestTx(signedTx, privKey, 1, false)
return tx
Expand All @@ -1089,7 +1091,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
nil,
&types.AccessList{},
)
signedTx.From = addr.Hex()
signedTx.From = addr.Bytes()

tx := suite.CreateTestTx(signedTx, privKey, 1, false)
return tx
Expand All @@ -1112,7 +1114,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
nil,
&types.AccessList{},
)
signedTx.From = addr.Hex()
signedTx.From = addr.Bytes()

tx := suite.CreateTestTx(signedTx, privKey, 1, false)
return tx
Expand All @@ -1135,7 +1137,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
nil,
&types.AccessList{},
)
signedTx.From = addr.Hex()
signedTx.From = addr.Bytes()

txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false)
// bigger than MaxGasWanted
Expand All @@ -1160,7 +1162,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
nil,
&types.AccessList{},
)
signedTx.From = addr.Hex()
signedTx.From = addr.Bytes()

txBuilder := suite.CreateTestTxBuilder(signedTx, privKey, 1, false)
txBuilder.SetMemo(strings.Repeat("*", 257))
Expand All @@ -1183,7 +1185,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithDynamicTxFee() {
nil,
&types.AccessList{},
)
signedContractTx.From = addr.Hex()
signedContractTx.From = addr.Bytes()

tx := suite.CreateTestTx(signedContractTx, privKey, 1, false)
return tx
Expand Down Expand Up @@ -1242,7 +1244,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithParams() {
nil,
&types.AccessList{},
)
signedContractTx.From = addr.Hex()
signedContractTx.From = addr.Bytes()

tx := suite.CreateTestTx(signedContractTx, privKey, 1, false)
return tx
Expand All @@ -1264,7 +1266,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithParams() {
nil,
&types.AccessList{},
)
signedContractTx.From = addr.Hex()
signedContractTx.From = addr.Bytes()

tx := suite.CreateTestTx(signedContractTx, privKey, 1, false)
return tx
Expand All @@ -1287,7 +1289,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithParams() {
nil,
&types.AccessList{},
)
signedTx.From = addr.Hex()
signedTx.From = addr.Bytes()

tx := suite.CreateTestTx(signedTx, privKey, 1, false)
return tx
Expand All @@ -1310,7 +1312,7 @@ func (suite AnteTestSuite) TestAnteHandlerWithParams() {
nil,
&types.AccessList{},
)
signedTx.From = addr.Hex()
signedTx.From = addr.Bytes()

tx := suite.CreateTestTx(signedTx, privKey, 1, false)
return tx
Expand Down
10 changes: 6 additions & 4 deletions app/ante/authz_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,8 @@ func (suite *AnteTestSuite) TestRejectDeliverMsgsInAuthz() {
_, testAddresses, err := generatePrivKeyAddressPairs(10)
suite.Require().NoError(err)

fromAddr := []byte{0, 0}

testcases := []struct {
name string
msgs []sdk.Msg
Expand All @@ -237,7 +239,7 @@ func (suite *AnteTestSuite) TestRejectDeliverMsgsInAuthz() {
msgs: []sdk.Msg{
newGenericMsgGrant(
testAddresses,
sdk.MsgTypeURL(&evmtypes.MsgEthereumTx{}),
sdk.MsgTypeURL(&evmtypes.MsgEthereumTx{From: fromAddr}),
),
},
expectedCode: sdkerrors.ErrUnauthorized.ABCICode(),
Expand All @@ -257,7 +259,7 @@ func (suite *AnteTestSuite) TestRejectDeliverMsgsInAuthz() {
msgs: []sdk.Msg{
newGenericMsgGrant(
testAddresses,
sdk.MsgTypeURL(&evmtypes.MsgEthereumTx{}),
sdk.MsgTypeURL(&evmtypes.MsgEthereumTx{From: fromAddr}),
),
},
expectedCode: sdkerrors.ErrUnauthorized.ABCICode(),
Expand All @@ -270,7 +272,7 @@ func (suite *AnteTestSuite) TestRejectDeliverMsgsInAuthz() {
testAddresses[1],
[]sdk.Msg{
createMsgSend(testAddresses),
&evmtypes.MsgEthereumTx{},
&evmtypes.MsgEthereumTx{From: fromAddr},
},
),
},
Expand All @@ -283,7 +285,7 @@ func (suite *AnteTestSuite) TestRejectDeliverMsgsInAuthz() {
testAddresses[1],
2,
[]sdk.Msg{
&evmtypes.MsgEthereumTx{},
&evmtypes.MsgEthereumTx{From: fromAddr},
},
),
},
Expand Down
6 changes: 2 additions & 4 deletions app/ante/eth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
evmtypes "github.com/evmos/ethermint/x/evm/types"

"github.com/ethereum/go-ethereum/common"
ethtypes "github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/params"
)

Expand Down Expand Up @@ -195,7 +194,7 @@ func (egcd EthGasConsumeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simula
return ctx, errorsmod.Wrapf(err, "failed to verify the fees")
}

err = egcd.evmKeeper.DeductTxCostsFromUserBalance(ctx, fees, common.HexToAddress(msgEthTx.From))
err = egcd.evmKeeper.DeductTxCostsFromUserBalance(ctx, fees, common.BytesToAddress(msgEthTx.From))
if err != nil {
return ctx, errorsmod.Wrapf(err, "failed to deduct transaction costs from user balance")
}
Expand Down Expand Up @@ -263,14 +262,13 @@ func NewCanTransferDecorator(evmKeeper EVMKeeper, baseFee *big.Int, evmParams *e
// AnteHandle creates an EVM from the message and calls the BlockContext CanTransfer function to
// see if the address can execute the transaction.
func (ctd CanTransferDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) {
signer := ethtypes.MakeSigner(ctd.ethCfg, big.NewInt(ctx.BlockHeight()))
for _, msg := range tx.GetMsgs() {
msgEthTx, ok := msg.(*evmtypes.MsgEthereumTx)
if !ok {
return ctx, errorsmod.Wrapf(errortypes.ErrUnknownRequest, "invalid message type %T, expected %T", msg, (*evmtypes.MsgEthereumTx)(nil))
}

coreMsg, err := msgEthTx.AsMessage(signer, ctd.baseFee)
coreMsg, err := msgEthTx.AsMessage(ctd.baseFee)
if err != nil {
return ctx, errorsmod.Wrapf(
err,
Expand Down
Loading

0 comments on commit 8e9320e

Please sign in to comment.