fix(rpc): Add getblock RPC fields to support the latest version of zcash/lightwalletd

[teor2345]

Motivation

The latest version of zcash/lightwalletd expects the getblock response to always contain the block hash field:
https://github.com/zcash/lightwalletd/blob/5d174f7feb702dc19aec5b09d8be8b3d5b17ce45/common/common.go#L282

This PR gets the hash if needed, using a state index lookup.

Close #6085.

Specifications

"hash" : "hash", (string) the block hash (same as provided hash)

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

The height to hash lookup isn't specified, but we can assume they want the hash at that height on the best chain.

Complex Code or Requirements

For consistency even if the chain changes, we look up the block hash first, then use the hash to look up the transaction IDs and block confirmations.

Solution

• Stabilise the BestChainBlockHash state request
  • previously, it was behind the getblockchain-rpcs feature, but we need it for the hash lookup in production RPCs
• Always include the block hash in the getblock RPC response
  • Since we always have the hash, it's easy to always get confirmations as well

Testing:

• Update existing test vectors to always require the hash and confirmations fields
• Update snapshots
• Update lightwalletd integration test to work with zcash/lightwalletd

I manually checked that zcash/lightwalletd works with Zebra's integration tests. (In CI we use adityapk00/lightwalletd.)

Review

We might want to get this in the next release, but it's not a blocker.

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

We might want to test both lightwalletd forks in CI, to avoid issues like this in future.
@gustavovalverde can you put this in the CI review doc?

[codecov]

Codecov Report

Merging #6134 (2f54a52) into main (4f28929) will decrease coverage by 0.11%.
The diff coverage is 53.19%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6134      +/-   ##
==========================================
- Coverage   78.14%   78.03%   -0.11%     
==========================================
  Files         304      304              
  Lines       39087    39125      +38     
==========================================
- Hits        30546    30533      -13     
- Misses       8541     8592      +51     

[arya2]

Looks good to me! Thank you for fixing this.

Is the 'A-consensus' label correct? (is it for the confirmations field?)

[teor2345]

This blocks our next Zebra release because we want to be able to support this bug fix in lightwalletd:
adityapk00/lightwalletd#13

For the Zebra team: this is an example of a race condition where using a hash rather than a height matters.

[teor2345]

Is the 'A-consensus' label correct? (is it for the confirmations field?)

I could go either way, it fixes a consensus bug in lightwalletd, but it's not really consensus-critical in Zebra. It's just re-using existing consensus rule implementations.

Feel free to take it off if you'd like?