From 3188ac9caf55ff19e869c23dc02c4d47683364e3 Mon Sep 17 00:00:00 2001 From: Antoine Riard Date: Tue, 24 Mar 2020 16:26:46 -0400 Subject: [PATCH] Remove SecretKey from DynamicOutputP2WSH descriptor Add sign_delayed_transaction in ChanSigner to be able to spend SpendableOutputDescriptor in test framework. --- lightning/src/chain/keysinterface.rs | 46 ++++++++++++++++---------- lightning/src/ln/chan_utils.rs | 6 ++-- lightning/src/ln/channelmonitor.rs | 17 +++++----- lightning/src/ln/functional_tests.rs | 48 ++++++++++++++++------------ 4 files changed, 68 insertions(+), 49 deletions(-) diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index fea11367..3a9c8168 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -50,28 +50,39 @@ pub enum SpendableOutputDescriptor { output: TxOut, }, /// An output to a P2WSH script which can be spent with a single signature after a CSV delay. - /// The private key which should be used to sign the transaction is provided, as well as the - /// full witness redeemScript which is hashed in the output script_pubkey. + /// /// The witness in the spending input should be: - /// (MINIMALIF standard rule) - /// - /// Note that the nSequence field in the input must be set to_self_delay (which corresponds to - /// the transaction not being broadcastable until at least to_self_delay blocks after the input - /// confirms). + /// (MINIMALIF standard rule) + /// + /// Note that the nSequence field in the spending input must be set to to_self_delay + /// (which means the transaction not being broadcastable until at least to_self_delay + /// blocks after the outpoint confirms). + /// /// These are generally the result of a "revocable" output to us, spendable only by us unless /// it is an output from us having broadcast an old state (which should never happen). + /// + /// WitnessScript may be regenerated by passing the revocation_pubkey, to_self_delay and + /// delayed_payment_pubkey to chan_utils::get_revokeable_redeemscript. + /// + /// To derive the delayed_payment key corresponding to the channel state, you must pass the + /// channel's delayed_payment_key and the provided per_commitment_point to + /// chan_utils::derive_private_key. The resulting key should be used to sign the spending + /// transaction. DynamicOutputP2WSH { /// The outpoint which is spendable outpoint: OutPoint, - /// The secret key which must be used to sign the spending transaction - key: SecretKey, - /// The witness redeemScript which is hashed to create the script_pubkey in the given output - witness_script: Script, + /// Per commitment point to derive delayed_payment_key by key holder + per_commitment_point: PublicKey, /// The nSequence value which must be set in the spending input to satisfy the OP_CSV in /// the witness_script. to_self_delay: u16, /// The output which is referenced by the given outpoint output: TxOut, + /// The channel keys state used to proceed to derivation of signing key. Must + /// be pass to KeysInterface::derive_channel_keys. + key_derivation_params: (u64, u64), + /// The remote_revocation_pubkey used to derive witnessScript + remote_revocation_pubkey: PublicKey }, // TODO: Note that because key is now static and exactly what is provided by us, we should drop // this in favor of StaticOutput: @@ -98,13 +109,15 @@ impl Writeable for SpendableOutputDescriptor { outpoint.write(writer)?; output.write(writer)?; }, - &SpendableOutputDescriptor::DynamicOutputP2WSH { ref outpoint, ref key, ref witness_script, ref to_self_delay, ref output } => { + &SpendableOutputDescriptor::DynamicOutputP2WSH { ref outpoint, ref per_commitment_point, ref to_self_delay, ref output, ref key_derivation_params, ref remote_revocation_pubkey } => { 1u8.write(writer)?; outpoint.write(writer)?; - key.write(writer)?; - witness_script.write(writer)?; + per_commitment_point.write(writer)?; to_self_delay.write(writer)?; output.write(writer)?; + key_derivation_params.0.write(writer)?; + key_derivation_params.1.write(writer)?; + remote_revocation_pubkey.write(writer)?; }, &SpendableOutputDescriptor::DynamicOutputP2WPKH { ref outpoint, ref key, ref output } => { 2u8.write(writer)?; @@ -126,10 +139,11 @@ impl Readable for SpendableOutputDescriptor { }), 1u8 => Ok(SpendableOutputDescriptor::DynamicOutputP2WSH { outpoint: Readable::read(reader)?, - key: Readable::read(reader)?, - witness_script: Readable::read(reader)?, + per_commitment_point: Readable::read(reader)?, to_self_delay: Readable::read(reader)?, output: Readable::read(reader)?, + key_derivation_params: (Readable::read(reader)?, Readable::read(reader)?), + remote_revocation_pubkey: Readable::read(reader)?, }), 2u8 => Ok(SpendableOutputDescriptor::DynamicOutputP2WPKH { outpoint: Readable::read(reader)?, diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index f79498c5..e619716a 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -184,7 +184,7 @@ pub(crate) fn derive_private_key(secp_ctx: &Secp256k1, Ok(key) } -pub(super) fn derive_public_key(secp_ctx: &Secp256k1, per_commitment_point: &PublicKey, base_point: &PublicKey) -> Result { +pub(crate) fn derive_public_key(secp_ctx: &Secp256k1, per_commitment_point: &PublicKey, base_point: &PublicKey) -> Result { let mut sha = Sha256::engine(); sha.input(&per_commitment_point.serialize()); sha.input(&base_point.serialize()); @@ -224,7 +224,7 @@ pub fn derive_private_revocation_key(secp_ctx: &Secp256k1 Ok(part_a) } -pub(super) fn derive_public_revocation_key(secp_ctx: &Secp256k1, per_commitment_point: &PublicKey, revocation_base_point: &PublicKey) -> Result { +pub(crate) fn derive_public_revocation_key(secp_ctx: &Secp256k1, per_commitment_point: &PublicKey, revocation_base_point: &PublicKey) -> Result { let rev_append_commit_hash_key = { let mut sha = Sha256::engine(); sha.input(&revocation_base_point.serialize()); @@ -313,7 +313,7 @@ impl TxCreationKeys { /// Gets the "to_local" output redeemscript, ie the script which is time-locked or spendable by /// the revocation key -pub(super) fn get_revokeable_redeemscript(revocation_key: &PublicKey, to_self_delay: u16, delayed_payment_key: &PublicKey) -> Script { +pub(crate) fn get_revokeable_redeemscript(revocation_key: &PublicKey, to_self_delay: u16, delayed_payment_key: &PublicKey) -> Script { Builder::new().push_opcode(opcodes::all::OP_IF) .push_slice(&revocation_key.serialize()) .push_opcode(opcodes::all::OP_ELSE) diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index 5ccde163..526a3623 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -708,7 +708,7 @@ pub struct ChannelMonitor { commitment_transaction_number_obscure_factor: u64, destination_script: Script, - broadcasted_local_revokable_script: Option<(Script, SecretKey, Script)>, + broadcasted_local_revokable_script: Option<(Script, PublicKey, PublicKey)>, remote_payment_script: Script, shutdown_script: Script, @@ -1615,14 +1615,12 @@ impl ChannelMonitor { (claimable_outpoints, Some((htlc_txid, tx.output.clone()))) } - fn broadcast_by_local_state(&self, commitment_tx: &Transaction, local_tx: &LocalSignedTx) -> (Vec, Vec, Option<(Script, SecretKey, Script)>) { + fn broadcast_by_local_state(&self, commitment_tx: &Transaction, local_tx: &LocalSignedTx) -> (Vec, Vec, Option<(Script, PublicKey, PublicKey)>) { let mut claim_requests = Vec::with_capacity(local_tx.htlc_outputs.len()); let mut watch_outputs = Vec::with_capacity(local_tx.htlc_outputs.len()); let redeemscript = chan_utils::get_revokeable_redeemscript(&local_tx.revocation_key, self.their_to_self_delay, &local_tx.delayed_payment_key); - let broadcasted_local_revokable_script = if let Ok(local_delayedkey) = chan_utils::derive_private_key(&self.secp_ctx, &local_tx.per_commitment_point, self.keys.delayed_payment_base_key()) { - Some((redeemscript.to_v0_p2wsh(), local_delayedkey, redeemscript)) - } else { None }; + let broadcasted_local_revokable_script = Some((redeemscript.to_v0_p2wsh(), local_tx.per_commitment_point.clone(), local_tx.revocation_key.clone())); for &(ref htlc, _, _) in local_tx.htlc_outputs.iter() { if let Some(transaction_output_index) = htlc.transaction_output_index { @@ -2121,10 +2119,11 @@ impl ChannelMonitor { if broadcasted_local_revokable_script.0 == outp.script_pubkey { spendable_output = Some(SpendableOutputDescriptor::DynamicOutputP2WSH { outpoint: BitcoinOutPoint { txid: tx.txid(), vout: i as u32 }, - key: broadcasted_local_revokable_script.1, - witness_script: broadcasted_local_revokable_script.2.clone(), + per_commitment_point: broadcasted_local_revokable_script.1, to_self_delay: self.their_to_self_delay, output: outp.clone(), + key_derivation_params: self.keys.key_derivation_params(), + remote_revocation_pubkey: broadcasted_local_revokable_script.2.clone(), }); break; } @@ -2183,9 +2182,9 @@ impl Readable for (BlockHash, ChannelMonitor let broadcasted_local_revokable_script = match ::read(reader)? { 0 => { let revokable_address = Readable::read(reader)?; - let local_delayedkey = Readable::read(reader)?; + let per_commitment_point = Readable::read(reader)?; let revokable_script = Readable::read(reader)?; - Some((revokable_address, local_delayedkey, revokable_script)) + Some((revokable_address, per_commitment_point, revokable_script)) }, 1 => { None }, _ => return Err(DecodeError::InvalidValue), diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 2311a03f..1bc38faa 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -4265,7 +4265,7 @@ fn test_manager_serialize_deserialize_inconsistent_monitor() { } macro_rules! check_spendable_outputs { - ($node: expr, $der_idx: expr) => { + ($node: expr, $der_idx: expr, $keysinterface: expr, $chan_value: expr) => { { let events = $node.chan_monitor.simple_monitor.get_and_clear_pending_events(); let mut txn = Vec::new(); @@ -4301,7 +4301,7 @@ macro_rules! check_spendable_outputs { spend_tx.input[0].witness.push(remotepubkey.serialize().to_vec()); txn.push(spend_tx); }, - SpendableOutputDescriptor::DynamicOutputP2WSH { ref outpoint, ref key, ref witness_script, ref to_self_delay, ref output } => { + SpendableOutputDescriptor::DynamicOutputP2WSH { ref outpoint, ref per_commitment_point, ref to_self_delay, ref output, ref key_derivation_params, ref remote_revocation_pubkey } => { let input = TxIn { previous_output: outpoint.clone(), script_sig: Script::new(), @@ -4319,12 +4319,18 @@ macro_rules! check_spendable_outputs { output: vec![outp], }; let secp_ctx = Secp256k1::new(); - let sighash = Message::from_slice(&bip143::SighashComponents::new(&spend_tx).sighash_all(&spend_tx.input[0], witness_script, output.value)[..]).unwrap(); - let local_delaysig = secp_ctx.sign(&sighash, key); - spend_tx.input[0].witness.push(local_delaysig.serialize_der().to_vec()); - spend_tx.input[0].witness[0].push(SigHashType::All as u8); - spend_tx.input[0].witness.push(vec!()); - spend_tx.input[0].witness.push(witness_script.clone().into_bytes()); + let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1); + if let Ok(delayed_payment_key) = chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, keys.delayed_payment_base_key()) { + + let delayed_payment_pubkey = PublicKey::from_secret_key(&secp_ctx, &delayed_payment_key); + let witness_script = chan_utils::get_revokeable_redeemscript(remote_revocation_pubkey, *to_self_delay, &delayed_payment_pubkey); + let sighash = Message::from_slice(&bip143::SighashComponents::new(&spend_tx).sighash_all(&spend_tx.input[0], &witness_script, output.value)[..]).unwrap(); + let local_delayedsig = secp_ctx.sign(&sighash, &delayed_payment_key); + spend_tx.input[0].witness.push(local_delayedsig.serialize_der().to_vec()); + spend_tx.input[0].witness[0].push(SigHashType::All as u8); + spend_tx.input[0].witness.push(vec!()); //MINIMALIF + spend_tx.input[0].witness.push(witness_script.clone().into_bytes()); + } else { panic!() } txn.push(spend_tx); }, SpendableOutputDescriptor::StaticOutput { ref outpoint, ref output } => { @@ -4397,7 +4403,7 @@ fn test_claim_sizeable_push_msat() { nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![node_txn[0].clone()] }, 0); connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); - let spend_txn = check_spendable_outputs!(nodes[1], 1); + let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000); assert_eq!(spend_txn.len(), 1); check_spends!(spend_txn[0], node_txn[0]); } @@ -4427,7 +4433,7 @@ fn test_claim_on_remote_sizeable_push_msat() { check_added_monitors!(nodes[1], 1); connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); - let spend_txn = check_spendable_outputs!(nodes[1], 1); + let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000); assert_eq!(spend_txn.len(), 2); assert_eq!(spend_txn[0], spend_txn[1]); check_spends!(spend_txn[0], node_txn[0]); @@ -4460,7 +4466,7 @@ fn test_claim_on_remote_revoked_sizeable_push_msat() { nodes[1].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone()] }, 1); connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); - let spend_txn = check_spendable_outputs!(nodes[1], 1); + let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000); assert_eq!(spend_txn.len(), 3); assert_eq!(spend_txn[0], spend_txn[1]); // to_remote output on revoked remote commitment_tx check_spends!(spend_txn[0], revoked_local_txn[0]); @@ -4511,7 +4517,7 @@ fn test_static_spendable_outputs_preimage_tx() { nodes[1].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone()] }, 1); connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); - let spend_txn = check_spendable_outputs!(nodes[1], 1); + let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000); assert_eq!(spend_txn.len(), 1); check_spends!(spend_txn[0], node_txn[0]); } @@ -4558,7 +4564,7 @@ fn test_static_spendable_outputs_timeout_tx() { connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); expect_payment_failed!(nodes[1], our_payment_hash, true); - let spend_txn = check_spendable_outputs!(nodes[1], 1); + let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000); assert_eq!(spend_txn.len(), 3); // SpendableOutput: remote_commitment_tx.to_remote (*2), timeout_tx.output (*1) check_spends!(spend_txn[2], node_txn[0].clone()); } @@ -4594,7 +4600,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_commitment_tx() { nodes[1].block_notifier.block_connected(&Block { header: header_1, txdata: vec![node_txn[0].clone()] }, 1); connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); - let spend_txn = check_spendable_outputs!(nodes[1], 1); + let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000); assert_eq!(spend_txn.len(), 1); check_spends!(spend_txn[0], node_txn[0]); } @@ -4649,7 +4655,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_timeout_tx() { connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); // Check B's ChannelMonitor was able to generate the right spendable output descriptor - let spend_txn = check_spendable_outputs!(nodes[1], 1); + let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000); assert_eq!(spend_txn.len(), 2); check_spends!(spend_txn[0], node_txn[0]); check_spends!(spend_txn[1], node_txn[2]); @@ -4699,7 +4705,7 @@ fn test_static_spendable_outputs_justice_tx_revoked_htlc_success_tx() { connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 1, true, header.bitcoin_hash()); // Check A's ChannelMonitor was able to generate the right spendable output descriptor - let spend_txn = check_spendable_outputs!(nodes[0], 1); + let spend_txn = check_spendable_outputs!(nodes[0], 1, node_cfgs[0].keys_manager, 100000); assert_eq!(spend_txn.len(), 5); // Duplicated SpendableOutput due to block rescan after revoked htlc output tracking assert_eq!(spend_txn[0], spend_txn[1]); assert_eq!(spend_txn[0], spend_txn[2]); @@ -4969,7 +4975,7 @@ fn test_dynamic_spendable_outputs_local_htlc_success_tx() { connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 201, true, header_201.bitcoin_hash()); // Verify that B is able to spend its own HTLC-Success tx thanks to spendable output event given back by its ChannelMonitor - let spend_txn = check_spendable_outputs!(nodes[1], 1); + let spend_txn = check_spendable_outputs!(nodes[1], 1, node_cfgs[1].keys_manager, 100000); assert_eq!(spend_txn.len(), 2); check_spends!(spend_txn[0], node_txn[0]); check_spends!(spend_txn[1], node_txn[1]); @@ -5266,7 +5272,7 @@ fn test_dynamic_spendable_outputs_local_htlc_timeout_tx() { expect_payment_failed!(nodes[0], our_payment_hash, true); // Verify that A is able to spend its own HTLC-Timeout tx thanks to spendable output event given back by its ChannelMonitor - let spend_txn = check_spendable_outputs!(nodes[0], 1); + let spend_txn = check_spendable_outputs!(nodes[0], 1, node_cfgs[0].keys_manager, 100000); assert_eq!(spend_txn.len(), 3); assert_eq!(spend_txn[0], spend_txn[1]); check_spends!(spend_txn[0], local_txn[0]); @@ -5289,14 +5295,14 @@ fn test_static_output_closing_tx() { nodes[0].block_notifier.block_connected(&Block { header, txdata: vec![closing_tx.clone()] }, 0); connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 0, true, header.bitcoin_hash()); - let spend_txn = check_spendable_outputs!(nodes[0], 2); + let spend_txn = check_spendable_outputs!(nodes[0], 2, node_cfgs[0].keys_manager, 100000); assert_eq!(spend_txn.len(), 1); check_spends!(spend_txn[0], closing_tx); nodes[1].block_notifier.block_connected(&Block { header, txdata: vec![closing_tx.clone()] }, 0); connect_blocks(&nodes[1].block_notifier, ANTI_REORG_DELAY - 1, 0, true, header.bitcoin_hash()); - let spend_txn = check_spendable_outputs!(nodes[1], 2); + let spend_txn = check_spendable_outputs!(nodes[1], 2, node_cfgs[1].keys_manager, 100000); assert_eq!(spend_txn.len(), 1); check_spends!(spend_txn[0], closing_tx); } @@ -7138,7 +7144,7 @@ fn test_data_loss_protect() { let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42}; nodes[0].block_notifier.block_connected(&Block { header, txdata: vec![node_txn[0].clone()]}, 0); connect_blocks(&nodes[0].block_notifier, ANTI_REORG_DELAY - 1, 0, true, header.bitcoin_hash()); - let spend_txn = check_spendable_outputs!(nodes[0], 1); + let spend_txn = check_spendable_outputs!(nodes[0], 1, node_cfgs[0].keys_manager, 100000); assert_eq!(spend_txn.len(), 1); check_spends!(spend_txn[0], node_txn[0]); } -- 2.30.2