Add basic support for the submitblock RPC call

[mpguerra]

Motivation

Mining pools use the submitblock RPC call.

Mining Pool Usage

s-nomp calls submitblock from this code:

• https://github.com/s-nomp/node-stratum-pool/blob/d86ae73f8ff968d9355bb61aac05e0ebef36ccb5/lib/pool.js#L245

Requirements

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

Detailed specification:
https://en.bitcoin.it/wiki/BIP_0023#Submission_Abbreviation

The goals of the submitblock RPC are to:

• get valid blocks onto the local and network chains as quickly as possible, so miners have the best chance of profiting from their work, and
• reject invalid blocks as quickly as possible, so miners can start on an updated block template.

Arguments:

• hexdata (required, the hex-encoded block data to submit)
• jsonparametersobject (optional, currently ignored by zcashd, should be ignored by Zebra)
  • holds a single field, workid, that must be included in submissions if provided by the server

Possible Design

• Check that Zebra has finished syncing
• From the RPC, call zebra_consensus::ChainVerifier to verify the block
• Return null if the block is accepted onto the best chain in the non-finalized state, otherwise return whatever error text Zebra returns

Error handling

Detailed error handling is out of scope, see #5487.

zcashd implementation

https://github.com/zcash/zcash/blob/6c392dbb0b43d6988283e40ce423242833df2d30/src/rpc/mining.cpp#L869-L905

Testing

• Snapshot the output of the RPC
• Test calling the RPC from the mining pool software
• Manually compare the zcashd and zebrad RPC output using https://github.com/ZcashFoundation/zebra/blob/main/zebra-utils/zcash-rpc-diff

We might also want to implement #5015 to make writing tests for this issue quicker.

[teor2345]

Hey @oxarbitrage you're using an outdated RPC documentation site, the latest one is https://zcash.github.io/rpc/

[mpguerra]

Hey team! Please add your planning poker estimate with Zenhub @arya2 @conradoplg @dconnolly @oxarbitrage @teor2345 @upbqdn

[teor2345]

@arya2 I rewrote the RPC return values in terms of Zebra's implementation, because it's a bit different from zcashd's.

[teor2345]

@arya2 I'm not sure if this ticket is ready to go yet, I think we'll also need to check which submission abbreviations are required by the mining pool software:
https://en.bitcoin.it/wiki/BIP_0023#Submission_Abbreviation

Or we could check which ones zcashd supports or requires.

[arya2]

Or we could check which ones zcashd supports or requires.

It looks like zcashd doesn't support or require submission abbreviations.

[teor2345]

@arya2 do miners rely on the error values returned by submitblock?

If they don't, we can just return a generic error.

If they do, we could try implementing the errors by changing the existing BoxError returned by zebra-consensus and zebra-state?

Most of the error statuses can be added using a quick in-memory function call. And they only need to be added when there is an error or chain fork, so we can skip them for most blocks.

I'm not sure about duplicate-inconclusive - that might need a separate validator and state request to check the queues and channels. But we could also just say "our architecture doesn't support that".

[arya2]

@teor2345 I'm not sure how the error values are being used, a generic error may be good enough. I added a check_state_for_duplicate method that looks at the queued/sent blocks for duplicate-inconclusive and a SubmitBlock request that uses it, but we can remove those if they're unnecessary. It might make sense to cache the max height of a block received by the state service so the duplicate check can be skipped in most high-impact cases if it's kept.

[mpguerra]

let's just go for generic errors for now, I don't think we have enough information yet and if we can save ourselves some time in the implementation better. We can always add more error types later on.

[teor2345]

I split out detailed error handling into #5487, and made that ticket optional. I'll update the design as well.

I also tweaked the description to match the current implementation.

[arya2]

@teor2345 I think this should be coupled with #5487 so we can check can_fork_chain_at when submitted blocks are received and reject them in a timely manner when their parent blocks are absent.

[teor2345]

@teor2345 I think this should be coupled with #5487 so we can check can_fork_chain_at when submitted blocks are received and reject them in a timely manner when their parent blocks are absent.

I don't think a missing parent will ever happen with submitblock?

The submitted block is based on a getblocktemplate with a parent block of Zebra's chain tip. And Zebra keeps all the non-finalized chains for 100 blocks. So if the parent block is missing, either Zebra or the mining pool is doing something very wrong.

I am also concerned that combining this ticket with #5487 is a really large change for one PR. I'd prefer to keep the required RPC separate from the optional error-handling, until we find out if the error handling is needed.

Can we start by implementing a basic submitblock which calls CommitBlock and waits for the result?
It shouldn't have to wait long, because the block write task returns the result as soon as it can:

zebra/zebra-state/src/service/write.rs

Lines 248 to 249 in 7b47aac

	// Update the caller with the result.
	let _ = rsp_tx.send(result.clone().map(|()| child_hash).map_err(BoxError::from));

(If submitting blocks takes too long, we can fix issues like #1968, #4794, #5425, or #4672, which will benefit all Zebra users. Or we can return a "pending" result, or do whatever zcashd does.)