-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat(api): chain export v2 support #13395
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
base: master
Are you sure you want to change the base?
Changes from all commits
20e8fd2
7e0058a
abf7f6d
9adf222
d636d4d
74d34c5
ad2bbec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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 going to be an annoyingly breaking change .. of the type we've kind of agreed to avoid if at all possible in v1
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.
Yes, this is a change that needs careful consideration, so I'm putting it in a separate PR now.
We will need it eventually, and using lotus-shed to export is not always convenient (need to take the node offline). But I agree that we can discuss it more. I just submitted it first so that you can see the diff
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.
We could just switch version depending on tipset, at some epoch we start exporting v2 and prior to that we export v1. As long as we document it clearly, and those exports work in active implementations then I don't think this would be a major issue?
Unless we still have use for v1 exports that I'm not aware of. @LesnyRumcajs might be able to fill us in here or bring in someone who can.
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.
From Forest's perspective, it's fine to have a breaking change here.
ChainExportis not a part of the Common Node API, see https://github.com/filecoin-project/FIPs/blob/master/FRCs/frc-0104.md and so there don't have to be very strong guarantees about breaking changes.lotus chain export. I don't expect any RPC provider to expose it. That said, maybe some tooling does it in a raw way.Filecoin.ChainExportand in fact, don't use it at all internally, preferring Forest.ChainExport. Again, this is due to it being strictly internal RPC and not supposed to be exposed in any way for obvious reasons.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 a simpler approach; it's always better to avoid breaking changes.