Temp 7298 #43
Closed
Temp 7298 #43
Conversation
This file contains 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
Pull reviewers statsStats of the last 30 days for lnd:
|
carlaKC
force-pushed
the
temp-7298
branch
22 times, most recently
from
f118132
to
6e23f42
Compare
carlaKC
force-pushed
the
temp-7298
branch
2 times, most recently
from
ba1c717
to
c41b5de
Compare
carlaKC
force-pushed
the
temp-7298
branch
4 times, most recently
from
69705b9
to
e5b2cff
Compare
carlaKC
force-pushed
the
temp-7298
branch
5 times, most recently
from
777b36c
to
bb88353
Compare
This commit adds handling for malformed HTLC errors related to blinded paths. We expect to receive these errors _within_ a blinded path, because all non-introduction nodes are instructed to return malformed errors for failures. Note that we may actually switch back to a malformed error later on if we too are a relaying node in the route, but we handle that case the incoming link.
…ypes This commit moves all our validation related to the presence of fields into ValidateParsedPayloadTypes so that we can handle them in a single place. We draw the distinction between: - Validation of the payload (and the context within it's being parsed, final hop / blinded hop etc) - Processing and validation of encrypted data, where we perform additional cryptographic operations and validate that the fields contained in the blob are valid. This helps draw the line more clearly between the two validation types, rather than splitting some payload-releated blinded hop processing into the encrypted data processing part. The downside of this approach (vs doing the blinded path payload check _after_ payload validation) is that we have to pass additional context into payload validation (ie, whether we got a blinding point in our UpdateAddHtlc - as we already do for isFinalHop).
When handling blinded errors, we need to know whether there was a blinding key in our payload when we successfully parsed our payload but then found an invalid set of fields. The combination of parsing and validation in NewPayloadFromReader means that we don't know whether a blinding point was available to us by the time the error is returned. This commit splits parsing and validation into two functions so that we can take a look at what we actually pulled of the payload in between parsing and TLV validation.
carlaKC
force-pushed
the
temp-7298
branch
2 times, most recently
from
ba012fc
to
db8ed95
Compare
We need to know what role we're playing to be able to handle errors correctly, but the information that we need for this is held by our iterator: - Whether we had a blinding point in update add (blinding kit) - Whether we had a blinding point in payload As we're now going to use the route role return value even when our err!=nil, we rename the error to signal that we're using less canonical golang here. An alternative to this approach is to attach a RouteRole to our ErrInvalidPayload. The downside of that approach is: - Propagate context through parsing (whether we had updateAddHtlc) - Clumsy handling for errors that are not of type ErrInvalidPayload
Introduce two wrapper types for our existing SphinxErrorEncrypter that are used to represent error encrypters where we're a part of a blinded route. These encrypters are functionally the same as a sphinx encrypter, and are just used as "markers" so that we know that we need to handle our error differently due to our different role. We need to persist this information to account for restart cases where we've resovled the outgoing HTLC, then restart and need to handle the error for the incoming link. Specifically, this is relevant for: - On chain resolution messages received after restart - Forwarding packages that are re-forwarded after restart This is also generally helpful, because we can store this information in one place (the circuit) rather than trying to reconstruct it in various places when forwarding the failure back over the switch.
Create our error encrypter with a wrapped type if we have a blinding point present. Doing this in the iterator allows us to track this information when we have both pieces of information available to us, compared to trying to handle this later down the line: - Downstream link on failure: we know that we've set a blinding point for out outgoing HTLC, but not whether we're introduction or not - Upstream link on failure: once the failure packet has been sent through the switch, we no longer know whether we were the introduction point (without looking it up / examining our payload again / propagating this information through the switch).
Set obfuscator for use in blinded error handling when we forward failures through the switch.
carlaKC
force-pushed
the
temp-7298
branch
3 times, most recently
from
324dc78
to
cdc18ce
Compare
For tests where our payments require an on-chain resolution, provide longer timeout and return cancel functions.
This itest adds a test that we still propagate blinded errors back properly after a restart with an on-chain resolution. The test also updates our sendpayment timeout to longer so that there's time to resolve the on chain claim.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Change Description
Description of change / link to associated issue.
Steps to Test
Steps for reviewers to follow to test the change.
Pull Request Checklist
Testing
Code Style and Documentation
[skip ci]in the commit message for small changes.📝 Please see our Contribution Guidelines for further guidance.