-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix: Set Era pipeline stage to last checkpoint when there is no target #19000
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
Conversation
do you mean that after this fix, it would be much easier to test the trieset changes against main? @mediocregopher |
mattsse
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice find
| }; | ||
|
|
||
| Ok(ExecOutput { checkpoint: StageCheckpoint::new(height), done: height == input.target() }) | ||
| Ok(ExecOutput { checkpoint: StageCheckpoint::new(height), done: height >= input.target() }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense
| // It's possible for a pipeline sync to be executed with a None target, e.g. after a | ||
| // stage was manually dropped, and `reth node` is then called without a `--debug.tip`. | ||
| // | ||
| // In this case we don't want to simply default to zero, as that would overwrite the | ||
| // previously stored checkpoint block number. Instead we default to that previous | ||
| // checkpoint. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this checks out
Now that I've figured this issue out I could potentially work around it, but yes with this fix in using reth-bench-compare on the trie changesets PR is much more straightforward |
* docs: yellowpaper sections in consensus implementation (paradigmxyz#18881) * fix(era-utils): fix off-by-one for Excluded end bound in process_iter (paradigmxyz#18731) Co-authored-by: Roman Hodulák <[email protected]> * refactor: eliminate redundant allocation in precompile cache example (paradigmxyz#18886) * chore: relax `ChainSpec` impls (paradigmxyz#18894) * chore: make clippy happy (paradigmxyz#18900) * fix(trie): Reveal extension child when extension is last remaining child of a branch (paradigmxyz#18891) * chore(node): simplify EngineApiExt bounds by removing redundant constraints (paradigmxyz#18905) * refactor(engine): separate concerns in on_forkchoice_updated for better maintainability (paradigmxyz#18661) Co-authored-by: Nathaniel Bajo <[email protected]> Co-authored-by: YK <[email protected]> Co-authored-by: Brian Picciano <[email protected]> * feat(provider): add get_account_before_block to ChangesetReader (paradigmxyz#18898) * refactor: replace collect().is_empty() with next().is_none() in tests (paradigmxyz#18902) Co-authored-by: Alexey Shekhirin <[email protected]> * ci: cache hive simulator images to reduce prepare-hive job time (paradigmxyz#18899) * docs: duplicate comment in Eip4844PoolTransactionError (paradigmxyz#18858) * chore: align node_config threshold constant (paradigmxyz#18914) * feat: wait for new blocks when build is in progress (paradigmxyz#18831) Co-authored-by: Roman Hodulák <[email protected]> * perf(tree): worker pooling for storage in multiproof generation (paradigmxyz#18887) Co-authored-by: Brian Picciano <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Alexey Shekhirin <[email protected]> * chore(grafana): use precompile address as legend (paradigmxyz#18913) * refactor: remove needless collect() calls in trie tests (paradigmxyz#18937) * fix(examples): change method to launch with debug capabilities (paradigmxyz#18946) * fix(example): launch with debug capabilities (paradigmxyz#18947) * fix(testsuite): Fix unused updates in e2e-test-utils (paradigmxyz#18953) * fix(payload): correct Debug label for PayloadTimestamp in PayloadServiceCommand (paradigmxyz#18954) * chore(rpc): Moves `SequencerMetrics` into `reth-optimism-rpc` (paradigmxyz#18921) * refactor: replace println! with structured logging in test_vectors (paradigmxyz#18956) * refactor(cli): use structured logging (tracing) in p2p command (paradigmxyz#18957) * perf(tree): add elapsed time to parallel state root completion log (paradigmxyz#18959) * fix(trie): Properly upsert into StoragesTrie in repair-trie (paradigmxyz#18941) * fix: misleading error message in db list: show actual table name (paradigmxyz#18896) * fix: remove noisy stderr prints in ERA1 cleanup (EraClient::delete_outside_range) (paradigmxyz#18895) * fix: use max B256 for upper bound in empty-storage check (paradigmxyz#18962) * ci: remove reproducible build from release.yml (paradigmxyz#18958) * chore(rpc): Remove redundant U256::from in suggested_priority_fee (paradigmxyz#18969) * chore(ci): update eest 7594 issue link in hive expected failures file (paradigmxyz#18976) * perf(tests): remove redundant format! in ef-tests run_only (paradigmxyz#18909) * feat(cli): enable traces export via `tracing-otlp` cli arg (paradigmxyz#18242) Co-authored-by: Dan Cline <[email protected]> * feat: allow otlp level to be configurable (paradigmxyz#18981) * chore(optimism): remove unnecessary Debug bounds from header generics (paradigmxyz#18989) * refactor: convert satisfy_base_fee_ids to use closure (paradigmxyz#18979) * chore: bump otlp crates (paradigmxyz#18984) * fix(network): prevent metric leak in outgoing message queue on session teardown (paradigmxyz#18847) * chore: remove unused imports in blockchain_provider (paradigmxyz#18867) Co-authored-by: Matthias Seitz <[email protected]> * fix(stateless): enforce BLOCKHASH ancestor header limit (paradigmxyz#18920) * chore(evm): mark ExecuteOutput as unused and slated for removal (paradigmxyz#18754) * refactor: unify `Pipeline` creation codepaths (paradigmxyz#18955) * fix(engine): flatten storage cache (paradigmxyz#18880) * perf(tree): worker pooling for account proofs (paradigmxyz#18901) Co-authored-by: Brian Picciano <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Alexey Shekhirin <[email protected]> * refactor(storage): fix ChainStateKey enum variant name (paradigmxyz#18992) * refactor(trie): remove proof task manager (paradigmxyz#18934) Co-authored-by: Brian Picciano <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Alexey Shekhirin <[email protected]> * fix: required optimism primitives features in db-api (paradigmxyz#19005) * refactor(engine): simplify InvalidBlockWitnessHook::on_invalid_block for better testability (paradigmxyz#18696) * chore: replace poll_next_unpin loop with poll_recv_many (paradigmxyz#18978) * fix: Set Era pipeline stage to last checkpoint when there is no target (paradigmxyz#19000) * ci: Add tests for Paris scenario in hive.yml (paradigmxyz#19013) * chore: bump book timeout (paradigmxyz#19016) * feat: add metrics for safe and finalized block heights (paradigmxyz#18987) * chore(privitives-traits): remove unused serde derives and camelCase attribute (paradigmxyz#19014) * chore: refactor loop in `add_new_transactions` (paradigmxyz#19006) * chore(ci): bump hive eest to v5.3.0 (paradigmxyz#19021) * feat(devp2p): make eth p2p networkId configurable (paradigmxyz#19020) Co-authored-by: frankudoags <frankudoags.com> * chore: remove unused Args struct from exex-subscription example (paradigmxyz#19019) * feat: add pending sequence as pub (paradigmxyz#19022) * chore: bump alloy-core (paradigmxyz#19026) * fix: unused warnings for tracing (paradigmxyz#19025) * fix: respect cli blob size setting (paradigmxyz#19024) * feat(engine): deprecate TestPipelineBuilder::with_executor_results (paradigmxyz#19017) * perf: background init of workers (paradigmxyz#19012) * chore(ci): update expected failures (paradigmxyz#19034) * fix: use header type generic for mask (paradigmxyz#19037) * fix: correct `Compact` impl for `Option` (paradigmxyz#19042) * chore: increase versioned hash index cache (paradigmxyz#19038) * chore(primitives-traits): relax SignerRecoverable bounds for Extended<B,T> (paradigmxyz#19045) * feat: bump revm (paradigmxyz#18999) * fix: resolve upstream merge Signed-off-by: Gregory Edison <[email protected]> * bump revm * update deps and fix lints * update openvm deps * skip exex wal storage test * pin revm tag scroll-v91 * bump openvm compat --------- Signed-off-by: Gregory Edison <[email protected]> Co-authored-by: josé v <[email protected]> Co-authored-by: Forostovec <[email protected]> Co-authored-by: Roman Hodulák <[email protected]> Co-authored-by: Skylar Ray <[email protected]> Co-authored-by: Arsenii Kulikov <[email protected]> Co-authored-by: Léa Narzis <[email protected]> Co-authored-by: Brian Picciano <[email protected]> Co-authored-by: radik878 <[email protected]> Co-authored-by: William Nwoke <[email protected]> Co-authored-by: Nathaniel Bajo <[email protected]> Co-authored-by: YK <[email protected]> Co-authored-by: Dan Cline <[email protected]> Co-authored-by: Merkel Tranjes <[email protected]> Co-authored-by: Alexey Shekhirin <[email protected]> Co-authored-by: Federico Gimenez <[email protected]> Co-authored-by: stevencartavia <[email protected]> Co-authored-by: emmmm <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: MIHAO PARK <[email protected]> Co-authored-by: Tilak Madichetti <[email protected]> Co-authored-by: Emilia Hane <[email protected]> Co-authored-by: maradini77 <[email protected]> Co-authored-by: sashaodessa <[email protected]> Co-authored-by: Alvarez <[email protected]> Co-authored-by: MozirDmitriy <[email protected]> Co-authored-by: drhgencer <[email protected]> Co-authored-by: Matthias Seitz <[email protected]> Co-authored-by: anim001k <[email protected]> Co-authored-by: Julian Meyer <[email protected]> Co-authored-by: Karl Yu <[email protected]> Co-authored-by: Jennifer <[email protected]> Co-authored-by: Ivan Wang <[email protected]> Co-authored-by: GarmashAlex <[email protected]> Co-authored-by: Udoagwa Franklin <[email protected]> Co-authored-by: Luca Provini <[email protected]> Co-authored-by: Galoretka <[email protected]> Co-authored-by: sashass1315 <[email protected]> Co-authored-by: frisitano <[email protected]>
I observed the following behavior on a node which was started without a
--debug.tip, after having manually dropped a stage:As shown, the Era stage was setting its checkpoint to zero. This was occurring because when the target has already been reached Era immediately returns ready from
poll_execute_ready:reth/crates/stages/stages/src/stages/era.rs
Lines 149 to 151 in 5065890
(
target_reachedassumes a zero target block number if the target field is None.)At this point
self.itemis None. Whenexecuteis then called Era has no work to do, but was returning the target (defaulting again to zero) rather than previous checkpoint:reth/crates/stages/stages/src/stages/era.rs
Line 214 in 5065890
If a stage was then manually dropped again then both Era (the first stage) and Finish (the last stage) would be zero. This check in
check_pipeline_consistencywould never hit, and no pipeline sync would be reran:reth/crates/node/builder/src/launch/common.rs
Lines 930 to 942 in cec30cd
Since the Finish stage is used to determine current block height generally, the node would then assume it has no blocks.