Wire eth_subscribe(newHeads) for Autobahn (CON-257) by wen-coding · Pull Request #3419 · sei-protocol/sei-chain

wen-coding · 2026-05-11T20:15:54Z

Summary

Under Autobahn, eth_subscribe("newHeads") is currently broken: the legacy fan-out subscribes to the Tendermint event bus on tm.event = 'NewBlockHeader', but Autobahn drives app.FinalizeBlock from giga_router rather than CometBFT consensus, so PublishEventNewBlockHeader never fires.
This PR adds a direct in-process notifier (BlockHeaderNotifier) consumed by evmrpc.SubscriptionAPI's fan-out goroutine. The notifier is fed from App.Commit itself — FinalizeBlocker stashes (hash, header, response) and App.Commit publishes after the underlying BaseApp.Commit succeeds. The legacy CometBFT path is untouched (notifier is only constructed when tmConfig.AutobahnConfigFile != "").
Notifier semantics: single-producer, single-consumer, capacity 1, overwrite-on-full — newHeads subscribers care about the latest head, not a backlog, so stale heads are dropped in favor of new ones.
Zero sei-tendermint API additions. The notification is driven entirely by ABCI methods that already exist (FinalizeBlock + Commit); giga_router stays a pure ABCI consumer.

Test plan

gofmt -s -l clean on all modified files; go build ./... clean.
go test ./evmrpc/ ./sei-tendermint/internal/p2p/ ./sei-tendermint/internal/proxy/ ./sei-cosmos/baseapp/ ./app/ — all PASS.
- existing TestSubscribeNewHeads (legacy event-bus path) — unchanged behaviour.
- TestSubscribeNewHeadsAutobahn (notifier path, in-process WS server end-to-end).
- unit tests for BlockHeaderNotifier (deliver, overwrite-on-full, nil-safe) and encodeCommittedBlock (happy path + zero gasLimit).
- pickHeadBaseFee off-by-one coverage.
evm_rpc_tests.sh (HTTP-method fixtures) against a real non-Autobahn cluster — 160/160 pass, no regressions on the request/response RPC surface.
integration_test/evm_module/ws_test/ — consensus-mode-agnostic, gated on SEI_EVM_WS_RUN_INTEGRATION=1, wired into evm_rpc_tests.sh. End-to-end against both cluster modes:
- Non-Autobahn cluster: PASS (number=0x2b, hash 0xe0dd089b...)
- Autobahn cluster: PASS (number=0xf5, hash 0xc4d1bc59..., GigaRouter initialized confirmed in all 4 node logs)
Reviewer: confirm non-app-hash-breaking label is correct — this PR only adds a notification side-effect after commit; no state machine or app-hash-relevant logic changes.

🤖 Generated with Claude Code

Note

Medium Risk
Touches the block execution lifecycle by stashing FinalizeBlock data and publishing from App.Commit, which could affect head notification correctness/timing if mis-ordered or if Commit error paths differ. WS subscription behavior now has two code paths (event bus vs notifier) and relies on correct base-fee/gas-limit derivation for the new notifier stream.

Overview
Fixes eth_subscribe("newHeads") under Autobahn by adding an in-process BlockHeaderNotifier that bypasses the Tendermint event bus.

FinalizeBlocker now stashes the (hash, header, finalize response) tuple and App.Commit publishes it only after a successful commit; stale stashes are defensively cleared to prevent announcing the wrong block. The WS server/SubscriptionAPI is extended to consume either the legacy event-bus stream or the new notifier stream (Autobahn), including a dedicated encodeCommittedBlock payload builder and a base-fee selection fix using the parent height.

Adds unit + integration coverage for notifier overwrite semantics, committed-head encoding, stale-stash clearing, and an end-to-end WS newHeads test for the notifier-backed path; integration scripts now run WS subscription tests.

^{Reviewed by Cursor Bugbot for commit fab0c5b. Bugbot is set up for automated code reviews on this repo. Configure here.}

github-actions · 2026-05-11T20:16:53Z

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
`✅ passed`	`✅ passed`	`✅ passed`	`✅ passed`	May 15, 2026, 10:57 PM

codecov · 2026-05-11T20:21:13Z

Codecov Report

❌ Patch coverage is 75.65217% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.34%. Comparing base (5060dcd) to head (062f0c3).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
evmrpc/subscribe.go	81.01%	9 Missing and 6 partials ⚠️
app/app.go	46.15%	6 Missing and 1 partial ⚠️
app/abci.go	0.00%	3 Missing and 1 partial ⚠️
evmrpc/notifier.go	88.88%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3419      +/-   ##
==========================================
+ Coverage   59.31%   59.34%   +0.03%     
==========================================
  Files        2120     2121       +1     
  Lines      175523   175709     +186     
==========================================
+ Hits       104106   104281     +175     
+ Misses      62338    62334       -4     
- Partials     9079     9094      +15

Flag	Coverage Δ
sei-chain-pr	`60.95% <75.65%> (?)`
sei-db	`70.41% <ø> (-0.22%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
evmrpc/server.go	`92.67% <100.00%> (+4.88%)`	⬆️
evmrpc/notifier.go	`88.88% <88.88%> (ø)`
app/abci.go	`62.79% <0.00%> (-1.50%)`	⬇️
app/app.go	`69.71% <46.15%> (-0.15%)`	⬇️
evmrpc/subscribe.go	`68.69% <81.01%> (+6.84%)`	⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

encodeCommittedBlock was mirroring the legacy encodeTmHeader, which emits both the typo'd "withdrawlsRoot" and the correctly-spelled "withdrawalsRoot". Since the Autobahn encoder is new code there is no back-compat reason to carry the typo; legacy encodeTmHeader is left untouched. Reported by Cursor Bugbot on #3419. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

ResponseFinalizeBlock.ConsensusParamUpdates is only populated when the app proposes a consensus-param update — nil on the vast majority of blocks. Reading it for newHeads gasLimit means the reported gasLimit would be 0 for nearly every notification under Autobahn. Match the pattern already used in evmrpc/block.go's GetBlockByNumber: pull gasLimit from ctx.ConsensusParams() in the consumer goroutine and pass it as a parameter into encodeCommittedBlock. Reported by Cursor Bugbot on #3419 (high severity). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

cursor

Cursor Bugbot has reviewed your changes 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 84cd2e6. Configure here.}

encodeCommittedBlock was mirroring the legacy encodeTmHeader, which emits both the typo'd "withdrawlsRoot" and the correctly-spelled "withdrawalsRoot". Since the Autobahn encoder is new code there is no back-compat reason to carry the typo; legacy encodeTmHeader is left untouched. Reported by Cursor Bugbot on #3419. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

ResponseFinalizeBlock.ConsensusParamUpdates is only populated when the app proposes a consensus-param update — nil on the vast majority of blocks. Reading it for newHeads gasLimit means the reported gasLimit would be 0 for nearly every notification under Autobahn. Match the pattern already used in evmrpc/block.go's GetBlockByNumber: pull gasLimit from ctx.ConsensusParams() in the consumer goroutine and pass it as a parameter into encodeCommittedBlock. Reported by Cursor Bugbot on #3419 (high severity). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

ExecTxResult has both a GasWanted and a GasUsed field with different semantics. The variable was accumulating GasUsed but named "gasWanted", inviting a future field-swap regression. Rename for clarity. Legacy encodeTmHeader carries the same anti-pattern but is left alone. Reported by Cursor Bugbot on #3419 (low severity). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@amir-deris

The five `if err := ...` blocks in TestEthSubscribeNewHeads each shadowed the outer err from the initial Dial call. Reuse with `err =` for clarity (the outer err is already in scope and gets overwritten on each call anyway). Reported by @amir-deris on #3419. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

pompon0 · 2026-05-14T15:14:39Z

 ) (service.Service, error) {
+	// Capture any optional BlockHeaderListener implementation before
+	// wrapping the app with the ABCI proxy, which only forwards ABCI
+	// methods.


this looks bad - either add a method returning BlockHeaderListener to Application interface, or add a separate argument to New

I feel like adding OnBlockCommitted to Application should be good enough

Adding OnBlockCommitted would require us to touch every App impl.

But I got your point. Thought about it more and decided to override Commit in App to push, see if this looks better? We don't need the duct tape any more.

Under Autobahn, app.FinalizeBlock is driven by giga_router rather than CometBFT consensus, so the legacy Tendermint event-bus subscription that backs eth_subscribe("newHeads") never fires. This adds a direct in-process notifier from giga_router's commit path into evmrpc's SubscriptionAPI fan-out (overwrite-on-full, capacity 1, so the latest head always wins if a consumer lags). The legacy path is untouched. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

encodeCommittedBlock was mirroring the legacy encodeTmHeader, which emits both the typo'd "withdrawlsRoot" and the correctly-spelled "withdrawalsRoot". Since the Autobahn encoder is new code there is no back-compat reason to carry the typo; legacy encodeTmHeader is left untouched. Reported by Cursor Bugbot on #3419. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

- Tighten hash field doc in blockHeaderEvent: receipt store records zero blockHash on disk and evmrpc overlays the autobahn hash at read time. The prior wording implied the receipt store stored it directly. - Clarify single-producer assumption in OnBlockCommitted and document multi-producer behaviour (still safe, latest-wins is acceptable). - Move SubscriptionManager construction inside the legacy event-bus branch in NewSubscriptionAPI so the field is nil under Autobahn instead of dead state. - Document the non-nil header/response contract on BlockHeaderListener, and add a defensive guard in runNewHeadsFromNotifier so a single malformed event does not kill the fan-out goroutine for all subscribers. - Annotate the listener call site in giga_router.executeBlock so the fire-after-Commit-before-mempool-update ordering is explicit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

ResponseFinalizeBlock.ConsensusParamUpdates is only populated when the app proposes a consensus-param update — nil on the vast majority of blocks. Reading it for newHeads gasLimit means the reported gasLimit would be 0 for nearly every notification under Autobahn. Match the pattern already used in evmrpc/block.go's GetBlockByNumber: pull gasLimit from ctx.ConsensusParams() in the consumer goroutine and pass it as a parameter into encodeCommittedBlock. Reported by Cursor Bugbot on #3419 (high severity). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

GetNextBaseFeePerGas(ctx_at_N) returns the base fee for block N+1, not block N. The previous code passed the current block's ctx, so the newHeads notification for block N reported what should have been attached to N+1. Match the pattern from block.go's GetBlockByNumber: derive baseFee from the parent block's ctx, with a genesis (height==1) fallback to DefaultMinFeePerGas. The legacy CometBFT path has the same bug but is left alone — the more important consistency to restore is between eth_subscribe(newHeads) and eth_getBlockByNumber on the same Autobahn cluster. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

ExecTxResult has both a GasWanted and a GasUsed field with different semantics. The variable was accumulating GasUsed but named "gasWanted", inviting a future field-swap regression. Rename for clarity. Legacy encodeTmHeader carries the same anti-pattern but is left alone. Reported by Cursor Bugbot on #3419 (low severity). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@amir-deris

The five `if err := ...` blocks in TestEthSubscribeNewHeads each shadowed the outer err from the initial Dial call. Reuse with `err =` for clarity (the outer err is already in scope and gets overwritten on each call anyway). Reported by @amir-deris on #3419. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The base-fee selection in runNewHeadsFromNotifier was the actual location of the off-by-one fix (parent ctx vs current ctx) but had no direct test coverage — encodeCommittedBlock takes baseFee as an arg, so testing it only proved the encoder forwards the value it's given, not that the caller picks the right ctx. Extract pickHeadBaseFee as a free function that takes getNextBaseFee as a function pointer (rather than reaching into *keeper.Keeper), then spy on the ctxProvider in tests to verify: - TestPickHeadBaseFee_UsesParentCtx: ctxProvider called with height-1 (NOT height), and result is forwarded. - TestPickHeadBaseFee_GenesisFallback: at height 1 the keeper call is skipped entirely and DefaultMinFeePerGas is returned. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…er in Option Header.AppHash by Tendermint convention holds the previous block's app hash. The OnBlockCommitted path was setting it to the current block's resp.AppHash, which is wrong. Construct the header once, leave AppHash unset, and have evmrpc's encodeCommittedBlock read stateRoot from response.AppHash (the current block's hash, which is what subscribers actually want). Also wrap GigaRouterConfig.BlockHeaderListener and the createRouter / makeNode parameters in utils.Option to make the optionality explicit at every call site (matches existing Option usage for validatorKey, txMempool, etc.).

The previous design routed eth_newHeads notifications through a BlockHeaderListener interface that giga_router invoked after Commit. That required a hidden type-assertion in node.New (to discover whether the app implemented the listener) and threaded the listener through New/makeNode/createRouter/gigaCfg. ABCI already provides exactly this signal: FinalizeBlock gives the app the hash, header, and the response it produces; Commit tells the app the block is durable. So the app can stash FinalizeBlock outputs and publish to the notifier from its own Commit override — no new interface, no parameter plumbing, no duck typing. - Delete sei-tendermint/types/block_listener.go - Delete BlockHeaderListener field from GigaRouterConfig - Delete listener invocation from GigaRouter.executeBlock (header dedup retained; Header.AppHash still unset per Tendermint convention) - Delete listener parameter from node.New / makeNode / createRouter - Delete the type-assertion capture from node/public.go - App.stashPendingHead at FinalizeBlocker success returns; publish inside existing App.Commit override after BaseApp.Commit succeeds

App.Commit publishes whatever sits in pendingHeadEvent. Today the field is only set by stashPendingHead at successful FinalizeBlocker returns and only cleared by Commit after publishing, so the lifecycle relies on every FinalizeBlocker return path either stashing or being preceded by a clearing Commit. That holds in production but is thin: an EthReplay or EthBlockTest early-return doesn't stash, and a failed Commit doesn't clear, so a future code path could republish a stale tuple on a later block. Clear the field at FinalizeBlocker entry so the stash always reflects only the current block's outputs (or nothing). Document the serialization invariant on the field. Add a test that pins the behavior via the optimistic + EthReplay early-return path.

masih · 2026-05-15T14:09:02Z

+		"extraData":             hexutil.Bytes{},              // inapplicable to Sei
+		"gasLimit":              hexutil.Uint64(gasLimit),     //nolint:gosec
+		"gasUsed":               hexutil.Uint64(totalGasUsed), //nolint:gosec
+		"logsBloom":             ethtypes.Bloom{},             // inapplicable to Sei


We are planning to fix this in the future right?

changed to TODO

masih · 2026-05-15T14:12:21Z

+	appHash := common.BytesToHash(evt.response.AppHash)
+	var totalGasUsed int64
+	for _, txRes := range evt.response.TxResults {
+		totalGasUsed += txRes.GasUsed


Is gas used from tx result always right? Thinking of specific edgecases here

hmm, good catch, but this is too large to fix in this PR, added a TODO as well

Two known gaps in the Autobahn newHeads payload: - logsBloom is returned as zero, but the field IS applicable to Sei (block.go ORs receipt blooms for full block reads). Replace the misleading "inapplicable to Sei" comment with a TODO. - gasUsed sums TxResult.GasUsed, which is inaccurate for ante-failing EVM txs; block.go reads from the receipt store instead. Document the approximation; subscribers needing exact numbers can call eth_getBlockByNumber.

cursor · 2026-05-15T16:10:52Z

PR Summary

Medium Risk
Adds a new block-commit notification path that runs on every block commit under Autobahn and changes how eth_subscribe("newHeads") is sourced there; correctness depends on publish timing and header encoding details, but legacy (non-Autobahn) behavior is gated to remain unchanged.

Overview
Fixes eth_subscribe("newHeads") under Autobahn by introducing an in-process BlockHeaderNotifier and switching the WS subscription fan-out to consume committed-block events instead of relying on the Tendermint event bus.

FinalizeBlocker now stashes the (hash, header, finalizeResponse) tuple and App.Commit publishes it only after a successful commit, ensuring subscribers see committed state; stale stashes are defensively cleared on FinalizeBlocker entry. The WS server is optionally wired with the notifier when Autobahn is enabled, and the Autobahn path encodes newHeads payloads via encodeCommittedBlock (including a base-fee height fix and consensus-param-derived gasLimit). Adds unit and integration tests covering notifier semantics (overwrite-on-full), encoding/baseFee behavior, and end-to-end WS newHeads delivery.

^{Reviewed by Cursor Bugbot for commit 062f0c3. Bugbot is set up for automated code reviews on this repo. Configure here.}

pompon0 · 2026-05-15T19:13:47Z

 	if err != nil {
 		panic(fmt.Sprintf("error reading EVM config due to %s", err))
 	}
+	if tmConfig != nil && tmConfig.AutobahnConfigFile != "" {


this is rather ugly that App needs to understand that it will interact with autobahn vs tendermint. Can we just make it always work instead?

You mean have non-Autobahn also ditch event bus and move here? It's possible but it might bring in semantics change, which is why I'm a bit hesitant. In this current code, there are a few header fields we haven't figured out how to properly populate yet.
Can I add a TODO for now, if we figure out it's semantically equivalent, then switch non-Autobahn?

…rified Pompon0 flagged that the app.go mode-gate is a smell — the App shouldn't need to know whether it's interacting with Autobahn or CometBFT. Agreed, but unifying requires reconciling the two encoders first: legacy encodeTmHeader populates parentHash/receiptsRoot/transactionsRoot from the real Tendermint Header and uses pre-execution stateRoot, while encodeCommittedBlock zeroes those fields and uses post-execution stateRoot. Switching legacy to the notifier path without that reconciliation would silently change non-Autobahn newHeads semantics. Add a TODO at the gate site describing what needs to be verified before removing it. Defer the actual refactor to a follow-up PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

wen-coding added the non-app-hash-breaking label May 11, 2026

wen-coding changed the title ~~Wire eth_subscribe(newHeads) for Autobahn via in-process notifier~~ Wire eth_subscribe(newHeads) for Autobahn (CON-257) May 11, 2026

cursor Bot reviewed May 11, 2026

View reviewed changes

Comment thread evmrpc/subscribe.go

wen-coding requested review from arajasek, codchen, jewei1997, masih and pompon0 May 11, 2026 20:52

codchen approved these changes May 12, 2026

View reviewed changes

cursor Bot reviewed May 12, 2026

View reviewed changes

Comment thread evmrpc/subscribe.go

wen-coding requested a review from amir-deris May 12, 2026 13:53

cursor Bot reviewed May 12, 2026

View reviewed changes

Comment thread evmrpc/subscribe.go

wen-coding force-pushed the wen/add_eth_subscribe_newhead_for_autobahn branch from 073496b to c8c0d46 Compare May 12, 2026 17:04

amir-deris approved these changes May 12, 2026

View reviewed changes

Comment thread integration_test/evm_module/ws_test/ws_test.go Outdated

arajasek approved these changes May 13, 2026

View reviewed changes

pompon0 reviewed May 14, 2026

View reviewed changes

Comment thread sei-tendermint/internal/p2p/giga_router.go Outdated

pompon0 reviewed May 14, 2026

View reviewed changes

Comment thread sei-tendermint/internal/p2p/giga_router.go Outdated

pompon0 reviewed May 14, 2026

View reviewed changes

Comment thread sei-tendermint/node/seed.go Outdated

wen-coding and others added 3 commits May 14, 2026 11:38

wen-coding and others added 6 commits May 14, 2026 11:38

wen-coding force-pushed the wen/add_eth_subscribe_newhead_for_autobahn branch from aa7e676 to e9eb7d3 Compare May 14, 2026 18:42

wen-coding requested a review from pompon0 May 14, 2026 20:04

wen-coding and others added 2 commits May 14, 2026 13:41

Merge branch 'main' into wen/add_eth_subscribe_newhead_for_autobahn

fab0c5b

masih approved these changes May 15, 2026

View reviewed changes

pompon0 reviewed May 15, 2026

View reviewed changes

Conversation

wen-coding commented May 11, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

github-actions Bot commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cursor Bot commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

wen-coding commented May 11, 2026 •

edited by cursor Bot

Loading

github-actions Bot commented May 11, 2026 •

edited

Loading

codecov Bot commented May 11, 2026 •

edited

Loading

cursor Bot commented May 15, 2026 •

edited

Loading