feat(rpc): add getblockheader endpoint

[dorianvp]

On top of #595, supersedes #473.

Motivation

#203.

Solution

Implement getblockheader.

PR Checklist

• The PR name is suitable for the release notes.
• The solution is tested.
• The documentation is up to date.

[pacu]

LGTM (untested)

[zancas]

I see CI fails...

[dorianvp]

I see CI fails...

Same flakes we're seeing on dev.

[ala-mode]

Nice job overall!

A couple things.

I didn't see any links to zcashd source code like ZL was doing for some previous rpcs (e.g. get_best_blockhash). If this is still something we want to do, someone should add links to the relevant source code, and make sure the logic follows that pattern, or any differences are understood and documented. If we're not following that anymore, please disregard.

Today I found myself fretting about the multiple fn get_block_header(...) in fetch and state backends, state indexer and jsonrpsee connector and service. This might be idiomatic to our project at this point, but I didn't love how this same name called itself on different levels, and then how there is also an inner function which is now 'legacy.'

I think we should at least understand but hopefully minimize this kind of name overloading.

Some other notes attached too.

I think I could probably pick up this PR and work with it in these ways, or I can leave these as requested changes and review again before long.

[ala-mode]

All tests passing locally with a few flakes (which I believe is a current good-standard).

[dorianvp]

All tests passing locally with a few flakes (which I believe is a current good-standard).

Extended the test to cover verbose responses & added a light reference to the zcashd implementation.

[ala-mode]

Hi @dorianvp! Please see my attached comments.

I was not sure how things were really derived from the zcash source code versus what's present in your tests or in the struct.. If you think I might be onto something good with my concrete suggested changes, please do go ahead an merge them in and then make changes on top of them - or whatever is most fruitful.

[dorianvp]

Comments addressed! Looks like I was adding a field that was present in getblock but not getblockheader. Good catch @ala-mode!

[zancas]

@ala-mode do you have any remaining concerns?

[ala-mode]

@ala-mode do you have any remaining concerns?

I didn't get to re-review, but I am sitting down to do it right now.

[ala-mode]

I approve, but I couldn't pass all tests so didn't merge, see below. A couple notes:

I noticed the Unknown variant in pub enum GetBlockHeader stayed in - It's not worth blocking over, but I still don't love this.. even as a library, the call is going to query full nodes that have responses described in the variants and types you (nicely!) created. Are there other cases?

(Even if we query another zaino hypothetically, it will have ingested the same info in a VerboseBlockHeader or String.)

Other than this I attached a couple comments which are also not blocking, but could point toward a future issue.

Running tests now, I'll merge if they all pass for me locally (right now, allowing flakes)

[ala-mode]

I had one test locally that was very slow:
integration-tests::wallet_to_validator zebrad::fetch_service::monitor_unverified_mempool

because I didn't complete this test, I didn't merge.

[ala-mode]

Just for referece that test passed for me (fairly quickly) on the 3rd try.