Feat/async block proposal #4228

kantai · 2024-01-04T15:13:02Z

Description

Adds the /v2/block_proposal endpoint with async processing and callback over the event observer interface.

It does this by spawning a thread (if no such thread is currently running) and passing it a handle to an event dispatcher.

Applicable issues

Nakamoto[Block Validation]: Comms channels for RPC handler and block template/validation #3864 (it doesn't need a "comms channel" anymore, it just uses a thread with its own dispatcher).
Supercedes feat: Add file for /v2/block_proposal endpoint #4084

Todo

Update the API/RPC docs with the synchronous -> asynchronous changes.

codecov · 2024-01-04T15:29:58Z

Codecov Report

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

Comparison is base (92180f0) 82.78% compared to head (dc82c12) 82.94%.

Files	Patch %	Lines
stackslib/src/net/api/postblock_proposal.rs	80.14%	56 Missing ⚠️
...net/stacks-node/src/tests/nakamoto_integrations.rs	92.38%	24 Missing ⚠️
stackslib/src/chainstate/stacks/mod.rs	9.09%	10 Missing ⚠️
testnet/stacks-node/src/event_dispatcher.rs	73.68%	10 Missing ⚠️
testnet/stacks-node/src/config.rs	0.00%	4 Missing ⚠️
testnet/stacks-node/src/tests/neon_integrations.rs	92.85%	2 Missing ⚠️
stackslib/src/chainstate/burn/db/sortdb.rs	85.71%	1 Missing ⚠️
stackslib/src/net/api/mod.rs	50.00%	1 Missing ⚠️
stackslib/src/net/api/posttransaction.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             next    #4228      +/-   ##
==========================================
+ Coverage   82.78%   82.94%   +0.16%     
==========================================
  Files         429      430       +1     
  Lines      302897   303578     +681     
==========================================
+ Hits       250756   251809    +1053     
+ Misses      52141    51769     -372

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

docs/rpc/openapi.yaml

jcnelson · 2024-01-04T18:34:56Z

stackslib/src/net/api/postblock_proposal.rs

+    ///   - Miner signature is valid
+    /// - Validate threshold signature of stackers
+    /// - Validation of transactions by executing them agains current chainstate.
+    ///   This is resource intensive, and therefore done only if previous checks pass


It's really a shame that we can't easily overlap this with the similar code in NakamotoChainState without substantial refactoring. Perhaps this is just going to have to be a back-burner issue.

Yeah, I agree. The code overlap isn't so excessive, though, because most of the work here is already taking place through invocations of begin_tenure, etc. There's also a repeated code block here that needs to be removed: the stacker signature check. That cannot be part of the proposal validation (because the proposal validation is occurring to decide whether or not to sign in the first place).

stackslib/src/net/api/postblock_proposal.rs

jcnelson

LGTM. Thanks!

jcnelson · 2024-01-05T15:37:28Z

Seems your OpenAPI validation is failing

…o_block_burnchain()`

…oint()`

stackslib/src/net/api/postblock_proposal.rs

jferrant · 2024-01-08T17:36:53Z

testnet/stacks-node/src/tests/nakamoto_integrations.rs

+        p
+    };
+
+    let block = {


I wonder if I should use something like this rather than the test observer to validate the block written to stacker db in my .miners PR.

I think the approach you use in your PR is better for the .miners check: you want to confirm that the miner thread's actual block matches the data read out of the stackerdb, not that an independently assembled block matches either of those.

jbencin

Looks good, I think there's a couple issues in the docs that need attention before merging

jbencin · 2024-01-08T17:41:15Z

docs/rpc/openapi.yaml

+        Used by stackers to validate a proposed Stacks block from a miner.
+
+        **This endpoint will only accept requests over the local loopback network interface.**
+      responses:


Where is the 4xx error response for an invalid block?

Okay I see that on a 202 results are returned via event observer. There are still situations where you can get a 400 response, like a NetError

Right, those 400 responses are common to all API endpoints though.

jbencin · 2024-01-08T17:53:55Z

stackslib/src/chainstate/stacks/mod.rs

@@ -693,6 +693,22 @@ impl ToSql for ThresholdSignature {
    }
 }

+impl serde::Serialize for ThresholdSignature {


It looks like you are replacing the wsts::common::Signature Serde format, which is kinda complex, with a simple hex string. Good call

Yep, and this is already done for the SQL implementations, so it seemed like a straightforward improvement.

stackslib/src/net/api/postblock_proposal.rs

stackslib/src/net/api/posttransaction.rs

docs/rpc/openapi.yaml

jbencin · 2024-01-08T18:19:12Z

stackslib/src/net/p2p.rs

@@ -500,6 +506,23 @@ impl PeerNetwork {
        network
    }

+    pub fn set_proposal_thread(&mut self, thread: JoinHandle<()>) {


Should this check if self.block_proposal_thread is None first?

I don't think so -- the caller needs to have already checked this, because it shouldn't even get a JoinHandle if the block_proposal_thread is still running.

blockstack-devops · 2024-11-07T00:23:10Z

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 force-pushed the feat/async-block-proposal branch from bf23258 to d5f53ad Compare January 4, 2024 15:29

jcnelson reviewed Jan 4, 2024

View reviewed changes

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

jcnelson reviewed Jan 4, 2024

View reviewed changes

stackslib/src/net/api/postblock_proposal.rs Outdated Show resolved Hide resolved

jcnelson reviewed Jan 4, 2024

View reviewed changes

stackslib/src/net/api/postblock_proposal.rs Outdated Show resolved Hide resolved

jcnelson reviewed Jan 4, 2024

View reviewed changes

stackslib/src/net/api/postblock_proposal.rs Outdated Show resolved Hide resolved

jcnelson reviewed Jan 4, 2024

View reviewed changes

stackslib/src/net/api/postblock_proposal.rs Outdated Show resolved Hide resolved

jcnelson reviewed Jan 4, 2024

View reviewed changes

stackslib/src/net/api/postblock_proposal.rs Outdated Show resolved Hide resolved

jcnelson approved these changes Jan 5, 2024

View reviewed changes

kantai requested review from xoloki, jbencin and jferrant January 8, 2024 15:47

kantai force-pushed the feat/async-block-proposal branch from debe795 to d26812f Compare January 8, 2024 15:48

jbencin added 15 commits January 8, 2024 09:48

feat: Add file for /v2/block_proposal endpoint

a4e190a

Fill in some functions for /v2/block_proposal endpoint

90c820e

Partial validation of NakamotoBlockProposal using `validate_nakamot…

4aa56b6

…o_block_burnchain()`

Validate NakamotoBlockProposal against chainstate

8a10cf6

docs: Update CHANGELOG and rpc-endpoints.md

ef6e133

Remove unused fields from NakamotoBlockProposal; Update OpenAPI doc

7cfd9c4

Add two simple test cases for /v2/block_proposal (not passing)

86eda2c

Add a couple test cases

9d5fe9f

Remove total_burn from NakamotoBlockProposal

28ae167

chore: Address a couple PR comments

73eac46

chore: Replace hardcoded total_burn value

28fb11c

docs: Add JSON schema for /v2/block_proposal

ddb5e0b

Move NakamotoBlockProposal into postblock_proposal.rs

6281782

Remove decode_stacks_block_proposal_accepted()

4fda146

Add /v2/block_proposal integration test to bitcoin-tests.yml

41ad95b

jbencin and others added 7 commits January 8, 2024 09:50

chore: Address PR comments

ed8b425

test: Add more checking of HTTP responses in `block_proposal_api_endp…

03c525c

…oint()`

test: Add test case for /v2/block_proposal

db7a836

feat: Add threshold signature validation to /v2/block_proposal

3572194

feat: implement async block proposal interface

682d19c

remove stacker verification from block proposal check, update docs

a63c6a3

fix openapi error

96c4f9b

kantai force-pushed the feat/async-block-proposal branch from d26812f to 96c4f9b Compare January 8, 2024 15:50

jferrant reviewed Jan 8, 2024

View reviewed changes

stackslib/src/net/api/postblock_proposal.rs Outdated Show resolved Hide resolved

jferrant reviewed Jan 8, 2024

View reviewed changes

jferrant approved these changes Jan 8, 2024

View reviewed changes

jbencin reviewed Jan 8, 2024

View reviewed changes

address PR reviews

dc82c12

jbencin approved these changes Jan 8, 2024

View reviewed changes

kantai merged commit 2285877 into next Jan 9, 2024
2 checks passed

kantai deleted the feat/async-block-proposal branch January 9, 2024 15:00

jbencin mentioned this pull request Jan 9, 2024

feat: Add file for /v2/block_proposal endpoint #4084

Closed

4 tasks

kantai mentioned this pull request Jan 9, 2024

Nakamoto[Block Validation]: Comms channels for RPC handler and block template/validation #3864

Closed

blockstack-devops added the locked label Nov 7, 2024

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/async block proposal #4228

Feat/async block proposal #4228

kantai commented Jan 4, 2024

codecov bot commented Jan 4, 2024 •

edited

Loading

jcnelson Jan 4, 2024

kantai Jan 4, 2024

jcnelson left a comment

jcnelson commented Jan 5, 2024

jferrant Jan 8, 2024

kantai Jan 8, 2024

jbencin left a comment

jbencin Jan 8, 2024

jbencin Jan 8, 2024

kantai Jan 8, 2024

jbencin Jan 8, 2024

kantai Jan 8, 2024

jbencin Jan 8, 2024

kantai Jan 8, 2024

blockstack-devops commented Nov 7, 2024

Feat/async block proposal #4228

Feat/async block proposal #4228

Conversation

kantai commented Jan 4, 2024

Description

Applicable issues

Todo

codecov bot commented Jan 4, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcnelson left a comment

Choose a reason for hiding this comment

jcnelson commented Jan 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbencin left a 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

blockstack-devops commented Nov 7, 2024

codecov bot commented Jan 4, 2024 •

edited

Loading