Skip to content

test: expand stateful tests to test all stateful RPCs #259

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
JonathanOppenheimer wants to merge 5 commits into from
+87 −12
Closed

test: expand stateful tests to test all stateful RPCs #259

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
54ee789
test: expand stateful tests to test all stateful RPCs
JonathanOppenheimer Mar 23, 2026
66fa130
Merge branch 'main' into JonathanOppenheimer/stateful-rpc-tests
JonathanOppenheimer Mar 24, 2026
47755d0
Merge branch 'main' into JonathanOppenheimer/stateful-rpc-tests
JonathanOppenheimer Mar 25, 2026
5dfbf16
Merge branch 'main' into JonathanOppenheimer/stateful-rpc-tests
JonathanOppenheimer Mar 26, 2026
5241902
Merge branch 'main' into JonathanOppenheimer/stateful-rpc-tests
JonathanOppenheimer Apr 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions sae/rpc/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,18 +57,27 @@ func (b *backend) server(filter *filters.FilterAPI) (*rpc.Server, error) {
{"eth", ethapi.NewEthereumAPI(b)},
// Standard Ethereum node APIs:
// - eth_blockNumber
// - eth_call
// - eth_chainId
// - eth_estimateGas
// - eth_getBalance
// - eth_getBlockByHash
// - eth_getBlockByNumber
// - eth_getBlockReceipts
// - eth_getCode
// - eth_getStorageAt
// - eth_getUncleByBlockHashAndIndex
// - eth_getUncleByBlockNumberAndIndex
// - eth_getUncleCountByBlockHash
// - eth_getUncleCountByBlockNumber
//
// geth-specific APIs:
// Geth-specific APIs:
// - eth_createAccessList
// - eth_getHeaderByHash
// - eth_getHeaderByNumber
//
// Undocumented APIs:
// - eth_getBlockReceipts
// - eth_getProof
{"eth", &blockChainAPI{ethapi.NewBlockChainAPI(b), b}},
// Standard Ethereum node APIs:
// - eth_getBlockTransactionCountByHash
Expand Down
86 changes: 76 additions & 10 deletions sae/rpc_stateful_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/ava-labs/libevm/core/vm"
"github.com/ava-labs/libevm/crypto"
"github.com/ava-labs/libevm/eth/tracers/logger"
"github.com/ava-labs/libevm/ethclient/gethclient"
"github.com/ava-labs/libevm/rpc"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
Expand Down Expand Up @@ -138,7 +139,7 @@ func TestDebugTrace(t *testing.T) {
sut.testRPC(ctx, t, tests...)
}

func TestEthCall(t *testing.T) {
func TestStatefulRPCs(t *testing.T) {
opt, vmTime := withVMTime(t, time.Unix(saeparams.TauSeconds, 0))
ctx, sut := newSUT(t, 1, opt)

Expand Down Expand Up @@ -175,6 +176,26 @@ func TestEthCall(t *testing.T) {
_, ok := sut.rawVM.consensusCritical.Load(b.Hash())
require.Falsef(t, ok, "%T[%#x] still in VM memory", b, b.Hash())

// Storage key for balances[recv] at mapping slot 0:
// keccak256(abi.encode(address, uint256(0)))
storageKey := crypto.Keccak256Hash(
common.LeftPadBytes(recv.Bytes(), 32),
common.Hash{}.Bytes(),
)
storageKeyHex := storageKey.Hex()

callMsg := ethereum.CallMsg{
From: sut.wallet.Addresses()[0],
To: &escrowAddr,
Data: escrow.CallDataForBalance(recv),
}

gc := gethclient.New(sut.rpcClient)

wantBig := big.NewInt(val)
wantBytes := uint256.NewInt(val).PaddedBytes(32)
wantCode := escrow.ByteCode()

tests := []struct {
name string
num rpc.BlockNumber
Expand All @@ -190,15 +211,60 @@ func TestEthCall(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
msg := ethereum.CallMsg{
To: &escrowAddr,
Data: escrow.CallDataForBalance(recv),
}

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
Comment thread
ARR4N marked this conversation as resolved.
require.NoError(t, err)
assert.Equal(t, uint256.NewInt(val).PaddedBytes(32), got)
blockNum := big.NewInt(int64(tt.num))

t.Run("eth_call", func(t *testing.T) {
Copy link
Copy Markdown
Contributor

@alarso16 alarso16 Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to have all of these subtests in the same test? I think the answer is no, since a lot of these do not need the infrastrucure required by others

Copy link
Copy Markdown
Contributor Author

@JonathanOppenheimer JonathanOppenheimer Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wdym? these all require the sut. are you referring to the last two?

Copy link
Copy Markdown
Contributor Author

@JonathanOppenheimer JonathanOppenheimer Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talked in person. I personally think leaving all of the appropriate tests in the loop so they can share the block test cases make sense, but I will split out the two outliers. Would appreciate other opinions.

Copy link
Copy Markdown
Contributor Author

@JonathanOppenheimer JonathanOppenheimer Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, hm I still don't like doing this because, then I need to literally 1-1 triply duplicate escrowAddr, recv, sign, and b.

Thoughts?

Copy link
Copy Markdown
Contributor

@powerslider powerslider Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The single-test approach is the right call here I think. All 7 RPCs need identical expensive setup (VM init -> deploy -> execute -> settle -> evict), and the block-number (in-memory vs on-disk) is the shared property being tested. Splitting would either duplicate the setup code or if we do a shared helper it would carry around 10 values back via a struct which is more indirection for no gain, since each test function would collapse to a trivial few lines wrapper. Seeing all RPCs side by side is easier to read. The two outliers (eth_estimateGas, eth_createAccessList) are correctly outside the loop with a clear comment.

Copy link
Copy Markdown
Contributor Author

@JonathanOppenheimer JonathanOppenheimer Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it! Appreciate the vote of confidence haha.

got, err := sut.CallContract(ctx, callMsg, blockNum)
require.NoError(t, err, "CallContract()")
assert.Equal(t, wantBytes, got, "CallContract() result")
})

t.Run("eth_getBalance", func(t *testing.T) {
got, err := sut.BalanceAt(ctx, escrowAddr, blockNum)
require.NoError(t, err, "BalanceAt()")
require.Zero(t, wantBig.Cmp(got), "BalanceAt(): want %d, got %s", val, got)
})

t.Run("eth_getCode", func(t *testing.T) {
got, err := sut.CodeAt(ctx, escrowAddr, blockNum)
require.NoError(t, err, "CodeAt()")
assert.Equal(t, wantCode, got, "CodeAt() result")
})

t.Run("eth_getStorageAt", func(t *testing.T) {
got, err := sut.StorageAt(ctx, escrowAddr, storageKey, blockNum)
require.NoError(t, err, "StorageAt()")
assert.Equal(t, wantBytes, got, "StorageAt() result")
})

t.Run("eth_getProof", func(t *testing.T) {
got, err := gc.GetProof(ctx, escrowAddr, []string{storageKeyHex}, blockNum)
require.NoError(t, err, "GetProof()")
require.NotNil(t, got, "GetProof() result")

assert.NotEmpty(t, got.AccountProof, "GetProof() accountProof")
require.Zero(t, wantBig.Cmp(got.Balance), "GetProof() balance: want %d, got %s", val, got.Balance)

require.Len(t, got.StorageProof, 1, "GetProof() storageProof length")
assert.NotEmpty(t, got.StorageProof[0].Proof, "GetProof() storageProof[0].Proof")
require.Zero(t, wantBig.Cmp(got.StorageProof[0].Value), "GetProof() storageProof[0].Value: want %d, got %s", val, got.StorageProof[0].Value)
})
})
}

// eth_estimateGas and eth_createAccessList don't accept a block number
// parameter via ethclient/gethclient, so they always run against latest.
t.Run("eth_estimateGas", func(t *testing.T) {
got, err := sut.EstimateGas(ctx, callMsg)
require.NoError(t, err, "EstimateGas()")
assert.Positive(t, got, "EstimateGas() result")
})

t.Run("eth_createAccessList", func(t *testing.T) {
al, gas, errMsg, err := gc.CreateAccessList(ctx, callMsg)
require.NoError(t, err, "CreateAccessList()")
assert.Empty(t, errMsg, "CreateAccessList() error message")
assert.NotNil(t, al, "CreateAccessList() access list")
assert.Positive(t, gas, "CreateAccessList() gasUsed")
})
}
Loading