Skip to content

Comments

feat(storage): read headers and transactions only from static files#18788

Merged
shekhirin merged 14 commits intomainfrom
alexey/static-file-only-read
Oct 6, 2025
Merged

feat(storage): read headers and transactions only from static files#18788
shekhirin merged 14 commits intomainfrom
alexey/static-file-only-read

Conversation

@shekhirin
Copy link
Member

@shekhirin shekhirin commented Sep 30, 2025

Closes #18684

This PR makes all provider methods that read headers or transactions to read only from static files, and never make a lookup in the database. Receipts are not touched yet, as they still can be written to the database.

This doesn't break existing nodes, because on startup we always copy to static files

// The new engine writes directly to static files. This ensures that they're up to the tip.
pipeline.move_to_static_files()?;

Hive passes https://github.com/paradigmxyz/reth/actions/runs/18220958085

@shekhirin shekhirin added the C-enhancement New feature or request label Sep 30, 2025
@shekhirin shekhirin added the A-static-files Related to static files label Sep 30, 2025
@github-project-automation github-project-automation bot moved this to Backlog in Reth Tracker Sep 30, 2025
@shekhirin shekhirin force-pushed the alexey/static-file-only-read branch 5 times, most recently from 6ae5738 to 4e33cd6 Compare September 30, 2025 12:18
@shekhirin shekhirin force-pushed the alexey/static-file-only-read branch from 4e33cd6 to ba3f65d Compare September 30, 2025 12:28
@shekhirin shekhirin force-pushed the alexey/static-file-only-read branch 3 times, most recently from 68f2450 to 36d3a7c Compare October 1, 2025 21:47
@shekhirin shekhirin force-pushed the alexey/static-file-only-read branch from 36d3a7c to 2e509c9 Compare October 1, 2025 21:47
@shekhirin shekhirin force-pushed the alexey/static-file-only-read branch from b80665d to de869c2 Compare October 1, 2025 22:12
@shekhirin shekhirin force-pushed the alexey/static-file-only-read branch from de869c2 to 35c4984 Compare October 1, 2025 22:17
@shekhirin shekhirin marked this pull request as ready for review October 3, 2025 12:15
@shekhirin shekhirin requested a review from joshieDo as a code owner October 3, 2025 12:15
Comment on lines -79 to -80
"reth-db-api",
"reth-db/test-utils",
Copy link
Member

Choose a reason for hiding this comment

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

curious why do we no longer need this?

Copy link
Member Author

Choose a reason for hiding this comment

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

checking static files instead of the database now

Copy link
Member

@yongkangc yongkangc left a comment

Choose a reason for hiding this comment

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

overall lgtm

},
|_| true,
)
self.static_file_provider.transaction_hashes_by_range(tx_range)
Copy link
Member

Choose a reason for hiding this comment

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

with this we can basically do the same thing but without the db?

this seems great, curious why did we not do this before?

Copy link
Member Author

Choose a reason for hiding this comment

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

no good reason tbh, we were writing to both locations for a while now...

@github-project-automation github-project-automation bot moved this from Backlog to In Progress in Reth Tracker Oct 3, 2025
@shekhirin shekhirin force-pushed the alexey/static-file-only-read branch from c1a44f3 to 1322bdd Compare October 3, 2025 13:43
@shekhirin shekhirin force-pushed the alexey/static-file-only-read branch from 1322bdd to 39623de Compare October 3, 2025 13:45
@shekhirin shekhirin requested a review from klkvr October 3, 2025 14:19
Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

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

lgtm, I just have one question

Comment on lines -1143 to -1148
warn!(
target: "provider::static_file",
?segment,
?number,
"Could not find block or tx number on a range request"
);
Copy link
Member

Choose a reason for hiding this comment

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

why do we go from warning and returning an err, to returning an Ok(result) now?

Copy link
Member Author

Choose a reason for hiding this comment

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

we never hit this error because we always query only what we know is inside static files

let end = block_or_tx_range.end.min(static_file_upper_bound + 1);
data.extend(fetch_from_static_file(
self,
block_or_tx_range.start..end,
&mut predicate,
)?);
block_or_tx_range.start = end;

but now we're querying methods such as self.static_file_provider.headers_range directly without checking the ranges first, so I mirror the behaviour of database range querying

@shekhirin shekhirin added this pull request to the merge queue Oct 6, 2025
Merged via the queue into main with commit e9598ba Oct 6, 2025
41 checks passed
@shekhirin shekhirin deleted the alexey/static-file-only-read branch October 6, 2025 11:55
@github-project-automation github-project-automation bot moved this from In Progress to Done in Reth Tracker Oct 6, 2025
CarlBeek added a commit to CarlBeek/reth that referenced this pull request Oct 10, 2025
* main: (280 commits)
  refactor: remove needless collect() calls in trie tests (paradigmxyz#18937)
  chore(grafana): use precompile address as legend (paradigmxyz#18913)
  perf(tree): worker pooling for storage in multiproof generation (paradigmxyz#18887)
  feat: wait for new blocks when build is in progress (paradigmxyz#18831)
  chore: align node_config threshold constant (paradigmxyz#18914)
  docs: duplicate comment in Eip4844PoolTransactionError (paradigmxyz#18858)
  ci: cache hive simulator images to reduce prepare-hive job time (paradigmxyz#18899)
  refactor: replace collect().is_empty() with next().is_none() in tests (paradigmxyz#18902)
  feat(provider): add get_account_before_block to ChangesetReader (paradigmxyz#18898)
  refactor(engine): separate concerns in on_forkchoice_updated for better maintainability (paradigmxyz#18661)
  chore(node): simplify EngineApiExt bounds by removing redundant constraints (paradigmxyz#18905)
  fix(trie): Reveal extension child when extension is last remaining child of a branch (paradigmxyz#18891)
  chore: make clippy happy (paradigmxyz#18900)
  chore: relax `ChainSpec` impls (paradigmxyz#18894)
  refactor: eliminate redundant allocation in precompile cache example (paradigmxyz#18886)
  fix(era-utils): fix off-by-one for Excluded end bound in process_iter (paradigmxyz#18731)
  docs: yellowpaper sections in consensus implementation (paradigmxyz#18881)
  feat(storage): read headers and transactions only from static files (paradigmxyz#18788)
  feat: Use generic `HeaderTy` for `reth db get static-file headers` (paradigmxyz#18870)
  fix: streamline payload conversion in custom engine API (paradigmxyz#18864)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-static-files Related to static files C-enhancement New feature or request

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Read headers and transactions only from static files

4 participants