routing: ignore unknown required features in pathfinding#3893
Merged
cfromknecht merged 5 commits intoJan 8, 2020
Merged
Conversation
cfromknecht
requested review from
Roasbeef,
halseth and
joostjager
as code owners
cfromknecht
force-pushed
the
pathfind-ignore-required-features
branch
from
b76baa9 to
90816ba
Compare
joostjager
reviewed
Jan 8, 2020
cfromknecht
force-pushed
the
pathfind-ignore-required-features
branch
2 times, most recently
from
321a1f7 to
80690d0
Compare
joostjager
approved these changes
Jan 8, 2020
cfromknecht
force-pushed
the
pathfind-ignore-required-features
branch
from
80690d0 to
dd1b2be
Compare
This wraps the raw UnknownRequiredFeatures method and returns a proper error type to enable more exact testing.
This commit brings us inline with recent modifications to the spec, that say we shouldn't pay nodes whose feature vectors signal unknown required features, and also that we shouldn't route through nodes signaling unknown required features. Currently we assert that invoices don't have such features during decoding, but now that users can specify feature vectors via the rpc interface, it makes sense to perform this check deeper in call stack. This will also allow us to remove the check from decoding entirely, making decodepayreq more useful for debugging.
This commit removes the unknown required feature bit check from the invoice decoding logic. This allows greater utility to users of the decodepayreq rpc since it can provide inspection of otherwise invalid invoices. In the prior commit, this check moved into our path finding logic, so invalid features taken from an invoice will instead cause a failure when attempting to pay.
This allows us to remove the custom error type originally implemented for this purpose.
cfromknecht
force-pushed
the
pathfind-ignore-required-features
branch
from
dd1b2be to
2510ec0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This PR brings us inline with recent modifications to the spec, that
say we shouldn't pay nodes whose feature vectors signal unknown required
features, and also that we shouldn't route through nodes signaling
unknown required features.
Currently we assert that invoices don't have such features during
decoding, but now that users can specify feature vectors via the rpc
interface, it makes sense to perform this check deeper in call stack.
This will also allow us to remove the check from decoding entirely,
making decodepayreq more useful for debugging.
We also fix one of the test vectors which had been incorrectly copied
from lightning/bolts#719.
Feature bit 9 was missing from the current encoding, which is caught
now that we consider the decoding valid and actually perform the
comparison with the expected struct.