diff --git a/apps/backend/src/sp-cleanup/sp-cleanup.service.spec.ts b/apps/backend/src/sp-cleanup/sp-cleanup.service.spec.ts index a2097bb3..a3e9a8b0 100644 --- a/apps/backend/src/sp-cleanup/sp-cleanup.service.spec.ts +++ b/apps/backend/src/sp-cleanup/sp-cleanup.service.spec.ts @@ -1219,20 +1219,54 @@ describe("SpCleanupService", () => { expect(warnSpy).not.toHaveBeenCalledWith(expect.objectContaining({ event: "stuck_terminations_detected" })); }); - it("treats a reverting getRail call as already-finalized and skips it silently (no persistence needed)", async () => { + it("deletes a terminated data set once its rail is finalized and it is outside the activity window", async () => { const dataSet = makeDataSet({ dataSetId: 22n, pdpEndEpoch: 500n, pdpRailId: 997n }); mockClientDataSets([dataSet] as any); vi.mocked(getBlockNumber).mockResolvedValueOnce(200000n); // A finalized rail reverts, which Multicall3 reports as a per-item failure. mockBatchedReads({ getRail: new ContractFunctionRevertedError({ abi: [], functionName: "getRail" }), + getDataSetLastProvenEpoch: 498n, + }); + const notInCleanupMode = new ContractFunctionRevertedError({ abi: [], functionName: "cleanupPieces" }); + notInCleanupMode.data = { errorName: "DataSetNotInCleanupMode", args: [] } as any; + vi.mocked(simulateContract) + .mockResolvedValueOnce({ request: { fake: "deleteDataSet-request" } } as any) + .mockRejectedValueOnce(notInCleanupMode); + vi.mocked(writeContract).mockResolvedValueOnce("0xtxhash" as any); + vi.mocked(waitForTransactionReceipt).mockResolvedValueOnce({ status: "success" } as any); + + await service.runAbandonedDataSetSweep(DEFAULT_NETWORK); + + expect(simulateContract).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ functionName: "deleteDataSet", args: [22n, "0x"] }), + ); + expect(settleRailCall).not.toHaveBeenCalled(); + expect(attemptsCounter.inc).toHaveBeenCalledWith({ + network: DEFAULT_NETWORK, + outcome: "success", + reason: "finalized", + }); + expect(stuckGauge.set).toHaveBeenCalledWith({ network: DEFAULT_NETWORK }, 0); + }); + + it("leaves a finalized rail's data set alone while it is still inside the activity window", async () => { + const dataSet = makeDataSet({ dataSetId: 25n, pdpEndEpoch: 150000n, pdpRailId: 993n }); + mockClientDataSets([dataSet] as any); + vi.mocked(getBlockNumber).mockResolvedValueOnce(200000n); + mockBatchedReads({ + getRail: new ContractFunctionRevertedError({ abi: [], functionName: "getRail" }), + // Proven until its end epoch, so deleteDataSet is still SP-only for ~30 more days. + getDataSetLastProvenEpoch: 149998n, }); await expect(service.runAbandonedDataSetSweep(DEFAULT_NETWORK)).resolves.toBeUndefined(); + expect(simulateContract).not.toHaveBeenCalled(); + expect(writeContract).not.toHaveBeenCalled(); expect(stuckGauge.set).toHaveBeenCalledWith({ network: DEFAULT_NETWORK }, 0); expect(warnSpy).not.toHaveBeenCalledWith(expect.objectContaining({ event: "stuck_terminations_detected" })); - expect(warnSpy).not.toHaveBeenCalledWith(expect.objectContaining({ event: "sp_cleanup_get_rail_read_failed" })); }); it("does NOT treat a transient rail-read failure as finalized — the whole batch fails instead", async () => { @@ -1260,7 +1294,8 @@ describe("SpCleanupService", () => { const outOfGas = new Error("message execution failed (exit=[SysErrOutOfGas(7)], vm error=...)"); vi.mocked(multicall) .mockResolvedValueOnce(dataSets.map(() => ({ status: "failure", error: outOfGas })) as never) - .mockResolvedValue([{ status: "success", result: { settledUpTo: 100n, endEpoch: 500n } }] as never); + .mockImplementation((async (_client: unknown, opts: any) => + opts.contracts.map(() => ({ status: "success", result: { settledUpTo: 100n, endEpoch: 500n } }))) as never); vi.mocked(simulateContract).mockResolvedValue({ request: { fake: "settleRail-request" } } as any); vi.mocked(writeContract).mockResolvedValue("0xsettle" as any); vi.mocked(waitForTransactionReceipt).mockResolvedValue({ status: "success" } as any); diff --git a/apps/backend/src/sp-cleanup/sp-cleanup.service.ts b/apps/backend/src/sp-cleanup/sp-cleanup.service.ts index 6dae8211..9b4dc68e 100644 --- a/apps/backend/src/sp-cleanup/sp-cleanup.service.ts +++ b/apps/backend/src/sp-cleanup/sp-cleanup.service.ts @@ -50,9 +50,12 @@ import { WalletSdkService } from "../wallet-sdk/wallet-sdk.service.js"; */ const PDP_INACTIVITY_WINDOW_BLOCKS = 86400n; -type TerminationReason = "blocked" | "trickle" | "full_rate" | "abandonment" | "settlement"; +type TerminationReason = "blocked" | "trickle" | "full_rate" | "abandonment" | "finalized" | "settlement"; type TerminationOutcome = "success" | "failure"; +/** Why a data set may be deleted: never terminated, or terminated with its rail finalized. */ +type DeletionReason = "abandonment" | "finalized"; + type ReadOnlyClient = Client; type SubmitWithNonce = (submit: (nonce: number) => Promise<`0x${string}`>) => Promise<`0x${string}`>; @@ -600,10 +603,12 @@ export class SpCleanupService { * Stateless, network-wide (not per-SP, not conditioned on blocklist status). * Scans every data set dealbot's wallet holds: * - * Branch 1 (abandonment): `pdpEndEpoch === 0n` and outside PDPVerifier's - * activity window -> `deleteDataSet` is fully permissionless past that - * window, so it's called directly with the session key's own wallet - * (no signature/relay, but real gas from the session key's own balance). + * Branch 1 (deletion): outside PDPVerifier's activity window and either never + * terminated (`pdpEndEpoch === 0n`, reason `abandonment`) or terminated with its + * rail finalized (reason `finalized`; FWSS rejects deleting a set whose rail is not + * fully settled) -> `deleteDataSet` is fully permissionless past that window, so + * it's called directly with the session key's own wallet (no signature/relay, but + * real gas from the session key's own balance). * * Branch 2 (stuck settlement): `pdpEndEpoch > 0n` and the lockup has fully * elapsed (`currentBlock > pdpEndEpoch`, strict) but the rail is still @@ -633,16 +638,32 @@ export class SpCleanupService { const abi = pdpVerifier.abi as Parameters[1]["abi"]; // Batch view calls so large wallets do not spend the whole job window reading serially. - const abandonmentCandidates = allDataSets.filter((dataSet) => dataSet.pdpEndEpoch === 0n); const settlementCandidates = allDataSets.filter( (dataSet) => dataSet.pdpEndEpoch > 0n && currentBlock > dataSet.pdpEndEpoch, ); + const getRailCall = (dataSet: DataSetInfo) => ({ + address: chain.contracts.filecoinPay.address, + abi: chain.contracts.filecoinPay.abi as Parameters[1]["abi"], + functionName: "getRail", + args: [dataSet.pdpRailId], + }); + + // A reverting getRail means the rail is finalized, i.e. fully settled. + const railsBeforeDeletion = await this.readInBatches(readClient, settlementCandidates, getRailCall, signal); + const deletionCandidates: { dataSet: DataSetInfo; reason: DeletionReason }[] = [ + ...allDataSets + .filter((dataSet) => dataSet.pdpEndEpoch === 0n) + .map((dataSet) => ({ dataSet, reason: "abandonment" as const })), + ...railsBeforeDeletion + .filter(({ reverted }) => reverted) + .map(({ item: dataSet }) => ({ dataSet, reason: "finalized" as const })), + ]; // Branch 1: only data sets outside PDPVerifier's activity window can be deleted. const lastProvenEpochs = await this.readInBatches( readClient, - abandonmentCandidates, - (dataSet) => ({ + deletionCandidates, + ({ dataSet }) => ({ address: pdpVerifier.address, abi, functionName: "getDataSetLastProvenEpoch", @@ -651,15 +672,16 @@ export class SpCleanupService { signal, ); - const abandoned: { dataSet: DataSetInfo; lastProvenEpoch: bigint }[] = []; - for (const { item: dataSet, value, reverted } of lastProvenEpochs) { + const deletable: { dataSet: DataSetInfo; reason: DeletionReason; lastProvenEpoch: bigint }[] = []; + for (const { item, value, reverted } of lastProvenEpochs) { + const { dataSet, reason } = item; if (reverted) { // A single data set's read must never abort the sweep — every data set after it // (including branch 2) would silently go unchecked for this run. - this.recordAttempt(network, "abandonment", "failure"); + this.recordAttempt(network, reason, "failure"); this.logger.warn({ network, - reason: "abandonment", + reason, providerAddress: dataSet.serviceProvider, dataSetId: dataSet.dataSetId.toString(), event: "sp_cleanup_last_proven_epoch_read_failed", @@ -669,16 +691,17 @@ export class SpCleanupService { } const lastProvenEpoch = value as bigint; if (currentBlock <= lastProvenEpoch + PDP_INACTIVITY_WINDOW_BLOCKS) continue; - abandoned.push({ dataSet, lastProvenEpoch }); + deletable.push({ dataSet, reason, lastProvenEpoch }); } - await mapWithConcurrency(abandoned, WRITE_CONCURRENCY, async ({ dataSet, lastProvenEpoch }) => { + await mapWithConcurrency(deletable, WRITE_CONCURRENCY, async ({ dataSet, reason, lastProvenEpoch }) => { signal?.throwIfAborted(); - await this.handleAbandonmentCandidate( + await this.handleDeletionCandidate( writeClient, pdpVerifier, network, dataSet, + reason, lastProvenEpoch, currentBlock, submitWithNonce, @@ -686,18 +709,9 @@ export class SpCleanupService { ); }); - // Branch 2: a reverting getRail means the rail is already finalized — nothing to do. - const rails = await this.readInBatches( - readClient, - settlementCandidates, - (dataSet) => ({ - address: chain.contracts.filecoinPay.address, - abi: chain.contracts.filecoinPay.abi as Parameters[1]["abi"], - functionName: "getRail", - args: [dataSet.pdpRailId], - }), - signal, - ); + // Branch 2: re-read rails, since deletion can take most of the run. A rail the SP settled + // meanwhile would make settleRail revert and be wrongly flagged as stuck. + const rails = await this.readInBatches(readClient, settlementCandidates, getRailCall, signal); const settleable = rails.filter(({ reverted }) => !reverted).map(({ item }) => item); await mapWithConcurrency(settleable, WRITE_CONCURRENCY, async (dataSet) => { @@ -716,11 +730,12 @@ export class SpCleanupService { } /** The caller has already established that this data set is outside the activity window. */ - private async handleAbandonmentCandidate( + private async handleDeletionCandidate( writeClient: SynapseViemClient, pdpVerifier: { address: `0x${string}`; abi: unknown }, network: Network, dataSet: { dataSetId: bigint; serviceProvider: string }, + reason: DeletionReason, lastProvenEpoch: bigint, currentBlock: bigint, submitWithNonce: SubmitWithNonce, @@ -729,7 +744,7 @@ export class SpCleanupService { const abi = pdpVerifier.abi as Parameters[1]["abi"]; const logContext = { network, - reason: "abandonment" as const, + reason, providerAddress: dataSet.serviceProvider, dataSetId: dataSet.dataSetId.toString(), lastProvenEpoch: lastProvenEpoch.toString(), @@ -752,17 +767,17 @@ export class SpCleanupService { if (receipt.status !== "success") { throw new Error(`deleteDataSet transaction reverted on-chain (hash: ${hash})`); } - this.recordAttempt(network, "abandonment", "success"); + this.recordAttempt(network, reason, "success"); this.logger.log({ ...logContext, event: "sp_cleanup_data_set_abandoned_deleted", - message: "Abandoned data set deleted directly via PDPVerifier.deleteDataSet (no signature required)", + message: "Data set deleted directly via PDPVerifier.deleteDataSet (no signature required)", txHash: hash, }); } catch (error) { // Only a pre-submission abort is a job timeout. if (this.isAbortError(error, signal)) throw error; - this.recordAttempt(network, "abandonment", "failure"); + this.recordAttempt(network, reason, "failure"); this.logger.warn({ ...logContext, event: "sp_cleanup_data_set_abandoned_delete_failed", diff --git a/docs/runbooks/wallet-and-session-keys.md b/docs/runbooks/wallet-and-session-keys.md index 13de9a5f..efc8c29c 100644 --- a/docs/runbooks/wallet-and-session-keys.md +++ b/docs/runbooks/wallet-and-session-keys.md @@ -241,6 +241,11 @@ dealbot's entire wallet, not scoped to the blocklist. For each data set: - **Abandoned** (never terminated, and outside PDPVerifier's ~30-day activity window since the last proof): `PDPVerifier.deleteDataSet` becomes fully permissionless once that window has elapsed, so dealbot's session key calls it **directly** — no SP signature or relay required. This is what actually cleans up dead SPs. + Metrics and logs use `reason="abandonment"`. +- **Finalized** (terminated, rail finalized, and outside the same ~30-day activity window): deleted the same + way, with `reason="finalized"`. FWSS rejects deleting a terminated set until its rail is fully settled + (`RailNotFullySettled`), and SPs usually prove until `pdpEndEpoch`, so these become deletable about 30 days + after `pdpEndEpoch`. A rail the sweep settles below becomes deletable on a later run. - **Stuck settlement** (terminated, but the termination lockup has fully elapsed and the SP never called `settleRail()` themselves): unlike `settleTerminatedRailWithoutValidation` (`onlyRailClient` — the Safe's address only, not the session key's own; same constraint as the direct 1-arg `terminateService`, see