Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
189 changes: 75 additions & 114 deletions fuzz/src/chanmon_consistency.rs

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion lightning-tests/src/upgrade_downgrade_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,8 @@ fn do_test_0_1_htlc_forward_after_splice(fail_htlc: bool) {
value: Amount::from_sat(1_000),
script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(),
}]);
let splice_tx = splice_channel(&nodes[0], &nodes[1], ChannelId(chan_id_bytes_a), contribution);
let (splice_tx, _) =
splice_channel(&nodes[0], &nodes[1], ChannelId(chan_id_bytes_a), contribution);
for node in nodes.iter() {
mine_transaction(node, &splice_tx);
connect_blocks(node, ANTI_REORG_DELAY - 1);
Expand Down
128 changes: 85 additions & 43 deletions lightning/src/events/bump_transaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use crate::ln::chan_utils::{
HTLC_TIMEOUT_INPUT_KEYED_ANCHOR_WITNESS_WEIGHT, HTLC_TIMEOUT_INPUT_P2A_ANCHOR_WITNESS_WEIGHT,
P2WSH_TXOUT_WEIGHT, SEGWIT_MARKER_FLAG_WEIGHT, TRUC_CHILD_MAX_WEIGHT, TRUC_MAX_WEIGHT,
};
use crate::ln::funding::FundingTxInput;
use crate::ln::types::ChannelId;
use crate::prelude::*;
use crate::sign::ecdsa::EcdsaChannelSigner;
Expand Down Expand Up @@ -270,6 +271,12 @@ pub struct Input {
pub satisfaction_weight: u64,
}

impl_writeable_tlv_based!(Input, {
(1, outpoint, required),
(3, previous_utxo, required),
(5, satisfaction_weight, required),
});

/// An unspent transaction output that is available to spend resulting from a successful
/// [`CoinSelection`] attempt.
#[derive(Clone, Debug, Hash, PartialOrd, Ord, PartialEq, Eq)]
Expand All @@ -282,12 +289,15 @@ pub struct Utxo {
/// with their lengths included, required to satisfy the output's script. The weight consumed by
/// the input's `script_sig` must account for [`WITNESS_SCALE_FACTOR`].
pub satisfaction_weight: u64,
/// The sequence number to use in the [`TxIn`] when spending the UTXO.
pub sequence: Sequence,
}

impl_writeable_tlv_based!(Utxo, {
(1, outpoint, required),
(3, output, required),
(5, satisfaction_weight, required),
(7, sequence, (default_value, Sequence::ENABLE_RBF_NO_LOCKTIME)),
});

impl Utxo {
Expand All @@ -302,6 +312,7 @@ impl Utxo {
outpoint,
output: TxOut { value, script_pubkey: ScriptBuf::new_p2pkh(pubkey_hash) },
satisfaction_weight: script_sig_size * WITNESS_SCALE_FACTOR as u64 + 1, /* empty witness */
sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
}
}

Expand All @@ -321,6 +332,7 @@ impl Utxo {
},
satisfaction_weight: script_sig_size * WITNESS_SCALE_FACTOR as u64
+ P2WPKH_WITNESS_WEIGHT,
sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
}
}

Expand All @@ -330,24 +342,38 @@ impl Utxo {
outpoint,
output: TxOut { value, script_pubkey: ScriptBuf::new_p2wpkh(pubkey_hash) },
satisfaction_weight: EMPTY_SCRIPT_SIG_WEIGHT + P2WPKH_WITNESS_WEIGHT,
sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
}
}
}

/// An unspent transaction output with at least one confirmation.
pub type ConfirmedUtxo = FundingTxInput;

/// The result of a successful coin selection attempt for a transaction requiring additional UTXOs
/// to cover its fees.
#[derive(Clone, Debug)]
pub struct CoinSelection {
/// The set of UTXOs (with at least 1 confirmation) to spend and use within a transaction
/// requiring additional fees.
pub confirmed_utxos: Vec<Utxo>,
pub confirmed_utxos: Vec<ConfirmedUtxo>,
/// An additional output tracking whether any change remained after coin selection. This output
/// should always have a value above dust for its given `script_pubkey`. It should not be
/// spent until the transaction it belongs to confirms to ensure mempool descendant limits are
/// not met. This implies no other party should be able to spend it except us.
pub change_output: Option<TxOut>,
}

impl CoinSelection {
fn satisfaction_weight(&self) -> u64 {
self.confirmed_utxos.iter().map(|ConfirmedUtxo { utxo, .. }| utxo.satisfaction_weight).sum()
}

fn amount(&self) -> Amount {
self.confirmed_utxos.iter().map(|ConfirmedUtxo { utxo, .. }| utxo.output.value).sum()
}
}

/// An abstraction over a bitcoin wallet that can perform coin selection over a set of UTXOs and can
/// sign for them. The coin selection method aims to mimic Bitcoin Core's `fundrawtransaction` RPC,
/// which most wallets should be able to satisfy. Otherwise, consider implementing [`WalletSource`],
Expand Down Expand Up @@ -393,7 +419,7 @@ pub trait CoinSelectionSource {
///
/// [`ChannelMonitor::rebroadcast_pending_claims`]: crate::chain::channelmonitor::ChannelMonitor::rebroadcast_pending_claims
fn select_confirmed_utxos<'a>(
&'a self, claim_id: ClaimId, must_spend: Vec<Input>, must_pay_to: &'a [TxOut],
&'a self, claim_id: Option<ClaimId>, must_spend: Vec<Input>, must_pay_to: &'a [TxOut],
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a fan that we're complicating this already complex argument even more, but this really needs docs explaining the desired behavior when None

target_feerate_sat_per_1000_weight: u32, max_tx_weight: u64,
) -> impl Future<Output = Result<CoinSelection, ()>> + MaybeSend + 'a;
/// Signs and provides the full witness for all inputs within the transaction known to the
Expand All @@ -418,11 +444,18 @@ pub trait WalletSource {
fn list_confirmed_utxos<'a>(
&'a self,
) -> impl Future<Output = Result<Vec<Utxo>, ()>> + MaybeSend + 'a;

/// Returns the previous transaction containing the UTXO.
fn get_prevtx<'a>(
&'a self, utxo: &Utxo,
) -> impl Future<Output = Result<Transaction, ()>> + MaybeSend + 'a;

/// Returns a script to use for change above dust resulting from a successful coin selection
/// attempt.
fn get_change_script<'a>(
&'a self,
) -> impl Future<Output = Result<ScriptBuf, ()>> + MaybeSend + 'a;

/// Signs and provides the full [`TxIn::script_sig`] and [`TxIn::witness`] for all inputs within
/// the transaction known to the wallet (i.e., any provided via
/// [`WalletSource::list_confirmed_utxos`]).
Expand Down Expand Up @@ -452,7 +485,7 @@ where
// TODO: Do we care about cleaning this up once the UTXOs have a confirmed spend? We can do so
// by checking whether any UTXOs that exist in the map are no longer returned in
// `list_confirmed_utxos`.
locked_utxos: Mutex<HashMap<OutPoint, ClaimId>>,
locked_utxos: Mutex<HashMap<OutPoint, Option<ClaimId>>>,
}

impl<W: Deref + MaybeSync + MaybeSend, L: Deref + MaybeSync + MaybeSend> Wallet<W, L>
Expand All @@ -475,7 +508,7 @@ where
/// least 1 satoshi at the current feerate, otherwise, we'll only attempt to spend those which
/// contribute at least twice their fee.
async fn select_confirmed_utxos_internal(
&self, utxos: &[Utxo], claim_id: ClaimId, force_conflicting_utxo_spend: bool,
&self, utxos: &[Utxo], claim_id: Option<ClaimId>, force_conflicting_utxo_spend: bool,
tolerate_high_network_feerates: bool, target_feerate_sat_per_1000_weight: u32,
preexisting_tx_weight: u64, input_amount_sat: Amount, target_amount_sat: Amount,
max_tx_weight: u64,
Expand All @@ -499,7 +532,9 @@ where
.iter()
.filter_map(|utxo| {
if let Some(utxo_claim_id) = locked_utxos.get(&utxo.outpoint) {
if *utxo_claim_id != claim_id && !force_conflicting_utxo_spend {
if (utxo_claim_id.is_none() || *utxo_claim_id != claim_id)
&& !force_conflicting_utxo_spend
{
log_trace!(
self.logger,
"Skipping UTXO {} to prevent conflicting spend",
Expand Down Expand Up @@ -610,10 +645,13 @@ where
Some(TxOut { script_pubkey: change_script, value: change_output_amount })
};

Ok(CoinSelection {
confirmed_utxos: selected_utxos.into_iter().map(|(utxo, _)| utxo).collect(),
change_output,
})
let mut confirmed_utxos = Vec::with_capacity(selected_utxos.len());
for (utxo, _) in selected_utxos {
let prevtx = self.source.get_prevtx(&utxo).await?;
confirmed_utxos.push(ConfirmedUtxo { utxo, prevtx });
Copy link
Contributor

Choose a reason for hiding this comment

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

Worth sanity checking the transaction actually matches the utxo?

}

Ok(CoinSelection { confirmed_utxos, change_output })
}
}

Expand All @@ -624,7 +662,7 @@ where
L::Target: Logger + MaybeSend + MaybeSync,
{
fn select_confirmed_utxos<'a>(
&'a self, claim_id: ClaimId, must_spend: Vec<Input>, must_pay_to: &'a [TxOut],
&'a self, claim_id: Option<ClaimId>, must_spend: Vec<Input>, must_pay_to: &'a [TxOut],
target_feerate_sat_per_1000_weight: u32, max_tx_weight: u64,
) -> impl Future<Output = Result<CoinSelection, ()>> + MaybeSend + 'a {
async move {
Expand Down Expand Up @@ -724,11 +762,11 @@ where

/// Updates a transaction with the result of a successful coin selection attempt.
fn process_coin_selection(&self, tx: &mut Transaction, coin_selection: &CoinSelection) {
for utxo in coin_selection.confirmed_utxos.iter() {
for ConfirmedUtxo { utxo, .. } in coin_selection.confirmed_utxos.iter() {
tx.input.push(TxIn {
previous_output: utxo.outpoint,
script_sig: ScriptBuf::new(),
sequence: Sequence::ZERO,
sequence: utxo.sequence,
witness: Witness::new(),
});
}
Expand Down Expand Up @@ -818,7 +856,7 @@ where
let coin_selection: CoinSelection = self
.utxo_source
.select_confirmed_utxos(
claim_id,
Some(claim_id),
must_spend,
&[],
package_target_feerate_sat_per_1000_weight,
Expand Down Expand Up @@ -846,12 +884,10 @@ where
output: vec![],
};

let input_satisfaction_weight: u64 =
coin_selection.confirmed_utxos.iter().map(|utxo| utxo.satisfaction_weight).sum();
let input_satisfaction_weight = coin_selection.satisfaction_weight();
let total_satisfaction_weight =
anchor_input_witness_weight + EMPTY_SCRIPT_SIG_WEIGHT + input_satisfaction_weight;
let total_input_amount = must_spend_amount
+ coin_selection.confirmed_utxos.iter().map(|utxo| utxo.output.value).sum();
let total_input_amount = must_spend_amount + coin_selection.amount();

self.process_coin_selection(&mut anchor_tx, &coin_selection);
let anchor_txid = anchor_tx.compute_txid();
Expand All @@ -866,10 +902,10 @@ where
let index = idx + 1;
debug_assert_eq!(
anchor_psbt.unsigned_tx.input[index].previous_output,
utxo.outpoint
utxo.outpoint()
);
if utxo.output.script_pubkey.is_witness_program() {
anchor_psbt.inputs[index].witness_utxo = Some(utxo.output);
if utxo.output().script_pubkey.is_witness_program() {
anchor_psbt.inputs[index].witness_utxo = Some(utxo.into_output());
}
}

Expand Down Expand Up @@ -1076,7 +1112,7 @@ where
let coin_selection: CoinSelection = match self
.utxo_source
.select_confirmed_utxos(
utxo_id,
Some(utxo_id),
must_spend,
&htlc_tx.output,
target_feerate_sat_per_1000_weight,
Expand All @@ -1101,13 +1137,11 @@ where
utxo_id = claim_id.step_with_bytes(&broadcasted_htlcs.to_be_bytes());

#[cfg(debug_assertions)]
let input_satisfaction_weight: u64 =
coin_selection.confirmed_utxos.iter().map(|utxo| utxo.satisfaction_weight).sum();
let input_satisfaction_weight = coin_selection.satisfaction_weight();
#[cfg(debug_assertions)]
let total_satisfaction_weight = must_spend_satisfaction_weight + input_satisfaction_weight;
#[cfg(debug_assertions)]
let input_value: u64 =
coin_selection.confirmed_utxos.iter().map(|utxo| utxo.output.value.to_sat()).sum();
let input_value = coin_selection.amount().to_sat();
#[cfg(debug_assertions)]
let total_input_amount = must_spend_amount + input_value;

Expand All @@ -1128,9 +1162,12 @@ where
for (idx, utxo) in coin_selection.confirmed_utxos.into_iter().enumerate() {
// offset to skip the htlc inputs
let index = idx + selected_htlcs.len();
debug_assert_eq!(htlc_psbt.unsigned_tx.input[index].previous_output, utxo.outpoint);
if utxo.output.script_pubkey.is_witness_program() {
htlc_psbt.inputs[index].witness_utxo = Some(utxo.output);
debug_assert_eq!(
htlc_psbt.unsigned_tx.input[index].previous_output,
utxo.outpoint()
);
if utxo.output().script_pubkey.is_witness_program() {
htlc_psbt.inputs[index].witness_utxo = Some(utxo.into_output());
}
}

Expand Down Expand Up @@ -1274,17 +1311,16 @@ mod tests {
use crate::util::ser::Readable;
use crate::util::test_utils::{TestBroadcaster, TestLogger};

use bitcoin::hashes::Hash;
use bitcoin::hex::FromHex;
use bitcoin::{Network, ScriptBuf, Transaction, Txid};
use bitcoin::{Network, ScriptBuf, Transaction};

struct TestCoinSelectionSource {
// (commitment + anchor value, commitment + input weight, target feerate, result)
expected_selects: Mutex<Vec<(u64, u64, u32, CoinSelection)>>,
}
impl CoinSelectionSourceSync for TestCoinSelectionSource {
fn select_confirmed_utxos(
&self, _claim_id: ClaimId, must_spend: Vec<Input>, _must_pay_to: &[TxOut],
&self, _claim_id: Option<ClaimId>, must_spend: Vec<Input>, _must_pay_to: &[TxOut],
target_feerate_sat_per_1000_weight: u32, _max_tx_weight: u64,
) -> Result<CoinSelection, ()> {
let mut expected_selects = self.expected_selects.lock().unwrap();
Expand All @@ -1297,12 +1333,10 @@ mod tests {
}
fn sign_psbt(&self, psbt: Psbt) -> Result<Transaction, ()> {
let mut tx = psbt.unsigned_tx;
for input in tx.input.iter_mut() {
if input.previous_output.txid != Txid::from_byte_array([44; 32]) {
// Channel output, add a realistic size witness to make the assertions happy
input.witness = Witness::from_slice(&[vec![42; 162]]);
}
}
// Channel output, add a realistic size witness to make the assertions happy
//
// FIXME: This doesn't seem to be needed since handle_channel_close overrides it
tx.input.first_mut().unwrap().witness = Witness::from_slice(&[vec![42; 162]]);
Ok(tx)
}
}
Expand Down Expand Up @@ -1339,6 +1373,13 @@ mod tests {
.weight()
.to_wu();

let prevtx = Transaction {
version: Version::TWO,
lock_time: LockTime::ZERO,
input: vec![],
output: vec![TxOut { value: Amount::from_sat(200), script_pubkey: ScriptBuf::new() }],
};

let broadcaster = TestBroadcaster::new(Network::Testnet);
let source = TestCoinSelectionSource {
expected_selects: Mutex::new(vec![
Expand All @@ -1353,13 +1394,14 @@ mod tests {
commitment_and_anchor_fee,
868,
CoinSelection {
confirmed_utxos: vec![Utxo {
outpoint: OutPoint { txid: Txid::from_byte_array([44; 32]), vout: 0 },
output: TxOut {
value: Amount::from_sat(200),
script_pubkey: ScriptBuf::new(),
confirmed_utxos: vec![ConfirmedUtxo {
utxo: Utxo {
outpoint: OutPoint { txid: prevtx.compute_txid(), vout: 0 },
output: prevtx.output[0].clone(),
satisfaction_weight: 5, // Just the script_sig and witness lengths
sequence: Sequence::ENABLE_RBF_NO_LOCKTIME,
},
satisfaction_weight: 5, // Just the script_sig and witness lengths
prevtx,
}],
change_output: None,
},
Expand Down
Loading
Loading