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

Add stack-aggregation-commit command signature and authorization #4628

Conversation

BowTiedRadone
Copy link
Contributor

This PR adds the stack-aggregation-commit command, using both signature and authorization, to the stateful property testing environment. It is part of #4548 and targets feat/pox-4-stateful-property-testing (#4550).

? model.wallets.get(this.operator.lockedAddresses[0])!.delegatedPoxAddress
: this.operator.btcAddress;

this.operator.lockedAddresses.forEach((stacker) => {
Copy link
Member

Choose a reason for hiding this comment

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

Could something like this work?

(Typed on the fly)

sameDelegatedPoxAddrAllStackers =
  !this.operator.lockedAddresses.some(stacker =>
    model.wallets.get(stacker)!.delegatedPoxAddress !== firstDelegatedPoxAddress
  );

?

Copy link
Member

Choose a reason for hiding this comment

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

And then sameDelegatedPoxAddrAllStackers can be const-bound.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The operator can lock stx for some stackers pox address 1 and some to pox address 2. Then there need to be two commits.

One for each pox address.

Copy link
Collaborator

Choose a reason for hiding this comment

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

A more common use case is that the operator calls commit several times in a cycle. In between the operator calls delegate-stack.. With the same index.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Second this^, the popular use-case is to "override" or "top-off" the same pool / index to include as many member as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@friedger @setzeus
So a more suitable flow is:

  • pick all the stackers' delegated pox addresses
  • call stack-aggregation-commit for all the pox addresses found (e.g. found 3 addresses, call stack-aggregation-commit 3 times, one for each pox address)

Is this correct? If not, please let me know what other flow will suit the command better.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sounds good. You could also add a random value whether the pox address belongs to the pool or not.
Then you would have maybe only 2 commit txs

Copy link
Collaborator

@friedger friedger left a comment

Choose a reason for hiding this comment

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

Looks good.

Fast Pool sends several commit txs in a cycle.

? model.wallets.get(this.operator.lockedAddresses[0])!.delegatedPoxAddress
: this.operator.btcAddress;

this.operator.lockedAddresses.forEach((stacker) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The operator can lock stx for some stackers pox address 1 and some to pox address 2. Then there need to be two commits.

One for each pox address.

? model.wallets.get(this.operator.lockedAddresses[0])!.delegatedPoxAddress
: this.operator.btcAddress;

this.operator.lockedAddresses.forEach((stacker) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

A more common use case is that the operator calls commit several times in a cycle. In between the operator calls delegate-stack.. With the same index.

Copy link

codecov bot commented Apr 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.52%. Comparing base (b26ad6a) to head (7c09274).

Additional details and impacted files
@@                            Coverage Diff                            @@
##           feat/pox-4-stateful-property-testing    #4628       +/-   ##
=========================================================================
- Coverage                                 82.86%   66.52%   -16.34%     
=========================================================================
  Files                                       470      470               
  Lines                                    332564   332151      -413     
  Branches                                    317      317               
=========================================================================
- Hits                                     275575   220974    -54601     
- Misses                                    56981   111169    +54188     
  Partials                                      8        8               

see 268 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 b26ad6a...7c09274. Read the comment docs.

Copy link
Collaborator

@friedger friedger left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@moodmosaic moodmosaic left a comment

Choose a reason for hiding this comment

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

LGTM

@moodmosaic
Copy link
Member

Merging into feat/pox-4-stateful-property-testing.

@moodmosaic moodmosaic merged commit 9eeb2d1 into feat/pox-4-stateful-property-testing Apr 4, 2024
1 of 2 checks passed
@moodmosaic moodmosaic deleted the feat/pox-4-stateful-stack-agg-commit branch April 4, 2024 08:54
moodmosaic pushed a commit that referenced this pull request Apr 11, 2024
- removed check that verifies all the stackers have delegated to the same PoX address
- addressed comments:
  - #4628 (comment)
  - #4628 (comment)
  - #4628 (comment)
moodmosaic pushed a commit that referenced this pull request Apr 11, 2024
- removed check that verifies all the stackers have delegated to the same PoX address
- addressed comments:
  - #4628 (comment)
  - #4628 (comment)
  - #4628 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants