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

Add block hash saving for non canon blocks #857

Conversation

Gustav-Simonsson
Copy link

  • Ensure block hash is stored with the block number as key even if
    a block is non-canonical (not a new tip of the chain), this is
    required by the new chain forking code in the edge cases tested
    by BlockTests/bcUncleTest.json
  • Rename and split up related DB / cache storage functions to make
    them easier to read.
  • Move if statement's simple statement to it's own line to get
    distinct stacktrace line number in case we get future bugs here.

Gustav Simonsson added 4 commits May 5, 2015 03:10
* Ensure block hash is stored with the block number as key even if
  a block is non-canonical (not a new tip of the chain), this is
  required by the new chain forking code in the edge cases tested
  by BlockTests/bcUncleTest.json
* Rename and split up related DB / cache storage functions to make
  them easier to read.
* Move if statement's simple statement to it's own line to get
  distinct stacktrace line number in case we get future bugs here.
@@ -537,7 +517,7 @@ func (self *ChainManager) InsertChain(chain types.Blocks) (int, error) {
}

block.Td = new(big.Int)
// Do not penelise on future block. We'll need a block queue eventually that will queue
// Do not penalize on future block. We'll need a block queue eventually that will queue
Copy link
Contributor

Choose a reason for hiding this comment

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

Please correct properly. Do not favour American English over correct English ;-) (i.e. penalise)

@Gustav-Simonsson
Copy link
Author

Yes, please wait with this one and check this fix first: #865

It avoids this scenario resulting in a panic and instead rejects the block. However the logic here still needs to be fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants