Skip to content

feat: basic staged sync crate#6

Merged
onbjerg merged 7 commits intomasterfrom
onbjerg/ssync-crate
Oct 3, 2022
Merged

feat: basic staged sync crate#6
onbjerg merged 7 commits intomasterfrom
onbjerg/ssync-crate

Conversation

@onbjerg
Copy link
Collaborator

@onbjerg onbjerg commented Sep 29, 2022

Basic crate intended to house staged syncing primitives (stage trait etc.) for use in reth stages

Some details are left out for now (for example stages take a &mut dyn Transaction) since it's an unknown quantity to me what the database abstraction will look like

Additionally I've left out the impl of Pipeline::run for a later PR

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

using U64 here since BlockId in the primitives crate is either a hash or a number (we just need a number) and BlockNumber is either a tag or a number (and we just need a number)

Copy link
Collaborator

Choose a reason for hiding this comment

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

We should probably start adding our own type in primitive crate, there is a difference between types for RPC and types for execution and I am not sure how much of them can be reused other than for jsonrpc. Can be done later

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Left out pruning stuff since we talked about seeing if it could fit in other components that are "closer" to the database (e.g. the database layer itself)

Copy link
Member

Choose a reason for hiding this comment

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

a bit sensitive of the dynamic dispatch here but maybe it's ok

Comment on lines 8 to 9
Copy link
Member

Choose a reason for hiding this comment

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

How is unwind priority determined? In the opposite order of normal stage priority?

What does require_tip do?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looking at silkworm it seems two stages switched place on unwind. I am not sure if this is silkworm specific:
https://github.com/torquem-ch/silkworm/blob/3aedb286efe536062c9ffb4b5f5de9856866abab/node/silkworm/stagedsync/sync_loop.cpp#L88-L116

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How is unwind priority determined? In the opposite order of normal stage priority?

We have to set it manually. Essentially stages may unwind in a different order than they are executed, and some stages may explicitly require unwinding before others and this helps that. Unwind priority works like this in Akula:

  • Unwind priority > 0: Higher unwind priority are unwound first
  • Unwind priority = 0: Unwound in the order that they were executed

An example:

Stage A (priority 0)
Stage B (priority 0)
Stage C (priority 2)
Stage D (priority 1)

Unwind order: Stage C, Stage D, Stage A, Stage B

I'm wondering if there is a better way to encapsulate the special handling of priority 0 though since it is a bit opaque

What does require_tip do?

A stage that has that flag set needs us to reach the tip of the chain before it will execute, otherwise it is skipped and the next stage is performed (permitted it also does not require the tip). My understanding is that currently the stages in Akula that require reaching the tip are every stage after the execution stage, but only if pruning is not enabled.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added some more docs in the crate that hopefully clarifies it, otherwise I'm OK with adding more.

Comment on lines 82 to 83
Copy link
Member

Choose a reason for hiding this comment

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

Does unwinding mean: I am at block 10 and unwind_to is Some(5), it will undo blocks 10, 9 ... 5 in that order? It's like canceling each stage and undoing its progress? Do we understand when this happens?

Copy link
Collaborator Author

@onbjerg onbjerg Oct 3, 2022

Choose a reason for hiding this comment

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

Yes that is correct. It happens whenever a stage encounters some sort of validation failure (e.g. invalid hash, we're on the wrong fork, state is invalid etc etc), so essentially any time ExecOutput::Unwind is returned

@gakonst gakonst force-pushed the onbjerg/ssync-crate branch from 3967724 to 7e6f228 Compare September 30, 2022 21:20
/// The unwind priority is set with [Pipeline::push_with_unwind_priority].
#[derive(Default)]
pub struct Pipeline {
stages: Vec<QueuedStage>,
Copy link
Contributor

Choose a reason for hiding this comment

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

shall we use VecDeque here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Don't think that would make sense since we never remove stages from here, and we only access in either a forwards fashion or sorted by unwind priority

@onbjerg onbjerg merged commit 6868dda into master Oct 3, 2022
@onbjerg onbjerg deleted the onbjerg/ssync-crate branch October 3, 2022 12:40
clabby added a commit to clabby/reth that referenced this pull request Aug 13, 2023
Resolution checkpoint

Resolution checkpoint paradigmxyz#2

Resolution checkpoint paradigmxyz#3

x

Resolution checkpoint paradigmxyz#4

Resolution checkpoint paradigmxyz#5

Resolution checkpoint paradigmxyz#6

Resolution checkpoint paradigmxyz#7

Resolution checkpoint paradigmxyz#8

Resolve checkpoint paradigmxyz#9 (transaction primitive)

Resolve checkpoint paradigmxyz#10 (rpc api transactions)

Resolve checkpoint paradigmxyz#11 (building w/o feature flag)

Start review

Compiling with and without `optimism` feature flag

Remove `DepositTx` from txpool mock tests, they never go into the txpool

fmt

code lint

fix signature tests

Co-authored-by: nicolas <48695862+merklefruit@users.noreply.github.com>

Use free CI runners (revert before upstream)

Co-authored-by: refcell <abigger87@gmail.com>

Signature test fixes

Co-authored-by refcell <abigger87@gmail.com>

Fix Receipt proptest

Co-authored-by BB <brian.t.bland@gmail.com>

lint

Fix variable-length compact for txtype/transaction

Co-authored-by: Brian Bland <brian.t.bland@gmail.com>

Fix basefee tests

Remove unnecessary rpc deps

Co-authored-by: Brian Bland <brian.t.bland@gmail.com>
Co-authored-by: refcell <abigger87@gmail.com>
Co-authored-by: nicolas <48695862+merklefruit@users.noreply.github.com>
Co-authored-by: Roberto <bayardo@alum.mit.edu>
clabby added a commit to clabby/reth that referenced this pull request Aug 13, 2023
Resolution checkpoint

Resolution checkpoint paradigmxyz#2

Resolution checkpoint paradigmxyz#3

x

Resolution checkpoint paradigmxyz#4

Resolution checkpoint paradigmxyz#5

Resolution checkpoint paradigmxyz#6

Resolution checkpoint paradigmxyz#7

Resolution checkpoint paradigmxyz#8

Resolve checkpoint paradigmxyz#9 (transaction primitive)

Resolve checkpoint paradigmxyz#10 (rpc api transactions)

Resolve checkpoint paradigmxyz#11 (building w/o feature flag)

Start review

Compiling with and without `optimism` feature flag

Remove `DepositTx` from txpool mock tests, they never go into the txpool

fmt

code lint

fix signature tests

Co-authored-by: nicolas <48695862+merklefruit@users.noreply.github.com>

Use free CI runners (revert before upstream)

Co-authored-by: refcell <abigger87@gmail.com>

Signature test fixes

Co-authored-by refcell <abigger87@gmail.com>

Fix Receipt proptest

Co-authored-by BB <brian.t.bland@gmail.com>

lint

Fix variable-length compact for txtype/transaction

Co-authored-by: Brian Bland <brian.t.bland@gmail.com>

Fix basefee tests

Remove unnecessary rpc deps

Co-authored-by: Brian Bland <brian.t.bland@gmail.com>
Co-authored-by: refcell <abigger87@gmail.com>
Co-authored-by: nicolas <48695862+merklefruit@users.noreply.github.com>
Co-authored-by: Roberto <bayardo@alum.mit.edu>
anonymousGiga pushed a commit to anonymousGiga/reth that referenced this pull request Feb 20, 2024
* add feature 'enable_execution_duration_record'

* set 'enable_execution_duration_record' feature for util mod
AshinGau added a commit to AshinGau/reth that referenced this pull request Sep 11, 2024
…xyz#6)

Separate the cache layer for scheduler and partition, and implement fall
back to sequential execute.
AshinGau added a commit to AshinGau/reth that referenced this pull request Oct 13, 2024
…xyz#6)

Separate the cache layer for scheduler and partition, and implement fall
back to sequential execute.
greged93 referenced this pull request in greged93/reth Nov 8, 2024
* fix: adjust ci workflows

* ci: pin clippy to working version (paradigmxyz#11237)

* ci: pin clippy to working version (paradigmxyz#11401)

* fix integration tests

---------

Co-authored-by: Federico Gimenez <fgimenez@users.noreply.github.com>
Co-authored-by: nk_ysg <nk_ysg@163.com>
anitaltius pushed a commit to Altius-Labs/1.2.2_db that referenced this pull request May 17, 2025
AnilChinchawale added a commit to AnilChinchawale/reth that referenced this pull request Mar 2, 2026
…unwind loop

- Documented P0 bug: Execution stage unwinds to 0 after ~500K blocks
- Pipeline successfully syncs Headers/Bodies/SenderRecovery
- Execution starts, reaches ~500K, then unwinds and restarts
- Added investigation steps and potential causes
AnilChinchawale added a commit to AnilChinchawale/reth that referenced this pull request Mar 2, 2026
…aradigmxyz#7 slow re-download

- Issue paradigmxyz#6 downgraded P0→P3: bad FCU feeder was the cause (all 3 hashes same)
- Fixed reth-fcu-feeder.py to use genesis as finalizedBlockHash
- Pipeline completes ALL 13 stages with single FCU (1M+ blocks!)
- Added Issue paradigmxyz#7: reverse headers downloader extremely slow after restart
- Updated status and strategy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants