Error handling in decoding onion

diff --git a/src/ln/channel.rs b/src/ln/channel.rs
index 1b7db97d3d342d1a97551b3a274c37425a07d8a2..82c26a0aa68a5e658e5a31835940688ed2ad61d5 100644 (file)
--- a/src/ln/channel.rs
+++ b/src/ln/channel.rs
@@ -2450,6 +2450,11 @@ impl Channel {
                self.our_htlc_minimum_msat
        }
 
+       /// Allowed in any state (including after shutdown)
+       pub fn get_their_htlc_minimum_msat(&self) -> u64 {
+               self.our_htlc_minimum_msat
+       }
+
        pub fn get_value_satoshis(&self) -> u64 {
                self.channel_value_satoshis
        }

diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs
index 5e5d531fa4b1eaa578469e4dad611107af33d687..ed86b511bd64d0b772b820a194dcf5e0457647f4 100644 (file)
--- a/src/ln/channelmanager.rs
+++ b/src/ln/channelmanager.rs
@@ -296,6 +296,7 @@ pub struct ChannelManager {
 /// the HTLC via a full update_fail_htlc/commitment_signed dance before we hit the
 /// CLTV_CLAIM_BUFFER point (we static assert that its at least 3 blocks more).
 const CLTV_EXPIRY_DELTA: u16 = 6 * 24 * 2; //TODO?
+const CLTV_FAR_FAR_AWAY: u32 = 6 * 24 * 7; //TODO?
 
 // Check that our CLTV_EXPIRY is at least CLTV_CLAIM_BUFFER + 2*HTLC_FAIL_TIMEOUT_BLOCKS, ie that
 // if the next-hop peer fails the HTLC within HTLC_FAIL_TIMEOUT_BLOCKS then we'll still have
@@ -896,13 +897,17 @@ impl ChannelManager {
                        }
                };
 
-               //TODO: Check that msg.cltv_expiry is within acceptable bounds!
-
                let pending_forward_info = if next_hop_data.hmac == [0; 32] {
                                // OUR PAYMENT!
-                               if next_hop_data.data.amt_to_forward != msg.amount_msat {
+                               // final_expiry_too_soon
+                               if (msg.cltv_expiry as u64) < self.latest_block_height.load(Ordering::Acquire) as u64 + (CLTV_CLAIM_BUFFER + HTLC_FAIL_TIMEOUT_BLOCKS) as u64 {
+                                       return_err!("The final CLTV expiry is too soon to handle", 17, &[0;0]);
+                               }
+                               // final_incorrect_htlc_amount
+                               if next_hop_data.data.amt_to_forward > msg.amount_msat {
                                        return_err!("Upstream node sent less than we were supposed to receive in payment", 19, &byte_utils::be64_to_array(msg.amount_msat));
                                }
+                               // final_incorrect_cltv_expiry
                                if next_hop_data.data.outgoing_cltv_value != msg.cltv_expiry {
                                        return_err!("Upstream node set CLTV to the wrong value", 18, &byte_utils::be32_to_array(msg.cltv_expiry));
                                }
@@ -967,29 +972,49 @@ impl ChannelManager {
                        if onion_packet.is_some() { // If short_channel_id is 0 here, we'll reject them in the body here
                                let id_option = channel_state.as_ref().unwrap().short_to_id.get(&short_channel_id).cloned();
                                let forwarding_id = match id_option {
-                                       None => {
+                                       None => { // unknown_next_peer
                                                return_err!("Don't have available channel for forwarding as requested.", 0x4000 | 10, &[0;0]);
                                        },
                                        Some(id) => id.clone(),
                                };
-                               if let Some((err, code, chan_update)) = {
+                               if let Some((err, code, chan_update)) = loop {
                                        let chan = channel_state.as_mut().unwrap().by_id.get_mut(&forwarding_id).unwrap();
-                                       if !chan.is_live() {
-                                               Some(("Forwarding channel is not in a ready state.", 0x1000 | 7, self.get_channel_update(chan).unwrap()))
-                                       } else {
-                                               let fee = amt_to_forward.checked_mul(self.fee_proportional_millionths as u64).and_then(|prop_fee| { (prop_fee / 1000000).checked_add(chan.get_our_fee_base_msat(&*self.fee_estimator) as u64) });
-                                               if fee.is_none() || msg.amount_msat < fee.unwrap() || (msg.amount_msat - fee.unwrap()) < *amt_to_forward {
-                                                       Some(("Prior hop has deviated from specified fees parameters or origin node has obsolete ones", 0x1000 | 12, self.get_channel_update(chan).unwrap()))
-                                               } else {
-                                                       if (msg.cltv_expiry as u64) < (*outgoing_cltv_value) as u64 + CLTV_EXPIRY_DELTA as u64 {
-                                                               Some(("Forwarding node has tampered with the intended HTLC values or origin node has an obsolete cltv_expiry_delta", 0x1000 | 13, self.get_channel_update(chan).unwrap()))
-                                                       } else {
-                                                               None
-                                                       }
-                                               }
+
+                                       if !chan.is_live() { // channel_disabled
+                                               break Some(("Forwarding channel is not in a ready state.", 0x1000 | 20, Some(self.get_channel_update(chan).unwrap())));
+                                       }
+                                       if *amt_to_forward < chan.get_their_htlc_minimum_msat() { // amount_below_minimum
+                                               break Some(("HTLC amount was below the htlc_minimum_msat", 0x1000 | 11, Some(self.get_channel_update(chan).unwrap())));
+                                       }
+                                       let fee = amt_to_forward.checked_mul(self.fee_proportional_millionths as u64).and_then(|prop_fee| { (prop_fee / 1000000).checked_add(chan.get_our_fee_base_msat(&*self.fee_estimator) as u64) });
+                                       if fee.is_none() || msg.amount_msat < fee.unwrap() || (msg.amount_msat - fee.unwrap()) < *amt_to_forward { // fee_insufficient
+                                               break Some(("Prior hop has deviated from specified fees parameters or origin node has obsolete ones", 0x1000 | 12, Some(self.get_channel_update(chan).unwrap())));
+                                       }
+                                       if (msg.cltv_expiry as u64) < (*outgoing_cltv_value) as u64 + CLTV_EXPIRY_DELTA as u64 { // incorrect_cltv_expiry
+                                               break Some(("Forwarding node has tampered with the intended HTLC values or origin node has an obsolete cltv_expiry_delta", 0x1000 | 13, Some(self.get_channel_update(chan).unwrap())));
+                                       }
+                                       let cur_height = self.latest_block_height.load(Ordering::Acquire) as u32 + 1;
+                                       // We want to have at least HTLC_FAIL_TIMEOUT_BLOCKS to fail prior to going on chain CLAIM_BUFFER blocks before expiration
+                                       if msg.cltv_expiry <= cur_height + CLTV_CLAIM_BUFFER + HTLC_FAIL_TIMEOUT_BLOCKS as u32 { // expiry_too_soon
+                                               break Some(("CLTV expiry is too close", 0x1000 | 14, Some(self.get_channel_update(chan).unwrap())));
+                                       }
+                                       if msg.cltv_expiry > cur_height + CLTV_FAR_FAR_AWAY as u32 { // expiry_too_far
+                                               break Some(("CLTV expiry is too far in the future", 21, None));
+                                       }
+                                       break None;
+                               }
+                               {
+                                       let mut res = Vec::with_capacity(8 + 128);
+                                       if code == 0x1000 | 11 || code == 0x1000 | 12 {
+                                               res.extend_from_slice(&byte_utils::be64_to_array(msg.amount_msat));
+                                       }
+                                       else if code == 0x1000 | 13 {
+                                               res.extend_from_slice(&byte_utils::be32_to_array(msg.cltv_expiry));
+                                       }
+                                       if let Some(chan_update) = chan_update {
+                                               res.extend_from_slice(&chan_update.encode_with_len()[..]);
                                        }
-                               } {
-                                       return_err!(err, code, &chan_update.encode_with_len()[..]);
+                                       return_err!(err, code, &res[..]);
                                }
                        }
                }
@@ -1328,6 +1353,8 @@ impl ChannelManager {
 
        /// Indicates that the preimage for payment_hash is unknown after a PaymentReceived event.
        pub fn fail_htlc_backwards(&self, payment_hash: &[u8; 32]) -> bool {
+               // TODO: Add ability to return 0x4000|16 (incorrect_payment_amount) if the amount we
+               // received is < expected or > 2*expected
                let mut channel_state = Some(self.channel_state.lock().unwrap());
                let removed_source = channel_state.as_mut().unwrap().claimable_htlcs.remove(payment_hash);
                if let Some(mut sources) = removed_source {