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

Make sure to not mark block header hash as invalid if only the body is wrong. #11356

Merged
merged 3 commits into from
Dec 30, 2019

Conversation

tomusdrw
Copy link
Collaborator

No description provided.

@tomusdrw tomusdrw added A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust. labels Dec 30, 2019
Error(ErrorKind::Block(BlockError::InvalidTransactionsRoot(_)), _) |
Error(ErrorKind::Block(BlockError::InvalidUnclesHash(_)), _) => {
self.verification.bad.lock().insert(raw_hash);
},
_ => {
self.verification.bad.lock().insert(hash);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@tomusdrw I'm thinking maybe it's better to always track bad blocks by raw_hash?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it may require either putting both hashes in or changing some sync code as we expose status(H256) -> Status method, so that other components can check by-hash if particular header was already marked as bad.
I didn't want to change too much in that PR, but I think it may indeed make sense long term.

@sorpaas sorpaas merged commit 54c2d61 into stable Dec 30, 2019
@sorpaas sorpaas deleted the td-patch-root branch December 30, 2019 21:15
s3krit pushed a commit that referenced this pull request Dec 30, 2019
…s wrong. (#11356)

* Patch invalid transaction root.

* Add raw hash to bad and include fix for uncles too.

* Fix submodules.
dvdplm added a commit that referenced this pull request Dec 30, 2019
dvdplm added a commit that referenced this pull request Dec 30, 2019
s3krit pushed a commit that referenced this pull request Dec 30, 2019
ordian pushed a commit that referenced this pull request Jan 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants