From: Wilmer Paulino Date: Mon, 25 Sep 2023 23:53:42 +0000 (-0700) Subject: Use correct to_remote script in counterparty commitments X-Git-Tag: v0.0.117-rc1~4^2~4 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=33b745f54e7d3ed7b34c40f27e9b2945f50b5d87;p=rust-lightning Use correct to_remote script in counterparty commitments While our commitment transactions did use the correct `to_remote` script, the `ChannelMonitor`'s was not as it is tracked separately. This would lead to users never receiving an `Event::SpendableOutputs` with a `StaticPaymentOutput` descriptor to claim the funds. Luckily, any users affected which had channel closures confirmed by a counterparty commitment just need to replay the closing transaction to receive the event. --- diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 7088d32d1..b535e4b6c 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -22,12 +22,11 @@ use bitcoin::blockdata::block::BlockHeader; use bitcoin::blockdata::transaction::{OutPoint as BitcoinOutPoint, TxOut, Transaction}; -use bitcoin::blockdata::script::{Script, Builder}; -use bitcoin::blockdata::opcodes; +use bitcoin::blockdata::script::Script; use bitcoin::hashes::Hash; use bitcoin::hashes::sha256::Hash as Sha256; -use bitcoin::hash_types::{Txid, BlockHash, WPubkeyHash}; +use bitcoin::hash_types::{Txid, BlockHash}; use bitcoin::secp256k1::{Secp256k1, ecdsa::Signature}; use bitcoin::secp256k1::{SecretKey, PublicKey}; @@ -1141,8 +1140,9 @@ impl ChannelMonitor { best_block: BestBlock, counterparty_node_id: PublicKey) -> ChannelMonitor { assert!(commitment_transaction_number_obscure_factor <= (1 << 48)); - let payment_key_hash = WPubkeyHash::hash(&keys.pubkeys().payment_point.serialize()); - let counterparty_payment_script = Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_key_hash[..]).into_script(); + let counterparty_payment_script = chan_utils::get_counterparty_payment_script( + &channel_parameters.channel_type_features, &keys.pubkeys().payment_point + ); let counterparty_channel_parameters = channel_parameters.counterparty_parameters.as_ref().unwrap(); let counterparty_delayed_payment_base_key = counterparty_channel_parameters.pubkeys.delayed_payment_basepoint; @@ -2263,6 +2263,7 @@ macro_rules! fail_unbroadcast_htlcs { #[cfg(test)] pub fn deliberately_bogus_accepted_htlc_witness_program() -> Vec { + use bitcoin::blockdata::opcodes; let mut ret = [opcodes::all::OP_NOP.to_u8(); 136]; ret[131] = opcodes::all::OP_DROP.to_u8(); ret[132] = opcodes::all::OP_DROP.to_u8(); diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index 7521da01f..6e683b2fb 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -19,7 +19,7 @@ use bitcoin::util::address::Payload; use bitcoin::hashes::{Hash, HashEngine}; use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::hashes::ripemd160::Hash as Ripemd160; -use bitcoin::hash_types::{Txid, PubkeyHash}; +use bitcoin::hash_types::{Txid, PubkeyHash, WPubkeyHash}; use crate::chain::chaininterface::fee_for_weight; use crate::chain::package::WEIGHT_REVOKED_OUTPUT; @@ -556,6 +556,16 @@ pub fn get_revokeable_redeemscript(revocation_key: &PublicKey, contest_delay: u1 res } +/// Returns the script for the counterparty's output on a holder's commitment transaction based on +/// the channel type. +pub fn get_counterparty_payment_script(channel_type_features: &ChannelTypeFeatures, payment_key: &PublicKey) -> Script { + if channel_type_features.supports_anchors_zero_fee_htlc_tx() { + get_to_countersignatory_with_anchors_redeemscript(payment_key).to_v0_p2wsh() + } else { + Script::new_v0_p2wpkh(&WPubkeyHash::hash(&payment_key.serialize())) + } +} + /// Information about an HTLC as it appears in a commitment transaction #[derive(Clone, Debug, PartialEq, Eq)] pub struct HTLCOutputInCommitment { diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs index f0cc872db..3a300bf07 100644 --- a/lightning/src/ln/monitor_tests.rs +++ b/lightning/src/ln/monitor_tests.rs @@ -2287,8 +2287,8 @@ fn test_anchors_aggregated_revoked_htlc_tx() { assert!(nodes[1].chain_monitor.chain_monitor.get_and_clear_pending_events().is_empty()); let spendable_output_events = nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events(); - assert_eq!(spendable_output_events.len(), 2); - for event in spendable_output_events.iter() { + assert_eq!(spendable_output_events.len(), 4); + for event in spendable_output_events { if let Event::SpendableOutputs { outputs, channel_id } = event { assert_eq!(outputs.len(), 1); assert!(vec![chan_b.2, chan_a.2].contains(&channel_id.unwrap())); @@ -2296,7 +2296,11 @@ fn test_anchors_aggregated_revoked_htlc_tx() { &[&outputs[0]], Vec::new(), Script::new_op_return(&[]), 253, None, &Secp256k1::new(), ).unwrap(); - check_spends!(spend_tx, revoked_claim_transactions.get(&spend_tx.input[0].previous_output.txid).unwrap()); + if let SpendableOutputDescriptor::StaticPaymentOutput(_) = &outputs[0] { + check_spends!(spend_tx, &revoked_commitment_a, &revoked_commitment_b); + } else { + check_spends!(spend_tx, revoked_claim_transactions.get(&spend_tx.input[0].previous_output.txid).unwrap()); + } } else { panic!("unexpected event"); }