test: expand stateful tests to test all stateful RPCs#5283
test: expand stateful tests to test all stateful RPCs#5283JonathanOppenheimer wants to merge 5 commits intomasterfrom
Conversation
JonathanOppenheimer
requested review from
ARR4N and
StephenButtolph
as code owners
| assert.Equal(t, uint256.NewInt(val).PaddedBytes(32), got) | ||
| blockNum := big.NewInt(int64(tt.num)) | ||
|
|
||
| t.Run("eth_call", func(t *testing.T) { |
There was a problem hiding this comment.
| // - eth_getHeaderByNumber | ||
| // | ||
| // Undocumented APIs: | ||
| // - eth_getBlockReceipts |
There was a problem hiding this comment.
This RPC is actually already supported/tested, but it wasn't included in the list. When going through the list to add the ones for this PR, I thought I'd include it.
| } | ||
|
|
||
| got, err := sut.CallContract(ctx, msg, big.NewInt(int64(tt.num))) | ||
| t.Logf("%T.CallContract(%+v, %d)", sut.Client, msg, tt.num) // avoids having to repeat in failure messages |
There was a problem hiding this comment.
The test name hierarchy now provides this information
There was a problem hiding this comment.
Pull request overview
Expands SAEVM RPC stateful testing to cover additional state-query JSON-RPC methods, ensuring they behave correctly across both in-memory and on-disk block lookup paths (issue #5244).
Changes:
- Renamed/expanded the existing stateful RPC test into
TestStatefulRPCsand added coverage foreth_getBalance,eth_getCode,eth_getStorageAt,eth_getProof,eth_estimateGas, andeth_createAccessList. - Added
gethclientusage to exercise geth-specific methods (eth_getProof,eth_createAccessList) via client helpers. - Updated RPC server API list comments to reflect newly tested/available methods.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
vms/saevm/sae/rpc_stateful_test.go |
Expands the stateful RPC test to validate more state-query RPC methods across latest vs historical block paths. |
vms/saevm/sae/rpc/server.go |
Updates the inline documentation list of registered eth_* APIs (comment-only change). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| common.LeftPadBytes(recv.Bytes(), 32), | ||
| common.Hash{}.Bytes(), |
There was a problem hiding this comment.
nit/question: should we actually use abi.encode from abi package?
There was a problem hiding this comment.
I don't know how much of an improvement that would be -- I think manually encoding is fine. There's a lot of ABI setup required, and the goal is test the RPCs. If you feel strongly I can change it.
Why this should be merged
This PR add RPC-level tests for 6 state query methods:
eth_getBalance,eth_getCode,eth_getStorageAt,eth_getProof,eth_estimateGas, andeth_createAccessList. I decided it would be cleanest if all of these RPC's just inhabited the same test, which I renamed toTestStatefulRPCs. This extension allows us to ensure each method is tested against both in-memory and on-disk block paths.Closes #5244