feat: use PDPVerifier.findPieceIdsByCid for efficient CID→ID lookups#718
feat: use PDPVerifier.findPieceIdsByCid for efficient CID→ID lookups#718Chaitu-Tatipamula wants to merge 2 commits intoFilOzone:masterfrom
Conversation
90b36bf to
c48c67e
Compare
There was a problem hiding this comment.
Pull request overview
This PR improves CID→pieceId lookups by switching from “fetch all pieces then search” patterns to a direct on-chain PDPVerifier.findPieceIdsByCid call, reducing RPC payload size and avoiding O(n) scans in the SDK.
Changes:
- Added
findPieceIdsByCid+findPieceIdsByCidCalltosynapse-corePDP verifier helpers (with mocks + presets support). - Updated
synapse-sdkpiece lookup logic (pieceStatus()and_getPieceIdByCID()) to usefindPieceIdsByCid(..., limit: 1n)instead of fetching full piece lists / SP dataset metadata. - Added/updated unit tests to cover the new call builder + mocked RPC behavior and the “piece not found” scenario.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/synapse-sdk/src/storage/context.ts | Replaces linear searches / SP dataset fetch with PDPVerifier.findPieceIdsByCid for piece existence + ID resolution. |
| packages/synapse-sdk/src/test/storage.test.ts | Updates pieceStatus() test setup to mock findPieceIdsByCid returning an empty result. |
| packages/synapse-core/src/pdp-verifier/find-piece-ids-by-cid.ts | Introduces the new contract read helper + call builder with sensible defaults. |
| packages/synapse-core/src/pdp-verifier/index.ts | Re-exports the new helper from the PDP verifier module entrypoint. |
| packages/synapse-core/src/mocks/jsonrpc/pdp.ts | Adds JSON-RPC mock handler support for findPieceIdsByCid. |
| packages/synapse-core/src/mocks/jsonrpc/index.ts | Extends the default mock preset to include findPieceIdsByCid. |
| packages/synapse-core/test/find-piece-ids-by-cid.test.ts | Adds unit tests for call builder defaults and mocked RPC return shapes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const parsedPieceCID = Piece.asPieceCID(pieceCid) | ||
| if (parsedPieceCID == null) { | ||
| throw createError('StorageContext', 'deletePiece', 'Invalid PieceCID provided') | ||
| } |
There was a problem hiding this comment.
In _getPieceIdByCID, the error context uses 'deletePiece' as the method name. This makes thrown errors misleading (they originate from getPieceIdByCID). Update the createError call to use 'getPieceIdByCID' here for accurate error reporting.
| if (pieceIds.length === 0) { | ||
| throw createError('StorageContext', 'deletePiece', 'Piece not found in data set') | ||
| } |
There was a problem hiding this comment.
This Piece not found in data set error is thrown from _getPieceIdByCID, but the createError context labels it as 'deletePiece'. Rename the method in the error context to 'getPieceIdByCID' so consumers can identify the real source of the failure.
| describe('pieceStatus()', () => { | ||
| const mockPieceCID = 'bafkzcibeqcad6efnpwn62p5vvs5x3nh3j7xkzfgb3xtitcdm2hulmty3xx4tl3wace' | ||
| it('should return exists=false when piece not in data set', async () => { | ||
| server.use( | ||
| Mocks.JSONRPC({ |
There was a problem hiding this comment.
Test name says it returns exists=false, but the assertion expects null (the current pieceStatus() API). Consider renaming the test to reflect the actual behavior (e.g., “returns null when piece not in data set”) to avoid confusion when reading failures.
Closes #667
Summary
Replaces expensive full-piece-list fetches with the new
PDPVerifier.findPieceIdsByCidcontract method for direct CID→ID lookups.Changes
synapse-core
findPieceIdsByCid+findPieceIdsByCidCallinpdp-verifier/findPieceIdsByCidhandler and default presetsynapse-sdk
pieceStatus(): ReplacedgetActivePieces()+.find()with singlefindPieceIdsByCid()call_getPieceIdByCID(): Replaced SP APIgetDataSet()+.find()withfindPieceIdsByCid()callTests
findPieceIdsByCidcall builder and mocked RPCpieceStatustest for "piece not found" scenarioWhy
Previously
pieceStatus()fetched all active pieces then searched linearly — O(n) and potentially fails on large datasets. Now it's a single targeted contract call.