Skip to content

Commit 620244d

Browse files
authored
Merge pull request #2605 from wpaulino/anchors-monitor-track-to-remote-script
Use correct to_remote script in counterparty commitments
2 parents d007d1b + f464aa9 commit 620244d

File tree

6 files changed

+235
-39
lines changed

6 files changed

+235
-39
lines changed

lightning/src/chain/channelmonitor.rs

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,11 @@
2222
2323
use bitcoin::blockdata::block::BlockHeader;
2424
use bitcoin::blockdata::transaction::{OutPoint as BitcoinOutPoint, TxOut, Transaction};
25-
use bitcoin::blockdata::script::{Script, Builder};
26-
use bitcoin::blockdata::opcodes;
25+
use bitcoin::blockdata::script::Script;
2726

2827
use bitcoin::hashes::Hash;
2928
use bitcoin::hashes::sha256::Hash as Sha256;
30-
use bitcoin::hash_types::{Txid, BlockHash, WPubkeyHash};
29+
use bitcoin::hash_types::{Txid, BlockHash};
3130

3231
use bitcoin::secp256k1::{Secp256k1, ecdsa::Signature};
3332
use bitcoin::secp256k1::{SecretKey, PublicKey};
@@ -1141,8 +1140,9 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
11411140
best_block: BestBlock, counterparty_node_id: PublicKey) -> ChannelMonitor<Signer> {
11421141

11431142
assert!(commitment_transaction_number_obscure_factor <= (1 << 48));
1144-
let payment_key_hash = WPubkeyHash::hash(&keys.pubkeys().payment_point.serialize());
1145-
let counterparty_payment_script = Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_key_hash[..]).into_script();
1143+
let counterparty_payment_script = chan_utils::get_counterparty_payment_script(
1144+
&channel_parameters.channel_type_features, &keys.pubkeys().payment_point
1145+
);
11461146

11471147
let counterparty_channel_parameters = channel_parameters.counterparty_parameters.as_ref().unwrap();
11481148
let counterparty_delayed_payment_base_key = counterparty_channel_parameters.pubkeys.delayed_payment_basepoint;
@@ -1702,6 +1702,16 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitor<Signer> {
17021702
});
17031703
spendable_outputs
17041704
}
1705+
1706+
#[cfg(test)]
1707+
pub fn get_counterparty_payment_script(&self) -> Script{
1708+
self.inner.lock().unwrap().counterparty_payment_script.clone()
1709+
}
1710+
1711+
#[cfg(test)]
1712+
pub fn set_counterparty_payment_script(&self, script: Script) {
1713+
self.inner.lock().unwrap().counterparty_payment_script = script;
1714+
}
17051715
}
17061716

17071717
impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
@@ -2269,6 +2279,7 @@ macro_rules! fail_unbroadcast_htlcs {
22692279

22702280
#[cfg(test)]
22712281
pub fn deliberately_bogus_accepted_htlc_witness_program() -> Vec<u8> {
2282+
use bitcoin::blockdata::opcodes;
22722283
let mut ret = [opcodes::all::OP_NOP.to_u8(); 136];
22732284
ret[131] = opcodes::all::OP_DROP.to_u8();
22742285
ret[132] = opcodes::all::OP_DROP.to_u8();
@@ -4079,6 +4090,7 @@ impl<Signer: WriteableEcdsaChannelSigner> ChannelMonitorImpl<Signer> {
40794090
output: outp.clone(),
40804091
channel_keys_id: self.channel_keys_id,
40814092
channel_value_satoshis: self.channel_value_satoshis,
4093+
channel_transaction_parameters: Some(self.onchain_tx_handler.channel_transaction_parameters.clone()),
40824094
}));
40834095
}
40844096
if self.shutdown_script.as_ref() == Some(&outp.script_pubkey) {
@@ -4181,7 +4193,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
41814193
1 => { None },
41824194
_ => return Err(DecodeError::InvalidValue),
41834195
};
4184-
let counterparty_payment_script = Readable::read(reader)?;
4196+
let mut counterparty_payment_script: Script = Readable::read(reader)?;
41854197
let shutdown_script = {
41864198
let script = <Script as Readable>::read(reader)?;
41874199
if script.is_empty() { None } else { Some(script) }
@@ -4382,6 +4394,17 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
43824394
(17, initial_counterparty_commitment_info, option),
43834395
});
43844396

4397+
// Monitors for anchor outputs channels opened in v0.0.116 suffered from a bug in which the
4398+
// wrong `counterparty_payment_script` was being tracked. Fix it now on deserialization to
4399+
// give them a chance to recognize the spendable output.
4400+
if onchain_tx_handler.channel_type_features().supports_anchors_zero_fee_htlc_tx() &&
4401+
counterparty_payment_script.is_v0_p2wpkh()
4402+
{
4403+
let payment_point = onchain_tx_handler.channel_transaction_parameters.holder_pubkeys.payment_point;
4404+
counterparty_payment_script =
4405+
chan_utils::get_to_countersignatory_with_anchors_redeemscript(&payment_point).to_v0_p2wsh();
4406+
}
4407+
43854408
Ok((best_block.block_hash(), ChannelMonitor::from_impl(ChannelMonitorImpl {
43864409
latest_update_id,
43874410
commitment_transaction_number_obscure_factor,

lightning/src/events/bump_transaction.rs

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use crate::ln::chan_utils::{
2626
use crate::ln::features::ChannelTypeFeatures;
2727
use crate::ln::PaymentPreimage;
2828
use crate::prelude::*;
29-
use crate::sign::{EcdsaChannelSigner, SignerProvider, WriteableEcdsaChannelSigner};
29+
use crate::sign::{EcdsaChannelSigner, SignerProvider, WriteableEcdsaChannelSigner, P2WPKH_WITNESS_WEIGHT};
3030
use crate::sync::Mutex;
3131
use crate::util::logger::Logger;
3232

@@ -384,12 +384,6 @@ pub struct Utxo {
384384
}
385385

386386
impl Utxo {
387-
const P2WPKH_WITNESS_WEIGHT: u64 = 1 /* num stack items */ +
388-
1 /* sig length */ +
389-
73 /* sig including sighash flag */ +
390-
1 /* pubkey length */ +
391-
33 /* pubkey */;
392-
393387
/// Returns a `Utxo` with the `satisfaction_weight` estimate for a legacy P2PKH output.
394388
pub fn new_p2pkh(outpoint: OutPoint, value: u64, pubkey_hash: &PubkeyHash) -> Self {
395389
let script_sig_size = 1 /* script_sig length */ +
@@ -419,7 +413,7 @@ impl Utxo {
419413
value,
420414
script_pubkey: Script::new_p2sh(&Script::new_v0_p2wpkh(pubkey_hash).script_hash()),
421415
},
422-
satisfaction_weight: script_sig_size * WITNESS_SCALE_FACTOR as u64 + Self::P2WPKH_WITNESS_WEIGHT,
416+
satisfaction_weight: script_sig_size * WITNESS_SCALE_FACTOR as u64 + P2WPKH_WITNESS_WEIGHT,
423417
}
424418
}
425419

@@ -431,7 +425,7 @@ impl Utxo {
431425
value,
432426
script_pubkey: Script::new_v0_p2wpkh(pubkey_hash),
433427
},
434-
satisfaction_weight: EMPTY_SCRIPT_SIG_WEIGHT + Self::P2WPKH_WITNESS_WEIGHT,
428+
satisfaction_weight: EMPTY_SCRIPT_SIG_WEIGHT + P2WPKH_WITNESS_WEIGHT,
435429
}
436430
}
437431
}

lightning/src/ln/chan_utils.rs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use bitcoin::util::address::Payload;
1919
use bitcoin::hashes::{Hash, HashEngine};
2020
use bitcoin::hashes::sha256::Hash as Sha256;
2121
use bitcoin::hashes::ripemd160::Hash as Ripemd160;
22-
use bitcoin::hash_types::{Txid, PubkeyHash};
22+
use bitcoin::hash_types::{Txid, PubkeyHash, WPubkeyHash};
2323

2424
use crate::chain::chaininterface::fee_for_weight;
2525
use crate::chain::package::WEIGHT_REVOKED_OUTPUT;
@@ -475,7 +475,7 @@ impl_writeable_tlv_based!(TxCreationKeys, {
475475
});
476476

477477
/// One counterparty's public keys which do not change over the life of a channel.
478-
#[derive(Clone, Debug, PartialEq, Eq)]
478+
#[derive(Clone, Debug, Hash, PartialEq, Eq)]
479479
pub struct ChannelPublicKeys {
480480
/// The public key which is used to sign all commitment transactions, as it appears in the
481481
/// on-chain channel lock-in 2-of-2 multisig output.
@@ -556,6 +556,16 @@ pub fn get_revokeable_redeemscript(revocation_key: &PublicKey, contest_delay: u1
556556
res
557557
}
558558

559+
/// Returns the script for the counterparty's output on a holder's commitment transaction based on
560+
/// the channel type.
561+
pub fn get_counterparty_payment_script(channel_type_features: &ChannelTypeFeatures, payment_key: &PublicKey) -> Script {
562+
if channel_type_features.supports_anchors_zero_fee_htlc_tx() {
563+
get_to_countersignatory_with_anchors_redeemscript(payment_key).to_v0_p2wsh()
564+
} else {
565+
Script::new_v0_p2wpkh(&WPubkeyHash::hash(&payment_key.serialize()))
566+
}
567+
}
568+
559569
/// Information about an HTLC as it appears in a commitment transaction
560570
#[derive(Clone, Debug, PartialEq, Eq)]
561571
pub struct HTLCOutputInCommitment {
@@ -853,7 +863,7 @@ pub fn build_anchor_input_witness(funding_key: &PublicKey, funding_sig: &Signatu
853863
///
854864
/// Normally, this is converted to the broadcaster/countersignatory-organized DirectedChannelTransactionParameters
855865
/// before use, via the as_holder_broadcastable and as_counterparty_broadcastable functions.
856-
#[derive(Clone, Debug, PartialEq, Eq)]
866+
#[derive(Clone, Debug, Hash, PartialEq, Eq)]
857867
pub struct ChannelTransactionParameters {
858868
/// Holder public keys
859869
pub holder_pubkeys: ChannelPublicKeys,
@@ -873,7 +883,7 @@ pub struct ChannelTransactionParameters {
873883
}
874884

875885
/// Late-bound per-channel counterparty data used to build transactions.
876-
#[derive(Clone, Debug, PartialEq, Eq)]
886+
#[derive(Clone, Debug, Hash, PartialEq, Eq)]
877887
pub struct CounterpartyChannelTransactionParameters {
878888
/// Counter-party public keys
879889
pub pubkeys: ChannelPublicKeys,

lightning/src/ln/monitor_tests.rs

Lines changed: 94 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2294,16 +2294,20 @@ fn test_anchors_aggregated_revoked_htlc_tx() {
22942294

22952295
assert!(nodes[1].chain_monitor.chain_monitor.get_and_clear_pending_events().is_empty());
22962296
let spendable_output_events = nodes[0].chain_monitor.chain_monitor.get_and_clear_pending_events();
2297-
assert_eq!(spendable_output_events.len(), 2);
2298-
for event in spendable_output_events.iter() {
2297+
assert_eq!(spendable_output_events.len(), 4);
2298+
for event in spendable_output_events {
22992299
if let Event::SpendableOutputs { outputs, channel_id } = event {
23002300
assert_eq!(outputs.len(), 1);
23012301
assert!(vec![chan_b.2, chan_a.2].contains(&channel_id.unwrap()));
23022302
let spend_tx = nodes[0].keys_manager.backing.spend_spendable_outputs(
23032303
&[&outputs[0]], Vec::new(), Script::new_op_return(&[]), 253, None, &Secp256k1::new(),
23042304
).unwrap();
23052305

2306-
check_spends!(spend_tx, revoked_claim_transactions.get(&spend_tx.input[0].previous_output.txid).unwrap());
2306+
if let SpendableOutputDescriptor::StaticPaymentOutput(_) = &outputs[0] {
2307+
check_spends!(spend_tx, &revoked_commitment_a, &revoked_commitment_b);
2308+
} else {
2309+
check_spends!(spend_tx, revoked_claim_transactions.get(&spend_tx.input[0].previous_output.txid).unwrap());
2310+
}
23072311
} else {
23082312
panic!("unexpected event");
23092313
}
@@ -2321,3 +2325,90 @@ fn test_anchors_aggregated_revoked_htlc_tx() {
23212325
// revoked commitment which Bob has the preimage for.
23222326
assert_eq!(nodes[1].chain_monitor.chain_monitor.get_claimable_balances(&[]).len(), 6);
23232327
}
2328+
2329+
fn do_test_anchors_monitor_fixes_counterparty_payment_script_on_reload(confirm_commitment_before_reload: bool) {
2330+
// Tests that we'll fix a ChannelMonitor's `counterparty_payment_script` for an anchor outputs
2331+
// channel upon deserialization.
2332+
let chanmon_cfgs = create_chanmon_cfgs(2);
2333+
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
2334+
let persister;
2335+
let chain_monitor;
2336+
let mut user_config = test_default_channel_config();
2337+
user_config.channel_handshake_config.negotiate_anchors_zero_fee_htlc_tx = true;
2338+
user_config.manually_accept_inbound_channels = true;
2339+
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(user_config), Some(user_config)]);
2340+
let node_deserialized;
2341+
let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);
2342+
2343+
let (_, _, chan_id, funding_tx) = create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 50_000_000);
2344+
2345+
// Set the monitor's `counterparty_payment_script` to a dummy P2WPKH script.
2346+
let secp = Secp256k1::new();
2347+
let privkey = bitcoin::PrivateKey::from_slice(&[1; 32], bitcoin::Network::Testnet).unwrap();
2348+
let pubkey = bitcoin::PublicKey::from_private_key(&secp, &privkey);
2349+
let p2wpkh_script = Script::new_v0_p2wpkh(&pubkey.wpubkey_hash().unwrap());
2350+
get_monitor!(nodes[1], chan_id).set_counterparty_payment_script(p2wpkh_script.clone());
2351+
assert_eq!(get_monitor!(nodes[1], chan_id).get_counterparty_payment_script(), p2wpkh_script);
2352+
2353+
// Confirm the counterparty's commitment and reload the monitor (either before or after) such
2354+
// that we arrive at the correct `counterparty_payment_script` after the reload.
2355+
nodes[0].node.force_close_broadcasting_latest_txn(&chan_id, &nodes[1].node.get_our_node_id()).unwrap();
2356+
check_added_monitors(&nodes[0], 1);
2357+
check_closed_broadcast(&nodes[0], 1, true);
2358+
check_closed_event!(&nodes[0], 1, ClosureReason::HolderForceClosed, false,
2359+
[nodes[1].node.get_our_node_id()], 100000);
2360+
2361+
let commitment_tx = {
2362+
let mut txn = nodes[0].tx_broadcaster.unique_txn_broadcast();
2363+
assert_eq!(txn.len(), 1);
2364+
assert_eq!(txn[0].output.len(), 4);
2365+
check_spends!(txn[0], funding_tx);
2366+
txn.pop().unwrap()
2367+
};
2368+
2369+
mine_transaction(&nodes[0], &commitment_tx);
2370+
let commitment_tx_conf_height = if confirm_commitment_before_reload {
2371+
// We should expect our round trip serialization check to fail as we're writing the monitor
2372+
// with the incorrect P2WPKH script but reading it with the correct P2WSH script.
2373+
*nodes[1].chain_monitor.expect_monitor_round_trip_fail.lock().unwrap() = Some(chan_id);
2374+
let commitment_tx_conf_height = block_from_scid(&mine_transaction(&nodes[1], &commitment_tx));
2375+
let serialized_monitor = get_monitor!(nodes[1], chan_id).encode();
2376+
reload_node!(nodes[1], user_config, &nodes[1].node.encode(), &[&serialized_monitor], persister, chain_monitor, node_deserialized);
2377+
commitment_tx_conf_height
2378+
} else {
2379+
let serialized_monitor = get_monitor!(nodes[1], chan_id).encode();
2380+
reload_node!(nodes[1], user_config, &nodes[1].node.encode(), &[&serialized_monitor], persister, chain_monitor, node_deserialized);
2381+
let commitment_tx_conf_height = block_from_scid(&mine_transaction(&nodes[1], &commitment_tx));
2382+
check_added_monitors(&nodes[1], 1);
2383+
check_closed_broadcast(&nodes[1], 1, true);
2384+
commitment_tx_conf_height
2385+
};
2386+
check_closed_event!(&nodes[1], 1, ClosureReason::CommitmentTxConfirmed, false,
2387+
[nodes[0].node.get_our_node_id()], 100000);
2388+
assert!(get_monitor!(nodes[1], chan_id).get_counterparty_payment_script().is_v0_p2wsh());
2389+
2390+
connect_blocks(&nodes[0], ANTI_REORG_DELAY - 1);
2391+
connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
2392+
2393+
if confirm_commitment_before_reload {
2394+
// If we saw the commitment before our `counterparty_payment_script` was fixed, we'll never
2395+
// get the spendable output event for the `to_remote` output, so we'll need to get it
2396+
// manually via `get_spendable_outputs`.
2397+
check_added_monitors(&nodes[1], 1);
2398+
let outputs = get_monitor!(nodes[1], chan_id).get_spendable_outputs(&commitment_tx, commitment_tx_conf_height);
2399+
assert_eq!(outputs.len(), 1);
2400+
let spend_tx = nodes[1].keys_manager.backing.spend_spendable_outputs(
2401+
&[&outputs[0]], Vec::new(), Builder::new().push_opcode(opcodes::all::OP_RETURN).into_script(),
2402+
253, None, &secp
2403+
).unwrap();
2404+
check_spends!(spend_tx, &commitment_tx);
2405+
} else {
2406+
test_spendable_output(&nodes[1], &commitment_tx);
2407+
}
2408+
}
2409+
2410+
#[test]
2411+
fn test_anchors_monitor_fixes_counterparty_payment_script_on_reload() {
2412+
do_test_anchors_monitor_fixes_counterparty_payment_script_on_reload(false);
2413+
do_test_anchors_monitor_fixes_counterparty_payment_script_on_reload(true);
2414+
}

0 commit comments

Comments
 (0)