Fix incorrect anchors `counterparty_payment_script` upon deserialization

diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
index 95f68ba22f6de4bd870dd04e43da32e1bac5582e..53ac85c29328bb74a45d0745e66802720b708ccb 100644 (file)
--- a/lightning/src/chain/channelmonitor.rs
+++ b/lightning/src/chain/channelmonitor.rs
@@ -1696,6 +1696,16 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
                        Vec::new()
                }
        }
                        Vec::new()
                }
        }

+
+       #[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> {
 }
 
 impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {


@@ -4146,7 +4156,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
                        1 => { None },
                        _ => return Err(DecodeError::InvalidValue),
                };
                        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) }
                let shutdown_script = {
                        let script = <Script as Readable>::read(reader)?;
                        if script.is_empty() { None } else { Some(script) }


@@ -4347,6 +4357,17 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
                        (17, initial_counterparty_commitment_info, option),
                });
 
                        (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,
                Ok((best_block.block_hash(), ChannelMonitor::from_impl(ChannelMonitorImpl {
                        latest_update_id,
                        commitment_transaction_number_obscure_factor,

diff --git a/lightning/src/ln/monitor_tests.rs b/lightning/src/ln/monitor_tests.rs
index 3a300bf07575a2e094a55ac621cd99640d6c2c74..00f8613cb911bd5887819ed933fe958ab46d593b 100644 (file)
--- a/lightning/src/ln/monitor_tests.rs
+++ b/lightning/src/ln/monitor_tests.rs
@@ -2318,3 +2318,90 @@ fn test_anchors_aggregated_revoked_htlc_tx() {
        // revoked commitment which Bob has the preimage for.
        assert_eq!(nodes[1].chain_monitor.chain_monitor.get_claimable_balances(&[]).len(), 6);
 }
        // 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);
+}

diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs
index 18c3db76f18efac38cb0ebf529f42c32b7ef4757..b912221fac372da5247879ff62fd7d506d655a6b 100644 (file)
--- a/lightning/src/util/test_utils.rs
+++ b/lightning/src/util/test_utils.rs
@@ -207,6 +207,9 @@ pub struct TestChainMonitor<'a> {
        /// ChannelForceClosed event for the given channel_id with should_broadcast set to the given
        /// boolean.
        pub expect_channel_force_closed: Mutex<Option<(ChannelId, bool)>>,
        /// ChannelForceClosed event for the given channel_id with should_broadcast set to the given
        /// boolean.
        pub expect_channel_force_closed: Mutex<Option<(ChannelId, bool)>>,

+       /// If this is set to Some(), the next round trip serialization check will not hold after an
+       /// update_channel call (not watch_channel) for the given channel_id.
+       pub expect_monitor_round_trip_fail: Mutex<Option<ChannelId>>,

 }
 impl<'a> TestChainMonitor<'a> {
        pub fn new(chain_source: Option<&'a TestChainSource>, broadcaster: &'a chaininterface::BroadcasterInterface, logger: &'a TestLogger, fee_estimator: &'a TestFeeEstimator, persister: &'a chainmonitor::Persist<TestChannelSigner>, keys_manager: &'a TestKeysInterface) -> Self {
 }
 impl<'a> TestChainMonitor<'a> {
        pub fn new(chain_source: Option<&'a TestChainSource>, broadcaster: &'a chaininterface::BroadcasterInterface, logger: &'a TestLogger, fee_estimator: &'a TestFeeEstimator, persister: &'a chainmonitor::Persist<TestChannelSigner>, keys_manager: &'a TestKeysInterface) -> Self {


@@ -217,6 +220,7 @@ impl<'a> TestChainMonitor<'a> {
                        chain_monitor: chainmonitor::ChainMonitor::new(chain_source, broadcaster, logger, fee_estimator, persister),
                        keys_manager,
                        expect_channel_force_closed: Mutex::new(None),
                        chain_monitor: chainmonitor::ChainMonitor::new(chain_source, broadcaster, logger, fee_estimator, persister),
                        keys_manager,
                        expect_channel_force_closed: Mutex::new(None),

+                       expect_monitor_round_trip_fail: Mutex::new(None),

                }
        }
 
                }
        }
 


@@ -267,7 +271,12 @@ impl<'a> chain::Watch<TestChannelSigner> for TestChainMonitor<'a> {
                monitor.write(&mut w).unwrap();
                let new_monitor = <(BlockHash, channelmonitor::ChannelMonitor<TestChannelSigner>)>::read(
                        &mut io::Cursor::new(&w.0), (self.keys_manager, self.keys_manager)).unwrap().1;
                monitor.write(&mut w).unwrap();
                let new_monitor = <(BlockHash, channelmonitor::ChannelMonitor<TestChannelSigner>)>::read(
                        &mut io::Cursor::new(&w.0), (self.keys_manager, self.keys_manager)).unwrap().1;

-               assert!(new_monitor == *monitor);
+               if let Some(chan_id) = self.expect_monitor_round_trip_fail.lock().unwrap().take() {
+                       assert_eq!(chan_id, funding_txo.to_channel_id());
+                       assert!(new_monitor != *monitor);
+               } else {
+                       assert!(new_monitor == *monitor);
+               }

                self.added_monitors.lock().unwrap().push((funding_txo, new_monitor));
                update_res
        }
                self.added_monitors.lock().unwrap().push((funding_txo, new_monitor));
                update_res
        }