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: get BlockMeta table values from static file or database #13844

Merged
merged 1 commit into from
Jan 20, 2025

Conversation

joshieDo
Copy link
Collaborator

@joshieDo joshieDo commented Jan 17, 2025

Uses get_with_static_file_or_database and get_range_with_static_file_or_database in provider implementations for Ommers, Withdrawals and BlockBodyIndices.

Since the block meta static file isn't being written to yet, it shouldn't have any meaningful effect. Just spreading out the change into smaller PRs.

@joshieDo joshieDo added C-enhancement New feature or request A-db Related to the database A-static-files Related to static files labels Jan 17, 2025
@joshieDo joshieDo requested review from mattsse and klkvr January 17, 2025 14:21
@joshieDo joshieDo force-pushed the joshie/providersmeta branch from 8589272 to a5080e2 Compare January 17, 2025 14:22
@@ -171,7 +173,7 @@ where
let ommers = if chain_spec.final_paris_total_difficulty(header.number).is_some() {
Vec::new()
} else {
ommers_cursor.seek_exact(header.number)?.map(|(_, o)| o.ommers).unwrap_or_default()
provider.ommers(header.number.into())?.unwrap_or_default()
Copy link
Collaborator Author

@joshieDo joshieDo Jan 17, 2025

Choose a reason for hiding this comment

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

smol perf hit since it's no longer using a cursor, but since it's just ommers i'd be okay with it. The alternative is to implement Provider::ommers_range.

I do plan to add withdrawals_range since it's actually used today

@joshieDo joshieDo changed the title feat: get BlockMeta tables from static file or database feat: get BlockMeta table values from static file or database Jan 17, 2025
Provider: DBProvider + ChainSpecProvider<ChainSpec: EthereumHardforks>,
Provider: DBProvider
+ ChainSpecProvider<ChainSpec: EthereumHardforks>
+ OmmersProvider<Header = Header>,
Copy link
Collaborator Author

@joshieDo joshieDo Jan 17, 2025

Choose a reason for hiding this comment

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

necessary since BlockBody expects Vec<Header> as Ommers

@@ -562,14 +562,29 @@ impl<N: ProviderNodeTypes> BlockBodyIndicesProvider for ProviderFactory<N> {
&self,
number: BlockNumber,
) -> ProviderResult<Option<StoredBlockBodyIndices>> {
self.provider()?.block_body_indices(number)
self.static_file_provider.get_with_static_file_or_database(
Copy link
Member

Choose a reason for hiding this comment

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

this should still delegate to the self.provider() I think? given that it already does get_with_static_file_or_database internally

Copy link
Collaborator Author

@joshieDo joshieDo Jan 20, 2025

Choose a reason for hiding this comment

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

this way a database transaction wont get opened unless truly necessary

@joshieDo joshieDo added this pull request to the merge queue Jan 20, 2025
Merged via the queue into main with commit f527b5a Jan 20, 2025
43 checks passed
@joshieDo joshieDo deleted the joshie/providersmeta branch January 20, 2025 11:42
refcell pushed a commit to ethereum-optimism/op-reth that referenced this pull request Jan 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-db Related to the database 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.

2 participants