refactor: move viem client type to upstream#565
Closed
silent-cipher wants to merge 8 commits into
Closed
Conversation
…ays cleanup
Pre-flight piece-status probe used to mark any deal cleaned_up whenever the
SP returned 404. That conflated two scenarios: pieces SPs legitimately
purged (terminated datasets, hard-removed pieces) and pieces SPs should
still serve (live datasets, scheduled-but-not-finalized removals). The
latter was being silently swept under skipped.piece_missing instead of
surfacing as a failed retrieval.
New flow in RetrievalService.performAllRetrievals:
1. Chain pre-check via PDPVerifier.pieceLive(dataSetId, pieceId).
- false: dataset terminated, piece never created, or piece hard-removed.
Mark deal cleaned_up, emit skipped.piece_missing, no SP probe needed.
- true: piece should be retrievable. Proceed.
2. SP HEAD probe on /pdp/piece/:pieceCid/status (HEAD with GET fallback on 405).
- 404 after pieceLive=true: SP failure. Emit failure.other, persist a
failed Retrieval row, keep deal in candidate pool for re-probing.
- 200/other: proceed to full retrieval.
Probe was GET, now HEAD; no body required.
Extracts isDataSetLive + its FWSS/SP-HTTP probes into a new
DatasetLivenessService (apps/backend/src/dataset-liveness/) so
RetrievalService doesn't depend on DealService. DealService.isDataSetLive
becomes a thin proxy. New isPieceLive method on the service reads
PDPVerifier.pieceLive via viem.
Updates docs/checks/events-and-metrics.md to document the new branching
behavior of skipped.piece_missing vs failure.other on the Retrieval path.
Probe-level tests for isDataSetLive in deal.service.spec.ts are skipped
with a TODO; coverage to be re-added in a new dataset-liveness spec.
Refs: #556
- New DatasetLivenessService spec ports the FWSS/SP-HTTP probe matrix
previously in deal.service.spec.ts and adds isPieceLive coverage:
happy/false/no-client/RPC error/abort.
- Retrieval pre-check now treats missing dataSetId/pieceId as an error
(emits failure.other and bails) instead of falling through to the SP
probe. Backfill of legacy null-ID deals is a prerequisite before this
branch deploys. The error path is loud enough to spot any stragglers.
- Add retrieval spec coverage for the missing-IDs branch and the HEAD
with GET-on-405 fallback.
- Delete the stale describe.skip("isDataSetLive") block from
deal.service.spec.ts; coverage now lives in the new spec.
Refs: #561 follow-up commit addressing peer-review consensus.
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors backend typing around the Synapse SDK’s underlying viem client by centralizing the client type in WalletSdkService and updating downstream consumers to use the stronger type instead of any/casts.
Changes:
- Introduced a typed
SynapseViemClientalias inWalletSdkServiceand changed_synapseClient/getSynapseClient()to use it. - Simplified
DatasetLivenessServiceto consumegetSynapseClient()without local type aliases or casts. - Minor retrieval pre-check refactor to pass
deal.dataSetIddirectly into the piece-liveness probe.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| apps/backend/src/wallet-sdk/wallet-sdk.service.ts | Replaces any/unknown with a concrete viem client type and adds a null-guard in ensureWalletAllowances(). |
| apps/backend/src/dataset-liveness/dataset-liveness.service.ts | Removes local viem client type alias and relies on WalletSdkService.getSynapseClient() typing. |
| apps/backend/src/retrieval/retrieval.service.ts | Simplifies checkPieceLive invocation by passing dataSetId directly. |
Base automatically changed from
feat/retrieval-mark-failed-when-piece-missing-on-live-dataset
to
main
May 22, 2026 12:59
Collaborator
|
@silent-cipher I didn't see this one, but I addressed your comment in my other PR. do we still need this? |
Collaborator
Author
|
yes, no need for this now |
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
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.
Built on top of #561