refactor(provider-node): extract chain traits for testability and harden CI#145
Merged
Conversation
…den CI Extract chain interaction traits from 4 coordinators (agreement, challenge, checkpoint, replica-sync) so each can be tested with mock chain clients. Move Subxt implementations to dedicated `_subxt.rs` files. - Add unified `ProviderError` type in `error.rs` - Wire `Box<dyn Trait>` in `command.rs` startup - Make `sign()` fallible across API handlers - Add testability helpers to `fs_index.rs` and `s3_index.rs` - CI: combine 3 pallet coverage jobs, glob discovery, validation guards - CI: fix integration test flakiness - Fix `//Bob` nonce issue in S3 CI integration test
This was referenced Jun 8, 2026
ilchu
reviewed
Jun 9, 2026
…emove tempfile from PR 1 Replace manual `Pin<Box<dyn Future>>` return types with `#[async_trait]` across all 4 coordinator chain-client traits and their subxt impls: - AgreementChainClient - ChallengeChainClient - CheckpointChainClient - ReplicaSyncChainClient This preserves object safety (still `Box<dyn Trait>`) while making the code cleaner and easier to implement mock versions in tests. Also removes `tempfile` dev-dependency (unused in this PR; belongs in PR 3).
…struction Replace verbose Value::named_composite/unnamed_variant calls with the scale_value::value! macro across all _subxt.rs files. Also shorten fully-qualified subxt::dynamic::Value:: paths with a use import.
…paritytech/web3-storage into refactor/provider-node-trait-extraction
ilchu
approved these changes
Jun 10, 2026
Each coordinator trait now has an `impl<T: Trait> Trait for Arc<T>` that delegates to the inner value, eliminating the need for SharedMock newtype wrappers in tests.
…paritytech/web3-storage into refactor/provider-node-trait-extraction
The merge from dev incorrectly removed the "Resolve chain-spec script" step and left a reference to matrix.runtime.chain_spec_script which doesn't exist in this job (no matrix strategy). Restore the env var approach that reads from runtimes-matrix.json.
franciscoaguirre
left a comment
franciscoaguirre
left a comment
Collaborator
There was a problem hiding this comment.
What do you think about having the same struct implement all the different chain client traits? Would it reuse connections?
| -p pallet-storage-provider \ | ||
| -p pallet-drive-registry \ | ||
| -p pallet-s3-registry \ | ||
| --ignore-filename-regex='(weights\.rs|runtime_api\.rs|mock\.rs|primitives/|benchmarking\.rs|bechmarking\.rs)' \ |
| -p storage-provider-node \ | ||
| --ignore-filename-regex='(\.cargo|rustc|_subxt\.rs|command\.rs|cli\.rs|main\.rs)' \ | ||
| --summary-only 2>&1 \ | ||
| | grep '^TOTAL' | awk '{print $10}' | tr -d '%') |
Collaborator
There was a problem hiding this comment.
I know it's unlikely to cause any issues, but can we add --json and use jq to extract the percentage. That way we need to fiddle less with text and it's easier to read
| } | ||
|
|
||
| #[async_trait::async_trait] | ||
| impl CheckpointChainClient for SubxtCheckpointChainClient { |
Collaborator
There was a problem hiding this comment.
Feels like all these different traits could be implemented by the same struct to reuse the subxt api and signer. We can still keep the impls in different files
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Summary
#144
Split from #139 (PR 1 of 4).
_subxt.rsfiles.ProviderErrorinerror.rs, makesign()fallible across API handlers.fs_index.rsands3_index.rsfor test construction.//Bobnonce issue in S3 CI test.PR stack
devdevTest plan
SKIP_WASM_BUILD=1 cargo clippy -p storage-provider-node --all-targetspasses