Skip to content
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

Snowbridge - Ethereum Client - Reject finalized updates without a sync committee in next store period #4483

Conversation

claravanstaden
Copy link
Contributor

This is a cherry-pick from master of #4478

Expected patches for (1.8.0):
snowbridge-pallet-ethereum-client

…c committee in next store period (paritytech#4478)

While syncing Ethereum consensus updates to the Snowbridge Ethereum
light client, the syncing process stalled due to error
`InvalidSyncCommitteeUpdate` when importing the next sync committee for
period `1087`.

This bug manifested specifically because our light client checkpoint is
a few weeks old (submitted to governance weeks ago) and had to catchup
until a recent block. Since then, we have done thorough testing of the
catchup sync process.

### Symptoms
- Import next sync committee for period `1086` (essentially period
`1087`). Light client store period = `1086`.
- Import header in period `1087`. Light client store period = `1087`.
The current and next sync committee is not updated, and is now in an
outdated state. (current sync committee = `1086` and current sync
committee = `1087`, where it should be current sync committee = `1087`
and current sync committee = `None`)
- Import next sync committee for period `1087` (essentially period
`1088`) fails because the expected next sync committee's roots don't
match.

### Bug
The bug here is that the current and next sync committee's didn't
handover when an update in the next period was received.

### Fix
There are two possible fixes here:
1. Correctly handover sync committees when a header in the next period
is received.
2. Reject updates in the next period until the next sync committee
period is known.

We opted for solution 2, which is more conservative and requires less
changes.

### Polkadot-sdk versions
This fix should be backported in polkadot-sdk versions 1.7 and up.

Snowfork PR: #145

---------

Co-authored-by: Vincent Geddes <117534+vgeddes@users.noreply.github.com>
@claravanstaden claravanstaden marked this pull request as ready for review May 16, 2024 14:42
@paritytech-review-bot paritytech-review-bot bot requested a review from a team May 16, 2024 14:43
@paritytech-cicd-pr
Copy link

The CI pipeline was cancelled due to failure one of the required jobs.
Job name: cargo-clippy
Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6224126

@bkontur bkontur added R0-silent Changes should not be mentioned in any release notes T15-bridges This PR/Issue is related to bridges. labels May 17, 2024
@acatangiu acatangiu merged commit dc27b7f into paritytech:release-crates-io-v1.8.0 May 17, 2024
38 of 137 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T15-bridges This PR/Issue is related to bridges.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants