NibiruChain · onikonychev · Nov 5, 2024 · Oct 25, 2024 · Oct 26, 2024 · Oct 28, 2024
@@ -75,6 +75,7 @@ throwing an error and (2) ERC20 transfers with other operations that don't bring
 about the expected resulting balance for the transfer recipient.
 - [#2091](https://github.com/NibiruChain/nibiru/pull/2091) - feat(evm): add fun token creation fee validation
 - [#2093](https://github.com/NibiruChain/nibiru/pull/2093) - feat(evm): gas usage in precompiles: limits, local gas meters
+- [#2092](https://github.com/NibiruChain/nibiru/pull/2092) - feat(evm): add validation for wasm multi message execution
 - [#2094](https://github.com/NibiruChain/nibiru/pull/2094) - fix(evm): Following
 from the changs in #2086, this pull request implements a new `JournalChange`
 struct that saves a deep copy of the state multi store before each
@@ -92,15 +93,14 @@ The `NibiruBankKeeper` holds a reference to the current EVM `StateDB` and record
 balance changes in wei as journal changes automatically. This guarantees that
 commits and reversions of the `StateDB` do not misalign with the state of the
 Bank module. This code change uses the `NibiruBankKeeper` on all modules that
-depend on x/bank, such as the EVM and Wasm modules. 
+depend on x/bank, such as the EVM and Wasm modules.
 - [#2097](https://github.com/NibiruChain/nibiru/pull/2097) - feat(evm): Add new query to get dated price from the oracle precompile
 - [#2098](https://github.com/NibiruChain/nibiru/pull/2098) - test(evm): statedb
 tests for race conditions within funtoken precompile
 - [#2100](https://github.com/NibiruChain/nibiru/pull/2100) - refactor: cleanup statedb and precompile sections
 - [#2101](https://github.com/NibiruChain/nibiru/pull/2101) - fix(evm): tx receipt proper marshalling
 - [#2105](https://github.com/NibiruChain/nibiru/pull/2105) - test(evm): precompile call with revert
 
-
 #### Nibiru EVM | Before Audit 1 - 2024-10-18
 
 - [#1837](https://github.com/NibiruChain/nibiru/pull/1837) - feat(eth): protos, eth types, and evm module types

@@ -0,0 +1,4 @@
+types
+artifacts
+cache
+.env
@@ -25,15 +25,21 @@ contract TestERC20TransferThenPrecompileSend {
             "ERC-20 transfer failed"
         );
 
-        (bool success, ) = FUNTOKEN_PRECOMPILE_ADDRESS.call(
-            abi.encodeWithSignature(
-                "bankSend(address,uint256,string)",
-                erc20,
-                uint256(precompileAmount),
-                    precompileRecipient
-            )
+        uint256 sentAmount = FUNTOKEN_PRECOMPILE.bankSend(
+            erc20,
+            precompileAmount,
+            precompileRecipient
         );
 
-        require(success, string.concat("Failed to call bankSend"));
+        require(
+            sentAmount == precompileAmount,
+            string.concat(
+                "IFunToken.bankSend succeeded but transferred the wrong amount",
+                "sentAmount ",
+                Strings.toString(sentAmount),
+                "expected ",
+                Strings.toString(precompileAmount)
+            )
+        );
     }
 }
@@ -1,7 +1,8 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.0;
 
-import "./FunToken.sol";
+import "./IFunToken.sol";
+import "@openzeppelin/contracts/utils/Strings.sol";
 
 contract TestFunTokenPrecompileLocalGas {
     address erc20;
@@ -16,15 +17,21 @@ contract TestFunTokenPrecompileLocalGas {
         uint256 amount,
         string memory bech32Recipient
     ) public {
-        (bool success,) = FUNTOKEN_PRECOMPILE_ADDRESS.call(
-            abi.encodeWithSignature(
-                "bankSend(address,uint256,string)",
-                erc20,
+        uint256 sentAmount = FUNTOKEN_PRECOMPILE.bankSend(
+            erc20,
                 amount,
-                bech32Recipient
+            bech32Recipient
+        );
+        require(
+            sentAmount == amount,
+            string.concat(
+                "IFunToken.bankSend succeeded but transferred the wrong amount",
+                "sentAmount ",
+                Strings.toString(sentAmount),
+                "expected ",
+                Strings.toString(amount)
             )
         );
-        require(success, "Failed to call bankSend");
     }
 
     // Calls bankSend of the FunToken Precompile with the gas amount set in parameter.
@@ -34,14 +41,20 @@ contract TestFunTokenPrecompileLocalGas {
         string memory bech32Recipient,
         uint256 customGas
     ) public {
-        (bool success,) = FUNTOKEN_PRECOMPILE_ADDRESS.call{gas: customGas}(
-            abi.encodeWithSignature(
-                "bankSend(address,uint256,string)",
-                erc20,
-                amount,
-                bech32Recipient
+        uint256 sentAmount = FUNTOKEN_PRECOMPILE.bankSend{gas: customGas}(
+            erc20,
+            amount,
+            bech32Recipient
+        );
+        require(
+            sentAmount == amount,
+            string.concat(
+                "IFunToken.bankSend succeeded but transferred the wrong amount",
+                "sentAmount ",
+                Strings.toString(sentAmount),
+                "expected ",
+                Strings.toString(amount)
             )
         );
-        require(success, "Failed to call bankSend with custom gas");
     }
-}
+}
@@ -54,11 +54,11 @@ func (s *Suite) TestExportInitGenesis() {
 	s.Require().NoError(err)
 
 	// Transfer ERC-20 tokens to user A
-	_, err = deps.EvmKeeper.ERC20().Transfer(erc20Addr, fromUser, toUserA, amountToSendA, deps.Ctx)
+	_, _, err = deps.EvmKeeper.ERC20().Transfer(erc20Addr, fromUser, toUserA, amountToSendA, deps.Ctx)
 	s.Require().NoError(err)
 
 	// Transfer ERC-20 tokens to user B
-	_, err = deps.EvmKeeper.ERC20().Transfer(erc20Addr, fromUser, toUserB, amountToSendB, deps.Ctx)
+	_, _, err = deps.EvmKeeper.ERC20().Transfer(erc20Addr, fromUser, toUserB, amountToSendB, deps.Ctx)
 	s.Require().NoError(err)
 
 	// Create fungible token from bank coin

@@ -71,11 +71,7 @@ func (k Keeper) CallContractWithInput(
 ) (evmResp *evm.MsgEthereumTxResponse, evmObj *vm.EVM, err error) {
 	// This is a `defer` pattern to add behavior that runs in the case that the
 	// error is non-nil, creating a concise way to add extra information.
-	defer func() {
-		if err != nil {
-			err = fmt.Errorf("CallContractError: %w", err)
-		}
-	}()
+	defer HandleOutOfGasPanic(&err, "CallContractError")
 	nonce := k.GetAccNonce(ctx, fromAcc)
 
 	unusedBigInt := big.NewInt(0)
@@ -108,11 +104,8 @@ func (k Keeper) CallContractWithInput(
 	// sent by a user
 	txConfig := k.TxConfig(ctx, gethcommon.BigToHash(big.NewInt(0)))
 
-	// Using tmp context to not modify the state in case of evm revert
-	tmpCtx, commitCtx := ctx.CacheContext()
-
 	evmResp, evmObj, err = k.ApplyEvmMsg(
-		tmpCtx, evmMsg, evm.NewNoOpTracer(), commit, evmCfg, txConfig, true,
+		ctx, evmMsg, evm.NewNoOpTracer(), commit, evmCfg, txConfig, true,
 	)
 	if err != nil {
 		// We don't know the actual gas used, so consuming the gas limit
@@ -135,20 +128,21 @@ func (k Keeper) CallContractWithInput(
 	} else {
 		// Success, committing the state to ctx
 		if commit {
-			commitCtx()
-			totalGasUsed, err := k.AddToBlockGasUsed(ctx, evmResp.GasUsed)
+			blockGasUsed, err := k.AddToBlockGasUsed(ctx, evmResp.GasUsed)
 			if err != nil {
 				k.ResetGasMeterAndConsumeGas(ctx, ctx.GasMeter().Limit())
 				return nil, nil, errors.Wrap(err, "error adding transient gas used to block")
 			}
-			k.ResetGasMeterAndConsumeGas(ctx, totalGasUsed)
+			k.ResetGasMeterAndConsumeGas(ctx, blockGasUsed)
 			k.updateBlockBloom(ctx, evmResp, uint64(txConfig.LogIndex))
-			err = k.EmitEthereumTxEvents(ctx, contract, gethcore.LegacyTxType, evmMsg, evmResp)
-			if err != nil {
-				return nil, nil, errors.Wrap(err, "error emitting ethereum tx events")
-			}
-			blockTxIdx := uint64(txConfig.TxIndex) + 1
-			k.EvmState.BlockTxIndex.Set(ctx, blockTxIdx)
+			// TODO: remove after migrating logs
+			//err = k.EmitLogEvents(ctx, evmResp)
+			//if err != nil {
+			//	return nil, nil, errors.Wrap(err, "error emitting tx logs")
+			//}
+
+			//blockTxIdx := uint64(txConfig.TxIndex) + 1
+			//k.EvmState.BlockTxIndex.Set(ctx, blockTxIdx)
 		}
 		return evmResp, evmObj, nil
 	}

@@ -81,37 +81,37 @@ Transfer implements "ERC20.transfer"
 func (e erc20Calls) Transfer(
 	contract, from, to gethcommon.Address, amount *big.Int,
 	ctx sdk.Context,
-) (balanceIncrease *big.Int, err error) {
+) (balanceIncrease *big.Int, resp *evm.MsgEthereumTxResponse, err error) {
 	recipientBalanceBefore, err := e.BalanceOf(contract, to, ctx)
 	if err != nil {
-		return balanceIncrease, errors.Wrap(err, "failed to retrieve recipient balance")
+		return balanceIncrease, nil, errors.Wrap(err, "failed to retrieve recipient balance")
 	}
 
 	input, err := e.ABI.Pack("transfer", to, amount)
 	if err != nil {
-		return balanceIncrease, fmt.Errorf("failed to pack ABI args: %w", err)
+		return balanceIncrease, nil, fmt.Errorf("failed to pack ABI args: %w", err)
 	}
 
-	resp, _, err := e.CallContractWithInput(ctx, from, &contract, true, input, Erc20GasLimitExecute)
+	resp, _, err = e.CallContractWithInput(ctx, from, &contract, true, input, Erc20GasLimitExecute)
 	if err != nil {
-		return balanceIncrease, err
+		return balanceIncrease, nil, err
 	}
 
 	var erc20Bool ERC20Bool
 	err = e.ABI.UnpackIntoInterface(&erc20Bool, "transfer", resp.Ret)
 	if err != nil {
-		return balanceIncrease, err
+		return balanceIncrease, nil, err
 	}
 
 	// Handle the case of success=false: https://github.com/NibiruChain/nibiru/issues/2080
 	success := erc20Bool.Value
 	if !success {
-		return balanceIncrease, fmt.Errorf("transfer executed but returned success=false")
+		return balanceIncrease, nil, fmt.Errorf("transfer executed but returned success=false")
 	}
 
 	recipientBalanceAfter, err := e.BalanceOf(contract, to, ctx)
 	if err != nil {
-		return balanceIncrease, errors.Wrap(err, "failed to retrieve recipient balance")
+		return balanceIncrease, nil, errors.Wrap(err, "failed to retrieve recipient balance")
 	}
 
 	balanceIncrease = new(big.Int).Sub(recipientBalanceAfter, recipientBalanceBefore)
@@ -121,13 +121,13 @@ func (e erc20Calls) Transfer(
 	// the call "amount". Instead, verify that the recipient got tokens and
 	// return the amount.
 	if balanceIncrease.Sign() <= 0 {
-		return balanceIncrease, fmt.Errorf(
+		return balanceIncrease, nil, fmt.Errorf(
 			"amount of ERC20 tokens received MUST be positive: the balance of recipient %s would've changed by %v for token %s",
 			to.Hex(), balanceIncrease.String(), contract.Hex(),
 		)
 	}
 
-	return balanceIncrease, err
+	return balanceIncrease, resp, err
 }
 
 // BalanceOf retrieves the balance of an ERC20 token for a specific account.

@@ -35,7 +35,7 @@ func (s *Suite) TestERC20Calls() {
 	s.T().Log("Transfer - Not enough funds")
 	{
 		amt := big.NewInt(9_420)
-		_, err := deps.EvmKeeper.ERC20().Transfer(contract, deps.Sender.EthAddr, evm.EVM_MODULE_ADDRESS, amt, deps.Ctx)
+		_, _, err := deps.EvmKeeper.ERC20().Transfer(contract, deps.Sender.EthAddr, evm.EVM_MODULE_ADDRESS, amt, deps.Ctx)
 		s.ErrorContains(err, "ERC20: transfer amount exceeds balance")
 		// balances unchanged
 		evmtest.AssertERC20BalanceEqual(s.T(), deps, contract, deps.Sender.EthAddr, big.NewInt(0))
@@ -45,7 +45,7 @@ func (s *Suite) TestERC20Calls() {
 	s.T().Log("Transfer - Success (sanity check)")
 	{
 		amt := big.NewInt(9_420)
-		sentAmt, err := deps.EvmKeeper.ERC20().Transfer(
+		sentAmt, _, err := deps.EvmKeeper.ERC20().Transfer(
 			contract, evm.EVM_MODULE_ADDRESS, deps.Sender.EthAddr, amt, deps.Ctx,
 		)
 		s.Require().NoError(err)

@@ -323,17 +323,25 @@ func (k Keeper) EstimateGasForEvmCallType(
 
 	ctx := sdk.UnwrapSDKContext(goCtx)
 	chainID := k.EthChainID(ctx)
+	cfg, err := k.GetEVMConfig(ctx, ParseProposerAddr(ctx, req.ProposerAddress), chainID)
+	if err != nil {
+		return nil, grpcstatus.Error(grpccodes.Internal, "failed to load evm config")
+	}
 
 	if req.GasCap < gethparams.TxGas {
 		return nil, grpcstatus.Errorf(grpccodes.InvalidArgument, "gas cap cannot be lower than %d", gethparams.TxGas)
 	}
 
 	var args evm.JsonTxArgs
-	err := json.Unmarshal(req.Args, &args)
+	err = json.Unmarshal(req.Args, &args)
 	if err != nil {
 		return nil, grpcstatus.Error(grpccodes.InvalidArgument, err.Error())
 	}
 
+	// ApplyMessageWithConfig expect correct nonce set in msg
+	nonce := k.GetAccNonce(ctx, args.GetFrom())
+	args.Nonce = (*hexutil.Uint64)(&nonce)
+
 	// Binary search the gas requirement, as it may be higher than the amount used
 	var (
 		lo     = gethparams.TxGas - 1
@@ -361,16 +369,6 @@ func (k Keeper) EstimateGasForEvmCallType(
 	}
 
 	gasCap = hi
-	cfg, err := k.GetEVMConfig(ctx, ParseProposerAddr(ctx, req.ProposerAddress), chainID)
-	if err != nil {
-		return nil, grpcstatus.Error(grpccodes.Internal, "failed to load evm config")
-	}
-
-	// ApplyMessageWithConfig expect correct nonce set in msg
-	nonce := k.GetAccNonce(ctx, args.GetFrom())
-	args.Nonce = (*hexutil.Uint64)(&nonce)
-
-	txConfig := statedb.NewEmptyTxConfig(gethcommon.BytesToHash(ctx.HeaderHash().Bytes()))
 
 	// convert the tx args to an ethereum message
 	msg, err := args.ToMessage(req.GasCap, cfg.BaseFeeWei)
@@ -422,6 +420,7 @@ func (k Keeper) EstimateGasForEvmCallType(
 				WithTransientKVGasConfig(storetypes.GasConfig{})
 		}
 		// pass false to not commit StateDB
+		txConfig := statedb.NewEmptyTxConfig(gethcommon.BytesToHash(ctx.HeaderHash().Bytes()))
 		rsp, _, err = k.ApplyEvmMsg(tmpCtx, msg, nil, false, cfg, txConfig, false)
 		if err != nil {
 			if errors.Is(err, core.ErrIntrinsicGas) {
@@ -438,24 +437,27 @@ func (k Keeper) EstimateGasForEvmCallType(
 		return nil, err
 	}
 
+	// The gas limit is now the highest gas limit that results in an executable transaction
 	// Reject the transaction as invalid if it still fails at the highest allowance
 	if hi == gasCap {
 		failed, result, err := executable(hi)
 		if err != nil {
 			return nil, fmt.Errorf("eth call exec error: %w", err)
 		}
 
-		if failed {
-			if result != nil && result.VmError != vm.ErrOutOfGas.Error() {
-				if result.VmError == vm.ErrExecutionReverted.Error() {
-					return nil, fmt.Errorf("VMError: %w", evm.NewExecErrorWithReason(result.Ret))
-				}
-				return nil, fmt.Errorf("VMError: %s", result.VmError)
+		if failed && result != nil {
+			if result.VmError == vm.ErrExecutionReverted.Error() {
+				return nil, fmt.Errorf("Estimate gas VMError: %w", evm.NewExecErrorWithReason(result.Ret))
+			}
+
+			if result.VmError == vm.ErrOutOfGas.Error() {
+				return nil, fmt.Errorf("gas required exceeds allowance (%d)", gasCap)
 			}
-			// Otherwise, the specified gas cap is too low
-			return nil, fmt.Errorf("gas required exceeds allowance (%d)", gasCap)
+
+			return nil, fmt.Errorf("Estimgate gas VMError: %s", result.VmError)
 		}
 	}
+
 	return &evm.EstimateGasResponse{Gas: hi}, nil
 }