Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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/params"
"github.com/ava-labs/libevm/rpc"
"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -198,7 +199,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 @@ -235,6 +236,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 @@ -250,15 +271,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

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

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

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

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

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

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