diff --git a/CHANGELOG.md b/CHANGELOG.md index 4121f12428..c0be85c7c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (rpc) [#558](https://github.com/crypto-org-chain/ethermint/pull/558) New tracer in predecessors to trace balance correctly when `debug_traceTransaction`. * (rpc) [#559](https://github.com/crypto-org-chain/ethermint/pull/559) Use basefee of transaction height instead of minus one height when `debug_traceTransaction`. * (rpc) [#562](https://github.com/crypto-org-chain/ethermint/pull/562) Fix nil pointer panic with legacy transaction format. +* (ante) [#566](https://github.com/crypto-org-chain/ethermint/pull/566) Disallow same sender tx appear after contract creation in the same batch tx. ### Improvements diff --git a/app/ante/eth.go b/app/ante/eth.go index ca608777a7..7269a30ef1 100644 --- a/app/ante/eth.go +++ b/app/ante/eth.go @@ -311,3 +311,38 @@ func CheckAndSetEthSenderNonce( return nil } + +// DetectContractCreationBatchTx returns error if same sender transaction appear after contract creation tx in the same +// batch +func DetectContractCreationBatchTx(ctx sdk.Context, tx sdk.Tx) error { + if !ctx.IsCheckTx() { + // only check in mempool logic + return nil + } + + if len(tx.GetMsgs()) <= 1 { + return nil + } + + senders := make(map[string]struct{}) + for _, msg := range tx.GetMsgs() { + msgEthTx, ok := msg.(*evmtypes.MsgEthereumTx) + if !ok { + return errorsmod.Wrapf(errortypes.ErrUnknownRequest, "invalid message type %T, expected %T", msg, (*evmtypes.MsgEthereumTx)(nil)) + } + + sender := string(msgEthTx.GetSender().Bytes()) + if _, ok := senders[sender]; ok { + return errorsmod.Wrapf( + errortypes.ErrInvalidRequest, + "same sender included after contract creation in the batch transaction", + ) + } + + if msgEthTx.AsTransaction().To() == nil { + senders[sender] = struct{}{} + } + } + + return nil +} diff --git a/app/ante/handler_options.go b/app/ante/handler_options.go index ccf446a88a..4849de991b 100644 --- a/app/ante/handler_options.go +++ b/app/ante/handler_options.go @@ -149,6 +149,11 @@ func newEthAnteHandler(options HandlerOptions) sdk.AnteHandler { return ctx, err } + // reject contract creation tx in batch tx + if err := DetectContractCreationBatchTx(ctx, tx); err != nil { + return ctx, err + } + extraDecorators := options.ExtraDecorators if options.PendingTxListener != nil { extraDecorators = append(extraDecorators, newTxListenerDecorator(options.PendingTxListener)) diff --git a/tests/integration_tests/hardhat/contracts/TestERC20Owner.sol b/tests/integration_tests/hardhat/contracts/TestERC20Owner.sol new file mode 100644 index 0000000000..1efe311ec2 --- /dev/null +++ b/tests/integration_tests/hardhat/contracts/TestERC20Owner.sol @@ -0,0 +1,10 @@ +pragma solidity 0.8.21; +import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; + +// An utility erc20 contract that has a fancy method +contract TestERC20Owner is ERC20 { + constructor(address owner) public ERC20("Fancy", "FNY") { + _mint(owner, 100000000000000000000000000); + } +} + diff --git a/tests/integration_tests/test_batch.py b/tests/integration_tests/test_batch.py index b43934a5b4..7f545b03c6 100644 --- a/tests/integration_tests/test_batch.py +++ b/tests/integration_tests/test_batch.py @@ -1,30 +1,46 @@ import json -from .utils import ADDRS, CONTRACTS, build_batch_tx, contract_address +from .utils import ( + ADDRS, + CONTRACTS, + KEYS, + build_batch_tx_signed, + contract_address, + sign_transaction, +) def test_batch_tx(ethermint): "send multiple eth txs in single cosmos tx" w3 = ethermint.w3 cli = ethermint.cosmos_cli() - sender = ADDRS["validator"] + deployer = ADDRS["validator"] + sender = ADDRS["signer1"] recipient = ADDRS["community"] + deploy_nonce = w3.eth.get_transaction_count(deployer) nonce = w3.eth.get_transaction_count(sender) - info = json.loads(CONTRACTS["TestERC20A"].read_text()) + info = json.loads(CONTRACTS["TestERC20Owner"].read_text()) contract = w3.eth.contract(abi=info["abi"], bytecode=info["bytecode"]) - deploy_tx = contract.constructor().build_transaction( - {"from": sender, "nonce": nonce} + deploy_tx = contract.constructor(sender).build_transaction( + {"from": deployer, "nonce": deploy_nonce} + ) + contract = w3.eth.contract( + address=contract_address(deployer, deploy_nonce), abi=info["abi"] ) - contract = w3.eth.contract(address=contract_address(sender, nonce), abi=info["abi"]) transfer_tx1 = contract.functions.transfer(recipient, 1000).build_transaction( - {"from": sender, "nonce": nonce + 1, "gas": 200000} + {"from": sender, "nonce": nonce, "gas": 200000} ) transfer_tx2 = contract.functions.transfer(recipient, 1000).build_transaction( - {"from": sender, "nonce": nonce + 2, "gas": 200000} + {"from": sender, "nonce": nonce + 1, "gas": 200000} ) - cosmos_tx, tx_hashes = build_batch_tx( - w3, cli, [deploy_tx, transfer_tx1, transfer_tx2] + cosmos_tx, tx_hashes = build_batch_tx_signed( + cli, + [ + sign_transaction(w3, deploy_tx, KEYS["validator"]), + sign_transaction(w3, transfer_tx1, KEYS["signer1"]), + sign_transaction(w3, transfer_tx2, KEYS["signer1"]), + ], ) rsp = cli.broadcast_tx_json(cosmos_tx) assert rsp["code"] == 0, rsp["raw_log"] @@ -33,6 +49,9 @@ def test_batch_tx(ethermint): assert 2000 == contract.caller.balanceOf(recipient) + assert w3.eth.get_transaction_count(deployer) == deploy_nonce + 1 + assert w3.eth.get_transaction_count(sender) == nonce + 2 + # check logs assert receipts[0].contractAddress == contract.address diff --git a/tests/integration_tests/utils.py b/tests/integration_tests/utils.py index 1893cdc374..e54d0c9b0e 100644 --- a/tests/integration_tests/utils.py +++ b/tests/integration_tests/utils.py @@ -32,6 +32,7 @@ ETHERMINT_ADDRESS_PREFIX = "ethm" TEST_CONTRACTS = { "TestERC20A": "TestERC20A.sol", + "TestERC20Owner": "TestERC20Owner.sol", "Greeter": "Greeter.sol", "BurnGas": "BurnGas.sol", "TestChainID": "ChainID.sol", @@ -291,6 +292,10 @@ def modify_command_in_supervisor_config(ini: Path, fn, **kwargs): def build_batch_tx(w3, cli, txs, key=KEYS["validator"]): "return cosmos batch tx and eth tx hashes" signed_txs = [sign_transaction(w3, tx, key) for tx in txs] + return build_batch_tx_signed(cli, signed_txs) + + +def build_batch_tx_signed(cli, signed_txs): tmp_txs = [cli.build_evm_tx(signed.rawTransaction.hex()) for signed in signed_txs] msgs = [tx["body"]["messages"][0] for tx in tmp_txs]