From efcfde1e7159ca3147bc863196891668608198a7 Mon Sep 17 00:00:00 2001 From: Kevin Yang <5478483+k-yang@users.noreply.github.com> Date: Mon, 27 Jan 2025 11:50:04 -0500 Subject: [PATCH 1/9] fix(evm): consume gas in CallContractWithInput --- x/evm/keeper/call_contract.go | 4 ++-- x/evm/keeper/erc20.go | 12 ++++++------ x/evm/keeper/funtoken_from_coin.go | 4 +--- x/evm/keeper/msg_server.go | 3 +-- x/evm/precompile/funtoken.go | 7 +++---- 5 files changed, 13 insertions(+), 17 deletions(-) diff --git a/x/evm/keeper/call_contract.go b/x/evm/keeper/call_contract.go index f9fdc2c8b..71a5391cf 100644 --- a/x/evm/keeper/call_contract.go +++ b/x/evm/keeper/call_contract.go @@ -63,9 +63,9 @@ func (k Keeper) CallContractWithInput( evmResp, err = k.ApplyEvmMsg( ctx, evmMsg, evmObj, evm.NewNoOpTracer(), commit, txConfig.TxHash, true, ) + ctx.GasMeter().ConsumeGas(evmResp.GasUsed, "CallContractWithInput") if err != nil { - err = errors.Wrap(err, "failed to apply ethereum core message") - return + return nil, errors.Wrap(err, "failed to apply ethereum core message") } if evmResp.Failed() { diff --git a/x/evm/keeper/erc20.go b/x/evm/keeper/erc20.go index f26bee952..3851e06dd 100644 --- a/x/evm/keeper/erc20.go +++ b/x/evm/keeper/erc20.go @@ -196,7 +196,7 @@ func (e erc20Calls) loadERC20String( if err != nil { return out, err } - res, err := e.Keeper.CallContractWithInput( + evmResp, err := e.Keeper.CallContractWithInput( ctx, evmObj, evm.EVM_MODULE_ADDRESS, @@ -211,13 +211,13 @@ func (e erc20Calls) loadERC20String( erc20Val := new(ERC20String) if err := erc20Abi.UnpackIntoInterface( - erc20Val, methodName, res.Ret, + erc20Val, methodName, evmResp.Ret, ); err == nil { return erc20Val.Value, err } erc20Bytes32Val := new(ERC20Bytes32) - if err := erc20Abi.UnpackIntoInterface(erc20Bytes32Val, methodName, res.Ret); err == nil { + if err := erc20Abi.UnpackIntoInterface(erc20Bytes32Val, methodName, evmResp.Ret); err == nil { return bytes32ToString(erc20Bytes32Val.Value), nil } @@ -239,7 +239,7 @@ func (e erc20Calls) loadERC20Uint8( if err != nil { return out, err } - res, err := e.Keeper.CallContractWithInput( + evmResp, err := e.Keeper.CallContractWithInput( ctx, evmObj, evm.EVM_MODULE_ADDRESS, @@ -254,14 +254,14 @@ func (e erc20Calls) loadERC20Uint8( erc20Val := new(ERC20Uint8) if err := erc20Abi.UnpackIntoInterface( - erc20Val, methodName, res.Ret, + erc20Val, methodName, evmResp.Ret, ); err == nil { return erc20Val.Value, err } erc20Uint256Val := new(ERC20BigInt) if err := erc20Abi.UnpackIntoInterface( - erc20Uint256Val, methodName, res.Ret, + erc20Uint256Val, methodName, evmResp.Ret, ); err == nil { // We can safely cast to uint8 because it's nonsense for decimals to be larger than 255 return uint8(erc20Uint256Val.Value.Uint64()), err diff --git a/x/evm/keeper/funtoken_from_coin.go b/x/evm/keeper/funtoken_from_coin.go index 294d72964..6cb4977d5 100644 --- a/x/evm/keeper/funtoken_from_coin.go +++ b/x/evm/keeper/funtoken_from_coin.go @@ -102,7 +102,7 @@ func (k *Keeper) deployERC20ForBankCoin( k.Bank.StateDB = nil }() evmObj := k.NewEVM(ctx, evmMsg, evmCfg, nil /*tracer*/, stateDB) - evmResp, err := k.CallContractWithInput( + _, err = k.CallContractWithInput( ctx, evmObj, evm.EVM_MODULE_ADDRESS, nil, true /*commit*/, input, Erc20GasLimitDeploy, ) if err != nil { @@ -114,7 +114,5 @@ func (k *Keeper) deployERC20ForBankCoin( return gethcommon.Address{}, errors.Wrap(err, "failed to commit stateDB") } - ctx.GasMeter().ConsumeGas(evmResp.GasUsed, "deploy erc20 funtoken contract") - return erc20Addr, nil } diff --git a/x/evm/keeper/msg_server.go b/x/evm/keeper/msg_server.go index 7c6e98021..42742099b 100644 --- a/x/evm/keeper/msg_server.go +++ b/x/evm/keeper/msg_server.go @@ -73,6 +73,7 @@ func (k *Keeper) EthereumTx( txConfig.TxHash, false, /*fullRefundLeftoverGas*/ ) + ctx.GasMeter().ConsumeGas(evmResp.GasUsed, "execute ethereum tx") if err != nil { return nil, errors.Wrap(err, "error applying ethereum core message") } @@ -89,7 +90,6 @@ func (k *Keeper) EthereumTx( if err = k.RefundGas(ctx, evmMsg.From(), refundGas, weiPerGas); err != nil { return nil, errors.Wrapf(err, "error refunding leftover gas to sender %s", evmMsg.From()) } - ctx.GasMeter().ConsumeGas(evmResp.GasUsed, "execute ethereum tx") err = k.EmitEthereumTxEvents(ctx, tx.To(), tx.Type(), evmMsg, evmResp) if err != nil { @@ -573,7 +573,6 @@ func (k Keeper) convertCoinToEvmBornCoin( if err != nil { return nil, err } - ctx.GasMeter().ConsumeGas(evmResp.GasUsed, "mint erc20 tokens") if evmResp.Failed() { return nil, diff --git a/x/evm/precompile/funtoken.go b/x/evm/precompile/funtoken.go index 6a1aa210b..8ea61f3fb 100644 --- a/x/evm/precompile/funtoken.go +++ b/x/evm/precompile/funtoken.go @@ -191,10 +191,9 @@ func (p precompileFunToken) sendToBank( // owns the ERC20 contract and was the original minter of the ERC20 tokens. // Since we're sending them away and want accurate total supply tracking, the // tokens need to be burned. - burnResp, e := p.evmKeeper.ERC20().Burn(erc20, evm.EVM_MODULE_ADDRESS, gotAmount, ctx, evmObj) - if e != nil { - err = fmt.Errorf("ERC20.Burn: %w", e) - return + burnResp, err := p.evmKeeper.ERC20().Burn(erc20, evm.EVM_MODULE_ADDRESS, gotAmount, ctx, evmObj) + if err != nil { + return nil, fmt.Errorf("ERC20.Burn: %w", err) } evmResponses = append(evmResponses, burnResp) } else { From 1cb20685ec2b3a85a08c12fd0ed2c61be2f7e9d3 Mon Sep 17 00:00:00 2001 From: Kevin Yang <5478483+k-yang@users.noreply.github.com> Date: Mon, 27 Jan 2025 11:51:48 -0500 Subject: [PATCH 2/9] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aec8bba4e..3c80971b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -87,6 +87,7 @@ needed to include double quotes around the hexadecimal string. - [#2172](https://github.com/NibiruChain/nibiru/pull/2172) - chore: close iterator in IterateEpochInfo - [#2173](https://github.com/NibiruChain/nibiru/pull/2173) - fix(evm): clear `StateDB` between calls - [#2177](https://github.com/NibiruChain/nibiru/pull/2177) - fix(cmd): Continue from #2127 and unwire vesting flags and logic from genaccounts.go +- [#2180](https://github.com/NibiruChain/nibiru/pull/2180) - fix(evm): apply gas consumption across the entire EVM codebase at `CallContractWithInput` #### Nibiru EVM | Before Audit 2 - 2024-12-06 From 99473307d10a63507ad864a449d7d0bce392c4e7 Mon Sep 17 00:00:00 2001 From: Kevin Yang <5478483+k-yang@users.noreply.github.com> Date: Mon, 27 Jan 2025 15:46:17 -0500 Subject: [PATCH 3/9] fix: gas consumption in ApplyEvmMsg --- x/evm/keeper/call_contract.go | 6 ++- x/evm/keeper/keeper.go | 3 +- x/evm/keeper/msg_server.go | 72 +++++++++++++++-------------------- x/evm/precompile/funtoken.go | 3 +- 4 files changed, 37 insertions(+), 47 deletions(-) diff --git a/x/evm/keeper/call_contract.go b/x/evm/keeper/call_contract.go index 71a5391cf..5b925304d 100644 --- a/x/evm/keeper/call_contract.go +++ b/x/evm/keeper/call_contract.go @@ -39,7 +39,7 @@ func (k Keeper) CallContractWithInput( ) (evmResp *evm.MsgEthereumTxResponse, 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 HandleOutOfGasPanic(&err, "CallContractError") + defer HandleOutOfGasPanic(&err, "CallContractError")() nonce := k.GetAccNonce(ctx, fromAcc) unusedBigInt := big.NewInt(0) @@ -63,7 +63,9 @@ func (k Keeper) CallContractWithInput( evmResp, err = k.ApplyEvmMsg( ctx, evmMsg, evmObj, evm.NewNoOpTracer(), commit, txConfig.TxHash, true, ) - ctx.GasMeter().ConsumeGas(evmResp.GasUsed, "CallContractWithInput") + if evmResp != nil { + ctx.GasMeter().ConsumeGas(evmResp.GasUsed, "CallContractWithInput") + } if err != nil { return nil, errors.Wrap(err, "failed to apply ethereum core message") } diff --git a/x/evm/keeper/keeper.go b/x/evm/keeper/keeper.go index ff36cd6cb..5917e9635 100644 --- a/x/evm/keeper/keeper.go +++ b/x/evm/keeper/keeper.go @@ -128,13 +128,12 @@ func HandleOutOfGasPanic(err *error, format string) func() { if r := recover(); r != nil { switch r.(type) { case sdk.ErrorOutOfGas: - *err = vm.ErrOutOfGas default: panic(r) } } - if err != nil && format != "" { + if *err != nil && format != "" { *err = fmt.Errorf("%s: %w", format, *err) } } diff --git a/x/evm/keeper/msg_server.go b/x/evm/keeper/msg_server.go index 42742099b..d00c9610e 100644 --- a/x/evm/keeper/msg_server.go +++ b/x/evm/keeper/msg_server.go @@ -10,7 +10,6 @@ import ( "strconv" "cosmossdk.io/errors" - "cosmossdk.io/math" tmbytes "github.com/cometbft/cometbft/libs/bytes" tmtypes "github.com/cometbft/cometbft/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -73,7 +72,9 @@ func (k *Keeper) EthereumTx( txConfig.TxHash, false, /*fullRefundLeftoverGas*/ ) - ctx.GasMeter().ConsumeGas(evmResp.GasUsed, "execute ethereum tx") + if evmResp != nil { + ctx.GasMeter().ConsumeGas(evmResp.GasUsed, "execute ethereum tx") + } if err != nil { return nil, errors.Wrap(err, "error applying ethereum core message") } @@ -261,21 +262,20 @@ func (k *Keeper) ApplyEvmMsg( txHash gethcommon.Hash, fullRefundLeftoverGas bool, ) (resp *evm.MsgEthereumTxResponse, err error) { - leftoverGas := msg.Gas() + gasRemaining := msg.Gas() // Allow the tracer captures the tx level events, mainly the gas consumption. vmCfg := evmObj.Config if vmCfg.Debug { - vmCfg.Tracer.CaptureTxStart(leftoverGas) + vmCfg.Tracer.CaptureTxStart(gasRemaining) defer func() { - vmCfg.Tracer.CaptureTxEnd(leftoverGas) + vmCfg.Tracer.CaptureTxEnd(gasRemaining) }() } - sender := vm.AccountRef(msg.From()) contractCreation := msg.To() == nil - intrinsicGas, err := core.IntrinsicGas( + intrinsicGasCost, err := core.IntrinsicGas( msg.Data(), msg.AccessList(), contractCreation, true, true, ) @@ -290,15 +290,15 @@ func (k *Keeper) ApplyEvmMsg( // // Should check again even if it is checked on Ante Handler, because eth_call // don't go through Ante Handler. - if leftoverGas < intrinsicGas { + if gasRemaining < intrinsicGasCost { // eth_estimateGas will check for this exact error return nil, errors.Wrapf( core.ErrIntrinsicGas, "ApplyEvmMsg: provided msg.Gas (%d) is less than intrinsic gas cost (%d)", - leftoverGas, intrinsicGas, + gasRemaining, intrinsicGasCost, ) } - leftoverGas = leftoverGas - intrinsicGas + gasRemaining -= intrinsicGasCost // access list preparation is moved from ante handler to here, because it's // needed when `ApplyMessage` is called under contexts where ante handlers @@ -318,28 +318,28 @@ func (k *Keeper) ApplyEvmMsg( // take over the nonce management from evm: // - reset sender's nonce to msg.Nonce() before calling evm. // - increase sender's nonce by one no matter the result. - evmObj.StateDB.SetNonce(sender.Address(), msg.Nonce()) + evmObj.StateDB.SetNonce(msg.From(), msg.Nonce()) - var ret []byte + var returnBz []byte var vmErr error if contractCreation { - ret, _, leftoverGas, vmErr = evmObj.Create( - sender, + returnBz, _, gasRemaining, vmErr = evmObj.Create( + vm.AccountRef(msg.From()), msg.Data(), - leftoverGas, + gasRemaining, msgWei, ) } else { - ret, leftoverGas, vmErr = evmObj.Call( - sender, + returnBz, gasRemaining, vmErr = evmObj.Call( + vm.AccountRef(msg.From()), *msg.To(), msg.Data(), - leftoverGas, + gasRemaining, msgWei, ) } // Increment nonce after processing the message - evmObj.StateDB.SetNonce(sender.Address(), msg.Nonce()+1) + evmObj.StateDB.SetNonce(msg.From(), msg.Nonce()+1) // EVM execution error needs to be available for the JSON-RPC client var vmError string @@ -353,10 +353,6 @@ func (k *Keeper) ApplyEvmMsg( return nil, errors.Wrap(err, "ApplyEvmMsg: failed to commit stateDB") } } - // Rare case of uint64 gas overflow - if msg.Gas() < leftoverGas { - return nil, errors.Wrapf(core.ErrGasUintOverflow, "ApplyEvmMsg: message gas limit (%d) < leftover gas (%d)", msg.Gas(), leftoverGas) - } // TODO: UD-DEBUG: Clarify text below. // GAS REFUND @@ -373,31 +369,25 @@ func (k *Keeper) ApplyEvmMsg( if fullRefundLeftoverGas { refundQuotient = 1 // 100% refund } - temporaryGasUsed := msg.Gas() - leftoverGas - refund := GasToRefund(evmObj.StateDB.GetRefund(), temporaryGasUsed, refundQuotient) - - // update leftoverGas and temporaryGasUsed with refund amount - leftoverGas += refund - temporaryGasUsed -= refund - if msg.Gas() < leftoverGas { - return nil, errors.Wrapf(core.ErrGasUintOverflow, "ApplyEvmMsg: message gas limit (%d) < leftover gas (%d)", msg.Gas(), leftoverGas) - } - // Min gas used is a % of gasLimit - gasUsed := math.LegacyNewDec(int64(temporaryGasUsed)).TruncateInt().Uint64() + gasUsed := msg.Gas() - gasRemaining + refund := GasToRefund(evmObj.StateDB.GetRefund(), gasUsed, refundQuotient) - // This resulting "leftoverGas" is used by the tracer. This happens as a - // result of the defer statement near the beginning of the function with - // "vm.Tracer". - leftoverGas = msg.Gas() - gasUsed + gasRemaining += refund + gasUsed -= refund - return &evm.MsgEthereumTxResponse{ + evmResp := &evm.MsgEthereumTxResponse{ GasUsed: gasUsed, VmError: vmError, - Ret: ret, + Ret: returnBz, Logs: evm.NewLogsFromEth(evmObj.StateDB.(*statedb.StateDB).Logs()), Hash: txHash.Hex(), - }, nil + } + + if gasRemaining > msg.Gas() { + return evmResp, errors.Wrapf(core.ErrGasUintOverflow, "ApplyEvmMsg: message gas limit (%d) < leftover gas (%d)", msg.Gas(), gasRemaining) + } + return evmResp, nil } func ParseWeiAsMultipleOfMicronibi(weiInt *big.Int) (newWeiInt *big.Int, err error) { diff --git a/x/evm/precompile/funtoken.go b/x/evm/precompile/funtoken.go index 8ea61f3fb..82739c612 100644 --- a/x/evm/precompile/funtoken.go +++ b/x/evm/precompile/funtoken.go @@ -143,8 +143,7 @@ func (p precompileFunToken) sendToBank( erc20, amount, to, err := p.parseArgsSendToBank(args) if err != nil { - err = ErrInvalidArgs(err) - return + return nil, ErrInvalidArgs(err) } var evmResponses []*evm.MsgEthereumTxResponse From 53c1500ff838b765d41c6baad0b3452ba288104a Mon Sep 17 00:00:00 2001 From: Kevin Yang <5478483+k-yang@users.noreply.github.com> Date: Mon, 27 Jan 2025 15:50:08 -0500 Subject: [PATCH 4/9] fix: move commit after gas refund calculation so that we can return a valid evmResp --- x/evm/keeper/msg_server.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/x/evm/keeper/msg_server.go b/x/evm/keeper/msg_server.go index d00c9610e..745838d26 100644 --- a/x/evm/keeper/msg_server.go +++ b/x/evm/keeper/msg_server.go @@ -347,13 +347,6 @@ func (k *Keeper) ApplyEvmMsg( vmError = vmErr.Error() } - // The dirty states in `StateDB` is either committed or discarded after return - if commit { - if err := evmObj.StateDB.(*statedb.StateDB).Commit(); err != nil { - return nil, errors.Wrap(err, "ApplyEvmMsg: failed to commit stateDB") - } - } - // TODO: UD-DEBUG: Clarify text below. // GAS REFUND // If msg.Gas() > gasUsed, we need to refund extra gas. @@ -387,6 +380,14 @@ func (k *Keeper) ApplyEvmMsg( if gasRemaining > msg.Gas() { return evmResp, errors.Wrapf(core.ErrGasUintOverflow, "ApplyEvmMsg: message gas limit (%d) < leftover gas (%d)", msg.Gas(), gasRemaining) } + + // The dirty states in `StateDB` is either committed or discarded after return + if commit { + if err := evmObj.StateDB.(*statedb.StateDB).Commit(); err != nil { + return evmResp, errors.Wrap(err, "ApplyEvmMsg: failed to commit stateDB") + } + } + return evmResp, nil } From be107437ecca4924b91bd1ff6bb70278697621f8 Mon Sep 17 00:00:00 2001 From: Kevin Yang <5478483+k-yang@users.noreply.github.com> Date: Mon, 27 Jan 2025 15:54:31 -0500 Subject: [PATCH 5/9] refactor: GasToRefund --- x/evm/keeper/gas_fees.go | 30 ++++++++++++++++++++++-------- x/evm/keeper/msg_server.go | 25 +++++-------------------- 2 files changed, 27 insertions(+), 28 deletions(-) diff --git a/x/evm/keeper/gas_fees.go b/x/evm/keeper/gas_fees.go index d5712c067..2a9aeea46 100644 --- a/x/evm/keeper/gas_fees.go +++ b/x/evm/keeper/gas_fees.go @@ -75,15 +75,29 @@ func (k *Keeper) RefundGas( } // GasToRefund calculates the amount of gas the state machine should refund to -// the sender. It is capped by the refund quotient value. Note that passing a -// jrefundQuotient of 0 will cause problems. -func GasToRefund(availableRefund, gasConsumed, refundQuotient uint64) uint64 { - // Apply refund counter - refund := gasConsumed / refundQuotient - if refund > availableRefund { - return availableRefund +// the sender. +// +// GAS REFUND +// If msg.Gas() > gasUsed, we need to refund extra gas. +// leftoverGas = amount of extra (not used) gas. +// If the msg comes from user, we apply refundQuotient capping the refund to 20% of used gas +// If msg is internal (funtoken), we refund 100% +// +// EIP-3529: refunds are capped to gasUsed / 5 +// We evaluate "fullRefundLeftoverGas" and use only the gas consumed (not the +// gas limit) if the `ApplyEvmMsg` call originated from a state transition +// where the chain set the gas limit and not an end-user. +func GasToRefund(availableRefundAmount, gasUsed uint64, fullRefundLeftoverGas bool) uint64 { + refundQuotient := params.RefundQuotientEIP3529 + if fullRefundLeftoverGas { + refundQuotient = 1 // 100% refund + } + // Apply refundAmount counter + refundAmount := gasUsed / refundQuotient + if refundAmount > availableRefundAmount { + return availableRefundAmount } - return refund + return refundAmount } // CheckSenderBalance validates that the tx cost value is positive and that the diff --git a/x/evm/keeper/msg_server.go b/x/evm/keeper/msg_server.go index 745838d26..470a47ecf 100644 --- a/x/evm/keeper/msg_server.go +++ b/x/evm/keeper/msg_server.go @@ -347,27 +347,11 @@ func (k *Keeper) ApplyEvmMsg( vmError = vmErr.Error() } - // TODO: UD-DEBUG: Clarify text below. - // GAS REFUND - // If msg.Gas() > gasUsed, we need to refund extra gas. - // leftoverGas = amount of extra (not used) gas. - // If the msg comes from user, we apply refundQuotient capping the refund to 20% of used gas - // If msg is internal (funtoken), we refund 100% - // - // EIP-3529: refunds are capped to gasUsed / 5 - // We evaluate "fullRefundLeftoverGas" and use only the gas consumed (not the - // gas limit) if the `ApplyEvmMsg` call originated from a state transition - // where the chain set the gas limit and not an end-user. - refundQuotient := params.RefundQuotientEIP3529 - if fullRefundLeftoverGas { - refundQuotient = 1 // 100% refund - } - + // process gas refunds (we refund a portion of the unused gas) gasUsed := msg.Gas() - gasRemaining - refund := GasToRefund(evmObj.StateDB.GetRefund(), gasUsed, refundQuotient) - - gasRemaining += refund - gasUsed -= refund + refundAmount := GasToRefund(evmObj.StateDB.GetRefund(), gasUsed, fullRefundLeftoverGas) + gasRemaining += refundAmount + gasUsed -= refundAmount evmResp := &evm.MsgEthereumTxResponse{ GasUsed: gasUsed, @@ -378,6 +362,7 @@ func (k *Keeper) ApplyEvmMsg( } if gasRemaining > msg.Gas() { + evmResp.GasUsed = msg.Gas() // cap the gas used to the original gas limit return evmResp, errors.Wrapf(core.ErrGasUintOverflow, "ApplyEvmMsg: message gas limit (%d) < leftover gas (%d)", msg.Gas(), gasRemaining) } From 60f3b5c31bdb20ec535c480d120fcd1feca4b0e3 Mon Sep 17 00:00:00 2001 From: Oleg Nikonychev Date: Tue, 28 Jan 2025 10:49:37 +0400 Subject: [PATCH 6/9] fix: proper checking of err pointer nil Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- x/evm/keeper/keeper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x/evm/keeper/keeper.go b/x/evm/keeper/keeper.go index 5917e9635..19de7266d 100644 --- a/x/evm/keeper/keeper.go +++ b/x/evm/keeper/keeper.go @@ -133,7 +133,7 @@ func HandleOutOfGasPanic(err *error, format string) func() { panic(r) } } - if *err != nil && format != "" { + if err != nil && *err != nil && format != "" { *err = fmt.Errorf("%s: %w", format, *err) } } From 7387a405472b839ad2969846000060cb9b3a524b Mon Sep 17 00:00:00 2001 From: Kevin Yang <5478483+k-yang@users.noreply.github.com> Date: Tue, 28 Jan 2025 12:28:07 -0500 Subject: [PATCH 7/9] fix: remove fullRefundLeftoverGas --- x/evm/keeper/call_contract.go | 2 +- x/evm/keeper/gas_fees.go | 15 ++++----------- x/evm/keeper/grpc_query.go | 8 ++++---- x/evm/keeper/msg_server.go | 11 +++++------ x/evm/precompile/funtoken_test.go | 24 ++++++++++++++---------- 5 files changed, 28 insertions(+), 32 deletions(-) diff --git a/x/evm/keeper/call_contract.go b/x/evm/keeper/call_contract.go index 5b925304d..032d30bd7 100644 --- a/x/evm/keeper/call_contract.go +++ b/x/evm/keeper/call_contract.go @@ -61,7 +61,7 @@ func (k Keeper) CallContractWithInput( // sent by a user txConfig := k.TxConfig(ctx, gethcommon.BigToHash(big.NewInt(0))) evmResp, err = k.ApplyEvmMsg( - ctx, evmMsg, evmObj, evm.NewNoOpTracer(), commit, txConfig.TxHash, true, + ctx, evmMsg, evmObj, evm.NewNoOpTracer(), commit, txConfig.TxHash, ) if evmResp != nil { ctx.GasMeter().ConsumeGas(evmResp.GasUsed, "CallContractWithInput") diff --git a/x/evm/keeper/gas_fees.go b/x/evm/keeper/gas_fees.go index 2a9aeea46..e4bc44a00 100644 --- a/x/evm/keeper/gas_fees.go +++ b/x/evm/keeper/gas_fees.go @@ -74,7 +74,7 @@ func (k *Keeper) RefundGas( return nil } -// GasToRefund calculates the amount of gas the state machine should refund to +// gasToRefund calculates the amount of gas the state machine should refund to // the sender. // // GAS REFUND @@ -84,17 +84,10 @@ func (k *Keeper) RefundGas( // If msg is internal (funtoken), we refund 100% // // EIP-3529: refunds are capped to gasUsed / 5 -// We evaluate "fullRefundLeftoverGas" and use only the gas consumed (not the -// gas limit) if the `ApplyEvmMsg` call originated from a state transition -// where the chain set the gas limit and not an end-user. -func GasToRefund(availableRefundAmount, gasUsed uint64, fullRefundLeftoverGas bool) uint64 { - refundQuotient := params.RefundQuotientEIP3529 - if fullRefundLeftoverGas { - refundQuotient = 1 // 100% refund - } - // Apply refundAmount counter - refundAmount := gasUsed / refundQuotient +func gasToRefund(availableRefundAmount, gasUsed uint64) uint64 { + refundAmount := gasUsed / params.RefundQuotientEIP3529 if refundAmount > availableRefundAmount { + // Apply refundAmount counter return availableRefundAmount } return refundAmount diff --git a/x/evm/keeper/grpc_query.go b/x/evm/keeper/grpc_query.go index 3255b8f73..da2274d50 100644 --- a/x/evm/keeper/grpc_query.go +++ b/x/evm/keeper/grpc_query.go @@ -286,7 +286,7 @@ func (k *Keeper) EthCall( // pass false to not commit StateDB stateDB := statedb.New(ctx, k, txConfig) evm := k.NewEVM(ctx, msg, evmCfg, nil /*tracer*/, stateDB) - res, err := k.ApplyEvmMsg(ctx, msg, evm, nil /*tracer*/, false /*commit*/, txConfig.TxHash, false /*fullRefundLeftoverGas*/) + res, err := k.ApplyEvmMsg(ctx, msg, evm, nil /*tracer*/, false /*commit*/, txConfig.TxHash) if err != nil { return nil, grpcstatus.Error(grpccodes.Internal, err.Error()) } @@ -421,7 +421,7 @@ func (k Keeper) EstimateGasForEvmCallType( txConfig := statedb.NewEmptyTxConfig(gethcommon.BytesToHash(ctx.HeaderHash().Bytes())) stateDB := statedb.New(ctx, &k, txConfig) evmObj := k.NewEVM(tmpCtx, evmMsg, evmCfg, nil /*tracer*/, stateDB) - rsp, err = k.ApplyEvmMsg(tmpCtx, evmMsg, evmObj, nil /*tracer*/, false /*commit*/, txConfig.TxHash, false /*fullRefundLeftoverGas*/) + rsp, err = k.ApplyEvmMsg(tmpCtx, evmMsg, evmObj, nil /*tracer*/, false /*commit*/, txConfig.TxHash) if err != nil { if errors.Is(err, core.ErrIntrinsicGas) { return true, nil, nil // Special case, raise gas limit @@ -516,7 +516,7 @@ func (k Keeper) TraceTx( WithTransientKVGasConfig(storetypes.GasConfig{}) stateDB := statedb.New(ctx, &k, txConfig) evmObj := k.NewEVM(ctx, msg, evmCfg, nil /*tracer*/, stateDB) - rsp, err := k.ApplyEvmMsg(ctx, msg, evmObj, nil /*tracer*/, false /*commit*/, txConfig.TxHash, false /*fullRefundLeftoverGas*/) + rsp, err := k.ApplyEvmMsg(ctx, msg, evmObj, nil /*tracer*/, false /*commit*/, txConfig.TxHash) if err != nil { continue } @@ -790,7 +790,7 @@ func (k *Keeper) TraceEthTxMsg( WithTransientKVGasConfig(storetypes.GasConfig{}) stateDB := statedb.New(ctx, k, txConfig) evmObj := k.NewEVM(ctx, msg, evmCfg, tracer, stateDB) - res, err := k.ApplyEvmMsg(ctx, msg, evmObj, tracer, false /*commit*/, txConfig.TxHash, false /*fullRefundLeftoverGas*/) + res, err := k.ApplyEvmMsg(ctx, msg, evmObj, tracer, false /*commit*/, txConfig.TxHash) if err != nil { return nil, 0, grpcstatus.Error(grpccodes.Internal, err.Error()) } diff --git a/x/evm/keeper/msg_server.go b/x/evm/keeper/msg_server.go index 470a47ecf..e5421725c 100644 --- a/x/evm/keeper/msg_server.go +++ b/x/evm/keeper/msg_server.go @@ -70,7 +70,6 @@ func (k *Keeper) EthereumTx( nil, /*tracer*/ true, /*commit*/ txConfig.TxHash, - false, /*fullRefundLeftoverGas*/ ) if evmResp != nil { ctx.GasMeter().ConsumeGas(evmResp.GasUsed, "execute ethereum tx") @@ -260,7 +259,6 @@ func (k *Keeper) ApplyEvmMsg( tracer vm.EVMLogger, commit bool, txHash gethcommon.Hash, - fullRefundLeftoverGas bool, ) (resp *evm.MsgEthereumTxResponse, err error) { gasRemaining := msg.Gas() @@ -349,7 +347,8 @@ func (k *Keeper) ApplyEvmMsg( // process gas refunds (we refund a portion of the unused gas) gasUsed := msg.Gas() - gasRemaining - refundAmount := GasToRefund(evmObj.StateDB.GetRefund(), gasUsed, fullRefundLeftoverGas) + // please see https://eips.ethereum.org/EIPS/eip-3529 for why we do refunds + refundAmount := gasToRefund(evmObj.StateDB.GetRefund(), gasUsed) gasRemaining += refundAmount gasUsed -= refundAmount @@ -361,7 +360,7 @@ func (k *Keeper) ApplyEvmMsg( Hash: txHash.Hex(), } - if gasRemaining > msg.Gas() { + if gasRemaining > msg.Gas() { // rare case of overflow evmResp.GasUsed = msg.Gas() // cap the gas used to the original gas limit return evmResp, errors.Wrapf(core.ErrGasUintOverflow, "ApplyEvmMsg: message gas limit (%d) < leftover gas (%d)", msg.Gas(), gasRemaining) } @@ -536,6 +535,7 @@ func (k Keeper) convertCoinToEvmBornCoin( defer func() { k.Bank.StateDB = nil }() + evmObj := k.NewEVM(ctx, evmMsg, k.GetEVMConfig(ctx), nil /*tracer*/, stateDB) evmResp, err := k.CallContractWithInput( ctx, @@ -555,8 +555,7 @@ func (k Keeper) convertCoinToEvmBornCoin( fmt.Errorf("failed to mint erc-20 tokens of contract %s", erc20Addr.String()) } - err = stateDB.Commit() - if err != nil { + if err = stateDB.Commit(); err != nil { return nil, errors.Wrap(err, "failed to commit stateDB") } diff --git a/x/evm/precompile/funtoken_test.go b/x/evm/precompile/funtoken_test.go index 2cd2bd34b..56cdf71ad 100644 --- a/x/evm/precompile/funtoken_test.go +++ b/x/evm/precompile/funtoken_test.go @@ -256,13 +256,14 @@ func (s *FuntokenSuite) TestPrecompileLocalGas() { s.Require().NoError(err) contractAddr := deployResp.ContractAddr - s.T().Log("Fund sender's wallet") - s.Require().NoError(testapp.FundAccount( - deps.App.BankKeeper, - deps.Ctx, - deps.Sender.NibiruAddr, - sdk.NewCoins(sdk.NewCoin(funtoken.BankDenom, sdk.NewInt(1000))), - )) + s.Run("Fund sender's wallet", func() { + s.Require().NoError(testapp.FundAccount( + deps.App.BankKeeper, + deps.Ctx, + deps.Sender.NibiruAddr, + sdk.NewCoins(sdk.NewCoin(funtoken.BankDenom, sdk.NewInt(1000))), + )) + }) s.Run("Fund contract with erc20 coins", func() { _, err = deps.EvmKeeper.ConvertCoinToEvm( @@ -286,7 +287,7 @@ func (s *FuntokenSuite) TestPrecompileLocalGas() { ) s.Require().NoError(err) evmObj, _ := deps.NewEVM() - _, err = deps.EvmKeeper.CallContractWithInput( + resp, err := deps.EvmKeeper.CallContractWithInput( deps.Ctx, evmObj, deps.Sender.EthAddr, @@ -296,6 +297,7 @@ func (s *FuntokenSuite) TestPrecompileLocalGas() { evmtest.FunTokenGasLimitSendToEvm, ) s.Require().NoError(err) + s.Require().NotZero(resp.GasUsed) }) s.Run("Happy: callBankSend with local gas - sufficient gas amount", func() { @@ -307,7 +309,7 @@ func (s *FuntokenSuite) TestPrecompileLocalGas() { ) s.Require().NoError(err) evmObj, _ := deps.NewEVM() - _, err = deps.EvmKeeper.CallContractWithInput( + resp, err := deps.EvmKeeper.CallContractWithInput( deps.Ctx, evmObj, deps.Sender.EthAddr, @@ -317,6 +319,7 @@ func (s *FuntokenSuite) TestPrecompileLocalGas() { evmtest.FunTokenGasLimitSendToEvm, // gasLimit for the entire call ) s.Require().NoError(err) + s.Require().NotZero(resp.GasUsed) }) s.Run("Sad: callBankSend with local gas - insufficient gas amount", func() { @@ -328,7 +331,7 @@ func (s *FuntokenSuite) TestPrecompileLocalGas() { ) s.Require().NoError(err) evmObj, _ := deps.NewEVM() - _, err = deps.EvmKeeper.CallContractWithInput( + resp, err := deps.EvmKeeper.CallContractWithInput( deps.Ctx, evmObj, deps.Sender.EthAddr, @@ -338,6 +341,7 @@ func (s *FuntokenSuite) TestPrecompileLocalGas() { evmtest.FunTokenGasLimitSendToEvm, // gasLimit for the entire call ) s.Require().ErrorContains(err, "execution reverted") + s.Require().NotZero(resp.GasUsed) }) } From 4850501032e586c0f38aa28418d8fe8bcba6b03d Mon Sep 17 00:00:00 2001 From: Kevin Yang <5478483+k-yang@users.noreply.github.com> Date: Tue, 28 Jan 2025 12:37:01 -0500 Subject: [PATCH 8/9] test: add gas assertions --- x/evm/keeper/funtoken_from_coin_test.go | 34 ++++- x/evm/keeper/funtoken_from_erc20_test.go | 183 ++++++++++++++--------- 2 files changed, 142 insertions(+), 75 deletions(-) diff --git a/x/evm/keeper/funtoken_from_coin_test.go b/x/evm/keeper/funtoken_from_coin_test.go index 7a75afca6..5f4d85630 100644 --- a/x/evm/keeper/funtoken_from_coin_test.go +++ b/x/evm/keeper/funtoken_from_coin_test.go @@ -82,6 +82,7 @@ func (s *FunTokenFromCoinSuite) TestCreateFunTokenFromCoin() { }) s.Run("happy: CreateFunToken for the bank coin", func() { + deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) s.Require().NoError(testapp.FundAccount( deps.App.BankKeeper, deps.Ctx, @@ -97,6 +98,7 @@ func (s *FunTokenFromCoinSuite) TestCreateFunTokenFromCoin() { }, ) s.Require().NoError(err) + s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed()) s.Equal( createFuntokenResp.FuntokenMapping, @@ -167,6 +169,7 @@ func (s *FunTokenFromCoinSuite) TestConvertCoinToEvmAndBack() { funToken := s.fundAndCreateFunToken(deps, 100) s.T().Log("Convert bank coin to erc-20") + deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) _, err := deps.EvmKeeper.ConvertCoinToEvm( sdk.WrapSDKContext(deps.Ctx), &evm.MsgConvertCoinToEvm{ @@ -178,6 +181,7 @@ func (s *FunTokenFromCoinSuite) TestConvertCoinToEvmAndBack() { }, ) s.Require().NoError(err) + s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed()) s.T().Log("Check typed event") testutil.RequireContainsTypedEvent( @@ -226,6 +230,7 @@ func (s *FunTokenFromCoinSuite) TestConvertCoinToEvmAndBack() { deps.Sender.NibiruAddr.String(), ) s.Require().NoError(err) + deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) evmObj, _ = deps.NewEVM() _, err = deps.EvmKeeper.CallContractWithInput( deps.Ctx, @@ -237,6 +242,7 @@ func (s *FunTokenFromCoinSuite) TestConvertCoinToEvmAndBack() { evmtest.FunTokenGasLimitSendToEvm, ) s.Require().NoError(err) + s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed()) // Check 1: module balance moduleBalance = deps.App.BankKeeper.GetBalance(deps.Ctx, authtypes.NewModuleAddress(evm.ModuleName), evm.EVMBankDenom) @@ -252,6 +258,7 @@ func (s *FunTokenFromCoinSuite) TestConvertCoinToEvmAndBack() { s.Require().Equal("0", balance.String()) s.T().Log("sad: Convert more erc-20 to back to bank coin, insufficient funds") + deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) evmObj, _ = deps.NewEVM() _, err = deps.EvmKeeper.CallContractWithInput( deps.Ctx, @@ -263,6 +270,7 @@ func (s *FunTokenFromCoinSuite) TestConvertCoinToEvmAndBack() { evmtest.FunTokenGasLimitSendToEvm, ) s.Require().ErrorContains(err, "transfer amount exceeds balance") + s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed()) } // TestNativeSendThenPrecompileSend tests a race condition where the state DB @@ -362,6 +370,7 @@ func (s *FunTokenFromCoinSuite) TestNativeSendThenPrecompileSend() { newSendAmtSendToBank, /*amount*/ ) s.Require().NoError(err) + deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) evmObj, _ = deps.NewEVM() evmResp, err := deps.EvmKeeper.CallContractWithInput( deps.Ctx, @@ -373,6 +382,9 @@ func (s *FunTokenFromCoinSuite) TestNativeSendThenPrecompileSend() { evmtest.FunTokenGasLimitSendToEvm, ) s.Require().NoError(err) + s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed()) + s.Require().NotZero(evmResp.GasUsed) + s.Require().Greaterf(deps.Ctx.GasMeter().GasConsumed(), evmResp.GasUsed, "total gas consumed on cosmos context should be greater than gas used by EVM") s.Empty(evmResp.VmError) gasUsedFor2Ops := evmResp.GasUsed @@ -404,6 +416,7 @@ func (s *FunTokenFromCoinSuite) TestNativeSendThenPrecompileSend() { newSendAmtSendToBank, /*amount*/ ) s.Require().NoError(err) + deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) evmObj, _ = deps.NewEVM() evmResp, err = deps.EvmKeeper.CallContractWithInput( deps.Ctx, @@ -415,6 +428,9 @@ func (s *FunTokenFromCoinSuite) TestNativeSendThenPrecompileSend() { evmtest.DefaultEthCallGasLimit, ) s.Require().NoError(err) + s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed()) + s.Require().NotZero(evmResp.GasUsed) + s.Require().Greaterf(deps.Ctx.GasMeter().GasConsumed(), evmResp.GasUsed, "total gas consumed on cosmos context should be greater than gas used by EVM") s.Empty(evmResp.VmError) gasUsedFor1Op := evmResp.GasUsed @@ -517,8 +533,9 @@ func (s *FunTokenFromCoinSuite) TestERC20TransferThenPrecompileSend() { big.NewInt(9e6), /*amount*/ ) s.Require().NoError(err) + deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) evmObj, _ = deps.NewEVM() - _, err = deps.EvmKeeper.CallContractWithInput( + evmResp, err := deps.EvmKeeper.CallContractWithInput( deps.Ctx, evmObj, deps.Sender.EthAddr, // from @@ -528,6 +545,9 @@ func (s *FunTokenFromCoinSuite) TestERC20TransferThenPrecompileSend() { 10_000_000, // gas limit ) s.Require().NoError(err) + s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed()) + s.Require().NotZero(evmResp.GasUsed) + s.Require().Greaterf(deps.Ctx.GasMeter().GasConsumed(), evmResp.GasUsed, "total gas consumed on cosmos context should be greater than gas used by EVM") evmtest.FunTokenBalanceAssert{ FunToken: funToken, @@ -620,6 +640,7 @@ func (s *FunTokenFromCoinSuite) TestPrecompileSelfCallRevert() { charles := evmtest.NewEthPrivAcc() s.T().Log("call test contract") + deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) evmObj, _ = deps.NewEVM() contractInput, err := embeds.SmartContract_TestPrecompileSelfCallRevert.ABI.Pack( "selfCallTransferFunds", @@ -629,7 +650,7 @@ func (s *FunTokenFromCoinSuite) TestPrecompileSelfCallRevert() { big.NewInt(9e6), ) s.Require().NoError(err) - _, err = deps.EvmKeeper.CallContractWithInput( + evpResp, err := deps.EvmKeeper.CallContractWithInput( deps.Ctx, evmObj, deps.Sender.EthAddr, @@ -639,6 +660,9 @@ func (s *FunTokenFromCoinSuite) TestPrecompileSelfCallRevert() { evmtest.FunTokenGasLimitSendToEvm, ) s.Require().NoError(err) + s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed()) + s.Require().NotZero(evpResp.GasUsed) + s.Require().Greaterf(deps.Ctx.GasMeter().GasConsumed(), evpResp.GasUsed, "total gas consumed on cosmos context should be greater than gas used by EVM") evmtest.FunTokenBalanceAssert{ FunToken: funToken, @@ -726,8 +750,9 @@ func (s *FunTokenFromCoinSuite) TestPrecompileSendToBankThenErc20Transfer() { "attack", ) s.Require().NoError(err) + deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) evmObj, _ := deps.NewEVM() - _, err = deps.EvmKeeper.CallContractWithInput( + evpResp, err := deps.EvmKeeper.CallContractWithInput( deps.Ctx, evmObj, deps.Sender.EthAddr, @@ -737,6 +762,9 @@ func (s *FunTokenFromCoinSuite) TestPrecompileSendToBankThenErc20Transfer() { evmtest.FunTokenGasLimitSendToEvm, ) s.Require().ErrorContains(err, "execution reverted") + s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed()) + s.Require().NotZero(evpResp.GasUsed) + s.Require().Greaterf(deps.Ctx.GasMeter().GasConsumed(), evpResp.GasUsed, "total gas consumed on cosmos context should be greater than gas used by EVM") evmtest.FunTokenBalanceAssert{ FunToken: funToken, diff --git a/x/evm/keeper/funtoken_from_erc20_test.go b/x/evm/keeper/funtoken_from_erc20_test.go index 438b928ba..86960a2f3 100644 --- a/x/evm/keeper/funtoken_from_erc20_test.go +++ b/x/evm/keeper/funtoken_from_erc20_test.go @@ -42,7 +42,6 @@ func (s *FunTokenFromErc20Suite) TestCreateFunTokenFromERC20() { s.Require().Equal(expectedERC20Addr, deployResp.ContractAddr) evmObj, _ := deps.NewEVM() - actualMetadata, err := deps.EvmKeeper.FindERC20Metadata(deps.Ctx, evmObj, deployResp.ContractAddr, nil) s.Require().NoError(err) s.Require().Equal(metadata, *actualMetadata) @@ -75,6 +74,7 @@ func (s *FunTokenFromErc20Suite) TestCreateFunTokenFromERC20() { deps.EvmKeeper.FeeForCreateFunToken(deps.Ctx), )) + deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) resp, err := deps.EvmKeeper.CreateFunToken( sdk.WrapSDKContext(deps.Ctx), &evm.MsgCreateFunToken{ @@ -83,6 +83,7 @@ func (s *FunTokenFromErc20Suite) TestCreateFunTokenFromERC20() { }, ) s.Require().NoError(err, "erc20 %s", erc20Addr) + s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed()) expectedBankDenom := fmt.Sprintf("erc20/%s", expectedERC20Addr.String()) s.Equal( @@ -199,79 +200,100 @@ func (s *FunTokenFromErc20Suite) TestSendFromEvmToBank_MadeFromErc20() { s.Require().NoError(err, "erc20 %s", deployResp.ContractAddr) bankDemon := resp.FuntokenMapping.BankDenom - s.T().Logf("mint erc20 tokens to %s", deps.Sender.EthAddr.String()) - contractInput, err := embeds.SmartContract_ERC20Minter.ABI.Pack("mint", deps.Sender.EthAddr, big.NewInt(69_420)) - s.Require().NoError(err) - evmObj, _ := deps.NewEVM() - _, err = deps.EvmKeeper.CallContractWithInput( - deps.Ctx, - evmObj, - deps.Sender.EthAddr, /*from*/ - &deployResp.ContractAddr, /*to*/ - true, /*commit*/ - contractInput, - keeper.Erc20GasLimitExecute, - ) - s.Require().NoError(err) + s.Run("happy: mint erc20 tokens", func() { + contractInput, err := embeds.SmartContract_ERC20Minter.ABI.Pack("mint", deps.Sender.EthAddr, big.NewInt(69_420)) + s.Require().NoError(err) + deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) + evmObj, _ := deps.NewEVM() + evmResp, err := deps.EvmKeeper.CallContractWithInput( + deps.Ctx, + evmObj, + deps.Sender.EthAddr, /*from*/ + &deployResp.ContractAddr, /*to*/ + true, /*commit*/ + contractInput, + keeper.Erc20GasLimitExecute, + ) + s.Require().NoError(err) + s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed()) + s.Require().NotZero(evmResp.GasUsed) + s.Require().Greater(deps.Ctx.GasMeter().GasConsumed(), evmResp.GasUsed) + }) randomAcc := testutil.AccAddress() + s.Run("happy: send erc20 tokens to Bank", func() { + contractInput, err := embeds.SmartContract_FunToken.ABI.Pack("sendToBank", deployResp.ContractAddr, big.NewInt(1), randomAcc.String()) + s.Require().NoError(err) + deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) + evmObj, _ := deps.NewEVM() + evmResp, err := deps.EvmKeeper.CallContractWithInput( + deps.Ctx, + evmObj, + deps.Sender.EthAddr, /*from*/ + &precompile.PrecompileAddr_FunToken, /*to*/ + true, /*commit*/ + contractInput, + evmtest.FunTokenGasLimitSendToEvm, + ) + s.Require().NoError(err) + s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed()) + s.Require().NotZero(evmResp.GasUsed) + s.Require().Greater(deps.Ctx.GasMeter().GasConsumed(), evmResp.GasUsed) + }) - s.T().Log("happy: send erc20 tokens to Bank") - contractInput, err = embeds.SmartContract_FunToken.ABI.Pack("sendToBank", deployResp.ContractAddr, big.NewInt(1), randomAcc.String()) - s.Require().NoError(err) - evmObj, _ = deps.NewEVM() - _, err = deps.EvmKeeper.CallContractWithInput( - deps.Ctx, - evmObj, - deps.Sender.EthAddr, /*from*/ - &precompile.PrecompileAddr_FunToken, /*to*/ - true, /*commit*/ - contractInput, - evmtest.FunTokenGasLimitSendToEvm, - ) - s.Require().NoError(err) - - s.T().Log("check balances") - evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, deployResp.ContractAddr, deps.Sender.EthAddr, big.NewInt(69_419), "expect nonzero balance") - evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, deployResp.ContractAddr, evm.EVM_MODULE_ADDRESS, big.NewInt(1), "expect nonzero balance") - s.Require().Equal(sdk.NewInt(1), - deps.App.BankKeeper.GetBalance(deps.Ctx, randomAcc, bankDemon).Amount, - ) + s.Run("happy: check balances", func() { + evmObj, _ := deps.NewEVM() + evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, deployResp.ContractAddr, deps.Sender.EthAddr, big.NewInt(69_419), "expect nonzero balance") + evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, deployResp.ContractAddr, evm.EVM_MODULE_ADDRESS, big.NewInt(1), "expect nonzero balance") + s.Require().Equal(sdk.NewInt(1), + deps.App.BankKeeper.GetBalance(deps.Ctx, randomAcc, bankDemon).Amount, + ) + }) - s.T().Log("sad: send too many erc20 tokens to Bank") - contractInput, err = embeds.SmartContract_FunToken.ABI.Pack("sendToBank", deployResp.ContractAddr, big.NewInt(70_000), randomAcc.String()) - s.Require().NoError(err) - evmObj, _ = deps.NewEVM() - evmResp, err := deps.EvmKeeper.CallContractWithInput( - deps.Ctx, - evmObj, - deps.Sender.EthAddr, /*from*/ - &precompile.PrecompileAddr_FunToken, /*to*/ - true, /*commit*/ - contractInput, - evmtest.FunTokenGasLimitSendToEvm, - ) - s.Require().Error(err, evmResp.String()) + s.Run("sad: send too many erc20 tokens to Bank", func() { + contractInput, err := embeds.SmartContract_FunToken.ABI.Pack("sendToBank", deployResp.ContractAddr, big.NewInt(70_000), randomAcc.String()) + s.Require().NoError(err) + deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) + evmObj, _ := deps.NewEVM() + evmResp, err := deps.EvmKeeper.CallContractWithInput( + deps.Ctx, + evmObj, + deps.Sender.EthAddr, /*from*/ + &precompile.PrecompileAddr_FunToken, /*to*/ + true, /*commit*/ + contractInput, + evmtest.FunTokenGasLimitSendToEvm, + ) + s.Require().Error(err, evmResp.String()) + s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed()) + s.Require().NotZero(evmResp.GasUsed) + s.Require().Greater(deps.Ctx.GasMeter().GasConsumed(), evmResp.GasUsed) + }) - s.T().Log("send Bank tokens back to erc20") - _, err = deps.EvmKeeper.ConvertCoinToEvm(sdk.WrapSDKContext(deps.Ctx), - &evm.MsgConvertCoinToEvm{ - ToEthAddr: eth.EIP55Addr{ - Address: deps.Sender.EthAddr, + s.Run("happy: send Bank tokens back to erc20", func() { + deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) + _, err = deps.EvmKeeper.ConvertCoinToEvm(sdk.WrapSDKContext(deps.Ctx), + &evm.MsgConvertCoinToEvm{ + ToEthAddr: eth.EIP55Addr{ + Address: deps.Sender.EthAddr, + }, + Sender: randomAcc.String(), + BankCoin: sdk.NewCoin(bankDemon, sdk.NewInt(1)), }, - Sender: randomAcc.String(), - BankCoin: sdk.NewCoin(bankDemon, sdk.NewInt(1)), - }, - ) - s.Require().NoError(err) + ) + s.Require().NoError(err) + s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed()) + }) s.T().Log("check balances") - evmObj, _ = deps.NewEVM() - evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, deployResp.ContractAddr, deps.Sender.EthAddr, big.NewInt(69_420), "expect nonzero balance") - evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, deployResp.ContractAddr, evm.EVM_MODULE_ADDRESS, big.NewInt(0), "expect nonzero balance") - s.Require().True( - deps.App.BankKeeper.GetBalance(deps.Ctx, randomAcc, bankDemon).Amount.Equal(sdk.NewInt(0)), - ) + s.Run("happy: check balances", func() { + evmObj, _ := deps.NewEVM() + evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, deployResp.ContractAddr, deps.Sender.EthAddr, big.NewInt(69_420), "expect nonzero balance") + evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, deployResp.ContractAddr, evm.EVM_MODULE_ADDRESS, big.NewInt(0), "expect nonzero balance") + s.Require().True( + deps.App.BankKeeper.GetBalance(deps.Ctx, randomAcc, bankDemon).Amount.Equal(sdk.NewInt(0)), + ) + }) s.T().Log("sad: send too many Bank tokens back to erc20") _, err = deps.EvmKeeper.ConvertCoinToEvm(sdk.WrapSDKContext(deps.Ctx), @@ -368,8 +390,9 @@ func (s *FunTokenFromErc20Suite) TestFunTokenFromERC20MaliciousTransfer() { s.T().Log("send erc20 tokens to cosmos") input, err := embeds.SmartContract_FunToken.ABI.Pack("sendToBank", deployResp.ContractAddr, big.NewInt(1), randomAcc.String()) s.Require().NoError(err) + deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) evmObj, _ := deps.NewEVM() - _, err = deps.EvmKeeper.CallContractWithInput( + evmResp, err := deps.EvmKeeper.CallContractWithInput( deps.Ctx, evmObj, evm.EVM_MODULE_ADDRESS, @@ -379,6 +402,9 @@ func (s *FunTokenFromErc20Suite) TestFunTokenFromERC20MaliciousTransfer() { evmtest.FunTokenGasLimitSendToEvm, ) s.Require().ErrorContains(err, "gas required exceeds allowance") + s.Require().NotZero(evmResp.GasUsed) + s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed()) + s.Require().Greater(deps.Ctx.GasMeter().GasConsumed(), evmResp.GasUsed) } // TestFunTokenInfiniteRecursionERC20 creates a funtoken from a contract @@ -422,7 +448,7 @@ func (s *FunTokenFromErc20Suite) TestFunTokenInfiniteRecursionERC20() { contractInput, err := embeds.SmartContract_TestInfiniteRecursionERC20.ABI.Pack("attackBalance") s.Require().NoError(err) evmObj, _ := deps.NewEVM() - _, err = deps.EvmKeeper.CallContractWithInput( + evmResp, err := deps.EvmKeeper.CallContractWithInput( deps.Ctx, evmObj, deps.Sender.EthAddr, /*from*/ @@ -432,12 +458,15 @@ func (s *FunTokenFromErc20Suite) TestFunTokenInfiniteRecursionERC20() { 10_000_000, ) s.Require().NoError(err) + s.Require().NotZero(evmResp.GasUsed) + s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed()) + s.Require().Greater(deps.Ctx.GasMeter().GasConsumed(), evmResp.GasUsed) s.T().Log("sad: call attackTransfer()") contractInput, err = embeds.SmartContract_TestInfiniteRecursionERC20.ABI.Pack("attackTransfer") s.Require().NoError(err) evmObj, _ = deps.NewEVM() - _, err = deps.EvmKeeper.CallContractWithInput( + evmResp, err = deps.EvmKeeper.CallContractWithInput( deps.Ctx, evmObj, deps.Sender.EthAddr, /*from*/ @@ -447,6 +476,9 @@ func (s *FunTokenFromErc20Suite) TestFunTokenInfiniteRecursionERC20() { 10_000_000, ) s.Require().ErrorContains(err, "execution reverted") + s.Require().NotZero(evmResp.GasUsed) + s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed()) + s.Require().Greater(deps.Ctx.GasMeter().GasConsumed(), evmResp.GasUsed) } // TestSendERC20WithFee creates a funtoken from a malicious contract which charges a 10% fee on any transfer. @@ -494,8 +526,9 @@ func (s *FunTokenFromErc20Suite) TestSendERC20WithFee() { randomAcc.String(), /*to*/ ) s.Require().NoError(err) + deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) evmObj, _ := deps.NewEVM() - _, err = deps.EvmKeeper.CallContractWithInput( + evmResp, err := deps.EvmKeeper.CallContractWithInput( deps.Ctx, evmObj, deps.Sender.EthAddr, /*from*/ @@ -505,6 +538,9 @@ func (s *FunTokenFromErc20Suite) TestSendERC20WithFee() { evmtest.FunTokenGasLimitSendToEvm, ) s.Require().NoError(err) + s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed()) + s.Require().NotZero(evmResp.GasUsed) + s.Require().Greater(deps.Ctx.GasMeter().GasConsumed(), evmResp.GasUsed) s.T().Log("check balances") evmtest.AssertERC20BalanceEqualWithDescription(s.T(), deps, evmObj, deployResp.ContractAddr, deps.Sender.EthAddr, big.NewInt(900), "expect 900 balance") @@ -569,8 +605,9 @@ func (s *FunTokenFromErc20Suite) TestFindMKRMetadata() { ) s.Require().NoError(err) + deps.Ctx = deps.Ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) evmObj, _ := deps.NewEVM() - _, err = deps.EvmKeeper.CallContractWithInput( + evmResp, err := deps.EvmKeeper.CallContractWithInput( deps.Ctx, evmObj, deps.Sender.EthAddr, @@ -579,8 +616,10 @@ func (s *FunTokenFromErc20Suite) TestFindMKRMetadata() { contractInput, evmtest.FunTokenGasLimitSendToEvm, ) - s.Require().NoError(err) + s.Require().NotZero(deps.Ctx.GasMeter().GasConsumed()) + s.Require().NotZero(evmResp.GasUsed) + s.Require().Greater(deps.Ctx.GasMeter().GasConsumed(), evmResp.GasUsed) info, err := deps.EvmKeeper.FindERC20Metadata(deps.Ctx, evmObj, deployResp.ContractAddr, embeds.SmartContract_TestBytes32Metadata.ABI) s.Require().NoError(err) From a55e5b23ca2ac10703703e0e10b8ad031885a320 Mon Sep 17 00:00:00 2001 From: Kevin Yang <5478483+k-yang@users.noreply.github.com> Date: Tue, 28 Jan 2025 13:30:45 -0500 Subject: [PATCH 9/9] Update gas_fees.go --- x/evm/keeper/gas_fees.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/x/evm/keeper/gas_fees.go b/x/evm/keeper/gas_fees.go index e4bc44a00..8b8a6a495 100644 --- a/x/evm/keeper/gas_fees.go +++ b/x/evm/keeper/gas_fees.go @@ -76,13 +76,6 @@ func (k *Keeper) RefundGas( // gasToRefund calculates the amount of gas the state machine should refund to // the sender. -// -// GAS REFUND -// If msg.Gas() > gasUsed, we need to refund extra gas. -// leftoverGas = amount of extra (not used) gas. -// If the msg comes from user, we apply refundQuotient capping the refund to 20% of used gas -// If msg is internal (funtoken), we refund 100% -// // EIP-3529: refunds are capped to gasUsed / 5 func gasToRefund(availableRefundAmount, gasUsed uint64) uint64 { refundAmount := gasUsed / params.RefundQuotientEIP3529