Skip to content

One call to ApplyChangeSets per block#3533

Open
cody-littley wants to merge 4 commits into
mainfrom
cjl/one-diff-per-block
Open

One call to ApplyChangeSets per block#3533
cody-littley wants to merge 4 commits into
mainfrom
cjl/one-diff-per-block

Conversation

@cody-littley
Copy link
Copy Markdown
Contributor

Describe your changes and provide context

Things can be simplified greatly (in future PRs) if we limit calls to ApplyChangeSets() to once per block. Current code calls this method with empty change sets sometimes, and the code required to support this is extremely nontrivial.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 2, 2026

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

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedJun 4, 2026, 5:49 PM

@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 2, 2026

Codecov Report

❌ Patch coverage is 92.15686% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.67%. Comparing base (2fe02f0) to head (bdeb178).

Files with missing lines Patch % Lines
sei-db/state_db/sc/migration/router_builder.go 90.00% 6 Missing ⚠️
sei-cosmos/storev2/rootmulti/store.go 77.77% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3533      +/-   ##
==========================================
- Coverage   59.14%   58.67%   -0.47%     
==========================================
  Files        2218     2171      -47     
  Lines      183100   178267    -4833     
==========================================
- Hits       108290   104599    -3691     
+ Misses      65046    64261     -785     
+ Partials     9764     9407     -357     
Flag Coverage Δ
sei-chain-pr 62.88% <77.77%> (?)
sei-db 70.41% <ø> (ø)
sei-db-state-db ?
sei-db-state-db-pr 76.42% <93.54%> (?)

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

Files with missing lines Coverage Δ
sei-db/state_db/sc/flatkv/store.go 76.94% <ø> (ø)
sei-db/state_db/sc/flatkv/store_apply.go 91.86% <100.00%> (+0.11%) ⬆️
sei-db/state_db/sc/flatkv/store_write.go 77.10% <100.00%> (-1.12%) ⬇️
...ei-db/state_db/sc/migration/accumulating_writer.go 100.00% <100.00%> (ø)
sei-db/state_db/sc/migration/flushing_router.go 100.00% <100.00%> (ø)
sei-cosmos/storev2/rootmulti/store.go 65.21% <77.77%> (+0.12%) ⬆️
sei-db/state_db/sc/migration/router_builder.go 60.59% <90.00%> (+4.89%) ⬆️

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

@cody-littley cody-littley requested a review from blindchaser June 4, 2026 17:48
@cody-littley cody-littley marked this pull request as ready for review June 4, 2026 17:48
@cursor
Copy link
Copy Markdown

cursor Bot commented Jun 4, 2026

PR Summary

High Risk
Changes the core block commit and state-commit path (AppHash, flatKV LtHash, migration boundary advancement); regressions could cause validator divergence or incorrect migration progress.

Overview
This PR locks in one successful ApplyChangeSets per commit cycle end-to-end, instead of allowing repeated or empty follow-up applies that were hard to reason about (especially under migration and LtHash).

Rootmulti adds a flushedVersion memo so flush() forwards change sets to the SC store at most once per pending version. That matters because both GetWorkingHash() and Commit() call flush(); without the memo, the second call would send an empty apply, hit flatKV’s guard, and in migration modes could advance boundaries twice or diverge AppHash from what Tendermint already saw. The memo resets on load and rollback.

FlatKV adds appliedSinceCommit: a second ApplyChangeSets without an intervening Commit is rejected. Tests that relied on multiple applies per block are removed or rewritten to coalesce all block changes into one call.

Migration routing introduces accumulatingWriter (buffer route-level applies, Flush once to the leaf backend) and flushingRouter (run those flushes after a successful dispatch). Migration router builders share one accumulator per memIAVL/flatKV backend so fan-out from multiple routes still yields one physical write per backend per top-level apply. Composite tests that asserted “second apply in the same block must not advance migration” are dropped in favor of rootmulti-level coverage.

New tests assert exactly one SC ApplyChangeSets per block (including empty blocks) across migrate_evm, evm_migrated, and memiavl_only configs.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant