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

Vote for aggregated public key #4116

Closed

Conversation

friedger
Copy link
Collaborator

@friedger friedger commented Dec 1, 2023

Description

This PR adds voting for the aggregated public key submitted by signers.

Applicable issues

Additional info (benefits, drawbacks, caveats)

Checklist

  • Test coverage for new or modified code paths
  • Changelog is updated
  • Required documentation changes (e.g., docs/rpc/openapi.yaml and rpc-endpoints.md for v2 endpoints, event-dispatcher.md for new events)
  • New clarity functions have corresponding PR in clarity-benchmarking repo
  • New integration test(s) added to bitcoin-tests.yml

(define-read-only (current-reward-cycle)
u0)

(define-public (vote-for-aggreated-public-key (key (buff 33)) (reward-cycle uint) (round uint) (tapleaves (list 4001 (buff 33))))
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@setzeus What must be contained in a vote?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Discussed during 12/5 Clarity WG
Possibly missing hardcoded internal key so we can guarantee that KeyPath isn't spendable.

@friedger friedger force-pushed the feat/pox-4-disallow-stacking-during-prepare-phase branch from ff2c499 to 6ed2a25 Compare December 12, 2023 11:45
Copy link

codecov bot commented Dec 16, 2023

Codecov Report

Attention: 34 lines in your changes are missing coverage. Please review.

Comparison is base (70e8d32) 76.96% compared to head (9966053) 82.78%.
Report is 60 commits behind head on feat/pox-4-disallow-stacking-during-prepare-phase.

Files Patch % Lines
stackslib/src/chainstate/stacks/boot/mod.rs 0.00% 31 Missing ⚠️
testnet/stacks-node/src/tests/signer.rs 0.00% 3 Missing ⚠️
Additional details and impacted files
@@                                  Coverage Diff                                  @@
##           feat/pox-4-disallow-stacking-during-prepare-phase    #4116      +/-   ##
=====================================================================================
+ Coverage                                              76.96%   82.78%   +5.82%     
=====================================================================================
  Files                                                    420      421       +1     
  Lines                                                 296655   299227    +2572     
=====================================================================================
+ Hits                                                  228318   247727   +19409     
+ Misses                                                 68337    51500   -16837     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

u0)

(define-public (vote-for-aggregated-public-key (key (buff 33)) (reward-cycle uint) (round uint) (tapleaves (list 4001 (buff 33))))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where is the tally-check / consensus (>70%) update happening?

In sBTC-mini, this function checked if the latest vote pushed an existing signing-key vote above the 70% threshold & updated state accordingly.

Is that logic handled elsewhere at the moment or is that logic missing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know where to get the weight of a stacker from. Should I rebase on #4164 ?

(define-map tally {reward-cycle: uint, round: uint, aggregate-public-key: (buff 33)} uint)

(define-constant err-not-allowed (err u10000))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you relocate to batch w/ rest of errors & update the format to match existing? (capitalized, currently in the 30s index).

(define-constant ERR_INCORRECT_REWARD_CYCLE 32)

@friedger friedger changed the base branch from feat/pox-4-disallow-stacking-during-prepare-phase to next January 10, 2024 10:16
@friedger friedger changed the base branch from next to chore/pox-4-single-file January 10, 2024 10:17
@friedger
Copy link
Collaborator Author

Replaced by #4239

@friedger friedger closed this Jan 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

5 participants