Skip to content
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

Don't rebroadcast channel updates from update_fail_htlc #2775

Merged
merged 1 commit into from
Nov 10, 2023

Conversation

t-bast
Copy link
Member

@t-bast t-bast commented Nov 10, 2023

When we receive a channel_update in update_fail_htlc, we should take it into account to exclude channels or correctly retry with updated fees, but we shouldn't apply it to our routing table.

If we did, that could be exploited to deanonymize our payments. It shouldn't be necessary anyway, as honest nodes should broadcast those channel_updates publicly, so we would receive them through the normal gossip mechanism.

Fixes #2767

When we receive a `channel_update` in `update_fail_htlc`, we should take
it into account to exclude channels or correctly retry with updated fees,
but we shouldn't apply it to our routing table.

If we did, that could be exploited to deanonymize our payments.
It shouldn't be necessary anyway, as honest nodes should broadcast
those `channel_update`s publicly, so we would receive them through the
normal gossip mechanism.

Fixes #2767
@t-bast t-bast requested a review from thomash-acinq November 10, 2023 15:07
Copy link
Member

@thomash-acinq thomash-acinq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice privacy fix

@codecov-commenter
Copy link

Codecov Report

Merging #2775 (fe969e6) into master (5fa7d4b) will decrease coverage by 0.02%.
Report is 1 commits behind head on master.
The diff coverage is n/a.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##           master    #2775      +/-   ##
==========================================
- Coverage   85.83%   85.82%   -0.02%     
==========================================
  Files         216      216              
  Lines       18129    18128       -1     
  Branches      775      773       -2     
==========================================
- Hits        15561    15558       -3     
- Misses       2568     2570       +2     
Files Coverage Δ
...r/acinq/eclair/payment/send/PaymentLifecycle.scala 86.84% <ø> (-0.07%) ⬇️

... and 8 files with indirect coverage changes

@t-bast t-bast merged commit 7be7d5d into master Nov 10, 2023
1 check passed
@t-bast t-bast deleted the no-failed-payment-updates-in-graph branch November 10, 2023 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don't apply channel_updates from onion errors to our graph
3 participants