change(rpc): Add proposal capability to getblocktemplate

[arya2]

Motivation

We want to support using the getblocktemplate RPC in proposal mode to use in our acceptance test.

Closes #5684.

Complex Code or Requirements

The new CheckValidBlock state request needs to check the block against all of the usual acceptance rules.

Solution

• Add a state request that checks the contextual validity of a block by calling validate_and_commit with a clone of the non-finalized state
• Use a request enum in the chain/block verifiers with a CheckProposal variant that skips checking the solution and calls the state with CheckValidBlock instead of CommitBlock
• Check the mode and data fields of the getblocktemplate parameters
  • Deserialize the block bytes in the data field and call chain verifier with a CheckProposal request

Related changes:

• Fixes formatting in prop tests for checks in state service
• Accepts and ignores optional workid parameter

Review

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?
  • Does it change concurrent code, unsafe code, or consensus rules?
• How do you know it works? Does it have tests?

Follow Up Work

• Test the getblocktemplate RPC's response as a block template proposal in CI #5685
• Detailed reject reasons (if needed)

[teor2345]

@arya2 can you please rebase this PR on the latest main branch?

The diff currently shows changes from other PRs, which makes it hard to review.

[teor2345]

Looks good!

This is a large and risky change, but if we make some changes we can do it without breaking production code, or diverging from the consensus rules.

[codecov]

Codecov Report

Merging #5870 (5a0850c) into main (6e61066) will decrease coverage by 0.21%.
The diff coverage is 52.10%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5870      +/-   ##
==========================================
- Coverage   78.23%   78.02%   -0.22%     
==========================================
  Files         310      311       +1     
  Lines       38837    38919      +82     
==========================================
- Hits        30383    30365      -18     
- Misses       8454     8554     +100     

[teor2345]

I'm really happy with the way you've made this change clearer and more consistent, and made it easier to see that we are following the consensus rules.

For testing, I'd like to see a diff between a Zebra and zcashd block proposal response. I'm also happy to do that test, if you don't have synced instances ready.

This PR took me a while to review, because it changes about 40 files. We might want to think about how to split up future large changes into stages, or how we can avoid unnecessary changes. (This is a general comment, there's no need to split this PR!)

For example, I think we could have done the FinalizedState to ZebraDb change in another PR, which would have split out about 10 files from this PR.

[teor2345]

For testing, I'd like to see a diff between a Zebra and zcashd block proposal response. I'm also happy to do that test, if you don't have synced instances ready.

This is now handled in ticket #5803, so it doesn't need to block this PR or the CI tests.

[teor2345]

I'm happy for anyone to approve once we've fixed the previous block hash check.

[arya2]

This PR took me a while to review, because it changes about 40 files. We might want to think about how to split up future large changes into stages, or how we can avoid unnecessary changes. (This is a general comment, there's no need to split this PR!)

I agree with splitting changes into multiple PRs. I also liked how you organized the getblocktemplate refactor in PR #5837 for when there are a lot of changes in one PR.

[arya2]

An expect_request in the snapshot timed out:

---- methods::tests::snapshot::test_rpc_response_data stdout ----

The application panicked (crashed).
Message: timeout while waiting for a request
in zebra_test::mock_service::MockService<zebra_node_services::mempool::Request, zebra_node_services::mempool::Response, zebra_test::mock_service::PanicAssertion, alloc::boxed::Box<dyn core::error::Error+core::marker::Sync+core::marker::Send>>

https://github.com/ZcashFoundation/zebra/actions/runs/3886245054/jobs/6631117711#step:14:3001

Update: I made issue #5940 for fixing the test.

[teor2345]

Thanks for fixing up my suggestions!

[arya2]

Thanks for fixing up my suggestions!

Thanks for the excellent suggestions!

[teor2345]

Same network error as #5938 (comment)

[teor2345]

@Mergifyio update

[mergify]

update

✅ Branch has been successfully updated