-
Notifications
You must be signed in to change notification settings - Fork 738
Fix/sortition stacks tip burn view #6881
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
base: develop
Are you sure you want to change the base?
Fix/sortition stacks tip burn view #6881
Conversation
…ip to the sortition of its burn view. Add unit test coverage to make sure that it works under sortition forks.
Codecov Report❌ Patch coverage is
❌ Your project check has failed because the head coverage (78.49%) is below the target coverage (80.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #6881 +/- ##
============================================
+ Coverage 53.78% 78.49% +24.71%
============================================
Files 604 604
Lines 361651 361943 +292
Branches 338 338
============================================
+ Hits 194518 284120 +89602
+ Misses 167133 77823 -89310
... and 447 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
…tips after the migration to schema 8
|
Putting this in draft until I can find out why all these integration tests failed (ETA tomorrow). |
| sort_id, | ||
| consensus_hash, | ||
| &sortition.sortition_id, | ||
| burn_view_consensus_hash, |
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.
Is tracking the burn view consensus hash necessary in the table? It's just a foreign key relationship between the sortition_id and snapshots.consensus_hash, right?
aaronb-stacks
left a comment
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.
I think the table needs to track the consensus hash of the stacks blocks (i.e., the consensus hash of their election block). The burn view consensus hash only needs to be used to determine which sortition_id to replace or insert an entry.
| // update arrival data across all Stacks forks | ||
| let (best_ch, best_bhh, best_height) = self.find_new_block_arrivals(burn_tip)?; | ||
| self.update_canonical_stacks_tip(&burn_tip.sortition_id, &best_ch, &best_bhh, best_height)?; | ||
| self.update_canonical_stacks_tip(&best_ch, &best_bhh, best_height)?; |
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.
best_ch is now a burn_view, rather than the consensus hash of best_bhh, right?
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.
The table tracks both now
| |row| Ok((row.get_unwrap(0), row.get_unwrap(1), (u64::try_from(row.get_unwrap::<_, i64>(2)).expect("FATAL: block height too high")))) | ||
| ).optional()?; | ||
| if let Some(stacks_tip) = result_at_tip { | ||
| return Ok(Some(stacks_tip)); |
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 returning the burn_view_consensus_hash and the block_header_hash -- these are not the usual inputs to a StacksBlockId. Instead, those use the consensus hash of the tenure electing burn block. I think that the new table needs to track the block's consensus hash.
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.
Yes, that was an oversight. The code will report the tenure consensus hash and the stacks block hash, but will use the burn view consensus hash to determine which row to update
Yes, you're right. |
…ion-stacks-tip-burn-view
…ion-stacks-tip-burn-view
This fixes part of the chain stall from the night of 7/8 Feb 2026. This patch alters the way the sortition DB memoizes the canonical Stacks chain tip by pairing each Stacks chain tip with the sortition of its burn view instead of whatever the canonical sortition tip happened to be.
The reason for this change is because it ensures that the canonical Stacks tip can be found even if a Bitcoin fork is processed before all of the preceding tenure's Stacks blocks can be processed. For example, suppose there are four sortitions: A, B, B', and C'. B and B' both descend from A, and C' descends from B'.
Now, suppose that the Stacks blocks with burn view A -- A1 through A6 -- are processed. Once A4 is processed, the
stacks_chain_tipstable would have assigned sortition A to A4. That is, the canonical Stacks tip at the canonical sortition tip is A4.But now suppose that in-between when A4 and A5 are processed, the Stacks node processes sortition B. In the old code, the
stacks_chain_tipstable would have assigned sortition B to A6. That is, the canonical Stacks tip as of canonical sortition B would be A6. In this patch, there would be no entry instacks_chain_tipsfor B; instead, the row for sortition A would be updated to be A6 (this does not change the behavior of the node, thus far).But now, suppose no Stacks blocks are mined in the B' tenure, and the node processes sortition C'. In the old code, when the node processed B', its
stacks_chain_tipsrow would report A4 as the canonical tip, because that's what its parent sortition A had (the new code does not add a row at all for B'). Now, C' is the canonical sortition history, and as with B', the node would incorrectly report that A4 was the canonical Stacks tip on the canonical sortition tip C' (since that was the tip for B', the parent of C'). In the new code, no row would be added tostacks_chain_tipsfor C'.The old code is what partially led to the chain stall -- despite the node having processed A1 through A6, the node would incorrectly report A4 as being the Stacks chain tip. The miner code used the buggy code paths to deduce what to build atop, but that has been fixed separately. In the new code, the code paths have been updated so that a sortition is not assigned a canonical Stacks tip unless the corresponding Stacks block reported that sortition as its burn view. This way, the code will correctly report A6 as the canonical Stacks tip for sortition tips B, B', and C'