change(rpc): check contextual validity of getblocktemplate response

[arya2]

Motivation

The getblocktemplate response must be contextually valid based on the best tip in the state.

Needs other getblocktemplate fields for testing the success case.

Closes #5376

Solution

• Add a new state request for checking the contextual validity of a prepared block without committing it.
• Make a prepared block with default nonce and solution fields to use the new state request.
• Update vectors and snapshot tests to use a MockService that returns a Validated response.

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?

[codecov]

Codecov Report

Merging #5630 (fada9d7) into main (a2f2a14) will decrease coverage by 0.00%.
The diff coverage is 50.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5630      +/-   ##
==========================================
- Coverage   78.72%   78.72%   -0.01%     
==========================================
  Files         305      305              
  Lines       38503    38503              
==========================================
- Hits        30313    30310       -3     
- Misses       8190     8193       +3     

[arya2]

qualifying the Arc::clone so it's obvious that nothing's being copied without looking at the type.

[arya2]

@Mergifyio update

[mergify]

update

✅ Branch has been successfully updated

[oxarbitrage]

This one will conflict with #5659 in the tests as i added some stuff from this PR there. I will prefer #5659 to go first just because i have another PR to push that is based on it and it will be easier for me to do it that way.

[arya2]

This one will conflict with #5659 in the tests as i added some stuff from this PR there. I will prefer #5659 to go first just because i have another PR to push that is based on it and it will be easier for me to do it that way.

Sounds good, I'd also prefer #5659 go first so this PR could use the populated target field for the difficulty_threshold of the prepared block.

[teor2345]

I think the ticket for this PR was a bit confusing. We need to change how the mempool checks transactions, so it always uses the best chain.

If we wait until the transactions are in the getblocktemplate response, it is too late, because all we can do is return an error to the caller. We don't know which transactions to remove to make the block valid.

Edit: It is also going to be tricky to verify the block, because the solution in the header requires Proof of Work. (Correct equihash solutions are expensive to create, so we can't do that in tests or in the RPC.)

We might want to turn this PR into a ticket, because the code looks like it could be useful for testing.

I'll add some specific notes to the ticket about where we want to change things.

[arya2]

I think the ticket for this PR was a bit confusing. We need to change how the mempool checks transactions, so it always uses the best chain.

If we wait until the transactions are in the getblocktemplate response, it is too late, because all we can do is return an error to the caller. We don't know which transactions to remove to make the block valid.

Oops. 🤦 I thought I just hadn't come across the code that was checking the nullifiers, I understand now.

It is also going to be tricky to verify the block, because the solution in the header requires Proof of Work. (Correct equihash solutions are expensive to create, so we can't do that in tests or in the RPC.)

The code looks like it could be useful for testing.

I believe the solution in the header is only checked by the block verifier. I created issue #5681 for adding this as a test.

I'm closing this because it would needlessly hurt performance in production.