Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Stop keeping track of epoch changes for the sync gap #13127

Merged
merged 4 commits into from
Jan 12, 2023
Merged

Conversation

davxy
Copy link
Member

@davxy davxy commented Jan 11, 2023

Epoch changes data for the gap (i.e. the blocks below the warp sync point) was used to verify the blocks imported after a warp sync.

Currently this verification is not really required since all our security depends on the last finalized block.

Once the last finalized block is imported (i.e. the warp sync point) then we start importing all the previous blocks and we can detect a bad history only after we finished importing the block before the warp sync point.

We may think that in this way we may be tricked by a malicious full node into downloading an history of blocks that in the end we find out to not be good WRT the finalized block we warp synced to. However, the same type of attack can be performed by one of the initial authorities by feeding an history that is coherently signed together with a crafted dummy authority set changes.

Thus our only stable grip is the last finalized block anyway. This is the point that in the end will allow us to distinguish between a good history from a bad one.


Note. In the future we may think to early detect this type of annoyance by downloading the blocks in reverse order. In this way, for each block we can immediately check if the header hash is equal to the parent of the block that follows.


Fixes zombienet test 0003 in #13078

@davxy davxy self-assigned this Jan 11, 2023
@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Jan 11, 2023
@davxy davxy added B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. A0-please_review Pull request needs code review. and removed A0-please_review Pull request needs code review. labels Jan 11, 2023
@davxy davxy requested review from a team and bkchr January 11, 2023 17:06
Copy link
Contributor

@melekes melekes left a comment

Choose a reason for hiding this comment

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

👍 (code-wise; don't know enough to reason about business logic)

@davxy davxy requested a review from a team January 12, 2023 07:44
client/consensus/babe/src/lib.rs Outdated Show resolved Hide resolved
client/consensus/babe/src/lib.rs Outdated Show resolved Hide resolved
davxy and others added 2 commits January 12, 2023 17:50
Co-authored-by: Bastian Köcher <git@kchr.de>
@davxy davxy merged commit 2bfc1dd into master Jan 12, 2023
@davxy davxy deleted the davxy-remove-gap branch January 12, 2023 19:36
rossbulat pushed a commit that referenced this pull request Feb 3, 2023
* Stop keeping track of epoch changes data within the sync gap

* Fix docs

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <git@kchr.de>

* Fix typo

Co-authored-by: Bastian Köcher <git@kchr.de>
ltfschoen pushed a commit to ltfschoen/substrate that referenced this pull request Feb 22, 2023
* Stop keeping track of epoch changes data within the sync gap

* Fix docs

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <git@kchr.de>

* Fix typo

Co-authored-by: Bastian Köcher <git@kchr.de>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
* Stop keeping track of epoch changes data within the sync gap

* Fix docs

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <git@kchr.de>

* Fix typo

Co-authored-by: Bastian Köcher <git@kchr.de>
@bkchr bkchr added B1-note_worthy Changes should be noted in the release notes E1-database_migration PR introduces code that does a one-way migration of the database. T0-node This PR/Issue is related to the topic “node”. and removed B0-silent Changes should not be mentioned in any release notes labels Apr 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. E1-database_migration PR introduces code that does a one-way migration of the database. T0-node This PR/Issue is related to the topic “node”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants