multi: Decouple orphan handling from blockchain. #2008
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
matheusd
reviewed
Nov 22, 2019
davecgh
force-pushed
the
blockchain_decouple_orphan_handling
branch
from
350e4bf
to
0e25aa9
Compare
dnldd
reviewed
Dec 2, 2019
davecgh
force-pushed
the
blockchain_decouple_orphan_handling
branch
from
0e25aa9
to
5db810a
Compare
dajohi
approved these changes
Dec 3, 2019
This decouples and removes the orphan handling from blockchain in favor
of implementing it in the block manager as part of the overall effort to
decouple the connection code from the download logic.
The change might not make a ton of sense in isolation, since there is no
major functional change, however, decoupling the orphan handling
independently helps make the review process easier and alleviates what
would otherwise result in additional intermediate code to handle cases
that ultimately will no longer exist.
The following is a high level overview of the changes:
- Introduce blockchain function to more easily determine if an error is
a rule error with a given error code
- Move core orphan handling code from blockchain to block manager
- Move data structures used to cache and track orphan blocks
- Move all functions releated to orphans
- BlockChain.IsKnownOrphan -> blockManager.isKnownOrphan
- BlockChain.GetOrphanRoot -> blockManager.orphanRoot
- BlockChain.removeOrphanBlock -> blockManager.removeOrphanBlock
- BlockChain.addOrphanBlock -> blockManager.addOrphanBlock
- Implement orphan handling in block manager
- Rework to use the moved functions and data structs
- Add check for known orphans in addition HaveBlock calls to retain
the same behavior
- Modify the semantics of the process block func exposed by the block
manager so that it no longer stores orphans since all consumers of
it ultimately reject orphans anyway
- Remove remaining orphan related code from blockchain
- Update ProcessBlock to return an error when called with an orphan
- Remove additional orphan processing from ProcessBlock
- Remove orphan cache check from HaveBlock
- Adjust example to account for the removed parameter
- Change chaingen harness tests to detect orphans via returned error
- Modify fullblock tests to detect orphans via returned error
- Adapt process order logic tests to cope with lack of orphan
processing
- Update all other tests accordingly
- Update various comments and the README.md and doc.go to properly
reflect the removal of orphan handling
davecgh
force-pushed
the
blockchain_decouple_orphan_handling
branch
from
5db810a
to
f140d33
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This decouples and removes the orphan handling from blockchain in favor of implementing it in the block manager as part of the overall effort to decouple the connection code from the download logic.
The change might not make a ton of sense in isolation, since there is no major functional change, however, decoupling the orphan handling independently helps make the review process easier and alleviates what would otherwise result in additional intermediate code to handle cases that ultimately will no longer exist.
The following is a high level overview of the changes:
BlockChain.IsKnownOrphan->blockManager.isKnownOrphanBlockChain.GetOrphanRoot->blockManager.orphanRootBlockChain.removeOrphanBlock->blockManager.removeOrphanBlockBlockChain.addOrphanBlock->blockManager.addOrphanBlockHaveBlockcalls to retain the same behaviorProcessBlockto return an error when called with an orphanProcessBlockHaveBlockchaingenharness tests to detect orphans via returned errorREADME.mdanddoc.goto properly reflect the removal of orphan handlingThis is work towards #1145.