Skip to content

Commit f96f304

Browse files
committed
Remove duplicate TxOut persistence
Now that ConstructedTransaction holds the actual Transaction, there's no need to keep track of and persist the outputs separately. However, the serial IDs are still needed to later reconstruct which outputs were contributed.
1 parent dc49b41 commit f96f304

File tree

3 files changed

+34
-39
lines changed

3 files changed

+34
-39
lines changed

lightning/src/ln/channel.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6108,8 +6108,8 @@ where
61086108
{
61096109
let mut output_index = None;
61106110
let expected_spk = funding.get_funding_redeemscript().to_p2wsh();
6111-
for (idx, outp) in signing_session.unsigned_tx().outputs().enumerate() {
6112-
if outp.script_pubkey() == &expected_spk && outp.value() == funding.get_value_satoshis() {
6111+
for (idx, outp) in signing_session.unsigned_tx().tx().output.iter().enumerate() {
6112+
if outp.script_pubkey == expected_spk && outp.value.to_sat() == funding.get_value_satoshis() {
61136113
if output_index.is_some() {
61146114
return Err(AbortReason::DuplicateFundingOutput);
61156115
}

lightning/src/ln/interactivetxs.rs

Lines changed: 30 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ pub(crate) struct ConstructedTransaction {
199199
holder_is_initiator: bool,
200200

201201
inputs: Vec<NegotiatedTxInput>,
202-
outputs: Vec<InteractiveTxOutput>,
202+
outputs: Vec<NegotiatedTxOutput>,
203203
tx: Transaction,
204204

205205
local_inputs_value_satoshis: u64,
@@ -217,6 +217,11 @@ pub(crate) struct NegotiatedTxInput {
217217
prev_output: TxOut,
218218
}
219219

220+
#[derive(Clone, Debug, Eq, PartialEq)]
221+
pub(crate) struct NegotiatedTxOutput {
222+
serial_id: SerialId,
223+
}
224+
220225
impl NegotiatedTxInput {
221226
pub(super) fn is_local(&self, holder_is_initiator: bool) -> bool {
222227
!is_serial_id_valid_for_counterparty(holder_is_initiator, self.serial_id)
@@ -227,11 +232,21 @@ impl NegotiatedTxInput {
227232
}
228233
}
229234

235+
impl NegotiatedTxOutput {
236+
pub(super) fn is_local(&self, holder_is_initiator: bool) -> bool {
237+
!is_serial_id_valid_for_counterparty(holder_is_initiator, self.serial_id)
238+
}
239+
}
240+
230241
impl_writeable_tlv_based!(NegotiatedTxInput, {
231242
(1, serial_id, required),
232243
(3, prev_output, required),
233244
});
234245

246+
impl_writeable_tlv_based!(NegotiatedTxOutput, {
247+
(1, serial_id, required),
248+
});
249+
235250
impl_writeable_tlv_based!(ConstructedTransaction, {
236251
(1, holder_is_initiator, required),
237252
(3, inputs, required),
@@ -282,9 +297,10 @@ impl ConstructedTransaction {
282297

283298
let mut inputs: Vec<(TxIn, NegotiatedTxInput)> =
284299
context.inputs.into_values().map(|input| input.into_txin_and_negotiated_input()).collect();
285-
let mut outputs: Vec<InteractiveTxOutput> = context.outputs.into_values().collect();
300+
let mut outputs: Vec<(TxOut, NegotiatedTxOutput)> =
301+
context.outputs.into_values().map(|output| output.into_txout_and_negotiated_output()).collect();
286302
inputs.sort_unstable_by_key(|(_, input)| input.serial_id);
287-
outputs.sort_unstable_by_key(|output| output.serial_id);
303+
outputs.sort_unstable_by_key(|(_, output)| output.serial_id);
288304

289305
let shared_input_index =
290306
context.shared_funding_input.as_ref().and_then(|shared_funding_input| {
@@ -297,7 +313,7 @@ impl ConstructedTransaction {
297313
});
298314

299315
let (input, inputs): (Vec<TxIn>, Vec<NegotiatedTxInput>) = inputs.into_iter().unzip();
300-
let output = outputs.iter().map(|output| output.tx_out().clone()).collect();
316+
let (output, outputs): (Vec<TxOut>, Vec<NegotiatedTxOutput>) = outputs.into_iter().unzip();
301317

302318
let tx = Transaction {
303319
version: Version::TWO,
@@ -332,10 +348,6 @@ impl ConstructedTransaction {
332348
&self.tx
333349
}
334350

335-
pub fn outputs(&self) -> impl Iterator<Item = &InteractiveTxOutput> {
336-
self.outputs.iter()
337-
}
338-
339351
pub fn inputs(&self) -> impl Iterator<Item = &NegotiatedTxInput> {
340352
self.inputs.iter()
341353
}
@@ -567,12 +579,7 @@ impl InteractiveTxSigningSession {
567579
.outputs
568580
.iter()
569581
.enumerate()
570-
.filter(|(_, output)| {
571-
!is_serial_id_valid_for_counterparty(
572-
self.unsigned_tx.holder_is_initiator,
573-
output.serial_id,
574-
)
575-
})
582+
.filter(|(_, output)| output.is_local(self.unsigned_tx.holder_is_initiator))
576583
.count()
577584
}
578585

@@ -1768,12 +1775,6 @@ pub(crate) struct InteractiveTxOutput {
17681775
output: OutputOwned,
17691776
}
17701777

1771-
impl_writeable_tlv_based!(InteractiveTxOutput, {
1772-
(1, serial_id, required),
1773-
(3, added_by, required),
1774-
(5, output, required),
1775-
});
1776-
17771778
impl InteractiveTxOutput {
17781779
pub fn tx_out(&self) -> &TxOut {
17791780
self.output.tx_out()
@@ -1798,6 +1799,11 @@ impl InteractiveTxOutput {
17981799
pub fn script_pubkey(&self) -> &ScriptBuf {
17991800
&self.output.tx_out().script_pubkey
18001801
}
1802+
1803+
fn into_txout_and_negotiated_output(self) -> (TxOut, NegotiatedTxOutput) {
1804+
let txout = self.output.into_tx_out();
1805+
(txout, NegotiatedTxOutput { serial_id: self.serial_id })
1806+
}
18011807
}
18021808

18031809
impl InteractiveTxInput {
@@ -2221,9 +2227,9 @@ mod tests {
22212227
use core::ops::Deref;
22222228

22232229
use super::{
2224-
get_output_weight, AddingRole, ConstructedTransaction, InteractiveTxOutput,
2225-
InteractiveTxSigningSession, NegotiatedTxInput, OutputOwned, P2TR_INPUT_WEIGHT_LOWER_BOUND,
2226-
P2WPKH_INPUT_WEIGHT_LOWER_BOUND, P2WSH_INPUT_WEIGHT_LOWER_BOUND, TX_COMMON_FIELDS_WEIGHT,
2230+
get_output_weight, ConstructedTransaction, InteractiveTxSigningSession, NegotiatedTxInput,
2231+
P2TR_INPUT_WEIGHT_LOWER_BOUND, P2WPKH_INPUT_WEIGHT_LOWER_BOUND,
2232+
P2WSH_INPUT_WEIGHT_LOWER_BOUND, TX_COMMON_FIELDS_WEIGHT,
22272233
};
22282234

22292235
const TEST_FEERATE_SATS_PER_KW: u32 = FEERATE_FLOOR_SATS_PER_KW * 10;
@@ -3302,21 +3308,10 @@ mod tests {
33023308
})
33033309
.collect();
33043310

3305-
let outputs: Vec<InteractiveTxOutput> = transaction
3306-
.output
3307-
.iter()
3308-
.cloned()
3309-
.map(|txout| InteractiveTxOutput {
3310-
serial_id: 0, // N/A for test
3311-
added_by: AddingRole::Local,
3312-
output: OutputOwned::Single(txout),
3313-
})
3314-
.collect();
3315-
33163311
let unsigned_tx = ConstructedTransaction {
33173312
holder_is_initiator: true,
33183313
inputs,
3319-
outputs,
3314+
outputs: vec![], // N/A for test
33203315
tx: transaction.clone(),
33213316
local_inputs_value_satoshis: 0, // N/A for test
33223317
local_outputs_value_satoshis: 0, // N/A for test

lightning/src/util/ser.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
1616
use crate::io::{self, BufRead, Read, Write};
1717
use crate::io_extras::{copy, sink};
18-
use crate::ln::interactivetxs::{InteractiveTxOutput, NegotiatedTxInput};
18+
use crate::ln::interactivetxs::{NegotiatedTxInput, NegotiatedTxOutput};
1919
use crate::ln::onion_utils::{HMAC_COUNT, HMAC_LEN, HOLD_TIME_LEN, MAX_HOPS};
2020
use crate::prelude::*;
2121
use crate::sync::{Mutex, RwLock};
@@ -1083,7 +1083,7 @@ impl_for_vec!(crate::ln::msgs::SocketAddress);
10831083
impl_for_vec!((A, B), A, B);
10841084
impl_for_vec!(SerialId);
10851085
impl_for_vec!(NegotiatedTxInput);
1086-
impl_for_vec!(InteractiveTxOutput);
1086+
impl_for_vec!(NegotiatedTxOutput);
10871087
impl_for_vec!(crate::ln::our_peer_storage::PeerStorageMonitorHolder);
10881088
impl_for_vec!(crate::blinded_path::message::BlindedMessagePath);
10891089
impl_writeable_for_vec!(&crate::routing::router::BlindedTail);

0 commit comments

Comments
 (0)