Use correct to_remote script in counterparty commitments
authorWilmer Paulino <wilmer@wilmerpaulino.com>
Mon, 25 Sep 2023 23:53:42 +0000 (16:53 -0700)
committerWilmer Paulino <wilmer@wilmerpaulino.com>
Fri, 29 Sep 2023 20:46:56 +0000 (13:46 -0700)
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.

lightning/src/chain/channelmonitor.rs
lightning/src/ln/chan_utils.rs
lightning/src/ln/monitor_tests.rs

index 7088d32d1607e1f4fa0137d6ceda9182d32ca3b2..b535e4b6cad34edbad3121e77f84d0976c3da0ee 100644 (file)
 
 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<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
                          best_block: BestBlock, counterparty_node_id: PublicKey) -> ChannelMonitor<Signer> {
 
                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<u8> {
+       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();
index 7521da01f97496932c357e0c0f974e37eee3b2bc..6e683b2fb9ecd5654f389889ec336e4902817136 100644 (file)
@@ -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 {
index f0cc872db0ad6731706cc380f59fd0c78be122f9..3a300bf07575a2e094a55ac621cd99640d6c2c74 100644 (file)
@@ -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");
                }