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

Prefactor PaymentParameters for blinded recipients #2258

Conversation

valentinewallace
Copy link
Contributor

Several pay params fields are specific to either blinded or unblinded recipients, so here we use an enum to separate the two cases and ensure that an invalid PaymentParameters can't be created.

@valentinewallace valentinewallace force-pushed the 2023-04-blinded-pathfinding-groundwork-2 branch from f7d4237 to 215ea05 Compare May 2, 2023 21:46
@valentinewallace valentinewallace force-pushed the 2023-04-blinded-pathfinding-groundwork-2 branch from 215ea05 to aec3d4d Compare May 3, 2023 18:20
@codecov-commenter
Copy link

codecov-commenter commented May 3, 2023

Codecov Report

Patch coverage: 83.82% and project coverage change: -0.01 ⚠️

Comparison is base (2cae6f0) 91.58% compared to head (fb9983c) 91.57%.

❗ Current head fb9983c differs from pull request most recent head a84e832. Consider uploading reports for the commit a84e832 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2258      +/-   ##
==========================================
- Coverage   91.58%   91.57%   -0.01%     
==========================================
  Files         104      105       +1     
  Lines       51993    60983    +8990     
  Branches    51993    60983    +8990     
==========================================
+ Hits        47616    55848    +8232     
- Misses       4377     5135     +758     
Impacted Files Coverage Δ
lightning/src/ln/features.rs 97.25% <0.00%> (-1.21%) ⬇️
lightning/src/ln/channelmanager.rs 91.20% <25.00%> (+2.40%) ⬆️
lightning/src/routing/router.rs 94.45% <83.22%> (-0.64%) ⬇️
lightning/src/ln/payment_tests.rs 97.57% <92.85%> (-0.01%) ⬇️
lightning-invoice/src/payment.rs 89.11% <100.00%> (ø)
lightning-invoice/src/utils.rs 97.17% <100.00%> (ø)
lightning/src/ln/functional_test_utils.rs 94.90% <100.00%> (+2.11%) ⬆️
lightning/src/ln/functional_tests.rs 98.25% <100.00%> (ø)
lightning/src/ln/onion_route_tests.rs 98.26% <100.00%> (ø)
lightning/src/ln/priv_short_conf_tests.rs 97.59% <100.00%> (ø)
... and 1 more

... and 18 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@valentinewallace valentinewallace force-pushed the 2023-04-blinded-pathfinding-groundwork-2 branch from aec3d4d to 0275873 Compare May 3, 2023 18:29
Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

Basically LGTM.

lightning/src/routing/router.rs Show resolved Hide resolved
lightning/src/routing/router.rs Outdated Show resolved Hide resolved
Minor changes in preparation for supporting route blinding in
PaymentParameters. In the next commit, we'll be moving more
unblinded-payee-specific fields from the top level parameters into the clear
enum variant.
@valentinewallace valentinewallace force-pushed the 2023-04-blinded-pathfinding-groundwork-2 branch 2 times, most recently from 1e3d7af to 0f823fb Compare May 4, 2023 14:49
@valentinewallace valentinewallace force-pushed the 2023-04-blinded-pathfinding-groundwork-2 branch from 0f823fb to 4729342 Compare May 4, 2023 14:54
lightning/src/ln/features.rs Show resolved Hide resolved
lightning/src/routing/router.rs Outdated Show resolved Hide resolved
@valentinewallace valentinewallace force-pushed the 2023-04-blinded-pathfinding-groundwork-2 branch from 4729342 to 554c1fc Compare May 4, 2023 18:13
jkczyz
jkczyz previously approved these changes May 4, 2023
lightning/src/routing/router.rs Outdated Show resolved Hide resolved
lightning/src/routing/router.rs Outdated Show resolved Hide resolved
@valentinewallace valentinewallace force-pushed the 2023-04-blinded-pathfinding-groundwork-2 branch 2 times, most recently from 1298418 to dca58c3 Compare May 5, 2023 19:36
Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

Basically lgtm, a few logging nits.

lightning/src/routing/router.rs Outdated Show resolved Hide resolved
lightning/src/routing/router.rs Outdated Show resolved Hide resolved
@valentinewallace valentinewallace force-pushed the 2023-04-blinded-pathfinding-groundwork-2 branch from dca58c3 to 9d7a370 Compare May 8, 2023 15:42
@valentinewallace valentinewallace force-pushed the 2023-04-blinded-pathfinding-groundwork-2 branch from 9d7a370 to fb9983c Compare May 8, 2023 19:24
@valentinewallace valentinewallace force-pushed the 2023-04-blinded-pathfinding-groundwork-2 branch from fb9983c to a84e832 Compare May 8, 2023 21:29
@valentinewallace valentinewallace force-pushed the 2023-04-blinded-pathfinding-groundwork-2 branch from a84e832 to d56672c Compare May 8, 2023 22:01
@TheBlueMatt TheBlueMatt merged commit 0ecb4b0 into lightningdevkit:main May 8, 2023
@jkczyz jkczyz mentioned this pull request May 10, 2023
60 tasks
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.

4 participants