Merge pull request #2605 from wpaulino/anchors-monitor-track-to-remote-script
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Fri, 29 Sep 2023 22:06:58 +0000 (22:06 +0000)
committerGitHub <noreply@github.com>
Fri, 29 Sep 2023 22:06:58 +0000 (22:06 +0000)
Use correct to_remote script in counterparty commitments

1  2 
lightning/src/chain/channelmonitor.rs
lightning/src/ln/monitor_tests.rs

index 5d3e0ac28bbf45058068761db4cad78f1283558c,53ac85c29328bb74a45d0745e66802720b708ccb..25bfa14d5420ac266b6254acfaba55b1c2e413ce
  
  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 +1140,9 @@@ impl<Signer: WriteableEcdsaChannelSigne
                          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;
  
        /// Returns the descriptors for relevant outputs (i.e., those that we can spend) within the
        /// transaction if they exist and the transaction has at least [`ANTI_REORG_DELAY`]
 +      /// confirmations. For [`SpendableOutputDescriptor::DelayedPaymentOutput`] descriptors to be
 +      /// returned, the transaction must have at least `max(ANTI_REORG_DELAY, to_self_delay)`
        /// confirmations.
        ///
        /// Descriptors returned by this method are primarily exposed via [`Event::SpendableOutputs`]
        /// missed/unhandled descriptors. For the purpose of gathering historical records, if the
        /// channel close has fully resolved (i.e., [`ChannelMonitor::get_claimable_balances`] returns
        /// an empty set), you can retrieve all spendable outputs by providing all descendant spending
 -      /// transactions starting from the channel's funding or closing transaction that have at least
 -      /// [`ANTI_REORG_DELAY`] confirmations.
 +      /// transactions starting from the channel's funding transaction and going down three levels.
        ///
        /// `tx` is a transaction we'll scan the outputs of. Any transaction can be provided. If any
        /// outputs which can be spent by us are found, at least one descriptor is returned.
        pub fn get_spendable_outputs(&self, tx: &Transaction, confirmation_height: u32) -> Vec<SpendableOutputDescriptor> {
                let inner = self.inner.lock().unwrap();
                let current_height = inner.best_block.height;
 -              if current_height.saturating_sub(ANTI_REORG_DELAY) + 1 >= confirmation_height {
 -                      inner.get_spendable_outputs(tx)
 -              } else {
 -                      Vec::new()
 -              }
 +              let mut spendable_outputs = inner.get_spendable_outputs(tx);
 +              spendable_outputs.retain(|descriptor| {
 +                      let mut conf_threshold = current_height.saturating_sub(ANTI_REORG_DELAY) + 1;
 +                      if let SpendableOutputDescriptor::DelayedPaymentOutput(descriptor) = descriptor {
 +                              conf_threshold = cmp::min(conf_threshold,
 +                                      current_height.saturating_sub(descriptor.to_self_delay as u32) + 1);
 +                      }
 +                      conf_threshold >= confirmation_height
 +              });
 +              spendable_outputs
        }
+       #[cfg(test)]
+       pub fn get_counterparty_payment_script(&self) -> Script{
+               self.inner.lock().unwrap().counterparty_payment_script.clone()
+       }
+       #[cfg(test)]
+       pub fn set_counterparty_payment_script(&self, script: Script) {
+               self.inner.lock().unwrap().counterparty_payment_script = script;
+       }
  }
  
  impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
@@@ -2269,6 -2273,7 +2279,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();
@@@ -2509,18 -2514,6 +2520,18 @@@ impl<Signer: WriteableEcdsaChannelSigne
        {
                self.payment_preimages.insert(payment_hash.clone(), payment_preimage.clone());
  
 +              let confirmed_spend_txid = self.funding_spend_confirmed.or_else(|| {
 +                      self.onchain_events_awaiting_threshold_conf.iter().find_map(|event| match event.event {
 +                              OnchainEvent::FundingSpendConfirmation { .. } => Some(event.txid),
 +                              _ => None,
 +                      })
 +              });
 +              let confirmed_spend_txid = if let Some(txid) = confirmed_spend_txid {
 +                      txid
 +              } else {
 +                      return;
 +              };
 +
                // If the channel is force closed, try to claim the output from this preimage.
                // First check if a counterparty commitment transaction has been broadcasted:
                macro_rules! claim_htlcs {
                        }
                }
                if let Some(txid) = self.current_counterparty_commitment_txid {
 -                      if let Some(commitment_number) = self.counterparty_commitment_txn_on_chain.get(&txid) {
 -                              claim_htlcs!(*commitment_number, txid);
 +                      if txid == confirmed_spend_txid {
 +                              if let Some(commitment_number) = self.counterparty_commitment_txn_on_chain.get(&txid) {
 +                                      claim_htlcs!(*commitment_number, txid);
 +                              } else {
 +                                      debug_assert!(false);
 +                                      log_error!(logger, "Detected counterparty commitment tx on-chain without tracking commitment number");
 +                              }
                                return;
                        }
                }
                if let Some(txid) = self.prev_counterparty_commitment_txid {
 -                      if let Some(commitment_number) = self.counterparty_commitment_txn_on_chain.get(&txid) {
 -                              claim_htlcs!(*commitment_number, txid);
 +                      if txid == confirmed_spend_txid {
 +                              if let Some(commitment_number) = self.counterparty_commitment_txn_on_chain.get(&txid) {
 +                                      claim_htlcs!(*commitment_number, txid);
 +                              } else {
 +                                      debug_assert!(false);
 +                                      log_error!(logger, "Detected counterparty commitment tx on-chain without tracking commitment number");
 +                              }
                                return;
                        }
                }
                // *we* sign a holder commitment transaction, not when e.g. a watchtower broadcasts one of our
                // holder commitment transactions.
                if self.broadcasted_holder_revokable_script.is_some() {
 -                      // Assume that the broadcasted commitment transaction confirmed in the current best
 -                      // block. Even if not, its a reasonable metric for the bump criteria on the HTLC
 -                      // transactions.
 -                      let (claim_reqs, _) = self.get_broadcasted_holder_claims(&self.current_holder_commitment_tx, self.best_block.height());
 -                      self.onchain_tx_handler.update_claims_view_from_requests(claim_reqs, self.best_block.height(), self.best_block.height(), broadcaster, fee_estimator, logger);
 -                      if let Some(ref tx) = self.prev_holder_signed_commitment_tx {
 -                              let (claim_reqs, _) = self.get_broadcasted_holder_claims(&tx, self.best_block.height());
 +                      let holder_commitment_tx = if self.current_holder_commitment_tx.txid == confirmed_spend_txid {
 +                              Some(&self.current_holder_commitment_tx)
 +                      } else if let Some(prev_holder_commitment_tx) = &self.prev_holder_signed_commitment_tx {
 +                              if prev_holder_commitment_tx.txid == confirmed_spend_txid {
 +                                      Some(prev_holder_commitment_tx)
 +                              } else {
 +                                      None
 +                              }
 +                      } else {
 +                              None
 +                      };
 +                      if let Some(holder_commitment_tx) = holder_commitment_tx {
 +                              // Assume that the broadcasted commitment transaction confirmed in the current best
 +                              // block. Even if not, its a reasonable metric for the bump criteria on the HTLC
 +                              // transactions.
 +                              let (claim_reqs, _) = self.get_broadcasted_holder_claims(&holder_commitment_tx, self.best_block.height());
                                self.onchain_tx_handler.update_claims_view_from_requests(claim_reqs, self.best_block.height(), self.best_block.height(), broadcaster, fee_estimator, logger);
                        }
                }
                                        output: outp.clone(),
                                        channel_keys_id: self.channel_keys_id,
                                        channel_value_satoshis: self.channel_value_satoshis,
+                                       channel_transaction_parameters: Some(self.onchain_tx_handler.channel_transaction_parameters.clone()),
                                }));
                        }
                        if self.shutdown_script.as_ref() == Some(&outp.script_pubkey) {
@@@ -4181,7 -4156,7 +4193,7 @@@ impl<'a, 'b, ES: EntropySource, SP: Sig
                        1 => { None },
                        _ => return Err(DecodeError::InvalidValue),
                };
-               let counterparty_payment_script = Readable::read(reader)?;
+               let mut counterparty_payment_script: Script = Readable::read(reader)?;
                let shutdown_script = {
                        let script = <Script as Readable>::read(reader)?;
                        if script.is_empty() { None } else { Some(script) }
                        (17, initial_counterparty_commitment_info, option),
                });
  
+               // Monitors for anchor outputs channels opened in v0.0.116 suffered from a bug in which the
+               // wrong `counterparty_payment_script` was being tracked. Fix it now on deserialization to
+               // give them a chance to recognize the spendable output.
+               if onchain_tx_handler.channel_type_features().supports_anchors_zero_fee_htlc_tx() &&
+                       counterparty_payment_script.is_v0_p2wpkh()
+               {
+                       let payment_point = onchain_tx_handler.channel_transaction_parameters.holder_pubkeys.payment_point;
+                       counterparty_payment_script =
+                               chan_utils::get_to_countersignatory_with_anchors_redeemscript(&payment_point).to_v0_p2wsh();
+               }
                Ok((best_block.block_hash(), ChannelMonitor::from_impl(ChannelMonitorImpl {
                        latest_update_id,
                        commitment_transaction_number_obscure_factor,
index c950d13bb01d7d4c63695e4acd3bf9654b7603bc,00f8613cb911bd5887819ed933fe958ab46d593b..905a4dc82d9fd066ad3aded10ad17332d2acfb10
@@@ -637,7 -637,7 +637,7 @@@ fn test_balances_on_local_commitment_ht
        // First confirm the commitment transaction on nodes[0], which should leave us with three
        // claimable balances.
        let node_a_commitment_claimable = nodes[0].best_block_info().1 + BREAKDOWN_TIMEOUT as u32;
 -      mine_transaction(&nodes[0], &as_txn[0]);
 +      let commitment_tx_conf_height_a = block_from_scid(&mine_transaction(&nodes[0], &as_txn[0]));
        check_added_monitors!(nodes[0], 1);
        check_closed_broadcast!(nodes[0], true);
        check_closed_event!(nodes[0], 1, ClosureReason::CommitmentTxConfirmed, [nodes[1].node.get_our_node_id()], 1000000);
  
        // Connect blocks until the commitment transaction's CSV expires, providing us the relevant
        // `SpendableOutputs` event and removing the claimable balance entry.
 -      connect_blocks(&nodes[0], node_a_commitment_claimable - nodes[0].best_block_info().1);
 +      connect_blocks(&nodes[0], node_a_commitment_claimable - nodes[0].best_block_info().1 - 1);
 +      assert!(get_monitor!(nodes[0], chan_id)
 +              .get_spendable_outputs(&as_txn[0], commitment_tx_conf_height_a).is_empty());
 +      connect_blocks(&nodes[0], 1);
        assert_eq!(vec![Balance::ClaimableAwaitingConfirmations {
                        amount_satoshis: 10_000,
                        confirmation_height: node_a_htlc_claimable,
                }],
                nodes[0].chain_monitor.chain_monitor.get_monitor(funding_outpoint).unwrap().get_claimable_balances());
 -      test_spendable_output(&nodes[0], &as_txn[0]);
 +      let to_self_spendable_output = test_spendable_output(&nodes[0], &as_txn[0]);
 +      assert_eq!(
 +              get_monitor!(nodes[0], chan_id).get_spendable_outputs(&as_txn[0], commitment_tx_conf_height_a),
 +              to_self_spendable_output
 +      );
  
        // Connect blocks until the HTLC-Timeout's CSV expires, providing us the relevant
        // `SpendableOutputs` event and removing the claimable balance entry.
@@@ -2294,8 -2287,8 +2294,8 @@@ fn test_anchors_aggregated_revoked_htlc
  
        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()));
                                &[&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");
                }
        // revoked commitment which Bob has the preimage for.
        assert_eq!(nodes[1].chain_monitor.chain_monitor.get_claimable_balances(&[]).len(), 6);
  }
+ fn do_test_anchors_monitor_fixes_counterparty_payment_script_on_reload(confirm_commitment_before_reload: bool) {
+       // Tests that we'll fix a ChannelMonitor's `counterparty_payment_script` for an anchor outputs
+       // channel upon deserialization.
+       let chanmon_cfgs = create_chanmon_cfgs(2);
+       let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+       let persister;
+       let chain_monitor;
+       let mut user_config = test_default_channel_config();
+       user_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true;
+       user_config.manually_accept_inbound_channels = true;
+       let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(user_config), Some(user_config)]);
+       let node_deserialized;
+       let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+       let (_, _, chan_id, funding_tx) = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 50_000_000);
+       // Set the monitor's `counterparty_payment_script` to a dummy P2WPKH script.
+       let secp = Secp256k1::new();
+       let privkey = bitcoin::PrivateKey::from_slice(&[1; 32], bitcoin::Network::Testnet).unwrap();
+       let pubkey = bitcoin::PublicKey::from_private_key(&secp, &privkey);
+       let p2wpkh_script = Script::new_v0_p2wpkh(&pubkey.wpubkey_hash().unwrap());
+       get_monitor!(nodes[1], chan_id).set_counterparty_payment_script(p2wpkh_script.clone());
+       assert_eq!(get_monitor!(nodes[1], chan_id).get_counterparty_payment_script(), p2wpkh_script);
+       // Confirm the counterparty's commitment and reload the monitor (either before or after) such
+       // that we arrive at the correct `counterparty_payment_script` after the reload.
+       nodes[0].node.force_close_broadcasting_latest_txn(&chan_id, &nodes[1].node.get_our_node_id()).unwrap();
+       check_added_monitors(&nodes[0], 1);
+       check_closed_broadcast(&nodes[0], 1, true);
+       check_closed_event!(&nodes[0], 1, ClosureReason::HolderForceClosed, false,
+                [nodes[1].node.get_our_node_id()], 100000);
+       let commitment_tx = {
+               let mut txn = nodes[0].tx_broadcaster.unique_txn_broadcast();
+               assert_eq!(txn.len(), 1);
+               assert_eq!(txn[0].output.len(), 4);
+               check_spends!(txn[0], funding_tx);
+               txn.pop().unwrap()
+       };
+       mine_transaction(&nodes[0], &commitment_tx);
+       let commitment_tx_conf_height = if confirm_commitment_before_reload {
+               // We should expect our round trip serialization check to fail as we're writing the monitor
+               // with the incorrect P2WPKH script but reading it with the correct P2WSH script.
+               *nodes[1].chain_monitor.expect_monitor_round_trip_fail.lock().unwrap() = Some(chan_id);
+               let commitment_tx_conf_height = block_from_scid(&mine_transaction(&nodes[1], &commitment_tx));
+               let serialized_monitor = get_monitor!(nodes[1], chan_id).encode();
+               reload_node!(nodes[1], user_config, &nodes[1].node.encode(), &[&serialized_monitor], persister, chain_monitor, node_deserialized);
+               commitment_tx_conf_height
+       } else {
+               let serialized_monitor = get_monitor!(nodes[1], chan_id).encode();
+               reload_node!(nodes[1], user_config, &nodes[1].node.encode(), &[&serialized_monitor], persister, chain_monitor, node_deserialized);
+               let commitment_tx_conf_height = block_from_scid(&mine_transaction(&nodes[1], &commitment_tx));
+               check_added_monitors(&nodes[1], 1);
+               check_closed_broadcast(&nodes[1], 1, true);
+               commitment_tx_conf_height
+       };
+       check_closed_event!(&nodes[1], 1, ClosureReason::CommitmentTxConfirmed, false,
+                [nodes[0].node.get_our_node_id()], 100000);
+       assert!(get_monitor!(nodes[1], chan_id).get_counterparty_payment_script().is_v0_p2wsh());
+       connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
+       connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
+       if confirm_commitment_before_reload {
+               // If we saw the commitment before our `counterparty_payment_script` was fixed, we'll never
+               // get the spendable output event for the `to_remote` output, so we'll need to get it
+               // manually via `get_spendable_outputs`.
+               check_added_monitors(&nodes[1], 1);
+               let outputs = get_monitor!(nodes[1], chan_id).get_spendable_outputs(&commitment_tx, commitment_tx_conf_height);
+               assert_eq!(outputs.len(), 1);
+               let spend_tx = nodes[1].keys_manager.backing.spend_spendable_outputs(
+                       &[&outputs[0]], Vec::new(), Builder::new().push_opcode(opcodes::all::OP_RETURN).into_script(),
+                       253, None, &secp
+               ).unwrap();
+               check_spends!(spend_tx, &commitment_tx);
+       } else {
+               test_spendable_output(&nodes[1], &commitment_tx);
+       }
+ }
+ #[test]
+ fn test_anchors_monitor_fixes_counterparty_payment_script_on_reload() {
+       do_test_anchors_monitor_fixes_counterparty_payment_script_on_reload(false);
+       do_test_anchors_monitor_fixes_counterparty_payment_script_on_reload(true);
+ }