-
Notifications
You must be signed in to change notification settings - Fork 174
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FIP-0045 updates removing opt-in migration #455
Conversation
- Specify `HasVerifiedClaims` field in `SectorOnChainInfo` - Remove opt-in migration for existing sectors - Add maximum expiration parameter for allocations - Describe migration for pending deals
I have pushed some updates:
|
- Figure out best migration plan for pending deals, pre-committed sectors | ||
- Confirm policy for built-in market actor's default term maximum | ||
- Spec token receiver hook payload schema for allocation requests | ||
- Spec token receiver hook payload for extending allocations with new datacap | ||
- Market method for fetching allocation/claim ids for deals (needed?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the potential use case here?
An allocation is created by a verified client by transferring DataCap tokens (see below) | ||
to the verified registry. | ||
The `Allocation` metadata must be provided as transfer metadata. | ||
The verified registry checks the same preconditions as the existing `UseBytes` method. | ||
The size of the allocation must match the number of tokens received. | ||
|
||
#### Removal of expired allocations | ||
An allocation may be removed after its expiration epoch has passed (by anyone). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we allowing expired allocations to be removed "by anyone"? What is the expected behavior here? Should clients remove them or are we supposed to have an automated process doing it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no reason to restrict it to the client address specifically, so this gives flexibility to users. E.g. if a client is a multisig or DAO or other complex entity, they don't need to go to the cost and trouble of multisigning this inconsequential message – one of the parties/delegates/employees can just do it.
There's no "should" or automation. Clients can clean up when the gas savings associated with a smaller collection justify it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. This makes sense.
@@ -245,7 +246,8 @@ struct SectorClaimResult { | |||
fn ClaimAllocations(params: ClaimAllocationsParams) -> ClaimAllocationsResult | |||
``` | |||
|
|||
The record of a claim may be removed by the provider after the term maximum has elapsed. | |||
#### Removal of expired claims | |||
The record of a claim may be removed after the term maximum has elapsed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Who can remove expired claims? Is this an automated process?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Iiuc, Anyone can remove the expired claims and it’s triggered by an onchain message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to above, anyone can. In practise this will probably be some ancillary or operations account associated with a provider. There's no automation.
The claim's term maximum can be extended up to `MaximumVerifiedAllocationTerm` beyond the current epoch. | ||
The client extending the claim need not be the one that made the original allocation. | ||
This is similar to issuing a new allocation/claim for the same data, | ||
but avoids overheads of re-sealing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This mechanism makes a lot of sense. There is no reason to re-seal the same data just because one needs to spend new DataCap.
- The maximum allowed value for `TermMaximum` is 5 years (can be increased by a future FIP). | ||
- `MinimumVerifiedAllocationTerm`: The minimum allowed value for `TermMinimum` is 6 months. | ||
- `MaximumVerifiedAllocationTerm`: The maximum allowed value for `TermMaximum` is 5 years (can be increased by a future FIP). | ||
- `MaximumVerifiedAllocationExpiration`: The maximum difference between `Expiration` and the epoch at which an allocation is created is 60 days. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 60 days? Is this the current expiration term for DataCap?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I chose 60 days as a reasonable balance between flexibility for providers scheduling deal-making, and the value being effective at reducing squatting. I'm very open to adjusting this value – please make a comment in the discussion thread if you would propose an alternative.
There's no current expiration, but it's worth introducing here because the changes make it practically a bit easier to squat on datacap (possible today, but more involved).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have any specific value in mind. I was trying to understand the reasoning for the 60 days. It sounds reasonable to me as well.
and QA power calculated according to the simple method described above, with no spreading out effect. | ||
|
||
A sector with `SimpleQAPower = false` continues to use the existing QA-power calculation. | ||
Such a sector cannot claim any allocations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is likely to result in increased expirations, with "old" sectors being replaced by new sectors that can take allocations.
In addition, CC sectors can take advantage of this new way to package deals, while sectors with data have to wait for their deals to end.
Could we have a mechanism to turn sectors with SimpleQAPower = false
into sectors with SimpleQAPower = true
? For instance, if I have a 1-year sector with an old 1-year deal, there's no reason why I shouldn't be able to pack new deals into it, using the new logic and QAP calculation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is impossible to change the data in a non-CC sector today anyway (there is no "re-snap"), so there are no sectors to which this would apply. It will be easier to implement re-snapping after this change, though, but by the time we do this will be an even smaller issue.
There are reasons not to do this. Perhaps for a narrow case of sectors where all the deals have already expired there would be fewer unwanted effects, but still this would cause a slight power loss for the SP, so is unlikely a major flow.
I agree that, for a capacity-constrained SP, a CC sector is more valuable than a sector with expired deals at the end of its life, due to the option of taking deals into it. But that's already the case today.
@@ -480,7 +500,7 @@ The built-in market specifies an allocation's minimum term to be equal to the de | |||
and maximum term to be some amount greater than the deal duration. | |||
|
|||
**Parameters** | |||
- The built-in market calculates a term maximum of 1 year greater than a deal's specified duration. | |||
- `MarketDefaultMaximumAllocationTerm`: The built-in market calculates a term maximum of 90 days greater than a deal's specified duration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why 90 days?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not? This value was chosen as a compromise between flexibility for SP deal packing, and reasonably requesting the client's nominated term. The discussion thread for this is #313 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trying to understand the reasoning. I don't have any arguments against the 90 days.
It is not clear to me what happens to Allocations and DataCap tokens when there is a termination. For instance, imagine there is a 2-year sector with a FIL+ deal that has a minimum term of 2 years. Then, after 1 year, the sector terminates. what happens here? Can the remaining 1-year DataCap allocation be used? Should DataCap tokens be given back to the client? Should a new allocation for 1 year be created? |
- The minimum allowed value for `TermMinimum` is 6 months. | ||
- The maximum allowed value for `TermMaximum` is 5 years (can be increased by a future FIP). | ||
- `MinimumVerifiedAllocationTerm`: The minimum allowed value for `TermMinimum` is 6 months. | ||
- `MaximumVerifiedAllocationTerm`: The maximum allowed value for `TermMaximum` is 5 years (can be increased by a future FIP). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- `MaximumVerifiedAllocationTerm`: The maximum allowed value for `TermMaximum` is 5 years (can be increased by a future FIP). | |
- `MaximumVerifiedAllocationTerm`: The maximum allowed value for `TermMaximum` is 5 years (5 years is the current sector max lifetime, both may be increased by a future FIP). |
@misilva73 discussion moved to #313 (comment) |
HasVerifiedClaims
field inSectorOnChainInfo
FYI @ZenGround0