Skip to content

evmonlyapp: time FinalizeBlock's stages around the executor - #4259

Merged
bdchatham merged 2 commits into
giga-1from
devin/1789825833-prepare-phase-timer
Sep 19, 2026
Merged

bdchatham merged 2 commits into
giga-1from
devin/1789825833-prepare-phase-timer

Conversation

@bdchatham

Copy link
Copy Markdown
Contributor

Describe your changes and provide context

On giga-testnet-2 the Autobahn main loop's execution share is ~81% of wall, but the executor's own phase timer (evmonly_block_phase_duration_seconds_total: open_view, occ_, encode_, commit_state, ...) accounts for only ~51%. The remaining ~30% is spent in FinalizeBlock outside ExecutePreparedBlock — and today nothing measures it.

This adds a PhaseTimer to evmOnlyApplication that charges every stage of FinalizeBlock to a phase, published as evmonly_finalize_phase_duration_seconds_total{phase=...}:

  • take_senders — keccak of every raw tx + lookup of the senders CheckTx already recovered
  • prepare — Executor.PrepareBlock: RLP decode + ECDSA recovery for every sender this validator did not admit (~39/40 of txs at 40 validators)
  • execute — Executor.ExecutePreparedBlock (the executor timer breaks this one down further)
  • tx_results — building the ABCI TxResults

Reset() at return closes the last phase, so the gap between blocks is charged to nothing. FinalizeBlock is serialized by the executor mutex, so a single timer is safe.

Measurement only, no behaviour change. It decides whether pipelining PrepareBlock for block n+1 while n executes (the second half of #4219's loadtest design, which the node does not do) is worth building: if prepare is the bulk of the unaccounted ~30%, it is.

Testing performed to validate your change

  • New TestEVMOnlyApplicationTimesEveryFinalizeBlockPhase: binds the timer to a manual OTel reader and asserts all four phases are recorded by one FinalizeBlock.
  • scripts/ramtest.sh ./sei-tendermint/internal/evmonlyapp/... -race green; golangci-lint run (v2.13.2) 0 issues on the package; make fmtcheck clean.

Link to Devin session: https://app.devin.ai/sessions/ff612badcded4aa5914ea408dbb41888
Open in Devin Desktop: https://app.devin.ai/desktop/session/ff612badcded4aa5914ea408dbb41888?variant=devin
Requested by: @bdchatham

…ders, prepare, execute, tx_results)

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@cursor

cursor Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
Observability-only refactor with no change to block execution, commit, or ABCI responses.

Overview
Adds OpenTelemetry phase timing around EVM-only FinalizeBlock so wall time outside the executor’s own breakdown is visible in metrics.

A PhaseTimer on evmOnlyApplication emits evmonly_finalize_phase_duration_seconds_total{phase=...} for take_senders, prepare, execute, and tx_results. The finalize path is refactored into finalizeBlockLocked; executeBlockPipelined accepts the timer and marks prepare vs execute. Reset() on exit avoids attributing idle time between blocks.

Measurement only—no consensus or execution behavior change. A new test wires a manual OTel reader and asserts all four phases are recorded after one finalize/commit.

Reviewed by Cursor Bugbot for commit c419b39. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions

github-actions Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

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

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedSep 19, 2026, 2:04 PM

@codecov

codecov Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.59459% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.17%. Comparing base (7e3ca24) to head (c419b39).
⚠️ Report is 39 commits behind head on giga-1.

Files with missing lines Patch % Lines
sei-tendermint/internal/evmonlyapp/app.go 94.59% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           giga-1    #4259       +/-   ##
===========================================
+ Coverage   65.55%   86.17%   +20.61%     
===========================================
  Files        2081        2     -2079     
  Lines      157460      405   -157055     
===========================================
- Hits       103222      349   -102873     
+ Misses      54097       56    -54041     
+ Partials      141        0      -141     
Flag Coverage Δ
sei-chain ?
sei-chain-pr 86.17% <94.59%> (?)
sei-db ?

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

Files with missing lines Coverage Δ
sei-tendermint/internal/evmonlyapp/app.go 85.51% <94.59%> (+3.74%) ⬆️

... and 2079 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.

@seidroid seidroid Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Measurement-only change that adds a FinalizeBlock phase timer; the instrumentation is correct and the new test genuinely covers all four phases. One minor issue: the deferred Reset runs after the executor mutex is released, so the comment's "only touched with that lock held" invariant does not hold.

Findings: 0 blocking | 1 non-blocking | 1 posted inline

Blockers

  • None at the file/PR level.

Non-blocking

  • None at the file/PR level.
  • 1 suggestion(s)/nit(s) flagged inline on specific lines.

Comment thread sei-tendermint/internal/evmonlyapp/app.go Outdated
…s released

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@bdchatham
bdchatham enabled auto-merge September 19, 2026 14:13
@bdchatham
bdchatham added this pull request to the merge queue Sep 19, 2026
Merged via the queue into giga-1 with commit 5565a61 Sep 19, 2026
68 checks passed
@bdchatham
bdchatham deleted the devin/1789825833-prepare-phase-timer branch September 19, 2026 14:31
wen-coding pushed a commit that referenced this pull request Sep 24, 2026
Adapted from giga-1 commit 5565a61 (#4259) onto main's FinalizeBlock,
which has no finalizeBlockLocked/cursor split: phases take_senders,
execute and tx_results wrap the same steps inline.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
wen-coding pushed a commit that referenced this pull request Sep 25, 2026
Adapted from giga-1 commit 5565a61 (#4259) onto main's FinalizeBlock,
which has no finalizeBlockLocked/cursor split: phases take_senders,
execute and tx_results wrap the same steps inline.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
revofusion pushed a commit to revofusion/sei-chain that referenced this pull request Sep 25, 2026
…ei-protocol#4330)

## Summary
- Backport Autobahn execute-loop phase metrics from giga-1 (`sei-protocol#4172` Go
path, plus `sei-protocol#4263` storage-tail split) so `/metrics` can show consensus
/ execution / storage, then vault commit / app commit / push app hash /
prune.
- Count txs packed into locally produced lane blocks
(`tendermint_internal_autobahn_avail_produced_txs`, `sei-protocol#4180`).
- Time `FinalizeBlock` stages around the EVM-only executor (`sei-protocol#4259`).

Rebased onto current `main`. Regenerated
`sei-tendermint/internal/autobahn/avail/metrics/metrics.gen.go`; it was
already in sync.

## Test plan
- [x] `go test ./giga/metrics/... ./giga/evmonly/...`
- [x] `go test ./internal/autobahn/avail/... ./internal/evmonlyapp/...`
(from `sei-tendermint`)
- [ ] On an Autobahn node, confirm
`sei_chain_autobahn_main_loop_phase_duration_seconds_total`,
`sei_chain_autobahn_storage_tail_phase_duration_seconds_total`, and
`tendermint_internal_autobahn_avail_produced_txs` appear on `/metrics`


Made with [Cursor](https://cursor.com)

Closes CON-426
Closes PLT-1309
Closes PLT-1308

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: FromTheRain <bdchatham@gmail.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: Philip Su <philip@seinetwork.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant