-
Notifications
You must be signed in to change notification settings - Fork 179
feat: Implement v0.68 block merkle tree structure #21930
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
Conversation
Signed-off-by: Matt Hess <[email protected]>
Signed-off-by: Matt Hess <[email protected]>
Signed-off-by: Matt Hess <[email protected]>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #21930 +/- ##
============================================
+ Coverage 70.79% 70.86% +0.06%
- Complexity 24381 24411 +30
============================================
Files 2667 2668 +1
Lines 104204 104323 +119
Branches 10942 10952 +10
============================================
+ Hits 73774 73926 +152
+ Misses 26383 26344 -39
- Partials 4047 4053 +6
... and 21 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Signed-off-by: Matt Hess <[email protected]>
Node: HAPI Test (Misc) Results101 files - 60 1 errors 100 suites - 61 4m 9s ⏱️ - 12m 57s For more details on these parsing errors and failures, see this check. Results for commit 8e04874. ± Comparison against base commit 5362176. This pull request removes 76 and adds 29 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Signed-off-by: Matt Hess <[email protected]>
Node: HAPI Test (Smart Contract) Results117 files - 8 110 errors 7 suites - 118 11s ⏱️ - 19m 51s For more details on these parsing errors and failures, see this check. Results for commit 8e04874. ± Comparison against base commit 5362176. This pull request removes 135 and adds 3 tests. Note that renamed tests count towards both. |
Signed-off-by: Matt Hess <[email protected]>
Signed-off-by: Matt Hess <[email protected]>
Signed-off-by: Matt Hess <[email protected]>
Signed-off-by: Matt Hess <[email protected]>
Signed-off-by: Matt Hess <[email protected]>
Signed-off-by: Matt Hess <[email protected]>
Node: HAPI Test (Crypto) Results30 files - 5 28 errors 2 suites - 33 3m 59s ⏱️ - 2m 35s For more details on these parsing errors and failures, see this check. Results for commit 9b322a0. ± Comparison against base commit 5362176. This pull request removes 34 and adds 2 tests. Note that renamed tests count towards both. |
| */ | ||
| message BlockItem { | ||
| // Reserved for future items that require separate handling for block hash purposes. | ||
| reserved 12,13,14,15,16,17,18,19; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesnt seem like correct way to re-use 12
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what Joseph suggested
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These reserved values (12-19) are properly "held for future use" rather than "previously used and not available".
When the block stream is completely final (hopefully by release 0.70), we'll stop replacing/reusing fields and begin treating it the same as the rest of the API.
| * Presence of `filtered_item` entries SHALL NOT prevent verification | ||
| * of a block, but MAY preclude verification or reconstruction of | ||
| * consensus state.<br/> | ||
| * A transaction intentionally filtered from the stream. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * A transaction intentionally filtered from the stream. | |
| * A transaction (or other block item) intentionally filtered from the stream. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to A block item intentionally filtered from the stream in the other PR (#21401)
TODO