Drop the amount parameter to claim_funds

Like the payment_secret parameter, this paramter has been the source
of much confusion, so we just drop it.

Users should prefer to do this check when registering the payment
secret instead of at claim-time.

Author: TheBlueMatt

fuzz/src/chanmon_consistency.rs

@@ -687,12 +687,12 @@ pub fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
 				let had_events = !events.is_empty();
 				for event in events.drain(..) {
 					match event {
-						events::Event::PaymentReceived { payment_hash, payment_secret: _, amt, user_payment_id: _ } => {
+						events::Event::PaymentReceived { payment_hash, .. } => {
 							if claim_set.insert(payment_hash.0) {
 								if $fail {
 									assert!(nodes[$node].fail_htlc_backwards(&payment_hash));
 								} else {
-									assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0), amt));
+									assert!(nodes[$node].claim_funds(PaymentPreimage(payment_hash.0)));
 								}
 							}
 						},

fuzz/src/full_stack.rs

@@ -371,7 +371,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
 	}, our_network_key, &[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 15, 0], Arc::clone(&logger)));
 
 	let mut should_forward = false;
-	let mut payments_received: Vec<(PaymentHash, u64)> = Vec::new();
+	let mut payments_received: Vec<PaymentHash> = Vec::new();
 	let mut payments_sent = 0;
 	let mut pending_funding_generation: Vec<([u8; 32], u64, Script)> = Vec::new();
 	let mut pending_funding_signatures = HashMap::new();
@@ -476,7 +476,7 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
 				}
 			},
 			8 => {
-				for (payment, amt) in payments_received.drain(..) {
+				for payment in payments_received.drain(..) {
 					// SHA256 is defined as XOR of all input bytes placed in the first byte, and 0s
 					// for the remaining bytes. Thus, if not all remaining bytes are 0s we cannot
 					// fulfill this HTLC, but if they are, we can just take the first byte and
@@ -486,12 +486,12 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
 					} else {
 						let mut payment_preimage = PaymentPreimage([0; 32]);
 						payment_preimage.0[0] = payment.0[0];
-						channelmanager.claim_funds(payment_preimage, amt);
+						channelmanager.claim_funds(payment_preimage);
 					}
 				}
 			},
 			9 => {
-				for (payment, _) in payments_received.drain(..) {
+				for payment in payments_received.drain(..) {
 					channelmanager.fail_htlc_backwards(&payment);
 				}
 			},
@@ -571,9 +571,9 @@ pub fn do_test(data: &[u8], logger: &Arc<dyn Logger>) {
 				Event::FundingGenerationReady { temporary_channel_id, channel_value_satoshis, output_script, .. } => {
 					pending_funding_generation.push((temporary_channel_id, channel_value_satoshis, output_script));
 				},
-				Event::PaymentReceived { payment_hash, payment_secret: _, amt, user_payment_id: _ } => {
+				Event::PaymentReceived { payment_hash, .. } => {
 					//TODO: enhance by fetching random amounts from fuzz input?
-					payments_received.push((payment_hash, amt));
+					payments_received.push(payment_hash);
 				},
 				Event::PaymentSent {..} => {},
 				Event::PaymentFailed {..} => {},

lightning-persister/src/lib.rs

@@ -251,9 +251,9 @@ mod tests {
 		check_persisted_data!(0);
 
 		// Send a few payments and make sure the monitors are updated to the latest.
-		send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000, 8_000_000);
+		send_payment(&nodes[0], &vec!(&nodes[1])[..], 8000000);
 		check_persisted_data!(5);
-		send_payment(&nodes[1], &vec!(&nodes[0])[..], 4000000, 4_000_000);
+		send_payment(&nodes[1], &vec!(&nodes[0])[..], 4000000);
 		check_persisted_data!(10);
 
 		// Force close because cooperative close doesn't result in any persisted

lightning/src/chain/chainmonitor.rs

@@ -344,7 +344,7 @@ mod tests {
 		let (commitment_tx, htlc_tx) = {
 			let payment_preimage = route_payment(&nodes[0], &vec!(&nodes[1])[..], 5_000_000).0;
 			let mut txn = get_local_commitment_txn!(nodes[0], channel.2);
-			claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage, 5_000_000);
+			claim_payment(&nodes[0], &vec!(&nodes[1])[..], payment_preimage);
 
 			assert_eq!(txn.len(), 2);
 			(txn.remove(0), txn.remove(0))

lightning/src/ln/chanmon_update_fail_tests.rs

@@ -90,7 +90,7 @@ fn test_monitor_and_persister_update_fail() {
 	let outpoint = OutPoint { txid: chan.3.txid(), index: 0 };
 
 	// Rebalance the network to generate htlc in the two directions
-	send_payment(&nodes[0], &vec!(&nodes[1])[..], 10_000_000, 10_000_000);
+	send_payment(&nodes[0], &vec!(&nodes[1])[..], 10_000_000);
 
 	// Route an HTLC from node 0 to node 1 (but don't settle)
 	let preimage = route_payment(&nodes[0], &vec!(&nodes[1])[..], 9_000_000).0;
@@ -121,7 +121,7 @@ fn test_monitor_and_persister_update_fail() {
 	persister.set_update_ret(Err(ChannelMonitorUpdateErr::TemporaryFailure));
 
 	// Try to update ChannelMonitor
-	assert!(nodes[1].node.claim_funds(preimage, 9_000_000));
+	assert!(nodes[1].node.claim_funds(preimage));
 	check_added_monitors!(nodes[1], 1);
 	let updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
 	assert_eq!(updates.update_fulfill_htlcs.len(), 1);
@@ -214,7 +214,7 @@ fn do_test_simple_monitor_temporary_update_fail(disconnect: bool, persister_fail
 		_ => panic!("Unexpected event"),
 	}
 
-	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_1, 1_000_000);
+	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_1);
 
 	// Now set it to failed again...
 	let (_, payment_hash_2, payment_secret_2) = get_payment_preimage_hash!(&nodes[1]);
@@ -305,7 +305,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
 
 	// Claim the previous payment, which will result in a update_fulfill_htlc/CS from nodes[1]
 	// but nodes[0] won't respond since it is frozen.
-	assert!(nodes[1].node.claim_funds(payment_preimage_1, 1_000_000));
+	assert!(nodes[1].node.claim_funds(payment_preimage_1));
 	check_added_monitors!(nodes[1], 1);
 	let events_2 = nodes[1].node.get_and_clear_pending_msg_events();
 	assert_eq!(events_2.len(), 1);
@@ -582,7 +582,7 @@ fn do_test_monitor_temporary_update_fail(disconnect_count: usize) {
 		_ => panic!("Unexpected event"),
 	}
 
-	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2, 1_000_000);
+	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2);
 }
 
 #[test]
@@ -696,7 +696,7 @@ fn test_monitor_update_fail_cs() {
 		_ => panic!("Unexpected event"),
 	};
 
-	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage, 1_000_000);
+	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage);
 }
 
 #[test]
@@ -747,7 +747,7 @@ fn test_monitor_update_fail_no_rebroadcast() {
 		_ => panic!("Unexpected event"),
 	}
 
-	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_1, 1_000_000);
+	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_1);
 }
 
 #[test]
@@ -761,7 +761,7 @@ fn test_monitor_update_raa_while_paused() {
 	let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known()).2;
 	let logger = test_utils::TestLogger::new();
 
-	send_payment(&nodes[0], &[&nodes[1]], 5000000, 5_000_000);
+	send_payment(&nodes[0], &[&nodes[1]], 5000000);
 	let (payment_preimage_1, our_payment_hash_1, our_payment_secret_1) = get_payment_preimage_hash!(nodes[1]);
 	{
 		let net_graph_msg_handler = &nodes[0].net_graph_msg_handler;
@@ -825,8 +825,8 @@ fn test_monitor_update_raa_while_paused() {
 	expect_pending_htlcs_forwardable!(nodes[1]);
 	expect_payment_received!(nodes[1], our_payment_hash_1, our_payment_secret_1, 1000000);
 
-	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_1, 1_000_000);
-	claim_payment(&nodes[1], &[&nodes[0]], payment_preimage_2, 1_000_000);
+	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_1);
+	claim_payment(&nodes[1], &[&nodes[0]], payment_preimage_2);
 }
 
 fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
@@ -840,7 +840,7 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
 	let logger = test_utils::TestLogger::new();
 
 	// Rebalance a bit so that we can send backwards from 2 to 1.
-	send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000, 5_000_000);
+	send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);
 
 	// Route a first payment that we'll fail backwards
 	let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
@@ -1079,10 +1079,10 @@ fn do_test_monitor_update_fail_raa(test_ignore_second_cs: bool) {
 			Event::PaymentReceived { payment_hash, .. } => assert_eq!(payment_hash, payment_hash_4.unwrap()),
 			_ => panic!("Unexpected event"),
 		};
-		claim_payment(&nodes[2], &[&nodes[1], &nodes[0]], payment_preimage_4.unwrap(), 1_000_000);
+		claim_payment(&nodes[2], &[&nodes[1], &nodes[0]], payment_preimage_4.unwrap());
 	}
 
-	claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage_2, 1_000_000);
+	claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], payment_preimage_2);
 }
 
 #[test]
@@ -1108,7 +1108,7 @@ fn test_monitor_update_fail_reestablish() {
 	nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
 	nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
 
-	assert!(nodes[2].node.claim_funds(our_payment_preimage, 1_000_000));
+	assert!(nodes[2].node.claim_funds(our_payment_preimage));
 	check_added_monitors!(nodes[2], 1);
 	let mut updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
 	assert!(updates.update_add_htlcs.is_empty());
@@ -1289,9 +1289,9 @@ fn raa_no_response_awaiting_raa_state() {
 	expect_pending_htlcs_forwardable!(nodes[1]);
 	expect_payment_received!(nodes[1], payment_hash_3, payment_secret_3, 1000000);
 
-	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_1, 1_000_000);
-	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2, 1_000_000);
-	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_3, 1_000_000);
+	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_1);
+	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2);
+	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_3);
 }
 
 #[test]
@@ -1315,7 +1315,7 @@ fn claim_while_disconnected_monitor_update_fail() {
 	nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
 	nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
 
-	assert!(nodes[1].node.claim_funds(payment_preimage_1, 1_000_000));
+	assert!(nodes[1].node.claim_funds(payment_preimage_1));
 	check_added_monitors!(nodes[1], 1);
 
 	nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
@@ -1417,7 +1417,7 @@ fn claim_while_disconnected_monitor_update_fail() {
 		_ => panic!("Unexpected event"),
 	}
 
-	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2, 1_000_000);
+	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2);
 }
 
 #[test]
@@ -1485,7 +1485,7 @@ fn monitor_failed_no_reestablish_response() {
 	expect_pending_htlcs_forwardable!(nodes[1]);
 	expect_payment_received!(nodes[1], payment_hash_1, payment_secret_1, 1000000);
 
-	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_1, 1_000_000);
+	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_1);
 }
 
 #[test]
@@ -1586,8 +1586,8 @@ fn first_message_on_recv_ordering() {
 	expect_pending_htlcs_forwardable!(nodes[1]);
 	expect_payment_received!(nodes[1], payment_hash_2, payment_secret_2, 1000000);
 
-	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_1, 1_000_000);
-	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2, 1_000_000);
+	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_1);
+	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2);
 }
 
 #[test]
@@ -1606,12 +1606,12 @@ fn test_monitor_update_fail_claim() {
 	let logger = test_utils::TestLogger::new();
 
 	// Rebalance a bit so that we can send backwards from 3 to 2.
-	send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000, 5_000_000);
+	send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);
 
 	let (payment_preimage_1, _, _) = route_payment(&nodes[0], &[&nodes[1]], 1000000);
 
 	*nodes[1].chain_monitor.update_ret.lock().unwrap() = Some(Err(ChannelMonitorUpdateErr::TemporaryFailure));
-	assert!(nodes[1].node.claim_funds(payment_preimage_1, 1_000_000));
+	assert!(nodes[1].node.claim_funds(payment_preimage_1));
 	check_added_monitors!(nodes[1], 1);
 
 	let (_, payment_hash_2, payment_secret_2) = get_payment_preimage_hash!(nodes[0]);
@@ -1687,7 +1687,7 @@ fn test_monitor_update_on_pending_forwards() {
 	let logger = test_utils::TestLogger::new();
 
 	// Rebalance a bit so that we can send backwards from 3 to 1.
-	send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000, 5_000_000);
+	send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);
 
 	let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
 	assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1));
@@ -1742,7 +1742,7 @@ fn test_monitor_update_on_pending_forwards() {
 	nodes[0].node.process_pending_htlc_forwards();
 	expect_payment_received!(nodes[0], payment_hash_2, payment_secret_2, 1000000);
 
-	claim_payment(&nodes[2], &[&nodes[1], &nodes[0]], payment_preimage_2, 1_000_000);
+	claim_payment(&nodes[2], &[&nodes[1], &nodes[0]], payment_preimage_2);
 }
 
 #[test]
@@ -1777,7 +1777,7 @@ fn monitor_update_claim_fail_no_response() {
 	let as_raa = commitment_signed_dance!(nodes[1], nodes[0], payment_event.commitment_msg, false, true, false, true);
 
 	*nodes[1].chain_monitor.update_ret.lock().unwrap() = Some(Err(ChannelMonitorUpdateErr::TemporaryFailure));
-	assert!(nodes[1].node.claim_funds(payment_preimage_1, 1_000_000));
+	assert!(nodes[1].node.claim_funds(payment_preimage_1));
 	check_added_monitors!(nodes[1], 1);
 	let events = nodes[1].node.get_and_clear_pending_msg_events();
 	assert_eq!(events.len(), 0);
@@ -1807,7 +1807,7 @@ fn monitor_update_claim_fail_no_response() {
 		_ => panic!("Unexpected event"),
 	}
 
-	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2, 1_000_000);
+	claim_payment(&nodes[0], &[&nodes[1]], payment_preimage_2);
 }
 
 // confirm_a_first and restore_b_before_conf are wholly unrelated to earlier bools and
@@ -1897,7 +1897,7 @@ fn do_during_funding_monitor_fail(confirm_a_first: bool, restore_b_before_conf:
 		node.net_graph_msg_handler.handle_channel_update(&bs_update).unwrap();
 	}
 
-	send_payment(&nodes[0], &[&nodes[1]], 8000000, 8_000_000);
+	send_payment(&nodes[0], &[&nodes[1]], 8000000);
 	close_channel(&nodes[0], &nodes[1], &channel_id, funding_tx, true);
 }
 
@@ -1965,5 +1965,5 @@ fn test_path_paused_mpp() {
 	assert_eq!(events.len(), 1);
 	pass_along_path(&nodes[0], &[&nodes[2], &nodes[3]], 200_000, payment_hash.clone(), payment_secret, events.pop().unwrap(), true);
 
-	claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, payment_preimage, 200_000);
+	claim_payment_along_route(&nodes[0], &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]], false, payment_preimage);
 }

lightning/src/ln/channelmanager.rs

@@ -2289,7 +2289,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 	///
 	/// [`get_payment_secret`]: Self::get_payment_secret
 	/// [`get_payment_secret_preimage`]: Self::get_payment_secret_preimage
-	pub fn claim_funds(&self, payment_preimage: PaymentPreimage, expected_amount: u64) -> bool {
+	pub fn claim_funds(&self, payment_preimage: PaymentPreimage) -> bool {
 		let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
 
 		let _persistence_guard = PersistenceNotifierGuard::new(&self.total_consistency_lock, &self.persistence_notifier);
@@ -2310,7 +2310,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 			// we got all the HTLCs and then a channel closed while we were waiting for the user to
 			// provide the preimage, so worrying too much about the optimal handling isn't worth
 			// it.
-			let mut valid_mpp = sources[0].payment_data.total_msat >= expected_amount;
+			let mut valid_mpp = true;
 			for htlc in sources.iter() {
 				if !valid_mpp { break; }
 				if let None = channel_state.as_ref().unwrap().short_to_id.get(&htlc.prev_hop.short_channel_id) {
@@ -4830,7 +4830,7 @@ pub mod bench {
 
 				expect_pending_htlcs_forwardable!(NodeHolder { node: &$node_b });
 				expect_payment_received!(NodeHolder { node: &$node_b }, payment_hash, payment_secret, 10_000);
-				assert!($node_b.claim_funds(payment_preimage, 10_000));
+				assert!($node_b.claim_funds(payment_preimage));
 
 				match $node_b.get_and_clear_pending_msg_events().pop().unwrap() {
 					MessageSendEvent::UpdateHTLCs { node_id, updates } => {

lightning/src/ln/functional_test_utils.rs

@@ -1048,11 +1048,11 @@ pub fn send_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, route: Route
 	(our_payment_preimage, our_payment_hash, our_payment_secret)
 }
 
-pub fn claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_paths: &[&[&Node<'a, 'b, 'c>]], skip_last: bool, our_payment_preimage: PaymentPreimage, expected_amount: u64) {
+pub fn claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_paths: &[&[&Node<'a, 'b, 'c>]], skip_last: bool, our_payment_preimage: PaymentPreimage) {
 	for path in expected_paths.iter() {
 		assert_eq!(path.last().unwrap().node.get_our_node_id(), expected_paths[0].last().unwrap().node.get_our_node_id());
 	}
-	assert!(expected_paths[0].last().unwrap().node.claim_funds(our_payment_preimage, expected_amount));
+	assert!(expected_paths[0].last().unwrap().node.claim_funds(our_payment_preimage));
 	check_added_monitors!(expected_paths[0].last().unwrap(), expected_paths.len());
 
 	macro_rules! msgs_from_ev {
@@ -1136,8 +1136,8 @@ pub fn claim_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, exp
 	}
 }
 
-pub fn claim_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], our_payment_preimage: PaymentPreimage, expected_amount: u64) {
-	claim_payment_along_route(origin_node, &[expected_route], false, our_payment_preimage, expected_amount);
+pub fn claim_payment<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], our_payment_preimage: PaymentPreimage) {
+	claim_payment_along_route(origin_node, &[expected_route], false, our_payment_preimage);
 }
 
 pub const TEST_FINAL_CLTV: u32 = 50;
@@ -1170,9 +1170,9 @@ pub fn route_over_limit<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_rou
 		assert!(err.contains("Cannot send value that would put us over the max HTLC value in flight our peer will accept")));
 }
 
-pub fn send_payment<'a, 'b, 'c>(origin: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64, expected_value: u64)  {
+pub fn send_payment<'a, 'b, 'c>(origin: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], recv_value: u64)  {
 	let our_payment_preimage = route_payment(&origin, expected_route, recv_value).0;
-	claim_payment(&origin, expected_route, our_payment_preimage, expected_value);
+	claim_payment(&origin, expected_route, our_payment_preimage);
 }
 
 pub fn fail_payment_along_route<'a, 'b, 'c>(origin_node: &Node<'a, 'b, 'c>, expected_route: &[&Node<'a, 'b, 'c>], skip_last: bool, our_payment_hash: PaymentHash)  {