Skip to content

[giga] remove Snapshot() call in Prepare()#2957

Open
codchen wants to merge 3 commits intomainfrom
tony/prepare-no-snapshot
Open

[giga] remove Snapshot() call in Prepare()#2957
codchen wants to merge 3 commits intomainfrom
tony/prepare-no-snapshot

Conversation

@codchen
Copy link
Collaborator

@codchen codchen commented Feb 23, 2026

Describe your changes and provide context

The Snapshot() call would return a rev number for potential Revert(rev) calls to revert to. However the Snapshot call in Prepare has its rev value discarded, so it isn't useful from a reverting perspective. The other side effects, like creating another nested layer of CacheKV, is also meaningless under the context of EVM (there will just be a layer of CacheKV with zero dirty values). So it can be removed in theory.

Removing it resulted in a 1k increase in theoretical tps (from 20k to 21k)

Testing performed to validate your change

existing tests should still stand

@github-actions
Copy link

github-actions bot commented Feb 23, 2026

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

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedFeb 23, 2026, 2:44 PM

coinbaseEvmAddress: feeCollector,
}
s.Snapshot() // take an initial snapshot for GetCommitted
if k.ShouldUseRegularStore() {
Copy link
Contributor

Choose a reason for hiding this comment

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

eventually for performance/clarity it might be good to have a way to only check these kind of flags initially when setting handlers (like set a s.Snapshot handler at init/creation). This is fine - just thinking out loud.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@stevenlanders i think that change was due to my stale local main. Rebased and this PR is just a one-liner

@codchen codchen force-pushed the tony/prepare-no-snapshot branch from 72c2bc1 to 67efdc2 Compare February 23, 2026 14:42
@codchen codchen force-pushed the tony/prepare-no-snapshot branch from 67efdc2 to dfe9ee6 Compare February 23, 2026 14:43
@codecov
Copy link

codecov bot commented Feb 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.05%. Comparing base (80b9aae) to head (dfe9ee6).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #2957       +/-   ##
===========================================
+ Coverage   57.83%   69.05%   +11.22%     
===========================================
  Files        2110       18     -2092     
  Lines      174123      989   -173134     
===========================================
- Hits       100710      683   -100027     
+ Misses      64451      247    -64204     
+ Partials     8962       59     -8903     
Flag Coverage Δ
sei-chain 69.54% <ø> (+11.73%) ⬆️
sei-db 68.49% <ø> (ø)

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

Files with missing lines Coverage Δ
giga/deps/xevm/state/accesslist.go 90.00% <ø> (-6.08%) ⬇️

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

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.

2 participants