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

BowTiedRadone · 2024-04-03T11:21:36Z

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).

contrib/core-contract-tests/tests/pox-4/pox_StackAggregationCommitAuthCommand.ts

contrib/core-contract-tests/tests/pox-4/pox_DelegateStackIncreaseCommand.ts

contrib/core-contract-tests/tests/pox-4/pox_DelegateStackStxCommand.ts

contrib/core-contract-tests/tests/pox-4/pox_StackAggregationCommitAuthCommand.ts

moodmosaic · 2024-04-03T11:34:31Z

contrib/core-contract-tests/tests/pox-4/pox_StackAggregationCommitAuthCommand.ts

+      ? model.wallets.get(this.operator.lockedAddresses[0])!.delegatedPoxAddress
+      : this.operator.btcAddress;
+
+    this.operator.lockedAddresses.forEach((stacker) => {


Could something like this work?

(Typed on the fly)

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

?

And then sameDelegatedPoxAddrAllStackers can be const-bound.

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.

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.

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

@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.

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

contrib/core-contract-tests/tests/pox-4/pox_StackAggregationCommitAuthCommand.ts

contrib/core-contract-tests/tests/pox-4/pox_StackAggregationCommitSigCommand.ts

friedger

Looks good.

Fast Pool sends several commit txs in a cycle.

friedger · 2024-04-03T11:38:27Z

contrib/core-contract-tests/tests/pox-4/pox_StackAggregationCommitAuthCommand.ts

+      ? model.wallets.get(this.operator.lockedAddresses[0])!.delegatedPoxAddress
+      : this.operator.btcAddress;
+
+    this.operator.lockedAddresses.forEach((stacker) => {


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.

friedger · 2024-04-03T11:41:17Z

contrib/core-contract-tests/tests/pox-4/pox_StackAggregationCommitAuthCommand.ts

+      ? model.wallets.get(this.operator.lockedAddresses[0])!.delegatedPoxAddress
+      : this.operator.btcAddress;
+
+    this.operator.lockedAddresses.forEach((stacker) => {


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.

contrib/core-contract-tests/tests/pox-4/pox_StackAggregationCommitAuthCommand.ts

codecov · 2024-04-03T12:02:42Z

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.

https://github.com/stacks-network/stacks-core/pull/4628/files/852b0c5ca8b3dd7c6f91a55e464b9095fa8776d5#r1549587969

contrib/core-contract-tests/tests/pox-4/pox_StackAggregationCommitAuthCommand.ts

https://github.com/stacks-network/stacks-core/pull/4628/files#r1549544398 Co-authored-by: Nikos Baxevanis <[email protected]>

https://github.com/stacks-network/stacks-core/pull/4628/files#r1549544650 Co-authored-by: Nikos Baxevanis <[email protected]>

https://github.com/stacks-network/stacks-core/pull/4628/files#r1549546009 Co-authored-by: Nikos Baxevanis <[email protected]>

contrib/core-contract-tests/tests/pox-4/pox_StackAggregationCommitSigCommand.ts

https://github.com/stacks-network/stacks-core/pull/4628/files#r1549557014 Co-authored-by: Nikos Baxevanis <[email protected]>

https://github.com/stacks-network/stacks-core/pull/4628/files#r1549556367 Co-authored-by: Nikos Baxevanis <[email protected]>

https://github.com/stacks-network/stacks-core/pull/4628/files#r1549622406

- removed check that verifies all the stackers have delegated to the same PoX address - addressed comments: #4628 (comment), #4628 (comment), #4628 (comment)

friedger

LGTM

moodmosaic

LGTM

moodmosaic · 2024-04-04T08:54:06Z

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

- removed check that verifies all the stackers have delegated to the same PoX address - addressed comments: - #4628 (comment) - #4628 (comment) - #4628 (comment)

blockstack-devops · 2024-11-02T00:20:24Z

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.

BowTiedRadone added 3 commits April 3, 2024 00:34

Add StackAggregationCommitCommand authorization-based

9fad446

Rename StackAggregationCommit to StackAggregationCommitAuth

708b703

Add StackAggregationCommitCommand signature-based

852b0c5

BowTiedRadone requested review from moodmosaic and friedger April 3, 2024 11:21

friedger reviewed Apr 3, 2024

View reviewed changes

contrib/core-contract-tests/tests/pox-4/pox_StackAggregationCommitAuthCommand.ts Outdated Show resolved Hide resolved

moodmosaic reviewed Apr 3, 2024

View reviewed changes

friedger reviewed Apr 3, 2024

View reviewed changes

setzeus reviewed Apr 3, 2024

View reviewed changes

contrib/core-contract-tests/tests/pox-4/pox_StackAggregationCommitAuthCommand.ts Outdated Show resolved Hide resolved

Address commands documentation comment

f2989e4

https://github.com/stacks-network/stacks-core/pull/4628/files/852b0c5ca8b3dd7c6f91a55e464b9095fa8776d5#r1549587969

setzeus reviewed Apr 3, 2024

View reviewed changes

contrib/core-contract-tests/tests/pox-4/pox_StackAggregationCommitAuthCommand.ts Outdated Show resolved Hide resolved

BowTiedRadone and others added 3 commits April 3, 2024 15:19

Address format comment

aa0c4ad

https://github.com/stacks-network/stacks-core/pull/4628/files#r1549544398 Co-authored-by: Nikos Baxevanis <[email protected]>

Address format comment

98a0ba6

https://github.com/stacks-network/stacks-core/pull/4628/files#r1549544650 Co-authored-by: Nikos Baxevanis <[email protected]>

Address documentation comment

bdbc1ef

https://github.com/stacks-network/stacks-core/pull/4628/files#r1549546009 Co-authored-by: Nikos Baxevanis <[email protected]>

setzeus reviewed Apr 3, 2024

View reviewed changes

contrib/core-contract-tests/tests/pox-4/pox_StackAggregationCommitSigCommand.ts Outdated Show resolved Hide resolved

setzeus reviewed Apr 3, 2024

View reviewed changes

contrib/core-contract-tests/tests/pox-4/pox_StackAggregationCommitSigCommand.ts Outdated Show resolved Hide resolved

BowTiedRadone and others added 4 commits April 3, 2024 15:26

Addres not needed new line comment

c966a8d

https://github.com/stacks-network/stacks-core/pull/4628/files#r1549557014 Co-authored-by: Nikos Baxevanis <[email protected]>

Address not needed new line comment

2bcb04d

https://github.com/stacks-network/stacks-core/pull/4628/files#r1549556367 Co-authored-by: Nikos Baxevanis <[email protected]>

Address committedAmount variable comment

49ee7e6

https://github.com/stacks-network/stacks-core/pull/4628/files#r1549622406

Address same pool pox address comment

7c09274

- removed check that verifies all the stackers have delegated to the same PoX address - addressed comments: #4628 (comment), #4628 (comment), #4628 (comment)

friedger approved these changes Apr 3, 2024

View reviewed changes

moodmosaic approved these changes Apr 4, 2024

View reviewed changes

moodmosaic merged commit 9eeb2d1 into feat/pox-4-stateful-property-testing Apr 4, 2024
1 of 2 checks passed

moodmosaic deleted the feat/pox-4-stateful-stack-agg-commit branch April 4, 2024 08:54

saralab added the stateful-testing label Apr 15, 2024

blockstack-devops added the locked label Nov 2, 2024

stacks-network locked as resolved and limited conversation to collaborators Nov 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

BowTiedRadone commented Apr 3, 2024

moodmosaic Apr 3, 2024

moodmosaic Apr 3, 2024

friedger Apr 3, 2024

friedger Apr 3, 2024

setzeus Apr 3, 2024

BowTiedRadone Apr 3, 2024

friedger Apr 3, 2024

friedger left a comment

friedger Apr 3, 2024

friedger Apr 3, 2024

codecov bot commented Apr 3, 2024 •

edited

Loading

friedger left a comment

moodmosaic left a comment

moodmosaic commented Apr 4, 2024

blockstack-devops commented Nov 2, 2024

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

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

Conversation

BowTiedRadone commented Apr 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

friedger left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 3, 2024 • edited Loading

Codecov Report

friedger left a comment

Choose a reason for hiding this comment

moodmosaic left a comment

Choose a reason for hiding this comment

moodmosaic commented Apr 4, 2024

blockstack-devops commented Nov 2, 2024

codecov bot commented Apr 3, 2024 •

edited

Loading