Merge pull request #269 from TheBlueMatt/2018-12-198-review

Detect HTLC-resolving on-chain actions and pass them to ChannelManager

Author: TheBlueMatt

fuzz/fuzz_targets/full_stack_target.rs

@@ -3,6 +3,8 @@ use bitcoin::blockdata::opcodes;
 use bitcoin::blockdata::transaction::{TxIn,TxOut,OutPoint,Transaction};
 use bitcoin::util::hash::{Hash160,Sha256dHash};
 
+use ln::channelmanager::PaymentHash;
+
 use secp256k1::key::{PublicKey,SecretKey};
 use secp256k1::Secp256k1;
 use secp256k1;
@@ -156,15 +158,15 @@ pub struct HTLCOutputInCommitment {
 	pub offered: bool,
 	pub amount_msat: u64,
 	pub cltv_expiry: u32,
-	pub payment_hash: [u8; 32],
+	pub payment_hash: PaymentHash,
 	pub transaction_output_index: u32,
 }
 
 #[inline]
 pub fn get_htlc_redeemscript_with_explicit_keys(htlc: &HTLCOutputInCommitment, a_htlc_key: &PublicKey, b_htlc_key: &PublicKey, revocation_key: &PublicKey) -> Script {
 	let payment_hash160 = {
 		let mut ripemd = Ripemd160::new();
-		ripemd.input(&htlc.payment_hash);
+		ripemd.input(&htlc.payment_hash.0[..]);
 		let mut res = [0; 20];
 		ripemd.result(&mut res);
 		res

src/ln/chan_utils.rs

@@ -29,6 +29,8 @@ use std::result::Result;
 use util::{byte_utils, events};
 use util::ser::{Readable, Writeable, Writer};
 
+use ln::channelmanager::{PaymentPreimage, PaymentHash};
+
 /// An error in decoding a message or struct.
 #[derive(Debug)]
 pub enum DecodeError {
@@ -256,7 +258,7 @@ pub struct UpdateAddHTLC {
 	pub(crate) channel_id: [u8; 32],
 	pub(crate) htlc_id: u64,
 	pub(crate) amount_msat: u64,
-	pub(crate) payment_hash: [u8; 32],
+	pub(crate) payment_hash: PaymentHash,
 	pub(crate) cltv_expiry: u32,
 	pub(crate) onion_routing_packet: OnionPacket,
 }
@@ -266,7 +268,7 @@ pub struct UpdateAddHTLC {
 pub struct UpdateFulfillHTLC {
 	pub(crate) channel_id: [u8; 32],
 	pub(crate) htlc_id: u64,
-	pub(crate) payment_preimage: [u8; 32],
+	pub(crate) payment_preimage: PaymentPreimage,
 }
 
 /// An update_fail_htlc message to be sent or received from a peer

src/ln/channel.rs

@@ -25,7 +25,7 @@ use std::collections::btree_map::Entry as BtreeEntry;
 use std;
 
 /// A hop in a route
-#[derive(Clone)]
+#[derive(Clone, PartialEq)]
 pub struct RouteHop {
 	/// The node_id of the node at this hop.
 	pub pubkey: PublicKey,
@@ -39,7 +39,7 @@ pub struct RouteHop {
 }
 
 /// A route from us through the network to a destination
-#[derive(Clone)]
+#[derive(Clone, PartialEq)]
 pub struct Route {
 	/// The list of hops, NOT INCLUDING our own, where the last hop is the destination. Thus, this
 	/// must always be at least length one. By protocol rules, this may not currently exceed 20 in

src/ln/channelmanager.rs

@@ -13,6 +13,7 @@
 //TODO: We need better separation of event types ^
 
 use ln::msgs;
+use ln::channelmanager::{PaymentPreimage, PaymentHash};
 use chain::transaction::OutPoint;
 use chain::keysinterface::SpendableOutputDescriptor;
 
@@ -59,7 +60,7 @@ pub enum Event {
 	/// the amount expected.
 	PaymentReceived {
 		/// The hash for which the preimage should be handed to the ChannelManager.
-		payment_hash: [u8; 32],
+		payment_hash: PaymentHash,
 		/// The value, in thousandths of a satoshi, that this payment is for.
 		amt: u64,
 	},
@@ -71,15 +72,15 @@ pub enum Event {
 		/// The preimage to the hash given to ChannelManager::send_payment.
 		/// Note that this serves as a payment receipt, if you wish to have such a thing, you must
 		/// store it somehow!
-		payment_preimage: [u8; 32],
+		payment_preimage: PaymentPreimage,
 	},
 	/// Indicates an outbound payment we made failed. Probably some intermediary node dropped
 	/// something. You may wish to retry with a different route.
 	/// Note that duplicative PaymentFailed Events may be generated - it is your responsibility to
 	/// deduplicate them by payment_hash (which MUST be unique)!
 	PaymentFailed {
 		/// The hash which was given to ChannelManager::send_payment.
-		payment_hash: [u8; 32],
+		payment_hash: PaymentHash,
 		/// Indicates the payment was rejected for some reason by the recipient. This implies that
 		/// the payment has failed, not just the route in question. If this is not set, you may
 		/// retry the payment via a different route.

src/ln/channelmonitor.rs

@@ -12,6 +12,7 @@ use bitcoin::util::hash::Sha256dHash;
 use bitcoin::blockdata::script::Script;
 use std::marker::Sized;
 use ln::msgs::DecodeError;
+use ln::channelmanager::{PaymentPreimage, PaymentHash};
 use util::byte_utils;
 
 use util::byte_utils::{be64_to_array, be48_to_array, be32_to_array, be16_to_array, slice_to_be16, slice_to_be32, slice_to_be48, slice_to_be64};
@@ -386,3 +387,29 @@ impl<R: Read> Readable<R> for Signature {
 		}
 	}
 }
+
+impl Writeable for PaymentPreimage {
+	fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
+		self.0.write(w)
+	}
+}
+
+impl<R: Read> Readable<R> for PaymentPreimage {
+	fn read(r: &mut R) -> Result<Self, DecodeError> {
+		let buf: [u8; 32] = Readable::read(r)?;
+		Ok(PaymentPreimage(buf))
+	}
+}
+
+impl Writeable for PaymentHash {
+	fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
+		self.0.write(w)
+	}
+}
+
+impl<R: Read> Readable<R> for PaymentHash {
+	fn read(r: &mut R) -> Result<Self, DecodeError> {
+		let buf: [u8; 32] = Readable::read(r)?;
+		Ok(PaymentHash(buf))
+	}
+}

src/ln/msgs.rs

@@ -4,6 +4,7 @@ use chain::transaction::OutPoint;
 use ln::channelmonitor;
 use ln::msgs;
 use ln::msgs::{HandleError};
+use ln::channelmonitor::HTLCUpdate;
 use util::events;
 use util::logger::{Logger, Level, Record};
 use util::ser::{ReadableArgs, Writer};
@@ -64,6 +65,10 @@ impl channelmonitor::ManyChannelMonitor for TestChannelMonitor {
 		assert!(self.simple_monitor.add_update_monitor(funding_txo, monitor).is_ok());
 		self.update_ret.lock().unwrap().clone()
 	}
+
+	fn fetch_pending_htlc_updated(&self) -> Vec<HTLCUpdate> {
+		return self.simple_monitor.fetch_pending_htlc_updated();
+	}
 }
 
 pub struct TestBroadcaster {
@@ -178,12 +183,17 @@ impl msgs::RoutingMessageHandler for TestRoutingMessageHandler {
 
 pub struct TestLogger {
 	level: Level,
+	id: String,
 }
 
 impl TestLogger {
 	pub fn new() -> TestLogger {
+		Self::with_id("".to_owned())
+	}
+	pub fn with_id(id: String) -> TestLogger {
 		TestLogger {
 			level: Level::Trace,
+			id,
 		}
 	}
 	pub fn enable(&mut self, level: Level) {
@@ -194,7 +204,7 @@ impl TestLogger {
 impl Logger for TestLogger {
 	fn log(&self, record: &Record) {
 		if self.level >= record.level {
-			println!("{:<5} [{} : {}, {}] {}", record.level.to_string(), record.module_path, record.file, record.line, record.args);
+			println!("{:<5} {} [{} : {}, {}] {}", record.level.to_string(), self.id, record.module_path, record.file, record.line, record.args);
 		}
 	}
 }