fix(era-utils): fix off-by-one for Excluded end bound in process_iter#18731
Conversation
mattsse
left a comment
There was a problem hiding this comment.
unsure if this is right
@RomanHodulak wdyt?
|
@mattsse Upon checking this, the Like this: let mut last_header_number = match block_numbers.start_bound() {
Bound::Included(&number) => number,
Bound::Excluded(&number) => number.saturating_sub(1),
Bound::Unbounded => 0,
};
let target = match block_numbers.end_bound() {
Bound::Included(&number) => Some(number),
Bound::Excluded(&number) => Some(number.saturating_add(1)),
Bound::Unbounded => None,
};Should be: let mut last_header_number = match block_numbers.start_bound() {
Bound::Included(&number) => number,
Bound::Excluded(&number) => number.saturating_add(1),
Bound::Unbounded => 0,
};
let target = match block_numbers.end_bound() {
Bound::Included(&number) => Some(number),
Bound::Excluded(&number) => Some(number.saturating_sub(1)),
Bound::Unbounded => None,
};Because if you exclude a starting number, you have to start above the number and if you exclude a target number you have to stop below it. We never encountered this because we haven't used the This PR does not fix the |
Thanks for review |
* main: (280 commits) refactor: remove needless collect() calls in trie tests (paradigmxyz#18937) chore(grafana): use precompile address as legend (paradigmxyz#18913) perf(tree): worker pooling for storage in multiproof generation (paradigmxyz#18887) feat: wait for new blocks when build is in progress (paradigmxyz#18831) chore: align node_config threshold constant (paradigmxyz#18914) docs: duplicate comment in Eip4844PoolTransactionError (paradigmxyz#18858) ci: cache hive simulator images to reduce prepare-hive job time (paradigmxyz#18899) refactor: replace collect().is_empty() with next().is_none() in tests (paradigmxyz#18902) feat(provider): add get_account_before_block to ChangesetReader (paradigmxyz#18898) refactor(engine): separate concerns in on_forkchoice_updated for better maintainability (paradigmxyz#18661) chore(node): simplify EngineApiExt bounds by removing redundant constraints (paradigmxyz#18905) fix(trie): Reveal extension child when extension is last remaining child of a branch (paradigmxyz#18891) chore: make clippy happy (paradigmxyz#18900) chore: relax `ChainSpec` impls (paradigmxyz#18894) refactor: eliminate redundant allocation in precompile cache example (paradigmxyz#18886) fix(era-utils): fix off-by-one for Excluded end bound in process_iter (paradigmxyz#18731) docs: yellowpaper sections in consensus implementation (paradigmxyz#18881) feat(storage): read headers and transactions only from static files (paradigmxyz#18788) feat: Use generic `HeaderTy` for `reth db get static-file headers` (paradigmxyz#18870) fix: streamline payload conversion in custom engine API (paradigmxyz#18864) ...
…paradigmxyz#18731) Co-authored-by: Roman Hodulák <roman.hodulak@polyglot-software.com>
* 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 <roman.hodulak@polyglot-software.com> * 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 <nathanielbajo@Nathaniels-MacBook-Pro.local> Co-authored-by: YK <chiayongkang@hotmail.com> Co-authored-by: Brian Picciano <me@mediocregopher.com> * 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 <5773434+shekhirin@users.noreply.github.com> * 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 <roman.hodulak@polyglot-software.com> * perf(tree): worker pooling for storage in multiproof generation (paradigmxyz#18887) Co-authored-by: Brian Picciano <me@mediocregopher.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> * 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 <6798349+Rjected@users.noreply.github.com> * 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 <matthias.seitz@outlook.de> * 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 <me@mediocregopher.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> * refactor(storage): fix ChainStateKey enum variant name (paradigmxyz#18992) * refactor(trie): remove proof task manager (paradigmxyz#18934) Co-authored-by: Brian Picciano <me@mediocregopher.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> * 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 <gregory.edison1993@gmail.com> * 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 <gregory.edison1993@gmail.com> Co-authored-by: josé v <52646071+Peponks9@users.noreply.github.com> Co-authored-by: Forostovec <ilonaforostovec22@gmail.com> Co-authored-by: Roman Hodulák <roman.hodulak@polyglot-software.com> Co-authored-by: Skylar Ray <137945430+sky-coderay@users.noreply.github.com> Co-authored-by: Arsenii Kulikov <klkvrr@gmail.com> Co-authored-by: Léa Narzis <78718413+lean-apple@users.noreply.github.com> Co-authored-by: Brian Picciano <me@mediocregopher.com> Co-authored-by: radik878 <radikpadik76@gmail.com> Co-authored-by: William Nwoke <willteey@gmail.com> Co-authored-by: Nathaniel Bajo <nathanielbajo@Nathaniels-MacBook-Pro.local> Co-authored-by: YK <chiayongkang@hotmail.com> Co-authored-by: Dan Cline <6798349+Rjected@users.noreply.github.com> Co-authored-by: Merkel Tranjes <140164174+rnkrtt@users.noreply.github.com> Co-authored-by: Alexey Shekhirin <5773434+shekhirin@users.noreply.github.com> Co-authored-by: Federico Gimenez <fgimenez@users.noreply.github.com> Co-authored-by: stevencartavia <112043913+stevencartavia@users.noreply.github.com> Co-authored-by: emmmm <155267286+eeemmmmmm@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: MIHAO PARK <wetkeyboard17@gmail.com> Co-authored-by: Tilak Madichetti <tilak.madichetti@gmail.com> Co-authored-by: Emilia Hane <elsaemiliaevahane@gmail.com> Co-authored-by: maradini77 <140460067+maradini77@users.noreply.github.com> Co-authored-by: sashaodessa <140454972+sashaodessa@users.noreply.github.com> Co-authored-by: Alvarez <140459501+prestoalvarez@users.noreply.github.com> Co-authored-by: MozirDmitriy <dmitriymozir@gmail.com> Co-authored-by: drhgencer <gancer16@gmail.com> Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de> Co-authored-by: anim001k <140460766+anim001k@users.noreply.github.com> Co-authored-by: Julian Meyer <julian.meyer@coinbase.com> Co-authored-by: Karl Yu <43113774+0xKarl98@users.noreply.github.com> Co-authored-by: Jennifer <jenpaff0@gmail.com> Co-authored-by: Ivan Wang <314130948@qq.com> Co-authored-by: GarmashAlex <garmasholeksii@gmail.com> Co-authored-by: Udoagwa Franklin <54338168+frankudoags@users.noreply.github.com> Co-authored-by: Luca Provini <luca.provini@usemerkle.com> Co-authored-by: Galoretka <galoretochka@gmail.com> Co-authored-by: sashass1315 <sashass1315@gmail.com> Co-authored-by: frisitano <francesco.risitano95@gmail.com>
Previously, process_iter computed target = end + 1 for Bound::Excluded(end) and stopped on number > target, which allowed processing up to end + 1 and was incorrect, especially at u64::MAX. This change standardizes to an exclusive end bound: Included(end) becomes end.checked_add(1) (None on overflow), Excluded(end) becomes Some(end), and the loop exits on number >= end_exclusive. This fixes the off-by-one and aligns range handling with the rest of the codebase.