Skip to content

fix(permissions): remove DeleteDataSet#694

Open
wjmelements wants to merge 1 commit intomasterfrom
rm-delete-permission
Open

fix(permissions): remove DeleteDataSet#694
wjmelements wants to merge 1 commit intomasterfrom
rm-delete-permission

Conversation

@wjmelements
Copy link
Copy Markdown
Contributor

Reviewer @hugomrdias
Closes #693
This permission was removed in FilOzone/filecoin-services#255
Session keys can create but cannot delete data sets :(

Changes

  • remove removed permission from SDK

@github-project-automation github-project-automation bot moved this to 📌 Triage in FOC Mar 24, 2026
@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
synapse-dev 237e1e1 Commit Preview URL

Branch Preview URL
Mar 24 2026, 08:46 PM

| `CreateDataSetPermission` | Create new datasets |
| `AddPiecesPermission` | Add pieces to datasets |
| `SchedulePieceRemovalsPermission` | Schedule piece removals |
| `DeleteDataSetPermission` | Delete datasets |
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we try to fix and upgrade the contracts instead? This permission is useful (if it works)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, agreed.

I think we should track down whether it was intentional to remove DeleteDataSetPermission. I don't remember this being discussed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would require changing this line in PDPVerifier.deleteDataSet:

require(storageProvider[setId] == msg.sender, "Only the storage provider can delete data sets");

We would move the security check into FWSS. That change is dangerous because if there were other services using PDPVerifier, we would be upending their security model around dataset deletion.

This path was changed late because the previous design had problematic several edge cases that I don't remember. We wanted both parties to be able to unilaterally terminate the deal. (The client path for deleting dataset is terminating the rail.)

@Kubuxu led that project and might remember more context.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Session key actions ultimately use storage provider gas.

Maybe we could provide a curio path for a session key to suggest that the storage provider kindly terminate, and if they did that then that would not require a contract change.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By design, only SP can initiate the dataset deletion, and it happens with the client's approval. Whether that happens through a session key or not is up to FWSS.
So it depends on whether dataset deletion is part of some user story for session keys.
I don't think anyone suggests unilateral deletion of datasets with session keys.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Started a Slack discussion for resolving this in https://filecoinproject.slack.com/archives/C08TVNKJV7C/p1775033334018889

@BigLep BigLep moved this from 📌 Triage to 🔎 Awaiting review in FOC Mar 25, 2026
@rjan90 rjan90 added this to the M4.2: mainnet GA milestone Mar 30, 2026
@rvagg
Copy link
Copy Markdown
Collaborator

rvagg commented Mar 30, 2026

As per #693 (comment), we didn't remove this when we noticed it was out of date because it's on the table for going back in to FWSS for filecoin-project/curio#1028 which is currently still marked as GA.

The gap is that termination currently requires an on-chain message from the client, which is fine as a last-resort, but the ideal flow is like for the other interactions where we ask the server to do it, then if they refuse, we can go to the chain and terminate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🔎 Awaiting review

Development

Successfully merging this pull request may close these issues.

DeleteDataSet permission has wrong constant

6 participants