Conversation
hugomrdias
commented
Feb 6, 2026
- remove pdp in sdk
- take File as upload input
- reorganize sp/pdp actions in core
- add sp response validation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
synapse-dev | ae45545 | Commit Preview URL Branch Preview URL |
Feb 10 2026, 05:14 PM |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
It would have been good if you had have separated these out, removal of pdp is fine, but this needs discussion: const file = new File([new Uint8Array([1, 2, 3, 4, 5])], "foo.txt");
const result = await synapse.storage.upload(file);I'd like to understand why we need to require a |
There was a problem hiding this comment.
why PdpDataSet / get-pdp-data-set? are you worried about the confusion with pdp/get-data-set? cause I'm not sure putting "pdp" in front really helps the case. maybe you have to accept "extended" or "enhanced" here .. cause that's what it is.
There was a problem hiding this comment.
no, sp/get-data-set will go away when we clean up the pieceStatus
theres:
- warmstorage.getDataSet which is the barebones contract call
- warmstorage.getPdpDateSet that adds pdp verifier data and PDP sp to the object
| ): Promise<getPdpDataSets.OutputType> { | ||
| const data = await getClientDataSets(client, options) | ||
|
|
||
| const promises = data.map(async (dataSet) => { |
There was a problem hiding this comment.
this is going to hurt for users like filecoin-pin demo, I wouldn't be surprised if it breaks for that case
There was a problem hiding this comment.
why, too many calls ?
we were already doing all theses calls before
There was a problem hiding this comment.
the sdk does not use this action
| }), | ||
| } | ||
| }) | ||
| .filter((piece) => !removals.includes(piece.id)), |
There was a problem hiding this comment.
hah, are you sure you want to do this? this feels like opt-in behaviour to me, the pieces are still there, they're only scheduled for removal and won't be removed from on-chain or on the SP until the next proving period
There was a problem hiding this comment.
that why theres the getActivePieces action with no dedupe same output
|
|
||
| export type CreateDataSetOptions = { | ||
| /** Whether the data set should use CDN. */ | ||
| cdn: boolean |
There was a problem hiding this comment.
changing convention here, not a firm objection but I think you should stick to the existing convention of withCDN unless you're planning on changing all of the instances and causing even more breakage
|
k, I've done a review, my biggest objection is requiring In #593 I have You could push that all the way down to Other items as mentioned:
Zod is good, PR could have been titled better! Probably best if it's not squash merged with that change. OTY |