Skip to content

Conversation

@wjmelements
Copy link
Contributor

Reviewer @rvagg
This reduces storage accumulation.
However the piece metadata will still not be removed when the whole data set is deleted.

Changes

  • rm piece metadata in piecesScheduledRemove

@wjmelements wjmelements requested a review from rvagg November 5, 2025 17:40
@FilOzzy FilOzzy added this to FS Nov 5, 2025
@github-project-automation github-project-automation bot moved this to 📌 Triage in FS Nov 5, 2025
@wjmelements wjmelements requested a review from Kubuxu November 5, 2025 17:55
@rvagg
Copy link
Collaborator

rvagg commented Nov 5, 2025

There's a missing window here though, piece won't actually be removed from data set until the nextProvingPeriod is called on that data set. So now we have a piece with no metadata - user, or others, will list pieces in a data set, see them, but with nothing about them.

Not a great state of affairs. Would it be practical to extend PDPListener#nextProvingPeriod to pass through uint256[] removals? I wonder if that could get too big to pass as an argument there...

@rjan90 rjan90 moved this from 📌 Triage to ⌨️ In Progress in FS Nov 11, 2025
@rvagg
Copy link
Collaborator

rvagg commented Nov 25, 2025

We're going to need some state for this I think, I don't see a good way around this that doesn't involve iterating over pieces during nextProvingPeriod. It'll probably have to mirror PDPVerifier's:

    // Enqueued piece ids for removal when starting the next proving period
    mapping(uint256 => uint256[]) scheduledRemovals;

And we update it when we get a call to piecesScheduledRemove. Then during nextProvingPeriod we can perform the metadata clean-up and purge that list.

@rvagg rvagg added this to the M4: Filecoin Service Liftoff milestone Dec 5, 2025
@rvagg
Copy link
Collaborator

rvagg commented Dec 5, 2025

Moving this to M4, we should try and get this landed, but it needs to be done in a queue to be processed at nextProvingPeriod and since we have more state, we need to be careful wrt upgrading existing contracts.

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

Labels

None yet

Projects

Status: ⌨️ In Progress

Development

Successfully merging this pull request may close these issues.

3 participants