Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: remove const generics from cursors and add segment masks #5181

Merged
merged 81 commits into from
Oct 30, 2023

Conversation

joshieDo
Copy link
Collaborator

@joshieDo joshieDo commented Oct 25, 2023

PR into #5170

  • Removes const generics from NippyJarCursor and SnapshotCursor.
  • Adds a bunch of helper traits and structs for using SnapshotCursor with masks to query and decode values.
  • Split storage/db/snapshot into different files

Feels way better/more readable to me. Follow-ups are implementing the rest of SnapshotJarProvider providers (after merging the whole PR stack)

Copy link
Collaborator

@shekhirin shekhirin left a comment

Choose a reason for hiding this comment

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

only nits, appreciate the comments. I wonder if there's better way to do that without too much boilerplate, but seems hard without at least rust-lang/rust#29661.

crates/storage/db/src/snapshot/masks.rs Outdated Show resolved Hide resolved
crates/storage/db/src/snapshot/cursor.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm

Base automatically changed from joshie/snap-prov-header to main October 30, 2023 12:07
@codecov
Copy link

codecov bot commented Oct 30, 2023

Codecov Report

Merging #5181 (8ac1426) into main (4dc15c3) will decrease coverage by 0.25%.
Report is 39 commits behind head on main.
The diff coverage is 0.00%.

❗ Current head 8ac1426 differs from pull request most recent head a94a6de. Consider uploading reports for the commit a94a6de to get more accurate results

Impacted file tree graph

Files Coverage Δ
crates/interfaces/src/provider.rs 0.00% <ø> (ø)
crates/primitives/src/transaction/mod.rs 36.75% <ø> (ø)
crates/storage/codecs/src/lib.rs 36.77% <ø> (ø)
crates/storage/nippy-jar/src/error.rs 0.00% <ø> (ø)
crates/storage/provider/src/providers/mod.rs 26.92% <ø> (ø)
crates/storage/db/src/snapshot/mask.rs 0.00% <0.00%> (ø)
crates/stages/src/stages/tx_lookup.rs 2.38% <0.00%> (+0.20%) ⬆️
crates/storage/db/src/snapshot/generation.rs 0.00% <0.00%> (ø)
bin/reth/src/db/snapshots/bench.rs 0.00% <0.00%> (ø)
crates/storage/provider/src/traits/transactions.rs 0.00% <0.00%> (ø)
... and 17 more

... and 4 files with indirect coverage changes

Flag Coverage Δ
integration-tests 25.83% <0.00%> (-0.25%) ⬇️

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

Components Coverage Δ
reth binary 24.91% <0.00%> (-0.87%) ⬇️
blockchain tree 28.46% <ø> (ø)
pipeline 5.07% <0.00%> (+0.02%) ⬆️
storage (db) 28.86% <0.00%> (-1.11%) ⬇️
trie 22.53% <ø> (ø)
txpool 41.11% <ø> (-0.26%) ⬇️
networking 30.88% <ø> (-0.02%) ⬇️
rpc 26.47% <ø> (ø)
consensus 25.06% <ø> (ø)
revm 9.85% <ø> (ø)
payload builder 14.16% <ø> (ø)
primitives 29.01% <0.00%> (-0.16%) ⬇️

@joshieDo joshieDo added this pull request to the merge queue Oct 30, 2023
Merged via the queue into main with commit e73ece9 Oct 30, 2023
22 checks passed
@joshieDo joshieDo deleted the joshie/snap-remove-const branch October 30, 2023 12:35
mattsse pushed a commit that referenced this pull request Nov 8, 2023
Co-authored-by: Alexey Shekhirin <a.shekhirin@gmail.com>
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
None yet
Development

Successfully merging this pull request may close these issues.

3 participants