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

multi: deprecate IncorrectHtlcAmount #2554

Merged
merged 3 commits into from Jan 30, 2019

Conversation

Projects
None yet
4 participants
@Roasbeef
Copy link
Member

Roasbeef commented Jan 29, 2019

Fixes #2321 (see issue for details).

One implementation level thing to note is that we'll still expect this error on the network when decoding and during routing retry attempts. Otherwise, we won't be able to parse the errors from nodes in the wild that haven't upgraded to this new version.

Replaces #2363.

@Roasbeef Roasbeef force-pushed the Roasbeef:deprecate-htlc-amt-error branch 2 times, most recently from 71ebea1 to 22f6728 Jan 29, 2019

@Roasbeef Roasbeef added this to the 0.5.2 milestone Jan 29, 2019

@wpaulino
Copy link
Collaborator

wpaulino left a comment

LGTM 🏮

Show resolved Hide resolved lnd_test.go
Show resolved Hide resolved routing/router.go Outdated

@Roasbeef Roasbeef force-pushed the Roasbeef:deprecate-htlc-amt-error branch from 22f6728 to 698f7db Jan 30, 2019

@Roasbeef

This comment has been minimized.

Copy link
Member Author

Roasbeef commented Jan 30, 2019

Commits squashed down, PTAL @cfromknecht!

multi: deprecate IncorrectHtlcAmount onion error
In this commit, we deprecate the `IncorrectHtlcAmount` onion error.
We'll still decode this error to use when retrying paths, but we'll no
longer send this ourselves. The `UnknownPaymentHash` error has been
amended to also include the value of the payment as well. This allows us
to worry about one less error.

@Roasbeef Roasbeef force-pushed the Roasbeef:deprecate-htlc-amt-error branch from 698f7db to 6dc9e3e Jan 30, 2019

@cfromknecht
Copy link
Collaborator

cfromknecht left a comment

LGTM! 🤛

Roasbeef added some commits Jan 29, 2019

test: update error propagation test for new onion error change
In this commit, we update the error propagation test to ensure that the
proper HTLC amount is included for the `CodeUnknownPaymentHash` error.

@Roasbeef Roasbeef dismissed stale reviews from wpaulino and cfromknecht via b1b42dc Jan 30, 2019

@Roasbeef Roasbeef force-pushed the Roasbeef:deprecate-htlc-amt-error branch from 6dc9e3e to b1b42dc Jan 30, 2019

@Roasbeef Roasbeef merged commit 8bd29de into lightningnetwork:master Jan 30, 2019

1 of 2 checks passed

coverage/coveralls Coverage decreased (-0.05%) to 59.069%
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment