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

Less picky epoch changes lookup, with gap fallback #13106

Closed
wants to merge 2 commits into from

Conversation

davxy
Copy link
Member

@davxy davxy commented Jan 9, 2023

Fix for the issue described by #12751 (comment)

In the (unlucky) event of skipped epochs, full nodes using BABE were receiving an error during the VRF output verification.

2022-11-26 10:44:22.488  WARN tokio-runtime-worker sync: 💔 Verification failed for block 0x2babfbf24066fa1b420934090c6472bf57f9b16a91fddf7ba3659ef7ad82449b received from peer: 12D3KooWPKzmmE2uYgF3z13xjpbFTp63g9dZFag8pG6MgnpSLF4S, "Could not fetch epoch at 0x5448eda4b7963ee85d28ea523149317dc39ce41173e198126b7ba67b7e123854"    

The problem was triggered because after warp sync the epoch changes data was contained within the gap structure AND (in contrast to the epoch changes tree) the gap is picky about the check. I.e. it requires that epoch.start_slot <= slot < epoch.end_slot.


Additionally we prevent epoch changes tree prune on finalization if there is something in the gap. This is because we may require (again, after warp sync and if some epoch hash been skipped) the first tree entry during import of last gap entries.

@andresilva this requires some brainstorming


I'm going to add some tests before merge

@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Jan 9, 2023
@davxy davxy changed the title Use gap entry as a fallback Less picky epoch changes lookup, with gap fallback Jan 9, 2023
@davxy davxy requested review from andresilva and a team January 9, 2023 18:33
@davxy davxy self-assigned this Jan 9, 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. labels Jan 9, 2023
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 marked this pull request as draft January 10, 2023 07:14
// Tree nodes may be required while importing gap blocks.
return Ok(())
}

Copy link
Contributor

Choose a reason for hiding this comment

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

This is interesting. In what circumstances we could have both gap and forktree?

Copy link
Member Author

Choose a reason for hiding this comment

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

this is a really bad hack and that is the motivation that I set as draft again.
I would like to open a discussion about it. I'll ping you when I have more info

@davxy davxy closed this Jan 11, 2023
@davxy davxy deleted the davxy-amenable-epochs-gap branch March 15, 2023 16:19
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. 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants