Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants