Skip to content

Conversation

vincenzopalazzo
Copy link
Owner

No description provided.

Signed-off-by: Vincenzo Palazzo <[email protected]>
@vincenzopalazzo vincenzopalazzo requested a review from Copilot April 24, 2025 12:43
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces BLIP-0042–based contact functionality into the invoice request flow by adding new TLV record types and associated data structures and methods.

  • Adds new contact-related TLV record types and structures (ContactSecret, Bip353Name) along with a helper function to derive deterministic contact secrets in payer.rs.
  • Extends the invoice request builder with methods to set or derive a contact secret, attach a payer offer, and include BIP‑353 name/signature, while updating TLV stream accessors and builders accordingly in invoice_request.rs.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
lightning/src/offers/payer.rs Added contact TLV types, structures, and a secret derivation helper
lightning/src/offers/invoice_request.rs Extended invoice request builder, accessors, and TLV streams for contact features

/// Successive calls to this method will override the previous setting.
pub fn with_payer_offer($($self_mut)* $self: $self_type, payer_offer: &Offer) -> $return_type {
let mut offer_bytes = Vec::new();
payer_offer.write(&mut offer_bytes).unwrap_or_default();
Copy link

Copilot AI Apr 24, 2025

Choose a reason for hiding this comment

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

Using unwrap_or_default when writing the payer offer may hide serialization errors. Consider handling the error explicitly to either propagate it or log a meaningful message.

Copilot uses AI. Check for mistakes.

Comment on lines 1566 to 1575
let name = String::from_utf8(bytes[1..1 + name_len].to_vec()).unwrap();
let domain_len = bytes[1 + name_len] as usize;
let domain = String::from_utf8(bytes[2 + name_len..2 + name_len + domain_len].to_vec()).unwrap();
Bip353Name { name, domain }
}),
payer_bip353_signature: payer_bip353_signature.map(|bytes| {
let pubkey = PublicKey::from_slice(&bytes[..33]).unwrap();
let signature = Signature::from_slice(&bytes[33..]).unwrap();
(pubkey, signature)
}),
Copy link

Copilot AI Apr 24, 2025

Choose a reason for hiding this comment

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

The deserialization of payer_bip353_name (and similarly payer_bip353_signature) uses unwrap(), which may cause a panic on malformed input. Consider adding proper error handling to safely manage conversion errors.

Suggested change
let name = String::from_utf8(bytes[1..1 + name_len].to_vec()).unwrap();
let domain_len = bytes[1 + name_len] as usize;
let domain = String::from_utf8(bytes[2 + name_len..2 + name_len + domain_len].to_vec()).unwrap();
Bip353Name { name, domain }
}),
payer_bip353_signature: payer_bip353_signature.map(|bytes| {
let pubkey = PublicKey::from_slice(&bytes[..33]).unwrap();
let signature = Signature::from_slice(&bytes[33..]).unwrap();
(pubkey, signature)
}),
let name = String::from_utf8(bytes[1..1 + name_len].to_vec())
.map_err(|_| Bolt12SemanticError::InvalidPayerBip353Name)?;
let domain_len = bytes[1 + name_len] as usize;
let domain = String::from_utf8(bytes[2 + name_len..2 + name_len + domain_len].to_vec())
.map_err(|_| Bolt12SemanticError::InvalidPayerBip353Name)?;
Ok(Bip353Name { name, domain })
}).transpose()?,
payer_bip353_signature: payer_bip353_signature.map(|bytes| {
let pubkey = PublicKey::from_slice(&bytes[..33])
.map_err(|_| Bolt12SemanticError::InvalidPayerBip353Signature)?;
let signature = Signature::from_slice(&bytes[33..])
.map_err(|_| Bolt12SemanticError::InvalidPayerBip353Signature)?;
Ok((pubkey, signature))
}).transpose()?,

Copilot uses AI. Check for mistakes.

Signed-off-by: Vincenzo Palazzo <[email protected]>
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.

1 participant