Skip to content

feat(rpc): add getblockhash endpoint#642

Draft
dorianvp wants to merge 15 commits intodevfrom
feat/rpc-getblockhash
Draft

feat(rpc): add getblockhash endpoint#642
dorianvp wants to merge 15 commits intodevfrom
feat/rpc-getblockhash

Conversation

@dorianvp
Copy link
Copy Markdown
Member

@dorianvp dorianvp commented Nov 5, 2025

Motivation

#201.

Solution

Implement getblockhash.

Specifications & References

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

https://github.com/zcash/zcash/blob/16ac743764a513e41dafb2cd79c2417c5bb41e81/src/rpc/blockchain.cpp#L646

PR Checklist

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

@dorianvp dorianvp self-assigned this Nov 5, 2025
@dorianvp dorianvp linked an issue Nov 5, 2025 that may be closed by this pull request
1.5: Add Zcash RPC getblockhash #201
Open
@zancas zancas assigned ala-mode and unassigned dorianvp Nov 7, 2025
@zancas zancas requested a review from nachog00 November 7, 2025 00:57
zancas added 7 commits April 23, 2026 17:50
@zancas
Merge remote-tracking branch 'zingolabs/dev' into feat_rpc-getblockhash
9895a6d
@zancas
fix(getblockhash): align return types and clear post-merge lints
c80a3f6 
  `connector::get_blockhash` was returning `zebra_rpc::methods::GetBlockHash`
  (the deprecated alias for `GetBlockHashResponse`), which does not implement
  `ResponseToError` and therefore failed the `send_request` bound. Return the
  local `response::GetBlockHash` wrapper instead, matching the existing
  `get_best_blockhash`.

  `FetchServiceSubscriber::get_blockhash` now returns `GetBlockHashResponse`
  so the impl matches the `ZcashIndexer` trait signature; the stray local
  `GetBlockHash` import introduced during the dev merge is removed.

  Also adds the missing doc comments on the new `block_hash` module,
  `BlockSelector` enum, `connector::get_blockhash`, and the trait method;
  drops the dead `TryIntoHeight` import in `indexer.rs`; and silences the
  unused `serialized_block` binding in the `StateService` stub.

  Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zancas
chore(getblockhash): migrate to canonical GetBlockHashResponse name
414e2f1 
  `zebra_rpc::methods::GetBlockHash` is a `#[deprecated]` re-export of
  `GetBlockHashResponse`. Update every zaino-side usage (the `ZcashIndexer`
  trait and both impls, the JSON-RPC service trait + handler, the `From`
  impl target in `zaino-fetch`, and the `fetch_service` integration test)
  to reference `GetBlockHashResponse` directly. No behavioral change — the
  two names resolve to the same type.

  Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zancas
chore(getblockhash): move BlockSelector to jsonrpsee::request
b319f43 
  `BlockSelector` is the input parameter to `getblockhash`, not a response
  type, but it lived under `jsonrpsee::response::block_hash`. Create a new
  `jsonrpsee::request` module and relocate the file as
  `request::block_selector::BlockSelector` so the path reflects the type's
  role. Update the four importers (`connector.rs`, `backends/fetch.rs`,
  `backends/state.rs`, `indexer.rs`) accordingly.

  No behavioral change.

  Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zancas
feat(getblockhash): add exhaustive GetBlockHashError enum
2ea9f3f 
  `connector::get_blockhash` was returning `RpcRequestError<Infallible>`
  with a `// TODO: use correct error` note. Replace it with a typed error
  surface that reflects the RPC's actual failure mode.

  Add `GetBlockHashError::HeightOutOfRange(String)` in `response.rs`,
  mapping zcashd's `RPC_INVALID_PARAMETER` (code `-8`, "Block number out
  of range") via `TryFrom<RpcError>`. Introduce a `pub(crate)`
  `GetBlockHashByIndex` newtype around `GetBlockHash` whose
  `ResponseToError::RpcError = GetBlockHashError`; the connector
  deserializes into it internally and returns `GetBlockHash` to callers.

  `get_best_blockhash` keeps its `Infallible` error type — `GetBlockHash`'s
  own `ResponseToError` impl is untouched, so each method's error surface
  now matches its real behavior.

  Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zancas
test(getblockhash): regression coverage for new response types
35fd126 
  Two narrow `#[cfg(test)] mod` blocks in `response.rs`, each placed next
  to the item it exercises:

  - `get_block_hash_by_index` confirms the `#[serde(transparent)]` newtype
    deserializes the same hex JSON as the inner `GetBlockHash`, pinning
    the transparent wrapper contract.
  - `get_block_hash_error` confirms `TryFrom<RpcError>` maps code `-8` to
    `HeightOutOfRange` and passes every other code through unchanged as
    `Err(original)`, pinning the error-discrimination branch.

  Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zancas
feat(getblockhash): expose over the JSON-RPC server
7d01431 
  The `ZcashIndexer` trait and both backend impls already had
  `get_blockhash`, but `ZcashIndexerRpcServer` in `zaino-serve` only
  declared `getbestblockhash` — external JSON-RPC clients had no way to
  reach the endpoint. Add the `#[method(name = "getblockhash")]` trait
  declaration and the matching impl, delegating to
  `ZcashIndexer::get_blockhash` with the same error-mapping shape as its
  `getbestblockhash` sibling.

  Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 24, 2026
Open
Closed
zancas added 5 commits April 24, 2026 10:57
@zancas
test(getblockhash): add integration coverage across fetch and state…
4ad7907 
… backends

  Three new integration tests exercising the `getblockhash` endpoint:

  - `fetch_service::{zcashd,zebrad}::get::blockhash` — happy path via
    `FetchService`, verifying both `BlockSelector::Height(n)` (non-tip)
    and `BlockSelector::Tip` against `z_get_block` and `get_best_blockhash`
    reference values.
  - `state_service::zebra::get::blockhash_regtest` — parallel happy-path
    exercising the same two selectors through `StateService`.
  - `state_service::zebra::get::blockhash_out_of_range_returns_err` —
    regression asserting that an out-of-range height must surface as
    `Err(…)` rather than panicking. Fails red today via a `.unwrap()` in
    `StateService::get_blockhash`; turns green once the downstream fix
    for that stub lands.

  Also ships `tools/scripts/test-getblockhash.sh` — runs exactly the
  new tests plus one `wallet_to_validator::zcashd::connect_to_node_get_info`
  control via `makers container-test --manifest-path
  integration-tests/Cargo.toml -E '…'`.

  Two prerequisite fixups were required before the `integration-tests`
  workspace would even compile:

  - Phantom pin `core2 = "=0.3.3"` in `integration-tests/Cargo.toml`.
    `zcash_protocol ^0.7.2` transitively requires `core2 ^0.3`, all
    `0.3.x` versions are yanked, and the root `Cargo.lock` (which already
    pins 0.3.3) isn't shared with the integration-tests workspace — so
    fresh resolves fail. Tracked in #1043.
  - Rebase `zaino-testutils::generate_blocks_and_poll_chain_index` onto
    the new async `ChainIndex::snapshot_nonfinalized_state() ->
    Result<ChainIndexSnapshot, _>` API and read the tip via
    `best_chaintip(&snapshot)`. The `pub(crate) NonfinalizedBlockCacheSnapshot`
    shift made the old `.best_tip.height` field access unreachable from
    outside `zaino-state`; a friction note in the doc comment points
    readers at #1043 and #1044 for the broader cleanup.

  Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@zancas
Merge remote-tracking branch 'zingolabs/dev' into feat_rpc-getblockhash
e707886
@zancas
Merge branch 'dev' into feat/rpc-getblockhash
1728503
@zancas
Merge branch 'dev' into feat/rpc-getblockhash
9ff1011
@zancas
Merge branch 'dev' into feat/rpc-getblockhash
8a6ba90
@idky137 idky137 self-requested a review April 28, 2026 12:32
idky137
idky137 reviewed Apr 28, 2026
View reviewed changes
Comment thread packages/zaino-state/src/backends/fetch.rs
&self,
block_index: BlockSelector,
) -> Result<GetBlockHashResponse, Self::Error> {
Ok(self.fetcher.get_blockhash(block_index).await?.into())
Copy link
Copy Markdown
Contributor

@idky137 idky137 Apr 28, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we are migrating FetchService and StateService away from using direct access to the underlying data fetch mechanisms (#1062, #1065) we should implement the new RPC methods in this way to avoid creating new tech debt.

The following methods are available in the ChainIndex and should provide all functionality required for this new RPC:

NOTE: These methods can be accessed using self.indexer, eg:

let snapshot = self.indexer
    .snapshot_nonfinalized_state()
    .await
    .map_err( ... )?;
let hash = self.indexer
    .get_block_hash(
        &snapshot,
        Height(REQUESTED_HEIGHT),
    )
    .await
    .map_err( ... )?;

idky137
idky137 reviewed Apr 28, 2026
View reviewed changes
Comment thread packages/zaino-state/src/backends/state.rs
}
}

async fn get_blockhash(
Copy link
Copy Markdown
Contributor

@idky137 idky137 Apr 28, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to fetch.rs.

The current aim is to have the exact same implementations in both FetchService and StateService (apart from error types / propagation), so that when all methods have been moved to ChainIndex / BlockChainSource the two current IndexerService implementations can be merged, simplifying zaino's architecture.

@ala-mode ala-mode removed their assignment Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@idky137 idky137 idky137 left review comments

@nachog00 nachog00 Awaiting requested review from nachog00

At least 1 approving review is required to merge this pull request.

Assignees

@zancas zancas

@idky137 idky137

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1.5: Add Zcash RPC getblockhash

4 participants

@dorianvp @idky137 @zancas @ala-mode