feat(evmonly): add eth_getTransactionCount, eth_blockNumber, eth_chainId RPC, backport eth_getBalance - #4192
Conversation
…nId RPC Extends the EVM-only JSON-RPC server with the remaining read-only chain metadata endpoints, and reorganizes the package to group methods by JSON-RPC namespace (state.go, tx.go, info.go, send.go) matching evmrpc's layout instead of one file per method. - add eth_getTransactionCount, eth_blockNumber, eth_chainId, backed by new Backend.EvmTransactionCount/EvmBlockNumber/EvmChainID methods - rename Backend.EvmNonce to Backend.EvmTransactionCount to match the RPC method name; the underlying abci.Application.EvmNonce is unchanged - cover the new endpoints in the Autobahn integration test and README - add EvmChainID to sei-cosmos/server/rollback_test.go's mockApplication to satisfy the updated abci.Application interface
shemnon
changed the title
giga-1: feat(evmonly): add eth_getTransactionCount, eth_blockNumber, eth_chainId RPC
PR SummaryMedium Risk Overview The Autobahn integration tests and the README now cover Reviewed by Cursor Bugbot for commit b484653. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean, well-tested backport of eth_getTransactionCount/eth_blockNumber/eth_chainId plus the eth_getBalance backport; the interface change is propagated to every abci.Application implementer and the file reorganization is behaviour-preserving. Two non-blocking notes: pending nonce semantics and a gap between the PR description's claimed integration-test coverage and what is actually asserted.
Findings: 0 blocking | 2 non-blocking | 2 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- None at the file/PR level.
- 2 suggestion(s)/nit(s) flagged inline on specific lines.
| } | ||
|
|
||
| // GetTransactionCount returns the address nonce from the current committed EVM state. | ||
| func (api *txAPI) GetTransactionCount(_ context.Context, address common.Address, block ethrpc.BlockNumberOrHash) (*hexutil.Uint64, error) { |
There was a problem hiding this comment.
[suggestion] pending is accepted here but resolves to the committed nonce (evmOnlyApplication.EvmNonce reads the committed StateDB view; the Autobahn mempool is not consulted). Standard Ethereum tooling — cast send, ethers, viem — requests eth_getTransactionCount(addr, "pending") to pick the next nonce, so two back-to-back sends from the same key will reuse a nonce while the first transaction is still in flight, and the second is rejected.
The README rationale added for this (integration_test/autobahn/README.md:437-438, "all four read the same current committed state, because Sei has instant finality") isn't quite right: instant finality removes reorg risk, not the broadcast-to-commit window that pending exists to cover. Returning latest is a defensible tradeoff (rejecting pending would break cast send outright), but the caveat is worth stating accurately in the README so the next person debugging a nonce collision doesn't have to rediscover it.
| assertEVMOnlyReceipts(t, ctx, clients, block.Txs) | ||
| assertEVMOnlyBalances(t, ctx, clients, block.Txs) | ||
| assertEVMOnlyChainID(t, ctx, clients) | ||
| assertEVMOnlyBlockNumber(t, ctx, clients, lastHeight) |
There was a problem hiding this comment.
[suggestion] The PR description says the new endpoints are covered in the Autobahn integration test, but eth_getTransactionCount is the one new endpoint with no assertion here — only balance, chain ID, and block number are checked (grep getTransactionCount integration_test/autobahn/autobahn_test.go returns nothing). The README also advertises cast nonce against this RPC, so it's the endpoint most likely to be exercised by users.
An assertEVMOnlyTransactionCount mirroring assertEVMOnlyBalances would close this: decode txs[nodeIndex], recover the sender, and assert the nonce advanced past the transaction's own nonce (and/or assert the fresh recipient's nonce is 0).
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## giga-1 #4192 +/- ##
==========================================
- Coverage 65.55% 65.53% -0.03%
==========================================
Files 2081 2080 -1
Lines 157460 157147 -313
==========================================
- Hits 103222 102985 -237
+ Misses 54097 54021 -76
Partials 141 141
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…ionCount Addresses seidroid review notes on #4192. - eth_getTransactionCount accepts pending for tooling compatibility but resolves it to the committed nonce; the README previously attributed this to instant finality, which explains away reorg risk, not the broadcast-to-commit window pending exists to cover - add assertEVMOnlyTransactionCount to the Autobahn integration test, mirroring assertEVMOnlyBalances, closing the gap between the PR description's claimed coverage and what was actually asserted
…nonce-blocknumber-chainid-giga1
…mber-chainid-giga1' into scope/giga-evmonly-nonce-blocknumber-chainid-giga1
2 participants
Describe your changes and provide context
Backports the eth_getTransactionCount, eth_blockNumber, and eth_chainId
RPC methods to giga-1, on top of the eth_getBalance backport (#4174).
eth_getTransactionCount,eth_blockNumber,eth_chainId, backedby new
Backend.EvmTransactionCount/EvmBlockNumber/EvmChainIDmethods
Backend.EvmNoncetoBackend.EvmTransactionCountto match theRPC method name; the underlying
abci.Application.EvmNonceisunchanged
giga/evmonly/rpcto group methods by JSON-RPC namespace(
state.go,tx.go,info.go,send.go), matchingevmrpc'slayout instead of one file per method
Testing performed to validate your change
go build ./...go test ./giga/evmonly/... ./sei-tendermint/internal/evmonlyapp/... ./sei-tendermint/internal/rpc/core/... ./sei-cosmos/server/... ./app/...gofmt -s/goimportsclean