Skip to content

Conversation

@Oscar-Pepper
Copy link
Contributor

@Oscar-Pepper Oscar-Pepper commented Nov 24, 2025

fix #677

This deprecates all fields from StateService and StateServiceSubscriber without deleting them, so that we can update the rpc implementations (ZcashIndexer and LightwalletdIndexer traits) while still compiling fine.

Once those fields are completely un-relied upon by any endpoint, we can remove them and call this done.

NOTE: this work is done on top of PR #650, and it's expected to land before it

@nachog00 nachog00 linked an issue Nov 24, 2025 that may be closed by this pull request
- Moved `NodeBackedChainIndex`, `NodeBackedChainIndexSubscriber`, and `State` import to maintain alphabetical order
- Updated deprecation messages to be more informative regarding the new indexer's role and related issue #677
- Added more fields to the deprecation, as I believe they are part of what chain_index replaces, so we better get it out from the get go.
- Reordered fields in `StateService` and `StateServiceSubscriber` structs in `zaino-state/src/backends/state.rs`
- Reordering aligns with ongoing transition to new indexer field as part of #677 completion
Since the whole state_service module is annotated as deprecated alaready, the deprecation warnings that stem from the fields we marked as deprecated on this PR get confused with the rest. Also, there were allow(deprecated) exceptions that canceled everything out, so:

- commented out deprecated annotation on StateService, StateServiceSubscriber and StateServiceError
- commented out allow(deprecated) on implementors of the above mentioned

this way we now get the right warnings just for the fields we are deprecating on this PR

one apparent problem is that state.rs being a massive 2.5k single file module, we get all warnings for all method implementations at once. It would be WAY cleaner if we had a file per implemented method, that way it would be overly clear which methods have been properly ported already.
@nachog00
Copy link
Contributor

Got warnings discriminating the deprecated fields only. Work on individual rpcs will be easier if done on top of this 👍🏼

nachog00 and others added 23 commits November 28, 2025 20:47
the method now uses the chain index interface
I haven't run tests yet, we'll see if this is the right implementation
- Removed #[deprecated] annotations in `StateService` and `StateServiceSubscriber`

Since NodeBackedChainINdex can't yet provide those fields, we leave them un-deprecated on a first migration pass.

reference #677 (comment) for reference
- Commented out the #[deprecated] attribute on StateServiceConfig in config.rs

This cluttered the deprecation wrning for the current migratiion work.
- Removed `#[deprecated]` attribute from `mempool` field in `state.rs`

Since i realised that FetchService is also keeping this field... we might as well complete this migration without deprecating it either on stateservice.

I suspect all changes will boil down to nbci replacing the blockcache field...
- Added ChainIndex trait and chain_types imports
- Replaced block_cache.get_compact_block() with indexer methods
- Uses snapshot_nonfinalized_state() for consistent queries
- Handles both hash and height lookups via get_block_height()
…ck_cache

- Added ChainIndex trait and chain_types imports
- Use indexer.get_compact_block() then apply compact_block_to_nullifiers()
- Simplified height conversion with map_err
- Added ChainIndex, NonFinalizedSnapshot traits and chain_types imports
- Simplified height parsing with chained map_err
- Use indexer.get_compact_block() in a loop for start..=end range
- Get chain height from snapshot.best_chaintip()
- Removed complex backwards-walking via prev_hash
- Added NonFinalizedSnapshot to imports
- Get chain_height from indexer snapshot's best_chaintip()
- Updated import to include ChainIndexError in state.rs
- Replaced block_cache with compact_block retrieval from indexer
- Added error handling for missing compact block data
- Removed unused `NonEmpty` import for clean-up

replaced block_cache for indexer
nachog00 and others added 6 commits December 19, 2025 15:54
- Implement Status for NodeBackedChainIndexSubscriber (renamed status()
  to combined_status() to avoid ambiguity with trait method)
- Implement Status for FetchServiceSubscriber and StateServiceSubscriber
- Add Status bound to ZcashService::Subscriber associated type
- Add poll_until_ready() utility function in zaino-testutils using
  tokio::time::timeout + interval pattern
- Replace FIXME sleep in TestManager::launch with proper readiness polling
- Fix missing Status import in fetch_service integration test
Use chain index snapshot instead of querying Zebra directly. This ensures
consistency between get_latest_block() and get_block() which both now use
the same snapshot, preventing "database hole" errors when the chain index
hasn't synced yet but Zebra has already advanced.
Support multiple processes writing to the same ZainoDB by:
- Checking if block already exists before writing (same hash = no-op)
- Handling KeyExist race conditions when another process writes between check and write
- Verifying block hashes match in race condition scenarios
- Improving error messages with block hash details

Also removes debug statement from FetchService get_latest_block.
…unctions

After height polling completes, explicitly wait for readiness to ensure the
chain index is fully ready before proceeding. This fixes potential race
conditions where height is reached but index isn't fully ready yet.

Also adds Readiness trait bound to generate_blocks_and_poll_indexer.
get_block_nullifiers method migrated to use chain_index
@AloeareV
Copy link
Contributor

Fixes #318

nachog00 and others added 7 commits December 20, 2025 12:54
Add readiness polling to test utilities after block generation

These seem useful!  Aren't they relevant to wider contexts than test?
Introduce Status trait for service readiness probing
@idky137
Copy link
Contributor

idky137 commented Feb 3, 2026

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.

StateService migration to chainindex backend

5 participants