state service chain index integration #690
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 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.
Contributor
|
Got warnings discriminating the deprecated fields only. Work on individual rpcs will be easier if done on top of this 👍🏼 |
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...
…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
- 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
…_block_nullifiers
…_block_nullifiers
- Updated mempool height retrieval to use `snapshot_nonfinalized_state()`
get_raw_mempool method migrated to use chain_index
get_block_count method migrated to use chain_index
…_block_nullifiers
get_mempool_info method migrated to use chain_index
…_block_nullifiers
…_block_nullifiers
- Created zaino-common/src/status.rs with StatusType enum - zaino-state/src/status.rs now only contains AtomicStatus - AtomicStatus imports StatusType from zaino-common
- Added Status trait for types that report their StatusType - Blanket impls derive Liveness and Readiness from Status - Added is_live() and is_ready() methods on StatusType
- Changed ZcashService::status() trait method from async to sync - Updated StateService::status() to use self.indexer.status() instead of fetching directly from validator - Removed unused fetch_status_from_validator() method and poll_fn import - Updated FetchService::status() to be sync - Made zainod Indexer status methods sync: status_int(), status(), log_status(), check_for_critical_errors(), check_for_shutdown() This simplifies the status flow by sourcing status from ChainIndex, which already abstracts over the validator connection.
- Added Status as a supertrait bound on ZcashService trait - Implemented Status trait separately for StateService and FetchService - Removed redundant status() method from ZcashService trait definition - Implementors now automatically gain Liveness and Readiness via Status
- 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
Contributor
|
Fixes #318 |
Add readiness polling to test utilities after block generation These seem useful! Aren't they relevant to wider contexts than test?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fix #677
This deprecates all fields from
StateServiceandStateServiceSubscriberwithout deleting them, so that we can update the rpc implementations (ZcashIndexerandLightwalletdIndexertraits) 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