Update incorrect_payment_amount generation/handling for BOLT uptd

ie dont generate them as they're a really obvious privacy leak.
Luckily we were already handling them the same aside from log
printing so don't have to touch anything there. I was lazy in
updating tests but it only effects log printing, so whatever.

diff --git a/fuzz/fuzz_targets/full_stack_target.rs b/fuzz/fuzz_targets/full_stack_target.rs
index 406fa1e3ee862ec05ff738ae9d10d8a986560415..3e105038c3ffb93fa5a6d7f027b03c6f3c859d94 100644 (file)
--- a/fuzz/fuzz_targets/full_stack_target.rs
+++ b/fuzz/fuzz_targets/full_stack_target.rs
@@ -17,7 +17,7 @@ use lightning::chain::chaininterface::{BroadcasterInterface,ConfirmationTarget,C
 use lightning::chain::transaction::OutPoint;
 use lightning::chain::keysinterface::{ChannelKeys, KeysInterface};
 use lightning::ln::channelmonitor;
-use lightning::ln::channelmanager::{ChannelManager, PaymentFailReason, PaymentHash, PaymentPreimage};
+use lightning::ln::channelmanager::{ChannelManager, PaymentHash, PaymentPreimage};
 use lightning::ln::peer_handler::{MessageHandler,PeerManager,SocketDescriptor};
 use lightning::ln::router::Router;
 use lightning::util::events::{EventsProvider,Event};
@@ -419,7 +419,7 @@ pub fn do_test(data: &[u8], logger: &Arc<Logger>) {
                                        // fulfill this HTLC, but if they are, we can just take the first byte and
                                        // place that anywhere in our preimage.
                                        if &payment.0[1..] != &[0; 31] {
-                                               channelmanager.fail_htlc_backwards(&payment, PaymentFailReason::PreimageUnknown);
+                                               channelmanager.fail_htlc_backwards(&payment, 0);
                                        } else {
                                                let mut payment_preimage = PaymentPreimage([0; 32]);
                                                payment_preimage.0[0] = payment.0[0];
@@ -429,7 +429,7 @@ pub fn do_test(data: &[u8], logger: &Arc<Logger>) {
                        },
                        9 => {
                                for payment in payments_received.drain(..) {
-                                       channelmanager.fail_htlc_backwards(&payment, PaymentFailReason::PreimageUnknown);
+                                       channelmanager.fail_htlc_backwards(&payment, 0);
                                }
                        },
                        10 => {

diff --git a/src/ln/channelmanager.rs b/src/ln/channelmanager.rs
index 8b45fde93b172c323078b8661374059ecfc978de..2a96344e73c9cfd1c791e7849a3c4f770c82bc3b 100644 (file)
--- a/src/ln/channelmanager.rs
+++ b/src/ln/channelmanager.rs
@@ -212,16 +212,6 @@ impl MsgHandleErrInternal {
        }
 }
 
-/// Pass to fail_htlc_backwwards to indicate the reason to fail the payment
-/// after a PaymentReceived event.
-#[derive(PartialEq)]
-pub enum PaymentFailReason {
-       /// Indicate the preimage for payment_hash is not known after a PaymentReceived event
-       PreimageUnknown,
-       /// Indicate the payment amount is incorrect ( received is < expected or > 2*expected ) after a PaymentReceived event
-       AmountMismatch,
-}
-
 /// We hold back HTLCs we intend to relay for a random interval in the range (this, 5*this). This
 /// provides some limited amount of privacy. Ideally this would range from somewhere like 1 second
 /// to 30 seconds, but people expect lightning to be, you know, kinda fast, sadly. We could
@@ -1530,8 +1520,11 @@ impl ChannelManager {
                events.append(&mut new_events);
        }
 
-       /// Indicates that the preimage for payment_hash is unknown or the received amount is incorrect after a PaymentReceived event.
-       pub fn fail_htlc_backwards(&self, payment_hash: &PaymentHash, reason: PaymentFailReason) -> bool {
+       /// Indicates that the preimage for payment_hash is unknown or the received amount is incorrect
+       /// after a PaymentReceived event.
+       /// expected_value is the value you expected the payment to be for (not the amount it actually
+       /// was for from the PaymentReceived event).
+       pub fn fail_htlc_backwards(&self, payment_hash: &PaymentHash, expected_value: u64) -> bool {
                let _ = self.total_consistency_lock.read().unwrap();
 
                let mut channel_state = Some(self.channel_state.lock().unwrap());
@@ -1539,7 +1532,9 @@ impl ChannelManager {
                if let Some(mut sources) = removed_source {
                        for htlc_with_hash in sources.drain(..) {
                                if channel_state.is_none() { channel_state = Some(self.channel_state.lock().unwrap()); }
-                               self.fail_htlc_backwards_internal(channel_state.take().unwrap(), HTLCSource::PreviousHopData(htlc_with_hash), payment_hash, HTLCFailReason::Reason { failure_code: if reason == PaymentFailReason::PreimageUnknown {0x4000 | 15} else {0x4000 | 16}, data: Vec::new() });
+                               self.fail_htlc_backwards_internal(channel_state.take().unwrap(),
+                                               HTLCSource::PreviousHopData(htlc_with_hash), payment_hash,
+                                               HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: byte_utils::be64_to_array(expected_value).to_vec() });
                        }
                        true
                } else { false }
@@ -3363,7 +3358,7 @@ mod tests {
        use chain::keysinterface::{KeysInterface, SpendableOutputDescriptor};
        use chain::keysinterface;
        use ln::channel::{COMMITMENT_TX_BASE_WEIGHT, COMMITMENT_TX_WEIGHT_PER_HTLC};
-       use ln::channelmanager::{ChannelManager,ChannelManagerReadArgs,OnionKeys,PaymentFailReason,RAACommitmentOrder, PaymentPreimage, PaymentHash};
+       use ln::channelmanager::{ChannelManager,ChannelManagerReadArgs,OnionKeys,RAACommitmentOrder, PaymentPreimage, PaymentHash};
        use ln::channelmonitor::{ChannelMonitor, ChannelMonitorUpdateErr, CLTV_CLAIM_BUFFER, HTLC_FAIL_TIMEOUT_BLOCKS, ManyChannelMonitor};
        use ln::channel::{ACCEPTED_HTLC_SCRIPT_WEIGHT, OFFERED_HTLC_SCRIPT_WEIGHT};
        use ln::router::{Route, RouteHop, Router};
@@ -4221,7 +4216,7 @@ mod tests {
        }
 
        fn fail_payment_along_route(origin_node: &Node, expected_route: &[&Node], skip_last: bool, our_payment_hash: PaymentHash) {
-               assert!(expected_route.last().unwrap().node.fail_htlc_backwards(&our_payment_hash, PaymentFailReason::PreimageUnknown));
+               assert!(expected_route.last().unwrap().node.fail_htlc_backwards(&our_payment_hash, 0));
                check_added_monitors!(expected_route.last().unwrap(), 1);
 
                let mut next_msgs: Option<(msgs::UpdateFailHTLC, msgs::CommitmentSigned)> = None;
@@ -6348,7 +6343,7 @@ mod tests {
                // Brodacast legit commitment tx from C on B's chain
                let commitment_tx = nodes[2].node.channel_state.lock().unwrap().by_id.get(&chan_2.2).unwrap().last_local_commitment_txn.clone();
                check_spends!(commitment_tx[0], chan_2.3.clone());
-               nodes[2].node.fail_htlc_backwards(&payment_hash, PaymentFailReason::PreimageUnknown);
+               nodes[2].node.fail_htlc_backwards(&payment_hash, 0);
                {
                        let mut added_monitors = nodes[2].chan_monitor.added_monitors.lock().unwrap();
                        assert_eq!(added_monitors.len(), 1);
@@ -6533,7 +6528,7 @@ mod tests {
                let (_, second_payment_hash) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 3000000);
                let (_, third_payment_hash) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 3000000);
 
-               assert!(nodes[2].node.fail_htlc_backwards(&first_payment_hash, PaymentFailReason::PreimageUnknown));
+               assert!(nodes[2].node.fail_htlc_backwards(&first_payment_hash, 0));
                check_added_monitors!(nodes[2], 1);
                let updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
                assert!(updates.update_add_htlcs.is_empty());
@@ -6545,7 +6540,7 @@ mod tests {
                let bs_raa = commitment_signed_dance!(nodes[1], nodes[2], updates.commitment_signed, false, true, false, true);
                // Drop the last RAA from 3 -> 2
 
-               assert!(nodes[2].node.fail_htlc_backwards(&second_payment_hash, PaymentFailReason::PreimageUnknown));
+               assert!(nodes[2].node.fail_htlc_backwards(&second_payment_hash, 0));
                check_added_monitors!(nodes[2], 1);
                let updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
                assert!(updates.update_add_htlcs.is_empty());
@@ -6561,7 +6556,7 @@ mod tests {
                nodes[2].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &as_raa).unwrap();
                check_added_monitors!(nodes[2], 1);
 
-               assert!(nodes[2].node.fail_htlc_backwards(&third_payment_hash, PaymentFailReason::PreimageUnknown));
+               assert!(nodes[2].node.fail_htlc_backwards(&third_payment_hash, 0));
                check_added_monitors!(nodes[2], 1);
                let updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
                assert!(updates.update_add_htlcs.is_empty());
@@ -8156,7 +8151,7 @@ mod tests {
                let (_, payment_hash_1) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
 
                // Fail the payment backwards, failing the monitor update on nodes[1]'s receipt of the RAA
-               assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1, PaymentFailReason::PreimageUnknown));
+               assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1, 0));
                check_added_monitors!(nodes[2], 1);
 
                let updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
@@ -9721,7 +9716,7 @@ mod tests {
                        let onion_keys = ChannelManager::construct_onion_keys(&Secp256k1::new(), &route, &session_priv).unwrap();
                        msg.reason = ChannelManager::build_first_hop_failure_packet(&onion_keys[1].shared_secret[..], NODE|2, &[0;0]);
                }, ||{
-                       nodes[2].node.fail_htlc_backwards(&payment_hash, PaymentFailReason::PreimageUnknown);
+                       nodes[2].node.fail_htlc_backwards(&payment_hash, 0);
                }, true, Some(NODE|2), Some(msgs::HTLCFailChannelUpdate::NodeFailure{node_id: route.hops[1].pubkey, is_permanent: false}));
 
                // intermediate node failure
@@ -9739,7 +9734,7 @@ mod tests {
                        let onion_keys = ChannelManager::construct_onion_keys(&Secp256k1::new(), &route, &session_priv).unwrap();
                        msg.reason = ChannelManager::build_first_hop_failure_packet(&onion_keys[1].shared_secret[..], PERM|NODE|2, &[0;0]);
                }, ||{
-                       nodes[2].node.fail_htlc_backwards(&payment_hash, PaymentFailReason::PreimageUnknown);
+                       nodes[2].node.fail_htlc_backwards(&payment_hash, 0);
                }, false, Some(PERM|NODE|2), Some(msgs::HTLCFailChannelUpdate::NodeFailure{node_id: route.hops[1].pubkey, is_permanent: true}));
 
                // intermediate node failure
@@ -9750,7 +9745,7 @@ mod tests {
                        let onion_keys = ChannelManager::construct_onion_keys(&Secp256k1::new(), &route, &session_priv).unwrap();
                        msg.reason = ChannelManager::build_first_hop_failure_packet(&onion_keys[0].shared_secret[..], PERM|NODE|3, &[0;0]);
                }, ||{
-                       nodes[2].node.fail_htlc_backwards(&payment_hash, PaymentFailReason::PreimageUnknown);
+                       nodes[2].node.fail_htlc_backwards(&payment_hash, 0);
                }, true, Some(PERM|NODE|3), Some(msgs::HTLCFailChannelUpdate::NodeFailure{node_id: route.hops[0].pubkey, is_permanent: true}));
 
                // final node failure
@@ -9759,7 +9754,7 @@ mod tests {
                        let onion_keys = ChannelManager::construct_onion_keys(&Secp256k1::new(), &route, &session_priv).unwrap();
                        msg.reason = ChannelManager::build_first_hop_failure_packet(&onion_keys[1].shared_secret[..], PERM|NODE|3, &[0;0]);
                }, ||{
-                       nodes[2].node.fail_htlc_backwards(&payment_hash, PaymentFailReason::PreimageUnknown);
+                       nodes[2].node.fail_htlc_backwards(&payment_hash, 0);
                }, false, Some(PERM|NODE|3), Some(msgs::HTLCFailChannelUpdate::NodeFailure{node_id: route.hops[1].pubkey, is_permanent: true}));
 
                run_onion_failure_test("invalid_onion_version", 0, &nodes, &route, &payment_hash, |msg| { msg.onion_routing_packet.version = 1; }, ||{}, true,
@@ -9826,13 +9821,9 @@ mod tests {
                }, ||{}, true, Some(UPDATE|14), Some(msgs::HTLCFailChannelUpdate::ChannelUpdateMessage{msg: ChannelUpdate::dummy()}));
 
                run_onion_failure_test("unknown_payment_hash", 2, &nodes, &route, &payment_hash, |_| {}, || {
-                       nodes[2].node.fail_htlc_backwards(&payment_hash, PaymentFailReason::PreimageUnknown);
+                       nodes[2].node.fail_htlc_backwards(&payment_hash, 0);
                }, false, Some(PERM|15), None);
 
-               run_onion_failure_test("incorrect_payment_amount", 2, &nodes, &route, &payment_hash, |_| {}, || {
-                       nodes[2].node.fail_htlc_backwards(&payment_hash, PaymentFailReason::AmountMismatch);
-               }, false, Some(PERM|16), None);
-
                run_onion_failure_test("final_expiry_too_soon", 1, &nodes, &route, &payment_hash, |msg| {
                        let height = msg.cltv_expiry - CLTV_CLAIM_BUFFER - HTLC_FAIL_TIMEOUT_BLOCKS + 1;
                        let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 };

diff --git a/src/util/errors.rs b/src/util/errors.rs
index 8032ea46e19c45d5e448d5e5afc0dc87fe9f2993..27f837775ef19ad4c605a6e3b0659d158ed47164 100644 (file)
--- a/src/util/errors.rs
+++ b/src/util/errors.rs
@@ -84,7 +84,7 @@ pub(crate) fn get_onion_error_description(error_code: u16) -> (&'static str, &'s
                _c if _c == UPDATE|12 => ("Node indicated the fee amount does not meet the required level", "fee_insufficient"),
                _c if _c == UPDATE|13 => ("Node indicated the cltv_expiry does not comply with the cltv_expiry_delta required by the outgoing channel", "incorrect_cltv_expiry"),
                _c if _c == UPDATE|14 => ("Node indicated the CLTV expiry too close to the current block height for safe handling", "expiry_too_soon"),
-               _c if _c == PERM|15 => ("The final node indicated the payment hash is unknown", "unknown_payment_hash"),
+               _c if _c == PERM|15 => ("The final node indicated the payment hash is unknown or amount is incorrect", "incorrect_or_unknown_payment_details"),
                _c if _c == PERM|16 => ("The final node indicated the payment amount is incorrect", "incorrect_payment_amount"),
                _c if _c == 17 => ("The final node indicated the CLTV expiry is too close to the current block height for safe handling", "final_expiry_too_soon"),
                _c if _c == 18 => ("The final node indicated the CLTV expiry in the HTLC does not match the value in the onion", "final_incorrect_cltv_expiry"),

diff --git a/src/util/events.rs b/src/util/events.rs
index 6d3ac33d12f7bce48c681109cd99670c328295a9..de9dd4286f92c07cea79416cca92119c274bafa9 100644 (file)
--- a/src/util/events.rs
+++ b/src/util/events.rs
@@ -54,14 +54,15 @@ pub enum Event {
        /// Indicates we've received money! Just gotta dig out that payment preimage and feed it to
        /// ChannelManager::claim_funds to get it....
        /// Note that if the preimage is not known or the amount paid is incorrect, you must call
-       /// ChannelManager::fail_htlc_backwards with PaymentFailReason::PreimageUnknown or
-       /// PaymentFailReason::AmountMismatch, respectively, to free up resources for this HTLC.
+       /// ChannelManager::fail_htlc_backwards to free up resources for this HTLC.
        /// The amount paid should be considered 'incorrect' when it is less than or more than twice
        /// the amount expected.
        PaymentReceived {
                /// The hash for which the preimage should be handed to the ChannelManager.
                payment_hash: PaymentHash,
-               /// The value, in thousandths of a satoshi, that this payment is for.
+               /// The value, in thousandths of a satoshi, that this payment is for. Note that you must
+               /// compare this to the expected value before accepting the payment (as otherwise you are
+               /// providing proof-of-payment for less than the value you expected!).
                amt: u64,
        },
        /// Indicates an outbound payment we made succeeded (ie it made it all the way to its target