Skip to content
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

2.5: PoX missed-slot updates #4576

Merged
merged 1 commit into from
Mar 21, 2024
Merged

2.5: PoX missed-slot updates #4576

merged 1 commit into from
Mar 21, 2024

Conversation

kantai
Copy link
Member

@kantai kantai commented Mar 21, 2024

Description

This PR removes the unlock behavior of PoX-4 when solo stackers miss a reward slot. This behavior was broken next: missed reward slot unlocks have consequences for signer set calculation, which has to be addressed. This PR addresses it by removing the unlock behavior.

This PR also adds a test that covers the case of a 2.5 reward cycle having participation but no slots. Such a cycle should still allow the system to make forward progress in 2.5.

* test for the missed slots PoX behavior in 2.5
* test for *near* zero PoX participation behavior in 2.5
Copy link
Member

@jcnelson jcnelson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as long as CI passes. I notice that this PR doesn't remove the ancillary PoX state that is required for auto-unlock. Is that scheduled for a follow-on PR, or should it be done here?

@kantai
Copy link
Member Author

kantai commented Mar 21, 2024

Is that scheduled for a follow-on PR, or should it be done here?

That's a great question -- it impacts 2.5 activation in testnet, because the PoX contract would necessarily change. I will try to see how easy of a change that is.

@kantai
Copy link
Member Author

kantai commented Mar 21, 2024

LGTM as long as CI passes. I notice that this PR doesn't remove the ancillary PoX state that is required for auto-unlock. Is that scheduled for a follow-on PR, or should it be done here?

Quick follow-up!

The reward set indexes data is used both for the (removed) handle-unlocks method and for stack-increase (its how it figures out which items to increase), so we can't remove that tracking state. However, that tracking state is much less volatile without handle-unlocks (because you never have to perform removals in the stacking-state).

@saralab saralab requested a review from obycode March 21, 2024 18:31
Copy link

codecov bot commented Mar 21, 2024

Codecov Report

Attention: Patch coverage is 41.92635% with 205 lines in your changes are missing coverage. Please review.

Project coverage is 77.87%. Comparing base (d2649ff) to head (19a373f).
Report is 22 commits behind head on next.

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #4576      +/-   ##
==========================================
- Coverage   83.25%   77.87%   -5.38%     
==========================================
  Files         456      456              
  Lines      330593   330898     +305     
  Branches      323      317       -6     
==========================================
- Hits       275248   257703   -17545     
- Misses      55337    73187   +17850     
  Partials        8        8              
Files Coverage Δ
stacks-common/src/types/mod.rs 92.54% <100.00%> (+0.14%) ⬆️
stackslib/src/chainstate/stacks/boot/pox-4.clar 14.33% <ø> (+0.87%) ⬆️
stackslib/src/net/mod.rs 72.93% <100.00%> (+0.01%) ⬆️
stackslib/src/chainstate/stacks/boot/mod.rs 88.25% <89.65%> (-6.50%) ⬇️
...tackslib/src/chainstate/stacks/boot/pox_4_tests.rs 87.57% <36.87%> (-12.10%) ⬇️

... and 171 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d2649ff...19a373f. Read the comment docs.

Copy link
Contributor

@obycode obycode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@kantai kantai added this pull request to the merge queue Mar 21, 2024
Merged via the queue into next with commit f0a890b Mar 21, 2024
1 of 2 checks passed
ASuciuX added a commit to ASuciuX/archived-stacks-core that referenced this pull request Mar 27, 2024
This became caught in the meantime `stacks-common/src/types/mod.rs:103:9: replace StacksEpochId::supports_pox_missed_slot_unlocks `
@blockstack-devops
Copy link
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@stacks-network stacks-network locked as resolved and limited conversation to collaborators Nov 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants