zebra icon indicating copy to clipboard operation
zebra copied to clipboard

Add support for `submitblock` RPC call

Open mpguerra opened this issue 1 year ago • 5 comments

Motivation

Mining pools use the submitblock RPC call.

Requirements

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

And any submission abbreviations required by the mining pool software: 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)
    • holds a single field, workid, that must be included in submissions if provided by the server

Possible Design

  • Check that Zebra has finished syncing
  • Add a FindBlockHashLocation request in zebra-state that returns Option<SideChain | BestChain | Pending | Finalized>
  • Call BlockVerifier with a Submit request

Return null if the block is accepted onto the best chain in the non-finalized state

Error handling

Return

  • "duplicate": if Zebra has already committed the block to the non-finalized or finalized state
  • "duplicate-invalid": if Zebra already has the block, but it is invalid
    • Zebra will never return this status, because it does not store invalid blocks
  • "duplicate-inconclusive": if Zebra already has the block in the state queue or channel, but has not committed it to the non-finalized state yet
  • "inconclusive": if Zebra has committed the block to the non-finalized state, but it is not on the best chain
  • "rejected": if Zebra rejected the block as invalid

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.

mpguerra avatar Sep 22 '22 09:09 mpguerra

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

teor2345 avatar Oct 05 '22 23:10 teor2345

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

mpguerra avatar Oct 06 '22 10:10 mpguerra

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

teor2345 avatar Oct 06 '22 21:10 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.

teor2345 avatar Oct 10 '22 20:10 teor2345

Or we could check which ones zcashd supports or requires.

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

arya2 avatar Oct 14 '22 17:10 arya2

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

teor2345 avatar Oct 20 '22 22:10 teor2345

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

arya2 avatar Oct 26 '22 01:10 arya2

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.

mpguerra avatar Oct 26 '22 08:10 mpguerra

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.

teor2345 avatar Oct 26 '22 21:10 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.

arya2 avatar Oct 28 '22 00:10 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.

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: https://github.com/ZcashFoundation/zebra/blob/7b47aac3703afe2e5efb45cdeb537a238ef45d4e/zebra-state/src/service/write.rs#L248-L249

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

teor2345 avatar Oct 28 '22 01:10 teor2345