Skip to content

Commit d356344

Browse files
codchencodchen
authored
disallow pre-155 transactions (#2306)
* disallow pre-155 transactions * fix tests
1 parent 1a20f68 commit d356344

13 files changed

Lines changed: 55 additions & 51 deletions

File tree

‎aclmapping/evm/mappings.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func TransactionDependencyGenerator(_ aclkeeper.Keeper, evmKeeper evmkeeper.Keep
3838
return []sdkacltypes.AccessOperation{*acltypes.CommitAccessOp()}, nil
3939
}
4040

41-
if err := ante.Preprocess(ctx, evmMsg, evmKeeper.ChainID(ctx)); err != nil {
41+
if err := ante.Preprocess(ctx, evmMsg, evmKeeper.ChainID(ctx), evmKeeper.EthBlockTestConfig.Enabled); err != nil {
4242
return []sdkacltypes.AccessOperation{}, err
4343
}
4444
ops := []sdkacltypes.AccessOperation{}

‎app/app.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1709,7 +1709,7 @@ func (app *App) DecodeTransactionsConcurrently(ctx sdk.Context, txs [][]byte) []
17091709
} else {
17101710
if isEVM, _ := evmante.IsEVMMessage(typedTx); isEVM {
17111711
msg := evmtypes.MustGetEVMTransactionMessage(typedTx)
1712-
if err := evmante.Preprocess(ctx, msg, app.EvmKeeper.ChainID(ctx)); err != nil {
1712+
if err := evmante.Preprocess(ctx, msg, app.EvmKeeper.ChainID(ctx), app.EvmKeeper.EthBlockTestConfig.Enabled); err != nil {
17131713
ctx.Logger().Error(fmt.Sprintf("error preprocessing EVM tx due to %s", err))
17141714
typedTxs[idx] = nil
17151715
return

‎contracts/test/EVMCompatabilityTest.js‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1357,7 +1357,7 @@ describe("EVM Validations ", function() {
13571357
expect(response.data.error.message).to.include("invalid chain-id")
13581358
});
13591359

1360-
it("should allow empty chainId for legacy txs", async function() {
1360+
it("should not allow empty chainId for legacy txs", async function() {
13611361
const nonce = await ethers.provider.getTransactionCount(signer, "pending")
13621362

13631363
// const accounts = await setupSigners(await hre.ethers.getSigners())
@@ -1377,11 +1377,8 @@ describe("EVM Validations ", function() {
13771377
jsonrpc: "2.0"
13781378
})
13791379

1380-
expect(response.data.result).to.match(/0x.*/)
1380+
expect(response.data.result).not.to.match(/0x.*/)
13811381
});
1382-
1383-
1384-
13851382
})
13861383

13871384
})

‎precompiles/bank/bank_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ func TestRun(t *testing.T) {
147147

148148
// send the transaction
149149
msgServer := keeper.NewMsgServerImpl(k)
150-
ante.Preprocess(ctx, req, k.ChainID(ctx))
150+
ante.Preprocess(ctx, req, k.ChainID(ctx), false)
151151
ctx = ctx.WithEventManager(sdk.NewEventManager())
152152
ctx, err = ante.NewEVMFeeCheckDecorator(k, upgradeKeeper).AnteHandle(ctx, mockTx{msgs: []sdk.Msg{req}}, false, func(sdk.Context, sdk.Tx, bool) (sdk.Context, error) {
153153
return ctx, nil

‎precompiles/distribution/distribution_test.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ func TestWithdraw(t *testing.T) {
9090

9191
msgServer := keeper.NewMsgServerImpl(k)
9292

93-
ante.Preprocess(ctx, req, k.ChainID(ctx))
93+
ante.Preprocess(ctx, req, k.ChainID(ctx), false)
9494
res, err := msgServer.EVMTransaction(sdk.WrapSDKContext(ctx), req)
9595
require.Nil(t, err)
9696
require.Empty(t, res.VmError)
@@ -121,7 +121,7 @@ func TestWithdraw(t *testing.T) {
121121
req, err = evmtypes.NewMsgEVMTransaction(txwrapper)
122122
require.Nil(t, err)
123123

124-
ante.Preprocess(ctx, req, k.ChainID(ctx))
124+
ante.Preprocess(ctx, req, k.ChainID(ctx), false)
125125
res, err = msgServer.EVMTransaction(sdk.WrapSDKContext(ctx), req)
126126
require.Nil(t, err)
127127
require.Empty(t, res.VmError)
@@ -145,7 +145,7 @@ func TestWithdraw(t *testing.T) {
145145
req, err = evmtypes.NewMsgEVMTransaction(txwrapper)
146146
require.Nil(t, err)
147147

148-
ante.Preprocess(ctx, req, k.ChainID(ctx))
148+
ante.Preprocess(ctx, req, k.ChainID(ctx), false)
149149
res, err = msgServer.EVMTransaction(sdk.WrapSDKContext(ctx), req)
150150
require.Nil(t, err)
151151
require.Empty(t, res.VmError)
@@ -225,7 +225,7 @@ func delegate(ctx sdk.Context,
225225
require.Nil(t, k.BankKeeper().MintCoins(ctx, evmtypes.ModuleName, sdk.NewCoins(sdk.NewCoin(k.GetBaseDenom(ctx), sdk.NewInt(200000000)))))
226226
require.Nil(t, k.BankKeeper().SendCoinsFromModuleToAccount(ctx, evmtypes.ModuleName, seiAddr, amt))
227227

228-
ante.Preprocess(ctx, req, k.ChainID(ctx))
228+
ante.Preprocess(ctx, req, k.ChainID(ctx), false)
229229
res, err := msgServer.EVMTransaction(sdk.WrapSDKContext(ctx), req)
230230
require.Nil(t, err)
231231
require.Empty(t, res.VmError)
@@ -269,7 +269,7 @@ func setWithdrawAddressAndWithdraw(
269269
req, err := evmtypes.NewMsgEVMTransaction(txwrapper)
270270
require.Nil(t, err)
271271

272-
ante.Preprocess(ctx, req, k.ChainID(ctx))
272+
ante.Preprocess(ctx, req, k.ChainID(ctx), false)
273273
res, err := msgServer.EVMTransaction(sdk.WrapSDKContext(ctx), req)
274274
require.Nil(t, err)
275275
require.Empty(t, res.VmError)
@@ -298,7 +298,7 @@ func setWithdrawAddressAndWithdraw(
298298
r, err := evmtypes.NewMsgEVMTransaction(txwrapper)
299299
require.Nil(t, err)
300300

301-
ante.Preprocess(ctx, r, k.ChainID(ctx))
301+
ante.Preprocess(ctx, r, k.ChainID(ctx), false)
302302
res, err = msgServer.EVMTransaction(sdk.WrapSDKContext(ctx), r)
303303
require.Nil(t, err)
304304
require.Empty(t, res.VmError)
@@ -1200,7 +1200,7 @@ func TestWithdrawValidatorCommission_noCommissionToWithdrawRightAfterDelegation(
12001200
require.Nil(t, err)
12011201

12021202
msgServer := keeper.NewMsgServerImpl(k)
1203-
ante.Preprocess(ctx, req, k.ChainID(ctx))
1203+
ante.Preprocess(ctx, req, k.ChainID(ctx), false)
12041204
res, err := msgServer.EVMTransaction(sdk.WrapSDKContext(ctx), req)
12051205
require.Nil(t, err)
12061206
require.Empty(t, res.VmError)
@@ -1232,7 +1232,7 @@ func TestWithdrawValidatorCommission_noCommissionToWithdrawRightAfterDelegation(
12321232
req, err = evmtypes.NewMsgEVMTransaction(txwrapper)
12331233
require.Nil(t, err)
12341234

1235-
ante.Preprocess(ctx, req, k.ChainID(ctx))
1235+
ante.Preprocess(ctx, req, k.ChainID(ctx), false)
12361236
res, err = msgServer.EVMTransaction(sdk.WrapSDKContext(ctx), req)
12371237
require.Nil(t, err)
12381238
require.Equal(t, "no validator commission to withdraw", string(res.ReturnData))

‎precompiles/gov/gov_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ func TestGovPrecompile(t *testing.T) {
308308
tt.setup(ctx, k, evmAddr, seiAddr)
309309

310310
msgServer := keeper.NewMsgServerImpl(k)
311-
ante.Preprocess(ctx, req, k.ChainID(ctx))
311+
ante.Preprocess(ctx, req, k.ChainID(ctx), false)
312312
res, err := msgServer.EVMTransaction(sdk.WrapSDKContext(ctx), req)
313313
if tt.wantErr {
314314
require.NotEmpty(t, res.VmError)
@@ -996,7 +996,7 @@ func TestPrecompileExecutor_submitProposal(t *testing.T) {
996996
require.Nil(t, err)
997997

998998
msgServer := keeper.NewMsgServerImpl(k)
999-
ante.Preprocess(ctx, req, k.ChainID(ctx))
999+
ante.Preprocess(ctx, req, k.ChainID(ctx), false)
10001000
gotRet, err := msgServer.EVMTransaction(sdk.WrapSDKContext(ctx), req)
10011001

10021002
if tt.wantErr {

‎precompiles/staking/staking_events_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -259,8 +259,8 @@ func TestStakingPrecompileEventsEmission(t *testing.T) {
259259

260260
// Now edit the validator
261261
newMoniker := "Edited Validator"
262-
newCommissionRate := "" // Empty string to not change commission rate
263-
newMinSelfDelegation := big.NewInt(0) // 0 to not change minimum self-delegation
262+
newCommissionRate := "" // Empty string to not change commission rate
263+
newMinSelfDelegation := big.NewInt(0) // 0 to not change minimum self-delegation
264264

265265
editArgs, err := abi.Pack("editValidator", newMoniker, newCommissionRate, newMinSelfDelegation)
266266
require.NoError(t, err)
@@ -323,7 +323,7 @@ func executeEVMTx(t *testing.T, testApp *app.App, ctx sdk.Context, tx *ethtypes.
323323
require.NoError(t, err)
324324

325325
msgServer := evmkeeper.NewMsgServerImpl(&testApp.EvmKeeper)
326-
ante.Preprocess(ctx, req, testApp.EvmKeeper.ChainID(ctx))
326+
ante.Preprocess(ctx, req, testApp.EvmKeeper.ChainID(ctx), false)
327327

328328
res, err := msgServer.EVMTransaction(sdk.WrapSDKContext(ctx), req)
329329
require.NoError(t, err)

‎precompiles/staking/staking_test.go‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ func TestStaking(t *testing.T) {
8787

8888
msgServer := keeper.NewMsgServerImpl(k)
8989

90-
ante.Preprocess(ctx, req, k.ChainID(ctx))
90+
ante.Preprocess(ctx, req, k.ChainID(ctx), false)
9191
res, err := msgServer.EVMTransaction(sdk.WrapSDKContext(ctx), req)
9292
require.Nil(t, err)
9393
require.Empty(t, res.VmError)
@@ -114,7 +114,7 @@ func TestStaking(t *testing.T) {
114114
req, err = evmtypes.NewMsgEVMTransaction(txwrapper)
115115
require.Nil(t, err)
116116

117-
ante.Preprocess(ctx, req, k.ChainID(ctx))
117+
ante.Preprocess(ctx, req, k.ChainID(ctx), false)
118118
res, err = msgServer.EVMTransaction(sdk.WrapSDKContext(ctx), req)
119119
require.Nil(t, err)
120120
require.Empty(t, res.VmError)
@@ -141,7 +141,7 @@ func TestStaking(t *testing.T) {
141141
req, err = evmtypes.NewMsgEVMTransaction(txwrapper)
142142
require.Nil(t, err)
143143

144-
ante.Preprocess(ctx, req, k.ChainID(ctx))
144+
ante.Preprocess(ctx, req, k.ChainID(ctx), false)
145145
res, err = msgServer.EVMTransaction(sdk.WrapSDKContext(ctx), req)
146146
require.Nil(t, err)
147147
require.Empty(t, res.VmError)
@@ -195,7 +195,7 @@ func TestStakingError(t *testing.T) {
195195

196196
msgServer := keeper.NewMsgServerImpl(k)
197197

198-
ante.Preprocess(ctx, req, k.ChainID(ctx))
198+
ante.Preprocess(ctx, req, k.ChainID(ctx), false)
199199
res, err := msgServer.EVMTransaction(sdk.WrapSDKContext(ctx), req)
200200
require.Nil(t, err)
201201
require.NotEmpty(t, res.VmError)
@@ -218,7 +218,7 @@ func TestStakingError(t *testing.T) {
218218
req, err = evmtypes.NewMsgEVMTransaction(txwrapper)
219219
require.Nil(t, err)
220220

221-
ante.Preprocess(ctx, req, k.ChainID(ctx))
221+
ante.Preprocess(ctx, req, k.ChainID(ctx), false)
222222
res, err = msgServer.EVMTransaction(sdk.WrapSDKContext(ctx), req)
223223
require.Nil(t, err)
224224
require.NotEmpty(t, res.VmError)
@@ -685,7 +685,7 @@ func TestCreateValidator(t *testing.T) {
685685
req, err := evmtypes.NewMsgEVMTransaction(txwrapper)
686686
require.NoError(t, err)
687687

688-
ante.Preprocess(setup.ctx, req, setup.k.ChainID(setup.ctx))
688+
ante.Preprocess(setup.ctx, req, setup.k.ChainID(setup.ctx), false)
689689
res, err := setup.msgServer.EVMTransaction(sdk.WrapSDKContext(setup.ctx), req)
690690
require.NoError(t, err)
691691

@@ -748,7 +748,7 @@ func TestCreateValidator_UnassociatedAddress(t *testing.T) {
748748
req, err := evmtypes.NewMsgEVMTransaction(txwrapper)
749749
require.NoError(t, err)
750750

751-
ante.Preprocess(setup.ctx, req, setup.k.ChainID(setup.ctx))
751+
ante.Preprocess(setup.ctx, req, setup.k.ChainID(setup.ctx), false)
752752
res, err := setup.msgServer.EVMTransaction(sdk.WrapSDKContext(setup.ctx), req)
753753
require.NoError(t, err)
754754
require.NotEmpty(t, res.VmError, "Should fail with unassociated address")
@@ -804,7 +804,7 @@ func TestEditValidator_ErorrIfDoesNotExist(t *testing.T) {
804804
require.NoError(t, err)
805805

806806
msgServer := keeper.NewMsgServerImpl(k)
807-
ante.Preprocess(ctx, req, k.ChainID(ctx))
807+
ante.Preprocess(ctx, req, k.ChainID(ctx), false)
808808
res, err := msgServer.EVMTransaction(sdk.WrapSDKContext(ctx), req)
809809
require.NoError(t, err)
810810
// Should fail because validator doesn't exist
@@ -870,7 +870,7 @@ func TestEditValidator(t *testing.T) {
870870
createReq, err := evmtypes.NewMsgEVMTransaction(createTxWrapper)
871871
require.NoError(t, err)
872872

873-
ante.Preprocess(ctx, createReq, k.ChainID(ctx))
873+
ante.Preprocess(ctx, createReq, k.ChainID(ctx), false)
874874
createRes, err := msgServer.EVMTransaction(sdk.WrapSDKContext(ctx), createReq)
875875
require.NoError(t, err)
876876
require.Empty(t, createRes.VmError, "Validator creation should succeed: %s", createRes.VmError)
@@ -909,7 +909,7 @@ func TestEditValidator(t *testing.T) {
909909
editReq, err := evmtypes.NewMsgEVMTransaction(editTxWrapper)
910910
require.NoError(t, err)
911911

912-
ante.Preprocess(ctx, editReq, k.ChainID(ctx))
912+
ante.Preprocess(ctx, editReq, k.ChainID(ctx), false)
913913
editRes, err := msgServer.EVMTransaction(sdk.WrapSDKContext(ctx), editReq)
914914
require.NoError(t, err)
915915
require.Empty(t, editRes.VmError, "Edit validator should succeed: %s", editRes.VmError)

‎x/evm/ante/preprocess.go‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package ante
22

33
import (
44
"encoding/hex"
5+
"errors"
56
"fmt"
67
"math/big"
78

@@ -58,7 +59,7 @@ func NewEVMPreprocessDecorator(evmKeeper *evmkeeper.Keeper, accountKeeper *accou
5859
//nolint:revive
5960
func (p *EVMPreprocessDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) {
6061
msg := evmtypes.MustGetEVMTransactionMessage(tx)
61-
if err := Preprocess(ctx, msg, p.evmKeeper.ChainID(ctx)); err != nil {
62+
if err := Preprocess(ctx, msg, p.evmKeeper.ChainID(ctx), p.evmKeeper.EthBlockTestConfig.Enabled); err != nil {
6263
return ctx, err
6364
}
6465

@@ -118,7 +119,7 @@ func (p *EVMPreprocessDecorator) IsAccountBalancePositive(ctx sdk.Context, seiAd
118119
}
119120

120121
// stateless
121-
func Preprocess(ctx sdk.Context, msgEVMTransaction *evmtypes.MsgEVMTransaction, chainID *big.Int) error {
122+
func Preprocess(ctx sdk.Context, msgEVMTransaction *evmtypes.MsgEVMTransaction, chainID *big.Int, isBlockTest bool) error {
122123
if msgEVMTransaction.Derived != nil {
123124
if msgEVMTransaction.Derived.PubKey == nil {
124125
// this means the message has `Derived` set from the outside, in which case we should reject
@@ -169,7 +170,13 @@ func Preprocess(ctx sdk.Context, msgEVMTransaction *evmtypes.MsgEVMTransaction,
169170
V = AdjustV(V, ethTx.Type(), ethCfg.ChainID)
170171
txHash = signer.Hash(ethTx)
171172
} else {
172-
txHash = ethtypes.FrontierSigner{}.Hash(ethTx)
173+
if isBlockTest {
174+
// need to allow unprotected legacy txs in blocktest
175+
// to not lose coverage for other parts of the code
176+
txHash = ethtypes.FrontierSigner{}.Hash(ethTx)
177+
} else {
178+
return errors.New("unsupported tx type: unsafe legacy tx")
179+
}
173180
}
174181
evmAddr, seiAddr, seiPubkey, err := helpers.GetAddresses(V, R, S, txHash)
175182
if err != nil {

‎x/evm/ante/preprocess_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ func TestPreprocessAnteHandler(t *testing.T) {
7070
require.Equal(t, sdk.ZeroInt(), k.BankKeeper().GetWeiBalance(ctx, sdk.AccAddress(evmAddr[:])))
7171
}
7272

73-
func TestPreprocessAnteHandlerUnprotected(t *testing.T) {
73+
func TestPreprocessAnteHandlerUnprotectedShouldFail(t *testing.T) {
7474
k := &testkeeper.EVMTestApp.EvmKeeper
7575
ctx := testkeeper.EVMTestApp.GetContextForDeliverTx(nil)
7676
handler := ante.NewEVMPreprocessDecorator(k, k.AccountKeeper())
@@ -95,8 +95,8 @@ func TestPreprocessAnteHandlerUnprotected(t *testing.T) {
9595
_, err = handler.AnteHandle(ctx, mockTx{msgs: []sdk.Msg{msg}}, false, func(ctx sdk.Context, _ sdk.Tx, _ bool) (sdk.Context, error) {
9696
return ctx, nil
9797
})
98-
require.Nil(t, err)
99-
require.Equal(t, "0xc39BDF685F289B1F261EE9b0b1B2Bf9eae4C1980", msg.Derived.SenderEVMAddr.Hex())
98+
require.NotNil(t, err)
99+
require.Contains(t, err.Error(), "unsafe legacy tx")
100100
}
101101

102102
func TestPreprocessAssociateTx(t *testing.T) {

0 commit comments

Comments
 (0)