X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fchain%2Fchannelmonitor.rs;h=48f821d5f0cb712292fee1afda7e94e627c85a36;hb=d9ba7ce8bfe0d2bd074bde19d03f33bfd45a57bb;hp=a61e6afbf26e1e0c12fa84bde8b868f6ae5826d4;hpb=44d1dfa23dcb19be6f3a02f7e18b3b905c1d4683;p=rust-lightning diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index a61e6afb..48f821d5 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -869,6 +869,9 @@ impl Writeable for ChannelMonitorImpl { writer.write_all(&txid[..])?; writer.write_all(&byte_utils::be64_to_array(htlc_infos.len() as u64))?; for &(ref htlc_output, ref htlc_source) in htlc_infos.iter() { + debug_assert!(htlc_source.is_none() || Some(**txid) == self.current_counterparty_commitment_txid + || Some(**txid) == self.prev_counterparty_commitment_txid, + "HTLC Sources for all revoked commitment transactions should be none!"); serialize_htlc_in_commitment!(htlc_output); htlc_source.as_ref().map(|b| b.as_ref()).write(writer)?; } @@ -1676,9 +1679,14 @@ macro_rules! fail_unbroadcast_htlcs { // cannot currently change after channel initialization, so we don't // need to here. let confirmed_htlcs_iter: &mut Iterator)> = &mut $confirmed_htlcs_list; + let mut matched_htlc = false; for (ref broadcast_htlc, ref broadcast_source) in confirmed_htlcs_iter { - if broadcast_htlc.transaction_output_index.is_some() && Some(&**source) == *broadcast_source { + if broadcast_htlc.transaction_output_index.is_some() && + (Some(&**source) == *broadcast_source || + (broadcast_source.is_none() && + broadcast_htlc.payment_hash == htlc.payment_hash && + broadcast_htlc.amount_msat == htlc.amount_msat)) { matched_htlc = true; break; } @@ -1721,6 +1729,26 @@ macro_rules! fail_unbroadcast_htlcs { } } } +// In the `test_invalid_funding_tx` test, we need a bogus script which matches the HTLC-Accepted +// witness length match (ie is 136 bytes long). We generate one here which we also use in some +// in-line tests later. + +#[cfg(test)] +pub fn deliberately_bogus_accepted_htlc_witness_program() -> Vec { + let mut ret = [opcodes::all::OP_NOP.into_u8(); 136]; + ret[131] = opcodes::all::OP_DROP.into_u8(); + ret[132] = opcodes::all::OP_DROP.into_u8(); + ret[133] = opcodes::all::OP_DROP.into_u8(); + ret[134] = opcodes::all::OP_DROP.into_u8(); + ret[135] = opcodes::OP_TRUE.into_u8(); + Vec::from(&ret[..]) +} + +#[cfg(test)] +pub fn deliberately_bogus_accepted_htlc_witness() -> Vec> { + vec![Vec::new(), Vec::new(), Vec::new(), Vec::new(), deliberately_bogus_accepted_htlc_witness_program().into()].into() +} + impl ChannelMonitorImpl { /// Inserts a revocation secret into this channel monitor. Prunes old preimages if neither /// needed by holder commitment transactions HTCLs nor by counterparty ones. Unless we haven't already seen @@ -2102,8 +2130,16 @@ impl ChannelMonitorImpl { } self.counterparty_commitment_txn_on_chain.insert(commitment_txid, commitment_number); - fail_unbroadcast_htlcs!(self, "revoked counterparty", commitment_txid, height, - [].iter().map(|a| *a), logger); + if let Some(per_commitment_data) = per_commitment_option { + fail_unbroadcast_htlcs!(self, "revoked_counterparty", commitment_txid, height, + per_commitment_data.iter().map(|(htlc, htlc_source)| + (htlc, htlc_source.as_ref().map(|htlc_source| htlc_source.as_ref())) + ), logger); + } else { + debug_assert!(false, "We should have per-commitment option for any recognized old commitment txn"); + fail_unbroadcast_htlcs!(self, "revoked counterparty", commitment_txid, height, + [].iter().map(|reference| *reference), logger); + } } } else if let Some(per_commitment_data) = per_commitment_option { // While this isn't useful yet, there is a potential race where if a counterparty @@ -2685,14 +2721,21 @@ impl ChannelMonitorImpl { if *idx == input.previous_output.vout { #[cfg(test)] { - // If the expected script is a known type, check that the witness - // appears to be spending the correct type (ie that the match would - // actually succeed in BIP 158/159-style filters). - if _script_pubkey.is_v0_p2wsh() { - assert_eq!(&bitcoin::Address::p2wsh(&Script::from(input.witness.last().unwrap().to_vec()), bitcoin::Network::Bitcoin).script_pubkey(), _script_pubkey); - } else if _script_pubkey.is_v0_p2wpkh() { - assert_eq!(&bitcoin::Address::p2wpkh(&bitcoin::PublicKey::from_slice(&input.witness.last().unwrap()).unwrap(), bitcoin::Network::Bitcoin).unwrap().script_pubkey(), _script_pubkey); - } else { panic!(); } + // If the expected script is a known type, check that the witness + // appears to be spending the correct type (ie that the match would + // actually succeed in BIP 158/159-style filters). + if _script_pubkey.is_v0_p2wsh() { + if input.witness.last().unwrap().to_vec() == deliberately_bogus_accepted_htlc_witness_program() { + // In at least one test we use a deliberately bogus witness + // script which hit an old panic. Thus, we check for that here + // and avoid the assert if its the expected bogus script. + return true; + } + + assert_eq!(&bitcoin::Address::p2wsh(&Script::from(input.witness.last().unwrap().to_vec()), bitcoin::Network::Bitcoin).script_pubkey(), _script_pubkey); + } else if _script_pubkey.is_v0_p2wpkh() { + assert_eq!(&bitcoin::Address::p2wpkh(&bitcoin::PublicKey::from_slice(&input.witness.last().unwrap()).unwrap(), bitcoin::Network::Bitcoin).unwrap().script_pubkey(), _script_pubkey); + } else { panic!(); } } return true; } @@ -2777,10 +2820,13 @@ impl ChannelMonitorImpl { let prev_last_witness_len = input.witness.second_to_last().map(|w| w.len()).unwrap_or(0); let revocation_sig_claim = (witness_items == 3 && htlctype == Some(HTLCType::OfferedHTLC) && prev_last_witness_len == 33) || (witness_items == 3 && htlctype == Some(HTLCType::AcceptedHTLC) && prev_last_witness_len == 33); - let accepted_preimage_claim = witness_items == 5 && htlctype == Some(HTLCType::AcceptedHTLC); + let accepted_preimage_claim = witness_items == 5 && htlctype == Some(HTLCType::AcceptedHTLC) + && input.witness.second_to_last().unwrap().len() == 32; #[cfg(not(fuzzing))] let accepted_timeout_claim = witness_items == 3 && htlctype == Some(HTLCType::AcceptedHTLC) && !revocation_sig_claim; - let offered_preimage_claim = witness_items == 3 && htlctype == Some(HTLCType::OfferedHTLC) && !revocation_sig_claim; + let offered_preimage_claim = witness_items == 3 && htlctype == Some(HTLCType::OfferedHTLC) && + !revocation_sig_claim && input.witness.second_to_last().unwrap().len() == 32; + #[cfg(not(fuzzing))] let offered_timeout_claim = witness_items == 5 && htlctype == Some(HTLCType::OfferedHTLC);