fix: allow abandonment and consensual termination of underfunded data sets#520
fix: allow abandonment and consensual termination of underfunded data sets#520wjmelements wants to merge 4 commits into
Conversation
Assisted-by: Claude:claude-sonnet-4-6
Assisted-by: Claude:claude-sonnet-4-6
…geRates Assisted-by: Claude:claude-sonnet-4-6
…ndonRails and updateStorageRates Assisted-by: Claude:claude-sonnet-4-6
| // Try to zero the lockup period (requires a fully-settled account). | ||
| // For underfunded payers the period change is blocked. | ||
| try payments.modifyRailLockup(pdpRailId, 0, pending) {} | ||
| catch { | ||
| // Always release the fixed lockup | ||
| payments.modifyRailLockup(pdpRailId, DEFAULT_LOCKUP_PERIOD, pending); | ||
| } |
There was a problem hiding this comment.
I don't think we should do this here, immedaiteTermination path implies that everyone is behaving properly and agreeing together, allowing an underfunded client here to proceed with immediate termination gives a bad actor privileges that I don't think they should get.
In the normal case I don't think we'll get here, as I said in #519 I think the only way we can get into the situation where this reverts is where the SP abandons the data set and doesn't discover underfunding with regular settles. The immediateTermination path suggests the SP is still engaged and willing to submit a msg to the chain for this data set.
I'd just leave a comment in here about why we're not doing a try/catch.
| // Try to zero the lockup period (requires a fully-settled account). | |
| // For underfunded payers the period change is blocked. | |
| try payments.modifyRailLockup(pdpRailId, 0, pending) {} | |
| catch { | |
| // Always release the fixed lockup | |
| payments.modifyRailLockup(pdpRailId, DEFAULT_LOCKUP_PERIOD, pending); | |
| } | |
| // No try/catch here to cover the underfunded-client case. immediateTermination implies | |
| // the payer is solvent enough to consent to releasing the lifecycle reserve and paying the | |
| // terminate fee from `pending` in one step. | |
| payments.modifyRailLockup(pdpRailId, 0, pending); |
There was a problem hiding this comment.
This makes sense to me. I'll think about it a bit more.
Reviewer @rvagg
Fixes #519
Consensual termination and abandonment both attempt to set the endEpoch to block.number in order to allow immediate cleanup.
Because we cannot modify the payment rate or the lockup for underfunded data sets before termination (see FilOzone/filecoin-pay#290), we are unable to reduce the lockup period to ensure endEpoch is the current block.
Instead of reverting, we will release the fixed lockup after termination.
Changes