bug(l1): wrong logic when validating peer ForkID #1685
Assignees
Milestone
Comments
fmoletta
added
bug
This was referenced Jan 21, 2025
github-merge-queue bot
pushed a commit
that referenced
this issue
Jan 21, 2025
**Motivation** We are currently checking whether the received fork ID is exactly the same as ours. Instead, we should follow the validation rules specified in https://eips.ethereum.org/EIPS/eip-2124. **Description** This PR adds a proper validation to the received fork id. Closes #1685 --------- Co-authored-by: fmoletta <99273364+fmoletta@users.noreply.github.com> Co-authored-by: Tomás Arjovsky <t.arjovsky@gmail.com>
github-merge-queue bot
pushed a commit
that referenced
this issue
Jan 29, 2025
**Motivation**
This PR introduces the following upgrades for snap-sync:
- Use DB-persisted checkpoints so we can persist the sync progress
throughout restarts & cycles
- Stop ForckChoices & NewPayloads being applied while syncing
- Improved handling of stale pivot during sub-processes
- Improved handling of pending requests when aborting due to stale pivot
- Fetching of large storage tries (that don't fit in a single range
request)
- Safer (but a bit slower) healing that can be restarted
- Faster storage fetching (multiple parallel fetches)
And also simplifies it by removing the following logic:
- No longer downloads bodies and receipts for blocks before the pivot
during snap sync (WARNING: this goes against the spec but shouldn't be a
problem for the time being)
- Removes restart from latest block when latest - 64 becomes stale. (By
this point it is more effective to wait for the next fork choice update)
- Periodically shows state sync progress
<!-- Why does this pull request exist? What are its goals? -->
**Description**
- Stores the last downloaded block's hash in the DB during snap sync to
serve as a checkpoint if the sync is aborted halfway (common case when
syncing from genesis). This checkpoint is cleared upon succesful snap
sync.
- No longer fetches receipts or block bodies past the pivot block during
snap sync
- Add method `sync_status` which returns an enum with the current sync
status (either Inactive, Active or Pending) and uses it in the
ForkChoiceUpdate & NewPayload engine rpc endpoints so that we don't
apply their logic during an active or pending sync.
- Fetcher process now identify stale pivots and remain passive until
they receive the end signal
- Fetcher processes now return their current queue upon return so that
it can be persisted into the next cycle
- Stores the latest state root during state sync and healing as a
checkpoint
- Stores the last fetched key during state sync as a checkpoint
- Healing no longer stores the nodes received via p2p, it instead
inserts the leaf values and rebuilds it to avoid trie corruption between
restarts.
- The current progress percentage and estimated time to finish is
periodically reported during state sync
- Disables the following Paris & Cancun engine hive tests that
previously yielded false positives due to new payloads being accepted on
top of a syncing chain:
* Invalid NewPayload (family)
* Re-Org Back to Canonical Chain From Syncing Chain
* Unknown HeadBlockHash
* In-Order Consecutive Payload Execution (Flaky)
* Valid NewPayload->ForkchoiceUpdated on Syncing Client
* Invalid Missing Ancestor ReOrg
* * Payload Build after New Invalid Payload
(only Cancun)
- And also disables the following tests that fail with the flag
Syncing=true for the same reason :
* Bad Hash on NewPayload
* ParentHash equals BlockHash on NewPayload (only for Paris)
* Invalid PayloadAttributes (family)
Misc:
- Replaces some noisy unwraps in networking module with errors
- Applies annotated hacky fixes for problems reported in #1684 #1685 &
#1686
<!-- A clear and concise general description of the changes this PR
introduces -->
<!-- Link to issues: Resolves #111, Resolves #222 -->
Closes None
<!-- A clear and concise general description of the changes this PR
introduces -->
<!-- Link to issues: Resolves #111, Resolves #222 -->
Closes #issue_number
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When we connect to a peer and exchange
Statusmessages we validate theirForkID, the fork id is derived from the genesis, chain config, and block number and timestamp of the latest block. When we connect to a peer with the purpose of syncing our fork ids will not match, as our peer should be on a further block. Therefore we shouldn't check that our fork ids are the exact same but instead check for compatibility as described in: https://eips.ethereum.org/EIPS/eip-2124The text was updated successfully, but these errors were encountered: