Generic provider #218
Merged
Generic provider #218
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
liamaharon
force-pushed
the
generic-db-provider
branch
2 times, most recently
from
01c9560
to
68ef419
Compare
Benchmark results for
|
| Date (UTC) | 2024-10-23T18:42:52+00:00 |
| Commit | ce433b212cd3b3eb8441279480484c6c0c94dad9 |
| Base SHA | d96e7215483bac0ab145459f4ddaa811d99459d6 |
Significant changes
None
ferranbt
reviewed
Oct 17, 2024
| impl<P, DB> CombinatorContext<P, DB> | ||
| where | ||
| DB: Database + Clone + 'static, | ||
| P: DatabaseProviderFactory<DB> + StateProviderFactory + Clone + 'static, |
There was a problem hiding this comment.
How does this work with Rbuilder as a module? AFAIK It only receives the StateProviderFactory, not DatabaseProviderFactory.
There was a problem hiding this comment.
It is also DatabaseProviderFactory https://github.com/paradigmxyz/reth/blob/661b260f6172c047e8530e2331b2c84141e03c2b/crates/storage/provider/src/traits/full.rs#L14
liamaharon
force-pushed
the
generic-db-provider
branch
from
68ef419
to
fe5d9a0
Compare
liamaharon
force-pushed
the
generic-db-provider
branch
from
fe5d9a0
to
b125994
Compare
dvush
reviewed
Oct 22, 2024
| if !self.testing_mode { | ||
| match check_provider_factory_health(current_block_number, &provider_factory) { | ||
|
|
||
| // Don't need to check consistency for the block that was just checked. |
There was a problem hiding this comment.
I am pretty sure that its true but it can be possible that its not.
The only place that I see that would suffer from too much checking here is nonce cache in sim tree. But even then this check can have a small impact.
I don't think that we should change it now but I marked it to remember to revisit if we see some problems with reopening.
dvush
reviewed
Oct 23, 2024
astarinmymind
pushed a commit
that referenced
this pull request
Nov 1, 2024
Closes #216 Replaces all concrete usage of `ProviderFactory` and `ProviderFactoryReopener` with generics bounded by [reth provider traits](https://github.com/paradigmxyz/reth/blob/661b260f6172c047e8530e2331b2c84141e03c2b/crates/storage/provider/src/traits/full.rs#L14-L24). This allows running rbuilder in-process using a node provider and eases swapping to a more modern provider like `BlockchainProvider` in the future. Note that not all trait methods can be implemented because `ProviderFactoryReopener` has no access to the blockchain tree (pending state etc.). Consistency checks have been moved out of the rbuilder core into the trait implementation for `ProviderFactoryReopener`. Inside the trait implementation, the current head block number is not known, so the reopener cannot assume that the state of the block given - 256 blocks will be available. To not redundantly re-check the consistency of the same block, `ProviderFactoryReopener` tracks the last block verified to have consistent state and will only re-check it when it changes. This adds 1 additional read per provider call to get the head of the chain, however I believe the cost will be insignificant.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #216
Replaces all concrete usage of
ProviderFactoryandProviderFactoryReopenerwith generics bounded by reth provider traits. This allows running rbuilder in-process using a node provider and eases swapping to a more modern provider likeBlockchainProviderin the future.Note that not all trait methods can be implemented because
ProviderFactoryReopenerhas no access to the blockchain tree (pending state etc.).DB Consistency Checks
Consistency checks have been moved out of the rbuilder core into the trait implementation for
ProviderFactoryReopener. Inside the trait implementation, the current head block number is not known, so the reopener cannot assume that the state of the block given - 256 blocks will be available.To not redundantly re-check the consistency of the same block,
ProviderFactoryReopenertracks the last block verified to have consistent state and will only re-check it when it changes. This adds 1 additional read per provider call to get the head of the chain, however I believe the cost will be insignificant.