- 
                Notifications
    
You must be signed in to change notification settings  - Fork 421
 
Introduce ReceiveAuthKey verification for Blinded Payment Paths #4126
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
base: main
Are you sure you want to change the base?
Conversation
| 
           I've assigned @jkczyz as a reviewer!  | 
    
69c57f9    to
    359267e      
    Compare
  
    359267e    to
    13c4041      
    Compare
  
    
          Codecov Report❌ Patch coverage is  Additional details and impacted files@@            Coverage Diff             @@
##             main    #4126      +/-   ##
==========================================
+ Coverage   88.57%   88.63%   +0.06%     
==========================================
  Files         179      180       +1     
  Lines      134374   135169     +795     
  Branches   134374   135169     +795     
==========================================
+ Hits       119016   119808     +792     
+ Misses      12604    12597       -7     
- Partials     2754     2764      +10     
 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
  | 
    
| 
           🔔 1st Reminder Hey @jkczyz! This PR has been waiting for your review.  | 
    
| 
           🔔 2nd Reminder Hey @jkczyz! This PR has been waiting for your review.  | 
    
| 
           🔔 3rd Reminder Hey @jkczyz! This PR has been waiting for your review.  | 
    
| 
           🔔 4th Reminder Hey @jkczyz! This PR has been waiting for your review.  | 
    
| 
           🔔 5th Reminder Hey @jkczyz! This PR has been waiting for your review.  | 
    
Extends the work started in [PR#3917](lightningdevkit#3917) by adding ReceiveAuthKey-based verification for Blinded Payment Paths. This reduces space previously taken by individual ReceiveTlvs and aligns the verification logic with that used for Blinded Message Paths.
Now that we have introduced an alternate mechanism for authentication in the codebase, we can safely remove the now redundant (hmac, nonce) fields from the Payment ReceiveTlvs's while maintaining the security of the onion messages.
| 
           🔔 6th Reminder Hey @jkczyz! This PR has been waiting for your review.  | 
    
| 
           🔔 7th Reminder Hey @jkczyz! This PR has been waiting for your review.  | 
    
| 
           🔔 8th Reminder Hey @jkczyz! This PR has been waiting for your review.  | 
    
| 
           🔔 9th Reminder Hey @jkczyz! This PR has been waiting for your review.  | 
    
| 
           🔔 10th Reminder Hey @jkczyz! This PR has been waiting for your review.  | 
    
| 
           🔔 11th Reminder Hey @jkczyz! This PR has been waiting for your review.  | 
    
| 
           🔔 12th Reminder Hey @jkczyz! This PR has been waiting for your review.  | 
    
| 
           🔔 13th Reminder Hey @jkczyz! This PR has been waiting for your review.  | 
    
| 
           🔔 14th Reminder Hey @jkczyz! This PR has been waiting for your review.  | 
    
| let node_chanmgr = create_node_chanmgrs(1, &node_cfg, &[None]); | ||
| let node = create_network(1, &node_cfg, &node_chanmgr); | ||
| let sender_intended_amt_msat = 100; | ||
| let logger = TestLogger::new(); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should be able to use the logger from Node.
| PeerOffline, | ||
| /// The HTLC was failed because the channel balance was overdrawn. | ||
| ChannelBalanceOverdrawn, | ||
| /// The payload we received was not authenticated by our node. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/was not/could not be
| /// The payload we received was not authenticated by our node. | ||
| /// | ||
| /// This can occur if we constructed an invalid [`BlindedPaymentPath`] | ||
| /// for our counterparty to send a payment over. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"counterparty" implies our immediate peer, but this is more likely from a node not directly connected to us.
| /// This can occur if we constructed an invalid [`BlindedPaymentPath`] | ||
| /// for our counterparty to send a payment over. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it more likely a malicious node tried to de-anonymize us but wasn't able to form a blinded path that we'd authenticate any payment over?
| } | ||
| } | ||
| 
               | 
          ||
| // Note: Authentication TLV field was removed in LDK v0.2 following the | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May need to bump this to 0.3 unless @TheBlueMatt wants to include it in 0.2.
| // `BlindedPaymentPaths`s. Because we do not support receiving to those | ||
| // contexts anymore (they will fail the `ReceiveAuthKey`-based | ||
| // authentication checks), we can reuse those fields here. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't follow the part about re-use.
| 
               | 
          ||
| // HMAC input for `ReceiveTlvs`. The HMAC is used in `blinded_path::payment::PaymentContext`. | ||
| const PAYMENT_TLVS_HMAC_INPUT: &[u8; 16] = &[8; 16]; | ||
| // const PAYMENT_TLVS_HMAC_INPUT: &[u8; 16] = &[8; 16]; | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove instead of commenting out.
| 
           👋 The first review has been submitted! Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.  | 
    
Building on the goals set forth in #3917, this PR introduces ReceiveAuthKey-based verification for Blinded Payment Paths.
Key Outcomes
ReceiveTlvsare noticeably reduced in size, making blinded paths lighter.Follow-Up Preparation
This PR also lays the groundwork for introducing dummy payment hops in a follow-up PR.
By minimizing per-hop authentication data, we keep dummy hops compact — preserving overall path size and ensuring that forward and dummy TLVs are padded to the same length, improving hop indistinguishability and privacy.