From: Matt Corallo <git@bluematt.me>
Date: Wed, 5 Sep 2018 18:08:38 +0000 (-0400)
Subject: Document+check commitment_signed generation success on send_htlc
X-Git-Tag: v0.0.12~327^2~2
X-Git-Url: http://git.bitcoin.ninja/?a=commitdiff_plain;h=8e4c062f1b5aacc1fb98b52537a7a98b6194c6b4;p=rust-lightning

Document+check commitment_signed generation success on send_htlc

Because we don't have an HTLCState for
update_add_htlc-generated-but-not-yet-commitment_signed to simplify
the mess of HTLCState match arms, any time a Channel::send_htlc
call returns Ok(Some(_)) we MUST call commitment_signed and it MUST
return success (or close the channel). We mention this in the docs
and panic if its not met in ChannelManager (which lets the fuzz
tester check this).
---

diff --git a/src/ln/channel.rs b/src/ln/channel.rs
index f6e97f12f..e5538a059 100644
--- a/src/ln/channel.rs
+++ b/src/ln/channel.rs
@@ -2334,6 +2334,7 @@ impl Channel {
 	/// This returns an option instead of a pure UpdateAddHTLC as we may be in a state where we are
 	/// waiting on the remote peer to send us a revoke_and_ack during which time we cannot add new
 	/// HTLCs on the wire or we wouldn't be able to determine what they actually ACK'ed.
+	/// You MUST call send_commitment prior to any other calls on this Channel
 	pub fn send_htlc(&mut self, amount_msat: u64, payment_hash: [u8; 32], cltv_expiry: u32, onion_routing_packet: msgs::OnionPacket) -> Result<Option<msgs::UpdateAddHTLC>, HandleError> {
 		if (self.channel_state & (ChannelState::ChannelFunded as u32 | BOTH_SIDES_SHUTDOWN_MASK)) != (ChannelState::ChannelFunded as u32) {
 			return Err(HandleError{err: "Cannot send HTLC until channel is fully established and we haven't started shutting down", action: None});
@@ -2401,6 +2402,8 @@ impl Channel {
 	}
 
 	/// Creates a signed commitment transaction to send to the remote peer.
+	/// Always returns a Channel-failing HandleError::action if an immediately-preceding (read: the
+	/// last call to this Channel) send_htlc returned Ok(Some(_)) and there is an Err.
 	pub fn send_commitment(&mut self) -> Result<(msgs::CommitmentSigned, ChannelMonitor), HandleError> {
 		if (self.channel_state & (ChannelState::ChannelFunded as u32)) != (ChannelState::ChannelFunded as u32) {
 			return Err(HandleError{err: "Cannot create commitment tx until channel is fully established", action: None});
diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs
index 508997833..825669402 100644
--- a/src/ln/channelmanager.rs
+++ b/src/ln/channelmanager.rs
@@ -1147,7 +1147,12 @@ impl ChannelManager {
 					if !add_htlc_msgs.is_empty() {
 						let (commitment_msg, monitor) = match forward_chan.send_commitment() {
 							Ok(res) => res,
-							Err(_e) => {
+							Err(e) => {
+								if let &Some(msgs::ErrorAction::DisconnectPeer{msg: Some(ref _err_msg)}) = &e.action {
+								} else if let &Some(msgs::ErrorAction::SendErrorMessage{msg: ref _err_msg}) = &e.action {
+								} else {
+									panic!("Stated return value requirements in send_commitment() were not met");
+								}
 								//TODO: Handle...this is bad!
 								continue;
 							},