Nakamoto Fixes: IssueBlockCommit and Reward Maturity Lookups #4205
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
jcnelson
reviewed
Dec 20, 2023
| use std::collections::VecDeque; | ||
| use std::sync::mpsc::Sender; | ||
| use std::time::Duration; | ||
|
|
||
| use hashbrown::{HashMap, HashSet}; |
There was a problem hiding this comment.
Is there a reason we're still using hashbrown?
There was a problem hiding this comment.
Unclear to me. Maybe open an issue for the signer? This changeline just came from running cargo fmt-stacks.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## next #4205 +/- ##
==========================================
- Coverage 82.83% 82.63% -0.21%
==========================================
Files 429 429
Lines 302091 302128 +37
==========================================
- Hits 250245 249662 -583
- Misses 51846 52466 +620 ☔ View full report in Codecov by Sentry. |
zone117x
approved these changes
Dec 21, 2023
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.
Description
This fixes two issues surfaced by the interim blocks mining in nakamoto-node.
calculate_matured_miner_rewardsshould use thecoinbase_height, not the block height. This was caught by running a long-running node with interim mining turned on, rather than an integration test. There's no integration test for this new behavior, but there probably should be. The issue surface from a panicking assertion because the 150th Stacks block had a coinbase height < 100, so the maturity lookup returned an empty list, triggering a panic. In testing mode, the maturity height is 2, which means that it would be hard to surface this exact panic in a test.