internal/ethapi: add transient storage override support for eth_call

[hero5512]

this pr implements #32671, adding an override for the transient storage function in eth_call.

[lightclient]

@s1na do these types of overrides need to be represented in rpc specs?

[lightclient]

@hero5512 this is still a draft - is that intentional?

[hero5512]

@hero5512 this is still a draft - is that intentional?

Yes, there are still some issues with this PR.

[hero5512]

@s1na This pr is ready to review. Please take a look.

[s1na]

@s1na do these types of overrides need to be represented in rpc specs?

@lightclient The good: they are all standardized as part of eth_simulateV1

The bad: not specified in the specs for eth_call and eth_estimateGas

The ugly: I think only geth and reth support them in eth_call and eth_estimateGas

[s1na]

I don't like adding transient storage to the state overrides. They are different. State overrides can be for a longer context than a transaction, e.g. in eth_simulateV1 you do a state override per block. I'd add it as a new parameter.

[hero5512]

I don't like adding transient storage to the state overrides. They are different. State overrides can be for a longer context than a transaction, e.g. in eth_simulateV1 you do a state override per block. I'd add it as a new parameter.

okay. I've refactored the implementation to separate transient storage overrides into a seperate parameter.

[rv64m]

🔍 Code Review Summary

The review covers the implementation of transient storage overrides for eth_call across JSON-RPC and GraphQL APIs. The changes are well-structured and include essential test coverage. Key findings focus on improving robustness by handling edge cases in state application, enhancing test completeness, and ensuring API consistency. Overall, the implementation is solid, and the suggestions aim to refine it for production readiness.

📋 Reviewed Changes

• core/state/statedb.go: Added SetTransientStorage to the StateDB object to apply overrides.
• ethclient/gethclient/gethclient.go: Updated the Go client's CallContract to support transient storage overrides.
• graphql/graphql.go: Exposed transient storage overrides in the GraphQL schema and resolver.
• internal/ethapi/api.go: Integrated the application of transient storage overrides within the eth_call logic.
• internal/ethapi/api_test.go: Added end-to-end tests for eth_call utilizing transient storage overrides.
• internal/ethapi/override/override.go: Extended the Account override struct to include a TransientStorage map.
• internal/ethapi/override/override_test.go: Added unit tests for applying transient storage overrides to the state.

📊 Architecture & Flow Diagrams

Diagram 1

graph TD
    subgraph "User Request"
        A["eth_call with TransientStorageOverride"]
    end
    subgraph "Geth Node"
        B[ethapi.API] --> C{"Parse Overrides"}
        C --> D["Create Ephemeral StateDB"]
        D --> E[statedb.SetTransientStorage]
        E --> F["EVM Execution"]
        F --> G["Return Call Result"]
    end
    A --> B

Loading

🤔 Review Quality Assessment

The review provides a comprehensive analysis of the transient storage override feature, examining its implementation from the state layer up to the API and client libraries. The focus was on correctness, robustness against edge cases, test coverage, and API consistency. Confidence in the review is high due to the localized nature of the changes and the ability to verify behavior through the newly added tests. No significant gaps were identified in the analysis, which assumes the correctness of the underlying EVM implementation of TLOAD and TSTORE.

💡 Suggestions Summary

• Inline Comments: 0 suggestions on modified code lines
• General Suggestions: 3 suggestions for overall improvements

📝 Additional Suggestions

The following suggestions apply to code outside the current PR diff:

1. 🔴 In override.go, the Apply function iterates over the TransientStorage map to set state. However, if the account for which the override is being applied does not exist in the state, s.GetOrNewStateObject(addr) will create it, but the loop continues. It's more robust and explicit to ensure the account object exists before iterating over its transient storage. This also prevents potential nil panics if the state object creation were to fail, although unlikely with GetOrNewStateObject.

File: internal/ethapi/override/override.go (Line 45)
Confidence: 90%

if account.TransientStorage != nil {
		object := s.GetOrNewStateObject(addr)
		for key, value := range *account.TransientStorage {
			object.SetTransientState(key, value)
		}
	}

2. 🔴 The test for transient storage overrides in api_test.go is good, but it could be more comprehensive. The current test should be expanded to verify that an override can successfully overwrite an existing transient storage slot set earlier in the same call. This ensures that TSTORE and TLOAD interactions with the overridden state behave as expected within a single transaction context.

File: internal/ethapi/api_test.go (Line 2855)
Confidence: 85%

func TestTransientStorageOverride(t *testing.T) {
	// This is a simplified example. The actual test code would be more complex.
	service, _, _, _, contractAddr := setupCallTest(t, tstoreABI, tstoreCode)

	key := common.HexToHash("0x123")
	value := common.HexToHash("0xabc")
	newValue := common.HexToHash("0xdef")

	// TSTORE key to value, then TSTORE key to newValue, then TLOAD key.
	// The final TLOAD should return newValue.
	data, _ := tstoreABI.Pack("doStuffAndReturn", key, value, newValue)

	override := map[common.Address]ethapi.Account{
		*contractAddr: {
			TransientStorage: &map[common.Hash]common.Hash{
				// Pre-seed a value to ensure overrides work even if the slot is not empty.
				key: common.HexToHash("0xdeadbeef"), 
			},
		},
	}

	args := &ethapi.CallArgs{
		To:              contractAddr,
		Data:            data,
		StateOverride:   override,
	}

	res, err := service.Call(context.Background(), *args, rpc.BlockNumberOrHashWithNumber(rpc.LatestBlockNumber), nil)
	if err != nil {
		t.Fatalf("call failed: %v", err)
	}
	got := common.BytesToHash(res)
	if got != newValue {
		t.Errorf("expected 0x%x, got 0x%x", newValue, got)
	}
}

3. 🔴 The newly added TransientStorage field in override.go lacks a comment explaining its purpose and structure. Adding a clear comment improves code documentation and maintainability for future developers.

File: internal/ethapi/override/override.go (Line 20)
Confidence: 95%

	// TransientStorage is a mapping of transient storage keys to values. When
	// specified, the state of these storage slots is overridden before execution.
	TransientStorage *map[common.Hash]common.Hash `json:"transientStorage,omitempty"`