Reward set calculation and RPC endpoint #4311

kantai · 2024-01-30T18:56:09Z

Description

Implement #4295 and #4298

4295: Reading the reward set data from `.signers`

Implementation of #4295 is kind of subtle, and will have to impose additional constraints on Epoch-2.5 (one of which isn't implemented in this PR, but could be added, I suppose).

Reading the reward set state from the calculation performed for .signers is only possible after that calculation is performed (due to the general nature of causality and the linear passage of time). This calculation is performed during the first block of a prepare phase. This does not pose a challenge for nakamoto, because that's the block it chooses as the PoX anchor block anyways, so, of course it can read that data. For Epoch 2.5, however, this is a challenge: the PoX anchor block always comes before the prepare phase.

To deal with this, this PR splits the reward set loading by epoch -- in Nakamoto, it reads from the .signers, but in Epoch 2.5, it continues to use the 2.x reward set routine. However, we still want to ensure that the calculation done by the 2.x reward set routine at the PoX anchor block and the calculation performed by .signers work out to the same result. The PoX anchor block (if one is chosen) in any given fork is the last block before the prepare phase. This means that the .signers
calculation will always occur with the PoX anchor block as its anchor block parent. Because the 2.x reward set routine uses the state at the end of block processing, we can ensure that the reward set calculations are the same if:

The .signers calculation occurs before any burn operation processing.
There is no microblock.

This PR implements the first step there. For the second step, I'd propose to disable microblocks in Epoch 2.5.

4298: New RPC endpoint `/v2/stacker_set/{cycle_number}

This endpoint returns the same RewardSet JSON data propagated over the event observer interface. Callers must supply a cycle number to query for. It will return 400 if the set hasn't been calculated yet or if the requested cycle wasn't a PoX-4 cycle.

codecov · 2024-01-30T19:12:15Z

Codecov Report

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

Comparison is base (2d1f1ae) 44.92% compared to head (c5f4d81) 76.67%.

Files	Patch %	Lines
stackslib/src/net/api/getstackers.rs	5.14%	129 Missing ⚠️
...net/stacks-node/src/tests/nakamoto_integrations.rs	0.00%	26 Missing ⚠️
stackslib/src/chainstate/nakamoto/signer_set.rs	89.45%	25 Missing ⚠️
...ackslib/src/chainstate/nakamoto/coordinator/mod.rs	68.00%	16 Missing ⚠️
stackslib/src/chainstate/coordinator/tests.rs	0.00%	9 Missing ⚠️
stackslib/src/chainstate/coordinator/mod.rs	93.54%	2 Missing ⚠️
stackslib/src/chainstate/nakamoto/mod.rs	94.11%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             next    #4311       +/-   ##
===========================================
+ Coverage   44.92%   76.67%   +31.74%     
===========================================
  Files         443      445        +2     
  Lines      316901   317161      +260     
===========================================
+ Hits       142370   243184   +100814     
+ Misses     174531    73977   -100554

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

jcnelson · 2024-02-01T17:07:14Z

clarity/src/vm/database/clarity_db.rs

@@ -611,6 +611,22 @@ impl<'a> ClarityDatabase<'a> {
        self.store.insert_metadata(contract_identifier, key, data);
    }

+    /// Set a metadata entry if it hasn't already been set, yielding
+    ///  a runtime error if it was. This should only be called by post-nakamoto
+    ///  contexts.


Can you help me understand how this error could be triggered?

Actually, this function should just be removed -- at first I was going to try to store the computed reward set as metadata, in which case, this function might have been needed. But metadata storage in Clarity is very closely tied to contract instantiation, so it would be really cumbersome to have done things this way.

jcnelson · 2024-02-01T17:07:53Z

stackslib/src/chainstate/coordinator/mod.rs

+        //
+        //  Data **cannot** be read from `.signers` in epoch 2.5 because the write occurs
+        //   in the first block of the prepare phase, but the PoX anchor block is *before*
+        //   the prepare phase. Therefore


unfinished thought?

Yes, I think I completed the thought in my latest commit, but it's hard to know exactly where I was planning to go with that sentence.

stackslib/src/chainstate/coordinator/tests.rs

jcnelson · 2024-02-01T17:09:58Z

stackslib/src/chainstate/nakamoto/coordinator/mod.rs

+            .expect_optional()
+            .map(|x| u64::try_from(x.expect_u128()).expect("FATAL: block height exceeded u64"))
+        else {
+            if debug_log {


Is there a way to DRY this up? Perhaps a custom logging function?

Yeah, a new macro definition would be necessary. I can add.

Added a local macro for this.

stackslib/src/chainstate/nakamoto/coordinator/mod.rs

jcnelson · 2024-02-01T17:11:31Z

stackslib/src/chainstate/nakamoto/coordinator/mod.rs

+                    "checked_block" => %reward_set_block.index_block_hash()
+                );
+            } else {
+                error!(


Got a log discrepancy (needs to be DRY'ed up)

jcnelson · 2024-02-01T17:13:05Z

stackslib/src/chainstate/nakamoto/signer_set.rs

@@ -0,0 +1,387 @@
+// Copyright (C) 2013-2020 Blockstack PBC, a public benefit corporation
+// Copyright (C) 2020-2023 Stacks Open Internet Foundation


stackslib/src/net/api/getstackers.rs

jcnelson · 2024-02-01T17:20:53Z

stackslib/src/net/api/mod.rs

@@ -114,6 +115,7 @@ impl StacksHttp {
        self.register_rpc_endpoint(postmicroblock::RPCPostMicroblockRequestHandler::new());
        self.register_rpc_endpoint(poststackerdbchunk::RPCPostStackerDBChunkRequestHandler::new());
        self.register_rpc_endpoint(posttransaction::RPCPostTransactionRequestHandler::new());
+        self.register_rpc_endpoint(getstackers::GetStackersRequestHandler::default());


Miner code aesthetic: can you place the call to self.register_rpc_endpoint() in alphabetical order with the other RPC endpoints? So, after the one for getstackerdbmetadata?

Sure, but are we sure it shouldn't be after getstackerdbchunk and before getpoxinfo ;) ?

jcnelson · 2024-02-01T17:21:57Z

stackslib/src/chainstate/nakamoto/coordinator/tests.rs

@@ -68,13 +68,19 @@ fn advance_to_nakamoto(
    )
    .unwrap();

+    // use the signing key of addr, otherwise the test stackers
+    //  will not stack enough for any single signing key
+    // let signing_key = StacksPublicKey::from_private(&private_key);


Commented code

stackslib/src/chainstate/nakamoto/coordinator/tests.rs

jcnelson · 2024-02-02T18:08:52Z

LGTM; just fix merge conflicts and make sure CI passes. Thanks!

* expand e2e correct_burns test to invoke the new RPC endpoint * fix reward-set storage information to write correct coinbase height

jferrant

LGTM :D

obycode

This looks good. I just had two minor comments.

docs/rpc/api/core-node/get_stacker_set.400.example.json

docs/rpc/openapi.yaml

obycode

👍

blockstack-devops · 2024-11-06T00:20:41Z

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.

kantai self-assigned this Jan 30, 2024

This was linked to issues Jan 30, 2024

Reward set must be written to and read from .signers #4295

Closed

[Nakamoto] Create an RPC endpoint to retrieve the list of signer public keys and their total number of reward slots (vote shares) #4298

Closed

kantai force-pushed the feat/reward-set-endpoint branch 2 times, most recently from 105860f to b92dcdd Compare January 31, 2024 20:25

saralab requested review from friedger, jcnelson, MarvinJanssen and obycode February 1, 2024 14:51

jcnelson reviewed Feb 1, 2024

View reviewed changes

stackslib/src/chainstate/coordinator/tests.rs Show resolved Hide resolved

jcnelson reviewed Feb 1, 2024

View reviewed changes

stackslib/src/chainstate/nakamoto/coordinator/mod.rs Show resolved Hide resolved

jcnelson reviewed Feb 1, 2024

View reviewed changes

stackslib/src/net/api/getstackers.rs Show resolved Hide resolved

jcnelson reviewed Feb 1, 2024

View reviewed changes

stackslib/src/chainstate/nakamoto/coordinator/tests.rs Show resolved Hide resolved

kantai requested a review from jcnelson February 1, 2024 20:50

jcnelson approved these changes Feb 2, 2024

View reviewed changes

kantai added 6 commits February 2, 2024 20:05

feat: read reward set from state updated in .signers

61302a7

feat: add stacker_set RPC endpoint

c44ba36

* expand e2e correct_burns test to invoke the new RPC endpoint * fix reward-set storage information to write correct coinbase height

fix: no need to bump coinbase height

4cb5be5

chore: fmt-stacks

05e0f69

ci: add correct_burn_outs to workflows

85a243b

chore: address PR comments

6ded8d7

fix: address merge issues from next

c6a54db

kantai force-pushed the feat/reward-set-endpoint branch from cda02b1 to c6a54db Compare February 3, 2024 03:17

chore: cargo fmt-stacks

c90d0b4

jferrant approved these changes Feb 5, 2024

View reviewed changes

obycode reviewed Feb 5, 2024

View reviewed changes

docs/rpc/api/core-node/get_stacker_set.400.example.json Outdated Show resolved Hide resolved

docs/rpc/openapi.yaml Outdated Show resolved Hide resolved

kantai and others added 2 commits February 5, 2024 13:55

Merge branch 'next' into feat/reward-set-endpoint

a955244

fix get_stacker_set docs

2c120ad

obycode approved these changes Feb 5, 2024

View reviewed changes

kantai added 2 commits February 5, 2024 14:45

Merge branch 'next' into feat/reward-set-endpoint

9eeddbb

Merge branch 'next' into feat/reward-set-endpoint

c5f4d81

kantai merged commit 7c7a527 into next Feb 6, 2024
2 checks passed

jferrant mentioned this pull request Mar 18, 2024

Add an API endpoint for retrieving the current signer set from the stacks blockchain #3837

Closed

blockstack-devops added the locked label Nov 6, 2024

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reward set calculation and RPC endpoint #4311

Reward set calculation and RPC endpoint #4311

kantai commented Jan 30, 2024

codecov bot commented Jan 30, 2024 •

edited

Loading

jcnelson Feb 1, 2024

kantai Feb 1, 2024

jcnelson Feb 1, 2024

kantai Feb 1, 2024

jcnelson Feb 1, 2024

kantai Feb 1, 2024

kantai Feb 1, 2024

jcnelson Feb 1, 2024

jcnelson Feb 1, 2024

jcnelson Feb 1, 2024

kantai Feb 1, 2024

jcnelson Feb 1, 2024

jcnelson commented Feb 2, 2024

jferrant left a comment

obycode left a comment

obycode left a comment

blockstack-devops commented Nov 6, 2024

		@@ -0,0 +1,387 @@
		// Copyright (C) 2013-2020 Blockstack PBC, a public benefit corporation
		// Copyright (C) 2020-2023 Stacks Open Internet Foundation

Reward set calculation and RPC endpoint #4311

Reward set calculation and RPC endpoint #4311

Conversation

kantai commented Jan 30, 2024

Description

4295: Reading the reward set data from .signers

4298: New RPC endpoint `/v2/stacker_set/{cycle_number}

codecov bot commented Jan 30, 2024 • edited Loading

Codecov Report

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

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

jcnelson commented Feb 2, 2024

jferrant left a comment

Choose a reason for hiding this comment

obycode left a comment

Choose a reason for hiding this comment

obycode left a comment

Choose a reason for hiding this comment

blockstack-devops commented Nov 6, 2024

4295: Reading the reward set data from `.signers`

codecov bot commented Jan 30, 2024 •

edited

Loading