Fix: tenure downloader reward set error #6234 #6276
+350
−46
Conversation
This file contains hidden or 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
obycode
previously approved these changes
Jul 10, 2025
obycode
previously approved these changes
Jul 11, 2025
Jiloc
previously approved these changes
Jul 11, 2025
Co-authored-by: Francesco <2530388+Jiloc@users.noreply.github.com>
obycode
approved these changes
Jul 11, 2025
github-project-automation
bot
moved this from Status: In Review
to Status: 💻 In Progress
in Stacks Core Eng
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 refactors the reward cycle fetching logic in the tenure downloader and fixes the #6234 bug.
First -- I moved the
downloader_block_height_to_reward_cyclemethod into a newly named (and commented) method thats a static method of the NakamotoDownloadStateMachine struct. The logic of that calculation is: given a block commit at burn height X, what is the latest possible reward cycle for a tenure the commit confirms?I think that the only places we want to use this calculation are in
WantedTenuresrequest generation -- basically, given a sortition tip, what tenures could we download and confirm?The bug in #6234 was caused by the fact that this calculation was also used for the reward set caching logic of the (unconfirmed) tenure downloader. This meant that for the first two tenures of a new reward cycle, the wrong reward sets would be used for download validation. The fix for this was to simply use the normal reward set calculation in those instances.
This PR adds an integration test for this behavior: basically, perform signer-set rollover with 2 active nodes. Importantly, in order for the test to actually cover the behavior, the nodes need to refuse block pushes (otherwise, they never need to use the TenureDownloader at all).