test: Ensure touched accounts also reverted

Kava-Labs · Mar 1, 2024 · 12c2917 · 12c2917
1 parent 152002b
commit 12c2917
Show file tree

Hide file tree

Showing 5 changed files with 121 additions and 12 deletions.
diff --git a/contract/contracts/EIP161Test.sol b/contract/contracts/EIP161Test.sol
@@ -7,13 +7,25 @@ contract EIP161Test {
         selfdestruct(payable(target));
     }
 
+    function selfDestructToRevert(address target) public payable {
+        // This contract will self-destruct and send its balance to the target address
+        selfdestruct(payable(target));
+
+        revert();
+    }
+
     function callAccount(address target) public payable returns (bytes memory) {
         (bool success, bytes memory data) = target.call{value: msg.value}("");
         require(success, "Failed to call empty account with value");
 
         return data;
     }
 
+    function callAccountRevert(address target) public payable {
+        callAccount(target);
+        revert();
+    }
+
     function createContract() public payable returns (address) {
         address newContract;
         bytes memory bytecode = hex"3859818153F3";
@@ -25,4 +37,13 @@ contract EIP161Test {
         }
         return newContract;
     }
+
+    function transferValue(address target) public payable {
+        payable(target).transfer(msg.value);
+    }
+
+    function transferValueRevert(address target) public payable {
+        transferValue(target);
+        revert();
+    }
 }
diff --git a/x/evm/keeper/integration_test.go b/x/evm/keeper/integration_test.go
@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/ethereum/go-ethereum/core/vm"
 	gethparams "github.com/ethereum/go-ethereum/params"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -330,9 +331,10 @@ func (suite *IntegrationTestSuite) TestEIP161_TouchEmptyDeletes() {
 			},
 		},
 		{
-			"self destruct target - 0 value",
+			"self destruct beneficiary - 0 value",
 			func() {
 				// beneficiary account with 0 value should be touched
+				// (self destruct sends the remaining funds to the beneficiary)
 				_, rsp, err := suite.CallContract(
 					testutil.EIP161TestContract,
 					contractAddr,
@@ -348,6 +350,28 @@ func (suite *IntegrationTestSuite) TestEIP161_TouchEmptyDeletes() {
 				targetDeleted:   true,
 			},
 		},
+		{
+			"REVERTED: self destruct beneficiary - 0 value",
+			func() {
+				// beneficiary account with 0 value should be touched
+				_, rsp, err := suite.CallContractWithGas(
+					testutil.EIP161TestContract,
+					contractAddr,
+					common.Big0,
+					28211, // Enough intrinsic gas, but not enough for entire tx
+					"selfDestructToRevert",
+					targetAddr,
+				)
+				suite.T().Logf("rsp: %+v", rsp.GasUsed)
+				suite.Require().NoError(err, "revert should be in rsp, not err")
+				suite.Require().Equal(vm.ErrOutOfGas.Error(), rsp.VmError)
+			},
+			accountState{
+				// Nothing is deleted on revert
+				contractDeleted: false,
+				targetDeleted:   false,
+			},
+		},
 		{
 			"call target - 0 value",
 			func() {
@@ -367,6 +391,35 @@ func (suite *IntegrationTestSuite) TestEIP161_TouchEmptyDeletes() {
 				targetDeleted:   true,
 			},
 		},
+		{
+			"REVERTED: call target - 0 value",
+			func() {
+				_, err := suite.EstimateCallGas(
+					testutil.EIP161TestContract,
+					contractAddr,
+					common.Big0,
+					"callAccountRevert",
+					targetAddr,
+				)
+				suite.Require().Error(err, "estimate gas should fail since it will revert")
+
+				_, rsp, err := suite.CallContractWithGas(
+					testutil.EIP161TestContract,
+					contractAddr,
+					common.Big0,
+					29277, // Enough gas to cover tx
+					"callAccountRevert",
+					targetAddr,
+				)
+				suite.Require().NoError(err, "revert should be in rsp, not err")
+				suite.Require().Equal(vm.ErrExecutionReverted.Error(), rsp.VmError)
+			},
+			accountState{
+				contractDeleted: false,
+				// also reverts account deletion, so target should not be deleted
+				targetDeleted: false,
+			},
+		},
 		{
 			"create call",
 			func() {
@@ -413,6 +466,29 @@ func (suite *IntegrationTestSuite) TestEIP161_TouchEmptyDeletes() {
 				targetDeleted:   true,
 			},
 		},
+		{
+			"REVERTED: transfer zero amount",
+			func() {
+				// Transfers funds from the account -> contract -> target, then
+				// reverts after transfer.
+				// Use a custom contract method so we can easily revert the
+				// transfer after the transfer.
+				_, rsp, err := suite.CallContractWithGas(
+					testutil.EIP161TestContract,
+					contractAddr,
+					common.Big0,
+					29277, // Enough gas to cover tx - estimategas will fail due to revert
+					"transferValueRevert",
+					targetAddr,
+				)
+				suite.Require().NoError(err, "revert should be in rsp, not err")
+				suite.Require().Equal(vm.ErrExecutionReverted.Error(), rsp.VmError)
+			},
+			accountState{
+				contractDeleted: false,
+				targetDeleted:   false,
+			},
+		},
 	}
 
 	for _, tt := range tests {
@@ -443,7 +519,7 @@ func (suite *IntegrationTestSuite) TestEIP161_TouchEmptyDeletes() {
 			} else {
 				suite.Require().NotNil(
 					targetAcc,
-					"EIP-161: empty account should not be deleted if not touched",
+					"EIP-161: empty account should not be deleted if not touched or reverted",
 				)
 			}
 
@@ -456,7 +532,7 @@ func (suite *IntegrationTestSuite) TestEIP161_TouchEmptyDeletes() {
 			} else {
 				suite.Require().NotNil(
 					contractAcc,
-					"EIP-161: contract should not be deleted if not touching empty account",
+					"EIP-161: contract should not be deleted if not touching empty account or reverted",
 				)
 			}
 		})

diff --git a/x/evm/testutil/EIP161Test.json b/x/evm/testutil/EIP161Test.json
@@ -1,4 +1,4 @@
 {
-  "abi": "[{\"inputs\":[{\"internalType\":\"address\",\"name\":\"target\",\"type\":\"address\"}],\"name\":\"callAccount\",\"outputs\":[{\"internalType\":\"bytes\",\"name\":\"\",\"type\":\"bytes\"}],\"stateMutability\":\"payable\",\"type\":\"function\"},{\"inputs\":[],\"name\":\"createContract\",\"outputs\":[{\"internalType\":\"address\",\"name\":\"\",\"type\":\"address\"}],\"stateMutability\":\"payable\",\"type\":\"function\"},{\"inputs\":[{\"internalType\":\"address\",\"name\":\"target\",\"type\":\"address\"}],\"name\":\"selfDestructTo\",\"outputs\":[],\"stateMutability\":\"payable\",\"type\":\"function\"}]",
-  "bin": "608060405234801561001057600080fd5b5061045c806100206000396000f3fe6080604052600436106100345760003560e01c8063412a5a6d146100395780636f43e2db14610057578063a28bf7cc14610087575b600080fd5b6100416100a3565b60405161004e9190610212565b60405180910390f35b610071600480360381019061006c919061025e565b6100fe565b60405161007e919061031b565b60405180910390f35b6100a1600480360381019061009c919061025e565b6101b8565b005b60008060006040518060400160405280600681526020017f3859818153f3000000000000000000000000000000000000000000000000000081525090508051602082016000f09150813b6100f657600080fd5b819250505090565b60606000808373ffffffffffffffffffffffffffffffffffffffff16346040516101279061036e565b60006040518083038185875af1925050503d8060008114610164576040519150601f19603f3d011682016040523d82523d6000602084013e610169565b606091505b5091509150816101ae576040517f08c379a00000000000000000000000000000000000000000000000000000000081526004016101a590610406565b60405180910390fd5b8092505050919050565b8073ffffffffffffffffffffffffffffffffffffffff16ff5b600073ffffffffffffffffffffffffffffffffffffffff82169050919050565b60006101fc826101d1565b9050919050565b61020c816101f1565b82525050565b60006020820190506102276000830184610203565b92915050565b600080fd5b61023b816101f1565b811461024657600080fd5b50565b60008135905061025881610232565b92915050565b6000602082840312156102745761027361022d565b5b600061028284828501610249565b91505092915050565b600081519050919050565b600082825260208201905092915050565b60005b838110156102c55780820151818401526020810190506102aa565b60008484015250505050565b6000601f19601f8301169050919050565b60006102ed8261028b565b6102f78185610296565b93506103078185602086016102a7565b610310816102d1565b840191505092915050565b6000602082019050818103600083015261033581846102e2565b905092915050565b600081905092915050565b50565b600061035860008361033d565b915061036382610348565b600082019050919050565b60006103798261034b565b9150819050919050565b600082825260208201905092915050565b7f4661696c656420746f2063616c6c20656d707479206163636f756e742077697460008201527f682076616c756500000000000000000000000000000000000000000000000000602082015250565b60006103f0602783610383565b91506103fb82610394565b604082019050919050565b6000602082019050818103600083015261041f816103e3565b905091905056fea2646970667358221220b5a8409b4a47065b523cc2a5359c3a45cbab6afcc549a7c55c883476739ca31764736f6c63430008180033"
+  "abi": "[{\"inputs\":[{\"internalType\":\"address\",\"name\":\"target\",\"type\":\"address\"}],\"name\":\"callAccount\",\"outputs\":[{\"internalType\":\"bytes\",\"name\":\"\",\"type\":\"bytes\"}],\"stateMutability\":\"payable\",\"type\":\"function\"},{\"inputs\":[{\"internalType\":\"address\",\"name\":\"target\",\"type\":\"address\"}],\"name\":\"callAccountRevert\",\"outputs\":[],\"stateMutability\":\"payable\",\"type\":\"function\"},{\"inputs\":[],\"name\":\"createContract\",\"outputs\":[{\"internalType\":\"address\",\"name\":\"\",\"type\":\"address\"}],\"stateMutability\":\"payable\",\"type\":\"function\"},{\"inputs\":[{\"internalType\":\"address\",\"name\":\"target\",\"type\":\"address\"}],\"name\":\"selfDestructTo\",\"outputs\":[],\"stateMutability\":\"payable\",\"type\":\"function\"},{\"inputs\":[{\"internalType\":\"address\",\"name\":\"target\",\"type\":\"address\"}],\"name\":\"selfDestructToRevert\",\"outputs\":[],\"stateMutability\":\"payable\",\"type\":\"function\"},{\"inputs\":[{\"internalType\":\"address\",\"name\":\"target\",\"type\":\"address\"}],\"name\":\"transferValue\",\"outputs\":[],\"stateMutability\":\"payable\",\"type\":\"function\"},{\"inputs\":[{\"internalType\":\"address\",\"name\":\"target\",\"type\":\"address\"}],\"name\":\"transferValueRevert\",\"outputs\":[],\"stateMutability\":\"payable\",\"type\":\"function\"}]",
+  "bin": "608060405234801561001057600080fd5b50610587806100206000396000f3fe6080604052600436106100705760003560e01c80636f43e2db1161004e5780636f43e2db146100cb578063a2242d6d146100fb578063a28bf7cc14610117578063a4ddc8591461013357610070565b80630473c5b6146100755780631e4198e014610091578063412a5a6d146100ad575b600080fd5b61008f600480360381019061008a919061035f565b61014f565b005b6100ab60048036038101906100a6919061035f565b610168565b005b6100b56101b2565b6040516100c2919061039b565b60405180910390f35b6100e560048036038101906100e0919061035f565b61020d565b6040516100f29190610446565b60405180910390f35b6101156004803603810190610110919061035f565b6102c7565b005b610131600480360381019061012c919061035f565b6102d5565b005b61014d6004803603810190610148919061035f565b6102ee565b005b8073ffffffffffffffffffffffffffffffffffffffff16ff5b8073ffffffffffffffffffffffffffffffffffffffff166108fc349081150290604051600060405180830381858888f193505050501580156101ae573d6000803e3d6000fd5b5050565b60008060006040518060400160405280600681526020017f3859818153f3000000000000000000000000000000000000000000000000000081525090508051602082016000f09150813b61020557600080fd5b819250505090565b60606000808373ffffffffffffffffffffffffffffffffffffffff163460405161023690610499565b60006040518083038185875af1925050503d8060008114610273576040519150601f19603f3d011682016040523d82523d6000602084013e610278565b606091505b5091509150816102bd576040517f08c379a00000000000000000000000000000000000000000000000000000000081526004016102b490610531565b60405180910390fd5b8092505050919050565b6102d08161020d565b600080fd5b8073ffffffffffffffffffffffffffffffffffffffff16ff5b6102f781610168565b600080fd5b600080fd5b600073ffffffffffffffffffffffffffffffffffffffff82169050919050565b600061032c82610301565b9050919050565b61033c81610321565b811461034757600080fd5b50565b60008135905061035981610333565b92915050565b600060208284031215610375576103746102fc565b5b60006103838482850161034a565b91505092915050565b61039581610321565b82525050565b60006020820190506103b0600083018461038c565b92915050565b600081519050919050565b600082825260208201905092915050565b60005b838110156103f05780820151818401526020810190506103d5565b60008484015250505050565b6000601f19601f8301169050919050565b6000610418826103b6565b61042281856103c1565b93506104328185602086016103d2565b61043b816103fc565b840191505092915050565b60006020820190508181036000830152610460818461040d565b905092915050565b600081905092915050565b50565b6000610483600083610468565b915061048e82610473565b600082019050919050565b60006104a482610476565b9150819050919050565b600082825260208201905092915050565b7f4661696c656420746f2063616c6c20656d707479206163636f756e742077697460008201527f682076616c756500000000000000000000000000000000000000000000000000602082015250565b600061051b6027836104ae565b9150610526826104bf565b604082019050919050565b6000602082019050818103600083015261054a8161050e565b905091905056fea264697066735822122016612d56d81bd79b764c4430f0888201dde429e264e165bc6316ff9a4c27f72b64736f6c63430008180033"
 }
diff --git a/x/evm/testutil/contract.go b/x/evm/testutil/contract.go
@@ -116,16 +116,18 @@ func (suite *TestSuite) CallContract(
 	method string,
 	params ...interface{},
 ) (*types.MsgEthereumTx, *types.MsgEthereumTxResponse, error) {
-	res, err := suite.EstimateGas(contract, contractAddr, value, method, params...)
+	res, err := suite.EstimateCallGas(contract, contractAddr, value, method, params...)
 	if err != nil {
 		return nil, nil, fmt.Errorf("EstimateGas failed: %w", err)
 	}
 
+	suite.T().Logf("estimated gas: %d", res.Gas)
+
 	return suite.CallContractWithGas(contract, contractAddr, value, res.Gas, method, params...)
 }
 
-// EstimateGas estimates the gas for a contract call
-func (suite *TestSuite) EstimateGas(
+// EstimateCallGas estimates the gas for a contract call
+func (suite *TestSuite) EstimateCallGas(
 	contract types.CompiledContract,
 	contractAddr common.Address,
 	value *big.Int,

diff --git a/x/evm/testutil/suite.go b/x/evm/testutil/suite.go
@@ -391,7 +391,6 @@ func (suite *TestSuite) TransferValue(
 	amount *big.Int,
 ) (*types.MsgEthereumTx, *types.MsgEthereumTxResponse, error) {
 	ctx := sdk.WrapSDKContext(suite.Ctx)
-	chainID := suite.App.EvmKeeper.ChainID()
 
 	args, err := json.Marshal(&types.TransactionArgs{
 		To:    &to,
@@ -410,6 +409,17 @@ func (suite *TestSuite) TransferValue(
 		return nil, nil, err
 	}
 
+	return suite.TransferValueWithGas(from, to, res.Gas, amount)
+}
+
+func (suite *TestSuite) TransferValueWithGas(
+	from, to common.Address,
+	gas uint64,
+	amount *big.Int,
+) (*types.MsgEthereumTx, *types.MsgEthereumTxResponse, error) {
+	ctx := sdk.WrapSDKContext(suite.Ctx)
+	chainID := suite.App.EvmKeeper.ChainID()
+
 	nonce := suite.App.EvmKeeper.GetNonce(suite.Ctx, suite.Address)
 
 	var nativeTransferTx *types.MsgEthereumTx
@@ -419,7 +429,7 @@ func (suite *TestSuite) TransferValue(
 			nonce,
 			&to,
 			amount,
-			res.Gas,
+			gas,
 			nil,
 			suite.App.FeeMarketKeeper.GetBaseFee(suite.Ctx),
 			big.NewInt(1),
@@ -432,7 +442,7 @@ func (suite *TestSuite) TransferValue(
 			nonce,
 			&to,
 			amount,
-			res.Gas,
+			gas,
 			nil,
 			nil, nil,
 			nil,
@@ -441,7 +451,7 @@ func (suite *TestSuite) TransferValue(
 	}
 
 	nativeTransferTx.From = suite.Address.Hex()
-	err = nativeTransferTx.Sign(ethtypes.LatestSignerForChainID(chainID), suite.Signer)
+	err := nativeTransferTx.Sign(ethtypes.LatestSignerForChainID(chainID), suite.Signer)
 	if err != nil {
 		return nil, nil, err
 	}