Introduce Padding for Payment and Message Blinded Tlvs

[shaavan]

Description

This PR introduces padding for Payment and Message Blinded TLVs to ensure that the size of each packet in the path is uniform.

[shaavan]

Notes:

1. Padding Implementation:
   • The padding field is now expanded to contain Option<Vec>. This change allows the inclusion of padding within the packet while retaining the flexibility to leave the padding as None.
2. Impact on Path Length:
   • With the added padding, the length of each individual packet increases, which results in a reduction of the maximum path length. Consequently, the max_path_length in forward_check_failures has been adjusted from 18 to 17.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 98.48485% with 2 lines in your changes missing coverage. Please review.

Project coverage is 88.53%. Comparing base (1434e9c) to head (758d06d).

Files with missing lines	Patch %	Lines
lightning/src/blinded_path/utils.rs	94.44%	1 Missing and 1 partial :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3177      +/-   ##
==========================================
- Coverage   88.54%   88.53%   -0.01%     
==========================================
  Files         149      149              
  Lines      114579   114704     +125     
  Branches   114579   114704     +125     
==========================================
+ Hits       101457   101558     +101     
- Misses      10633    10650      +17     
- Partials     2489     2496       +7     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[shaavan]

Updated from pr3177.01 to pr3177.02 (diff): Addressed @jkczyz's comments.

Changes:

1. Reordered commits and separated the update to the Padding struct into a separate commit.
2. Updated the Padding struct to contain a usize instead of a Vec to save on heap allocation, and made the Writable implementation more efficient.
3. Introduced two f: commits to use iter clone instead of Vec allocation, keeping the code efficient.
4. Introduced a test and a test utility to verify that blinded message and payment paths are properly padded, ensuring a consistent payload size.

[shaavan]

Updated from pr3177.02 to pr3177.03 (diff):

Changes:

1. Rebase on main to fix ci.

Diff post rebase:

--- a/lightning/src/ln/offers_tests.rs
+++ b/lightning/src/ln/offers_tests.rs
@@ -1789,7 +1789,7 @@ fn test_blinded_path_padding() {
        let onion_message = charlie.onion_messenger.next_onion_message_for_peer(david_id).unwrap();
        david.onion_messenger.handle_onion_message(&charlie_id, &onion_message);
 
-       let invoice = extract_invoice(david, &onion_message);
+       let (invoice, _) = extract_invoice(david, &onion_message);
        assert_eq!(invoice, expected_invoice);

[shaavan]

Updated from pr3177.03 to pr3177.04 (diff): Addressed @jkczyz comments

1. Refactor max_length calculation to be cleaner.
2. Introduce wrapper struct instead of tuple to make the code clearer to understand.
3. Move the padding testing to onion_message/functional_tests.rs to simplify the setup, and test the code at a more appropriate place.

[shaavan]

Updated from pr3177.04 to pr3177.05 (diff):

Updates:

1. Rebase on main.

[shaavan]

Updated from pr3177.05 to pr3177.06 (diff): Addressed @jkczyz comments

Changes:

1. Squash commits.
2. Introduce a generic struct WithPadding to keep the code DRY.
3. Introduce a test for Blinded Payment Paths.
4. Use saturating sub in padding length calculation to avoid overflows.

[shaavan]

Updated from pr3177.06 to pr3177.07 (diff): Addressed @jkczyz comments

1. Update comments.
2. Introduce debug_assert!() in Writeable for WithPadding.
3. Properly assert!() the condition in test.

[shaavan]

Updated from pr3177.07 to pr3177.08 (diff): Addressed @jkczyz comments

Changes:

1. Introduce an f: commit with an alternative approach to WithPadding.
2. The new approach introduces padding as an optional field within the ForwardTlvs and the ReceiveTlvs struct.

[shaavan]

Update: From pr3177.08 to pr3177.09 (diff):

Changes based on @jkczyz's feedback:

1. Fixed the imports.
2. Removed BlindedPaymentTlvsRef and replaced it with BlindedPaymentTlvs. The former contained an immutable reference to the underlying TLVs, but the new implementation required mutability. Therefore, I transitioned to using BlindedPaymentTlvs directly, making BlindedPaymentTlvsRef redundant.

[shaavan]

Update: From pr3177.09 to pr3177.10 (diff):

Changes based on @jkczyz's feedback:

1. Rename pad_tlvs -> pad_to_length.

[shaavan]

Update: From pr3177.10 to pr3177.11 (diff):

Changes:

1. Cleaned up commit history (no changes to the code)

[shaavan]

Update: From pr3177.11 to pr3177.12 (diff):

Changes:

1. Rebase on @jkczyz's branch (with no changes to the approach) to test the PR with new construct_blinded_hops, and experiment with BlindedPaymentTlvsMut approach.

[shaavan]

Update: From pr3177.12 to pr3177.13 (diff):

Changes:

1. Rebase on #3248.
2. Introduce a new utility function inner_blinded_path in BlindedMessagePath for testing.

[shaavan]

Update: From pr3177.13 to pr3177.14 (diff):

Changes:

1. Rebase on main.
2. Changes from the last version: Rename ForwardNode -> MessageForwardNode.

[shaavan]

Update: From pr3177.14 to pr3177.15 (diff):

Addressed @jkczyz comments:

1. Update docs, to be grammatically and syntactically.
2. Introduce the BlindedPaymentTlvsMut approach.
3. Clean commit history to keep changes properly separate.
4. Update the test function to remove redundant utils.

[jkczyz]

Generally looks good. Just waiting on review for #3248.

[dunxen]

#3248 Merged. Needs rebase :)

[shaavan]

Update: From pr3177.15 to pr3177.16 (diff):

Changes:

1. Rebase on main.

[shaavan]

Update: From pr3177.16 to pr3177.17 (diff):

Addressed @jkczyz comment

Changes:

1. Update pad_to_length functions to use match instead of unwrap()
2. Update the test to have non-zero context, to ensure the size of receive tlvs and forward tlvs vary before padding.

[shaavan]

Update: From pr3177.17 to pr3177.18 (diff): Addressed @jkczyz comment

Changes:

1. Reverted visibility changes for BlindedPath in BlindedMessagePath.

[shaavan]

Update: From pr3177.18 to pr3177.19 (diff):

Changes:

1. Rebase on main to resolve merge conflicts.

[github-advanced-security[bot]]

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

[shaavan]

Update:
From pr3177.19 to pr3177.20 (diff):
Addressed feedback from @TheBlueMatt and @jkczyz.

Changes:

1. Simplified the padding approach by removing padding as a field from {Forward, Receive} TLVs.
2. Introduced a WithPadding generic that handles padding for the given TLVs.

Considerations & Shortcomings:

• Instead of padding to a maximum value, WithPadding pads the TLVs to a rounded value. This avoids wasting space for very small packets while providing flexibility for future TLV expansions.
• The current padding round-off value is 50 bytes. I believe this should offer enough buffer to obscure the internal TLV types, but would love to get feedback on a more optimal value.
• This approach doesn't safeguard against the improper use of a 1 TLV record within the TLVs themselves. We may need to add a comment to prevent this misuse.

Todo:

• Currently, this approach also pads the compact blinded path. I'll address this in the next update.

[shaavan]

Update: From pr3177.20 to pr3177.21 (diff): Addressed @TheBlueMatt comment

Changes:

1. Update code so that we would not be padding the compact blinded paths.
2. Introduce a test to check this condition.

[shaavan]

Update: From pr3177.21 to pr3177.22 (diff):

Changes:

1. Rebase on main to resolve merge conflicts.

[shaavan]

Update: From pr3177.22 to pr3177.23 (diff):

Changes:

Fixed the Padding approach.

1. Previous version: Packets were padded individually based on compact forward TLVs. This incorrectly led to padding ReceiveTlvs, even when they belonged to a compact blinded path.
2. Fix: Now, the entire blinded path remains unpadded if any short_channel_id is present. This ensures correct handling of compact paths and eliminates unnecessary padding.

[shaavan]

Update: From pr3177.23 to pr3177.24 (diff):

Addressed @jkczyz comments

Changes:

1. Fix typo, and visibility.

[shaavan]

Update: From pr3177.24 to pr3177.25 (diff): Addressed comments from @jkczyz and @TheBlueMatt.

Changes:

1. Introduced different padding round-off for message and payment TLVs.
2. Padding sizes were chosen based on the discussion in this thread.

[shaavan]

Update: From pr3177.25 to pr3177.26 (diff): Addressed @jkczyz comments

Changes:

1. Update the Padding for Payment Tlvs to 35, according to this discussion.