Skip to content

Commit f045c0e

Browse files
TheBlueMattTheBlueMatt
authored
Merge pull request #3494 from jkczyz/2024-12-invoice-byte-allocation
Don't over-allocate invoice bytes
2 parents 8e90841 + 0912b51 commit f045c0e

File tree

6 files changed

+34
-151
lines changed

6 files changed

+34
-151
lines changed

‎lightning/src/offers/invoice.rs

+12-53
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ use crate::offers::invoice_macros::{invoice_accessors_common, invoice_builder_me
122122
#[cfg(test)]
123123
use crate::offers::invoice_macros::invoice_builder_methods_test_common;
124124
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};
125-
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self, SIGNATURE_TLV_RECORD_SIZE};
125+
use crate::offers::merkle::{SignError, SignFn, SignatureTlvStream, SignatureTlvStreamRef, TaggedHash, TlvStream, self};
126126
use crate::offers::nonce::Nonce;
127127
use crate::offers::offer::{Amount, EXPERIMENTAL_OFFER_TYPES, ExperimentalOfferTlvStream, ExperimentalOfferTlvStreamRef, OFFER_TYPES, OfferTlvStream, OfferTlvStreamRef, Quantity};
128128
use crate::offers::parse::{Bolt12ParseError, Bolt12SemanticError, ParsedMessage};
@@ -520,19 +520,8 @@ impl UnsignedBolt12Invoice {
520520
let (_, _, _, invoice_tlv_stream, _, _, experimental_invoice_tlv_stream) =
521521
contents.as_tlv_stream();
522522

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

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

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

548-
let mut experimental_tlv_stream = TlvStream::new(remaining_bytes)
549-
.range(EXPERIMENTAL_TYPES)
550-
.peekable();
551-
let mut experimental_bytes = Vec::with_capacity(
552-
remaining_bytes.len()
553-
- experimental_tlv_stream
554-
.peek()
555-
.map_or(remaining_bytes.len(), |first_record| first_record.start)
556-
+ experimental_invoice_tlv_stream.serialized_length(),
557-
);
537+
const EXPERIMENTAL_TLV_ALLOCATION_SIZE: usize = 0;
538+
let mut experimental_bytes = Vec::with_capacity(EXPERIMENTAL_TLV_ALLOCATION_SIZE);
558539

540+
let experimental_tlv_stream = TlvStream::new(remaining_bytes)
541+
.range(EXPERIMENTAL_TYPES);
559542
for record in experimental_tlv_stream {
560543
record.write(&mut experimental_bytes).unwrap();
561544
}
562545

563546
experimental_invoice_tlv_stream.write(&mut experimental_bytes).unwrap();
564-
debug_assert_eq!(experimental_bytes.len(), experimental_bytes.capacity());
565547

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

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

605579
Ok(Bolt12Invoice {
@@ -965,13 +939,6 @@ impl Hash for Bolt12Invoice {
965939
}
966940

967941
impl InvoiceContents {
968-
fn is_for_offer(&self) -> bool {
969-
match self {
970-
InvoiceContents::ForOffer { .. } => true,
971-
InvoiceContents::ForRefund { .. } => false,
972-
}
973-
}
974-
975942
/// Whether the original offer or refund has expired.
976943
#[cfg(feature = "std")]
977944
fn is_offer_or_refund_expired(&self) -> bool {
@@ -1362,7 +1329,11 @@ pub(super) const EXPERIMENTAL_INVOICE_TYPES: core::ops::RangeFrom<u64> = 3_000_0
13621329

13631330
#[cfg(not(test))]
13641331
tlv_stream!(
1365-
ExperimentalInvoiceTlvStream, ExperimentalInvoiceTlvStreamRef, EXPERIMENTAL_INVOICE_TYPES, {}
1332+
ExperimentalInvoiceTlvStream, ExperimentalInvoiceTlvStreamRef, EXPERIMENTAL_INVOICE_TYPES, {
1333+
// When adding experimental TLVs, update EXPERIMENTAL_TLV_ALLOCATION_SIZE accordingly in
1334+
// both UnsignedBolt12Invoice:new and UnsignedStaticInvoice::new to avoid unnecessary
1335+
// allocations.
1336+
}
13661337
);
13671338

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

2883-
unsigned_invoice.bytes.reserve_exact(
2884-
unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
2885-
);
28862854
unsigned_invoice.bytes.extend_from_slice(&unknown_bytes);
28872855
unsigned_invoice.tagged_hash =
28882856
TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice.bytes);
@@ -2917,9 +2885,6 @@ mod tests {
29172885
BigSize(32).write(&mut unknown_bytes).unwrap();
29182886
[42u8; 32].write(&mut unknown_bytes).unwrap();
29192887

2920-
unsigned_invoice.bytes.reserve_exact(
2921-
unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
2922-
);
29232888
unsigned_invoice.bytes.extend_from_slice(&unknown_bytes);
29242889
unsigned_invoice.tagged_hash =
29252890
TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice.bytes);
@@ -2982,9 +2947,6 @@ mod tests {
29822947
BigSize(32).write(&mut unknown_bytes).unwrap();
29832948
[42u8; 32].write(&mut unknown_bytes).unwrap();
29842949

2985-
unsigned_invoice.bytes.reserve_exact(
2986-
unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
2987-
);
29882950
unsigned_invoice.experimental_bytes.extend_from_slice(&unknown_bytes);
29892951

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

3024-
unsigned_invoice.bytes.reserve_exact(
3025-
unsigned_invoice.bytes.capacity() - unsigned_invoice.bytes.len() + unknown_bytes.len(),
3026-
);
30272986
unsigned_invoice.experimental_bytes.extend_from_slice(&unknown_bytes);
30282987

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

‎lightning/src/offers/invoice_request.rs

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

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

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

@@ -495,23 +486,16 @@ impl UnsignedInvoiceRequest {
495486

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

498-
let mut experimental_tlv_stream = TlvStream::new(remaining_bytes)
499-
.range(EXPERIMENTAL_OFFER_TYPES)
500-
.peekable();
501-
let mut experimental_bytes = Vec::with_capacity(
502-
remaining_bytes.len()
503-
- experimental_tlv_stream
504-
.peek()
505-
.map_or(remaining_bytes.len(), |first_record| first_record.start)
506-
+ experimental_invoice_request_tlv_stream.serialized_length(),
507-
);
489+
const EXPERIMENTAL_TLV_ALLOCATION_SIZE: usize = 0;
490+
let mut experimental_bytes = Vec::with_capacity(EXPERIMENTAL_TLV_ALLOCATION_SIZE);
508491

492+
let experimental_tlv_stream = TlvStream::new(remaining_bytes)
493+
.range(EXPERIMENTAL_OFFER_TYPES);
509494
for record in experimental_tlv_stream {
510495
record.write(&mut experimental_bytes).unwrap();
511496
}
512497

513498
experimental_invoice_request_tlv_stream.write(&mut experimental_bytes).unwrap();
514-
debug_assert_eq!(experimental_bytes.len(), experimental_bytes.capacity());
515499

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

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

555533
Ok(InvoiceRequest {
@@ -1127,7 +1105,10 @@ pub(super) const EXPERIMENTAL_INVOICE_REQUEST_TYPES: core::ops::Range<u64> =
11271105
#[cfg(not(test))]
11281106
tlv_stream!(
11291107
ExperimentalInvoiceRequestTlvStream, ExperimentalInvoiceRequestTlvStreamRef,
1130-
EXPERIMENTAL_INVOICE_REQUEST_TYPES, {}
1108+
EXPERIMENTAL_INVOICE_REQUEST_TYPES, {
1109+
// When adding experimental TLVs, update EXPERIMENTAL_TLV_ALLOCATION_SIZE accordingly in
1110+
// UnsignedInvoiceRequest::new to avoid unnecessary allocations.
1111+
}
11311112
);
11321113

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

2425-
unsigned_invoice_request.bytes.reserve_exact(
2426-
unsigned_invoice_request.bytes.capacity()
2427-
- unsigned_invoice_request.bytes.len()
2428-
+ unknown_bytes.len(),
2429-
);
24302406
unsigned_invoice_request.bytes.extend_from_slice(&unknown_bytes);
24312407
unsigned_invoice_request.tagged_hash =
24322408
TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice_request.bytes);
@@ -2460,11 +2436,6 @@ mod tests {
24602436
BigSize(32).write(&mut unknown_bytes).unwrap();
24612437
[42u8; 32].write(&mut unknown_bytes).unwrap();
24622438

2463-
unsigned_invoice_request.bytes.reserve_exact(
2464-
unsigned_invoice_request.bytes.capacity()
2465-
- unsigned_invoice_request.bytes.len()
2466-
+ unknown_bytes.len(),
2467-
);
24682439
unsigned_invoice_request.bytes.extend_from_slice(&unknown_bytes);
24692440
unsigned_invoice_request.tagged_hash =
24702441
TaggedHash::from_valid_tlv_stream_bytes(SIGNATURE_TAG, &unsigned_invoice_request.bytes);
@@ -2508,11 +2479,6 @@ mod tests {
25082479
BigSize(32).write(&mut unknown_bytes).unwrap();
25092480
[42u8; 32].write(&mut unknown_bytes).unwrap();
25102481

2511-
unsigned_invoice_request.bytes.reserve_exact(
2512-
unsigned_invoice_request.bytes.capacity()
2513-
- unsigned_invoice_request.bytes.len()
2514-
+ unknown_bytes.len(),
2515-
);
25162482
unsigned_invoice_request.experimental_bytes.extend_from_slice(&unknown_bytes);
25172483

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

2552-
unsigned_invoice_request.bytes.reserve_exact(
2553-
unsigned_invoice_request.bytes.capacity()
2554-
- unsigned_invoice_request.bytes.len()
2555-
+ unknown_bytes.len(),
2556-
);
25572518
unsigned_invoice_request.experimental_bytes.extend_from_slice(&unknown_bytes);
25582519

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

‎lightning/src/offers/merkle.rs

+1-7
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
1212
use bitcoin::hashes::{Hash, HashEngine, sha256};
1313
use bitcoin::secp256k1::{Message, PublicKey, Secp256k1, self};
14-
use bitcoin::secp256k1::constants::SCHNORR_SIGNATURE_SIZE;
1514
use bitcoin::secp256k1::schnorr::Signature;
1615
use crate::io;
1716
use crate::util::ser::{BigSize, Readable, Writeable, Writer};
@@ -26,10 +25,6 @@ tlv_stream!(SignatureTlvStream, SignatureTlvStreamRef<'a>, SIGNATURE_TYPES, {
2625
(240, signature: Signature),
2726
});
2827

29-
/// Size of a TLV record in `SIGNATURE_TYPES` when the type is 1000. TLV types are encoded using
30-
/// BigSize, so a TLV record with type 240 will use two less bytes.
31-
pub(super) const SIGNATURE_TLV_RECORD_SIZE: usize = 3 + 1 + SCHNORR_SIGNATURE_SIZE;
32-
3328
/// A hash for use in a specific context by tweaking with a context-dependent tag as per [BIP 340]
3429
/// and computed over the merkle root of a TLV stream to sign as defined in [BOLT 12].
3530
///
@@ -253,7 +248,6 @@ pub(super) struct TlvRecord<'a> {
253248
type_bytes: &'a [u8],
254249
// The entire TLV record.
255250
pub(super) record_bytes: &'a [u8],
256-
pub(super) start: usize,
257251
pub(super) end: usize,
258252
}
259253

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

280274
Some(TlvRecord {
281-
r#type, type_bytes, record_bytes, start: start as usize, end: end as usize,
275+
r#type, type_bytes, record_bytes, end: end as usize,
282276
})
283277
} else {
284278
None

‎lightning/src/offers/offer.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,8 @@ macro_rules! offer_builder_methods { (
438438
}
439439
}
440440

441-
let mut bytes = Vec::new();
441+
const OFFER_ALLOCATION_SIZE: usize = 512;
442+
let mut bytes = Vec::with_capacity(OFFER_ALLOCATION_SIZE);
442443
$self.offer.write(&mut bytes).unwrap();
443444

444445
let id = OfferId::from_valid_offer_tlv_stream(&bytes);

‎lightning/src/offers/refund.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,8 @@ macro_rules! refund_builder_methods { (
338338
$self.refund.payer.0 = metadata;
339339
}
340340

341-
let mut bytes = Vec::new();
341+
const REFUND_ALLOCATION_SIZE: usize = 512;
342+
let mut bytes = Vec::with_capacity(REFUND_ALLOCATION_SIZE);
342343
$self.refund.write(&mut bytes).unwrap();
343344

344345
Ok(Refund {

0 commit comments

Comments
 (0)