Skip to content

Don't over-allocate invoice bytes#3494

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.

Sign up for GitHub

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

Merged
TheBlueMatt merged 2 commits into from
Feb 7, 2025
Merged

Don't over-allocate invoice bytes #3494

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ffaccc0
Use fixed-size Vec allocations for BOLT12 messages
jkczyz Jan 28, 2025
0912b51
Drop use of reserve_exact in BOLT12 tests
jkczyz Feb 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 12 additions & 53 deletions lightning/src/offers/invoice.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -122,7 +122,7 @@ use crate::offers::invoice_macros::{invoice_accessors_common, invoice_builder_me
#[cfg(test)]
use crate::offers::invoice_macros::invoice_builder_methods_test_common;
use crate::offers::invoice_request::{EXPERIMENTAL_INVOICE_REQUEST_TYPES, ExperimentalInvoiceRequestTlvStream, ExperimentalInvoiceRequestTlvStreamRef, INVOICE_REQUEST_PAYER_ID_TYPE, INVOICE_REQUEST_TYPES, IV_BYTES as INVOICE_REQUEST_IV_BYTES, InvoiceRequest, InvoiceRequestContents, InvoiceRequestTlvStream, InvoiceRequestTlvStreamRef};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self, SIGNATURE_TLV_RECORD_SIZE};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self};
use crate::offers::nonce::Nonce;
use crate::offers::offer::{Amount, EXPERIMENTAL_OFFER_TYPES, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OFFER_TYPES, OfferTlvStream, OfferTlvStreamRef, Quantity};
use crate::offers::parse::{Bolt12ParseError, Bolt12SemanticError, ParsedMessage};
Expand DownExpand Up@@ -520,19 +520,8 @@ impl UnsignedBolt12Invoice {
let (_, _, _, invoice_tlv_stream, _, _, experimental_invoice_tlv_stream) =
contents.as_tlv_stream();

// Allocate enough space for the invoice, which will include:
// - all TLV records from `invreq_bytes` except signatures,
// - all invoice-specific TLV records, and
// - a signature TLV record once the invoice is signed.
//
// This assumes both the invoice request and the invoice will each only have one signature
// using SIGNATURE_TYPES.start as the TLV record. Thus, it is accounted for by invreq_bytes.
let mut bytes = Vec::with_capacity(
invreq_bytes.len()
+ invoice_tlv_stream.serialized_length()
+ if contents.is_for_offer() { 0 } else { SIGNATURE_TLV_RECORD_SIZE }
+ experimental_invoice_tlv_stream.serialized_length(),
);
const INVOICE_ALLOCATION_SIZE: usize = 1024;
let mut bytes = Vec::with_capacity(INVOICE_ALLOCATION_SIZE);

// Use the invoice_request bytes instead of the invoice_request TLV stream as the latter may
// have contained unknown TLV records, which are not stored in `InvoiceRequestContents` or
Expand All@@ -545,23 +534,16 @@ impl UnsignedBolt12Invoice {

invoice_tlv_stream.write(&mut bytes).unwrap();

let mut experimental_tlv_stream = TlvStream::new(remaining_bytes)
.range(EXPERIMENTAL_TYPES)
.peekable();
let mut experimental_bytes = Vec::with_capacity(
remaining_bytes.len()
- experimental_tlv_stream
.peek()
.map_or(remaining_bytes.len(), |first_record| first_record.start)
+ experimental_invoice_tlv_stream.serialized_length(),
);
const EXPERIMENTAL_TLV_ALLOCATION_SIZE: usize = 0;
let mut experimental_bytes = Vec::with_capacity(EXPERIMENTAL_TLV_ALLOCATION_SIZE);

@TheBlueMattTheBlueMattJan 30, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should we default to no allocation here instead? If we expect these to rarely be used that'd save us an allocation and if we only ever have one TLV then we always only do one allocation anyway, so it'd only cost us an allocation if we have multiple TLVs.

@jkczyzjkczyzJan 31, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure, we can do that. Though the number of allocations will likely be at least two since we write the type and length before writing the value. Depends on how large the value is and how much the initial Vec allocation size is. Rust playground shows 8 bytes.

@TheBlueMattTheBlueMattJan 31, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Are you sure about that? TlvStream iterates over TlvRecords which implement Writeable with a single call to write_all.

@jkczyzjkczyzJan 31, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Ah, yeah, for experimental TLVs from the offer/invreq (i.e., in experimental_tlv_stream), that is true. For those that we set in the invoice (i.e., anything in experimental_invoice_tlv_stream), it would not be the case though.

@TheBlueMattTheBlueMattJan 31, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Mmm, hmmm, right. We could check if any of the supported experimental fields are set and allocate based on that? I can imagine someone having paid 100k BOLT12s, loading them in memory, and us wasting 48MB plus overhead for them...Basically this would just be a comment in ExperimentalInvoiceTlvStream that we should allocate if we add fields since we don't support any fields outside of test anyway.

@jkczyzjkczyzJan 31, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Updated


let experimental_tlv_stream = TlvStream::new(remaining_bytes)
.range(EXPERIMENTAL_TYPES);
for record in experimental_tlv_stream {
record.write(&mut experimental_bytes).unwrap();
}

experimental_invoice_tlv_stream.write(&mut experimental_bytes).unwrap();
debug_assert_eq!(experimental_bytes.len(), experimental_bytes.capacity());

let tlv_stream = TlvStream::new(&bytes).chain(TlvStream::new(&experimental_bytes));
let tagged_hash = TaggedHash::from_tlv_stream(SIGNATURE_TAG, tlv_stream);
Expand DownExpand Up@@ -592,14 +574,6 @@ macro_rules! unsigned_invoice_sign_method { ($self: ident, $self_type: ty $(, $s
signature_tlv_stream.write(&mut $self.bytes).unwrap();

// Append the experimental bytes after the signature.
debug_assert_eq!(
// The two-byte overallocation results from SIGNATURE_TLV_RECORD_SIZE accommodating TLV
// records with types >= 253.
$self.bytes.len()
+ $self.experimental_bytes.len()
+ if $self.contents.is_for_offer() { 0 } else { 2 },
$self.bytes.capacity(),
);
$self.bytes.extend_from_slice(&$self.experimental_bytes);

Ok(Bolt12Invoice {
Expand DownExpand Up@@ -965,13 +939,6 @@ impl Hash for Bolt12Invoice {
}

impl InvoiceContents {
fn is_for_offer(&self) -> bool {
match self {
InvoiceContents::ForOffer { .. } => true,
InvoiceContents::ForRefund { .. } => false,
}
}

/// Whether the original offer or refund has expired.
#[cfg(feature = "std")]
fn is_offer_or_refund_expired(&self) -> bool {
Expand DownExpand Up@@ -1362,7 +1329,11 @@ pub(super) const EXPERIMENTAL_INVOICE_TYPES: core::ops::RangeFrom<u64> = 3_000_0

#[cfg(not(test))]
tlv_stream!(
ExperimentalInvoiceTlvStream, ExperimentalInvoiceTlvStreamRef, EXPERIMENTAL_INVOICE_TYPES, {}
ExperimentalInvoiceTlvStream, ExperimentalInvoiceTlvStreamRef, EXPERIMENTAL_INVOICE_TYPES, {
// When adding experimental TLVs, update EXPERIMENTAL_TLV_ALLOCATION_SIZE accordingly in
// both UnsignedBolt12Invoice:new and UnsignedStaticInvoice::new to avoid unnecessary
// allocations.
}
);

#[cfg(test)]
Expand DownExpand Up@@ -2880,9 +2851,6 @@ mod tests {
BigSize(32).write(&mut unknown_bytes).unwrap();
[42u8; 32].write(&mut unknown_bytes).unwrap();

unsigned_invoice.bytes.reserve_exact(
unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
);
unsigned_invoice.bytes.extend_from_slice(&unknown_bytes);
unsigned_invoice.tagged_hash =
TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice.bytes);
Expand DownExpand Up@@ -2917,9 +2885,6 @@ mod tests {
BigSize(32).write(&mut unknown_bytes).unwrap();
[42u8; 32].write(&mut unknown_bytes).unwrap();

unsigned_invoice.bytes.reserve_exact(
unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
);
unsigned_invoice.bytes.extend_from_slice(&unknown_bytes);
unsigned_invoice.tagged_hash =
TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice.bytes);
Expand DownExpand Up@@ -2982,9 +2947,6 @@ mod tests {
BigSize(32).write(&mut unknown_bytes).unwrap();
[42u8; 32].write(&mut unknown_bytes).unwrap();

unsigned_invoice.bytes.reserve_exact(
unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
);
unsigned_invoice.experimental_bytes.extend_from_slice(&unknown_bytes);

let tlv_stream = TlvStream::new(&unsigned_invoice.bytes)
Expand DownExpand Up@@ -3021,9 +2983,6 @@ mod tests {
BigSize(32).write(&mut unknown_bytes).unwrap();
[42u8; 32].write(&mut unknown_bytes).unwrap();

unsigned_invoice.bytes.reserve_exact(
unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
);
unsigned_invoice.experimental_bytes.extend_from_slice(&unknown_bytes);

let tlv_stream = TlvStream::new(&unsigned_invoice.bytes)
Expand Down
61 changes: 11 additions & 50 deletions lightning/src/offers/invoice_request.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -77,7 +77,7 @@ use crate::ln::channelmanager::PaymentId;
use crate::types::features::InvoiceRequestFeatures;
use crate::ln::inbound_payment::{ExpandedKey, IV_LEN};
use crate::ln::msgs::DecodeError;
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self, SIGNATURE_TLV_RECORD_SIZE};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self};
use crate::offers::nonce::Nonce;
use crate::offers::offer::{Amount, EXPERIMENTAL_OFFER_TYPES, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OFFER_TYPES, Offer, OfferContents, OfferId, OfferTlvStream, OfferTlvStreamRef};
use crate::offers::parse::{Bolt12ParseError, ParsedMessage, Bolt12SemanticError};
Expand DownExpand Up@@ -473,17 +473,8 @@ impl UnsignedInvoiceRequest {
_experimental_offer_tlv_stream, experimental_invoice_request_tlv_stream,
) = contents.as_tlv_stream();

// Allocate enough space for the invoice_request, which will include:
// - all TLV records from `offer.bytes`,
// - all invoice_request-specific TLV records, and
// - a signature TLV record once the invoice_request is signed.
let mut bytes = Vec::with_capacity(
offer.bytes.len()
+ payer_tlv_stream.serialized_length()
+ invoice_request_tlv_stream.serialized_length()
+ SIGNATURE_TLV_RECORD_SIZE
+ experimental_invoice_request_tlv_stream.serialized_length(),
);
const INVOICE_REQUEST_ALLOCATION_SIZE: usize = 512;
let mut bytes = Vec::with_capacity(INVOICE_REQUEST_ALLOCATION_SIZE);

payer_tlv_stream.write(&mut bytes).unwrap();

Expand All@@ -495,23 +486,16 @@ impl UnsignedInvoiceRequest {

invoice_request_tlv_stream.write(&mut bytes).unwrap();

let mut experimental_tlv_stream = TlvStream::new(remaining_bytes)
.range(EXPERIMENTAL_OFFER_TYPES)
.peekable();
let mut experimental_bytes = Vec::with_capacity(
remaining_bytes.len()
- experimental_tlv_stream
.peek()
.map_or(remaining_bytes.len(), |first_record| first_record.start)
+ experimental_invoice_request_tlv_stream.serialized_length(),
);
const EXPERIMENTAL_TLV_ALLOCATION_SIZE: usize = 0;
let mut experimental_bytes = Vec::with_capacity(EXPERIMENTAL_TLV_ALLOCATION_SIZE);

let experimental_tlv_stream = TlvStream::new(remaining_bytes)
.range(EXPERIMENTAL_OFFER_TYPES);
for record in experimental_tlv_stream {
record.write(&mut experimental_bytes).unwrap();
}

experimental_invoice_request_tlv_stream.write(&mut experimental_bytes).unwrap();
debug_assert_eq!(experimental_bytes.len(), experimental_bytes.capacity());

let tlv_stream = TlvStream::new(&bytes).chain(TlvStream::new(&experimental_bytes));
let tagged_hash = TaggedHash::from_tlv_stream(SIGNATURE_TAG, tlv_stream);
Expand DownExpand Up@@ -544,12 +528,6 @@ macro_rules! unsigned_invoice_request_sign_method { (
signature_tlv_stream.write(&mut $self.bytes).unwrap();

// Append the experimental bytes after the signature.
debug_assert_eq!(
// The two-byte overallocation results from SIGNATURE_TLV_RECORD_SIZE accommodating TLV
// records with types >= 253.
$self.bytes.len() + $self.experimental_bytes.len() + 2,
$self.bytes.capacity(),
);
$self.bytes.extend_from_slice(&$self.experimental_bytes);

Ok(InvoiceRequest {
Expand DownExpand Up@@ -1127,7 +1105,10 @@ pub(super) const EXPERIMENTAL_INVOICE_REQUEST_TYPES: core::ops::Range<u64> =
#[cfg(not(test))]
tlv_stream!(
ExperimentalInvoiceRequestTlvStream, ExperimentalInvoiceRequestTlvStreamRef,
EXPERIMENTAL_INVOICE_REQUEST_TYPES, {}
EXPERIMENTAL_INVOICE_REQUEST_TYPES, {
// When adding experimental TLVs, update EXPERIMENTAL_TLV_ALLOCATION_SIZE accordingly in
// UnsignedInvoiceRequest::new to avoid unnecessary allocations.
}
);

#[cfg(test)]
Expand DownExpand Up@@ -2422,11 +2403,6 @@ mod tests {
BigSize(32).write(&mut unknown_bytes).unwrap();
[42u8; 32].write(&mut unknown_bytes).unwrap();

unsigned_invoice_request.bytes.reserve_exact(
unsigned_invoice_request.bytes.capacity()
- unsigned_invoice_request.bytes.len()
+ unknown_bytes.len(),
);
unsigned_invoice_request.bytes.extend_from_slice(&unknown_bytes);
unsigned_invoice_request.tagged_hash =
TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice_request.bytes);
Expand DownExpand Up@@ -2460,11 +2436,6 @@ mod tests {
BigSize(32).write(&mut unknown_bytes).unwrap();
[42u8; 32].write(&mut unknown_bytes).unwrap();

unsigned_invoice_request.bytes.reserve_exact(
unsigned_invoice_request.bytes.capacity()
- unsigned_invoice_request.bytes.len()
+ unknown_bytes.len(),
);
unsigned_invoice_request.bytes.extend_from_slice(&unknown_bytes);
unsigned_invoice_request.tagged_hash =
TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice_request.bytes);
Expand DownExpand Up@@ -2508,11 +2479,6 @@ mod tests {
BigSize(32).write(&mut unknown_bytes).unwrap();
[42u8; 32].write(&mut unknown_bytes).unwrap();

unsigned_invoice_request.bytes.reserve_exact(
unsigned_invoice_request.bytes.capacity()
- unsigned_invoice_request.bytes.len()
+ unknown_bytes.len(),
);
unsigned_invoice_request.experimental_bytes.extend_from_slice(&unknown_bytes);

let tlv_stream = TlvStream::new(&unsigned_invoice_request.bytes)
Expand DownExpand Up@@ -2549,11 +2515,6 @@ mod tests {
BigSize(32).write(&mut unknown_bytes).unwrap();
[42u8; 32].write(&mut unknown_bytes).unwrap();

unsigned_invoice_request.bytes.reserve_exact(
unsigned_invoice_request.bytes.capacity()
- unsigned_invoice_request.bytes.len()
+ unknown_bytes.len(),
);
unsigned_invoice_request.experimental_bytes.extend_from_slice(&unknown_bytes);

let tlv_stream = TlvStream::new(&unsigned_invoice_request.bytes)
Expand Down
8 changes: 1 addition & 7 deletions lightning/src/offers/merkle.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,7 +11,6 @@

use bitcoin::hashes::{Hash, HashEngine, sha256};
use bitcoin::secp256k1::{Message, PublicKey, Secp256k1, self};
use bitcoin::secp256k1::constants::SCHNORR_SIGNATURE_SIZE;
use bitcoin::secp256k1::schnorr::Signature;
use crate::io;
use crate::util::ser::{BigSize, Readable, Writeable, Writer};
Expand All@@ -26,10 +25,6 @@ tlv_stream!(SignatureTlvStream, SignatureTlvStreamRef<'a>, SIGNATURE_TYPES, {
(240, signature: Signature),
});

/// Size of a TLV record in `SIGNATURE_TYPES` when the type is 1000. TLV types are encoded using
/// BigSize, so a TLV record with type 240 will use two less bytes.
pub(super) const SIGNATURE_TLV_RECORD_SIZE: usize = 3 + 1 + SCHNORR_SIGNATURE_SIZE;

/// A hash for use in a specific context by tweaking with a context-dependent tag as per [BIP 340]
/// and computed over the merkle root of a TLV stream to sign as defined in [BOLT 12].
///
Expand DownExpand Up@@ -253,7 +248,6 @@ pub(super) struct TlvRecord<'a> {
type_bytes: &'a [u8],
// The entire TLV record.
pub(super) record_bytes: &'a [u8],
pub(super) start: usize,
pub(super) end: usize,
}

Expand All@@ -278,7 +272,7 @@ impl<'a> Iterator for TlvStream<'a> {
self.data.set_position(end);

Some(TlvRecord {
r#type, type_bytes, record_bytes, start: start as usize, end: end as usize,
r#type, type_bytes, record_bytes, end: end as usize,
})
} else {
None
Expand Down
3 changes: 2 additions & 1 deletion lightning/src/offers/offer.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -438,7 +438,8 @@ macro_rules! offer_builder_methods { (
}
}

let mut bytes = Vec::new();
const OFFER_ALLOCATION_SIZE: usize = 512;
let mut bytes = Vec::with_capacity(OFFER_ALLOCATION_SIZE);
$self.offer.write(&mut bytes).unwrap();

let id = OfferId::from_valid_offer_tlv_stream(&bytes);
Expand Down
3 changes: 2 additions & 1 deletion lightning/src/offers/refund.rs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -338,7 +338,8 @@ macro_rules! refund_builder_methods { (
$self.refund.payer.0 = metadata;
}

let mut bytes = Vec::new();
const REFUND_ALLOCATION_SIZE: usize = 512;
let mut bytes = Vec::with_capacity(REFUND_ALLOCATION_SIZE);
$self.refund.write(&mut bytes).unwrap();

Ok(Refund {
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all 
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Don't over-allocate invoice bytes by jkczyz · Pull Request #3494 · lightningdevkit/rust-lightning · GitHub
Skip to content
Don't over-allocate invoice bytes#3494

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.

 Sign up for GitHub

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

 Merged

TheBlueMatt
merged 2 commits into

from

Feb 7, 2025
 Merged


Don't over-allocate invoice bytes
#3494
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
ffaccc0

Use fixed-size Vec allocations for BOLT12 messages


jkczyz Jan 28, 2025
0912b51

Drop use of reserve_exact in BOLT12 tests


jkczyz Feb 6, 2025

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


65 changes: 12 additions & 53 deletions
lightning/src/offers/invoice.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -122,7 +122,7 @@ use crate::offers::invoice_macros::{invoice_accessors_common, invoice_builder_me
#[cfg(test)]
use crate::offers::invoice_macros::invoice_builder_methods_test_common;
use crate::offers::invoice_request::{EXPERIMENTAL_INVOICE_REQUEST_TYPES, ExperimentalInvoiceRequestTlvStream, ExperimentalInvoiceRequestTlvStreamRef, INVOICE_REQUEST_PAYER_ID_TYPE, INVOICE_REQUEST_TYPES, IV_BYTES as INVOICE_REQUEST_IV_BYTES, InvoiceRequest, InvoiceRequestContents, InvoiceRequestTlvStream, InvoiceRequestTlvStreamRef};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self, SIGNATURE_TLV_RECORD_SIZE};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self};
use crate::offers::nonce::Nonce;
use crate::offers::offer::{Amount, EXPERIMENTAL_OFFER_TYPES, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OFFER_TYPES, OfferTlvStream, OfferTlvStreamRef, Quantity};
use crate::offers::parse::{Bolt12ParseError, Bolt12SemanticError, ParsedMessage};
Expand DownExpand Up@@ -520,19 +520,8 @@ impl UnsignedBolt12Invoice {
 let (_, _, _, invoice_tlv_stream, _, _, experimental_invoice_tlv_stream) =
 contents.as_tlv_stream();

 // Allocate enough space for the invoice, which will include:
 // - all TLV records from `invreq_bytes` except signatures,
 // - all invoice-specific TLV records, and
 // - a signature TLV record once the invoice is signed.
 //
 // This assumes both the invoice request and the invoice will each only have one signature
 // using SIGNATURE_TYPES.start as the TLV record. Thus, it is accounted for by invreq_bytes.
 let mut bytes = Vec::with_capacity(
 invreq_bytes.len()
 + invoice_tlv_stream.serialized_length()
 + if contents.is_for_offer() { 0 } else { SIGNATURE_TLV_RECORD_SIZE }
 + experimental_invoice_tlv_stream.serialized_length(),
 );
 const INVOICE_ALLOCATION_SIZE: usize = 1024;
 let mut bytes = Vec::with_capacity(INVOICE_ALLOCATION_SIZE);

 // Use the invoice_request bytes instead of the invoice_request TLV stream as the latter may
 // have contained unknown TLV records, which are not stored in `InvoiceRequestContents` or
Expand All@@ -545,23 +534,16 @@ impl UnsignedBolt12Invoice {

 invoice_tlv_stream.write(&mut bytes).unwrap();

 let mut experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_TYPES)
 .peekable();
 let mut experimental_bytes = Vec::with_capacity(
 remaining_bytes.len()
 - experimental_tlv_stream
 .peek()
 .map_or(remaining_bytes.len(), |first_record| first_record.start)
 + experimental_invoice_tlv_stream.serialized_length(),
 );
 const EXPERIMENTAL_TLV_ALLOCATION_SIZE: usize = 0;
 let mut experimental_bytes = Vec::with_capacity(EXPERIMENTAL_TLV_ALLOCATION_SIZE);
@TheBlueMattTheBlueMattJan 30, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Should we default to no allocation here instead? If we expect these to rarely be used that'd save us an allocation and if we only ever have one TLV then we always only do one allocation anyway, so it'd only cost us an allocation if we have multiple TLVs.
@jkczyzjkczyzJan 31, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Sure, we can do that. Though the number of allocations will likely be at least two since we write the type and length before writing the value. Depends on how large the value is and how much the initial Vec allocation size is. Rust playground shows 8 bytes.
@TheBlueMattTheBlueMattJan 31, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Are you sure about that? TlvStream iterates over TlvRecords which implement Writeable with a single call to write_all.
@jkczyzjkczyzJan 31, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Ah, yeah, for experimental TLVs from the offer/invreq (i.e., in experimental_tlv_stream), that is true. For those that we set in the invoice (i.e., anything in experimental_invoice_tlv_stream), it would not be the case though.
@TheBlueMattTheBlueMattJan 31, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Mmm, hmmm, right. We could check if any of the supported experimental fields are set and allocate based on that? I can imagine someone having paid 100k BOLT12s, loading them in memory, and us wasting 48MB plus overhead for them...Basically this would just be a comment in ExperimentalInvoiceTlvStream that we should allocate if we add fields since we don't support any fields outside of test anyway.
@jkczyzjkczyzJan 31, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Updated

 let experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_TYPES);
 for record in experimental_tlv_stream {
 record.write(&mut experimental_bytes).unwrap();
 }

 experimental_invoice_tlv_stream.write(&mut experimental_bytes).unwrap();
 debug_assert_eq!(experimental_bytes.len(), experimental_bytes.capacity());

 let tlv_stream = TlvStream::new(&bytes).chain(TlvStream::new(&experimental_bytes));
 let tagged_hash = TaggedHash::from_tlv_stream(SIGNATURE_TAG, tlv_stream);
Expand DownExpand Up@@ -592,14 +574,6 @@ macro_rules! unsigned_invoice_sign_method { ($self: ident, $self_type: ty $(, $s
 signature_tlv_stream.write(&mut $self.bytes).unwrap();

 // Append the experimental bytes after the signature.
 debug_assert_eq!(
 // The two-byte overallocation results from SIGNATURE_TLV_RECORD_SIZE accommodating TLV
 // records with types >= 253.
 $self.bytes.len()
 + $self.experimental_bytes.len()
 + if $self.contents.is_for_offer() { 0 } else { 2 },
 $self.bytes.capacity(),
 );
 $self.bytes.extend_from_slice(&$self.experimental_bytes);

 Ok(Bolt12Invoice {
Expand DownExpand Up@@ -965,13 +939,6 @@ impl Hash for Bolt12Invoice {
}

impl InvoiceContents {
	fn is_for_offer(&self) -> bool {
 match self {
 InvoiceContents::ForOffer { .. } => true,
 InvoiceContents::ForRefund { .. } => false,
 }
	}

	/// Whether the original offer or refund has expired.
	#[cfg(feature = "std")]
	fn is_offer_or_refund_expired(&self) -> bool {
Expand DownExpand Up@@ -1362,7 +1329,11 @@ pub(super) const EXPERIMENTAL_INVOICE_TYPES: core::ops::RangeFrom<u64> = 3_000_0

#[cfg(not(test))]
tlv_stream!(
	ExperimentalInvoiceTlvStream, ExperimentalInvoiceTlvStreamRef, EXPERIMENTAL_INVOICE_TYPES, {}
	ExperimentalInvoiceTlvStream, ExperimentalInvoiceTlvStreamRef, EXPERIMENTAL_INVOICE_TYPES, {
 // When adding experimental TLVs, update EXPERIMENTAL_TLV_ALLOCATION_SIZE accordingly in
 // both UnsignedBolt12Invoice:new and UnsignedStaticInvoice::new to avoid unnecessary
 // allocations.
	}
);

#[cfg(test)]
Expand DownExpand Up@@ -2880,9 +2851,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice.bytes);
Expand DownExpand Up@@ -2917,9 +2885,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice.bytes);
Expand DownExpand Up@@ -2982,9 +2947,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice.bytes)
Expand DownExpand Up@@ -3021,9 +2983,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice.bytes)
Expand Down

61 changes: 11 additions & 50 deletions
lightning/src/offers/invoice_request.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -77,7 +77,7 @@ use crate::ln::channelmanager::PaymentId;
use crate::types::features::InvoiceRequestFeatures;
use crate::ln::inbound_payment::{ExpandedKey, IV_LEN};
use crate::ln::msgs::DecodeError;
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self, SIGNATURE_TLV_RECORD_SIZE};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self};
use crate::offers::nonce::Nonce;
use crate::offers::offer::{Amount, EXPERIMENTAL_OFFER_TYPES, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OFFER_TYPES, Offer, OfferContents, OfferId, OfferTlvStream, OfferTlvStreamRef};
use crate::offers::parse::{Bolt12ParseError, ParsedMessage, Bolt12SemanticError};
Expand DownExpand Up@@ -473,17 +473,8 @@ impl UnsignedInvoiceRequest {
 _experimental_offer_tlv_stream, experimental_invoice_request_tlv_stream,
 ) = contents.as_tlv_stream();

 // Allocate enough space for the invoice_request, which will include:
 // - all TLV records from `offer.bytes`,
 // - all invoice_request-specific TLV records, and
 // - a signature TLV record once the invoice_request is signed.
 let mut bytes = Vec::with_capacity(
 offer.bytes.len()
 + payer_tlv_stream.serialized_length()
 + invoice_request_tlv_stream.serialized_length()
 + SIGNATURE_TLV_RECORD_SIZE
 + experimental_invoice_request_tlv_stream.serialized_length(),
 );
 const INVOICE_REQUEST_ALLOCATION_SIZE: usize = 512;
 let mut bytes = Vec::with_capacity(INVOICE_REQUEST_ALLOCATION_SIZE);

 payer_tlv_stream.write(&mut bytes).unwrap();

Expand All@@ -495,23 +486,16 @@ impl UnsignedInvoiceRequest {

 invoice_request_tlv_stream.write(&mut bytes).unwrap();

 let mut experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_OFFER_TYPES)
 .peekable();
 let mut experimental_bytes = Vec::with_capacity(
 remaining_bytes.len()
 - experimental_tlv_stream
 .peek()
 .map_or(remaining_bytes.len(), |first_record| first_record.start)
 + experimental_invoice_request_tlv_stream.serialized_length(),
 );
 const EXPERIMENTAL_TLV_ALLOCATION_SIZE: usize = 0;
 let mut experimental_bytes = Vec::with_capacity(EXPERIMENTAL_TLV_ALLOCATION_SIZE);

 let experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_OFFER_TYPES);
 for record in experimental_tlv_stream {
 record.write(&mut experimental_bytes).unwrap();
 }

 experimental_invoice_request_tlv_stream.write(&mut experimental_bytes).unwrap();
 debug_assert_eq!(experimental_bytes.len(), experimental_bytes.capacity());

 let tlv_stream = TlvStream::new(&bytes).chain(TlvStream::new(&experimental_bytes));
 let tagged_hash = TaggedHash::from_tlv_stream(SIGNATURE_TAG, tlv_stream);
Expand DownExpand Up@@ -544,12 +528,6 @@ macro_rules! unsigned_invoice_request_sign_method { (
 signature_tlv_stream.write(&mut $self.bytes).unwrap();

 // Append the experimental bytes after the signature.
 debug_assert_eq!(
 // The two-byte overallocation results from SIGNATURE_TLV_RECORD_SIZE accommodating TLV
 // records with types >= 253.
 $self.bytes.len() + $self.experimental_bytes.len() + 2,
 $self.bytes.capacity(),
 );
 $self.bytes.extend_from_slice(&$self.experimental_bytes);

 Ok(InvoiceRequest {
Expand DownExpand Up@@ -1127,7 +1105,10 @@ pub(super) const EXPERIMENTAL_INVOICE_REQUEST_TYPES: core::ops::Range<u64> =
#[cfg(not(test))]
tlv_stream!(
	ExperimentalInvoiceRequestTlvStream, ExperimentalInvoiceRequestTlvStreamRef,
	EXPERIMENTAL_INVOICE_REQUEST_TYPES, {}
	EXPERIMENTAL_INVOICE_REQUEST_TYPES, {
 // When adding experimental TLVs, update EXPERIMENTAL_TLV_ALLOCATION_SIZE accordingly in
 // UnsignedInvoiceRequest::new to avoid unnecessary allocations.
	}
);

#[cfg(test)]
Expand DownExpand Up@@ -2422,11 +2403,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice_request.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice_request.bytes);
Expand DownExpand Up@@ -2460,11 +2436,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice_request.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice_request.bytes);
Expand DownExpand Up@@ -2508,11 +2479,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice_request.bytes)
Expand DownExpand Up@@ -2549,11 +2515,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice_request.bytes)
Expand Down

8 changes: 1 addition & 7 deletions
lightning/src/offers/merkle.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -11,7 +11,6 @@

use bitcoin::hashes::{Hash, HashEngine, sha256};
use bitcoin::secp256k1::{Message, PublicKey, Secp256k1, self};
use bitcoin::secp256k1::constants::SCHNORR_SIGNATURE_SIZE;
use bitcoin::secp256k1::schnorr::Signature;
use crate::io;
use crate::util::ser::{BigSize, Readable, Writeable, Writer};
Expand All@@ -26,10 +25,6 @@ tlv_stream!(SignatureTlvStream, SignatureTlvStreamRef<'a>, SIGNATURE_TYPES, {
	(240, signature: Signature),
});

/// Size of a TLV record in `SIGNATURE_TYPES` when the type is 1000. TLV types are encoded using
/// BigSize, so a TLV record with type 240 will use two less bytes.
pub(super) const SIGNATURE_TLV_RECORD_SIZE: usize = 3 + 1 + SCHNORR_SIGNATURE_SIZE;

/// A hash for use in a specific context by tweaking with a context-dependent tag as per [BIP 340]
/// and computed over the merkle root of a TLV stream to sign as defined in [BOLT 12].
///
Expand DownExpand Up@@ -253,7 +248,6 @@ pub(super) struct TlvRecord<'a> {
	type_bytes: &'a [u8],
	// The entire TLV record.
	pub(super) record_bytes: &'a [u8],
	pub(super) start: usize,
	pub(super) end: usize,
}

Expand All@@ -278,7 +272,7 @@ impl<'a> Iterator for TlvStream<'a> {
 self.data.set_position(end);

 Some(TlvRecord {
 r#type, type_bytes, record_bytes, start: start as usize, end: end as usize,
 r#type, type_bytes, record_bytes, end: end as usize,
 })
 } else {
 None
Expand Down

3 changes: 2 additions & 1 deletion
lightning/src/offers/offer.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -438,7 +438,8 @@ macro_rules! offer_builder_methods { (
 }
 }

 let mut bytes = Vec::new();
 const OFFER_ALLOCATION_SIZE: usize = 512;
 let mut bytes = Vec::with_capacity(OFFER_ALLOCATION_SIZE);
 $self.offer.write(&mut bytes).unwrap();

 let id = OfferId::from_valid_offer_tlv_stream(&bytes);
Expand Down

3 changes: 2 additions & 1 deletion
lightning/src/offers/refund.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -338,7 +338,8 @@ macro_rules! refund_builder_methods { (
 $self.refund.payer.0 = metadata;
 }

 let mut bytes = Vec::new();
 const REFUND_ALLOCATION_SIZE: usize = 512;
 let mut bytes = Vec::with_capacity(REFUND_ALLOCATION_SIZE);
 $self.refund.write(&mut bytes).unwrap();

 Ok(Refund {
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Force GitHub README to respect dark mode
(function() {
var style = document.createElement('style');
style.textContent = '
.markdown-body {
color-scheme: dark light;
}
.markdown-body pre { background: #161b22 !important; }
.markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }
.markdown-body table th, .markdown-body table td { border-color: #30363d !important; }
.markdown-body img { background: #0d1117; }
.markdown-body blockquote { border-left-color: #8b949e; }
.markdown-body hr { border-color: #30363d; }
';
document.head.appendChild(style);
})();
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Don't over-allocate invoice bytes by jkczyz · Pull Request #3494 · lightningdevkit/rust-lightning · GitHub
Skip to content
Don't over-allocate invoice bytes#3494

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.

 Sign up for GitHub

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

 Merged

TheBlueMatt
merged 2 commits into

from

Feb 7, 2025
 Merged


Don't over-allocate invoice bytes
#3494
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
ffaccc0

Use fixed-size Vec allocations for BOLT12 messages


jkczyz Jan 28, 2025
0912b51

Drop use of reserve_exact in BOLT12 tests


jkczyz Feb 6, 2025

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


65 changes: 12 additions & 53 deletions
lightning/src/offers/invoice.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -122,7 +122,7 @@ use crate::offers::invoice_macros::{invoice_accessors_common, invoice_builder_me
#[cfg(test)]
use crate::offers::invoice_macros::invoice_builder_methods_test_common;
use crate::offers::invoice_request::{EXPERIMENTAL_INVOICE_REQUEST_TYPES, ExperimentalInvoiceRequestTlvStream, ExperimentalInvoiceRequestTlvStreamRef, INVOICE_REQUEST_PAYER_ID_TYPE, INVOICE_REQUEST_TYPES, IV_BYTES as INVOICE_REQUEST_IV_BYTES, InvoiceRequest, InvoiceRequestContents, InvoiceRequestTlvStream, InvoiceRequestTlvStreamRef};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self, SIGNATURE_TLV_RECORD_SIZE};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self};
use crate::offers::nonce::Nonce;
use crate::offers::offer::{Amount, EXPERIMENTAL_OFFER_TYPES, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OFFER_TYPES, OfferTlvStream, OfferTlvStreamRef, Quantity};
use crate::offers::parse::{Bolt12ParseError, Bolt12SemanticError, ParsedMessage};
Expand DownExpand Up@@ -520,19 +520,8 @@ impl UnsignedBolt12Invoice {
 let (_, _, _, invoice_tlv_stream, _, _, experimental_invoice_tlv_stream) =
 contents.as_tlv_stream();

 // Allocate enough space for the invoice, which will include:
 // - all TLV records from `invreq_bytes` except signatures,
 // - all invoice-specific TLV records, and
 // - a signature TLV record once the invoice is signed.
 //
 // This assumes both the invoice request and the invoice will each only have one signature
 // using SIGNATURE_TYPES.start as the TLV record. Thus, it is accounted for by invreq_bytes.
 let mut bytes = Vec::with_capacity(
 invreq_bytes.len()
 + invoice_tlv_stream.serialized_length()
 + if contents.is_for_offer() { 0 } else { SIGNATURE_TLV_RECORD_SIZE }
 + experimental_invoice_tlv_stream.serialized_length(),
 );
 const INVOICE_ALLOCATION_SIZE: usize = 1024;
 let mut bytes = Vec::with_capacity(INVOICE_ALLOCATION_SIZE);

 // Use the invoice_request bytes instead of the invoice_request TLV stream as the latter may
 // have contained unknown TLV records, which are not stored in `InvoiceRequestContents` or
Expand All@@ -545,23 +534,16 @@ impl UnsignedBolt12Invoice {

 invoice_tlv_stream.write(&mut bytes).unwrap();

 let mut experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_TYPES)
 .peekable();
 let mut experimental_bytes = Vec::with_capacity(
 remaining_bytes.len()
 - experimental_tlv_stream
 .peek()
 .map_or(remaining_bytes.len(), |first_record| first_record.start)
 + experimental_invoice_tlv_stream.serialized_length(),
 );
 const EXPERIMENTAL_TLV_ALLOCATION_SIZE: usize = 0;
 let mut experimental_bytes = Vec::with_capacity(EXPERIMENTAL_TLV_ALLOCATION_SIZE);
@TheBlueMattTheBlueMattJan 30, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Should we default to no allocation here instead? If we expect these to rarely be used that'd save us an allocation and if we only ever have one TLV then we always only do one allocation anyway, so it'd only cost us an allocation if we have multiple TLVs.
@jkczyzjkczyzJan 31, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Sure, we can do that. Though the number of allocations will likely be at least two since we write the type and length before writing the value. Depends on how large the value is and how much the initial Vec allocation size is. Rust playground shows 8 bytes.
@TheBlueMattTheBlueMattJan 31, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Are you sure about that? TlvStream iterates over TlvRecords which implement Writeable with a single call to write_all.
@jkczyzjkczyzJan 31, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Ah, yeah, for experimental TLVs from the offer/invreq (i.e., in experimental_tlv_stream), that is true. For those that we set in the invoice (i.e., anything in experimental_invoice_tlv_stream), it would not be the case though.
@TheBlueMattTheBlueMattJan 31, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Mmm, hmmm, right. We could check if any of the supported experimental fields are set and allocate based on that? I can imagine someone having paid 100k BOLT12s, loading them in memory, and us wasting 48MB plus overhead for them...Basically this would just be a comment in ExperimentalInvoiceTlvStream that we should allocate if we add fields since we don't support any fields outside of test anyway.
@jkczyzjkczyzJan 31, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Updated

 let experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_TYPES);
 for record in experimental_tlv_stream {
 record.write(&mut experimental_bytes).unwrap();
 }

 experimental_invoice_tlv_stream.write(&mut experimental_bytes).unwrap();
 debug_assert_eq!(experimental_bytes.len(), experimental_bytes.capacity());

 let tlv_stream = TlvStream::new(&bytes).chain(TlvStream::new(&experimental_bytes));
 let tagged_hash = TaggedHash::from_tlv_stream(SIGNATURE_TAG, tlv_stream);
Expand DownExpand Up@@ -592,14 +574,6 @@ macro_rules! unsigned_invoice_sign_method { ($self: ident, $self_type: ty $(, $s
 signature_tlv_stream.write(&mut $self.bytes).unwrap();

 // Append the experimental bytes after the signature.
 debug_assert_eq!(
 // The two-byte overallocation results from SIGNATURE_TLV_RECORD_SIZE accommodating TLV
 // records with types >= 253.
 $self.bytes.len()
 + $self.experimental_bytes.len()
 + if $self.contents.is_for_offer() { 0 } else { 2 },
 $self.bytes.capacity(),
 );
 $self.bytes.extend_from_slice(&$self.experimental_bytes);

 Ok(Bolt12Invoice {
Expand DownExpand Up@@ -965,13 +939,6 @@ impl Hash for Bolt12Invoice {
}

impl InvoiceContents {
	fn is_for_offer(&self) -> bool {
 match self {
 InvoiceContents::ForOffer { .. } => true,
 InvoiceContents::ForRefund { .. } => false,
 }
	}

	/// Whether the original offer or refund has expired.
	#[cfg(feature = "std")]
	fn is_offer_or_refund_expired(&self) -> bool {
Expand DownExpand Up@@ -1362,7 +1329,11 @@ pub(super) const EXPERIMENTAL_INVOICE_TYPES: core::ops::RangeFrom<u64> = 3_000_0

#[cfg(not(test))]
tlv_stream!(
	ExperimentalInvoiceTlvStream, ExperimentalInvoiceTlvStreamRef, EXPERIMENTAL_INVOICE_TYPES, {}
	ExperimentalInvoiceTlvStream, ExperimentalInvoiceTlvStreamRef, EXPERIMENTAL_INVOICE_TYPES, {
 // When adding experimental TLVs, update EXPERIMENTAL_TLV_ALLOCATION_SIZE accordingly in
 // both UnsignedBolt12Invoice:new and UnsignedStaticInvoice::new to avoid unnecessary
 // allocations.
	}
);

#[cfg(test)]
Expand DownExpand Up@@ -2880,9 +2851,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice.bytes);
Expand DownExpand Up@@ -2917,9 +2885,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice.bytes);
Expand DownExpand Up@@ -2982,9 +2947,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice.bytes)
Expand DownExpand Up@@ -3021,9 +2983,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice.bytes)
Expand Down

61 changes: 11 additions & 50 deletions
lightning/src/offers/invoice_request.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -77,7 +77,7 @@ use crate::ln::channelmanager::PaymentId;
use crate::types::features::InvoiceRequestFeatures;
use crate::ln::inbound_payment::{ExpandedKey, IV_LEN};
use crate::ln::msgs::DecodeError;
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self, SIGNATURE_TLV_RECORD_SIZE};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self};
use crate::offers::nonce::Nonce;
use crate::offers::offer::{Amount, EXPERIMENTAL_OFFER_TYPES, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OFFER_TYPES, Offer, OfferContents, OfferId, OfferTlvStream, OfferTlvStreamRef};
use crate::offers::parse::{Bolt12ParseError, ParsedMessage, Bolt12SemanticError};
Expand DownExpand Up@@ -473,17 +473,8 @@ impl UnsignedInvoiceRequest {
 _experimental_offer_tlv_stream, experimental_invoice_request_tlv_stream,
 ) = contents.as_tlv_stream();

 // Allocate enough space for the invoice_request, which will include:
 // - all TLV records from `offer.bytes`,
 // - all invoice_request-specific TLV records, and
 // - a signature TLV record once the invoice_request is signed.
 let mut bytes = Vec::with_capacity(
 offer.bytes.len()
 + payer_tlv_stream.serialized_length()
 + invoice_request_tlv_stream.serialized_length()
 + SIGNATURE_TLV_RECORD_SIZE
 + experimental_invoice_request_tlv_stream.serialized_length(),
 );
 const INVOICE_REQUEST_ALLOCATION_SIZE: usize = 512;
 let mut bytes = Vec::with_capacity(INVOICE_REQUEST_ALLOCATION_SIZE);

 payer_tlv_stream.write(&mut bytes).unwrap();

Expand All@@ -495,23 +486,16 @@ impl UnsignedInvoiceRequest {

 invoice_request_tlv_stream.write(&mut bytes).unwrap();

 let mut experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_OFFER_TYPES)
 .peekable();
 let mut experimental_bytes = Vec::with_capacity(
 remaining_bytes.len()
 - experimental_tlv_stream
 .peek()
 .map_or(remaining_bytes.len(), |first_record| first_record.start)
 + experimental_invoice_request_tlv_stream.serialized_length(),
 );
 const EXPERIMENTAL_TLV_ALLOCATION_SIZE: usize = 0;
 let mut experimental_bytes = Vec::with_capacity(EXPERIMENTAL_TLV_ALLOCATION_SIZE);

 let experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_OFFER_TYPES);
 for record in experimental_tlv_stream {
 record.write(&mut experimental_bytes).unwrap();
 }

 experimental_invoice_request_tlv_stream.write(&mut experimental_bytes).unwrap();
 debug_assert_eq!(experimental_bytes.len(), experimental_bytes.capacity());

 let tlv_stream = TlvStream::new(&bytes).chain(TlvStream::new(&experimental_bytes));
 let tagged_hash = TaggedHash::from_tlv_stream(SIGNATURE_TAG, tlv_stream);
Expand DownExpand Up@@ -544,12 +528,6 @@ macro_rules! unsigned_invoice_request_sign_method { (
 signature_tlv_stream.write(&mut $self.bytes).unwrap();

 // Append the experimental bytes after the signature.
 debug_assert_eq!(
 // The two-byte overallocation results from SIGNATURE_TLV_RECORD_SIZE accommodating TLV
 // records with types >= 253.
 $self.bytes.len() + $self.experimental_bytes.len() + 2,
 $self.bytes.capacity(),
 );
 $self.bytes.extend_from_slice(&$self.experimental_bytes);

 Ok(InvoiceRequest {
Expand DownExpand Up@@ -1127,7 +1105,10 @@ pub(super) const EXPERIMENTAL_INVOICE_REQUEST_TYPES: core::ops::Range<u64> =
#[cfg(not(test))]
tlv_stream!(
	ExperimentalInvoiceRequestTlvStream, ExperimentalInvoiceRequestTlvStreamRef,
	EXPERIMENTAL_INVOICE_REQUEST_TYPES, {}
	EXPERIMENTAL_INVOICE_REQUEST_TYPES, {
 // When adding experimental TLVs, update EXPERIMENTAL_TLV_ALLOCATION_SIZE accordingly in
 // UnsignedInvoiceRequest::new to avoid unnecessary allocations.
	}
);

#[cfg(test)]
Expand DownExpand Up@@ -2422,11 +2403,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice_request.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice_request.bytes);
Expand DownExpand Up@@ -2460,11 +2436,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice_request.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice_request.bytes);
Expand DownExpand Up@@ -2508,11 +2479,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice_request.bytes)
Expand DownExpand Up@@ -2549,11 +2515,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice_request.bytes)
Expand Down

8 changes: 1 addition & 7 deletions
lightning/src/offers/merkle.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -11,7 +11,6 @@

use bitcoin::hashes::{Hash, HashEngine, sha256};
use bitcoin::secp256k1::{Message, PublicKey, Secp256k1, self};
use bitcoin::secp256k1::constants::SCHNORR_SIGNATURE_SIZE;
use bitcoin::secp256k1::schnorr::Signature;
use crate::io;
use crate::util::ser::{BigSize, Readable, Writeable, Writer};
Expand All@@ -26,10 +25,6 @@ tlv_stream!(SignatureTlvStream, SignatureTlvStreamRef<'a>, SIGNATURE_TYPES, {
	(240, signature: Signature),
});

/// Size of a TLV record in `SIGNATURE_TYPES` when the type is 1000. TLV types are encoded using
/// BigSize, so a TLV record with type 240 will use two less bytes.
pub(super) const SIGNATURE_TLV_RECORD_SIZE: usize = 3 + 1 + SCHNORR_SIGNATURE_SIZE;

/// A hash for use in a specific context by tweaking with a context-dependent tag as per [BIP 340]
/// and computed over the merkle root of a TLV stream to sign as defined in [BOLT 12].
///
Expand DownExpand Up@@ -253,7 +248,6 @@ pub(super) struct TlvRecord<'a> {
	type_bytes: &'a [u8],
	// The entire TLV record.
	pub(super) record_bytes: &'a [u8],
	pub(super) start: usize,
	pub(super) end: usize,
}

Expand All@@ -278,7 +272,7 @@ impl<'a> Iterator for TlvStream<'a> {
 self.data.set_position(end);

 Some(TlvRecord {
 r#type, type_bytes, record_bytes, start: start as usize, end: end as usize,
 r#type, type_bytes, record_bytes, end: end as usize,
 })
 } else {
 None
Expand Down

3 changes: 2 additions & 1 deletion
lightning/src/offers/offer.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -438,7 +438,8 @@ macro_rules! offer_builder_methods { (
 }
 }

 let mut bytes = Vec::new();
 const OFFER_ALLOCATION_SIZE: usize = 512;
 let mut bytes = Vec::with_capacity(OFFER_ALLOCATION_SIZE);
 $self.offer.write(&mut bytes).unwrap();

 let id = OfferId::from_valid_offer_tlv_stream(&bytes);
Expand Down

3 changes: 2 additions & 1 deletion
lightning/src/offers/refund.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -338,7 +338,8 @@ macro_rules! refund_builder_methods { (
 $self.refund.payer.0 = metadata;
 }

 let mut bytes = Vec::new();
 const REFUND_ALLOCATION_SIZE: usize = 512;
 let mut bytes = Vec::with_capacity(REFUND_ALLOCATION_SIZE);
 $self.refund.write(&mut bytes).unwrap();

 Ok(Refund {
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Highlight search terms from Google/DuckDuckGo/Bing referrer
(function() {
var ref = document.referrer;
var terms = [];
if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {
var url = new URL(ref);
var q = url.searchParams.get('q') || url.searchParams.get('p');
if (q) {
terms = q.split(/\s+/).filter(function(t) { return t.length > 2; });
}
}
if (terms.length === 0) return;
var style = document.createElement('style');
style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';
document.head.appendChild(style);
function highlight(node) {
if (node.nodeType === 3) { // text node
var text = node.textContent;
var found = false;
terms.forEach(function(term) {
var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi');
if (regex.test(text)) {
found = true;
var frag = document.createDocumentFragment();
var parts = text.split(regex);
parts.forEach(function(part, i) {
if (i % 2 === 0) {
frag.appendChild(document.createTextNode(part));
} else {
var span = document.createElement('span');
span.className = 'userscript-highlight';
span.textContent = part;
frag.appendChild(span);
}
});
node.parentNode.replaceChild(frag, node);
}
});
} else if (node.nodeType === 1 && node.childNodes) { // element
var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];
if (!skipTags.includes(node.tagName)) {
Array.from(node.childNodes).forEach(highlight);
}
}
}
highlight(document.body);
// Re-highlight on dynamic content
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1 || node.nodeType === 3) highlight(node);
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Don't over-allocate invoice bytes by jkczyz · Pull Request #3494 · lightningdevkit/rust-lightning · GitHub
Skip to content
Don't over-allocate invoice bytes#3494

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.

 Sign up for GitHub

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

 Merged

TheBlueMatt
merged 2 commits into

from

Feb 7, 2025
 Merged


Don't over-allocate invoice bytes
#3494
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
ffaccc0

Use fixed-size Vec allocations for BOLT12 messages


jkczyz Jan 28, 2025
0912b51

Drop use of reserve_exact in BOLT12 tests


jkczyz Feb 6, 2025

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


65 changes: 12 additions & 53 deletions
lightning/src/offers/invoice.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -122,7 +122,7 @@ use crate::offers::invoice_macros::{invoice_accessors_common, invoice_builder_me
#[cfg(test)]
use crate::offers::invoice_macros::invoice_builder_methods_test_common;
use crate::offers::invoice_request::{EXPERIMENTAL_INVOICE_REQUEST_TYPES, ExperimentalInvoiceRequestTlvStream, ExperimentalInvoiceRequestTlvStreamRef, INVOICE_REQUEST_PAYER_ID_TYPE, INVOICE_REQUEST_TYPES, IV_BYTES as INVOICE_REQUEST_IV_BYTES, InvoiceRequest, InvoiceRequestContents, InvoiceRequestTlvStream, InvoiceRequestTlvStreamRef};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self, SIGNATURE_TLV_RECORD_SIZE};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self};
use crate::offers::nonce::Nonce;
use crate::offers::offer::{Amount, EXPERIMENTAL_OFFER_TYPES, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OFFER_TYPES, OfferTlvStream, OfferTlvStreamRef, Quantity};
use crate::offers::parse::{Bolt12ParseError, Bolt12SemanticError, ParsedMessage};
Expand DownExpand Up@@ -520,19 +520,8 @@ impl UnsignedBolt12Invoice {
 let (_, _, _, invoice_tlv_stream, _, _, experimental_invoice_tlv_stream) =
 contents.as_tlv_stream();

 // Allocate enough space for the invoice, which will include:
 // - all TLV records from `invreq_bytes` except signatures,
 // - all invoice-specific TLV records, and
 // - a signature TLV record once the invoice is signed.
 //
 // This assumes both the invoice request and the invoice will each only have one signature
 // using SIGNATURE_TYPES.start as the TLV record. Thus, it is accounted for by invreq_bytes.
 let mut bytes = Vec::with_capacity(
 invreq_bytes.len()
 + invoice_tlv_stream.serialized_length()
 + if contents.is_for_offer() { 0 } else { SIGNATURE_TLV_RECORD_SIZE }
 + experimental_invoice_tlv_stream.serialized_length(),
 );
 const INVOICE_ALLOCATION_SIZE: usize = 1024;
 let mut bytes = Vec::with_capacity(INVOICE_ALLOCATION_SIZE);

 // Use the invoice_request bytes instead of the invoice_request TLV stream as the latter may
 // have contained unknown TLV records, which are not stored in `InvoiceRequestContents` or
Expand All@@ -545,23 +534,16 @@ impl UnsignedBolt12Invoice {

 invoice_tlv_stream.write(&mut bytes).unwrap();

 let mut experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_TYPES)
 .peekable();
 let mut experimental_bytes = Vec::with_capacity(
 remaining_bytes.len()
 - experimental_tlv_stream
 .peek()
 .map_or(remaining_bytes.len(), |first_record| first_record.start)
 + experimental_invoice_tlv_stream.serialized_length(),
 );
 const EXPERIMENTAL_TLV_ALLOCATION_SIZE: usize = 0;
 let mut experimental_bytes = Vec::with_capacity(EXPERIMENTAL_TLV_ALLOCATION_SIZE);
@TheBlueMattTheBlueMattJan 30, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Should we default to no allocation here instead? If we expect these to rarely be used that'd save us an allocation and if we only ever have one TLV then we always only do one allocation anyway, so it'd only cost us an allocation if we have multiple TLVs.
@jkczyzjkczyzJan 31, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Sure, we can do that. Though the number of allocations will likely be at least two since we write the type and length before writing the value. Depends on how large the value is and how much the initial Vec allocation size is. Rust playground shows 8 bytes.
@TheBlueMattTheBlueMattJan 31, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Are you sure about that? TlvStream iterates over TlvRecords which implement Writeable with a single call to write_all.
@jkczyzjkczyzJan 31, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Ah, yeah, for experimental TLVs from the offer/invreq (i.e., in experimental_tlv_stream), that is true. For those that we set in the invoice (i.e., anything in experimental_invoice_tlv_stream), it would not be the case though.
@TheBlueMattTheBlueMattJan 31, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Mmm, hmmm, right. We could check if any of the supported experimental fields are set and allocate based on that? I can imagine someone having paid 100k BOLT12s, loading them in memory, and us wasting 48MB plus overhead for them...Basically this would just be a comment in ExperimentalInvoiceTlvStream that we should allocate if we add fields since we don't support any fields outside of test anyway.
@jkczyzjkczyzJan 31, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Updated

 let experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_TYPES);
 for record in experimental_tlv_stream {
 record.write(&mut experimental_bytes).unwrap();
 }

 experimental_invoice_tlv_stream.write(&mut experimental_bytes).unwrap();
 debug_assert_eq!(experimental_bytes.len(), experimental_bytes.capacity());

 let tlv_stream = TlvStream::new(&bytes).chain(TlvStream::new(&experimental_bytes));
 let tagged_hash = TaggedHash::from_tlv_stream(SIGNATURE_TAG, tlv_stream);
Expand DownExpand Up@@ -592,14 +574,6 @@ macro_rules! unsigned_invoice_sign_method { ($self: ident, $self_type: ty $(, $s
 signature_tlv_stream.write(&mut $self.bytes).unwrap();

 // Append the experimental bytes after the signature.
 debug_assert_eq!(
 // The two-byte overallocation results from SIGNATURE_TLV_RECORD_SIZE accommodating TLV
 // records with types >= 253.
 $self.bytes.len()
 + $self.experimental_bytes.len()
 + if $self.contents.is_for_offer() { 0 } else { 2 },
 $self.bytes.capacity(),
 );
 $self.bytes.extend_from_slice(&$self.experimental_bytes);

 Ok(Bolt12Invoice {
Expand DownExpand Up@@ -965,13 +939,6 @@ impl Hash for Bolt12Invoice {
}

impl InvoiceContents {
	fn is_for_offer(&self) -> bool {
 match self {
 InvoiceContents::ForOffer { .. } => true,
 InvoiceContents::ForRefund { .. } => false,
 }
	}

	/// Whether the original offer or refund has expired.
	#[cfg(feature = "std")]
	fn is_offer_or_refund_expired(&self) -> bool {
Expand DownExpand Up@@ -1362,7 +1329,11 @@ pub(super) const EXPERIMENTAL_INVOICE_TYPES: core::ops::RangeFrom<u64> = 3_000_0

#[cfg(not(test))]
tlv_stream!(
	ExperimentalInvoiceTlvStream, ExperimentalInvoiceTlvStreamRef, EXPERIMENTAL_INVOICE_TYPES, {}
	ExperimentalInvoiceTlvStream, ExperimentalInvoiceTlvStreamRef, EXPERIMENTAL_INVOICE_TYPES, {
 // When adding experimental TLVs, update EXPERIMENTAL_TLV_ALLOCATION_SIZE accordingly in
 // both UnsignedBolt12Invoice:new and UnsignedStaticInvoice::new to avoid unnecessary
 // allocations.
	}
);

#[cfg(test)]
Expand DownExpand Up@@ -2880,9 +2851,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice.bytes);
Expand DownExpand Up@@ -2917,9 +2885,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice.bytes);
Expand DownExpand Up@@ -2982,9 +2947,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice.bytes)
Expand DownExpand Up@@ -3021,9 +2983,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice.bytes)
Expand Down

61 changes: 11 additions & 50 deletions
lightning/src/offers/invoice_request.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -77,7 +77,7 @@ use crate::ln::channelmanager::PaymentId;
use crate::types::features::InvoiceRequestFeatures;
use crate::ln::inbound_payment::{ExpandedKey, IV_LEN};
use crate::ln::msgs::DecodeError;
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self, SIGNATURE_TLV_RECORD_SIZE};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self};
use crate::offers::nonce::Nonce;
use crate::offers::offer::{Amount, EXPERIMENTAL_OFFER_TYPES, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OFFER_TYPES, Offer, OfferContents, OfferId, OfferTlvStream, OfferTlvStreamRef};
use crate::offers::parse::{Bolt12ParseError, ParsedMessage, Bolt12SemanticError};
Expand DownExpand Up@@ -473,17 +473,8 @@ impl UnsignedInvoiceRequest {
 _experimental_offer_tlv_stream, experimental_invoice_request_tlv_stream,
 ) = contents.as_tlv_stream();

 // Allocate enough space for the invoice_request, which will include:
 // - all TLV records from `offer.bytes`,
 // - all invoice_request-specific TLV records, and
 // - a signature TLV record once the invoice_request is signed.
 let mut bytes = Vec::with_capacity(
 offer.bytes.len()
 + payer_tlv_stream.serialized_length()
 + invoice_request_tlv_stream.serialized_length()
 + SIGNATURE_TLV_RECORD_SIZE
 + experimental_invoice_request_tlv_stream.serialized_length(),
 );
 const INVOICE_REQUEST_ALLOCATION_SIZE: usize = 512;
 let mut bytes = Vec::with_capacity(INVOICE_REQUEST_ALLOCATION_SIZE);

 payer_tlv_stream.write(&mut bytes).unwrap();

Expand All@@ -495,23 +486,16 @@ impl UnsignedInvoiceRequest {

 invoice_request_tlv_stream.write(&mut bytes).unwrap();

 let mut experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_OFFER_TYPES)
 .peekable();
 let mut experimental_bytes = Vec::with_capacity(
 remaining_bytes.len()
 - experimental_tlv_stream
 .peek()
 .map_or(remaining_bytes.len(), |first_record| first_record.start)
 + experimental_invoice_request_tlv_stream.serialized_length(),
 );
 const EXPERIMENTAL_TLV_ALLOCATION_SIZE: usize = 0;
 let mut experimental_bytes = Vec::with_capacity(EXPERIMENTAL_TLV_ALLOCATION_SIZE);

 let experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_OFFER_TYPES);
 for record in experimental_tlv_stream {
 record.write(&mut experimental_bytes).unwrap();
 }

 experimental_invoice_request_tlv_stream.write(&mut experimental_bytes).unwrap();
 debug_assert_eq!(experimental_bytes.len(), experimental_bytes.capacity());

 let tlv_stream = TlvStream::new(&bytes).chain(TlvStream::new(&experimental_bytes));
 let tagged_hash = TaggedHash::from_tlv_stream(SIGNATURE_TAG, tlv_stream);
Expand DownExpand Up@@ -544,12 +528,6 @@ macro_rules! unsigned_invoice_request_sign_method { (
 signature_tlv_stream.write(&mut $self.bytes).unwrap();

 // Append the experimental bytes after the signature.
 debug_assert_eq!(
 // The two-byte overallocation results from SIGNATURE_TLV_RECORD_SIZE accommodating TLV
 // records with types >= 253.
 $self.bytes.len() + $self.experimental_bytes.len() + 2,
 $self.bytes.capacity(),
 );
 $self.bytes.extend_from_slice(&$self.experimental_bytes);

 Ok(InvoiceRequest {
Expand DownExpand Up@@ -1127,7 +1105,10 @@ pub(super) const EXPERIMENTAL_INVOICE_REQUEST_TYPES: core::ops::Range<u64> =
#[cfg(not(test))]
tlv_stream!(
	ExperimentalInvoiceRequestTlvStream, ExperimentalInvoiceRequestTlvStreamRef,
	EXPERIMENTAL_INVOICE_REQUEST_TYPES, {}
	EXPERIMENTAL_INVOICE_REQUEST_TYPES, {
 // When adding experimental TLVs, update EXPERIMENTAL_TLV_ALLOCATION_SIZE accordingly in
 // UnsignedInvoiceRequest::new to avoid unnecessary allocations.
	}
);

#[cfg(test)]
Expand DownExpand Up@@ -2422,11 +2403,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice_request.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice_request.bytes);
Expand DownExpand Up@@ -2460,11 +2436,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice_request.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice_request.bytes);
Expand DownExpand Up@@ -2508,11 +2479,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice_request.bytes)
Expand DownExpand Up@@ -2549,11 +2515,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice_request.bytes)
Expand Down

8 changes: 1 addition & 7 deletions
lightning/src/offers/merkle.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -11,7 +11,6 @@

use bitcoin::hashes::{Hash, HashEngine, sha256};
use bitcoin::secp256k1::{Message, PublicKey, Secp256k1, self};
use bitcoin::secp256k1::constants::SCHNORR_SIGNATURE_SIZE;
use bitcoin::secp256k1::schnorr::Signature;
use crate::io;
use crate::util::ser::{BigSize, Readable, Writeable, Writer};
Expand All@@ -26,10 +25,6 @@ tlv_stream!(SignatureTlvStream, SignatureTlvStreamRef<'a>, SIGNATURE_TYPES, {
	(240, signature: Signature),
});

/// Size of a TLV record in `SIGNATURE_TYPES` when the type is 1000. TLV types are encoded using
/// BigSize, so a TLV record with type 240 will use two less bytes.
pub(super) const SIGNATURE_TLV_RECORD_SIZE: usize = 3 + 1 + SCHNORR_SIGNATURE_SIZE;

/// A hash for use in a specific context by tweaking with a context-dependent tag as per [BIP 340]
/// and computed over the merkle root of a TLV stream to sign as defined in [BOLT 12].
///
Expand DownExpand Up@@ -253,7 +248,6 @@ pub(super) struct TlvRecord<'a> {
	type_bytes: &'a [u8],
	// The entire TLV record.
	pub(super) record_bytes: &'a [u8],
	pub(super) start: usize,
	pub(super) end: usize,
}

Expand All@@ -278,7 +272,7 @@ impl<'a> Iterator for TlvStream<'a> {
 self.data.set_position(end);

 Some(TlvRecord {
 r#type, type_bytes, record_bytes, start: start as usize, end: end as usize,
 r#type, type_bytes, record_bytes, end: end as usize,
 })
 } else {
 None
Expand Down

3 changes: 2 additions & 1 deletion
lightning/src/offers/offer.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -438,7 +438,8 @@ macro_rules! offer_builder_methods { (
 }
 }

 let mut bytes = Vec::new();
 const OFFER_ALLOCATION_SIZE: usize = 512;
 let mut bytes = Vec::with_capacity(OFFER_ALLOCATION_SIZE);
 $self.offer.write(&mut bytes).unwrap();

 let id = OfferId::from_valid_offer_tlv_stream(&bytes);
Expand Down

3 changes: 2 additions & 1 deletion
lightning/src/offers/refund.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -338,7 +338,8 @@ macro_rules! refund_builder_methods { (
 $self.refund.payer.0 = metadata;
 }

 let mut bytes = Vec::new();
 const REFUND_ALLOCATION_SIZE: usize = 512;
 let mut bytes = Vec::with_capacity(REFUND_ALLOCATION_SIZE);
 $self.refund.write(&mut bytes).unwrap();

 Ok(Refund {
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Strip utm_, fbclid, gclid, etc. from all links on page
(function() {
var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',
'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',
'ref', 'ref_src', 'source', 'medium', 'campaign'];
function cleanUrl(url) {
try {
var u = new URL(url, window.location.origin);
var changed = false;
trackingParams.forEach(function(p) {
if (u.searchParams.has(p)) {
u.searchParams.delete(p);
changed = true;
}
});
return changed ? u.toString() : url;
} catch (e) {
return url;
}
}
function cleanLinks() {
document.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
cleanLinks();
var observer = new MutationObserver(function(mutations) {
mutations.forEach(function(m) {
m.addedNodes.forEach(function(node) {
if (node.nodeType === 1) {
if (node.tagName === 'A') cleanLinks();
node.querySelectorAll('a[href]').forEach(function(a) {
var clean = cleanUrl(a.href);
if (clean !== a.href) a.href = clean;
});
}
});
});
});
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '
Don't over-allocate invoice bytes by jkczyz · Pull Request #3494 · lightningdevkit/rust-lightning · GitHub
Skip to content
Don't over-allocate invoice bytes#3494

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.

 Sign up for GitHub

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

 Merged

TheBlueMatt
merged 2 commits into

from

Feb 7, 2025
 Merged


Don't over-allocate invoice bytes
#3494
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
ffaccc0

Use fixed-size Vec allocations for BOLT12 messages


jkczyz Jan 28, 2025
0912b51

Drop use of reserve_exact in BOLT12 tests


jkczyz Feb 6, 2025

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


65 changes: 12 additions & 53 deletions
lightning/src/offers/invoice.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -122,7 +122,7 @@ use crate::offers::invoice_macros::{invoice_accessors_common, invoice_builder_me
#[cfg(test)]
use crate::offers::invoice_macros::invoice_builder_methods_test_common;
use crate::offers::invoice_request::{EXPERIMENTAL_INVOICE_REQUEST_TYPES, ExperimentalInvoiceRequestTlvStream, ExperimentalInvoiceRequestTlvStreamRef, INVOICE_REQUEST_PAYER_ID_TYPE, INVOICE_REQUEST_TYPES, IV_BYTES as INVOICE_REQUEST_IV_BYTES, InvoiceRequest, InvoiceRequestContents, InvoiceRequestTlvStream, InvoiceRequestTlvStreamRef};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self, SIGNATURE_TLV_RECORD_SIZE};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self};
use crate::offers::nonce::Nonce;
use crate::offers::offer::{Amount, EXPERIMENTAL_OFFER_TYPES, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OFFER_TYPES, OfferTlvStream, OfferTlvStreamRef, Quantity};
use crate::offers::parse::{Bolt12ParseError, Bolt12SemanticError, ParsedMessage};
Expand DownExpand Up@@ -520,19 +520,8 @@ impl UnsignedBolt12Invoice {
 let (_, _, _, invoice_tlv_stream, _, _, experimental_invoice_tlv_stream) =
 contents.as_tlv_stream();

 // Allocate enough space for the invoice, which will include:
 // - all TLV records from `invreq_bytes` except signatures,
 // - all invoice-specific TLV records, and
 // - a signature TLV record once the invoice is signed.
 //
 // This assumes both the invoice request and the invoice will each only have one signature
 // using SIGNATURE_TYPES.start as the TLV record. Thus, it is accounted for by invreq_bytes.
 let mut bytes = Vec::with_capacity(
 invreq_bytes.len()
 + invoice_tlv_stream.serialized_length()
 + if contents.is_for_offer() { 0 } else { SIGNATURE_TLV_RECORD_SIZE }
 + experimental_invoice_tlv_stream.serialized_length(),
 );
 const INVOICE_ALLOCATION_SIZE: usize = 1024;
 let mut bytes = Vec::with_capacity(INVOICE_ALLOCATION_SIZE);

 // Use the invoice_request bytes instead of the invoice_request TLV stream as the latter may
 // have contained unknown TLV records, which are not stored in `InvoiceRequestContents` or
Expand All@@ -545,23 +534,16 @@ impl UnsignedBolt12Invoice {

 invoice_tlv_stream.write(&mut bytes).unwrap();

 let mut experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_TYPES)
 .peekable();
 let mut experimental_bytes = Vec::with_capacity(
 remaining_bytes.len()
 - experimental_tlv_stream
 .peek()
 .map_or(remaining_bytes.len(), |first_record| first_record.start)
 + experimental_invoice_tlv_stream.serialized_length(),
 );
 const EXPERIMENTAL_TLV_ALLOCATION_SIZE: usize = 0;
 let mut experimental_bytes = Vec::with_capacity(EXPERIMENTAL_TLV_ALLOCATION_SIZE);
@TheBlueMattTheBlueMattJan 30, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Should we default to no allocation here instead? If we expect these to rarely be used that'd save us an allocation and if we only ever have one TLV then we always only do one allocation anyway, so it'd only cost us an allocation if we have multiple TLVs.
@jkczyzjkczyzJan 31, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Sure, we can do that. Though the number of allocations will likely be at least two since we write the type and length before writing the value. Depends on how large the value is and how much the initial Vec allocation size is. Rust playground shows 8 bytes.
@TheBlueMattTheBlueMattJan 31, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Are you sure about that? TlvStream iterates over TlvRecords which implement Writeable with a single call to write_all.
@jkczyzjkczyzJan 31, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Ah, yeah, for experimental TLVs from the offer/invreq (i.e., in experimental_tlv_stream), that is true. For those that we set in the invoice (i.e., anything in experimental_invoice_tlv_stream), it would not be the case though.
@TheBlueMattTheBlueMattJan 31, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Mmm, hmmm, right. We could check if any of the supported experimental fields are set and allocate based on that? I can imagine someone having paid 100k BOLT12s, loading them in memory, and us wasting 48MB plus overhead for them...Basically this would just be a comment in ExperimentalInvoiceTlvStream that we should allocate if we add fields since we don't support any fields outside of test anyway.
@jkczyzjkczyzJan 31, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Updated

 let experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_TYPES);
 for record in experimental_tlv_stream {
 record.write(&mut experimental_bytes).unwrap();
 }

 experimental_invoice_tlv_stream.write(&mut experimental_bytes).unwrap();
 debug_assert_eq!(experimental_bytes.len(), experimental_bytes.capacity());

 let tlv_stream = TlvStream::new(&bytes).chain(TlvStream::new(&experimental_bytes));
 let tagged_hash = TaggedHash::from_tlv_stream(SIGNATURE_TAG, tlv_stream);
Expand DownExpand Up@@ -592,14 +574,6 @@ macro_rules! unsigned_invoice_sign_method { ($self: ident, $self_type: ty $(, $s
 signature_tlv_stream.write(&mut $self.bytes).unwrap();

 // Append the experimental bytes after the signature.
 debug_assert_eq!(
 // The two-byte overallocation results from SIGNATURE_TLV_RECORD_SIZE accommodating TLV
 // records with types >= 253.
 $self.bytes.len()
 + $self.experimental_bytes.len()
 + if $self.contents.is_for_offer() { 0 } else { 2 },
 $self.bytes.capacity(),
 );
 $self.bytes.extend_from_slice(&$self.experimental_bytes);

 Ok(Bolt12Invoice {
Expand DownExpand Up@@ -965,13 +939,6 @@ impl Hash for Bolt12Invoice {
}

impl InvoiceContents {
	fn is_for_offer(&self) -> bool {
 match self {
 InvoiceContents::ForOffer { .. } => true,
 InvoiceContents::ForRefund { .. } => false,
 }
	}

	/// Whether the original offer or refund has expired.
	#[cfg(feature = "std")]
	fn is_offer_or_refund_expired(&self) -> bool {
Expand DownExpand Up@@ -1362,7 +1329,11 @@ pub(super) const EXPERIMENTAL_INVOICE_TYPES: core::ops::RangeFrom<u64> = 3_000_0

#[cfg(not(test))]
tlv_stream!(
	ExperimentalInvoiceTlvStream, ExperimentalInvoiceTlvStreamRef, EXPERIMENTAL_INVOICE_TYPES, {}
	ExperimentalInvoiceTlvStream, ExperimentalInvoiceTlvStreamRef, EXPERIMENTAL_INVOICE_TYPES, {
 // When adding experimental TLVs, update EXPERIMENTAL_TLV_ALLOCATION_SIZE accordingly in
 // both UnsignedBolt12Invoice:new and UnsignedStaticInvoice::new to avoid unnecessary
 // allocations.
	}
);

#[cfg(test)]
Expand DownExpand Up@@ -2880,9 +2851,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice.bytes);
Expand DownExpand Up@@ -2917,9 +2885,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice.bytes);
Expand DownExpand Up@@ -2982,9 +2947,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice.bytes)
Expand DownExpand Up@@ -3021,9 +2983,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice.bytes)
Expand Down

61 changes: 11 additions & 50 deletions
lightning/src/offers/invoice_request.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -77,7 +77,7 @@ use crate::ln::channelmanager::PaymentId;
use crate::types::features::InvoiceRequestFeatures;
use crate::ln::inbound_payment::{ExpandedKey, IV_LEN};
use crate::ln::msgs::DecodeError;
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self, SIGNATURE_TLV_RECORD_SIZE};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self};
use crate::offers::nonce::Nonce;
use crate::offers::offer::{Amount, EXPERIMENTAL_OFFER_TYPES, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OFFER_TYPES, Offer, OfferContents, OfferId, OfferTlvStream, OfferTlvStreamRef};
use crate::offers::parse::{Bolt12ParseError, ParsedMessage, Bolt12SemanticError};
Expand DownExpand Up@@ -473,17 +473,8 @@ impl UnsignedInvoiceRequest {
 _experimental_offer_tlv_stream, experimental_invoice_request_tlv_stream,
 ) = contents.as_tlv_stream();

 // Allocate enough space for the invoice_request, which will include:
 // - all TLV records from `offer.bytes`,
 // - all invoice_request-specific TLV records, and
 // - a signature TLV record once the invoice_request is signed.
 let mut bytes = Vec::with_capacity(
 offer.bytes.len()
 + payer_tlv_stream.serialized_length()
 + invoice_request_tlv_stream.serialized_length()
 + SIGNATURE_TLV_RECORD_SIZE
 + experimental_invoice_request_tlv_stream.serialized_length(),
 );
 const INVOICE_REQUEST_ALLOCATION_SIZE: usize = 512;
 let mut bytes = Vec::with_capacity(INVOICE_REQUEST_ALLOCATION_SIZE);

 payer_tlv_stream.write(&mut bytes).unwrap();

Expand All@@ -495,23 +486,16 @@ impl UnsignedInvoiceRequest {

 invoice_request_tlv_stream.write(&mut bytes).unwrap();

 let mut experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_OFFER_TYPES)
 .peekable();
 let mut experimental_bytes = Vec::with_capacity(
 remaining_bytes.len()
 - experimental_tlv_stream
 .peek()
 .map_or(remaining_bytes.len(), |first_record| first_record.start)
 + experimental_invoice_request_tlv_stream.serialized_length(),
 );
 const EXPERIMENTAL_TLV_ALLOCATION_SIZE: usize = 0;
 let mut experimental_bytes = Vec::with_capacity(EXPERIMENTAL_TLV_ALLOCATION_SIZE);

 let experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_OFFER_TYPES);
 for record in experimental_tlv_stream {
 record.write(&mut experimental_bytes).unwrap();
 }

 experimental_invoice_request_tlv_stream.write(&mut experimental_bytes).unwrap();
 debug_assert_eq!(experimental_bytes.len(), experimental_bytes.capacity());

 let tlv_stream = TlvStream::new(&bytes).chain(TlvStream::new(&experimental_bytes));
 let tagged_hash = TaggedHash::from_tlv_stream(SIGNATURE_TAG, tlv_stream);
Expand DownExpand Up@@ -544,12 +528,6 @@ macro_rules! unsigned_invoice_request_sign_method { (
 signature_tlv_stream.write(&mut $self.bytes).unwrap();

 // Append the experimental bytes after the signature.
 debug_assert_eq!(
 // The two-byte overallocation results from SIGNATURE_TLV_RECORD_SIZE accommodating TLV
 // records with types >= 253.
 $self.bytes.len() + $self.experimental_bytes.len() + 2,
 $self.bytes.capacity(),
 );
 $self.bytes.extend_from_slice(&$self.experimental_bytes);

 Ok(InvoiceRequest {
Expand DownExpand Up@@ -1127,7 +1105,10 @@ pub(super) const EXPERIMENTAL_INVOICE_REQUEST_TYPES: core::ops::Range<u64> =
#[cfg(not(test))]
tlv_stream!(
	ExperimentalInvoiceRequestTlvStream, ExperimentalInvoiceRequestTlvStreamRef,
	EXPERIMENTAL_INVOICE_REQUEST_TYPES, {}
	EXPERIMENTAL_INVOICE_REQUEST_TYPES, {
 // When adding experimental TLVs, update EXPERIMENTAL_TLV_ALLOCATION_SIZE accordingly in
 // UnsignedInvoiceRequest::new to avoid unnecessary allocations.
	}
);

#[cfg(test)]
Expand DownExpand Up@@ -2422,11 +2403,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice_request.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice_request.bytes);
Expand DownExpand Up@@ -2460,11 +2436,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice_request.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice_request.bytes);
Expand DownExpand Up@@ -2508,11 +2479,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice_request.bytes)
Expand DownExpand Up@@ -2549,11 +2515,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice_request.bytes)
Expand Down

8 changes: 1 addition & 7 deletions
lightning/src/offers/merkle.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -11,7 +11,6 @@

use bitcoin::hashes::{Hash, HashEngine, sha256};
use bitcoin::secp256k1::{Message, PublicKey, Secp256k1, self};
use bitcoin::secp256k1::constants::SCHNORR_SIGNATURE_SIZE;
use bitcoin::secp256k1::schnorr::Signature;
use crate::io;
use crate::util::ser::{BigSize, Readable, Writeable, Writer};
Expand All@@ -26,10 +25,6 @@ tlv_stream!(SignatureTlvStream, SignatureTlvStreamRef<'a>, SIGNATURE_TYPES, {
	(240, signature: Signature),
});

/// Size of a TLV record in `SIGNATURE_TYPES` when the type is 1000. TLV types are encoded using
/// BigSize, so a TLV record with type 240 will use two less bytes.
pub(super) const SIGNATURE_TLV_RECORD_SIZE: usize = 3 + 1 + SCHNORR_SIGNATURE_SIZE;

/// A hash for use in a specific context by tweaking with a context-dependent tag as per [BIP 340]
/// and computed over the merkle root of a TLV stream to sign as defined in [BOLT 12].
///
Expand DownExpand Up@@ -253,7 +248,6 @@ pub(super) struct TlvRecord<'a> {
	type_bytes: &'a [u8],
	// The entire TLV record.
	pub(super) record_bytes: &'a [u8],
	pub(super) start: usize,
	pub(super) end: usize,
}

Expand All@@ -278,7 +272,7 @@ impl<'a> Iterator for TlvStream<'a> {
 self.data.set_position(end);

 Some(TlvRecord {
 r#type, type_bytes, record_bytes, start: start as usize, end: end as usize,
 r#type, type_bytes, record_bytes, end: end as usize,
 })
 } else {
 None
Expand Down

3 changes: 2 additions & 1 deletion
lightning/src/offers/offer.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -438,7 +438,8 @@ macro_rules! offer_builder_methods { (
 }
 }

 let mut bytes = Vec::new();
 const OFFER_ALLOCATION_SIZE: usize = 512;
 let mut bytes = Vec::with_capacity(OFFER_ALLOCATION_SIZE);
 $self.offer.write(&mut bytes).unwrap();

 let id = OfferId::from_valid_offer_tlv_stream(&bytes);
Expand Down

3 changes: 2 additions & 1 deletion
lightning/src/offers/refund.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -338,7 +338,8 @@ macro_rules! refund_builder_methods { (
 $self.refund.payer.0 = metadata;
 }

 let mut bytes = Vec::new();
 const REFUND_ALLOCATION_SIZE: usize = 512;
 let mut bytes = Vec::with_capacity(REFUND_ALLOCATION_SIZE);
 $self.refund.write(&mut bytes).unwrap();

 Ok(Refund {
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Auto-enable theater mode on YouTube
(function() {
function tryTheater() {
var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]');
if (btn && !btn.classList.contains('activated')) {
btn.click();
}
}
// Try immediately
tryTheater();
// Try after navigation (SPA)
var lastUrl = location.href;
setInterval(function() {
if (location.href !== lastUrl) {
lastUrl = location.href;
setTimeout(tryTheater, 500);
}
}, 1000);
// Also try on player load
var observer = new MutationObserver(tryTheater);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Don't over-allocate invoice bytes by jkczyz · Pull Request #3494 · lightningdevkit/rust-lightning · GitHub
Skip to content
Don't over-allocate invoice bytes#3494

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.

 Sign up for GitHub

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

 Merged

TheBlueMatt
merged 2 commits into

from

Feb 7, 2025
 Merged


Don't over-allocate invoice bytes
#3494
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
ffaccc0

Use fixed-size Vec allocations for BOLT12 messages


jkczyz Jan 28, 2025
0912b51

Drop use of reserve_exact in BOLT12 tests


jkczyz Feb 6, 2025

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


65 changes: 12 additions & 53 deletions
lightning/src/offers/invoice.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -122,7 +122,7 @@ use crate::offers::invoice_macros::{invoice_accessors_common, invoice_builder_me
#[cfg(test)]
use crate::offers::invoice_macros::invoice_builder_methods_test_common;
use crate::offers::invoice_request::{EXPERIMENTAL_INVOICE_REQUEST_TYPES, ExperimentalInvoiceRequestTlvStream, ExperimentalInvoiceRequestTlvStreamRef, INVOICE_REQUEST_PAYER_ID_TYPE, INVOICE_REQUEST_TYPES, IV_BYTES as INVOICE_REQUEST_IV_BYTES, InvoiceRequest, InvoiceRequestContents, InvoiceRequestTlvStream, InvoiceRequestTlvStreamRef};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self, SIGNATURE_TLV_RECORD_SIZE};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self};
use crate::offers::nonce::Nonce;
use crate::offers::offer::{Amount, EXPERIMENTAL_OFFER_TYPES, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OFFER_TYPES, OfferTlvStream, OfferTlvStreamRef, Quantity};
use crate::offers::parse::{Bolt12ParseError, Bolt12SemanticError, ParsedMessage};
Expand DownExpand Up@@ -520,19 +520,8 @@ impl UnsignedBolt12Invoice {
 let (_, _, _, invoice_tlv_stream, _, _, experimental_invoice_tlv_stream) =
 contents.as_tlv_stream();

 // Allocate enough space for the invoice, which will include:
 // - all TLV records from `invreq_bytes` except signatures,
 // - all invoice-specific TLV records, and
 // - a signature TLV record once the invoice is signed.
 //
 // This assumes both the invoice request and the invoice will each only have one signature
 // using SIGNATURE_TYPES.start as the TLV record. Thus, it is accounted for by invreq_bytes.
 let mut bytes = Vec::with_capacity(
 invreq_bytes.len()
 + invoice_tlv_stream.serialized_length()
 + if contents.is_for_offer() { 0 } else { SIGNATURE_TLV_RECORD_SIZE }
 + experimental_invoice_tlv_stream.serialized_length(),
 );
 const INVOICE_ALLOCATION_SIZE: usize = 1024;
 let mut bytes = Vec::with_capacity(INVOICE_ALLOCATION_SIZE);

 // Use the invoice_request bytes instead of the invoice_request TLV stream as the latter may
 // have contained unknown TLV records, which are not stored in `InvoiceRequestContents` or
Expand All@@ -545,23 +534,16 @@ impl UnsignedBolt12Invoice {

 invoice_tlv_stream.write(&mut bytes).unwrap();

 let mut experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_TYPES)
 .peekable();
 let mut experimental_bytes = Vec::with_capacity(
 remaining_bytes.len()
 - experimental_tlv_stream
 .peek()
 .map_or(remaining_bytes.len(), |first_record| first_record.start)
 + experimental_invoice_tlv_stream.serialized_length(),
 );
 const EXPERIMENTAL_TLV_ALLOCATION_SIZE: usize = 0;
 let mut experimental_bytes = Vec::with_capacity(EXPERIMENTAL_TLV_ALLOCATION_SIZE);
@TheBlueMattTheBlueMattJan 30, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Should we default to no allocation here instead? If we expect these to rarely be used that'd save us an allocation and if we only ever have one TLV then we always only do one allocation anyway, so it'd only cost us an allocation if we have multiple TLVs.
@jkczyzjkczyzJan 31, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Sure, we can do that. Though the number of allocations will likely be at least two since we write the type and length before writing the value. Depends on how large the value is and how much the initial Vec allocation size is. Rust playground shows 8 bytes.
@TheBlueMattTheBlueMattJan 31, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Are you sure about that? TlvStream iterates over TlvRecords which implement Writeable with a single call to write_all.
@jkczyzjkczyzJan 31, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Ah, yeah, for experimental TLVs from the offer/invreq (i.e., in experimental_tlv_stream), that is true. For those that we set in the invoice (i.e., anything in experimental_invoice_tlv_stream), it would not be the case though.
@TheBlueMattTheBlueMattJan 31, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Mmm, hmmm, right. We could check if any of the supported experimental fields are set and allocate based on that? I can imagine someone having paid 100k BOLT12s, loading them in memory, and us wasting 48MB plus overhead for them...Basically this would just be a comment in ExperimentalInvoiceTlvStream that we should allocate if we add fields since we don't support any fields outside of test anyway.
@jkczyzjkczyzJan 31, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Updated

 let experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_TYPES);
 for record in experimental_tlv_stream {
 record.write(&mut experimental_bytes).unwrap();
 }

 experimental_invoice_tlv_stream.write(&mut experimental_bytes).unwrap();
 debug_assert_eq!(experimental_bytes.len(), experimental_bytes.capacity());

 let tlv_stream = TlvStream::new(&bytes).chain(TlvStream::new(&experimental_bytes));
 let tagged_hash = TaggedHash::from_tlv_stream(SIGNATURE_TAG, tlv_stream);
Expand DownExpand Up@@ -592,14 +574,6 @@ macro_rules! unsigned_invoice_sign_method { ($self: ident, $self_type: ty $(, $s
 signature_tlv_stream.write(&mut $self.bytes).unwrap();

 // Append the experimental bytes after the signature.
 debug_assert_eq!(
 // The two-byte overallocation results from SIGNATURE_TLV_RECORD_SIZE accommodating TLV
 // records with types >= 253.
 $self.bytes.len()
 + $self.experimental_bytes.len()
 + if $self.contents.is_for_offer() { 0 } else { 2 },
 $self.bytes.capacity(),
 );
 $self.bytes.extend_from_slice(&$self.experimental_bytes);

 Ok(Bolt12Invoice {
Expand DownExpand Up@@ -965,13 +939,6 @@ impl Hash for Bolt12Invoice {
}

impl InvoiceContents {
	fn is_for_offer(&self) -> bool {
 match self {
 InvoiceContents::ForOffer { .. } => true,
 InvoiceContents::ForRefund { .. } => false,
 }
	}

	/// Whether the original offer or refund has expired.
	#[cfg(feature = "std")]
	fn is_offer_or_refund_expired(&self) -> bool {
Expand DownExpand Up@@ -1362,7 +1329,11 @@ pub(super) const EXPERIMENTAL_INVOICE_TYPES: core::ops::RangeFrom<u64> = 3_000_0

#[cfg(not(test))]
tlv_stream!(
	ExperimentalInvoiceTlvStream, ExperimentalInvoiceTlvStreamRef, EXPERIMENTAL_INVOICE_TYPES, {}
	ExperimentalInvoiceTlvStream, ExperimentalInvoiceTlvStreamRef, EXPERIMENTAL_INVOICE_TYPES, {
 // When adding experimental TLVs, update EXPERIMENTAL_TLV_ALLOCATION_SIZE accordingly in
 // both UnsignedBolt12Invoice:new and UnsignedStaticInvoice::new to avoid unnecessary
 // allocations.
	}
);

#[cfg(test)]
Expand DownExpand Up@@ -2880,9 +2851,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice.bytes);
Expand DownExpand Up@@ -2917,9 +2885,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice.bytes);
Expand DownExpand Up@@ -2982,9 +2947,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice.bytes)
Expand DownExpand Up@@ -3021,9 +2983,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice.bytes)
Expand Down

61 changes: 11 additions & 50 deletions
lightning/src/offers/invoice_request.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -77,7 +77,7 @@ use crate::ln::channelmanager::PaymentId;
use crate::types::features::InvoiceRequestFeatures;
use crate::ln::inbound_payment::{ExpandedKey, IV_LEN};
use crate::ln::msgs::DecodeError;
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self, SIGNATURE_TLV_RECORD_SIZE};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self};
use crate::offers::nonce::Nonce;
use crate::offers::offer::{Amount, EXPERIMENTAL_OFFER_TYPES, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OFFER_TYPES, Offer, OfferContents, OfferId, OfferTlvStream, OfferTlvStreamRef};
use crate::offers::parse::{Bolt12ParseError, ParsedMessage, Bolt12SemanticError};
Expand DownExpand Up@@ -473,17 +473,8 @@ impl UnsignedInvoiceRequest {
 _experimental_offer_tlv_stream, experimental_invoice_request_tlv_stream,
 ) = contents.as_tlv_stream();

 // Allocate enough space for the invoice_request, which will include:
 // - all TLV records from `offer.bytes`,
 // - all invoice_request-specific TLV records, and
 // - a signature TLV record once the invoice_request is signed.
 let mut bytes = Vec::with_capacity(
 offer.bytes.len()
 + payer_tlv_stream.serialized_length()
 + invoice_request_tlv_stream.serialized_length()
 + SIGNATURE_TLV_RECORD_SIZE
 + experimental_invoice_request_tlv_stream.serialized_length(),
 );
 const INVOICE_REQUEST_ALLOCATION_SIZE: usize = 512;
 let mut bytes = Vec::with_capacity(INVOICE_REQUEST_ALLOCATION_SIZE);

 payer_tlv_stream.write(&mut bytes).unwrap();

Expand All@@ -495,23 +486,16 @@ impl UnsignedInvoiceRequest {

 invoice_request_tlv_stream.write(&mut bytes).unwrap();

 let mut experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_OFFER_TYPES)
 .peekable();
 let mut experimental_bytes = Vec::with_capacity(
 remaining_bytes.len()
 - experimental_tlv_stream
 .peek()
 .map_or(remaining_bytes.len(), |first_record| first_record.start)
 + experimental_invoice_request_tlv_stream.serialized_length(),
 );
 const EXPERIMENTAL_TLV_ALLOCATION_SIZE: usize = 0;
 let mut experimental_bytes = Vec::with_capacity(EXPERIMENTAL_TLV_ALLOCATION_SIZE);

 let experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_OFFER_TYPES);
 for record in experimental_tlv_stream {
 record.write(&mut experimental_bytes).unwrap();
 }

 experimental_invoice_request_tlv_stream.write(&mut experimental_bytes).unwrap();
 debug_assert_eq!(experimental_bytes.len(), experimental_bytes.capacity());

 let tlv_stream = TlvStream::new(&bytes).chain(TlvStream::new(&experimental_bytes));
 let tagged_hash = TaggedHash::from_tlv_stream(SIGNATURE_TAG, tlv_stream);
Expand DownExpand Up@@ -544,12 +528,6 @@ macro_rules! unsigned_invoice_request_sign_method { (
 signature_tlv_stream.write(&mut $self.bytes).unwrap();

 // Append the experimental bytes after the signature.
 debug_assert_eq!(
 // The two-byte overallocation results from SIGNATURE_TLV_RECORD_SIZE accommodating TLV
 // records with types >= 253.
 $self.bytes.len() + $self.experimental_bytes.len() + 2,
 $self.bytes.capacity(),
 );
 $self.bytes.extend_from_slice(&$self.experimental_bytes);

 Ok(InvoiceRequest {
Expand DownExpand Up@@ -1127,7 +1105,10 @@ pub(super) const EXPERIMENTAL_INVOICE_REQUEST_TYPES: core::ops::Range<u64> =
#[cfg(not(test))]
tlv_stream!(
	ExperimentalInvoiceRequestTlvStream, ExperimentalInvoiceRequestTlvStreamRef,
	EXPERIMENTAL_INVOICE_REQUEST_TYPES, {}
	EXPERIMENTAL_INVOICE_REQUEST_TYPES, {
 // When adding experimental TLVs, update EXPERIMENTAL_TLV_ALLOCATION_SIZE accordingly in
 // UnsignedInvoiceRequest::new to avoid unnecessary allocations.
	}
);

#[cfg(test)]
Expand DownExpand Up@@ -2422,11 +2403,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice_request.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice_request.bytes);
Expand DownExpand Up@@ -2460,11 +2436,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice_request.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice_request.bytes);
Expand DownExpand Up@@ -2508,11 +2479,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice_request.bytes)
Expand DownExpand Up@@ -2549,11 +2515,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice_request.bytes)
Expand Down

8 changes: 1 addition & 7 deletions
lightning/src/offers/merkle.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -11,7 +11,6 @@

use bitcoin::hashes::{Hash, HashEngine, sha256};
use bitcoin::secp256k1::{Message, PublicKey, Secp256k1, self};
use bitcoin::secp256k1::constants::SCHNORR_SIGNATURE_SIZE;
use bitcoin::secp256k1::schnorr::Signature;
use crate::io;
use crate::util::ser::{BigSize, Readable, Writeable, Writer};
Expand All@@ -26,10 +25,6 @@ tlv_stream!(SignatureTlvStream, SignatureTlvStreamRef<'a>, SIGNATURE_TYPES, {
	(240, signature: Signature),
});

/// Size of a TLV record in `SIGNATURE_TYPES` when the type is 1000. TLV types are encoded using
/// BigSize, so a TLV record with type 240 will use two less bytes.
pub(super) const SIGNATURE_TLV_RECORD_SIZE: usize = 3 + 1 + SCHNORR_SIGNATURE_SIZE;

/// A hash for use in a specific context by tweaking with a context-dependent tag as per [BIP 340]
/// and computed over the merkle root of a TLV stream to sign as defined in [BOLT 12].
///
Expand DownExpand Up@@ -253,7 +248,6 @@ pub(super) struct TlvRecord<'a> {
	type_bytes: &'a [u8],
	// The entire TLV record.
	pub(super) record_bytes: &'a [u8],
	pub(super) start: usize,
	pub(super) end: usize,
}

Expand All@@ -278,7 +272,7 @@ impl<'a> Iterator for TlvStream<'a> {
 self.data.set_position(end);

 Some(TlvRecord {
 r#type, type_bytes, record_bytes, start: start as usize, end: end as usize,
 r#type, type_bytes, record_bytes, end: end as usize,
 })
 } else {
 None
Expand Down

3 changes: 2 additions & 1 deletion
lightning/src/offers/offer.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -438,7 +438,8 @@ macro_rules! offer_builder_methods { (
 }
 }

 let mut bytes = Vec::new();
 const OFFER_ALLOCATION_SIZE: usize = 512;
 let mut bytes = Vec::with_capacity(OFFER_ALLOCATION_SIZE);
 $self.offer.write(&mut bytes).unwrap();

 let id = OfferId::from_valid_offer_tlv_stream(&bytes);
Expand Down

3 changes: 2 additions & 1 deletion
lightning/src/offers/refund.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -338,7 +338,8 @@ macro_rules! refund_builder_methods { (
 $self.refund.payer.0 = metadata;
 }

 let mut bytes = Vec::new();
 const REFUND_ALLOCATION_SIZE: usize = 512;
 let mut bytes = Vec::with_capacity(REFUND_ALLOCATION_SIZE);
 $self.refund.write(&mut bytes).unwrap();

 Ok(Refund {
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Remove or un-stick sticky/fixed headers that block content
(function() {
function unstick() {
document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) {
if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {
el.style.position = 'static';
el.style.top = 'auto';
el.style.zIndex = 'auto';
}
});
}
unstick();
var observer = new MutationObserver(unstick);
observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });
})();
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '
Don't over-allocate invoice bytes by jkczyz · Pull Request #3494 · lightningdevkit/rust-lightning · GitHub
Skip to content
Don't over-allocate invoice bytes#3494

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.

 Sign up for GitHub

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

 Merged

TheBlueMatt
merged 2 commits into

from

Feb 7, 2025
 Merged


Don't over-allocate invoice bytes
#3494
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
ffaccc0

Use fixed-size Vec allocations for BOLT12 messages


jkczyz Jan 28, 2025
0912b51

Drop use of reserve_exact in BOLT12 tests


jkczyz Feb 6, 2025

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


65 changes: 12 additions & 53 deletions
lightning/src/offers/invoice.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -122,7 +122,7 @@ use crate::offers::invoice_macros::{invoice_accessors_common, invoice_builder_me
#[cfg(test)]
use crate::offers::invoice_macros::invoice_builder_methods_test_common;
use crate::offers::invoice_request::{EXPERIMENTAL_INVOICE_REQUEST_TYPES, ExperimentalInvoiceRequestTlvStream, ExperimentalInvoiceRequestTlvStreamRef, INVOICE_REQUEST_PAYER_ID_TYPE, INVOICE_REQUEST_TYPES, IV_BYTES as INVOICE_REQUEST_IV_BYTES, InvoiceRequest, InvoiceRequestContents, InvoiceRequestTlvStream, InvoiceRequestTlvStreamRef};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self, SIGNATURE_TLV_RECORD_SIZE};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self};
use crate::offers::nonce::Nonce;
use crate::offers::offer::{Amount, EXPERIMENTAL_OFFER_TYPES, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OFFER_TYPES, OfferTlvStream, OfferTlvStreamRef, Quantity};
use crate::offers::parse::{Bolt12ParseError, Bolt12SemanticError, ParsedMessage};
Expand DownExpand Up@@ -520,19 +520,8 @@ impl UnsignedBolt12Invoice {
 let (_, _, _, invoice_tlv_stream, _, _, experimental_invoice_tlv_stream) =
 contents.as_tlv_stream();

 // Allocate enough space for the invoice, which will include:
 // - all TLV records from `invreq_bytes` except signatures,
 // - all invoice-specific TLV records, and
 // - a signature TLV record once the invoice is signed.
 //
 // This assumes both the invoice request and the invoice will each only have one signature
 // using SIGNATURE_TYPES.start as the TLV record. Thus, it is accounted for by invreq_bytes.
 let mut bytes = Vec::with_capacity(
 invreq_bytes.len()
 + invoice_tlv_stream.serialized_length()
 + if contents.is_for_offer() { 0 } else { SIGNATURE_TLV_RECORD_SIZE }
 + experimental_invoice_tlv_stream.serialized_length(),
 );
 const INVOICE_ALLOCATION_SIZE: usize = 1024;
 let mut bytes = Vec::with_capacity(INVOICE_ALLOCATION_SIZE);

 // Use the invoice_request bytes instead of the invoice_request TLV stream as the latter may
 // have contained unknown TLV records, which are not stored in `InvoiceRequestContents` or
Expand All@@ -545,23 +534,16 @@ impl UnsignedBolt12Invoice {

 invoice_tlv_stream.write(&mut bytes).unwrap();

 let mut experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_TYPES)
 .peekable();
 let mut experimental_bytes = Vec::with_capacity(
 remaining_bytes.len()
 - experimental_tlv_stream
 .peek()
 .map_or(remaining_bytes.len(), |first_record| first_record.start)
 + experimental_invoice_tlv_stream.serialized_length(),
 );
 const EXPERIMENTAL_TLV_ALLOCATION_SIZE: usize = 0;
 let mut experimental_bytes = Vec::with_capacity(EXPERIMENTAL_TLV_ALLOCATION_SIZE);
@TheBlueMattTheBlueMattJan 30, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Should we default to no allocation here instead? If we expect these to rarely be used that'd save us an allocation and if we only ever have one TLV then we always only do one allocation anyway, so it'd only cost us an allocation if we have multiple TLVs.
@jkczyzjkczyzJan 31, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Sure, we can do that. Though the number of allocations will likely be at least two since we write the type and length before writing the value. Depends on how large the value is and how much the initial Vec allocation size is. Rust playground shows 8 bytes.
@TheBlueMattTheBlueMattJan 31, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Are you sure about that? TlvStream iterates over TlvRecords which implement Writeable with a single call to write_all.
@jkczyzjkczyzJan 31, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Ah, yeah, for experimental TLVs from the offer/invreq (i.e., in experimental_tlv_stream), that is true. For those that we set in the invoice (i.e., anything in experimental_invoice_tlv_stream), it would not be the case though.
@TheBlueMattTheBlueMattJan 31, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Mmm, hmmm, right. We could check if any of the supported experimental fields are set and allocate based on that? I can imagine someone having paid 100k BOLT12s, loading them in memory, and us wasting 48MB plus overhead for them...Basically this would just be a comment in ExperimentalInvoiceTlvStream that we should allocate if we add fields since we don't support any fields outside of test anyway.
@jkczyzjkczyzJan 31, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Updated

 let experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_TYPES);
 for record in experimental_tlv_stream {
 record.write(&mut experimental_bytes).unwrap();
 }

 experimental_invoice_tlv_stream.write(&mut experimental_bytes).unwrap();
 debug_assert_eq!(experimental_bytes.len(), experimental_bytes.capacity());

 let tlv_stream = TlvStream::new(&bytes).chain(TlvStream::new(&experimental_bytes));
 let tagged_hash = TaggedHash::from_tlv_stream(SIGNATURE_TAG, tlv_stream);
Expand DownExpand Up@@ -592,14 +574,6 @@ macro_rules! unsigned_invoice_sign_method { ($self: ident, $self_type: ty $(, $s
 signature_tlv_stream.write(&mut $self.bytes).unwrap();

 // Append the experimental bytes after the signature.
 debug_assert_eq!(
 // The two-byte overallocation results from SIGNATURE_TLV_RECORD_SIZE accommodating TLV
 // records with types >= 253.
 $self.bytes.len()
 + $self.experimental_bytes.len()
 + if $self.contents.is_for_offer() { 0 } else { 2 },
 $self.bytes.capacity(),
 );
 $self.bytes.extend_from_slice(&$self.experimental_bytes);

 Ok(Bolt12Invoice {
Expand DownExpand Up@@ -965,13 +939,6 @@ impl Hash for Bolt12Invoice {
}

impl InvoiceContents {
	fn is_for_offer(&self) -> bool {
 match self {
 InvoiceContents::ForOffer { .. } => true,
 InvoiceContents::ForRefund { .. } => false,
 }
	}

	/// Whether the original offer or refund has expired.
	#[cfg(feature = "std")]
	fn is_offer_or_refund_expired(&self) -> bool {
Expand DownExpand Up@@ -1362,7 +1329,11 @@ pub(super) const EXPERIMENTAL_INVOICE_TYPES: core::ops::RangeFrom<u64> = 3_000_0

#[cfg(not(test))]
tlv_stream!(
	ExperimentalInvoiceTlvStream, ExperimentalInvoiceTlvStreamRef, EXPERIMENTAL_INVOICE_TYPES, {}
	ExperimentalInvoiceTlvStream, ExperimentalInvoiceTlvStreamRef, EXPERIMENTAL_INVOICE_TYPES, {
 // When adding experimental TLVs, update EXPERIMENTAL_TLV_ALLOCATION_SIZE accordingly in
 // both UnsignedBolt12Invoice:new and UnsignedStaticInvoice::new to avoid unnecessary
 // allocations.
	}
);

#[cfg(test)]
Expand DownExpand Up@@ -2880,9 +2851,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice.bytes);
Expand DownExpand Up@@ -2917,9 +2885,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice.bytes);
Expand DownExpand Up@@ -2982,9 +2947,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice.bytes)
Expand DownExpand Up@@ -3021,9 +2983,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice.bytes)
Expand Down

61 changes: 11 additions & 50 deletions
lightning/src/offers/invoice_request.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -77,7 +77,7 @@ use crate::ln::channelmanager::PaymentId;
use crate::types::features::InvoiceRequestFeatures;
use crate::ln::inbound_payment::{ExpandedKey, IV_LEN};
use crate::ln::msgs::DecodeError;
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self, SIGNATURE_TLV_RECORD_SIZE};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self};
use crate::offers::nonce::Nonce;
use crate::offers::offer::{Amount, EXPERIMENTAL_OFFER_TYPES, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OFFER_TYPES, Offer, OfferContents, OfferId, OfferTlvStream, OfferTlvStreamRef};
use crate::offers::parse::{Bolt12ParseError, ParsedMessage, Bolt12SemanticError};
Expand DownExpand Up@@ -473,17 +473,8 @@ impl UnsignedInvoiceRequest {
 _experimental_offer_tlv_stream, experimental_invoice_request_tlv_stream,
 ) = contents.as_tlv_stream();

 // Allocate enough space for the invoice_request, which will include:
 // - all TLV records from `offer.bytes`,
 // - all invoice_request-specific TLV records, and
 // - a signature TLV record once the invoice_request is signed.
 let mut bytes = Vec::with_capacity(
 offer.bytes.len()
 + payer_tlv_stream.serialized_length()
 + invoice_request_tlv_stream.serialized_length()
 + SIGNATURE_TLV_RECORD_SIZE
 + experimental_invoice_request_tlv_stream.serialized_length(),
 );
 const INVOICE_REQUEST_ALLOCATION_SIZE: usize = 512;
 let mut bytes = Vec::with_capacity(INVOICE_REQUEST_ALLOCATION_SIZE);

 payer_tlv_stream.write(&mut bytes).unwrap();

Expand All@@ -495,23 +486,16 @@ impl UnsignedInvoiceRequest {

 invoice_request_tlv_stream.write(&mut bytes).unwrap();

 let mut experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_OFFER_TYPES)
 .peekable();
 let mut experimental_bytes = Vec::with_capacity(
 remaining_bytes.len()
 - experimental_tlv_stream
 .peek()
 .map_or(remaining_bytes.len(), |first_record| first_record.start)
 + experimental_invoice_request_tlv_stream.serialized_length(),
 );
 const EXPERIMENTAL_TLV_ALLOCATION_SIZE: usize = 0;
 let mut experimental_bytes = Vec::with_capacity(EXPERIMENTAL_TLV_ALLOCATION_SIZE);

 let experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_OFFER_TYPES);
 for record in experimental_tlv_stream {
 record.write(&mut experimental_bytes).unwrap();
 }

 experimental_invoice_request_tlv_stream.write(&mut experimental_bytes).unwrap();
 debug_assert_eq!(experimental_bytes.len(), experimental_bytes.capacity());

 let tlv_stream = TlvStream::new(&bytes).chain(TlvStream::new(&experimental_bytes));
 let tagged_hash = TaggedHash::from_tlv_stream(SIGNATURE_TAG, tlv_stream);
Expand DownExpand Up@@ -544,12 +528,6 @@ macro_rules! unsigned_invoice_request_sign_method { (
 signature_tlv_stream.write(&mut $self.bytes).unwrap();

 // Append the experimental bytes after the signature.
 debug_assert_eq!(
 // The two-byte overallocation results from SIGNATURE_TLV_RECORD_SIZE accommodating TLV
 // records with types >= 253.
 $self.bytes.len() + $self.experimental_bytes.len() + 2,
 $self.bytes.capacity(),
 );
 $self.bytes.extend_from_slice(&$self.experimental_bytes);

 Ok(InvoiceRequest {
Expand DownExpand Up@@ -1127,7 +1105,10 @@ pub(super) const EXPERIMENTAL_INVOICE_REQUEST_TYPES: core::ops::Range<u64> =
#[cfg(not(test))]
tlv_stream!(
	ExperimentalInvoiceRequestTlvStream, ExperimentalInvoiceRequestTlvStreamRef,
	EXPERIMENTAL_INVOICE_REQUEST_TYPES, {}
	EXPERIMENTAL_INVOICE_REQUEST_TYPES, {
 // When adding experimental TLVs, update EXPERIMENTAL_TLV_ALLOCATION_SIZE accordingly in
 // UnsignedInvoiceRequest::new to avoid unnecessary allocations.
	}
);

#[cfg(test)]
Expand DownExpand Up@@ -2422,11 +2403,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice_request.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice_request.bytes);
Expand DownExpand Up@@ -2460,11 +2436,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice_request.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice_request.bytes);
Expand DownExpand Up@@ -2508,11 +2479,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice_request.bytes)
Expand DownExpand Up@@ -2549,11 +2515,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice_request.bytes)
Expand Down

8 changes: 1 addition & 7 deletions
lightning/src/offers/merkle.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -11,7 +11,6 @@

use bitcoin::hashes::{Hash, HashEngine, sha256};
use bitcoin::secp256k1::{Message, PublicKey, Secp256k1, self};
use bitcoin::secp256k1::constants::SCHNORR_SIGNATURE_SIZE;
use bitcoin::secp256k1::schnorr::Signature;
use crate::io;
use crate::util::ser::{BigSize, Readable, Writeable, Writer};
Expand All@@ -26,10 +25,6 @@ tlv_stream!(SignatureTlvStream, SignatureTlvStreamRef<'a>, SIGNATURE_TYPES, {
	(240, signature: Signature),
});

/// Size of a TLV record in `SIGNATURE_TYPES` when the type is 1000. TLV types are encoded using
/// BigSize, so a TLV record with type 240 will use two less bytes.
pub(super) const SIGNATURE_TLV_RECORD_SIZE: usize = 3 + 1 + SCHNORR_SIGNATURE_SIZE;

/// A hash for use in a specific context by tweaking with a context-dependent tag as per [BIP 340]
/// and computed over the merkle root of a TLV stream to sign as defined in [BOLT 12].
///
Expand DownExpand Up@@ -253,7 +248,6 @@ pub(super) struct TlvRecord<'a> {
	type_bytes: &'a [u8],
	// The entire TLV record.
	pub(super) record_bytes: &'a [u8],
	pub(super) start: usize,
	pub(super) end: usize,
}

Expand All@@ -278,7 +272,7 @@ impl<'a> Iterator for TlvStream<'a> {
 self.data.set_position(end);

 Some(TlvRecord {
 r#type, type_bytes, record_bytes, start: start as usize, end: end as usize,
 r#type, type_bytes, record_bytes, end: end as usize,
 })
 } else {
 None
Expand Down

3 changes: 2 additions & 1 deletion
lightning/src/offers/offer.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -438,7 +438,8 @@ macro_rules! offer_builder_methods { (
 }
 }

 let mut bytes = Vec::new();
 const OFFER_ALLOCATION_SIZE: usize = 512;
 let mut bytes = Vec::with_capacity(OFFER_ALLOCATION_SIZE);
 $self.offer.write(&mut bytes).unwrap();

 let id = OfferId::from_valid_offer_tlv_stream(&bytes);
Expand Down

3 changes: 2 additions & 1 deletion
lightning/src/offers/refund.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -338,7 +338,8 @@ macro_rules! refund_builder_methods { (
 $self.refund.payer.0 = metadata;
 }

 let mut bytes = Vec::new();
 const REFUND_ALLOCATION_SIZE: usize = 512;
 let mut bytes = Vec::with_capacity(REFUND_ALLOCATION_SIZE);
 $self.refund.write(&mut bytes).unwrap();

 Ok(Refund {
Expand Down
Loading

, 'i');
if (__m === '*' || __re.test(location.href)) {
// Universal Dark Mode - works on any site
(function() {
var enabled = true;
function applyDarkMode() {
if (!enabled) return;
// Create style element if it doesn't exist
var style = document.getElementById('universal-dark-mode-style');
if (!style) {
style = document.createElement('style');
style.id = 'universal-dark-mode-style';
document.head.appendChild(style);
}
// Dark mode CSS - inverts colors but preserves images/video
style.textContent = '
/* Invert everything except media */
html {
filter: invert(1) hue-rotate(180deg) !important;
background: #1a1a2e !important;
}
/* Restore images, videos, iframes, canvas */
img, video, iframe, canvas, svg, picture, [style*="background-image"] {
filter: invert(1) hue-rotate(180deg) !important;
}
/* Preserve specific elements that should not be inverted */
.no-dark-mode, .no-dark-mode *,
[data-theme="light"], [data-theme="light"],
.ace_editor, .ace_editor *,
.CodeMirror, .CodeMirror *,
.monaco-editor, .monaco-editor *,
.markdown-body pre, .markdown-body pre *,
.highlight, .highlight *,
pre code, pre code * {
filter: none !important;
}
/* Fix common UI elements */
.modal, .popup, .dropdown-menu, .tooltip, .popover {
filter: invert(1) hue-rotate(180deg) !important;
background: #2d2d44 !important;
border-color: #444 !important;
}
/* Scrollbars */
::-webkit-scrollbar { background: #1a1a2e !important; }
::-webkit-scrollbar-thumb { background: #444 !important; }
::-webkit-scrollbar-thumb:hover { background: #555 !important; }
/* Selection */
::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }
';
}
function removeDarkMode() {
var style = document.getElementById('universal-dark-mode-style');
if (style) style.remove();
}
// Toggle with Alt+Shift+D
document.addEventListener('keydown', function(e) {
if (e.altKey && e.shiftKey && e.key === 'D') {
e.preventDefault();
enabled = !enabled;
if (enabled) {
applyDarkMode();
console.log('[Universal Dark Mode] Enabled');
} else {
removeDarkMode();
console.log('[Universal Dark Mode] Disabled');
}
}
});
// Apply on load
applyDarkMode();
// Re-apply on dynamic content
var observer = new MutationObserver(function(mutations) {
if (enabled && !document.getElementById('universal-dark-mode-style')) {
applyDarkMode();
}
});
observer.observe(document.head, { childList: true });
console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');
})();
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();
Don't over-allocate invoice bytes by jkczyz · Pull Request #3494 · lightningdevkit/rust-lightning · GitHub
Skip to content
Don't over-allocate invoice bytes#3494

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.

 Sign up for GitHub

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

 Merged

TheBlueMatt
merged 2 commits into

from

Feb 7, 2025
 Merged


Don't over-allocate invoice bytes
#3494
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


2 commits


Select commit
Hold shift + click to select a range
ffaccc0

Use fixed-size Vec allocations for BOLT12 messages


jkczyz Jan 28, 2025
0912b51

Drop use of reserve_exact in BOLT12 tests


jkczyz Feb 6, 2025

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


65 changes: 12 additions & 53 deletions
lightning/src/offers/invoice.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -122,7 +122,7 @@ use crate::offers::invoice_macros::{invoice_accessors_common, invoice_builder_me
#[cfg(test)]
use crate::offers::invoice_macros::invoice_builder_methods_test_common;
use crate::offers::invoice_request::{EXPERIMENTAL_INVOICE_REQUEST_TYPES, ExperimentalInvoiceRequestTlvStream, ExperimentalInvoiceRequestTlvStreamRef, INVOICE_REQUEST_PAYER_ID_TYPE, INVOICE_REQUEST_TYPES, IV_BYTES as INVOICE_REQUEST_IV_BYTES, InvoiceRequest, InvoiceRequestContents, InvoiceRequestTlvStream, InvoiceRequestTlvStreamRef};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self, SIGNATURE_TLV_RECORD_SIZE};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self};
use crate::offers::nonce::Nonce;
use crate::offers::offer::{Amount, EXPERIMENTAL_OFFER_TYPES, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OFFER_TYPES, OfferTlvStream, OfferTlvStreamRef, Quantity};
use crate::offers::parse::{Bolt12ParseError, Bolt12SemanticError, ParsedMessage};
Expand DownExpand Up@@ -520,19 +520,8 @@ impl UnsignedBolt12Invoice {
 let (_, _, _, invoice_tlv_stream, _, _, experimental_invoice_tlv_stream) =
 contents.as_tlv_stream();

 // Allocate enough space for the invoice, which will include:
 // - all TLV records from `invreq_bytes` except signatures,
 // - all invoice-specific TLV records, and
 // - a signature TLV record once the invoice is signed.
 //
 // This assumes both the invoice request and the invoice will each only have one signature
 // using SIGNATURE_TYPES.start as the TLV record. Thus, it is accounted for by invreq_bytes.
 let mut bytes = Vec::with_capacity(
 invreq_bytes.len()
 + invoice_tlv_stream.serialized_length()
 + if contents.is_for_offer() { 0 } else { SIGNATURE_TLV_RECORD_SIZE }
 + experimental_invoice_tlv_stream.serialized_length(),
 );
 const INVOICE_ALLOCATION_SIZE: usize = 1024;
 let mut bytes = Vec::with_capacity(INVOICE_ALLOCATION_SIZE);

 // Use the invoice_request bytes instead of the invoice_request TLV stream as the latter may
 // have contained unknown TLV records, which are not stored in `InvoiceRequestContents` or
Expand All@@ -545,23 +534,16 @@ impl UnsignedBolt12Invoice {

 invoice_tlv_stream.write(&mut bytes).unwrap();

 let mut experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_TYPES)
 .peekable();
 let mut experimental_bytes = Vec::with_capacity(
 remaining_bytes.len()
 - experimental_tlv_stream
 .peek()
 .map_or(remaining_bytes.len(), |first_record| first_record.start)
 + experimental_invoice_tlv_stream.serialized_length(),
 );
 const EXPERIMENTAL_TLV_ALLOCATION_SIZE: usize = 0;
 let mut experimental_bytes = Vec::with_capacity(EXPERIMENTAL_TLV_ALLOCATION_SIZE);
@TheBlueMattTheBlueMattJan 30, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Should we default to no allocation here instead? If we expect these to rarely be used that'd save us an allocation and if we only ever have one TLV then we always only do one allocation anyway, so it'd only cost us an allocation if we have multiple TLVs.
@jkczyzjkczyzJan 31, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Sure, we can do that. Though the number of allocations will likely be at least two since we write the type and length before writing the value. Depends on how large the value is and how much the initial Vec allocation size is. Rust playground shows 8 bytes.
@TheBlueMattTheBlueMattJan 31, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Are you sure about that? TlvStream iterates over TlvRecords which implement Writeable with a single call to write_all.
@jkczyzjkczyzJan 31, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Ah, yeah, for experimental TLVs from the offer/invreq (i.e., in experimental_tlv_stream), that is true. For those that we set in the invoice (i.e., anything in experimental_invoice_tlv_stream), it would not be the case though.
@TheBlueMattTheBlueMattJan 31, 2025

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

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

Mmm, hmmm, right. We could check if any of the supported experimental fields are set and allocate based on that? I can imagine someone having paid 100k BOLT12s, loading them in memory, and us wasting 48MB plus overhead for them...Basically this would just be a comment in ExperimentalInvoiceTlvStream that we should allocate if we add fields since we don't support any fields outside of test anyway.
@jkczyzjkczyzJan 31, 2025

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

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

Updated

 let experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_TYPES);
 for record in experimental_tlv_stream {
 record.write(&mut experimental_bytes).unwrap();
 }

 experimental_invoice_tlv_stream.write(&mut experimental_bytes).unwrap();
 debug_assert_eq!(experimental_bytes.len(), experimental_bytes.capacity());

 let tlv_stream = TlvStream::new(&bytes).chain(TlvStream::new(&experimental_bytes));
 let tagged_hash = TaggedHash::from_tlv_stream(SIGNATURE_TAG, tlv_stream);
Expand DownExpand Up@@ -592,14 +574,6 @@ macro_rules! unsigned_invoice_sign_method { ($self: ident, $self_type: ty $(, $s
 signature_tlv_stream.write(&mut $self.bytes).unwrap();

 // Append the experimental bytes after the signature.
 debug_assert_eq!(
 // The two-byte overallocation results from SIGNATURE_TLV_RECORD_SIZE accommodating TLV
 // records with types >= 253.
 $self.bytes.len()
 + $self.experimental_bytes.len()
 + if $self.contents.is_for_offer() { 0 } else { 2 },
 $self.bytes.capacity(),
 );
 $self.bytes.extend_from_slice(&$self.experimental_bytes);

 Ok(Bolt12Invoice {
Expand DownExpand Up@@ -965,13 +939,6 @@ impl Hash for Bolt12Invoice {
}

impl InvoiceContents {
	fn is_for_offer(&self) -> bool {
 match self {
 InvoiceContents::ForOffer { .. } => true,
 InvoiceContents::ForRefund { .. } => false,
 }
	}

	/// Whether the original offer or refund has expired.
	#[cfg(feature = "std")]
	fn is_offer_or_refund_expired(&self) -> bool {
Expand DownExpand Up@@ -1362,7 +1329,11 @@ pub(super) const EXPERIMENTAL_INVOICE_TYPES: core::ops::RangeFrom<u64> = 3_000_0

#[cfg(not(test))]
tlv_stream!(
	ExperimentalInvoiceTlvStream, ExperimentalInvoiceTlvStreamRef, EXPERIMENTAL_INVOICE_TYPES, {}
	ExperimentalInvoiceTlvStream, ExperimentalInvoiceTlvStreamRef, EXPERIMENTAL_INVOICE_TYPES, {
 // When adding experimental TLVs, update EXPERIMENTAL_TLV_ALLOCATION_SIZE accordingly in
 // both UnsignedBolt12Invoice:new and UnsignedStaticInvoice::new to avoid unnecessary
 // allocations.
	}
);

#[cfg(test)]
Expand DownExpand Up@@ -2880,9 +2851,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice.bytes);
Expand DownExpand Up@@ -2917,9 +2885,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice.bytes);
Expand DownExpand Up@@ -2982,9 +2947,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice.bytes)
Expand DownExpand Up@@ -3021,9 +2983,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice.bytes.reserve_exact(
 unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
 );
 unsigned_invoice.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice.bytes)
Expand Down

61 changes: 11 additions & 50 deletions
lightning/src/offers/invoice_request.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -77,7 +77,7 @@ use crate::ln::channelmanager::PaymentId;
use crate::types::features::InvoiceRequestFeatures;
use crate::ln::inbound_payment::{ExpandedKey, IV_LEN};
use crate::ln::msgs::DecodeError;
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self, SIGNATURE_TLV_RECORD_SIZE};
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self};
use crate::offers::nonce::Nonce;
use crate::offers::offer::{Amount, EXPERIMENTAL_OFFER_TYPES, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OFFER_TYPES, Offer, OfferContents, OfferId, OfferTlvStream, OfferTlvStreamRef};
use crate::offers::parse::{Bolt12ParseError, ParsedMessage, Bolt12SemanticError};
Expand DownExpand Up@@ -473,17 +473,8 @@ impl UnsignedInvoiceRequest {
 _experimental_offer_tlv_stream, experimental_invoice_request_tlv_stream,
 ) = contents.as_tlv_stream();

 // Allocate enough space for the invoice_request, which will include:
 // - all TLV records from `offer.bytes`,
 // - all invoice_request-specific TLV records, and
 // - a signature TLV record once the invoice_request is signed.
 let mut bytes = Vec::with_capacity(
 offer.bytes.len()
 + payer_tlv_stream.serialized_length()
 + invoice_request_tlv_stream.serialized_length()
 + SIGNATURE_TLV_RECORD_SIZE
 + experimental_invoice_request_tlv_stream.serialized_length(),
 );
 const INVOICE_REQUEST_ALLOCATION_SIZE: usize = 512;
 let mut bytes = Vec::with_capacity(INVOICE_REQUEST_ALLOCATION_SIZE);

 payer_tlv_stream.write(&mut bytes).unwrap();

Expand All@@ -495,23 +486,16 @@ impl UnsignedInvoiceRequest {

 invoice_request_tlv_stream.write(&mut bytes).unwrap();

 let mut experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_OFFER_TYPES)
 .peekable();
 let mut experimental_bytes = Vec::with_capacity(
 remaining_bytes.len()
 - experimental_tlv_stream
 .peek()
 .map_or(remaining_bytes.len(), |first_record| first_record.start)
 + experimental_invoice_request_tlv_stream.serialized_length(),
 );
 const EXPERIMENTAL_TLV_ALLOCATION_SIZE: usize = 0;
 let mut experimental_bytes = Vec::with_capacity(EXPERIMENTAL_TLV_ALLOCATION_SIZE);

 let experimental_tlv_stream = TlvStream::new(remaining_bytes)
 .range(EXPERIMENTAL_OFFER_TYPES);
 for record in experimental_tlv_stream {
 record.write(&mut experimental_bytes).unwrap();
 }

 experimental_invoice_request_tlv_stream.write(&mut experimental_bytes).unwrap();
 debug_assert_eq!(experimental_bytes.len(), experimental_bytes.capacity());

 let tlv_stream = TlvStream::new(&bytes).chain(TlvStream::new(&experimental_bytes));
 let tagged_hash = TaggedHash::from_tlv_stream(SIGNATURE_TAG, tlv_stream);
Expand DownExpand Up@@ -544,12 +528,6 @@ macro_rules! unsigned_invoice_request_sign_method { (
 signature_tlv_stream.write(&mut $self.bytes).unwrap();

 // Append the experimental bytes after the signature.
 debug_assert_eq!(
 // The two-byte overallocation results from SIGNATURE_TLV_RECORD_SIZE accommodating TLV
 // records with types >= 253.
 $self.bytes.len() + $self.experimental_bytes.len() + 2,
 $self.bytes.capacity(),
 );
 $self.bytes.extend_from_slice(&$self.experimental_bytes);

 Ok(InvoiceRequest {
Expand DownExpand Up@@ -1127,7 +1105,10 @@ pub(super) const EXPERIMENTAL_INVOICE_REQUEST_TYPES: core::ops::Range<u64> =
#[cfg(not(test))]
tlv_stream!(
	ExperimentalInvoiceRequestTlvStream, ExperimentalInvoiceRequestTlvStreamRef,
	EXPERIMENTAL_INVOICE_REQUEST_TYPES, {}
	EXPERIMENTAL_INVOICE_REQUEST_TYPES, {
 // When adding experimental TLVs, update EXPERIMENTAL_TLV_ALLOCATION_SIZE accordingly in
 // UnsignedInvoiceRequest::new to avoid unnecessary allocations.
	}
);

#[cfg(test)]
Expand DownExpand Up@@ -2422,11 +2403,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice_request.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice_request.bytes);
Expand DownExpand Up@@ -2460,11 +2436,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.bytes.extend_from_slice(&unknown_bytes);
 unsigned_invoice_request.tagged_hash =
 TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice_request.bytes);
Expand DownExpand Up@@ -2508,11 +2479,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice_request.bytes)
Expand DownExpand Up@@ -2549,11 +2515,6 @@ mod tests {
 BigSize(32).write(&mut unknown_bytes).unwrap();
 [42u8; 32].write(&mut unknown_bytes).unwrap();

 unsigned_invoice_request.bytes.reserve_exact(
 unsigned_invoice_request.bytes.capacity()
 - unsigned_invoice_request.bytes.len()
 + unknown_bytes.len(),
 );
 unsigned_invoice_request.experimental_bytes.extend_from_slice(&unknown_bytes);

 let tlv_stream = TlvStream::new(&unsigned_invoice_request.bytes)
Expand Down

8 changes: 1 addition & 7 deletions
lightning/src/offers/merkle.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -11,7 +11,6 @@

use bitcoin::hashes::{Hash, HashEngine, sha256};
use bitcoin::secp256k1::{Message, PublicKey, Secp256k1, self};
use bitcoin::secp256k1::constants::SCHNORR_SIGNATURE_SIZE;
use bitcoin::secp256k1::schnorr::Signature;
use crate::io;
use crate::util::ser::{BigSize, Readable, Writeable, Writer};
Expand All@@ -26,10 +25,6 @@ tlv_stream!(SignatureTlvStream, SignatureTlvStreamRef<'a>, SIGNATURE_TYPES, {
	(240, signature: Signature),
});

/// Size of a TLV record in `SIGNATURE_TYPES` when the type is 1000. TLV types are encoded using
/// BigSize, so a TLV record with type 240 will use two less bytes.
pub(super) const SIGNATURE_TLV_RECORD_SIZE: usize = 3 + 1 + SCHNORR_SIGNATURE_SIZE;

/// A hash for use in a specific context by tweaking with a context-dependent tag as per [BIP 340]
/// and computed over the merkle root of a TLV stream to sign as defined in [BOLT 12].
///
Expand DownExpand Up@@ -253,7 +248,6 @@ pub(super) struct TlvRecord<'a> {
	type_bytes: &'a [u8],
	// The entire TLV record.
	pub(super) record_bytes: &'a [u8],
	pub(super) start: usize,
	pub(super) end: usize,
}

Expand All@@ -278,7 +272,7 @@ impl<'a> Iterator for TlvStream<'a> {
 self.data.set_position(end);

 Some(TlvRecord {
 r#type, type_bytes, record_bytes, start: start as usize, end: end as usize,
 r#type, type_bytes, record_bytes, end: end as usize,
 })
 } else {
 None
Expand Down

3 changes: 2 additions & 1 deletion
lightning/src/offers/offer.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -438,7 +438,8 @@ macro_rules! offer_builder_methods { (
 }
 }

 let mut bytes = Vec::new();
 const OFFER_ALLOCATION_SIZE: usize = 512;
 let mut bytes = Vec::with_capacity(OFFER_ALLOCATION_SIZE);
 $self.offer.write(&mut bytes).unwrap();

 let id = OfferId::from_valid_offer_tlv_stream(&bytes);
Expand Down

3 changes: 2 additions & 1 deletion
lightning/src/offers/refund.rs

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -338,7 +338,8 @@ macro_rules! refund_builder_methods { (
 $self.refund.payer.0 = metadata;
 }

 let mut bytes = Vec::new();
 const REFUND_ALLOCATION_SIZE: usize = 512;
 let mut bytes = Vec::with_capacity(REFUND_ALLOCATION_SIZE);
 $self.refund.write(&mut bytes).unwrap();

 Ok(Refund {
Expand Down
Loading