core: ensure the canonical block is written before the canonical hash is set #2873
Conversation
|
The code change looks good to me. Does the order also need to change in HeaderChain? |
|
Thank you for your contribution! Your commits seem to not adhere to the repository coding standards
Please check the contribution guidelines for more details. This message was auto-generated by https://gitcop.com |
| @@ -196,6 +187,18 @@ func (hc *HeaderChain) WriteHeader(header *types.Header) (status WriteStatus, er | |||
| if err := WriteHeader(hc.chainDb, header); err != nil { | |||
| glog.Fatalf("failed to write header contents: %v", err) | |||
| } | |||
There was a problem hiding this comment.
The loop above still rewrites canonical hashes before the header is written. A safer change would be to move the writes of Header and TD up so they happen before the canonical numbers are dealt with.
|
Thank you for your contribution! Your commits seem to not adhere to the repository coding standards
Please check the contribution guidelines for more details. This message was auto-generated by https://gitcop.com |
|
👍 |
|
|
||
| if err := WriteHeadHeaderHash(hc.chainDb, hash); err != nil { | ||
| glog.Fatalf("failed to insert head header hash: %v", err) | ||
| } |
There was a problem hiding this comment.
I'm not sure this block should be moved. The head header is the topmost index that signals the current position of the header chain. Imho it should not be updated to the new head before the canonical number is written out as the invariant is that if head points to X, everything until X is present and complete.
There was a problem hiding this comment.
I of course could be wrong, if so, please enlighten me :)
|
LGTM 👍 |
Inserting a new chain into the blockchain can cause a temporary inconsistency in the database. The canonical hash for a particular block is set before the header and body are written.
Code that tries to fetch the current block by its number will first get the canonical hash for that block number through
core.GetCanonicalHash. This will yield a valid block hash. When the block is fetched immediately after withcore.GetBlockthis can return nil because the header and/or body isn't yet written. This is unexpected since the canonical hash could be found and code that fetched the block can expect this function to return a proper block. Currently the filter system crashes on this and maybe other parts of the system as well.This PR ensures that the header and body are written during chain insertion before the canonical hash is set.
Fixes:
#2858
ethereum/mist#999
PTAL: @fjl @karalabe