fix: normalize empty outputs for pdp-verifier#723
Merged
hugomrdias merged 4 commits intomasterfrom Apr 16, 2026
Merged
Conversation
…imitMustBeGreaterThanZeroError - Introduced LimitMustBeGreaterThanZeroError for pagination limits. - Updated various PDP verifier functions to return 0 or null when the data set is not live. - Enhanced error handling in getActivePieces, getActivePieceCount, getDataSetLeafCount, getDataSetListener, getDataSetStorageProvider, getNextChallengeEpoch, and getNextPieceId functions. - Added utility functions for error string comparison and parsing. - Updated tests to cover new error handling scenarios.
- Changed the condition for nextChallengeEpoch to check for null instead of 0. - Added comments to clarify the implications of nextChallengeEpoch being null, indicating potential states of the data set. - Removed redundant debug logging for when nextChallengeEpoch is 0, streamlining the logic.
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
synapse-dev | 2c6009e | Commit Preview URL Branch Preview URL |
Apr 15 2026, 03:03 PM |
Contributor
There was a problem hiding this comment.
Pull request overview
Normalizes PDP Verifier contract “not live” / empty-ish states across synapse-core (and adjusts synapse-sdk usage) by converting specific revert reasons into consistent null/0/[] outputs, plus adds better multicall error mocking and a new limit-validation error.
Changes:
- Add
STRING_ERRORS+ helpers to detect/normalize “Data set not live” errors across PDP Verifier reads/multicalls. - Update PDP Verifier query helpers to return normalized outputs (e.g.,
0n,null,[]) and addLimitMustBeGreaterThanZeroError. - Expand/adjust tests and JSON-RPC mocks to preserve revert details in multicall results.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/synapse-sdk/src/storage/context.ts | Adjust piece timing logic to handle getNextChallengeEpoch returning null. |
| packages/synapse-core/test/jsonrpc-multicall.test.ts | Adds coverage ensuring aggregate3 multicall preserves mixed success/failure. |
| packages/synapse-core/test/get-scheduled-removals.test.ts | Adds tests for deduplication + not-live normalization to []. |
| packages/synapse-core/test/get-pieces.test.ts | Adds tests for scheduled-removal dedupe, not-live normalization, and negative limit validation. |
| packages/synapse-core/test/get-next-piece-id.test.ts | Adds test for not-live normalization to 0n. |
| packages/synapse-core/test/get-next-challenge-epoch.test.ts | Adds tests for null normalization (non-positive epoch + not-live). |
| packages/synapse-core/test/get-dataset-size.test.ts | Adds test ensuring not-live datasets contribute 0n size. |
| packages/synapse-core/test/get-data-set-storage-provider.test.ts | Updates expectations to allow null proposed provider + not-live normalization. |
| packages/synapse-core/test/get-data-set-listener.test.ts | Adds tests for zero-address → null and not-live → null. |
| packages/synapse-core/test/get-data-set-leaf-count.test.ts | Adds test for not-live normalization to 0n. |
| packages/synapse-core/test/get-active-pieces.test.ts | Adds tests for negative limit validation and not-live normalization. |
| packages/synapse-core/test/get-active-piece-count.test.ts | Adds test for not-live normalization to 0n. |
| packages/synapse-core/src/utils/contract-errors.ts | Introduces shared helpers/constants for matching known revert-string errors. |
| packages/synapse-core/src/pdp-verifier/get-scheduled-removals.ts | Adds not-live handling and factors dedupe into parseScheduledRemovals. |
| packages/synapse-core/src/pdp-verifier/get-pieces.ts | Adds limit validation, not-live handling, and avoids metadata requests when empty. |
| packages/synapse-core/src/pdp-verifier/get-next-piece-id.ts | Normalizes not-live to 0n. |
| packages/synapse-core/src/pdp-verifier/get-next-challenge-epoch.ts | Normalizes non-positive epochs and not-live to null via parseNextChallengeEpoch. |
| packages/synapse-core/src/pdp-verifier/get-dataset-size.ts | Uses allowFailure: true multicall to map not-live → 0n per dataset. |
| packages/synapse-core/src/pdp-verifier/get-data-set-storage-provider.ts | Normalizes not-live to null and converts zero proposed provider to null. |
| packages/synapse-core/src/pdp-verifier/get-data-set-listener.ts | Normalizes not-live to null and converts zero listener address to null. |
| packages/synapse-core/src/pdp-verifier/get-data-set-leaf-count.ts | Normalizes not-live to 0n. |
| packages/synapse-core/src/pdp-verifier/get-active-pieces.ts | Adds limit validation and normalizes not-live to an empty page. |
| packages/synapse-core/src/pdp-verifier/get-active-piece-count.ts | Normalizes not-live to 0n. |
| packages/synapse-core/src/mocks/jsonrpc/index.ts | Improves mock revert encoding and aggregate3 failure returnData handling. |
| packages/synapse-core/src/errors/pdp-verifier.ts | Adds LimitMustBeGreaterThanZeroError. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Member
|
Removed review request as I need to focus on pay explorer for now |
- Updated limit checks in getActivePieces and getPieces functions to ensure options.limit is not null before comparison. - Modified getPiecesWithMetadata to correctly return the hasMore property from pieces instead of a hardcoded false value. - Added export for pdp-verifier module to enhance functionality.
rvagg
reviewed
Apr 15, 2026
rvagg
reviewed
Apr 15, 2026
rvagg
reviewed
Apr 15, 2026
Comment on lines
+136
to
+138
| if (data <= 0n) { | ||
| return null | ||
| } |
Collaborator
There was a problem hiding this comment.
I htink this can be === 0n but also we're now returning null for two cases so we're limiting the amount of information we can present here; maybe we should just pass through 0n in this case because it does say something, we'd just need to document what that means (no challenge epoch set, as opposed to not live)
Member
Author
There was a problem hiding this comment.
is that information we need to exposes in this action?
my feeling is here we just say no challenge set in both cases, user can check if dataset is live with the specific action for that.
rvagg
approved these changes
Apr 15, 2026
Collaborator
rvagg
left a comment
rvagg
left a comment
There was a problem hiding this comment.
sgtm; I'll leave the comments up to your judgement though I reckon you could do better on the error matching in here
github-project-automation
bot
moved this from 🔎 Awaiting review
to ✔️ Approved by reviewer
in FOC
- Improved the stringErrorEquals function to better handle nested error causes and extract the reason from them. - Added a new test suite for error handling, including tests for detecting viem errors and matching revert reasons from nested causes. - Ensured comprehensive coverage for various error scenarios in contract error handling.
hugomrdias
changed the title
This was referenced Apr 15, 2026
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.
Handling these portions of #574