From 632e53154286eddd5ab802d6b77f6f5c9a23eb10 Mon Sep 17 00:00:00 2001 From: Rod Vagg Date: Sat, 6 Jun 2026 00:10:03 +1000 Subject: [PATCH 1/2] fix(cleanup): anchor cleanupPieces gate to last proving activity The permissionless gate was anchored to cleanup-mode entry, so the abandonment path took two INACTIVITY_WINDOWs (~60 days): a third party could delete an abandoned data set after 30 days but had to wait another 30 to clean up and collect the deposit. Both gates now share _withinActivityWindow, making delete and cleanup permissionless at the same moment. Deprecates the now-unused cleanupModeEpoch in place. --- CHANGELOG.md | 3 + src/PDPVerifier.sol | 42 +++++++------ src/PDPVerifierLayout.json | 2 +- src/PDPVerifierLayout.sol | 2 +- test/CleanupPieces.t.sol | 120 ++++++++++++++++++++++++++++++++++++- 5 files changed, 146 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 950dd2f..a391759 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ## [Unreleased] +### Fixed +- `cleanupPieces()` now uses the same permission gate as `deleteDataSet()`, anchored to last proving activity instead of cleanup-mode entry. The abandonment path previously required two full `INACTIVITY_WINDOW` periods (~60 days); a permissionless deleter can now clean up and collect the deposit immediately. An SP deleting after exceeding the inactivity window no longer gets an exclusive cleanup period (the data set was already permissionlessly deletable). The unused `cleanupModeEpoch` storage is deprecated in place. + ## [3.4.0] - 2026-05-28 This release upgrades the deployed PDPVerifier contract with data-set cleanup deposits and explicit piece-cleanup finalization. New data sets now hold a 0.1 FIL cleanup deposit that is returned to whoever completes cleanup after deletion, giving storage providers and permissionless cleanup callers a concrete incentive to clear on-chain piece state. diff --git a/src/PDPVerifier.sol b/src/PDPVerifier.sol index 985b6bb..e5f97b4 100644 --- a/src/PDPVerifier.sol +++ b/src/PDPVerifier.sol @@ -176,8 +176,8 @@ contract PDPVerifier is Initializable, UUPSUpgradeable, OwnableUpgradeable { // FIL deposit collected at createDataSet and returned to whoever finalizes cleanup for that data set. mapping(uint256 => uint256) cleanupDeposit; - // Block number when deleteDataSet was called, used to gate permissionless cleanupPieces calls. - mapping(uint256 => uint256) cleanupModeEpoch; + // Former cleanupPieces gate anchor; only written by v3.4.0 deleteDataSet. + mapping(uint256 => uint256) deprecatedCleanupModeEpoch; // Methods @@ -584,6 +584,17 @@ contract PDPVerifier is Initializable, UUPSUpgradeable, OwnableUpgradeable { return setId; } + // True within INACTIVITY_WINDOW blocks of the last proving activity. Past that the data set + // is abandoned and deleteDataSet/cleanupPieces become permissionless. + // Legacy data sets (lastProvenEpoch == 0) use the implementation deployment block as baseline. + function _withinActivityWindow(uint256 setId) internal view returns (bool) { + uint256 lastActivity = dataSetLastProvenEpoch[setId]; + if (lastActivity == NO_PROVEN_EPOCH) { + lastActivity = LEGACY_ACTIVITY_EPOCH; + } + return block.number <= lastActivity + INACTIVITY_WINDOW; + } + // Removes a data set. Normally called by the storage provider; permissionless after INACTIVITY_WINDOW // blocks of SP inactivity (measured from dataSetLastProvenEpoch). // @@ -595,13 +606,7 @@ contract PDPVerifier is Initializable, UUPSUpgradeable, OwnableUpgradeable { require(sp != address(0), DataSetNotLive()); require(nextChallengeEpoch[setId] != CLEANUP_MODE_SENTINEL, DataSetAlreadyInCleanup()); - // Permissionless if the SP has been inactive for more than INACTIVITY_WINDOW blocks. - // Legacy datasets (lastProvenEpoch == 0) use the implementation deployment block as baseline. - uint256 lastActivity = dataSetLastProvenEpoch[setId]; - if (lastActivity == NO_PROVEN_EPOCH) { - lastActivity = LEGACY_ACTIVITY_EPOCH; - } - if (block.number <= lastActivity + INACTIVITY_WINDOW) { + if (_withinActivityWindow(setId)) { require(msg.sender == sp, OnlyStorageProviderCanDelete()); } @@ -619,9 +624,9 @@ contract PDPVerifier is Initializable, UUPSUpgradeable, OwnableUpgradeable { _finalizeCleanup(setId); } else { // Pieces remain: enter cleanup mode. storageProvider and dataSetLastProvenEpoch are kept - // so cleanupPieces can apply the same permission gate. + // so cleanupPieces can apply the same permission gate; the latter cannot advance here + // since provePossession, nextProvingPeriod and addPieces all revert on a deleted set. nextChallengeEpoch[setId] = CLEANUP_MODE_SENTINEL; - cleanupModeEpoch[setId] = block.number; } } @@ -629,8 +634,9 @@ contract PDPVerifier is Initializable, UUPSUpgradeable, OwnableUpgradeable { // has placed the data set in cleanup mode (nextChallengeEpoch == CLEANUP_MODE_SENTINEL), or for // legacy data sets deleted before this feature was added (storageProvider == 0 && nextPieceId > 0). // - // The caller gate mirrors deleteDataSet: SP-exclusive within INACTIVITY_WINDOW blocks of deleteDataSet, - // permissionless after. Legacy data sets are always permissionless. + // The caller gate is identical to deleteDataSet: SP-exclusive within INACTIVITY_WINDOW blocks of + // the last proving activity, permissionless after, so an abandoned data set can be deleted and + // cleaned up back-to-back by one caller. Legacy data sets are always permissionless. // // On the final call that clears all pieces, all remaining data set state is also cleared and // the cleanup deposit is transferred to msg.sender. Returns true when cleanup is complete. @@ -643,11 +649,8 @@ contract PDPVerifier is Initializable, UUPSUpgradeable, OwnableUpgradeable { require(isCleanupMode || isLegacyDataset, DataSetNotInCleanupMode()); - if (isCleanupMode) { - // Same inactivity gate as deleteDataSet, anchored to when cleanup mode was entered. - if (block.number <= cleanupModeEpoch[setId] + INACTIVITY_WINDOW) { - require(msg.sender == storageProvider[setId], OnlyStorageProviderCanCleanupPieces()); - } + if (isCleanupMode && _withinActivityWindow(setId)) { + require(msg.sender == storageProvider[setId], OnlyStorageProviderCanCleanupPieces()); } uint256 pieceCount = nextPieceId[setId]; @@ -684,7 +687,8 @@ contract PDPVerifier is Initializable, UUPSUpgradeable, OwnableUpgradeable { delete storageProvider[setId]; delete dataSetLastProvenEpoch[setId]; delete nextChallengeEpoch[setId]; - delete cleanupModeEpoch[setId]; + // Clears residue from v3.4.0-era deletes. + delete deprecatedCleanupModeEpoch[setId]; uint256 deposit = cleanupDeposit[setId]; delete cleanupDeposit[setId]; diff --git a/src/PDPVerifierLayout.json b/src/PDPVerifierLayout.json index 2440ab1..4764e9c 100644 --- a/src/PDPVerifierLayout.json +++ b/src/PDPVerifierLayout.json @@ -431,7 +431,7 @@ } }, { - "label": "cleanupModeEpoch", + "label": "deprecatedCleanupModeEpoch", "offset": 0, "slot": "18", "type": { diff --git a/src/PDPVerifierLayout.sol b/src/PDPVerifierLayout.sol index 81a1739..5abb167 100644 --- a/src/PDPVerifierLayout.sol +++ b/src/PDPVerifierLayout.sol @@ -23,4 +23,4 @@ bytes32 constant DATA_SET_LAST_PROVEN_EPOCH_SLOT = bytes32(uint256(14)); bytes32 constant FEE_STATUS_SLOT = bytes32(uint256(15)); bytes32 constant NEXT_UPGRADE_SLOT = bytes32(uint256(16)); bytes32 constant CLEANUP_DEPOSIT_SLOT = bytes32(uint256(17)); -bytes32 constant CLEANUP_MODE_EPOCH_SLOT = bytes32(uint256(18)); +bytes32 constant DEPRECATED_CLEANUP_MODE_EPOCH_SLOT = bytes32(uint256(18)); diff --git a/test/CleanupPieces.t.sol b/test/CleanupPieces.t.sol index 039e955..83915e5 100644 --- a/test/CleanupPieces.t.sol +++ b/test/CleanupPieces.t.sol @@ -9,7 +9,14 @@ import {PDPFees} from "../src/Fees.sol"; import {PDPRecordKeeper} from "../src/SimplePDPService.sol"; import {PieceHelper} from "./PieceHelper.t.sol"; import {NEW_DATA_SET_SENTINEL} from "../src/PDPVerifier.sol"; -import {PIECE_CIDS_SLOT, PIECE_LEAF_COUNTS_SLOT, SUM_TREE_COUNTS_SLOT} from "../src/PDPVerifierLayout.sol"; +import { + DATA_SET_LAST_PROVEN_EPOCH_SLOT, + DEPRECATED_CLEANUP_MODE_EPOCH_SLOT, + PIECE_CIDS_SLOT, + PIECE_LEAF_COUNTS_SLOT, + STORAGE_PROVIDER_SLOT, + SUM_TREE_COUNTS_SLOT +} from "../src/PDPVerifierLayout.sol"; contract TestListener is PDPListener, PDPRecordKeeper { function storageProviderChanged(uint256, address, address, bytes calldata) external override {} @@ -188,7 +195,7 @@ contract PDPVerifierCleanupTest is MockFVMTest, PieceHelper { uint256 setId = _createAndPopulate(1); pdpVerifier.deleteDataSet(setId, empty); - // block.number (1) <= cleanupModeEpoch + INACTIVITY_WINDOW, so only SP can call + // block.number (1) <= lastProvenEpoch + INACTIVITY_WINDOW, so only SP can call address notSp = address(0xBEEF); vm.prank(notSp); vm.expectRevert(PDPVerifier.OnlyStorageProviderCanCleanupPieces.selector); @@ -229,6 +236,115 @@ contract PDPVerifierCleanupTest is MockFVMTest, PieceHelper { assertFalse(pdpVerifier.dataSetLive(setId)); } + function testAbandonedDataSetDeleteAndCleanupInOneGo() public { + uint256 setId = _createAndPopulate(2); + + vm.roll(block.number + pdpVerifier.INACTIVITY_WINDOW() + 1); + + // One third party deletes the abandoned set and cleans up back-to-back, + // collecting the deposit as the cleanup bounty. + address anyone = address(0xCAFE); + vm.deal(anyone, 10 ether); + uint256 balanceBefore = anyone.balance; + + vm.startPrank(anyone); + pdpVerifier.deleteDataSet(setId, empty); + bool done = pdpVerifier.cleanupPieces(setId, 10); + vm.stopPrank(); + + assertTrue(done); + assertEq(anyone.balance - balanceBefore, PDPFees.cleanupDeposit(), "deleter collects cleanup deposit"); + assertPieceSlotsCleared(setId, 2); + } + + function testCleanupGateAnchorsToActivityNotDeleteEpoch() public { + uint256 setId = _createAndPopulate(1); + uint256 lastProven = pdpVerifier.getDataSetLastProvenEpoch(setId); + uint256 window = pdpVerifier.INACTIVITY_WINDOW(); + + // SP deletes late in its activity window + vm.roll(lastProven + window - 100); + pdpVerifier.deleteDataSet(setId, empty); + + // Still within the activity window: third party blocked + address anyone = address(0xBEEF); + vm.prank(anyone); + vm.expectRevert(PDPVerifier.OnlyStorageProviderCanCleanupPieces.selector); + pdpVerifier.cleanupPieces(setId, 10); + + // Just past the activity window, well before deleteEpoch + window: the gate + // anchors to proving activity, not cleanup-mode entry. + vm.roll(lastProven + window + 1); + vm.deal(anyone, 10 ether); + vm.prank(anyone); + bool done = pdpVerifier.cleanupPieces(setId, 10); + assertTrue(done); + assertPieceSlotsCleared(setId, 1); + } + + function testSpDeleteAfterAbandonmentCleanupImmediatelyPermissionless() public { + uint256 setId = _createAndPopulate(1); + + vm.roll(block.number + pdpVerifier.INACTIVITY_WINDOW() + 1); + + // SP can always delete, but past the activity window cleanup is open to everyone + pdpVerifier.deleteDataSet(setId, empty); + + address anyone = address(0xCAFE); + vm.deal(anyone, 10 ether); + vm.prank(anyone); + assertTrue(pdpVerifier.cleanupPieces(setId, 10)); + assertPieceSlotsCleared(setId, 1); + } + + function testCleanupLegacyActivityBaseline() public { + uint256 setId = _createAndPopulate(1); + + // Simulate a data set created before activity tracking: lastProvenEpoch == 0 + vm.store(address(pdpVerifier), keccak256(abi.encode(setId, DATA_SET_LAST_PROVEN_EPOCH_SLOT)), bytes32(0)); + assertEq(pdpVerifier.getDataSetLastProvenEpoch(setId), 0); + + pdpVerifier.deleteDataSet(setId, empty); + + // Gate falls back to LEGACY_ACTIVITY_EPOCH (implementation deployment block) + address notSp = address(0xBEEF); + vm.prank(notSp); + vm.expectRevert(PDPVerifier.OnlyStorageProviderCanCleanupPieces.selector); + pdpVerifier.cleanupPieces(setId, 10); + + vm.roll(pdpVerifier.LEGACY_ACTIVITY_EPOCH() + pdpVerifier.INACTIVITY_WINDOW() + 1); + vm.deal(notSp, 10 ether); + vm.prank(notSp); + assertTrue(pdpVerifier.cleanupPieces(setId, 10)); + assertPieceSlotsCleared(setId, 1); + } + + function testLegacyDeletedDataSetCleanupAlwaysPermissionless() public { + uint256 setId = _createAndPopulate(2); + + // Simulate a pre-3.4.0 delete: storageProvider zeroed, pieces left behind + vm.store(address(pdpVerifier), keccak256(abi.encode(setId, STORAGE_PROVIDER_SLOT)), bytes32(0)); + + // No activity-window gate applies; anyone can clean immediately + address anyone = address(0xCAFE); + vm.deal(anyone, 10 ether); + vm.prank(anyone); + assertTrue(pdpVerifier.cleanupPieces(setId, 10)); + assertPieceSlotsCleared(setId, 2); + } + + function testFinalizeClearsDeprecatedCleanupModeEpochResidue() public { + uint256 setId = _createAndPopulate(1); + pdpVerifier.deleteDataSet(setId, empty); + + // Simulate a v3.4.0-era delete that wrote the now-deprecated gate anchor + bytes32 slot = keccak256(abi.encode(setId, DEPRECATED_CLEANUP_MODE_EPOCH_SLOT)); + vm.store(address(pdpVerifier), slot, bytes32(uint256(123))); + + assertTrue(pdpVerifier.cleanupPieces(setId, 10)); + assertEq(vm.load(address(pdpVerifier), slot), bytes32(0), "deprecated slot cleared"); + } + function testOnlySpCanDeleteWithinInactivityWindow() public { uint256 setId = _createAndPopulate(1); From f5c4dc35b0aa6664d312d576eb44134663079bc9 Mon Sep 17 00:00:00 2001 From: William Morriss Date: Fri, 5 Jun 2026 15:31:31 -0500 Subject: [PATCH 2/2] test(cleanup): verify SP can delete and claim cleanup deposit in same block Assisted-by: Claude:claude-sonnet-4-6 --- test/CleanupPieces.t.sol | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/CleanupPieces.t.sol b/test/CleanupPieces.t.sol index 83915e5..5f03d0b 100644 --- a/test/CleanupPieces.t.sol +++ b/test/CleanupPieces.t.sol @@ -236,6 +236,18 @@ contract PDPVerifierCleanupTest is MockFVMTest, PieceHelper { assertFalse(pdpVerifier.dataSetLive(setId)); } + function testSpCanDeleteAndCleanupInSameBlock() public { + uint256 setId = _createAndPopulate(2); + + uint256 balanceBefore = address(this).balance; + pdpVerifier.deleteDataSet(setId, empty); + bool done = pdpVerifier.cleanupPieces(setId, 10); + + assertTrue(done); + assertEq(address(this).balance - balanceBefore, PDPFees.cleanupDeposit(), "SP receives deposit in same block"); + assertPieceSlotsCleared(setId, 2); + } + function testAbandonedDataSetDeleteAndCleanupInOneGo() public { uint256 setId = _createAndPopulate(2);