diff --git a/CHANGELOG.md b/CHANGELOG.md index 06a5b4824..78369bfbf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,8 @@ - [\#967](https://github.com/cosmos/evm/pull/967) Fix return value of erc20 ibcv2 middleware to properly reflect application success and middleware failure. - [\#992](https://github.com/cosmos/evm/pull/992) Respect the provided `gasCap` in `CallEVMWithData` instead of always used the default cap. - [\#993](https://github.com/cosmos/evm/pull/993) Enforce `src_callback` contract address to match the packet sender for IBC acknowledgement and timeout callbacks to prevent arbitrary contract execution. +- Align precompile gas calculation with expected EVM gas semantics. + ## v0.6.0 diff --git a/contracts/solidity/precompiles/bank/testdata/BankCaller.sol b/contracts/solidity/precompiles/bank/testdata/BankCaller.sol index 7322fe118..f9ed0a7b0 100644 --- a/contracts/solidity/precompiles/bank/testdata/BankCaller.sol +++ b/contracts/solidity/precompiles/bank/testdata/BankCaller.sol @@ -16,4 +16,14 @@ contract BankCaller { function callSupplyOf(address erc20Address) external view returns (uint256) { return IBANK_CONTRACT.supplyOf(erc20Address); } + + // Calls totalSupply with explicit gas forwarding and measures the gas consumed + // by the inner call. Returns whether the call succeeded and the actual gas used. + function callTotalSupplyWithGas(uint256 gasForward) external view returns (bool success, uint256 innerGasUsed) { + uint256 gasBefore = gasleft(); + (success, ) = IBANK_PRECOMPILE_ADDRESS.staticcall{gas: gasForward}( + abi.encodeWithSelector(IBank.totalSupply.selector) + ); + innerGasUsed = gasBefore - gasleft(); + } } \ No newline at end of file diff --git a/precompiles/bank/testdata/BankCaller.json b/precompiles/bank/testdata/BankCaller.json index 049940cd2..ae0f8dfef 100644 --- a/precompiles/bank/testdata/BankCaller.json +++ b/precompiles/bank/testdata/BankCaller.json @@ -77,10 +77,34 @@ ], "stateMutability": "view", "type": "function" + }, + { + "inputs": [ + { + "internalType": "uint256", + "name": "gasForward", + "type": "uint256" + } + ], + "name": "callTotalSupplyWithGas", + "outputs": [ + { + "internalType": "bool", + "name": "success", + "type": "bool" + }, + { + "internalType": "uint256", + "name": "innerGasUsed", + "type": "uint256" + } + ], + "stateMutability": "view", + "type": "function" } ], - "bytecode": "0x6080806040523461001657610378908161001c8239f35b600080fdfe60806040908082526004918236101561001757600080fd5b600091823560e01c90816389129c681461016257508063acab2f94146100e95763bba60ca01461004657600080fd5b346100e557602092836003193601126100e15780356001600160a01b038116908190036100dd578251631890039360e21b81529182015283816024816108045afa9283156100d257809361009d575b505051908152f35b909192508382813d83116100cb575b6100b68183610234565b810103126100c8575051903880610095565b80fd5b503d6100ac565b8251903d90823e3d90fd5b8380fd5b8280fd5b5080fd5b508290346100e157826003193601126100e15782815180936318160ddd60e01b8252816108045afa918215610158578361012f9493610133575b505051918291826101e1565b0390f35b6101509293503d8091833e6101488183610234565b81019061026c565b908380610123565b81513d85823e3d90fd5b92939050346100dd5760203660031901126100dd5780356001600160a01b03811691908290036101dd576327e235e360e01b845283015282826024816108045afa918215610158578361012f94936101c057505051918291826101e1565b6101d59293503d8091833e6101488183610234565b903880610123565b8480fd5b60208082019080835283518092528060408094019401926000905b83821061020b57505050505090565b845180516001600160a01b031687528301518684015294850194938201936001909101906101fc565b90601f8019910116810190811067ffffffffffffffff82111761025657604052565b634e487b7160e01b600052604160045260246000fd5b6020808284031261032857815167ffffffffffffffff9283821161032857019083601f83011215610328578151838111610256576040938451956102b5848460051b0188610234565b828752838088019360061b86010194818611610328578401925b8584106102e0575050505050505090565b8684830312610328578651908782018281108582111761032d5788528451906001600160a01b0382168203610328578287928a945282870151838201528152019301926102cf565b600080fd5b60246000634e487b7160e01b81526041600452fdfea2646970667358221220069405aa45fc21b29f725237543db2b8600a62c69a04e7a4c44dce45d314303e64736f6c63430008140033", - "deployedBytecode": "0x60806040908082526004918236101561001757600080fd5b600091823560e01c90816389129c681461016257508063acab2f94146100e95763bba60ca01461004657600080fd5b346100e557602092836003193601126100e15780356001600160a01b038116908190036100dd578251631890039360e21b81529182015283816024816108045afa9283156100d257809361009d575b505051908152f35b909192508382813d83116100cb575b6100b68183610234565b810103126100c8575051903880610095565b80fd5b503d6100ac565b8251903d90823e3d90fd5b8380fd5b8280fd5b5080fd5b508290346100e157826003193601126100e15782815180936318160ddd60e01b8252816108045afa918215610158578361012f9493610133575b505051918291826101e1565b0390f35b6101509293503d8091833e6101488183610234565b81019061026c565b908380610123565b81513d85823e3d90fd5b92939050346100dd5760203660031901126100dd5780356001600160a01b03811691908290036101dd576327e235e360e01b845283015282826024816108045afa918215610158578361012f94936101c057505051918291826101e1565b6101d59293503d8091833e6101488183610234565b903880610123565b8480fd5b60208082019080835283518092528060408094019401926000905b83821061020b57505050505090565b845180516001600160a01b031687528301518684015294850194938201936001909101906101fc565b90601f8019910116810190811067ffffffffffffffff82111761025657604052565b634e487b7160e01b600052604160045260246000fd5b6020808284031261032857815167ffffffffffffffff9283821161032857019083601f83011215610328578151838111610256576040938451956102b5848460051b0188610234565b828752838088019360061b86010194818611610328578401925b8584106102e0575050505050505090565b8684830312610328578651908782018281108582111761032d5788528451906001600160a01b0382168203610328578287928a945282870151838201528152019301926102cf565b600080fd5b60246000634e487b7160e01b81526041600452fdfea2646970667358221220069405aa45fc21b29f725237543db2b8600a62c69a04e7a4c44dce45d314303e64736f6c63430008140033", + "bytecode": "0x608080604052346100165761045b908161001c8239f35b600080fdfe60806040908082526004908136101561001757600080fd5b600090813560e01c90816389129c681461024557508063acab2f94146101cd578063bba60ca01461012b5763d47052031461005157600080fd5b34610128576020366003190112610128575a90835192602084016318160ddd60e01b81528185528585019467ffffffffffffffff95818110878211176101155787525183918291906108048535fa933d1561010f573d9081116100fc578551906100c5601f8201601f191660200183610317565b81528260203d92013e5b5a83039283116100e9575050825191151582526020820152f35b634e487b7160e01b825260119052602490fd5b634e487b7160e01b835260418252602483fd5b506100cf565b634e487b7160e01b855260418452602485fd5b80fd5b5082346101c957602092836003193601126101c55780356001600160a01b038116908190036101c1578251631890039360e21b81529182015283816024816108045afa9283156101b6578093610184575b505051908152f35b909192508382813d83116101af575b61019d8183610317565b8101031261012857505190838061017c565b503d610193565b8251903d90823e3d90fd5b8380fd5b8280fd5b5080fd5b5091346101c557826003193601126101c55782815180936318160ddd60e01b8252816108045afa91821561023b57836102129493610216575b505051918291826102c4565b0390f35b6102339293503d8091833e61022b8183610317565b81019061034f565b903880610206565b81513d85823e3d90fd5b90508383346101c15760203660031901126101c15780356001600160a01b03811691908290036102c0576327e235e360e01b845283015282826024816108045afa91821561023b578361021294936102a357505051918291826102c4565b6102b89293503d8091833e61022b8183610317565b908380610206565b8480fd5b60208082019080835283518092528060408094019401926000905b8382106102ee57505050505090565b845180516001600160a01b031687528301518684015294850194938201936001909101906102df565b90601f8019910116810190811067ffffffffffffffff82111761033957604052565b634e487b7160e01b600052604160045260246000fd5b6020808284031261040b57815167ffffffffffffffff9283821161040b57019083601f8301121561040b57815183811161033957604093845195610398848460051b0188610317565b828752838088019360061b8601019481861161040b578401925b8584106103c3575050505050505090565b868483031261040b57865190878201828110858211176104105788528451906001600160a01b038216820361040b578287928a945282870151838201528152019301926103b2565b600080fd5b60246000634e487b7160e01b81526041600452fdfea26469706673582212206291c412c52d95f56ebd6bcace337eb3149a918c5ed8398ed4845c859755136a64736f6c63430008140033", + "deployedBytecode": "0x60806040908082526004908136101561001757600080fd5b600090813560e01c90816389129c681461024557508063acab2f94146101cd578063bba60ca01461012b5763d47052031461005157600080fd5b34610128576020366003190112610128575a90835192602084016318160ddd60e01b81528185528585019467ffffffffffffffff95818110878211176101155787525183918291906108048535fa933d1561010f573d9081116100fc578551906100c5601f8201601f191660200183610317565b81528260203d92013e5b5a83039283116100e9575050825191151582526020820152f35b634e487b7160e01b825260119052602490fd5b634e487b7160e01b835260418252602483fd5b506100cf565b634e487b7160e01b855260418452602485fd5b80fd5b5082346101c957602092836003193601126101c55780356001600160a01b038116908190036101c1578251631890039360e21b81529182015283816024816108045afa9283156101b6578093610184575b505051908152f35b909192508382813d83116101af575b61019d8183610317565b8101031261012857505190838061017c565b503d610193565b8251903d90823e3d90fd5b8380fd5b8280fd5b5080fd5b5091346101c557826003193601126101c55782815180936318160ddd60e01b8252816108045afa91821561023b57836102129493610216575b505051918291826102c4565b0390f35b6102339293503d8091833e61022b8183610317565b81019061034f565b903880610206565b81513d85823e3d90fd5b90508383346101c15760203660031901126101c15780356001600160a01b03811691908290036102c0576327e235e360e01b845283015282826024816108045afa91821561023b578361021294936102a357505051918291826102c4565b6102b89293503d8091833e61022b8183610317565b908380610206565b8480fd5b60208082019080835283518092528060408094019401926000905b8382106102ee57505050505090565b845180516001600160a01b031687528301518684015294850194938201936001909101906102df565b90601f8019910116810190811067ffffffffffffffff82111761033957604052565b634e487b7160e01b600052604160045260246000fd5b6020808284031261040b57815167ffffffffffffffff9283821161040b57019083601f8301121561040b57815183811161033957604093845195610398848460051b0188610317565b828752838088019360061b8601019481861161040b578401925b8584106103c3575050505050505090565b868483031261040b57865190878201828110858211176104105788528451906001600160a01b038216820361040b578287928a945282870151838201528152019301926103b2565b600080fd5b60246000634e487b7160e01b81526041600452fdfea26469706673582212206291c412c52d95f56ebd6bcace337eb3149a918c5ed8398ed4845c859755136a64736f6c63430008140033", "linkReferences": {}, "deployedLinkReferences": {} } diff --git a/precompiles/bank/testdata/BankCaller.sol b/precompiles/bank/testdata/BankCaller.sol index 7322fe118..f9ed0a7b0 100644 --- a/precompiles/bank/testdata/BankCaller.sol +++ b/precompiles/bank/testdata/BankCaller.sol @@ -16,4 +16,14 @@ contract BankCaller { function callSupplyOf(address erc20Address) external view returns (uint256) { return IBANK_CONTRACT.supplyOf(erc20Address); } + + // Calls totalSupply with explicit gas forwarding and measures the gas consumed + // by the inner call. Returns whether the call succeeded and the actual gas used. + function callTotalSupplyWithGas(uint256 gasForward) external view returns (bool success, uint256 innerGasUsed) { + uint256 gasBefore = gasleft(); + (success, ) = IBANK_PRECOMPILE_ADDRESS.staticcall{gas: gasForward}( + abi.encodeWithSelector(IBank.totalSupply.selector) + ); + innerGasUsed = gasBefore - gasleft(); + } } \ No newline at end of file diff --git a/precompiles/common/precompile.go b/precompiles/common/precompile.go index 242d525b3..488ef9379 100644 --- a/precompiles/common/precompile.go +++ b/precompiles/common/precompile.go @@ -48,6 +48,9 @@ func (p Precompile) RequiredGas(input []byte, isTransaction bool) uint64 { func (p Precompile) RunNativeAction(evm *vm.EVM, contract *vm.Contract, action NativeAction) ([]byte, error) { bz, err := p.runNativeAction(evm, contract, action) if err != nil { + if errors.Is(err, vm.ErrOutOfGas) { + return nil, vm.ErrOutOfGas + } return ReturnRevertError(evm, err) } @@ -84,14 +87,14 @@ func (p Precompile) runNativeAction(evm *vm.EVM, contract *vm.Contract, action N initialGas := ctx.GasMeter().GasConsumed() - defer HandleGasError(ctx, contract, initialGas, &err)() - // set the default SDK gas configuration to track gas usage // we are changing the gas meter type, so it panics gracefully when out of gas ctx = ctx.WithGasMeter(storetypes.NewGasMeter(contract.Gas)). WithKVGasConfig(p.KvGasConfig). WithTransientKVGasConfig(p.TransientKVGasConfig) + defer HandleGasError(ctx, contract, initialGas, &err)() + // we need to consume the gas that was already used by the EVM ctx.GasMeter().ConsumeGas(initialGas, "creating a new gas meter") diff --git a/tests/integration/precompiles/bank/test_integration.go b/tests/integration/precompiles/bank/test_integration.go index c79f42c10..0b98f36da 100644 --- a/tests/integration/precompiles/bank/test_integration.go +++ b/tests/integration/precompiles/bank/test_integration.go @@ -1,6 +1,7 @@ package bank import ( + "fmt" "math/big" "testing" @@ -25,6 +26,7 @@ import ( evmtypes "github.com/cosmos/evm/x/vm/types" "cosmossdk.io/math" + storetypes "cosmossdk.io/store/types" sdk "github.com/cosmos/cosmos-sdk/types" minttypes "github.com/cosmos/cosmos-sdk/x/mint/types" @@ -255,6 +257,67 @@ func TestIntegrationSuite(t *testing.T, create network.CreateEvmApp, options ... Expect(balances[0].Amount.String()).To(Equal(cosmosEVMTotalSupply.String())) Expect(balances[1].Amount.String()).To(Equal(xmplTotalSupply.String())) }) + + It("should properly handle OOG in precompile and consume all gas", func() { + numDenoms := 5000 + // Mint denoms to make TotalSupply expensive enough to OOG. + ctx := is.network.GetContext() + for i := 0; i < numDenoms; i++ { + denom := fmt.Sprintf("token%d", i) + err = is.network.App.GetBankKeeper().MintCoins( + ctx, minttypes.ModuleName, + sdk.Coins{{Denom: denom, Amount: math.NewInt(1e18)}}, + ) + Expect(err).ToNot(HaveOccurred(), "failed to mint coin %s", denom) + } + // Commit keeper changes directly to state. + store := is.network.GetContext().MultiStore() + cms, ok := store.(storetypes.CacheMultiStore) + Expect(ok).To(BeTrue()) + cms.Write() + + Expect(is.network.NextBlock()).ToNot(HaveOccurred(), "failed to advance block") + + // Use callTotalSupplyWithGas to measure inner call gas consumption. + // Forward enough gas for the precompile to OOG iterating 5000+ denoms. + gasForward := big.NewInt(9_000_000) + + txArgs := evmtypes.EvmTxArgs{ + To: &bankCallerContractAddr, + GasLimit: 20_000_000, + } + callArgs := testutiltypes.CallArgs{ + ContractABI: bankCallerContract.ABI, + MethodName: "callTotalSupplyWithGas", + Args: []interface{}{gasForward}, + } + + res, execErr := is.factory.ExecuteContractCall(sender.Priv, txArgs, callArgs) + Expect(execErr).ToNot(HaveOccurred(), "failed to execute callTotalSupplyWithGas") + + ethRes, decErr := evmtypes.DecodeTxResponse(res.Data) + Expect(decErr).ToNot(HaveOccurred(), "failed to decode eth tx response") + Expect(ethRes.VmError).To(BeEmpty(), "outer call should not revert") + + // Unpack: (bool success, uint256 innerGasUsed) + out, unpackErr := bankCallerContract.ABI.Unpack("callTotalSupplyWithGas", ethRes.Ret) + Expect(unpackErr).ToNot(HaveOccurred(), "failed to unpack result") + + innerSuccess := out[0].(bool) + innerGasUsed := out[1].(*big.Int) + + fmt.Printf("\nInner call: success=%v, innerGasUsed=%s, gasForwarded=%s, txGasUsed=%d\n", + innerSuccess, innerGasUsed.String(), gasForward.String(), ethRes.GasUsed) + + // The inner call should have failed (OOG with 5000 denoms). + Expect(innerSuccess).To(BeFalse(), "inner call should OOG with insufficient gas for 5000 denoms") + + // The inner call should have consumed all (or nearly all) forwarded gas. + // With correct gas accounting, innerGasUsed ≈ gasForward. + // With the bug, innerGasUsed would be ~3k (only EVM overhead, no precompile charge). + Expect(innerGasUsed.Uint64()).To(BeNumerically(">=", gasForward.Uint64()*90/100), + "inner OOG call should consume all forwarded gas, got %s out of %s", innerGasUsed.String(), gasForward.String()) + }) }) Context("supplyOf query", func() { diff --git a/tests/integration/precompiles/staking/test_staking.go b/tests/integration/precompiles/staking/test_staking.go index de521636c..5efe96569 100644 --- a/tests/integration/precompiles/staking/test_staking.go +++ b/tests/integration/precompiles/staking/test_staking.go @@ -133,6 +133,7 @@ func (s *PrecompileTestSuite) TestRun() { gas uint64 readOnly bool expPass bool + expRevert bool // true if error returns ABI-encoded revert reason bytes (not OOG) errContains string }{ { @@ -150,6 +151,7 @@ func (s *PrecompileTestSuite) TestRun() { 8000, false, false, + false, "out of gas", }, { @@ -167,6 +169,7 @@ func (s *PrecompileTestSuite) TestRun() { 1000000, false, true, + false, "", }, { @@ -184,6 +187,7 @@ func (s *PrecompileTestSuite) TestRun() { 1000000, false, true, + false, "", }, { @@ -202,6 +206,7 @@ func (s *PrecompileTestSuite) TestRun() { 1000000, false, true, + false, "failed to redelegate tokens", }, { @@ -242,6 +247,7 @@ func (s *PrecompileTestSuite) TestRun() { 1000000, false, true, + false, "", }, { @@ -258,6 +264,7 @@ func (s *PrecompileTestSuite) TestRun() { 1000000, false, true, + false, "", }, { @@ -276,6 +283,7 @@ func (s *PrecompileTestSuite) TestRun() { 1000000, false, true, + false, "", }, { @@ -314,6 +322,7 @@ func (s *PrecompileTestSuite) TestRun() { 1000000, false, true, + false, "", }, { @@ -330,6 +339,7 @@ func (s *PrecompileTestSuite) TestRun() { 1000000, true, true, + false, "", }, { @@ -368,6 +378,7 @@ func (s *PrecompileTestSuite) TestRun() { 1000000, true, true, + false, "", }, { @@ -385,6 +396,7 @@ func (s *PrecompileTestSuite) TestRun() { 21559, // use enough gas to avoid out of gas error true, false, + true, "write protection", }, { @@ -395,6 +407,7 @@ func (s *PrecompileTestSuite) TestRun() { 21559, // use enough gas to avoid out of gas error false, false, + true, "no method with id", }, } @@ -455,16 +468,20 @@ func (s *PrecompileTestSuite) TestRun() { bz, err := s.precompile.Run(evm, contract, tc.readOnly) // Check results - if tc.expPass { + switch { + case tc.expPass: s.Require().NoError(err, "expected no error when running the precompile") s.Require().NotNil(bz, "expected returned bytes not to be nil") - } else { - s.Require().Error(err, "expected error to be returned when running the precompile") - s.Require().NotNil(bz, "expected returned bytes to be nil") - execRevertErr := evmtypes.NewExecErrorWithReason(bz) - s.Require().ErrorContains(execRevertErr, tc.errContains) + case tc.expRevert: + s.Require().ErrorIs(err, vm.ErrExecutionReverted) + s.Require().NotNil(bz, "expected revert reason bytes") + revertErr := evmtypes.NewExecErrorWithReason(bz) + s.Require().ErrorContains(revertErr, tc.errContains) + default: + s.Require().ErrorIs(err, vm.ErrOutOfGas) + s.Require().Nil(bz, "expected nil bytes on out of gas") consumed := ctx.GasMeter().GasConsumed() - s.Require().Greater(consumed, uint64(0), "expected gas to be consumed") + s.Require().LessOrEqual(tc.gas, consumed, "expected all gas to be consumed on OOG") } }) }