Skip to content

[FIX] Return the last blocks when fetching the ommers ancestors from memory #790

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

Merged
merged 1 commit into from
Nov 11, 2020

Conversation

ntallar
Copy link

@ntallar ntallar commented Nov 11, 2020

Description

Addresses issue on testnet

Scenario

  • Importing a chain of 12 blocks (1858 - 1870)
  • Block 1870 had an ommer of number 1865 (it's chain ancestor is then block 1864)

When importing block 1970:

  1. Blocks 1858-1869 were returned from the BlockQueue when fetching the branch from it
  2. Taking 6 blocks from it resulted in blocks: 1858-1863

The above chain didn't have the ommer ancestor, so the validation of them failed with OmmersAncestorsError. As the above mentioned chain was followed by 3/5 miners they forked irrecoverably.

Proposed Solution

Changing Taking 6 blocks from it resulted in blocks: 1858-1863 to Taking the last 6 blocks from it resulted in blocks: 1864-1869

Testing

Due to the complexity of testing this scenario and the time requirements for the having the internal testnet running no test was added.

@ntallar ntallar added the bug Something isn't working label Nov 11, 2020
@ntallar ntallar requested a review from mirkoAlic November 11, 2020 15:57
Copy link
Contributor

@mirkoAlic mirkoAlic left a comment

Choose a reason for hiding this comment

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

LGTM! please create a task for future analysis over block queue in order to see how we could start adding tests for this kind of things.

@mirkoAlic mirkoAlic merged commit 1adf114 into develop Nov 11, 2020
@mirkoAlic mirkoAlic deleted the fix-queue-usage-ommers-ancestors branch November 11, 2020 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants