Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
On-chain Retrieval Expectations #862
base: master
Are you sure you want to change the base?
On-chain Retrieval Expectations #862
Changes from 1 commit
0d2e9cc
0ea0f85
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Nit update to be more aligned with what appears to be convention.
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 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.
Is this change specific to f05 market deals, or intended to set a standard for future user programmed storage markets?
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.
@willscott during your presentation at LabWeek, I believe you articulated five specific tiers. Have these been reduced to three?
If not, please be specific in listing exactly which tiers/retrieval types are being specified. The abstract should be brief, but also explicit.
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 section is lacking describing the motivation for any consensus change here.
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.
+1 to above.
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.
Please elaborate on how encoding these SLAs at the protocol level will affect Filecoin functionality in the future, and/or address concrete issues in the present.
Why should retrieval expectations become policy at the protocol level, as opposed to incorporated as more arbitrary (i.e., optional) deal metadata? Why should SLA tiers be a required component of deal-making for all network participants?
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.
Can you expand on how these touch the issue?
FastRetrieval
isn't part of consensus protocol AFAIK so is a good example of FRC-type data here.Label
is opaque at the consensus level, but can support conventions being built on it in FRCs.VerifiedDeal
is an instruction to the built-in market actor to act as an operator for the client's datacap and make a verified allocation concurrent with publishing a deal, but I don't see the relevance to this proposal.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.
Is
overview
TODO or tiers, deal flow change and so on should be heading 4?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.
For my own understanding- where does 5x/10x coming from? Fil+?
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 think the replication factor would have a direct or indirect relationship to FIL+, but rather to external expectations and potentially observed fault and retrievability odds. It's probably just where clients/providers landed, like web3.storage picking "minimum 5 deals".
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 change implies a bunch more changes. The DealProposal is part of built-in market actor API and state.
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 also quite under-specified.
The whole design intent behind a piece activation manifest is that the consensus layer need know nothing about markets and deals etc. There is an opaque payload field in order to pass information through to a market contract but, like deal label, it's completely up to market contracts to interpret. I can't tell if you mean to specify some format of data in that payload, or put a field into the consensus-level schema. If it's the latter, I will object (but can take it to the discussion topic).
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 section doesn't have much to do with the FVM, and should probably just be folded into the main actor change specification.
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.
First, this retrieval expectation cannot be enforced onchain, just like
fastretrieval
. There have been many discussion on havingfastretrieval
in the protocol sends wrong expectation and should be removed, therefore I’m not sure adding another explicit field similarly is a good idea.second, have you considered to propose an FRC for setting such “tag” in the
label
field instead? Im trying to guess the motivation of the fip here as its under specified currently, and it seems like one of the main goal is to allow data client to signal retrieval expectation? If so, I think adding in the label is worth considering to avoid migrations. However, if the goal is also providing retrieval guarantees, then this fip needs more work on that.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.
To this point particularly - I’m curious if the teirs should be defined in the consensus protocol level, as that implies for any changes it requires a network upgrade.
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.
Adding an optional field is not a backwards compatible change to actor APIs. As noted above, this probably needs additional methods and a state migration.
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.
Question- where does the 'direct calculation of expected network bandwidth provisioning' take place? Is this a business hypothetical, or a concrete part of the deal-making process/fee calculation that I'm unaware of?
More frequently retrieved data of course has higher bandwidth requirements during retrieval, but does this anticipated bandwidth use actually get passed on to the SP or deal client during deal making? Is there a higher gas fee, etc., charged to deals that are likely to require greater network bandwidth, etc.?