rpc sharding by tx sender for autobahn#3438
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3438 +/- ##
==========================================
+ Coverage 59.30% 59.31% +0.01%
==========================================
Files 2127 2127
Lines 175830 175943 +113
==========================================
+ Hits 104268 104369 +101
- Misses 62477 62479 +2
- Partials 9085 9095 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9c3516e. Configure here.
| SEI_NODE_ID=$(seid tendermint show-node-id) | ||
| NODE_IP=$(hostname -i | awk '{print $1}') | ||
| P2P_PORT=26656 # Must match [p2p] laddr in config.toml | ||
| EVMRPC_PORT=8545 # Must match the EVM RPC HTTP port (evmrpc DefaultConfig HTTPPort). |
There was a problem hiding this comment.
nit: would be nice if both ports come from a common config
There was a problem hiding this comment.
these are preexisting configs. Merging them would be backward incompatible.
| )), | ||
| redirectedRequestCount: must(rpcTelemetryMeter.Int64Counter( | ||
| "evmrpc_redirected_requests_total", | ||
| metric.WithDescription("Number of EVM RPC requests redirected to another validator"), |
There was a problem hiding this comment.
nit: can you clarify whether this is request you forwarded to someone else or it is request you received from redirection?
There was a problem hiding this comment.
reworded to "forwarded to another validator"
| // the underlying TCP connection lifecycle is strictly bound to Dial -> Close calls. | ||
| client, err := rpc.DialContext(ctx, url.String()) | ||
| if err != nil { | ||
| return hash, fmt.Errorf("rpc.DialContext(%q): %w", url.String(), err) |
There was a problem hiding this comment.
Would we see one error log per tx when one validator is down? That would be too spammy
There was a problem hiding this comment.
I'm not sure if we log all evmrpc RPC errors, but this will be consistent with the current behavior - we return error if tx does not land in the correct mempool
| // but we still need to handle it. | ||
| sender, senderErr := getSender(tx, s.keeper.ChainID(s.ctxProvider(LatestCtxHeight))) | ||
| if senderErr == nil { | ||
| if url, ok := s.tmClient.EvmProxy(sender); ok { |
There was a problem hiding this comment.
What's our plan for handling the case where one validator is down?
There was a problem hiding this comment.
for testnet - some accounts will be censored. We need to implement a proper solution afterwards.
| from, err = signer.Sender(tx) | ||
| return from, nil | ||
| case tx.Protected(): | ||
| from, err := ethtypes.NewEIP155Signer(chainID).Sender(tx) |
There was a problem hiding this comment.
Would we always have NewEIP155Signer in tx.Protected()? Would ethtypes.LatestSignerForChainID(chainID)) be better? So we accept non-1559 transactions?
There was a problem hiding this comment.
I have just copied over what simulateTx did. Is this not correct?
There was a problem hiding this comment.
I think LatestSignerForChainID(chainID) is a better approach guaranteed in go-ethereum library to handle all cases. I think the current code has a gap, but it only affected simulateTx's gas estimate, now we are skipping the proxy for it.
There was a problem hiding this comment.
make sense, switched to LatestSignerForChainID
| h := sha256.Sum256(addr[:]) | ||
| x := new(big.Int).SetBytes(h[:]) | ||
| i := int(x.Mod(x, big.NewInt(int64(c.replicas.Len()))).Int64()) | ||
| return c.replicas.At(i) |
There was a problem hiding this comment.
We don't have that many validators so maybe we can use a cheaper algorithm?
i := int(binary.BigEndian.Uint64(addr[:8]) % uint64(c.replicas.Len()))
I suppose we will have a different algorithm when Autobahn hits mainnet, so we don't need to worry about attacker reverse-engineering this algorithm for now?
There was a problem hiding this comment.
good point, although imo it will not be a bottleneck and eventually it will be even more expensive.
PR SummaryHigh Risk Overview Extends Autobahn config generation and node setup to carry per-validator Reviewed by Cursor Bugbot for commit ce8109f. Bugbot is set up for automated code reviews on this repo. Configure here. |
3 participants
For giga testnet we need every account to send transaction only to a single lane, so that they are included in nonce order. In this pr we are adding proxying of transactions to the correct lane (validator). How it works:
Note
Medium Risk
Introduces request redirection for
eth_sendRawTransactionand pendingeth_getTransactionCount, which changes transaction submission/nonce semantics and adds cross-validator HTTP RPC dependencies driven by config.Overview
Adds sender-based RPC sharding for Autobahn by introducing
LocalClient.EvmProxy(sender)and wiring it through the Tendermint local client/environment andGigaRouter(deterministic shard selection viaCommittee.EvmShard).evmrpcnow redirectseth_sendRawTransactionand pendingeth_getTransactionCountto the shard owner’s EVM RPC HTTP endpoint when the sender maps to a different validator, with a fallback to existing local behavior when no proxy is configured or Autobahn isn’t active. Adds a new OpenTelemetry counterevmrpc_redirected_requests_total.Extends Autobahn config generation/consumption to include per-validator
evmrpcURLs (newevmrpc_url.txtin localnode init, newevmrpcfield in config with URL marshal/unmarshal) and updates/expands tests to cover proxy behavior and the shard→URL mapping.Reviewed by Cursor Bugbot for commit 33794ff. Bugbot is set up for automated code reviews on this repo. Configure here.