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

Trampoline to blinded #2811

Merged
merged 3 commits into from Jan 25, 2024
Merged

Trampoline to blinded #2811

merged 3 commits into from Jan 25, 2024

Conversation

thomash-acinq
Copy link
Member

Allow trampoline to pay a list of blinded paths instead of a node id. Only the last trampoline hop can target blinded paths, trampoline nodes are still reached with their node id, not with blinded paths.

@codecov-commenter
Copy link

codecov-commenter commented Jan 11, 2024

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (e66e6d2) 85.86% compared to head (df1773c) 85.97%.

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

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2811      +/-   ##
==========================================
+ Coverage   85.86%   85.97%   +0.10%     
==========================================
  Files         216      217       +1     
  Lines       18228    18248      +20     
  Branches      772      800      +28     
==========================================
+ Hits        15652    15688      +36     
+ Misses       2576     2560      -16     
Files Coverage Δ
.../scala/fr/acinq/eclair/payment/PaymentPacket.scala 91.45% <100.00%> (+1.27%) ⬆️
...la/fr/acinq/eclair/payment/relay/NodeRelayer.scala 100.00% <100.00%> (ø)
.../scala/fr/acinq/eclair/payment/relay/Relayer.scala 89.13% <100.00%> (ø)
...air/payment/send/CompactBlindedPathsResolver.scala 100.00% <100.00%> (ø)
...scala/fr/acinq/eclair/payment/send/Recipient.scala 98.63% <100.00%> (+2.79%) ⬆️
...a/fr/acinq/eclair/wire/protocol/PaymentOnion.scala 99.36% <ø> (+3.82%) ⬆️
...r-core/src/main/scala/fr/acinq/eclair/Eclair.scala 56.43% <80.00%> (+0.36%) ⬆️
...cala/fr/acinq/eclair/payment/relay/NodeRelay.scala 96.22% <98.00%> (-0.35%) ⬇️
...la/fr/acinq/eclair/payment/send/OfferPayment.scala 72.34% <87.50%> (-3.66%) ⬇️
...r/acinq/eclair/payment/send/PaymentInitiator.scala 93.49% <94.11%> (+6.92%) ⬆️

... and 7 files with indirect coverage changes

@thomash-acinq
Copy link
Member Author

Attention: 3 lines in your changes are missing coverage. Please review.

Does anyone know how to see these 3 lines ?

@t-bast
Copy link
Member

t-bast commented Jan 12, 2024

Does anyone know how to see these 3 lines ?

The codecov website doesn't seem to show these, I guess they're trying to incentivize us to install their GitHub app but it's probably not worth it!

@thomash-acinq thomash-acinq force-pushed the trampoline-to-blinded branch 3 times, most recently from 087d96b to ee277d6 Compare January 15, 2024 15:19
@thomash-acinq thomash-acinq force-pushed the trampoline-to-blinded branch 5 times, most recently from 3d7f4bf to e94b8b8 Compare January 23, 2024 08:55
Allow trampoline to pay a list of blinded paths instead of a node id.
Only the last trampoline hop can target blinded paths, trampoline nodes are still reached with their node id, not with blinded paths.
Copy link
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

The architecture looks good to me, most feedback is nit.

Copy link
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

LGTM, a few nits in #2814 and this should be ready to go!

@thomash-acinq
Copy link
Member Author

Merged #2814

@thomash-acinq thomash-acinq merged commit aae16cf into master Jan 25, 2024
1 check passed
@thomash-acinq thomash-acinq deleted the trampoline-to-blinded branch January 25, 2024 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants