Skip to content

Commit 1f4ce55

Browse files
committed
Remove TransactionU16LenLimited
TransactionU16LenLimited was used to limit Transaction serialization size to u16::MAX. This was because messages can not be longer than u16::MAX bytes when serialized for the transport layer. However, this limit doesn't take into account other fields in a message containing a Transaction, including the length of the transaction itself. Remove TransactionU16LenLimited and instead check any user supplied transactions in the context of the enclosing message (e.g. TxAddInput).
1 parent 95e4ac3 commit 1f4ce55

File tree

6 files changed

+106
-125
lines changed

6 files changed

+106
-125
lines changed

lightning/src/ln/channel.rs

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,7 @@ use crate::util::config::{
8585
use crate::util::errors::APIError;
8686
use crate::util::logger::{Logger, Record, WithContext};
8787
use crate::util::scid_utils::{block_from_scid, scid_from_parts};
88-
use crate::util::ser::{
89-
Readable, ReadableArgs, RequiredWrapper, TransactionU16LenLimited, Writeable, Writer,
90-
};
88+
use crate::util::ser::{Readable, ReadableArgs, RequiredWrapper, Writeable, Writer};
9189

9290
use alloc::collections::{btree_map, BTreeMap};
9391

@@ -6081,7 +6079,7 @@ pub enum FundingTxContributions {
60816079
InputsOnly {
60826080
/// The inputs used to meet the contributed amount. Any excess amount will be sent to a
60836081
/// change output.
6084-
inputs: Vec<(TxIn, TransactionU16LenLimited)>,
6082+
inputs: Vec<(TxIn, Transaction)>,
60856083

60866084
/// An optional change output script. This will be used if needed or, if not set, generated
60876085
/// using `SignerProvider::get_destination_script`.
@@ -6091,7 +6089,7 @@ pub enum FundingTxContributions {
60916089

60926090
impl FundingTxContributions {
60936091
/// Returns an inputs to be contributed to the funding transaction.
6094-
pub fn inputs(&self) -> &[(TxIn, TransactionU16LenLimited)] {
6092+
pub fn inputs(&self) -> &[(TxIn, Transaction)] {
60956093
match self {
60966094
FundingTxContributions::InputsOnly { inputs, .. } => &inputs[..],
60976095
}
@@ -10705,10 +10703,23 @@ where
1070510703
})?;
1070610704
// Convert inputs
1070710705
let mut funding_inputs = Vec::new();
10708-
for (tx_in, tx, _w) in our_funding_inputs.into_iter() {
10709-
let tx16 = TransactionU16LenLimited::new(tx)
10710-
.map_err(|_e| APIError::APIMisuseError { err: format!("Too large transaction") })?;
10711-
funding_inputs.push((tx_in, tx16));
10706+
for (txin, tx, _) in our_funding_inputs.into_iter() {
10707+
const MESSAGE_TEMPLATE: msgs::TxAddInput = msgs::TxAddInput {
10708+
channel_id: ChannelId([0; 32]),
10709+
serial_id: 0,
10710+
prevtx: None,
10711+
prevtx_out: 0,
10712+
sequence: 0,
10713+
shared_input_txid: None,
10714+
};
10715+
let message_len = MESSAGE_TEMPLATE.serialized_length() + tx.serialized_length();
10716+
if message_len > u16::MAX as usize {
10717+
return Err(APIError::APIMisuseError {
10718+
err: format!("Funding input's prevtx is too large for tx_add_input"),
10719+
});
10720+
}
10721+
10722+
funding_inputs.push((txin, tx));
1071210723
}
1071310724

1071410725
let funding_tx_contributions =
@@ -12489,7 +12500,7 @@ where
1248912500
pub fn new_outbound<ES: Deref, F: Deref, L: Deref>(
1249012501
fee_estimator: &LowerBoundedFeeEstimator<F>, entropy_source: &ES, signer_provider: &SP,
1249112502
counterparty_node_id: PublicKey, their_features: &InitFeatures, funding_satoshis: u64,
12492-
funding_inputs: Vec<(TxIn, TransactionU16LenLimited)>, user_id: u128, config: &UserConfig,
12503+
funding_inputs: Vec<(TxIn, Transaction)>, user_id: u128, config: &UserConfig,
1249312504
current_chain_height: u32, outbound_scid_alias: u64, funding_confirmation_target: ConfirmationTarget,
1249412505
logger: L,
1249512506
) -> Result<Self, APIError>

lightning/src/ln/dual_funding_tests.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ use {
2323
crate::ln::msgs::{CommitmentSigned, TxAddInput, TxAddOutput, TxComplete, TxSignatures},
2424
crate::ln::types::ChannelId,
2525
crate::prelude::*,
26-
crate::util::ser::TransactionU16LenLimited,
2726
crate::util::test_utils,
2827
bitcoin::Witness,
2928
};
@@ -51,7 +50,7 @@ fn do_test_v2_channel_establishment(session: V2ChannelEstablishmentTestSession)
5150
&[session.initiator_input_value_satoshis],
5251
)
5352
.into_iter()
54-
.map(|(txin, tx, _)| (txin, TransactionU16LenLimited::new(tx).unwrap()))
53+
.map(|(txin, tx, _)| (txin, tx))
5554
.collect();
5655

5756
// Alice creates a dual-funded channel as initiator.
@@ -94,7 +93,7 @@ fn do_test_v2_channel_establishment(session: V2ChannelEstablishmentTestSession)
9493
sequence: initiator_funding_inputs[0].0.sequence.0,
9594
shared_input_txid: None,
9695
};
97-
let input_value = tx_add_input_msg.prevtx.as_ref().unwrap().as_transaction().output
96+
let input_value = tx_add_input_msg.prevtx.as_ref().unwrap().output
9897
[tx_add_input_msg.prevtx_out as usize]
9998
.value;
10099
assert_eq!(input_value.to_sat(), session.initiator_input_value_satoshis);

lightning/src/ln/interactivetxs.rs

Lines changed: 24 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ use crate::ln::msgs;
2727
use crate::ln::msgs::{MessageSendEvent, SerialId, TxSignatures};
2828
use crate::ln::types::ChannelId;
2929
use crate::sign::{EntropySource, P2TR_KEY_PATH_WITNESS_WEIGHT, P2WPKH_WITNESS_WEIGHT};
30-
use crate::util::ser::TransactionU16LenLimited;
3130

3231
use core::fmt::Display;
3332
use core::ops::Deref;
@@ -676,10 +675,9 @@ impl NegotiationContext {
676675
return Err(AbortReason::UnexpectedFundingInput);
677676
}
678677
} else if let Some(prevtx) = &msg.prevtx {
679-
let transaction = prevtx.as_transaction();
680-
let txid = transaction.compute_txid();
678+
let txid = prevtx.compute_txid();
681679

682-
if let Some(tx_out) = transaction.output.get(msg.prevtx_out as usize) {
680+
if let Some(tx_out) = prevtx.output.get(msg.prevtx_out as usize) {
683681
if !tx_out.script_pubkey.is_witness_program() {
684682
// The receiving node:
685683
// - MUST fail the negotiation if:
@@ -860,14 +858,9 @@ impl NegotiationContext {
860858
return Err(AbortReason::UnexpectedFundingInput);
861859
}
862860
} else if let Some(prevtx) = &msg.prevtx {
863-
let prev_txid = prevtx.as_transaction().compute_txid();
861+
let prev_txid = prevtx.compute_txid();
864862
let prev_outpoint = OutPoint { txid: prev_txid, vout: msg.prevtx_out };
865-
let prev_output = prevtx
866-
.as_transaction()
867-
.output
868-
.get(vout)
869-
.ok_or(AbortReason::PrevTxOutInvalid)?
870-
.clone();
863+
let prev_output = prevtx.output.get(vout).ok_or(AbortReason::PrevTxOutInvalid)?.clone();
871864
let txin = TxIn {
872865
previous_output: prev_outpoint,
873866
sequence: Sequence(msg.sequence),
@@ -1247,7 +1240,7 @@ impl_writeable_tlv_based_enum!(AddingRole,
12471240
#[derive(Clone, Debug, Eq, PartialEq)]
12481241
struct SingleOwnedInput {
12491242
input: TxIn,
1250-
prev_tx: TransactionU16LenLimited,
1243+
prev_tx: Transaction,
12511244
prev_output: TxOut,
12521245
}
12531246

@@ -1652,7 +1645,7 @@ where
16521645
pub feerate_sat_per_kw: u32,
16531646
pub is_initiator: bool,
16541647
pub funding_tx_locktime: AbsoluteLockTime,
1655-
pub inputs_to_contribute: Vec<(TxIn, TransactionU16LenLimited)>,
1648+
pub inputs_to_contribute: Vec<(TxIn, Transaction)>,
16561649
pub shared_funding_input: Option<SharedOwnedInput>,
16571650
pub shared_funding_output: SharedOwnedOutput,
16581651
pub outputs_to_contribute: Vec<TxOut>,
@@ -1694,7 +1687,7 @@ impl InteractiveTxConstructor {
16941687
// Check for the existence of prevouts'
16951688
for (txin, tx) in inputs_to_contribute.iter() {
16961689
let vout = txin.previous_output.vout as usize;
1697-
if tx.as_transaction().output.get(vout).is_none() {
1690+
if tx.output.get(vout).is_none() {
16981691
return Err(AbortReason::PrevTxOutInvalid);
16991692
}
17001693
}
@@ -1703,7 +1696,7 @@ impl InteractiveTxConstructor {
17031696
.map(|(txin, tx)| {
17041697
let serial_id = generate_holder_serial_id(entropy_source, is_initiator);
17051698
let vout = txin.previous_output.vout as usize;
1706-
let prev_output = tx.as_transaction().output.get(vout).unwrap().clone(); // checked above
1699+
let prev_output = tx.output.get(vout).unwrap().clone(); // checked above
17071700
let input =
17081701
InputOwned::Single(SingleOwnedInput { input: txin, prev_tx: tx, prev_output });
17091702
(serial_id, input)
@@ -1892,12 +1885,11 @@ pub(super) fn calculate_change_output_value(
18921885
let mut total_input_satoshis = 0u64;
18931886
let mut our_funding_inputs_weight = 0u64;
18941887
for (txin, tx) in context.funding_tx_contributions.inputs().iter() {
1895-
let txid = tx.as_transaction().compute_txid();
1888+
let txid = tx.compute_txid();
18961889
if txin.previous_output.txid != txid {
18971890
return Err(AbortReason::PrevTxOutInvalid);
18981891
}
18991892
let output = tx
1900-
.as_transaction()
19011893
.output
19021894
.get(txin.previous_output.vout as usize)
19031895
.ok_or(AbortReason::PrevTxOutInvalid)?;
@@ -1956,7 +1948,6 @@ mod tests {
19561948
use crate::ln::types::ChannelId;
19571949
use crate::sign::EntropySource;
19581950
use crate::util::atomic_counter::AtomicCounter;
1959-
use crate::util::ser::TransactionU16LenLimited;
19601951
use bitcoin::absolute::LockTime as AbsoluteLockTime;
19611952
use bitcoin::amount::Amount;
19621953
use bitcoin::hashes::Hash;
@@ -2020,12 +2011,12 @@ mod tests {
20202011

20212012
struct TestSession {
20222013
description: &'static str,
2023-
inputs_a: Vec<(TxIn, TransactionU16LenLimited)>,
2014+
inputs_a: Vec<(TxIn, Transaction)>,
20242015
a_shared_input: Option<(OutPoint, TxOut, u64)>,
20252016
/// The funding output, with the value contributed
20262017
shared_output_a: (TxOut, u64),
20272018
outputs_a: Vec<TxOut>,
2028-
inputs_b: Vec<(TxIn, TransactionU16LenLimited)>,
2019+
inputs_b: Vec<(TxIn, Transaction)>,
20292020
b_shared_input: Option<(OutPoint, TxOut, u64)>,
20302021
/// The funding output, with the value contributed
20312022
shared_output_b: (TxOut, u64),
@@ -2291,7 +2282,7 @@ mod tests {
22912282
}
22922283
}
22932284

2294-
fn generate_inputs(outputs: &[TestOutput]) -> Vec<(TxIn, TransactionU16LenLimited)> {
2285+
fn generate_inputs(outputs: &[TestOutput]) -> Vec<(TxIn, Transaction)> {
22952286
let tx = generate_tx(outputs);
22962287
let txid = tx.compute_txid();
22972288
tx.output
@@ -2304,7 +2295,7 @@ mod tests {
23042295
sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
23052296
witness: Default::default(),
23062297
};
2307-
(txin, TransactionU16LenLimited::new(tx.clone()).unwrap())
2298+
(txin, tx.clone())
23082299
})
23092300
.collect()
23102301
}
@@ -2352,12 +2343,12 @@ mod tests {
23522343
(generate_txout(&TestOutput::P2WSH(value)), local_value)
23532344
}
23542345

2355-
fn generate_fixed_number_of_inputs(count: u16) -> Vec<(TxIn, TransactionU16LenLimited)> {
2346+
fn generate_fixed_number_of_inputs(count: u16) -> Vec<(TxIn, Transaction)> {
23562347
// Generate transactions with a total `count` number of outputs such that no transaction has a
23572348
// serialized length greater than u16::MAX.
23582349
let max_outputs_per_prevtx = 1_500;
23592350
let mut remaining = count;
2360-
let mut inputs: Vec<(TxIn, TransactionU16LenLimited)> = Vec::with_capacity(count as usize);
2351+
let mut inputs: Vec<(TxIn, Transaction)> = Vec::with_capacity(count as usize);
23612352

23622353
while remaining > 0 {
23632354
let tx_output_count = remaining.min(max_outputs_per_prevtx);
@@ -2370,7 +2361,7 @@ mod tests {
23702361
);
23712362
let txid = tx.compute_txid();
23722363

2373-
let mut temp: Vec<(TxIn, TransactionU16LenLimited)> = tx
2364+
let mut temp: Vec<(TxIn, Transaction)> = tx
23742365
.output
23752366
.iter()
23762367
.enumerate()
@@ -2381,7 +2372,7 @@ mod tests {
23812372
sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
23822373
witness: Default::default(),
23832374
};
2384-
(input, TransactionU16LenLimited::new(tx.clone()).unwrap())
2375+
(input, tx.clone())
23852376
})
23862377
.collect();
23872378

@@ -2592,10 +2583,9 @@ mod tests {
25922583
expect_error: Some((AbortReason::PrevTxOutInvalid, ErrorCulprit::NodeA)),
25932584
});
25942585

2595-
let tx =
2596-
TransactionU16LenLimited::new(generate_tx(&[TestOutput::P2WPKH(1_000_000)])).unwrap();
2586+
let tx = generate_tx(&[TestOutput::P2WPKH(1_000_000)]);
25972587
let invalid_sequence_input = TxIn {
2598-
previous_output: OutPoint { txid: tx.as_transaction().compute_txid(), vout: 0 },
2588+
previous_output: OutPoint { txid: tx.compute_txid(), vout: 0 },
25992589
..Default::default()
26002590
};
26012591
do_test_interactive_tx_constructor(TestSession {
@@ -2611,7 +2601,7 @@ mod tests {
26112601
expect_error: Some((AbortReason::IncorrectInputSequenceValue, ErrorCulprit::NodeA)),
26122602
});
26132603
let duplicate_input = TxIn {
2614-
previous_output: OutPoint { txid: tx.as_transaction().compute_txid(), vout: 0 },
2604+
previous_output: OutPoint { txid: tx.compute_txid(), vout: 0 },
26152605
sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
26162606
..Default::default()
26172607
};
@@ -2629,7 +2619,7 @@ mod tests {
26292619
});
26302620
// Non-initiator uses same prevout as initiator.
26312621
let duplicate_input = TxIn {
2632-
previous_output: OutPoint { txid: tx.as_transaction().compute_txid(), vout: 0 },
2622+
previous_output: OutPoint { txid: tx.compute_txid(), vout: 0 },
26332623
sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
26342624
..Default::default()
26352625
};
@@ -2646,7 +2636,7 @@ mod tests {
26462636
expect_error: Some((AbortReason::PrevTxOutInvalid, ErrorCulprit::NodeA)),
26472637
});
26482638
let duplicate_input = TxIn {
2649-
previous_output: OutPoint { txid: tx.as_transaction().compute_txid(), vout: 0 },
2639+
previous_output: OutPoint { txid: tx.compute_txid(), vout: 0 },
26502640
sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
26512641
..Default::default()
26522642
};
@@ -2979,9 +2969,9 @@ mod tests {
29792969
sequence: Sequence::ZERO,
29802970
witness: Witness::new(),
29812971
};
2982-
(txin, TransactionU16LenLimited::new(tx).unwrap())
2972+
(txin, tx)
29832973
})
2984-
.collect::<Vec<(TxIn, TransactionU16LenLimited)>>();
2974+
.collect::<Vec<(TxIn, Transaction)>>();
29852975
let funding_tx_contributions =
29862976
FundingTxContributions::InputsOnly { inputs, change_script: None };
29872977
let our_contributed = 110_000;

0 commit comments

Comments
 (0)