Skip to content

Indexer trait#53

Merged
zancas merged 41 commits intodevfrom
indexer_trait
Mar 29, 2026
Merged

Indexer trait#53
zancas merged 41 commits intodevfrom
indexer_trait

Conversation

@zancas
Copy link
Copy Markdown
Member

@zancas zancas commented Mar 13, 2026

Fixes: #52

When tested against zingolib all lints and (140) tests pass.

This trait-based interface is going to be very useful for mocking out calls to the indexer. I wanted it to test offline-mode.

@zancas zancas marked this pull request as draft March 13, 2026 17:31
@dorianvp
Copy link
Copy Markdown
Member

This may be relevant: #55 (comment)

@zancas
Copy link
Copy Markdown
Member Author

zancas commented Mar 26, 2026

Does #55 block this PR, or can it be addressed next?

@dorianvp
Copy link
Copy Markdown
Member

I think the only blocker, or question, is whether we want to expose zcash_client_backend (a higher-level library) at such low level.

pacu
pacu previously approved these changes Mar 26, 2026
@zancas zancas mentioned this pull request Mar 27, 2026
zancas added 11 commits March 27, 2026 10:26
  Add `globally-public-transparent` feature gating all transparent address
  methods (get_taddress_*, get_address_utxos*) including the new
  client-streaming get_taddress_balance_stream(Vec<Address>). Off by
  default so consumers must opt in to transparent functionality.

  Add `back_compatible` opt-in feature providing get_zcb_client() which returns
  zcash_client_backend's CompactTxStreamerClient for pepper-sync compat
  during migration.

  Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
● All three clean — default, ping-only, and all features. Commit message:

  gate Ping RPC behind ping-very-insecure feature

  Add the Ping(Duration) -> PingResponse method to the Indexer trait,
  gated behind `ping-very-insecure`. The feature name mirrors the
  lightwalletd CLI flag `--ping-very-insecure` that must be set
  server-side to enable this endpoint.

  Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
  Move all transparent address methods into a TransparentIndexer: Indexer
  sub-trait with a single TransparentError associated type. The trait
  definition and GrpcIndexer impl live in src/globally_public.rs, gated
  by one #[cfg(feature = "globally-public-transparent")] on the module.

  Also:
  - Add ping-very-insecure feature gate for Ping RPC (name mirrors the
    lightwalletd --ping-very-insecure CLI flag required server-side)
  - Bound all associated error types with std::error::Error
  - Rename GetTreesError to GetTreeStateError to match the proto/method
  - Strengthen doc comments across all trait methods

  Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Addresses #53 (comment)

● verify trait-to-proto agreement and fix get_tree_state signature

  Change get_tree_state to accept BlockId instead of u64, matching the
  proto: GetTreeState(BlockID) returns (TreeState). Callers who have a
  height construct BlockId { height, hash: vec![] }; callers with a
  block hash can now use it directly.

  Endpoint-by-endpoint proto agreement audit:

    Exact matches (13):
      GetTreeState(BlockID) -> TreeState
      GetBlock(BlockID) -> CompactBlock
      GetBlockNullifiers(BlockID) -> CompactBlock
      GetBlockRange(BlockRange) -> stream CompactBlock
      GetBlockRangeNullifiers(BlockRange) -> stream CompactBlock
      GetTransaction(TxFilter) -> RawTransaction
      GetTaddressTxids(TABF) -> stream RawTransaction
      GetTaddressTransactions(TABF) -> stream RawTransaction
      GetTaddressBalance(AddressList) -> Balance
      GetMempoolTx(GetMempoolTxRequest) -> stream CompactTx
      GetSubtreeRoots(GetSubtreeRootsArg) -> stream SubtreeRoot
      GetAddressUtxos(GetAddressUtxosArg) -> GetAddressUtxosReplyList
      GetAddressUtxosStream(GetAddressUtxosArg) -> stream GetAddressUtxosReply
      Ping(Duration) -> PingResponse

    Intentional divergences (7):
      get_info() — hides Empty request arg
      get_latest_block() — hides ChainSpec request arg
      get_mempool_stream() — hides Empty request arg
      get_latest_tree_state() — hides Empty request arg
      send_transaction(Box<[u8]>) -> String — abstracts RawTransaction
        input and SendResponse output into raw bytes / txid string
      get_taddress_balance_stream(Vec<Address>) — models client-streaming
        as Vec, impl converts to stream internally
      ping(ProtoDuration) — alias avoids std::time::Duration collision

  Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
  Introduce src/proto_agreement.rs with 20 dead-code async fns that
  reference both the generated CompactTxStreamerClient method and the
  corresponding Indexer/TransparentIndexer trait method with explicit
  type annotations. If either side drifts from the proto, the test
  module fails to compile.

  The proto source of truth is walletrpc/service.proto from the
  lightwallet-protocol crate (version pinned in Cargo.toml), compiled
  from https://github.com/zingolabs/lightwallet-protocol-rust.

  TransparentIndexer tests are grouped in a `mod transparent` sub-module
  gated once by #[cfg(feature = "globally-public-transparent")]. Ping is
  gated by #[cfg(feature = "ping-very-insecure")].

  Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
● fix get_block_range doc: support ascending and descending order

  The proto spec says:
    "If range.start <= range.end, blocks are returned increasing height
     order; otherwise blocks are returned in decreasing height order."

  Our doc incorrectly claimed ascending-only. Fixed to document both
  directions. Added integration test
  `tests::get_block_range_supports_descending_order` that verifies
  descending order against zec.rocks (passes).

  Addresses #53 (comment)

  Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@zancas zancas requested review from dorianvp and pacu March 27, 2026 18:54
@zancas
Copy link
Copy Markdown
Member Author

zancas commented Mar 27, 2026

Adding explicit 1:1 method-error-enums where they were missing...

zancas added 5 commits March 27, 2026 12:22
  Replace RpcError and TransparentError with 20 dedicated error enums,
  one per trait method. Each enum has a GetClientError variant (connection
  failure) and a method-specific tonic::Status variant.
  SendTransactionError adds SendRejected(String).

  All error definitions live in src/error.rs with transparent errors in
  an error::transparent submodule gated by globally-public-transparent.

  Every error enum has:
  - Contract documentation explaining what each variant means
  - A doc-test proving From conversions and variant matching
  - Unit tests exercising each variant

  Feature-gated doc-tests use #[cfg] so cargo test --doc passes with or
  without features, and assertions execute when the feature is enabled.

  README and CHANGELOG updated with the full 20-row RPC/method/error
  audit table.

  Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
  Add organizing principle, feature gate table, and backwards
  compatibility section to the lib.rs module doc. Content is a strict
  subset of the README, covering what isn't directly accessible from
  doc comments or source: architectural intent, feature discovery, and
  migration guidance.

  Intra-doc links to feature-gated items (TransparentIndexer,
  Indexer::ping, GrpcIndexer::get_zcb_client) require --all-features.
  CI doc command updated to:
    RUSTDOCFLAGS="-D warnings" cargo doc --all-features --document-private-items

  Fix redundant explicit link in globally_public.rs module doc.

  Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
  Update ci-pr.yaml to use infrastructure@672fe8a which supports the
  doc-all-features input. This makes the doc phase run
  cargo doc --all-features --document-private-items instead of
  cargo-checkmate run doc, so intra-doc links to feature-gated items
  (TransparentIndexer, Indexer::ping, GrpcIndexer::get_zcb_client)
  resolve.

  Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@zancas
Copy link
Copy Markdown
Member Author

zancas commented Mar 27, 2026

There's a non-blocking CI-doc-test issue, that may resolve, but shouldn't prevent this code landing as I can get doc-tests to pass locally.

@zancas
Copy link
Copy Markdown
Member Author

zancas commented Mar 27, 2026

It passed!

@zancas zancas dismissed dorianvp’s stale review March 27, 2026 21:35

I believe I have addressed all concerns.

@zancas
Copy link
Copy Markdown
Member Author

zancas commented Mar 28, 2026

There's a non-blocking CI-doc-test issue, that may resolve, but shouldn't prevent this code landing as I can get doc-tests to pass locally.

I have resolved this CI issue.

Copy link
Copy Markdown

@juanky201271 juanky201271 left a comment

Choose a reason for hiding this comment

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

let's go...

@zancas zancas merged commit b4a8b61 into dev Mar 29, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

connection behavior needs to be abstracted across different specific backends, connection types need to be well defined

5 participants