From 890e3cb165196b6a5b7ad68dbafea7ed35c8a50a Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 26 Mar 2018 14:03:59 -0400 Subject: [PATCH] Update for new rust-bitcoin API, avoid some duplicate hashing --- Cargo.toml | 2 +- fuzz/Cargo.toml | 2 +- fuzz/fuzz_targets/channel_target.rs | 2 +- src/ln/channel.rs | 83 ++++++++++++++--------------- src/ln/channelmanager.rs | 2 +- src/ln/channelmonitor.rs | 27 +++++----- 6 files changed, 57 insertions(+), 61 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f5aa85db..3e07441c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,7 @@ non_bitcoin_chain_hash_routing = [] fuzztarget = ["secp256k1/fuzztarget", "bitcoin/fuzztarget"] [dependencies] -bitcoin = "0.12" +bitcoin = { git = "https://github.com/rust-bitcoin/rust-bitcoin" } rust-crypto = "0.2" rand = "0.4" secp256k1 = "0.9" diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 6b45066e..7ed7533a 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -13,7 +13,7 @@ honggfuzz_fuzz = ["honggfuzz"] [dependencies] lightning = { path = "..", features = ["fuzztarget"] } -bitcoin = { version = "0.12", features = ["fuzztarget"] } +bitcoin = { git = "https://github.com/rust-bitcoin/rust-bitcoin", features = ["fuzztarget"] } secp256k1 = { version = "0.9", features = ["fuzztarget"] } honggfuzz = { version = "0.5", optional = true } afl = { version = "0.3", optional = true } diff --git a/fuzz/fuzz_targets/channel_target.rs b/fuzz/fuzz_targets/channel_target.rs index ec82fbd6..0dd0d6a7 100644 --- a/fuzz/fuzz_targets/channel_target.rs +++ b/fuzz/fuzz_targets/channel_target.rs @@ -163,7 +163,7 @@ pub fn do_test(data: &[u8]) { let their_pubkey = get_pubkey!(); - let tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: Vec::new(), witness: Vec::new() }; + let tx = Transaction { version: 0, lock_time: 0, input: Vec::new(), output: Vec::new() }; let funding_output = (Sha256dHash::from_data(&serialize(&tx).unwrap()[..]), 0); let mut channel = if get_slice!(1)[0] != 0 { diff --git a/src/ln/channel.rs b/src/ln/channel.rs index fe1cfd24..6f51c0a6 100644 --- a/src/ln/channel.rs +++ b/src/ln/channel.rs @@ -511,12 +511,11 @@ impl Channel { prev_hash: self.channel_monitor.get_funding_txo().unwrap().0, prev_index: self.channel_monitor.get_funding_txo().unwrap().1 as u32, script_sig: Script::new(), - sequence: ((0x80 as u32) << 8*3) | ((obscured_commitment_transaction_number >> 3*8) as u32) + sequence: ((0x80 as u32) << 8*3) | ((obscured_commitment_transaction_number >> 3*8) as u32), + witness: Vec::new(), }); ins }; - let mut witness: Vec>> = Vec::new(); - witness.push(Vec::new()); let mut txouts: Vec<(TxOut, Option)> = Vec::new(); @@ -596,7 +595,6 @@ impl Channel { lock_time: ((0x20 as u32) << 8*3) | ((obscured_commitment_transaction_number & 0xffffffu64) as u32), input: txins, output: outputs, - witness: witness }, htlcs_used)) } @@ -646,30 +644,30 @@ impl Channel { if tx.input.len() != 1 { panic!("Tried to sign commitment transaction that had input count != 1!"); } - if tx.witness.len() != 1 || tx.witness[0].len() != 0 { + if tx.input[0].witness.len() != 0 { panic!("Tried to re-sign commitment transaction"); } let funding_redeemscript = self.get_funding_redeemscript(); - let sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&tx).sighash_all(&tx, 0, &funding_redeemscript, self.channel_value_satoshis)[..])); + let sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&tx).sighash_all(&tx.input[0], &funding_redeemscript, self.channel_value_satoshis)[..])); let our_sig = secp_call!(self.secp_ctx.sign(&sighash, &self.local_keys.funding_key)); - tx.witness[0].push(Vec::new()); // First is the multisig dummy + tx.input[0].witness.push(Vec::new()); // First is the multisig dummy let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, &self.local_keys.funding_key).unwrap().serialize(); let their_funding_key = self.their_funding_pubkey.serialize(); if our_funding_key[..] < their_funding_key[..] { - tx.witness[0].push(our_sig.serialize_der(&self.secp_ctx).to_vec()); - tx.witness[0].push(their_sig.serialize_der(&self.secp_ctx).to_vec()); + tx.input[0].witness.push(our_sig.serialize_der(&self.secp_ctx).to_vec()); + tx.input[0].witness.push(their_sig.serialize_der(&self.secp_ctx).to_vec()); } else { - tx.witness[0].push(their_sig.serialize_der(&self.secp_ctx).to_vec()); - tx.witness[0].push(our_sig.serialize_der(&self.secp_ctx).to_vec()); + tx.input[0].witness.push(their_sig.serialize_der(&self.secp_ctx).to_vec()); + tx.input[0].witness.push(our_sig.serialize_der(&self.secp_ctx).to_vec()); } - tx.witness[0][1].push(SigHashType::All as u8); - tx.witness[0][2].push(SigHashType::All as u8); + tx.input[0].witness[1].push(SigHashType::All as u8); + tx.input[0].witness[2].push(SigHashType::All as u8); - tx.witness[0].push(funding_redeemscript.into_vec()); + tx.input[0].witness.push(funding_redeemscript.into_vec()); Ok(()) } @@ -683,12 +681,10 @@ impl Channel { prev_hash: prev_hash.clone(), prev_index: htlc.transaction_output_index, script_sig: Script::new(), - sequence: 0 + sequence: 0, + witness: Vec::new(), }); - let mut witnesses: Vec>> = Vec::new(); - witnesses.push(Vec::new()); - let total_fee = if htlc.offered { self.feerate_per_kw * HTLC_TIMEOUT_TX_WEIGHT / 1000 } else { @@ -708,7 +704,6 @@ impl Channel { lock_time: if htlc.offered { htlc.cltv_expiry } else { 0 }, input: txins, output: txouts, - witness: witnesses }) } @@ -718,37 +713,37 @@ impl Channel { if tx.input.len() != 1 { panic!("Tried to sign HTLC transaction that had input count != 1!"); } - if tx.witness.len() != 1 || tx.witness[0].len() != 0 { + if tx.input[0].witness.len() != 0 { panic!("Tried to re-sign HTLC transaction"); } let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &keys, htlc.offered); let our_htlc_key = secp_call!(chan_utils::derive_private_key(&self.secp_ctx, &keys.per_commitment_point, &self.local_keys.htlc_base_key)); - let sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&tx).sighash_all(&tx, 0, &htlc_redeemscript, htlc.amount_msat / 1000)[..])); + let sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&tx).sighash_all(&tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..])); let our_sig = secp_call!(self.secp_ctx.sign(&sighash, &our_htlc_key)); let local_tx = PublicKey::from_secret_key(&self.secp_ctx, &our_htlc_key).unwrap() == keys.a_htlc_key; - tx.witness[0].push(Vec::new()); // First is the multisig dummy + tx.input[0].witness.push(Vec::new()); // First is the multisig dummy if local_tx { // b, then a - tx.witness[0].push(their_sig.serialize_der(&self.secp_ctx).to_vec()); - tx.witness[0].push(our_sig.serialize_der(&self.secp_ctx).to_vec()); + tx.input[0].witness.push(their_sig.serialize_der(&self.secp_ctx).to_vec()); + tx.input[0].witness.push(our_sig.serialize_der(&self.secp_ctx).to_vec()); } else { - tx.witness[0].push(our_sig.serialize_der(&self.secp_ctx).to_vec()); - tx.witness[0].push(their_sig.serialize_der(&self.secp_ctx).to_vec()); + tx.input[0].witness.push(our_sig.serialize_der(&self.secp_ctx).to_vec()); + tx.input[0].witness.push(their_sig.serialize_der(&self.secp_ctx).to_vec()); } - tx.witness[0][1].push(SigHashType::All as u8); - tx.witness[0][2].push(SigHashType::All as u8); + tx.input[0].witness[1].push(SigHashType::All as u8); + tx.input[0].witness[2].push(SigHashType::All as u8); if htlc.offered { - tx.witness[0].push(Vec::new()); + tx.input[0].witness.push(Vec::new()); } else { - tx.witness[0].push(preimage.unwrap().to_vec()); + tx.input[0].witness.push(preimage.unwrap().to_vec()); } - tx.witness[0].push(htlc_redeemscript.into_vec()); + tx.input[0].witness.push(htlc_redeemscript.into_vec()); Ok(()) } @@ -880,11 +875,11 @@ impl Channel { let remote_keys = self.build_remote_transaction_keys()?; let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false)?.0; - let remote_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&remote_initial_commitment_tx).sighash_all(&remote_initial_commitment_tx, 0, &funding_script, self.channel_value_satoshis)[..])); + let remote_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&remote_initial_commitment_tx).sighash_all(&remote_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..])); let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number)?; let local_initial_commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false)?.0; - let local_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&local_initial_commitment_tx).sighash_all(&local_initial_commitment_tx, 0, &funding_script, self.channel_value_satoshis)[..])); + let local_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&local_initial_commitment_tx).sighash_all(&local_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..])); // They sign the "local" commitment transaction, allowing us to broadcast the tx if we wish. secp_call!(self.secp_ctx.verify(&local_sighash, &sig, &self.their_funding_pubkey)); @@ -946,7 +941,7 @@ impl Channel { let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number)?; let local_initial_commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false)?.0; - let local_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&local_initial_commitment_tx).sighash_all(&local_initial_commitment_tx, 0, &funding_script, self.channel_value_satoshis)[..])); + let local_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&local_initial_commitment_tx).sighash_all(&local_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..])); // They sign the "local" commitment transaction, allowing us to broadcast the tx if we wish. secp_call!(self.secp_ctx.verify(&local_sighash, &msg.signature, &self.their_funding_pubkey)); @@ -1167,7 +1162,8 @@ impl Channel { let local_keys = self.build_local_transaction_keys(self.cur_local_commitment_transaction_number)?; let local_commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false)?; - let local_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&local_commitment_tx.0).sighash_all(&local_commitment_tx.0, 0, &funding_script, self.channel_value_satoshis)[..])); + let local_commitment_txid = local_commitment_tx.0.txid(); + let local_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&local_commitment_tx.0).sighash_all(&local_commitment_tx.0.input[0], &funding_script, self.channel_value_satoshis)[..])); secp_call!(self.secp_ctx.verify(&local_sighash, &msg.signature, &self.their_funding_pubkey)); if msg.htlc_signatures.len() != local_commitment_tx.1.len() { @@ -1175,9 +1171,9 @@ impl Channel { } for (idx, ref htlc) in local_commitment_tx.1.iter().enumerate() { - let htlc_tx = self.build_htlc_transaction(&local_commitment_tx.0.txid(), htlc, true, &local_keys)?; + let htlc_tx = self.build_htlc_transaction(&local_commitment_txid, htlc, true, &local_keys)?; let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &local_keys, htlc.offered); - let htlc_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx, 0, &htlc_redeemscript, htlc.amount_msat / 1000)[..])); + let htlc_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..])); secp_call!(self.secp_ctx.verify(&htlc_sighash, &msg.htlc_signatures[idx], &local_keys.b_htlc_key)); } @@ -1464,7 +1460,7 @@ impl Channel { let remote_keys = self.build_remote_transaction_keys()?; let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false)?.0; - let remote_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&remote_initial_commitment_tx).sighash_all(&remote_initial_commitment_tx, 0, &funding_script, self.channel_value_satoshis)[..])); + let remote_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&remote_initial_commitment_tx).sighash_all(&remote_initial_commitment_tx.input[0], &funding_script, self.channel_value_satoshis)[..])); // We sign the "remote" commitment transaction, allowing them to broadcast the tx if they wish. Ok(secp_call!(self.secp_ctx.sign(&remote_sighash, &self.local_keys.funding_key))) @@ -1639,15 +1635,16 @@ impl Channel { let remote_keys = self.build_remote_transaction_keys()?; let remote_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, true)?; - let remote_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&remote_commitment_tx.0).sighash_all(&remote_commitment_tx.0, 0, &funding_script, self.channel_value_satoshis)[..])); + let remote_commitment_txid = remote_commitment_tx.0.txid(); + let remote_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&remote_commitment_tx.0).sighash_all(&remote_commitment_tx.0.input[0], &funding_script, self.channel_value_satoshis)[..])); let our_sig = secp_call!(self.secp_ctx.sign(&remote_sighash, &self.local_keys.funding_key)); let mut htlc_sigs = Vec::new(); for ref htlc in remote_commitment_tx.1.iter() { - let htlc_tx = self.build_htlc_transaction(&remote_commitment_tx.0.txid(), htlc, false, &remote_keys)?; + let htlc_tx = self.build_htlc_transaction(&remote_commitment_txid, htlc, false, &remote_keys)?; let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &remote_keys, htlc.offered); - let htlc_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx, 0, &htlc_redeemscript, htlc.amount_msat / 1000)[..])); + let htlc_sighash = secp_call!(Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..])); let our_htlc_key = secp_call!(chan_utils::derive_private_key(&self.secp_ctx, &remote_keys.per_commitment_point, &self.local_keys.htlc_base_key)); htlc_sigs.push(secp_call!(self.secp_ctx.sign(&htlc_sighash, &our_htlc_key))); } @@ -1749,7 +1746,7 @@ mod tests { ( $their_sig_hex: expr, $our_sig_hex: expr, $tx_hex: expr) => { unsigned_tx = chan.build_commitment_transaction(42, &keys, true, false).unwrap(); let their_signature = Signature::from_der(&secp_ctx, &hex_bytes($their_sig_hex).unwrap()[..]).unwrap(); - let sighash = Message::from_slice(&bip143::SighashComponents::new(&unsigned_tx.0).sighash_all(&unsigned_tx.0, 0, &chan.get_funding_redeemscript(), chan.channel_value_satoshis)[..]).unwrap(); + let sighash = Message::from_slice(&bip143::SighashComponents::new(&unsigned_tx.0).sighash_all(&unsigned_tx.0.input[0], &chan.get_funding_redeemscript(), chan.channel_value_satoshis)[..]).unwrap(); secp_ctx.verify(&sighash, &their_signature, &chan.their_funding_pubkey).unwrap(); chan.sign_commitment_transaction(&mut unsigned_tx.0, &their_signature).unwrap(); @@ -1766,7 +1763,7 @@ mod tests { let ref htlc = unsigned_tx.1[$htlc_idx]; let mut htlc_tx = chan.build_htlc_transaction(&unsigned_tx.0.txid(), &htlc, true, &keys).unwrap(); let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, &keys, htlc.offered); - let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx, 0, &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap(); + let htlc_sighash = Message::from_slice(&bip143::SighashComponents::new(&htlc_tx).sighash_all(&htlc_tx.input[0], &htlc_redeemscript, htlc.amount_msat / 1000)[..]).unwrap(); secp_ctx.verify(&htlc_sighash, &remote_signature, &keys.b_htlc_key).unwrap(); let mut preimage: Option<[u8; 32]> = None; diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs index ff6f924c..96764523 100644 --- a/src/ln/channelmanager.rs +++ b/src/ln/channelmanager.rs @@ -1576,7 +1576,7 @@ mod tests { node_a.handle_accept_channel(&node_b.get_our_node_id(), &accept_chan).unwrap(); let chan_id = unsafe { CHAN_COUNT }; - let tx = Transaction { version: chan_id as u32, lock_time: 0, input: Vec::new(), output: Vec::new(), witness: Vec::new() }; + let tx = Transaction { version: chan_id as u32, lock_time: 0, input: Vec::new(), output: Vec::new() }; let funding_output = (Sha256dHash::from_data(&serialize(&tx).unwrap()[..]), chan_id); let events_1 = node_a.get_and_clear_pending_events(); diff --git a/src/ln/channelmonitor.rs b/src/ln/channelmonitor.rs index 5ac9a6b0..53440f43 100644 --- a/src/ln/channelmonitor.rs +++ b/src/ln/channelmonitor.rs @@ -323,6 +323,7 @@ impl ChannelMonitor { prev_index: per_commitment_data.revoked_output_index, script_sig: Script::new(), sequence: 0xffffffff, + witness: Vec::new(), }); values.push(tx.output[per_commitment_data.revoked_output_index as usize].value); total_value += tx.output[per_commitment_data.revoked_output_index as usize].value; @@ -340,6 +341,7 @@ impl ChannelMonitor { prev_index: htlc.transaction_output_index, script_sig: Script::new(), sequence: 0xffffffff, + witness: Vec::new(), }); values.push(tx.output[htlc.transaction_output_index as usize].value); total_value += htlc.amount_msat / 1000; @@ -358,6 +360,7 @@ impl ChannelMonitor { prev_index: idx as u32, script_sig: Script::new(), sequence: 0xffffffff, + witness: Vec::new(), }); values.push(outp.value); total_value += outp.value; @@ -378,18 +381,16 @@ impl ChannelMonitor { lock_time: 0, input: inputs, output: outputs, - witness: Vec::new(), }; let mut values_drain = values.drain(..); // First input is the generic revokeable_redeemscript - // TODO: Make one SighashComponents and use that throughout instead of re-building it - // each time. + let sighash_parts = bip143::SighashComponents::new(&spend_tx); { let sig = match self.revocation_base_key { RevocationStorage::PrivMode { ref revocation_base_key } => { - let sighash = ignore_error!(Message::from_slice(&bip143::SighashComponents::new(&spend_tx).sighash_all(&spend_tx, 0, &revokeable_redeemscript, values_drain.next().unwrap())[..])); + let sighash = ignore_error!(Message::from_slice(&sighash_parts.sighash_all(&spend_tx.input[0], &revokeable_redeemscript, values_drain.next().unwrap())[..])); let revocation_key = ignore_error!(chan_utils::derive_private_revocation_key(&self.secp_ctx, &per_commitment_key, &revocation_base_key)); ignore_error!(self.secp_ctx.sign(&sighash, &revocation_key)) }, @@ -398,17 +399,16 @@ impl ChannelMonitor { } }; - spend_tx.witness.push(Vec::new()); - spend_tx.witness[0].push(sig.serialize_der(&self.secp_ctx).to_vec()); - spend_tx.witness[0][0].push(SigHashType::All as u8); - spend_tx.witness[0].push(vec!(1)); // First if branch is revocation_key + spend_tx.input[0].witness.push(sig.serialize_der(&self.secp_ctx).to_vec()); + spend_tx.input[0].witness[0].push(SigHashType::All as u8); + spend_tx.input[0].witness.push(vec!(1)); // First if branch is revocation_key } match self.claimable_outpoints.get(&commitment_txid) { None => {}, Some(per_commitment_data) => { let mut htlc_idx = 0; - for (idx, _) in spend_tx.input.iter().enumerate() { + for (idx, input) in spend_tx.input.iter_mut().enumerate() { if idx == 0 { continue; } // We already signed the first input let mut htlc; @@ -421,7 +421,7 @@ impl ChannelMonitor { let sig = match self.revocation_base_key { RevocationStorage::PrivMode { ref revocation_base_key } => { let htlc_redeemscript = chan_utils::get_htlc_redeemscript_with_explicit_keys(htlc, &a_htlc_key, &b_htlc_key, &revocation_pubkey, htlc.offered); - let sighash = ignore_error!(Message::from_slice(&bip143::SighashComponents::new(&spend_tx).sighash_all(&spend_tx, idx, &htlc_redeemscript, values_drain.next().unwrap())[..])); + let sighash = ignore_error!(Message::from_slice(&sighash_parts.sighash_all(&input, &htlc_redeemscript, values_drain.next().unwrap())[..])); let revocation_key = ignore_error!(chan_utils::derive_private_revocation_key(&self.secp_ctx, &per_commitment_key, &revocation_base_key)); ignore_error!(self.secp_ctx.sign(&sighash, &revocation_key)) @@ -431,10 +431,9 @@ impl ChannelMonitor { } }; - spend_tx.witness.push(Vec::new()); - spend_tx.witness[0].push(revocation_pubkey.serialize().to_vec()); // First if branch is revocation_key - spend_tx.witness[0].push(sig.serialize_der(&self.secp_ctx).to_vec()); - spend_tx.witness[0][0].push(SigHashType::All as u8); + input.witness.push(revocation_pubkey.serialize().to_vec()); // First if branch is revocation_key + input.witness.push(sig.serialize_der(&self.secp_ctx).to_vec()); + input.witness[0].push(SigHashType::All as u8); } } } -- 2.30.2