-
Notifications
You must be signed in to change notification settings - Fork 15
feat: add piece deletion to storage context #285
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Uses Curio PDP APIs to trigger piece deletion resolves #207 Signed-off-by: Jakub Sztandera <[email protected]>
Deploying with
|
Status | Name | Latest Commit | Preview URL | Updated (UTC) |
---|---|---|---|---|
✅ Deployment successful! View logs |
synapse-dev | f5709ad | Commit Preview URL Branch Preview URL |
Oct 08 2025, 03:05 PM |
There are a few things I don't love about the implementation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'll do, I would assume that typically this isn't a common operation.
I also want to get away with asking the server for our piece list and use the contract instead (outlined here #249 (comment) but I realise from this that we still need piece ID).
@Kubuxu can you see a good reason why we validate on clientDataSetId and not just dataSetId? Would it be terrible if we just switched in the contract because event there we have indirection when we validate.
Opened #286 to deal with the extra call, I think we can make this nicer by just storing a copy of clientDataSetId when we have it on context creation. It's also used in AddPieces and we have another dedicated call for it then, although at least in that case we can do it in parallel with 3 other calls. |
* @param pieceCid - The PieceCID identifier | ||
* @returns Transaction hash of the delete operation | ||
*/ | ||
async deletePiece(pieceCid: string | PieceCID): Promise<string> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey, this could be:
async deletePiece(pieceCid: string | PieceCID): Promise<string> { | |
async deletePiece(piece: string | PieceCID | number): Promise<string> { |
and if it's a number
then we skip the pieceId look-up and just use that.
Then, if we do #249 (comment) and expose a getPieces(): AsyncGenerator<{ cid: PieceCID, id: number }>
on the context, the user can be in charge of choosing which piece to delete and if there's duplicates they can pinpoint exactly which one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, I've implemented accepting the pieceID
Signed-off-by: Jakub Sztandera <[email protected]>
cool, lgtm sans that |
Co-authored-by: Rod Vagg <[email protected]>
* feat: add piece deletion to storage context Uses Curio PDP APIs to trigger piece deletion resolves #207 Signed-off-by: Jakub Sztandera <[email protected]> * feat(delete): allow deletion of pieces by the pieceID Signed-off-by: Jakub Sztandera <[email protected]> * Update packages/synapse-sdk/src/storage/context.ts Co-authored-by: Rod Vagg <[email protected]> --------- Signed-off-by: Jakub Sztandera <[email protected]> Co-authored-by: Rod Vagg <[email protected]>
Uses Curio PDP APIs to trigger piece deletion resolves #207