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
Support onion message pathfinding #1669
Support onion message pathfinding #1669
Changes from all commits
8c68980
6d446a8
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.
So we wouldn't reuse all the information we're learning and storing in our
ProbabilisticScorer
. I can see how we're limited with the current penalty being based on the link-level and here we might be interested by node-level reliability in the path construction. That said, we can also assume that a reliable channel == a reliable onion message communication channel and go with it. I don't know if the spec says anything here or what are the thinking of other implementations ?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 an option in the spec to return a separate error for "peer offline" from "no capacity", but I'm not sure how common that is. Once we land the historical scoring PR, we could use the "time in the zero-available-capacity bucket" as a score 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.
Hmmmmm, good question. In general, the MIT and Apache licenses both require attribution, including of downstream projects. However, the Apache license only requires it if there is a file called "NOTICE" or any "copyright, patent, trademark, and attribution notices", which then must be provided downstream, and the MIT license only requires that "the above copyright notice be included", but the original repo doesn't actually include the MIT license anywhere, nor does it include any relevant notices as far as I can see, so there is no relevant "above copyright notice" to include, aside from the first paragraph of the MIT license, which we of course include as LICENSE-MIT. Thus, I think we can reasonably argue that a simple comment above this code indicating that it is adapted from (link) which is code
Copyright Samuel Tardieu
should suffice, which complies with the Apache "You must cause any modified files to carry prominent noticesstating that You changed the files" requirement.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.
#1724 ended up not adapting this crate's implementation anymore (shoutout to Wikpedia)