fix: align gas estimation logic with go-ethereum v1.16.3 (#766)

* WIP: add estimateGas test file

* fix: align gas estimation binary search with geth

* fix(x/vm): fix EstimateGasInternal

- let EstimateGasInternal call ApplyMessageWithConfig with internal = true
- fix lowest possible gas

* apply EIP-7623: Data-heavy transactions pay the floor gas

* fix: state overrides handling

* fix test code for gas estimation

* chore: update CHANGELOG.md

* test(x/vm): fix test code

* chore: resolve codeQL warnings

* fix(x/vm): StateDB.Commit

* test(jsonrpc): enhance eth_estimateGas test case

* chore: remove temporary file for test

* chore: apply code review

---------

Co-authored-by: Alex | Cosmos Labs <[email protected]>

Author: cloudgray

CHANGELOG.md

@@ -13,6 +13,7 @@
 ### BUG FIXES
 
 - [\#748](https://github.com/cosmos/evm/pull/748) Fix DynamicFeeChecker in Cosmos ante handler to respect NoBaseFee feemarkets' parameter.
+- [\#766](https://github.com/cosmos/evm/pull/766) Align gas estimation logic with go-ethereum v1.16.3
 - [\#769](https://github.com/cosmos/evm/pull/769) Fix erc20 ibc middleware to not to validate sender address format.
 - [\#756](https://github.com/cosmos/evm/pull/756) Fix error message typo in NewMsgCancelProposal.
 - [\#772](https://github.com/cosmos/evm/pull/772) Avoid panic on close if evm mempool not used.

rpc/types/types.go

@@ -127,7 +127,7 @@ func (diff *StateOverride) Apply(db *statedb.StateDB, precompiles vm.Precompiled
 		// Apply state diff into specified accounts.
 		if account.StateDiff != nil {
 			for key, value := range *account.StateDiff {
-				db.SetState(addr, key, value)
+				db.SetStateOverride(addr, key, value)
 			}
 		}
 	}

tests/integration/x/vm/test_grpc_query.go

@@ -574,7 +574,7 @@ func (s *KeeperTestSuite) TestEstimateGas() {
 				return types.TransactionArgs{}
 			},
 			true,
-			ethparams.TxGasContractCreation,
+			53793,
 			false,
 			config.DefaultGasCap,
 		},
@@ -699,7 +699,7 @@ func (s *KeeperTestSuite) TestEstimateGas() {
 				}
 			},
 			true,
-			1187108,
+			1197697,
 			false,
 			config.DefaultGasCap,
 		},
@@ -727,7 +727,7 @@ func (s *KeeperTestSuite) TestEstimateGas() {
 				}
 			},
 			true,
-			51880,
+			52669,
 			false,
 			config.DefaultGasCap,
 		},
@@ -809,7 +809,7 @@ func (s *KeeperTestSuite) TestEstimateGas() {
 				}
 			},
 			true,
-			1187108,
+			1197697,
 			true,
 			config.DefaultGasCap,
 		},
@@ -838,7 +838,7 @@ func (s *KeeperTestSuite) TestEstimateGas() {
 				}
 			},
 			true,
-			51880,
+			52669,
 			true,
 			config.DefaultGasCap,
 		},
@@ -1067,7 +1067,7 @@ func (s *KeeperTestSuite) TestEstimateGasWithStateOverrides() {
 				return string(bz)
 			},
 			true,
-			49140,
+			52114,
 			false,
 			config.DefaultGasCap,
 		},

tests/jsonrpc/simulator/namespaces/eth.go

@@ -1532,6 +1532,16 @@ func estimateNativeTransferGas(rCtx *types.RPCContext) (string, error) {
 		return "", err
 	}
 
+	if rCtx.EnableComparison && rCtx.Geth != nil && rCtx.Geth.Client != nil {
+		gethGasLimit, err := rCtx.Geth.EstimateGas(context.Background(), callMsg)
+		if err != nil {
+			return "", fmt.Errorf("geth estimateGas failed: %w", err)
+		}
+		if gasLimit != gethGasLimit {
+			return "", fmt.Errorf("estimateGas mismatch (native): evmd=%d geth=%d", gasLimit, gethGasLimit)
+		}
+	}
+
 	return fmt.Sprintf("0x%x", gasLimit), nil
 }
 
@@ -1567,6 +1577,16 @@ func estimateERC20TransferGas(rCtx *types.RPCContext) (string, bool, error) {
 		return "", false, err
 	}
 
+	if rCtx.EnableComparison && rCtx.Geth != nil && rCtx.Geth.Client != nil {
+		gethGas, err := rCtx.Geth.EstimateGas(context.Background(), erc20Call)
+		if err != nil {
+			return "", false, fmt.Errorf("geth estimateGas failed: %w", err)
+		}
+		if gas != gethGas {
+			return "", false, fmt.Errorf("estimateGas mismatch (erc20 transfer): evmd=%d geth=%d", gas, gethGas)
+		}
+	}
+
 	return fmt.Sprintf("0x%x", gas), false, nil
 }
 
@@ -1631,6 +1651,16 @@ func estimateERC20OverrideGas(rCtx *types.RPCContext) (string, bool, error) {
 		return "", false, err
 	}
 
+	if rCtx.EnableComparison && rCtx.Geth != nil && rCtx.Geth.Client != nil {
+		var gethGas hexutil.Uint64
+		if err := rCtx.Geth.RPCClient().Call(&gethGas, string(MethodNameEthEstimateGas), overrideParams, "latest", overrideState); err != nil {
+			return "", false, fmt.Errorf("geth estimateGas failed: %w", err)
+		}
+		if uint64(overrideGas) != uint64(gethGas) {
+			return "", false, fmt.Errorf("estimateGas mismatch (erc20 override): evmd=%d geth=%d", uint64(overrideGas), uint64(gethGas))
+		}
+	}
+
 	return fmt.Sprintf("0x%x", uint64(overrideGas)), false, nil
 }
 

x/vm/keeper/grpc_query.go

@@ -308,7 +308,7 @@ func (k Keeper) EstimateGasInternal(c context.Context, req *types.EthCallRequest
 
 	// Binary search the gas requirement, as it may be higher than the amount used
 	var (
-		lo     = ethparams.TxGas - 1
+		lo     uint64
 		hi     uint64
 		gasCap uint64
 	)
@@ -410,8 +410,9 @@ func (k Keeper) EstimateGasInternal(c context.Context, req *types.EthCallRequest
 			// resetting the gasMeter after increasing the sequence to have an accurate gas estimation on EVM extensions transactions
 			tmpCtx = buildTraceCtx(tmpCtx, msg.GasLimit)
 		}
-		// pass false to not commit StateDB
-		rsp, err = k.ApplyMessageWithConfig(tmpCtx, *msg, nil, false, cfg, txConfig, false, overrides)
+		// pass "commit" as false to avoid committing StateDB
+		// pass "internal" as true to avoid applying feemarketParams.MinGasMultiplier for gas estimation.
+		rsp, err = k.ApplyMessageWithConfig(tmpCtx, *msg, nil, false, cfg, txConfig, true, overrides)
 		if err != nil {
 			if errors.Is(err, core.ErrIntrinsicGas) || errors.Is(err, core.ErrFloorDataGas) {
 				return true, nil, nil // Special case, raise gas limit
@@ -461,6 +462,12 @@ func (k Keeper) EstimateGasInternal(c context.Context, req *types.EthCallRequest
 		// If no larger allowance is available, fail fast
 		return nil, fmt.Errorf("gas required exceeds allowance (%d)", hi)
 	}
+	// For almost any transaction, the gas consumed by the unconstrained execution
+	// above lower-bounds the gas limit required for it to succeed. One exception
+	// is those that explicitly check gas remaining in order to execute within a
+	// given limit, but we probably don't want to return the lowest possible gas
+	// limit for these cases anyway.
+	lo = result.GasUsed - 1
 
 	// There's a fairly high chance for the transaction to execute successfully
 	// with gasLimit set to the first execution's usedGas + gasRefund. Explicitly

x/vm/keeper/state_transition.go

@@ -393,8 +393,9 @@ func (k *Keeper) ApplyMessageWithConfig(
 	overrides *rpctypes.StateOverride,
 ) (*types.MsgEthereumTxResponse, error) {
 	var (
-		ret   []byte // return bytes from evm execution
-		vmErr error  // vm errors do not effect consensus and are therefore not assigned to err
+		ret          []byte // return bytes from evm execution
+		vmErr        error  // vm errors do not effect consensus and are therefore not assigned to err
+		floorDataGas uint64
 	)
 
 	stateDB := statedb.New(ctx, k, txConfig)
@@ -443,7 +444,7 @@ func (k *Keeper) ApplyMessageWithConfig(
 		return nil, errorsmod.Wrap(core.ErrIntrinsicGas, "apply message")
 	}
 	if rules.IsPrague {
-		floorDataGas, err := core.FloorDataGas(msg.Data)
+		floorDataGas, err = core.FloorDataGas(msg.Data)
 		if err != nil {
 			return nil, err
 		}
@@ -498,10 +499,6 @@ func (k *Keeper) ApplyMessageWithConfig(
 		refundQuotient = params.RefundQuotientEIP3529
 	}
 
-	if internal {
-		refundQuotient = 1 // full refund on internal calls
-	}
-
 	// calculate gas refund
 	if msg.GasLimit < leftoverGas {
 		return nil, errorsmod.Wrap(types.ErrGasOverflow, "apply message")
@@ -513,6 +510,20 @@ func (k *Keeper) ApplyMessageWithConfig(
 	// update leftoverGas and temporaryGasUsed with refund amount
 	leftoverGas += refund
 	temporaryGasUsed := maxUsedGas - refund
+	if rules.IsPrague {
+		// After EIP-7623: Data-heavy transactions pay the floor gas.
+		if temporaryGasUsed < floorDataGas {
+			prev := leftoverGas
+			leftoverGas = msg.GasLimit - floorDataGas
+			temporaryGasUsed = floorDataGas
+			if vmCfg.Tracer != nil && vmCfg.Tracer.OnGasChange != nil {
+				vmCfg.Tracer.OnGasChange(prev, leftoverGas, tracing.GasChangeTxDataFloor)
+			}
+		}
+		if maxUsedGas < floorDataGas {
+			maxUsedGas = floorDataGas
+		}
+	}
 
 	// EVM execution error needs to be available for the JSON-RPC client
 	var vmError string

x/vm/statedb/journal.go

@@ -51,6 +51,14 @@ func newJournal() *journal {
 	}
 }
 
+// reset clears the journal so it can be reused without reallocating.
+func (j *journal) reset() {
+	j.entries = j.entries[:0]
+	for _, k := range j.sortedDirties() {
+		delete(j.dirties, k)
+	}
+}
+
 // sortedDirties sort the dirty addresses for deterministic iteration
 func (j *journal) sortedDirties() []common.Address {
 	keys := make([]common.Address, 0, len(j.dirties))
@@ -117,8 +125,10 @@ type (
 		prev    uint64
 	}
 	storageChange struct {
-		account       *common.Address
-		key, prevalue common.Hash
+		account   *common.Address
+		key       common.Hash
+		preValue  common.Hash
+		origValue common.Hash
 	}
 	transientStorageChange struct {
 		account       *common.Address
@@ -240,7 +250,7 @@ func (ch codeChange) Dirtied() *common.Address {
 }
 
 func (ch storageChange) Revert(s *StateDB) {
-	s.getStateObject(*ch.account).setState(ch.key, ch.prevalue)
+	s.getStateObject(*ch.account).setState(ch.key, ch.preValue, ch.origValue)
 }
 
 func (ch storageChange) Dirtied() *common.Address {

x/vm/statedb/state_object.go

@@ -251,13 +251,15 @@ func (s *stateObject) SetState(key common.Hash, value common.Hash) common.Hash {
 	if prev == value {
 		return prev
 	}
+	origin := s.GetCommittedState(key)
 	// New value is different, update and journal the change
 	s.db.journal.append(storageChange{
-		account:  &s.address,
-		key:      key,
-		prevalue: prev,
+		account:   &s.address,
+		key:       key,
+		preValue:  prev,
+		origValue: origin,
 	})
-	s.setState(key, value)
+	s.setState(key, value, origin)
 	return prev
 }
 
@@ -270,6 +272,26 @@ func (s *stateObject) SetStorage(storage Storage) {
 	s.dirtyStorage = make(Storage)
 }
 
-func (s *stateObject) setState(key, value common.Hash) {
+// setState updates a value in account dirty storage. The dirtiness will be
+// removed if the value being set equals to the original value.
+func (s *stateObject) setState(key, value, origin common.Hash) {
+	// Storage slot is set back to its original value, undo the dirty marker
+	if value == origin {
+		delete(s.dirtyStorage, key)
+		return
+	}
 	s.dirtyStorage[key] = value
 }
+
+// SetStateOverride installs the provided storage value as part of the base state used for
+// simulations. Subsequent reads treat the slot as if it were committed on-chain.
+func (s *stateObject) SetStateOverride(key, value common.Hash) {
+	if s.originStorage == nil {
+		s.originStorage = make(Storage)
+	}
+	s.originStorage[key] = value
+	delete(s.dirtyStorage, key)
+	if s.overrideStorage != nil {
+		s.overrideStorage[key] = value
+	}
+}

x/vm/statedb/statedb.go

@@ -142,6 +142,8 @@ func (s *StateDB) Finalise(deleteEmptyObjects bool) {
 			delete(s.stateObjects, obj.address)
 		}
 	}
+	// Invalidate journal because reverting across transactions is not allowed.
+	s.clearJournalAndRefund()
 }
 
 // New creates a new state from a given trie.
@@ -331,6 +333,25 @@ func (s *StateDB) GetStateAndCommittedState(addr common.Address, hash common.Has
 	return common.Hash{}, common.Hash{}
 }
 
+// SetStateOverride installs the provided storage value as part of the base state used by simulations.
+func (s *StateDB) SetStateOverride(addr common.Address, key, value common.Hash) {
+	stateObject := s.getOrNewStateObject(addr)
+	if stateObject != nil {
+		stateObject.SetStateOverride(key, value)
+	}
+}
+
+func (s *StateDB) clearJournalAndRefund() {
+	if s.journal == nil {
+		s.journal = newJournal()
+	} else {
+		s.journal.reset()
+	}
+	s.validRevisions = nil
+	s.nextRevisionID = 0
+	s.refund = 0
+}
+
 // GetRefund returns the current value of the refund counter.
 func (s *StateDB) GetRefund() uint64 {
 	return s.refund
@@ -697,19 +718,19 @@ func (s *StateDB) Commit() error {
 	if s.writeCache != nil {
 		s.writeCache()
 	}
-	return s.commitWithCtx(s.ctx)
+	return s.commitWithCtx(s.ctx, false)
 }
 
 // CommitWithCacheCtx writes the dirty states to keeper using the cacheCtx.
 // This function is used before any precompile call to make sure the cacheCtx
 // is updated with the latest changes within the tx (StateDB's journal entries).
 func (s *StateDB) CommitWithCacheCtx() error {
-	return s.commitWithCtx(s.cacheCtx)
+	return s.commitWithCtx(s.cacheCtx, true)
 }
 
 // commitWithCtx writes the dirty states to keeper
 // using the provided context
-func (s *StateDB) commitWithCtx(ctx sdk.Context) error {
+func (s *StateDB) commitWithCtx(ctx sdk.Context, keepDirty bool) error {
 	for _, addr := range s.journal.sortedDirties() {
 		obj := s.stateObjects[addr]
 		if obj.selfDestructed {
@@ -735,6 +756,32 @@ func (s *StateDB) commitWithCtx(ctx sdk.Context) error {
 				} else {
 					s.keeper.SetState(ctx, obj.Address(), key, valueBytes)
 				}
+
+				// Track the persisted value as the new baseline so later writes compare
+				// against the most recently flushed state. In go-ethereum the same happens
+				// during commitStorage: originStorage follows the latest flush and act as
+				// the reference for future dirty detection. The actual revert path still
+				// consults the journal's origvalue, so keeping this cache in sync during
+				// cacheCtx flushes is safe even if a precompile subsequently reverts.
+				if obj.originStorage == nil {
+					obj.originStorage = make(Storage)
+				}
+				if len(valueBytes) == 0 {
+					delete(obj.originStorage, key)
+				} else {
+					obj.originStorage[key] = obj.dirtyStorage[key]
+				}
+				// Only the final Commit against the root context should clear dirtyStorage.
+				// During precompile execution we pass keepDirty=true so that the slots remain
+				// marked dirty after cacheCtx flushes, letting the outer transaction still
+				// persist those writes (or revert them via the journal) once execution finishes.
+				//
+				// This is essential after SetState started syncing originStorage: the cacheCtx
+				// flush must leave the dirty slots intact so the final Commit() can push the
+				// same updates into the keeper context.
+				if !keepDirty {
+					delete(obj.dirtyStorage, key)
+				}
 			}
 		}
 	}

x/vm/statedb/statedb_test.go

@@ -295,9 +295,7 @@ func (suite *StateDBTestSuite) TestState() {
 		{"set state even if same as original value (due to possible reverts within precompile calls)", func(db *statedb.StateDB) {
 			db.SetState(address, key1, value1)
 			db.SetState(address, key1, common.Hash{})
-		}, statedb.Storage{
-			key1: common.Hash{},
-		}},
+		}, statedb.Storage{}},
 		{"set state", func(db *statedb.StateDB) {
 			// check empty initial state
 			suite.Require().Equal(common.Hash{}, db.GetState(address, key1))