Refactor/header caching #1347
Merged
Refactor/header caching #1347
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
Codecov Report
@@ Coverage Diff @@
## main #1347 +/- ##
===========================================
- Coverage 86.03% 73.72% -12.31%
===========================================
Files 361 253 -108
Lines 32457 17095 -15362
===========================================
- Hits 27925 12604 -15321
- Misses 2747 3931 +1184
+ Partials 1785 560 -1225
|
greymistcube
added
discussion needed
longfin
reviewed
Jun 21, 2021
greymistcube
removed
invalid
limebell
previously approved these changes
Jun 21, 2021
greymistcube
force-pushed
the
refactor/header-caching
branch
from
3439240 to
114265a
Compare
limebell
requested changes
Jul 1, 2021
greymistcube
force-pushed
the
refactor/header-caching
branch
3 times, most recently
from
47df0aa to
85cd2e4
Compare
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
not stale |
Minor comment
greymistcube
force-pushed
the
refactor/header-caching
branch
from
85cd2e4 to
fe88cd9
Compare
limebell
approved these changes
Jul 21, 2021
longfin
approved these changes
Jul 21, 2021
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 PR caches
BlockHeaderwhen creating aBlock<T>instance. This is in preparation for dealing with #1164, like #1326 and #1341. Other than caching, this includes the following changes:BlockHeaderinstead ofBlock<T>.PreEvaluationHashvalidation moved toBlockHeader.Block<T>constructor.Several new constructors for
BlockHeaderhave been introduced to convert some implicit logic more explicit. The main issue remains where there is still too much implicit logic embedded in the "manual" constructor forBlock<T>. As left in theFIXMEcomments, this should be resolved on further refactoring. The firstBlock<T>constructor in the code should be separated into at least two, one where the constructor is purely used for manual creation of aBlock<T>instance (possibly intended for testing) and another intended for block generation through mining.