-
Notifications
You must be signed in to change notification settings - Fork 919
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
feat(share/getter) Add support for Non-inclusion proofs #2256
feat(share/getter) Add support for Non-inclusion proofs #2256
Conversation
9fe8e73
to
8128c04
Compare
# Conflicts: # share/get_test.go # share/ipld/get.go # share/ipld/namespace_data.go
93a356c
to
4bce3ea
Compare
5052044
to
2046250
Compare
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 have only one general API concern. The impl LGTM turned out to be much easier than I originally thought.
Codecov Report
@@ Coverage Diff @@
## main #2256 +/- ##
==========================================
+ Coverage 50.48% 50.60% +0.12%
==========================================
Files 157 157
Lines 9933 9925 -8
==========================================
+ Hits 5015 5023 +8
+ Misses 4472 4455 -17
- Partials 446 447 +1
|
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.
Ok, so we decided in sync to remove the error and make the data not found the case a success case, so requesting changes until this is done
c54d9c4
to
11c71db
Compare
## Notable Changes - `namespace.ID` to `share.Namespace` - Changes every comment namespace ID mentioning to just namespace. I renamed every such mention besides in ADRs. I don't want to touch ADRs here, as they need a more holistic re-review and up-to-date catchup. - Uses all the utility methods on the type, where suitable - Namespace constructor now only creates Blob Namespaces. For other reserved namespaces, the predefined globals should be used. - Uses namespace.ValidateDataNamespace everywhere data is requested. This is guarantees we verify the namespace are 100% valid and forbids requesting parity and padding namespaces. - Restricts PFBs for reserved namespaces - Reversed the dependency from `share -> share/ipld` to `share/ipld -> share` - NewBlobV0 constructor. Similar to NewNamespaceV0 - `sharetest` pkg for share related testing utilities - `edstest` pkg for eds related testing utilities ## Follow-ups - `blobtest` pkg to generate node's blob type ## Refs * Substitutes zombie PR #2376. I push to the branch, but GH does not see commits. * Based on #2367 * Closes #2301 * Closes #2309 * Blocked on #2256
Overview
This PR implements support for non-inclusion proof for getters and shrex protocol.
Requires #2242 to be merged first