feat: run StaticFileProvider::check_consistency on start up
#8143
Conversation
joshieDo
added
A-db
| loop { | ||
| if let Some(indices) = provider.block_body_indices(last_block)? { | ||
| if indices.last_tx_num() <= highest_tx { | ||
| break | ||
| } | ||
| } | ||
| if last_block == 0 { | ||
| break | ||
| } | ||
| last_block -= 1; | ||
|
|
||
| highest_block = Some(last_block); | ||
| update_unwind_target(highest_block); | ||
| } |
There was a problem hiding this comment.
can we move this to static file manager, so that we can re-use it in stages as well?
| ) -> ProviderResult<Option<PipelineTarget>> { | ||
| let mut unwind_target: Option<BlockNumber> = None; | ||
| let mut update_unwind_target = |new_target: Option<BlockNumber>| { | ||
| let new_target = new_target.unwrap_or_default(); |
There was a problem hiding this comment.
This unwrap_or_default seems correct, because we need to unwind to 0 if we pass None as a new target. I would prefer passing just the BlockNumber here instead, and do the unwrap_or_default() only in the call where None is possible.
| let static_file_entry = static_file_entry.unwrap_or_default(); | ||
| let highest_static_block = highest_static_block.unwrap_or_default(); |
There was a problem hiding this comment.
is it correct that if we don't even have a genesis block in the segment, we unwrap these to 0?
| @@ -138,6 +138,11 @@ impl StaticFileSegment { | |||
| pub fn is_headers(&self) -> bool { | |||
| matches!(self, StaticFileSegment::Headers) | |||
| } | |||
|
|
|||
| /// Returns `true` if the segment is `StaticFileSegment::Receipts`. | |||
| pub fn is_receipts(&self) -> bool { | |||
There was a problem hiding this comment.
| pub fn is_receipts(&self) -> bool { | |
| pub const fn is_receipts(&self) -> bool { |
| /// A [BlockExecutorProvider] implementation that does nothing. | ||
| #[derive(Debug, Default, Clone)] | ||
| #[non_exhaustive] | ||
| pub struct NoopBlockExecutorProvider; |
There was a problem hiding this comment.
can we move this to noop.rs
| /// Only used for NoopBlockExecutorProvider | ||
| #[error("execution unavailable for noop")] | ||
| UnavailableForNoop, |
There was a problem hiding this comment.
we don't need this, we should use the Other variant instead
| #[cfg(any(test, feature = "test-utils"))] | ||
| /// Helper function to override block range for testing. |
| #[cfg(any(test, feature = "test-utils"))] | ||
| /// Helper function to override block range for testing. |
| { | ||
| /// Returns the [ProviderFactory] for the attached database. | ||
| pub fn create_provider_factory(&self) -> eyre::Result<ProviderFactory<DB>> { | ||
| pub async fn create_provider_factory(&self) -> eyre::Result<ProviderFactory<DB>> { |
There was a problem hiding this comment.
this now needs more docs that describe what this does
| has_receipt_pruning: bool, | ||
| read_only: bool, |
There was a problem hiding this comment.
I'd like to use a new type for this, in case we need more settings
Introduces a function to check the overall consistency between static files and database:
reth/crates/storage/provider/src/providers/static_file/manager.rs
Lines 476 to 481 in ea0ef7f
This will be called on starting up the node, before any data is queried from storage. If it fails the check, it will return a
PipelineTarget::Unwind(N)that will be straight away handled by a standalone unwind-only pipeline.The tests live in
crates/stages/mod.rsbecause there are other necessary test functions there and can not be imported into providers crate. Follow-up will refactor the test-utils in stages crate and move them to providers crate.Pending live testing - but open to review