Disable mempool traffic in freeze mode - #3990
Conversation
Freeze mode prevents the node from executing future blocks, so transactions accepted through RPC or P2P cannot be committed. Continuing to validate, retain, and gossip them wastes application and network resources and leaves stale transactions in the mempool. Keep the internal mempool required by block execution, but do not expose it to RPC or start the mempool reactor when freeze mode is enabled.
PR SummaryMedium Risk Overview When freeze is active, the node stays up for queries but stops accepting mutating RPC: all Operator-facing text in Reviewed by Cursor Bugbot for commit 0cd4a12. 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).
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3990 +/- ##
==========================================
- Coverage 59.34% 58.27% -1.08%
==========================================
Files 2278 2179 -99
Lines 196008 184466 -11542
==========================================
- Hits 116326 107492 -8834
+ Misses 68927 67158 -1769
+ Partials 10755 9816 -939
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 using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 07bdb2f. Configure here.
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
Skipping the mempool reactor in freeze mode leaves the node advertising MempoolChannel in its NodeInfo, so peer tx gossip lands on a channel the connection does not know and tears the P2P connection down; separately, dropping the RPC mempool makes eth_getTransactionCount(..., "pending") silently return 0. Both need addressing before merge.
Findings: 2 blocking | 1 non-blocking | 2 posted inline
Blockers
- None at the file/PR level.
- 2 blocking issue(s) flagged inline on specific lines.
Non-blocking
- [suggestion]
sei_content/txpool_content(evmrpc/txpool.go →UnconfirmedTxs) now returns the errormempool is not availableon a frozen node instead of an empty pending set. Returning an empty result would keep read-only RPC consumers working; if erroring is intended, the message could name freeze mode so operators can tell it apart from a misconfigured node.
|
@seidroid review |
|
@seidroid review |
There was a problem hiding this comment.
Both previously reported blockers are fixed: MempoolChannel is no longer advertised when frozen (so peers won't gossip txs onto an unopened channel), and rpcEnv.Mempool stays populated so pending-nonce reads keep working while only the four broadcast endpoints are gated. Remaining notes are non-blocking: tx admission is disabled from startup rather than from the freeze height (relevant if freeze-height is ever set on a validator), and the operator-facing docs/changelog still describe the old behavior.
Findings: 0 blocking | 3 non-blocking | 1 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- [suggestion] Operator-facing documentation still describes the pre-PR behavior.
sei-cosmos/server/config/config.go:117-119and thefreeze-heighttemplate comment insei-cosmos/server/config/toml.go:44-46both say the node "stops before executing the block while continuing to serve RPC", butbroadcast_tx_async|sync|commit(and thereforeeth_sendRawTransaction) now fail on a freeze node. Worth updating those comments, and adding a CHANGELOGUnreleasedentry — the change is operator- and peer-visible (tx submission errors;MempoolChanneldisappears from NodeInfo). - 1 suggestion(s)/nit(s) flagged inline on specific lines.
- 1 non-blocking pre-existing issue(s) listed below under pre-existing issues.
Pre-existing issues
- [suggestion] Autobahn/giga nodes hit the same channel mismatch this PR fixes for freeze mode: the mempool and its reactor live entirely inside
if !gigaEnabled { ... }(sei-tendermint/node/node.go:334), so channel 0x30 is never opened, yetmakeNodeInfostill advertisesMempoolChannelfor them. Benign as long as every peer on an autobahn network is also autobahn (nobody gossips txs), but in a mixed network an inbound mempool packet would failmconn.Runand tear the connection down. The newmempoolP2PEnabledargument is the natural place to fold this in (!gigaEnabled && freezeHeight == 0).
Superseded: latest AI review found no blocking issues.
|
@seidroid review |
There was a problem hiding this comment.
Freeze mode is now restricted to full nodes, tx broadcast is gated at the four broadcast_tx_* entry points, the mempool reactor is skipped, and MempoolChannel is only advertised when the reactor actually opened it — the last outstanding review thread (freeze mode on validators) is addressed, docs and CHANGELOG are updated, and the gating covers every RPC path that admits transactions. One non-blocking test-coverage note.
Findings: 0 blocking | 1 non-blocking | 0 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- [suggestion] The CHANGELOG claims Autobahn nodes also stop advertising the mempool channel, and that now falls out of
mempoolP2PEnableddefaulting to false whengigaEnabled— but nothing tests it.TestFreezeModeDisablesMempoolTrafficonly exercises the freeze path. SincemakeNodeInfois in-package and takes the flag explicitly, a direct unit test assertingbyte(mempoolreactor.MempoolChannel)is present formempoolP2PEnabled=trueand absent forfalsewould pin both cases cheaply (sei-tendermint/node/setup.go:630).
|
@seidroid review |
There was a problem hiding this comment.
Freeze mode is now restricted to full nodes, RPC writes are gated at all four broadcast endpoints plus evidence submission via a single requireWritable choke point, and MempoolChannel is advertised only when the reactor that opens it actually exists. One non-blocking test-coverage gap remains.
Findings: 0 blocking | 1 non-blocking | 0 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- [suggestion] The
mempoolP2PEnabled=falsepath is only exercised for freeze mode. Autobahn nodes now also omitMempoolChannel(the flag is set inside the!gigaEnabledbranch), and the CHANGELOG advertises that, but no test pins it.makeNodeInfois in-package and takes the flag explicitly, so a direct unit test assertingbyte(mempoolreactor.MempoolChannel)is present fortrueand absent forfalsewould cover both cases cheaply (sei-tendermint/node/setup.go:630).
|
Created backport PR for
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-3990-to-release/v6.6
git worktree add --checkout .worktree/backport-3990-to-release/v6.6 backport-3990-to-release/v6.6
cd .worktree/backport-3990-to-release/v6.6
git reset --hard HEAD^
git cherry-pick -x 50e1129ebfc92487f59067d13daca22c26bcb4b1
git push --force-with-lease |
Backport of #3990 to `release/v6.6`. --------- Co-authored-by: Masih H. Derkani <m@derkani.org>
2 participants
Freeze mode prevents the node from executing future blocks, so transactions accepted through RPC or P2P cannot be committed. Continuing to validate, retain, and gossip them wastes application and network resources and leaves stale transactions in the mempool.
Keep the internal mempool required by block execution, but do not expose it to RPC or start the mempool reactor when freeze mode is enabled.