change(rpc): add submitblock RPC method #5526

arya2 · 2022-11-01T19:59:32Z

Motivation

Introduces basic support for the submitblock rpc method used by mining pools.

Closes #5236 as part of #5234

Designs

Data flow diagram

Specifications

https://zcash.github.io/rpc/submitblock.html

Solution

Add field for the block verifier to the GetBlockTemplateRpcImpl struct
Call the block verifier with the submitted block

Review

This PR is part of regular scheduled work.

Reviewer Checklist

Will the PR name make sense to users?
- Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
Are the PR labels correct?
Does the code do what the ticket and PR says?
How do you know it works? Does it have tests?

Follow Up Work

See Add submitblock test to CI #5527
See Return detailed errors from the submitblock RPC #5487
Update acceptance test (similar to Get transactions from the non-finalized state in the send transactions test #5015)

codecov · 2022-11-01T21:19:48Z

Codecov Report

Merging #5526 (2ebe202) into main (34313b8) will increase coverage by 0.08%.
The diff coverage is 50.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5526      +/-   ##
==========================================
+ Coverage   78.76%   78.85%   +0.08%     
==========================================
  Files         305      305              
  Lines       38114    38116       +2     
==========================================
+ Hits        30022    30057      +35     
+ Misses       8092     8059      -33

teor2345 · 2022-11-02T01:55:09Z

@Mergifyio update

mergify · 2022-11-02T01:55:45Z

update

✅ Branch has been successfully updated

teor2345 · 2022-11-02T04:30:46Z

@Mergifyio update

mergify · 2022-11-02T04:31:24Z

update

✅ Branch has been successfully updated

teor2345 · 2022-11-02T05:53:49Z

Looks great!

I think we might need to fix a few things before we merge:

use the null JSON value rather than the string "null" for Accepted blocks
add a snapshot test that submits an invalid block (an empty vector is ok!)

I also opened a PR with a refactor that simplifies this code a bit, with some minor doc bug fixes:

suggestion: Use the same ChainVerifier for downloaded and submitted blocks #5533

teor2345 · 2022-11-02T05:55:18Z

Here are some details:

I think submit_block::Response::Accepted will return a JSON string:
https://docs.rs/serde_json/latest/src/serde_json/ser.rs.html#213

"null"

But what is actually expected is the JSON null value:
https://docs.rs/serde_json/latest/src/serde_json/ser.rs.html#203

null

It looks like the easiest way to generate a null is by returning a Result<Option<RejectionReason>> from the RPC, and making Accepted into Ok(None):
https://docs.rs/serde_json/latest/src/serde_json/ser.rs.html#270

oxarbitrage

I think it looks great. I added a few minor comments while the only really blocking is the test failure.

I was not able to run submitblock test locally yet but i will.

zebra-rpc/src/methods/get_block_template_rpcs.rs

zebra-rpc/src/methods/tests/snapshot.rs

zebra-rpc/src/methods/tests/vectors.rs

zebrad/tests/common/getblocktemplate.rs

zebrad/tests/acceptance.rs

arya2 · 2022-11-02T20:10:11Z

use the null JSON value rather than the string "null" for Accepted blocks

🤦 Oops.

It looks like the easiest way to generate a null is by returning a Result<Option>

I used an untagged enum which behaves the same way, let me know if you think Option makes more sense.

add a snapshot test that submits an invalid block (an empty vector is ok!)

The empty hexdata gives a deserialization error. 😐 I could try using Mainnet and BAD_BLOCK_MAINNET_202_BYTES (https://github.com/ZcashFoundation/zebra/blob/v1.0.0-rc.0/zebra-test/src/vectors/block.rs#L337) if that's more in line with what you had in mind.

teor2345 · 2022-11-02T20:41:16Z

add a snapshot test that submits an invalid block (an empty vector is ok!)

The empty hexdata gives a deserialization error. 😐 I could try using Mainnet and BAD_BLOCK_MAINNET_202_BYTES (https://github.com/ZcashFoundation/zebra/blob/v1.0.0-rc.0/zebra-test/src/vectors/block.rs#L337) if that's more in line with what you had in mind.

Both tests seem good!

Let's name the snapshot files after the kind of failure we expect?

teor2345

Thanks for this!

There are a bunch of minor changes we can do, but none of them are blockers.

I've made some suggestions for cleaning up the acceptance test, but I'd like to deal with any further revisions in another PR. (When we do #5015 for this acceptance test.)

zebra-rpc/src/methods/get_block_template_rpcs.rs

zebra-rpc/src/methods/tests/vectors.rs

zebra-rpc/src/server.rs

zebrad/tests/acceptance.rs

zebra-rpc/src/methods/get_block_template_rpcs.rs

zebrad/tests/common/get_block_template_rpcs.rs

Co-authored-by: Alfredo Garcia <[email protected]>

…to chain_verifier to keep it consistent with naming in zebra-consensus

…mit_block method from server error to parse error

oxarbitrage · 2022-11-03T18:41:40Z

please fix conflicts, thanks :)

zebra-rpc/src/errors.rs

zebra-rpc/src/methods/get_block_template_rpcs/types/submit_block.rs

teor2345

Is the RPC JSON response in the expected format?

All my other suggestions are optional.

...a-rpc/src/methods/tests/snapshot/snapshots/snapshot_rpc_submit_block_invalid@mainnet_10.snap

zebrad/tests/common/get_block_template_rpcs.rs

zebrad/tests/common/get_block_template_rpcs/submit_block.rs

…nd removes the unrelated make_server_error refactor for now

arya2 · 2022-11-04T00:11:19Z

Is the RPC JSON response in the expected format?

They are, but the snapshot test was not.

teor2345

Thanks!

teor2345 · 2022-11-04T02:18:25Z

@Mergifyio update

mergify · 2022-11-04T02:19:03Z

update

✅ Branch has been successfully updated

arya2 added A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement P-Medium ⚡ A-rpc Area: Remote Procedure Call interfaces C-feature Category: New features labels Nov 1, 2022

arya2 requested a review from a team as a code owner November 1, 2022 19:59

arya2 requested review from dconnolly and removed request for a team November 1, 2022 19:59

arya2 changed the title ~~adds submitblock rpc method~~ change(rpc): add submitblock RPC method Nov 1, 2022

arya2 requested review from teor2345, oxarbitrage and a team and removed request for dconnolly November 1, 2022 20:05

arya2 force-pushed the submitblock branch 2 times, most recently from 60ec68d to 618a33d Compare November 1, 2022 20:14

arya2 mentioned this pull request Nov 1, 2022

Add submitblock test to CI #5527

Closed

teor2345 mentioned this pull request Nov 2, 2022

suggestion: Use the same ChainVerifier for downloaded and submitted blocks #5533

Merged

2 tasks

oxarbitrage requested changes Nov 2, 2022

View reviewed changes

arya2 requested a review from oxarbitrage November 2, 2022 21:24

arya2 force-pushed the submitblock branch from 6a6f6f2 to 3322752 Compare November 2, 2022 22:05

teor2345 reviewed Nov 2, 2022

View reviewed changes

teor2345 and others added 9 commits November 3, 2022 14:39

Document currently-unreachable code

d04ad71

updates tests and submit_block response for AlreadyVerified error

2160fbb

Update zebra-rpc/src/methods/get_block_template_rpcs.rs

daffc8f

Co-authored-by: Alfredo Garcia <[email protected]>

changes names from BlockVerifier to ChainVerifier and block_verifier …

6595f07

…to chain_verifier to keep it consistent with naming in zebra-consensus

move how to run the submit_block test example to acceptance.rs

3635525

updates snapshot tests

e9e7a82

moved acceptance test to a separate file

6b68c07

removes extra tower::ServiceBuilder::new(), updates docs

0267fb4

updates vectors and snapshot tests, changes hex decoding error in sub…

2640b3d

…mit_block method from server error to parse error

arya2 force-pushed the submitblock branch from 54cf105 to 2640b3d Compare November 3, 2022 18:41

oxarbitrage reviewed Nov 3, 2022

View reviewed changes

zebra-rpc/src/errors.rs Outdated Show resolved Hide resolved

hides errors module in zebra-rpc behind a feature flag and adds docs.

dd181c1

oxarbitrage previously approved these changes Nov 3, 2022

View reviewed changes

teor2345 reviewed Nov 3, 2022

View reviewed changes

zebra-rpc/src/errors.rs Outdated Show resolved Hide resolved

teor2345 reviewed Nov 3, 2022

View reviewed changes

zebra-rpc/src/methods/get_block_template_rpcs/types/submit_block.rs Outdated Show resolved Hide resolved

teor2345 suggested changes Nov 3, 2022

View reviewed changes

...a-rpc/src/methods/tests/snapshot/snapshots/snapshot_rpc_submit_block_invalid@mainnet_10.snap Outdated Show resolved Hide resolved

zebrad/tests/common/get_block_template_rpcs.rs Show resolved Hide resolved

zebrad/tests/common/get_block_template_rpcs/submit_block.rs Show resolved Hide resolved

Updates snapshot test, adds mod docs, moves HexData to its own mod, a…

cf4e937

…nd removes the unrelated make_server_error refactor for now

arya2 dismissed oxarbitrage’s stale review via cf4e937 November 3, 2022 23:39

arya2 mentioned this pull request Nov 3, 2022

Refactor RPC methods to use HexData and an error constructor #5548

Closed

update submit block acceptance test mod doc

4959a78

teor2345 approved these changes Nov 4, 2022

View reviewed changes

Merge branch 'main' into submitblock

2ebe202

teor2345 mentioned this pull request Nov 4, 2022

4. change(rpc): Refactor RPCs using a state tip height function #5540

Merged

5 tasks

mergify bot merged commit 2f3b05f into main Nov 4, 2022

mergify bot deleted the submitblock branch November 4, 2022 03:57

dconnolly mentioned this pull request Dec 5, 2022

release(1.0.0-rc.2): updated versions and CHANGELOG.md #5784

Merged

33 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

change(rpc): add submitblock RPC method #5526

change(rpc): add submitblock RPC method #5526

arya2 commented Nov 1, 2022 •

edited

Loading

codecov bot commented Nov 1, 2022 •

edited

Loading

teor2345 commented Nov 2, 2022

mergify bot commented Nov 2, 2022

teor2345 commented Nov 2, 2022

mergify bot commented Nov 2, 2022

teor2345 commented Nov 2, 2022 •

edited

Loading

teor2345 commented Nov 2, 2022

oxarbitrage left a comment •

edited

Loading

arya2 commented Nov 2, 2022

teor2345 commented Nov 2, 2022

teor2345 left a comment

oxarbitrage commented Nov 3, 2022

teor2345 left a comment

arya2 commented Nov 4, 2022

teor2345 left a comment

teor2345 commented Nov 4, 2022

mergify bot commented Nov 4, 2022

change(rpc): add submitblock RPC method #5526

change(rpc): add submitblock RPC method #5526

Conversation

arya2 commented Nov 1, 2022 • edited Loading

Motivation

Designs

Specifications

Solution

Review

Reviewer Checklist

Follow Up Work

codecov bot commented Nov 1, 2022 • edited Loading

Codecov Report

teor2345 commented Nov 2, 2022

mergify bot commented Nov 2, 2022

✅ Branch has been successfully updated

teor2345 commented Nov 2, 2022

mergify bot commented Nov 2, 2022

✅ Branch has been successfully updated

teor2345 commented Nov 2, 2022 • edited Loading

teor2345 commented Nov 2, 2022

oxarbitrage left a comment • edited Loading

Choose a reason for hiding this comment

arya2 commented Nov 2, 2022

teor2345 commented Nov 2, 2022

teor2345 left a comment

Choose a reason for hiding this comment

oxarbitrage commented Nov 3, 2022

teor2345 left a comment

Choose a reason for hiding this comment

arya2 commented Nov 4, 2022

teor2345 left a comment

Choose a reason for hiding this comment

teor2345 commented Nov 4, 2022

mergify bot commented Nov 4, 2022

✅ Branch has been successfully updated

arya2 commented Nov 1, 2022 •

edited

Loading

codecov bot commented Nov 1, 2022 •

edited

Loading

teor2345 commented Nov 2, 2022 •

edited

Loading

oxarbitrage left a comment •

edited

Loading