Merge pull request #253 from TheBlueMatt/2018-11-misc-tweaks

[rust-lightning] / src / ln / channel.rs

diff --git a/src/ln/channel.rs b/src/ln/channel.rs
index b9a81f6ebc0465cf34396eb44073b790fd7c3925..63d8754d903e9c79412b8b993fd3645b3fb68cb7 100644 (file)
--- a/src/ln/channel.rs
+++ b/src/ln/channel.rs
@@ -2,11 +2,9 @@ use bitcoin::blockdata::block::BlockHeader;
 use bitcoin::blockdata::script::{Script,Builder};
 use bitcoin::blockdata::transaction::{TxIn, TxOut, Transaction, SigHashType};
 use bitcoin::blockdata::opcodes;
-use bitcoin::util::hash::{Sha256dHash, Hash160};
+use bitcoin::util::hash::{BitcoinHash, Sha256dHash, Hash160};
 use bitcoin::util::bip143;
-use bitcoin::network;
-use bitcoin::network::serialize::{BitcoinHash, RawDecoder, RawEncoder};
-use bitcoin::network::encodable::{ConsensusEncodable, ConsensusDecodable};
+use bitcoin::consensus::encode::{self, Encodable, Decodable};
 
 use secp256k1::key::{PublicKey,SecretKey};
 use secp256k1::{Secp256k1,Message,Signature};
@@ -1122,8 +1120,9 @@ impl Channel {
                Ok(our_sig)
        }
 
-       /// May return an IgnoreError, but should not, and will always return Ok(_) when
-       /// debug_assertions are turned on
+       /// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
+       /// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
+       /// Ok(_) if debug assertions are turned on and preconditions are met.
        fn get_update_fulfill_htlc(&mut self, htlc_id_arg: u64, payment_preimage_arg: [u8; 32]) -> Result<(Option<msgs::UpdateFulfillHTLC>, Option<ChannelMonitor>), HandleError> {
                // Either ChannelFunded got set (which means it wont bet unset) or there is no way any
                // caller thought we could have something claimed (cause we wouldn't have accepted in an
@@ -1175,7 +1174,9 @@ impl Channel {
                                        &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, .. } => {
                                                if htlc_id_arg == htlc_id {
                                                        debug_assert!(false, "Tried to fulfill an HTLC we already had a holding-cell failure on");
-                                                       return Err(HandleError{err: "Unable to find a pending HTLC which matched the given HTLC ID", action: Some(msgs::ErrorAction::IgnoreError)});
+                                                       // Return the new channel monitor in a last-ditch effort to hit the
+                                                       // chain and claim the funds
+                                                       return Ok((None, Some(self.channel_monitor.clone())));
                                                }
                                        },
                                        _ => {}
@@ -1215,8 +1216,9 @@ impl Channel {
                }
        }
 
-       /// May return an IgnoreError, but should not, and will always return Ok(_) when
-       /// debug_assertions are turned on
+       /// Per HTLC, only one get_update_fail_htlc or get_update_fulfill_htlc call may be made.
+       /// In such cases we debug_assert!(false) and return an IgnoreError. Thus, will always return
+       /// Ok(_) if debug assertions are turned on and preconditions are met.
        pub fn get_update_fail_htlc(&mut self, htlc_id_arg: u64, err_packet: msgs::OnionErrorPacket) -> Result<Option<msgs::UpdateFailHTLC>, HandleError> {
                if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
                        panic!("Was asked to fail an HTLC when channel was not in an operational state");
@@ -2823,6 +2825,16 @@ impl Channel {
                                                self.channel_update_count += 1;
                                                return Err(HandleError{err: "funding tx had wrong script/value", action: Some(ErrorAction::DisconnectPeer{msg: None})});
                                        } else {
+                                               if self.channel_outbound {
+                                                       for input in tx.input.iter() {
+                                                               if input.witness.is_empty() {
+                                                                       // We generated a malleable funding transaction, implying we've
+                                                                       // just exposed ourselves to funds loss to our counterparty.
+                                                                       #[cfg(not(feature = "fuzztarget"))]
+                                                                       panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
+                                                               }
+                                                       }
+                                               }
                                                self.funding_tx_confirmations = 1;
                                                self.short_channel_id = Some(((height as u64)          << (5*8)) |
                                                                             ((*index_in_block as u64) << (2*8)) |
@@ -3511,9 +3523,9 @@ impl Writeable for Channel {
 
                (self.last_local_commitment_txn.len() as u64).write(writer)?;
                for tx in self.last_local_commitment_txn.iter() {
-                       if let Err(e) = tx.consensus_encode(&mut RawEncoder::new(WriterWriteAdaptor(writer))) {
+                       if let Err(e) = tx.consensus_encode(&mut WriterWriteAdaptor(writer)) {
                                match e {
-                                       network::serialize::Error::Io(e) => return Err(e),
+                                       encode::Error::Io(e) => return Err(e),
                                        _ => panic!("last_local_commitment_txn must have been well-formed!"),
                                }
                        }
@@ -3690,7 +3702,7 @@ impl<R : ::std::io::Read> ReadableArgs<R, Arc<Logger>> for Channel {
                let last_local_commitment_txn_count: u64 = Readable::read(reader)?;
                let mut last_local_commitment_txn = Vec::with_capacity(cmp::min(last_local_commitment_txn_count as usize, OUR_MAX_HTLCS as usize*2 + 1));
                for _ in 0..last_local_commitment_txn_count {
-                       last_local_commitment_txn.push(match Transaction::consensus_decode(&mut RawDecoder::new(reader.by_ref())) {
+                       last_local_commitment_txn.push(match Transaction::consensus_decode(reader.by_ref()) {
                                Ok(tx) => tx,
                                Err(_) => return Err(DecodeError::InvalidValue),
                        });
@@ -3818,7 +3830,7 @@ impl<R : ::std::io::Read> ReadableArgs<R, Arc<Logger>> for Channel {
 mod tests {
        use bitcoin::util::hash::{Sha256dHash, Hash160};
        use bitcoin::util::bip143;
-       use bitcoin::network::serialize::serialize;
+       use bitcoin::consensus::encode::serialize;
        use bitcoin::blockdata::script::{Script, Builder};
        use bitcoin::blockdata::transaction::Transaction;
        use bitcoin::blockdata::opcodes;
@@ -3940,7 +3952,7 @@ mod tests {
 
                                chan.sign_commitment_transaction(&mut unsigned_tx.0, &their_signature);
 
-                               assert_eq!(serialize(&unsigned_tx.0).unwrap()[..],
+                               assert_eq!(serialize(&unsigned_tx.0)[..],
                                                hex::decode($tx_hex).unwrap()[..]);
                        };
                }
@@ -3973,7 +3985,7 @@ mod tests {
                                }
 
                                chan.sign_htlc_transaction(&mut htlc_tx, &remote_signature, &preimage, &htlc, &keys).unwrap();
-                               assert_eq!(serialize(&htlc_tx).unwrap()[..],
+                               assert_eq!(serialize(&htlc_tx)[..],
                                                hex::decode($tx_hex).unwrap()[..]);
                        };
                }