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
DAS protobuffs #710
DAS protobuffs #710
Changes from 2 commits
91feb28
fe9abf1
4f5aaf4
3fd77c7
4a1a1ac
135bae7
b8795c0
31610fb
dce9903
808311c
204949a
0dad952
45bb359
d8e15e8
cc291e4
60c0390
0a29fb2
06005c7
0d88c86
3659a68
28f3e20
bcf4718
a82c869
267b0c6
e8fec17
ad6c587
a524c0b
d3a9be4
ad0a710
9f44421
9ac3965
4be81a3
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.
As above, should we support both lookup schemes?
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.
Would it make sense to support requesting multiple chunks at once?
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.
Exactly how this is implemented depends a lot on whether we will eventually stop supporting lookup by blob header hash. Will make this change once I know the answer to this question.
Supporting multiple requests at once makes a lot of sense to me. All also add that in when I make the change to support lookup by both mechanisms.
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.
Hmm... the more I think about it, the more I'd like to delay adding things that are not absolutely necessary in this iteration. Could be circle back and add methods that return multiple chunks at a future time?
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 is this API specific to agent light node? Should it applicable to all light nodes?
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.
All light nodes will expose this API. Non-agents will return an error if it is called. Agents will return an error if the proper authentication token is not provided, and that error will look identical to if a non-agent is called. It's important for the public interface of an agent light node to be indistinguishable from the public interface of an agent light node.
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.
Good to know this, it may be worth documenting the difference of behavior between agent vs. non-agent LNs.