From 4b43aa042d8ed76fe3cb35c8c32a5a7464d6c1fe Mon Sep 17 00:00:00 2001 From: yukang Date: Wed, 13 Nov 2024 18:28:41 +0800 Subject: [PATCH 01/26] add expiry check for channel open and channel update --- src/fiber/channel.rs | 8 +++++++- src/fiber/config.rs | 3 +++ src/fiber/network.rs | 9 ++++++++- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/fiber/channel.rs b/src/fiber/channel.rs index 90499355..6108a054 100644 --- a/src/fiber/channel.rs +++ b/src/fiber/channel.rs @@ -61,7 +61,7 @@ use crate::{ }; use super::{ - config::{DEFAULT_CHANNEL_MINIMAL_CKB_AMOUNT, MIN_UDT_OCCUPIED_CAPACITY}, + config::{DEFAULT_CHANNEL_MINIMAL_CKB_AMOUNT, MIN_TLC_EXPIRY_DELTA, MIN_UDT_OCCUPIED_CAPACITY}, fee::{calculate_shutdown_tx_fee, default_minimal_ckb_amount}, hash_algorithm::HashAlgorithm, key::blake2b_hash_with_salt, @@ -1229,6 +1229,12 @@ where } if let Some(delta) = tlc_expiry_delta { + if delta < MIN_TLC_EXPIRY_DELTA { + return Err(ProcessingChannelError::InvalidParameter(format!( + "TLC expiry delta is too small, expect larger than {}", + MIN_TLC_EXPIRY_DELTA + ))); + } updated |= state.update_our_tlc_expiry_delta(delta); } diff --git a/src/fiber/config.rs b/src/fiber/config.rs index f07b2cc8..6bb6ee48 100644 --- a/src/fiber/config.rs +++ b/src/fiber/config.rs @@ -38,6 +38,9 @@ pub const DEFAULT_CHANNEL_MIN_AUTO_CKB_AMOUNT: u64 = /// The expiry delta to forward a tlc, in milliseconds, default to 1 day. pub const DEFAULT_TLC_EXPIRY_DELTA: u64 = 24 * 60 * 60 * 1000; +/// The minimal expiry delta to forward a tlc, in milliseconds. 15 minutes. +pub const MIN_TLC_EXPIRY_DELTA: u64 = 15 * 60 * 1000; // 15 minutes + /// The minimal value of a tlc. 0 means no minimal value. pub const DEFAULT_TLC_MIN_VALUE: u128 = 0; diff --git a/src/fiber/network.rs b/src/fiber/network.rs index 2215de11..52dc2ee6 100644 --- a/src/fiber/network.rs +++ b/src/fiber/network.rs @@ -52,7 +52,7 @@ use super::channel::{ ProcessingChannelResult, PublicChannelInfo, ShuttingDownFlags, DEFAULT_COMMITMENT_FEE_RATE, DEFAULT_FEE_RATE, }; -use super::config::AnnouncedNodeName; +use super::config::{AnnouncedNodeName, MIN_TLC_EXPIRY_DELTA}; use super::fee::{calculate_commitment_tx_fee, default_minimal_ckb_amount}; use super::graph::{NetworkGraph, NetworkGraphStateStore}; use super::graph_syncer::{GraphSyncer, GraphSyncerMessage}; @@ -2916,6 +2916,13 @@ where )); } } + + if let Some(_delta) = tlc_expiry_delta.filter(|&d| d < MIN_TLC_EXPIRY_DELTA) { + return Err(ProcessingChannelError::InvalidParameter(format!( + "TLC expiry delta is too small, expect larger than {}", + MIN_TLC_EXPIRY_DELTA + ))); + } // NOTE: here we only check the amount is valid, we will also check more in the `pre_start` from channel creation let (_funding_amount, _reserved_ckb_amount) = self.get_funding_and_reserved_amount(funding_amount, &funding_udt_type_script)?; From 3c11c1624173b79159632cd41dfd16cc202e4913 Mon Sep 17 00:00:00 2001 From: yukang Date: Wed, 13 Nov 2024 20:36:10 +0800 Subject: [PATCH 02/26] add check for final htlc minimum expiry delta --- src/fiber/network.rs | 11 +++++++++++ src/rpc/channel.rs | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/fiber/network.rs b/src/fiber/network.rs index 52dc2ee6..7a437e27 100644 --- a/src/fiber/network.rs +++ b/src/fiber/network.rs @@ -383,6 +383,17 @@ impl SendPaymentData { Err(e) => return Err(e), }; + if let Some(final_htlc_expiry_delta) = command.final_htlc_expiry_delta { + if let Some(invoice_final_htlc_minimum_expiry_delta) = invoice + .as_ref() + .and_then(|i| i.final_htlc_minimum_expiry_delta()) + { + if *invoice_final_htlc_minimum_expiry_delta > final_htlc_expiry_delta { + return Err("final_htlc_expiry_delta is less than invoice final_htlc_minimum_expiry_delta".to_string()); + } + } + } + let keysend = command.keysend.unwrap_or(false); let (payment_hash, preimage) = if !keysend { ( diff --git a/src/rpc/channel.rs b/src/rpc/channel.rs index 92a41632..197a2b4a 100644 --- a/src/rpc/channel.rs +++ b/src/rpc/channel.rs @@ -285,7 +285,7 @@ pub(crate) struct SendPaymentCommandParams { /// the hash to use within the payment's HTLC payment_hash: Option, - /// the htlc expiry delta should be used to set the timelock for the final hop + /// the htlc expiry delta should be used to set the timelock for the final hop, in milliseconds #[serde_as(as = "Option")] final_htlc_expiry_delta: Option, From de5114cbcf4b787d1bc9ee04de8fef66877c711f Mon Sep 17 00:00:00 2001 From: yukang Date: Wed, 13 Nov 2024 23:43:21 +0800 Subject: [PATCH 03/26] add ui test for htlc expiry --- src/fiber/tests/network.rs | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/src/fiber/tests/network.rs b/src/fiber/tests/network.rs index 3e7b685a..8b90a77d 100644 --- a/src/fiber/tests/network.rs +++ b/src/fiber/tests/network.rs @@ -728,7 +728,6 @@ fn test_send_payment_validate_invoice() { }; let result = SendPaymentData::new(send_command, generate_pubkey().into()); - eprintln!("invoice: {:?}", result); assert!(result.is_err()); assert!(result .unwrap_err() @@ -788,7 +787,6 @@ fn test_send_payment_validate_invoice() { }; let result = SendPaymentData::new(send_command, generate_pubkey().into()); - eprintln!("invoice: {:?}", result); assert!(result.is_ok()); // normal keysend send payment @@ -807,6 +805,26 @@ fn test_send_payment_validate_invoice() { }; let result = SendPaymentData::new(send_command, generate_pubkey().into()); - eprintln!("invoice: {:?}", result); assert!(result.is_ok()); + + // invoice with invalid final_htlc_expiry_delta + let send_command = SendPaymentCommand { + target_pubkey: None, + amount: None, + payment_hash: None, + final_htlc_expiry_delta: Some(11), + invoice: Some(invoice_encoded.clone()), + timeout: None, + max_fee_amount: None, + max_parts: None, + keysend: None, + udt_type_script: None, + allow_self_payment: false, + }; + + let result = SendPaymentData::new(send_command, generate_pubkey().into()); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .contains("final_htlc_expiry_delta is less than invoice")); } From ebd89578b07482c68d844302025ca51dfef0b2ea Mon Sep 17 00:00:00 2001 From: yukang Date: Thu, 14 Nov 2024 12:09:07 +0800 Subject: [PATCH 04/26] add validation for tlc expiry --- src/fiber/config.rs | 3 ++ src/fiber/graph.rs | 1 - src/fiber/network.rs | 38 ++++++++++++++++++++++---- src/fiber/tests/channel.rs | 18 ++++++++---- src/fiber/tests/graph.rs | 53 +++++++++++++++++++++++++----------- src/fiber/tests/network.rs | 56 +++++++++++++++++++++++++++++++------- src/rpc/README.md | 2 +- src/rpc/channel.rs | 9 ++++-- src/rpc/invoice.rs | 11 ++++++++ src/tests/store.rs | 4 ++- 10 files changed, 153 insertions(+), 42 deletions(-) diff --git a/src/fiber/config.rs b/src/fiber/config.rs index 6bb6ee48..90f78a8f 100644 --- a/src/fiber/config.rs +++ b/src/fiber/config.rs @@ -41,6 +41,9 @@ pub const DEFAULT_TLC_EXPIRY_DELTA: u64 = 24 * 60 * 60 * 1000; /// The minimal expiry delta to forward a tlc, in milliseconds. 15 minutes. pub const MIN_TLC_EXPIRY_DELTA: u64 = 15 * 60 * 1000; // 15 minutes +/// The maximum expiry delta for a payment, in milliseconds. 2 days +pub const MAX_PAYMENT_TLC_EXPIRY_LIMIT: u64 = 2 * 24 * 60 * 60 * 1000; // 2 days + /// The minimal value of a tlc. 0 means no minimal value. pub const DEFAULT_TLC_MIN_VALUE: u128 = 0; diff --git a/src/fiber/graph.rs b/src/fiber/graph.rs index c8d6ed99..ed6da8fd 100644 --- a/src/fiber/graph.rs +++ b/src/fiber/graph.rs @@ -523,7 +523,6 @@ where &self, payment_data: SendPaymentData, ) -> Result, PathFindError> { - let payment_data = payment_data.clone(); let source = self.get_source_pubkey(); let target = payment_data.target_pubkey; let amount = payment_data.amount; diff --git a/src/fiber/network.rs b/src/fiber/network.rs index 55de0dc0..6f61e530 100644 --- a/src/fiber/network.rs +++ b/src/fiber/network.rs @@ -74,6 +74,7 @@ use crate::ckb::{CkbChainMessage, FundingRequest, FundingTx, TraceTxRequest, Tra use crate::fiber::channel::{ AddTlcCommand, AddTlcResponse, TxCollaborationCommand, TxUpdateCommand, }; +use crate::fiber::config::MAX_PAYMENT_TLC_EXPIRY_LIMIT; use crate::fiber::graph::{ChannelInfo, PaymentSession, PaymentSessionStatus}; use crate::fiber::serde_utils::EntityHex; use crate::fiber::types::{ @@ -288,7 +289,9 @@ pub struct SendPaymentCommand { // the encoded invoice to send to the recipient pub invoice: Option, // The htlc expiry delta that should be used to set the timelock for the final hop - pub final_htlc_expiry_delta: Option, + pub final_tlc_expiry_delta: Option, + // The htlc expiry for whole payment, in milliseconds + pub tlc_expiry_limit: Option, // the payment timeout in seconds, if the payment is not completed within this time, it will be cancelled pub timeout: Option, // the maximum fee amounts in shannons that the sender is willing to pay, default is 1000 shannons CKB. @@ -311,7 +314,8 @@ pub struct SendPaymentData { pub amount: u128, pub payment_hash: Hash256, pub invoice: Option, - pub final_htlc_expiry_delta: Option, + pub final_tlc_expiry_delta: Option, + pub tlc_expiry_limit: u64, pub timeout: Option, pub max_fee_amount: Option, pub max_parts: Option, @@ -383,15 +387,34 @@ impl SendPaymentData { Err(e) => return Err(e), }; - if let Some(final_htlc_expiry_delta) = command.final_htlc_expiry_delta { + // check htlc expiry delta and limit are both valid if it is set + if let Some(final_tlc_expiry_delta) = command.final_tlc_expiry_delta { if let Some(invoice_final_htlc_minimum_expiry_delta) = invoice .as_ref() .and_then(|i| i.final_htlc_minimum_expiry_delta()) { - if *invoice_final_htlc_minimum_expiry_delta > final_htlc_expiry_delta { - return Err("final_htlc_expiry_delta is less than invoice final_htlc_minimum_expiry_delta".to_string()); + if *invoice_final_htlc_minimum_expiry_delta > final_tlc_expiry_delta { + return Err("final_tlc_expiry_delta is less than invoice final_htlc_minimum_expiry_delta".to_string()); } } + + if final_tlc_expiry_delta < MIN_TLC_EXPIRY_DELTA { + return Err("final_tlc_expiry_delta is too small".to_string()); + } + } + if let Some(tlc_expiry_limit) = command.tlc_expiry_limit { + let final_tlc_expiry_delta = command.final_tlc_expiry_delta.unwrap_or(0); + if tlc_expiry_limit <= final_tlc_expiry_delta + || tlc_expiry_limit <= MIN_TLC_EXPIRY_DELTA + { + return Err("tlc_expiry_limit too small".to_string()); + } + if tlc_expiry_limit > MAX_PAYMENT_TLC_EXPIRY_LIMIT { + return Err(format!( + "tlc_expiry_limit too large, expect it to less than {}", + MAX_PAYMENT_TLC_EXPIRY_LIMIT + )); + } } let keysend = command.keysend.unwrap_or(false); @@ -426,7 +449,10 @@ impl SendPaymentData { amount, payment_hash, invoice: command.invoice, - final_htlc_expiry_delta: command.final_htlc_expiry_delta, + final_tlc_expiry_delta: command.final_tlc_expiry_delta, + tlc_expiry_limit: command + .tlc_expiry_limit + .unwrap_or(MAX_PAYMENT_TLC_EXPIRY_LIMIT), timeout: command.timeout, max_fee_amount: command.max_fee_amount, max_parts: command.max_parts, diff --git a/src/fiber/tests/channel.rs b/src/fiber/tests/channel.rs index aeabc0d6..81cbfde9 100644 --- a/src/fiber/tests/channel.rs +++ b/src/fiber/tests/channel.rs @@ -315,7 +315,8 @@ async fn test_network_send_payment_normal_keysend_workflow() { target_pubkey: Some(node_b_pubkey), amount: Some(10000), payment_hash: None, - final_htlc_expiry_delta: None, + final_tlc_expiry_delta: None, + tlc_expiry_limit: None, invoice: None, timeout: None, max_fee_amount: None, @@ -371,7 +372,9 @@ async fn test_network_send_payment_keysend_with_payment_hash() { target_pubkey: Some(node_b_pubkey), amount: Some(10000), payment_hash: Some(payment_hash), - final_htlc_expiry_delta: None, + final_tlc_expiry_delta: None, + tlc_expiry_limit: None, + invoice: None, timeout: None, max_fee_amount: None, @@ -415,7 +418,9 @@ async fn test_network_send_payment_final_incorrect_hash() { target_pubkey: Some(node_b_pubkey), amount: Some(10000), payment_hash: Some(payment_hash), - final_htlc_expiry_delta: None, + final_tlc_expiry_delta: None, + tlc_expiry_limit: None, + invoice: None, timeout: None, max_fee_amount: None, @@ -475,7 +480,9 @@ async fn test_network_send_payment_target_not_found() { target_pubkey: Some(node_b_pubkey), amount: Some(10000), payment_hash: Some(gen_sha256_hash()), - final_htlc_expiry_delta: None, + final_tlc_expiry_delta: None, + tlc_expiry_limit: None, + invoice: None, timeout: None, max_fee_amount: None, @@ -514,7 +521,8 @@ async fn test_network_send_payment_amount_is_too_large() { target_pubkey: Some(node_b_pubkey), amount: Some(100000000000 + 5), payment_hash: Some(gen_sha256_hash()), - final_htlc_expiry_delta: None, + final_tlc_expiry_delta: None, + tlc_expiry_limit: None, invoice: None, timeout: None, max_fee_amount: None, diff --git a/src/fiber/tests/graph.rs b/src/fiber/tests/graph.rs index 64cb22cd..1740ee21 100644 --- a/src/fiber/tests/graph.rs +++ b/src/fiber/tests/graph.rs @@ -1,3 +1,4 @@ +use crate::fiber::config::MAX_PAYMENT_TLC_EXPIRY_LIMIT; use crate::fiber::graph::{PathFindError, SessionRoute}; use crate::fiber::types::Pubkey; use crate::{ @@ -532,7 +533,8 @@ fn test_graph_build_route_three_nodes() { amount: 100, payment_hash: Hash256::default(), invoice: None, - final_htlc_expiry_delta: Some(100), + final_tlc_expiry_delta: None, + tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, timeout: Some(10), max_fee_amount: Some(1000), max_parts: None, @@ -570,7 +572,8 @@ fn test_graph_build_route_exceed_max_htlc_value() { amount: 100, // Exceeds max_htlc_value of 50 payment_hash: Hash256::default(), invoice: None, - final_htlc_expiry_delta: Some(100), + final_tlc_expiry_delta: Some(100), + tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, timeout: Some(10), max_fee_amount: Some(1000), max_parts: None, @@ -596,7 +599,8 @@ fn test_graph_build_route_below_min_htlc_value() { amount: 10, // Below min_htlc_value of 50 payment_hash: Hash256::default(), invoice: None, - final_htlc_expiry_delta: Some(100), + final_tlc_expiry_delta: Some(100), + tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, timeout: Some(10), max_fee_amount: Some(1000), max_parts: None, @@ -641,7 +645,8 @@ fn test_graph_mark_failed_channel() { amount: 100, payment_hash: Hash256::default(), invoice: None, - final_htlc_expiry_delta: Some(100), + final_tlc_expiry_delta: None, + tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, timeout: Some(10), max_fee_amount: Some(1000), max_parts: None, @@ -661,7 +666,8 @@ fn test_graph_mark_failed_channel() { amount: 100, payment_hash: Hash256::default(), invoice: None, - final_htlc_expiry_delta: Some(100), + final_tlc_expiry_delta: None, + tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, timeout: Some(10), max_fee_amount: Some(1000), max_parts: None, @@ -691,7 +697,8 @@ fn test_graph_session_router() { amount: 100, payment_hash: Hash256::default(), invoice: None, - final_htlc_expiry_delta: Some(100), + final_tlc_expiry_delta: Some(100), + tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, timeout: Some(10), max_fee_amount: Some(1000), max_parts: None, @@ -727,7 +734,9 @@ fn test_graph_mark_failed_node() { amount: 100, payment_hash: Hash256::default(), invoice: None, - final_htlc_expiry_delta: Some(100), + final_tlc_expiry_delta: Some(100), + tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, + timeout: Some(10), max_fee_amount: Some(1000), max_parts: None, @@ -744,7 +753,9 @@ fn test_graph_mark_failed_node() { amount: 100, payment_hash: Hash256::default(), invoice: None, - final_htlc_expiry_delta: Some(100), + final_tlc_expiry_delta: Some(100), + tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, + timeout: Some(10), max_fee_amount: Some(1000), max_parts: None, @@ -763,7 +774,9 @@ fn test_graph_mark_failed_node() { amount: 100, payment_hash: Hash256::default(), invoice: None, - final_htlc_expiry_delta: Some(100), + final_tlc_expiry_delta: Some(100), + tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, + timeout: Some(10), max_fee_amount: Some(1000), max_parts: None, @@ -780,7 +793,8 @@ fn test_graph_mark_failed_node() { amount: 100, payment_hash: Hash256::default(), invoice: None, - final_htlc_expiry_delta: Some(100), + final_tlc_expiry_delta: Some(100), + tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, timeout: Some(10), max_fee_amount: Some(1000), max_parts: None, @@ -806,7 +820,9 @@ fn test_graph_payment_self_default_is_false() { target_pubkey: Some(node0.into()), amount: Some(100), payment_hash: Some(Hash256::default()), - final_htlc_expiry_delta: Some(100), + final_tlc_expiry_delta: Some(100), + tlc_expiry_limit: None, + invoice: None, timeout: Some(10), max_fee_amount: Some(1000), @@ -824,7 +840,8 @@ fn test_graph_payment_self_default_is_false() { amount: 100, payment_hash: Hash256::default(), invoice: None, - final_htlc_expiry_delta: Some(100), + final_tlc_expiry_delta: Some(100), + tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, timeout: Some(10), max_fee_amount: Some(1000), max_parts: None, @@ -851,7 +868,8 @@ fn test_graph_payment_pay_single_path() { target_pubkey: Some(network.keys[6].into()), amount: Some(100), payment_hash: Some(Hash256::default()), - final_htlc_expiry_delta: Some(100), + final_tlc_expiry_delta: None, + tlc_expiry_limit: None, invoice: None, timeout: Some(10), max_fee_amount: Some(1000), @@ -881,7 +899,8 @@ fn test_graph_payment_pay_self_with_one_node() { target_pubkey: Some(network.keys[0].into()), amount: Some(100), payment_hash: Some(Hash256::default()), - final_htlc_expiry_delta: Some(100), + final_tlc_expiry_delta: None, + tlc_expiry_limit: None, invoice: None, timeout: Some(10), max_fee_amount: Some(1000), @@ -913,7 +932,8 @@ fn test_graph_build_route_with_double_edge_node() { target_pubkey: Some(network.keys[0].into()), amount: Some(100), payment_hash: Some(Hash256::default()), - final_htlc_expiry_delta: Some(100), + final_tlc_expiry_delta: None, + tlc_expiry_limit: None, invoice: None, timeout: Some(10), max_fee_amount: Some(1000), @@ -943,7 +963,8 @@ fn test_graph_payment_pay_self_will_ok() { target_pubkey: Some(network.keys[0].into()), amount: Some(100), payment_hash: Some(Hash256::default()), - final_htlc_expiry_delta: Some(100), + final_tlc_expiry_delta: None, + tlc_expiry_limit: None, invoice: None, timeout: Some(10), max_fee_amount: Some(1000), diff --git a/src/fiber/tests/network.rs b/src/fiber/tests/network.rs index 8b90a77d..52eb88b4 100644 --- a/src/fiber/tests/network.rs +++ b/src/fiber/tests/network.rs @@ -2,6 +2,7 @@ use super::test_utils::{init_tracing, NetworkNode}; use crate::fiber::network::SendPaymentData; use crate::fiber::tests::test_utils::gen_rand_keypair; use crate::fiber::tests::test_utils::generate_pubkey; +use crate::fiber::tests::test_utils::rand_sha256_hash; use crate::invoice::InvoiceBuilder; use crate::{ fiber::{ @@ -654,7 +655,9 @@ fn test_send_payment_validate_payment_hash() { target_pubkey: Some(generate_pubkey()), amount: Some(10000), payment_hash: None, - final_htlc_expiry_delta: None, + final_tlc_expiry_delta: None, + tlc_expiry_limit: None, + invoice: None, timeout: None, max_fee_amount: None, @@ -675,7 +678,9 @@ fn test_send_payment_validate_amount() { target_pubkey: Some(generate_pubkey()), amount: None, payment_hash: None, - final_htlc_expiry_delta: None, + final_tlc_expiry_delta: None, + tlc_expiry_limit: None, + invoice: None, timeout: None, max_fee_amount: None, @@ -717,7 +722,8 @@ fn test_send_payment_validate_invoice() { target_pubkey: Some(generate_pubkey()), amount: None, payment_hash: None, - final_htlc_expiry_delta: None, + final_tlc_expiry_delta: None, + tlc_expiry_limit: None, invoice: Some(invoice_encoded.clone()), timeout: None, max_fee_amount: None, @@ -737,7 +743,8 @@ fn test_send_payment_validate_invoice() { target_pubkey: None, amount: Some(10), payment_hash: None, - final_htlc_expiry_delta: None, + final_tlc_expiry_delta: None, + tlc_expiry_limit: None, invoice: Some(invoice_encoded.clone()), timeout: None, max_fee_amount: None, @@ -758,7 +765,8 @@ fn test_send_payment_validate_invoice() { target_pubkey: None, amount: None, payment_hash: None, - final_htlc_expiry_delta: None, + final_tlc_expiry_delta: None, + tlc_expiry_limit: None, invoice: Some(invoice_encoded.clone()), timeout: None, max_fee_amount: None, @@ -776,7 +784,8 @@ fn test_send_payment_validate_invoice() { target_pubkey: None, amount: None, payment_hash: None, - final_htlc_expiry_delta: None, + final_tlc_expiry_delta: None, + tlc_expiry_limit: None, invoice: Some(invoice_encoded.clone()), timeout: None, max_fee_amount: None, @@ -794,7 +803,8 @@ fn test_send_payment_validate_invoice() { target_pubkey: Some(generate_pubkey()), amount: Some(10), payment_hash: None, - final_htlc_expiry_delta: None, + final_tlc_expiry_delta: None, + tlc_expiry_limit: None, invoice: None, timeout: None, max_fee_amount: None, @@ -807,12 +817,13 @@ fn test_send_payment_validate_invoice() { let result = SendPaymentData::new(send_command, generate_pubkey().into()); assert!(result.is_ok()); - // invoice with invalid final_htlc_expiry_delta + // invoice with invalid final_tlc_expiry_delta let send_command = SendPaymentCommand { target_pubkey: None, amount: None, payment_hash: None, - final_htlc_expiry_delta: Some(11), + final_tlc_expiry_delta: Some(11), + tlc_expiry_limit: None, invoice: Some(invoice_encoded.clone()), timeout: None, max_fee_amount: None, @@ -826,5 +837,30 @@ fn test_send_payment_validate_invoice() { assert!(result.is_err()); assert!(result .unwrap_err() - .contains("final_htlc_expiry_delta is less than invoice")); + .contains("final_tlc_expiry_delta is less than invoice")); +} + +#[test] +fn test_send_payment_validate_htlc_expiry_delta() { + let send_command = SendPaymentCommand { + target_pubkey: Some(generate_pubkey()), + amount: Some(1000), + payment_hash: Some(rand_sha256_hash()), + final_tlc_expiry_delta: Some(100), + tlc_expiry_limit: None, + invoice: None, + timeout: None, + max_fee_amount: None, + max_parts: None, + keysend: None, + udt_type_script: None, + allow_self_payment: false, + }; + + let result = SendPaymentData::new(send_command, generate_pubkey().into()); + eprintln!("{:?}", result); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .contains("final_tlc_expiry_delta is too small")); } diff --git a/src/rpc/README.md b/src/rpc/README.md index 13e9c55e..f401ccba 100644 --- a/src/rpc/README.md +++ b/src/rpc/README.md @@ -293,7 +293,7 @@ Sends a payment to a peer. * `target_pubkey` - `Option`, the identifier of the payment target * `amount` - `Option`, the amount of the payment * `payment_hash` - `Option`, the hash to use within the payment's HTLC -* `final_htlc_expiry_delta` - `Option`, the htlc expiry delta should be used to set the timelock for the final hop +* `final_tlc_expiry_delta` - `Option`, the htlc expiry delta should be used to set the timelock for the final hop * `invoice` - `Option`, the encoded invoice to send to the recipient * `timeout` - `Option`, the payment timeout in seconds, if the payment is not completed within this time, it will be cancelled * `max_fee_amount` - `Option`, the maximum fee amounts in shannons that the sender is willing to pay diff --git a/src/rpc/channel.rs b/src/rpc/channel.rs index 197a2b4a..eb437fd2 100644 --- a/src/rpc/channel.rs +++ b/src/rpc/channel.rs @@ -287,7 +287,11 @@ pub(crate) struct SendPaymentCommandParams { /// the htlc expiry delta should be used to set the timelock for the final hop, in milliseconds #[serde_as(as = "Option")] - final_htlc_expiry_delta: Option, + final_tlc_expiry_delta: Option, + + /// the htlc expiry limit for the whole payment, in milliseconds + #[serde_as(as = "Option")] + tlc_expiry_limit: Option, /// the encoded invoice to send to the recipient invoice: Option, @@ -618,7 +622,8 @@ where target_pubkey: params.target_pubkey, amount: params.amount, payment_hash: params.payment_hash, - final_htlc_expiry_delta: params.final_htlc_expiry_delta, + final_tlc_expiry_delta: params.final_tlc_expiry_delta, + tlc_expiry_limit: params.tlc_expiry_limit, invoice: params.invoice.clone(), timeout: params.timeout, max_fee_amount: params.max_fee_amount, diff --git a/src/rpc/invoice.rs b/src/rpc/invoice.rs index d3d604d6..2b6408cf 100644 --- a/src/rpc/invoice.rs +++ b/src/rpc/invoice.rs @@ -1,3 +1,4 @@ +use crate::fiber::config::MIN_TLC_EXPIRY_DELTA; use crate::fiber::hash_algorithm::HashAlgorithm; use crate::fiber::serde_utils::{U128Hex, U64Hex}; use crate::fiber::types::{Hash256, Privkey}; @@ -180,6 +181,16 @@ where invoice_builder = invoice_builder.fallback_address(fallback_address); }; if let Some(final_expiry_delta) = params.final_expiry_delta { + if final_expiry_delta < MIN_TLC_EXPIRY_DELTA { + return Err(ErrorObjectOwned::owned( + CALL_EXECUTION_FAILED_CODE, + format!( + "final_expiry_delta must be greater than or equal to {}", + MIN_TLC_EXPIRY_DELTA + ), + Some(params), + )); + } invoice_builder = invoice_builder.final_expiry_delta(final_expiry_delta); }; if let Some(udt_type_script) = ¶ms.udt_type_script { diff --git a/src/tests/store.rs b/src/tests/store.rs index 03335969..6d23b2f3 100644 --- a/src/tests/store.rs +++ b/src/tests/store.rs @@ -1,4 +1,5 @@ use crate::fiber::config::AnnouncedNodeName; +use crate::fiber::config::MAX_PAYMENT_TLC_EXPIRY_LIMIT; use crate::fiber::graph::ChannelInfo; use crate::fiber::graph::NetworkGraphStateStore; use crate::fiber::graph::NodeInfo; @@ -220,7 +221,8 @@ fn test_store_payment_session() { amount: 100, payment_hash, invoice: None, - final_htlc_expiry_delta: Some(100), + final_tlc_expiry_delta: Some(100), + tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, timeout: Some(10), max_fee_amount: Some(1000), max_parts: None, From 4abcb7ccaa422120fe4e148bc8eacbf9a46bf062 Mon Sep 17 00:00:00 2001 From: yukang Date: Thu, 14 Nov 2024 12:52:16 +0800 Subject: [PATCH 05/26] fix e2e invoice expiry --- tests/bruno/e2e/3-nodes-transfer/11-node3-gen-invoice.bru | 2 +- tests/bruno/e2e/3-nodes-transfer/16-node3-gen-invoice.bru | 2 +- tests/bruno/e2e/invoice-ops/1-gen-invoice.bru | 2 +- tests/bruno/e2e/invoice-ops/2-gen-invoice-duplicate.bru | 2 +- tests/bruno/e2e/router-pay/11-node3-gen-invoice.bru | 2 +- tests/bruno/e2e/router-pay/14-node3-gen-invoice-later.bru | 2 +- tests/bruno/e2e/router-pay/22-node3-gen-expiring-invoice.bru | 2 +- tests/bruno/e2e/router-pay/24-node1-gen-invoice-for-self.bru | 2 +- tests/bruno/e2e/udt-router-pay/11-node3-gen-invoice.bru | 2 +- tests/bruno/e2e/udt-router-pay/13-node3-gen-invoice-later.bru | 2 +- tests/bruno/e2e/udt/07-node2-gen-invoice.bru | 2 +- 11 files changed, 11 insertions(+), 11 deletions(-) diff --git a/tests/bruno/e2e/3-nodes-transfer/11-node3-gen-invoice.bru b/tests/bruno/e2e/3-nodes-transfer/11-node3-gen-invoice.bru index 15d87811..30ee4fa5 100644 --- a/tests/bruno/e2e/3-nodes-transfer/11-node3-gen-invoice.bru +++ b/tests/bruno/e2e/3-nodes-transfer/11-node3-gen-invoice.bru @@ -26,7 +26,7 @@ body:json { "currency": "Fibd", "description": "test invoice generated by node3", "expiry": "0xe10", - "final_expiry_delta": "0x28", + "final_expiry_delta": "0x28000", "payment_preimage": "{{payment_preimage}}" } ] diff --git a/tests/bruno/e2e/3-nodes-transfer/16-node3-gen-invoice.bru b/tests/bruno/e2e/3-nodes-transfer/16-node3-gen-invoice.bru index 1986e446..dd4d87c3 100644 --- a/tests/bruno/e2e/3-nodes-transfer/16-node3-gen-invoice.bru +++ b/tests/bruno/e2e/3-nodes-transfer/16-node3-gen-invoice.bru @@ -26,7 +26,7 @@ body:json { "currency": "Fibd", "description": "test invoice generated by node3", "expiry": "0xe10", - "final_expiry_delta": "0x28", + "final_expiry_delta": "0xDFFA0", "payment_preimage": "{{payment_preimage}}" } ] diff --git a/tests/bruno/e2e/invoice-ops/1-gen-invoice.bru b/tests/bruno/e2e/invoice-ops/1-gen-invoice.bru index a4a63dac..8bb43dfa 100644 --- a/tests/bruno/e2e/invoice-ops/1-gen-invoice.bru +++ b/tests/bruno/e2e/invoice-ops/1-gen-invoice.bru @@ -39,7 +39,7 @@ body:json { "currency": "Fibd", "description": "test invoice", "expiry": "0xe10", - "final_expiry_delta": "0x28", + "final_expiry_delta": "0xDFFA0", "payment_preimage": "{{payment_preimage}}" } ] diff --git a/tests/bruno/e2e/invoice-ops/2-gen-invoice-duplicate.bru b/tests/bruno/e2e/invoice-ops/2-gen-invoice-duplicate.bru index 38f6e77a..14001eeb 100644 --- a/tests/bruno/e2e/invoice-ops/2-gen-invoice-duplicate.bru +++ b/tests/bruno/e2e/invoice-ops/2-gen-invoice-duplicate.bru @@ -30,7 +30,7 @@ body:json { "currency": "Fibd", "description": "test invoice", "expiry": "0xe10", - "final_expiry_delta": "0x28", + "final_expiry_delta": "0xDFFA0", "payment_preimage": "{{payment_preimage}}" } ] diff --git a/tests/bruno/e2e/router-pay/11-node3-gen-invoice.bru b/tests/bruno/e2e/router-pay/11-node3-gen-invoice.bru index 15d87811..7a57b1d9 100644 --- a/tests/bruno/e2e/router-pay/11-node3-gen-invoice.bru +++ b/tests/bruno/e2e/router-pay/11-node3-gen-invoice.bru @@ -26,7 +26,7 @@ body:json { "currency": "Fibd", "description": "test invoice generated by node3", "expiry": "0xe10", - "final_expiry_delta": "0x28", + "final_expiry_delta": "0xDFFA0", "payment_preimage": "{{payment_preimage}}" } ] diff --git a/tests/bruno/e2e/router-pay/14-node3-gen-invoice-later.bru b/tests/bruno/e2e/router-pay/14-node3-gen-invoice-later.bru index f62cf864..d7ae3b9f 100644 --- a/tests/bruno/e2e/router-pay/14-node3-gen-invoice-later.bru +++ b/tests/bruno/e2e/router-pay/14-node3-gen-invoice-later.bru @@ -26,7 +26,7 @@ body:json { "currency": "Fibd", "description": "test invoice generated by node3", "expiry": "0xe10", - "final_expiry_delta": "0x28", + "final_expiry_delta": "0xDFFA0", "payment_preimage": "{{payment_preimage}}" } ] diff --git a/tests/bruno/e2e/router-pay/22-node3-gen-expiring-invoice.bru b/tests/bruno/e2e/router-pay/22-node3-gen-expiring-invoice.bru index a321dfac..d49d9bef 100644 --- a/tests/bruno/e2e/router-pay/22-node3-gen-expiring-invoice.bru +++ b/tests/bruno/e2e/router-pay/22-node3-gen-expiring-invoice.bru @@ -26,7 +26,7 @@ body:json { "currency": "Fibd", "description": "test invoice generated by node3", "expiry": "0x2", - "final_expiry_delta": "0x28", + "final_expiry_delta": "0xDFFA0", "payment_preimage": "{{payment_preimage}}" } ] diff --git a/tests/bruno/e2e/router-pay/24-node1-gen-invoice-for-self.bru b/tests/bruno/e2e/router-pay/24-node1-gen-invoice-for-self.bru index 46e06ca2..9b91a9db 100644 --- a/tests/bruno/e2e/router-pay/24-node1-gen-invoice-for-self.bru +++ b/tests/bruno/e2e/router-pay/24-node1-gen-invoice-for-self.bru @@ -26,7 +26,7 @@ body:json { "currency": "Fibd", "description": "test invoice generated by node1", "expiry": "0xe10", - "final_expiry_delta": "0x28", + "final_expiry_delta": "0xDFFA0", "payment_preimage": "{{payment_preimage}}" } ] diff --git a/tests/bruno/e2e/udt-router-pay/11-node3-gen-invoice.bru b/tests/bruno/e2e/udt-router-pay/11-node3-gen-invoice.bru index 65dff4e2..0b01bbe5 100644 --- a/tests/bruno/e2e/udt-router-pay/11-node3-gen-invoice.bru +++ b/tests/bruno/e2e/udt-router-pay/11-node3-gen-invoice.bru @@ -26,7 +26,7 @@ body:json { "currency": "Fibd", "description": "test invoice generated by node3", "expiry": "0xe10", - "final_expiry_delta": "0x28", + "final_expiry_delta": "0xDFFA0", "payment_preimage": "{{payment_preimage}}", "udt_type_script": { "code_hash": "{{UDT_CODE_HASH}}", diff --git a/tests/bruno/e2e/udt-router-pay/13-node3-gen-invoice-later.bru b/tests/bruno/e2e/udt-router-pay/13-node3-gen-invoice-later.bru index 99633888..d63ea12a 100644 --- a/tests/bruno/e2e/udt-router-pay/13-node3-gen-invoice-later.bru +++ b/tests/bruno/e2e/udt-router-pay/13-node3-gen-invoice-later.bru @@ -26,7 +26,7 @@ body:json { "currency": "Fibd", "description": "test invoice generated by node3", "expiry": "0xe10", - "final_expiry_delta": "0x28", + "final_expiry_delta": "0xDFFA0", "payment_preimage": "{{payment_preimage}}", "udt_type_script": { "code_hash": "{{UDT_CODE_HASH}}", diff --git a/tests/bruno/e2e/udt/07-node2-gen-invoice.bru b/tests/bruno/e2e/udt/07-node2-gen-invoice.bru index 3f68bea8..47562c64 100644 --- a/tests/bruno/e2e/udt/07-node2-gen-invoice.bru +++ b/tests/bruno/e2e/udt/07-node2-gen-invoice.bru @@ -39,7 +39,7 @@ body:json { "currency": "Fibd", "description": "test invoice generated by node2", "expiry": "0xe10", - "final_expiry_delta": "0x28", + "final_expiry_delta": "0xDFFA0", "payment_preimage": "{{payment_preimage}}", "udt_type_script": { "code_hash": "{{UDT_CODE_HASH}}", From 085d95a05a99fd4421a618f5fc5d85d9f7b49782 Mon Sep 17 00:00:00 2001 From: yukang Date: Thu, 14 Nov 2024 12:52:51 +0800 Subject: [PATCH 06/26] fix doc --- src/rpc/README.md | 3 ++- tests/bruno/e2e/3-nodes-transfer/11-node3-gen-invoice.bru | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/rpc/README.md b/src/rpc/README.md index f401ccba..83bb5870 100644 --- a/src/rpc/README.md +++ b/src/rpc/README.md @@ -293,7 +293,8 @@ Sends a payment to a peer. * `target_pubkey` - `Option`, the identifier of the payment target * `amount` - `Option`, the amount of the payment * `payment_hash` - `Option`, the hash to use within the payment's HTLC -* `final_tlc_expiry_delta` - `Option`, the htlc expiry delta should be used to set the timelock for the final hop +* `final_tlc_expiry_delta` - `Option`, the htlc expiry delta should be used to set the timelock for the final hop, in milliseconds +* `tlc_expiry_limit` - `Option`, the htlc expiry limit for the whole payment, in milliseconds * `invoice` - `Option`, the encoded invoice to send to the recipient * `timeout` - `Option`, the payment timeout in seconds, if the payment is not completed within this time, it will be cancelled * `max_fee_amount` - `Option`, the maximum fee amounts in shannons that the sender is willing to pay diff --git a/tests/bruno/e2e/3-nodes-transfer/11-node3-gen-invoice.bru b/tests/bruno/e2e/3-nodes-transfer/11-node3-gen-invoice.bru index 30ee4fa5..7a57b1d9 100644 --- a/tests/bruno/e2e/3-nodes-transfer/11-node3-gen-invoice.bru +++ b/tests/bruno/e2e/3-nodes-transfer/11-node3-gen-invoice.bru @@ -26,7 +26,7 @@ body:json { "currency": "Fibd", "description": "test invoice generated by node3", "expiry": "0xe10", - "final_expiry_delta": "0x28000", + "final_expiry_delta": "0xDFFA0", "payment_preimage": "{{payment_preimage}}" } ] From 73499b100478c3800ac7c0faf8dc73a50f47dda6 Mon Sep 17 00:00:00 2001 From: yukang Date: Thu, 14 Nov 2024 17:23:06 +0800 Subject: [PATCH 07/26] fix final htlc expiry --- src/fiber/network.rs | 62 +++++++++++++++++++------------------- src/fiber/tests/graph.rs | 25 ++++++++------- src/fiber/tests/network.rs | 43 ++++++++++++++++++++++++-- src/tests/store.rs | 3 +- 4 files changed, 85 insertions(+), 48 deletions(-) diff --git a/src/fiber/network.rs b/src/fiber/network.rs index 32d08dc1..d01249fb 100644 --- a/src/fiber/network.rs +++ b/src/fiber/network.rs @@ -74,7 +74,7 @@ use crate::ckb::{CkbChainMessage, FundingRequest, FundingTx, TraceTxRequest, Tra use crate::fiber::channel::{ AddTlcCommand, AddTlcResponse, TxCollaborationCommand, TxUpdateCommand, }; -use crate::fiber::config::MAX_PAYMENT_TLC_EXPIRY_LIMIT; +use crate::fiber::config::{DEFAULT_TLC_EXPIRY_DELTA, MAX_PAYMENT_TLC_EXPIRY_LIMIT}; use crate::fiber::graph::{ChannelInfo, PaymentSession, PaymentSessionStatus}; use crate::fiber::serde_utils::EntityHex; use crate::fiber::types::{ @@ -314,7 +314,7 @@ pub struct SendPaymentData { pub amount: u128, pub payment_hash: Hash256, pub invoice: Option, - pub final_tlc_expiry_delta: Option, + pub final_tlc_expiry_delta: u64, pub tlc_expiry_limit: u64, pub timeout: Option, pub max_fee_amount: Option, @@ -388,33 +388,35 @@ impl SendPaymentData { }; // check htlc expiry delta and limit are both valid if it is set - if let Some(final_tlc_expiry_delta) = command.final_tlc_expiry_delta { - if let Some(invoice_final_htlc_minimum_expiry_delta) = invoice - .as_ref() - .and_then(|i| i.final_htlc_minimum_expiry_delta()) - { - if *invoice_final_htlc_minimum_expiry_delta > final_tlc_expiry_delta { - return Err("final_tlc_expiry_delta is less than invoice final_htlc_minimum_expiry_delta".to_string()); - } - } + let final_tlc_expiry_delta = command + .final_tlc_expiry_delta + .or_else(|| { + invoice + .as_ref() + .and_then(|i| i.final_htlc_minimum_expiry_delta().copied()) + }) + .unwrap_or(DEFAULT_TLC_EXPIRY_DELTA); + if final_tlc_expiry_delta < MIN_TLC_EXPIRY_DELTA + || final_tlc_expiry_delta > MAX_PAYMENT_TLC_EXPIRY_LIMIT + { + return Err(format!( + "invalid final_tlc_expiry_delta, expect between {} and {}", + MIN_TLC_EXPIRY_DELTA, MAX_PAYMENT_TLC_EXPIRY_LIMIT + )); + } - if final_tlc_expiry_delta < MIN_TLC_EXPIRY_DELTA { - return Err("final_tlc_expiry_delta is too small".to_string()); - } + let tlc_expiry_limit = command + .tlc_expiry_limit + .unwrap_or(MAX_PAYMENT_TLC_EXPIRY_LIMIT); + + if tlc_expiry_limit < final_tlc_expiry_delta || tlc_expiry_limit < MIN_TLC_EXPIRY_DELTA { + return Err("tlc_expiry_limit is too small".to_string()); } - if let Some(tlc_expiry_limit) = command.tlc_expiry_limit { - let final_tlc_expiry_delta = command.final_tlc_expiry_delta.unwrap_or(0); - if tlc_expiry_limit <= final_tlc_expiry_delta - || tlc_expiry_limit <= MIN_TLC_EXPIRY_DELTA - { - return Err("tlc_expiry_limit too small".to_string()); - } - if tlc_expiry_limit > MAX_PAYMENT_TLC_EXPIRY_LIMIT { - return Err(format!( - "tlc_expiry_limit too large, expect it to less than {}", - MAX_PAYMENT_TLC_EXPIRY_LIMIT - )); - } + if tlc_expiry_limit > MAX_PAYMENT_TLC_EXPIRY_LIMIT { + return Err(format!( + "tlc_expiry_limit is too large, expect it to less than {}", + MAX_PAYMENT_TLC_EXPIRY_LIMIT + )); } let keysend = command.keysend.unwrap_or(false); @@ -449,10 +451,8 @@ impl SendPaymentData { amount, payment_hash, invoice: command.invoice, - final_tlc_expiry_delta: command.final_tlc_expiry_delta, - tlc_expiry_limit: command - .tlc_expiry_limit - .unwrap_or(MAX_PAYMENT_TLC_EXPIRY_LIMIT), + final_tlc_expiry_delta, + tlc_expiry_limit, timeout: command.timeout, max_fee_amount: command.max_fee_amount, max_parts: command.max_parts, diff --git a/src/fiber/tests/graph.rs b/src/fiber/tests/graph.rs index 1740ee21..9e9fb021 100644 --- a/src/fiber/tests/graph.rs +++ b/src/fiber/tests/graph.rs @@ -1,4 +1,4 @@ -use crate::fiber::config::MAX_PAYMENT_TLC_EXPIRY_LIMIT; +use crate::fiber::config::{DEFAULT_TLC_EXPIRY_DELTA, MAX_PAYMENT_TLC_EXPIRY_LIMIT}; use crate::fiber::graph::{PathFindError, SessionRoute}; use crate::fiber::types::Pubkey; use crate::{ @@ -533,7 +533,7 @@ fn test_graph_build_route_three_nodes() { amount: 100, payment_hash: Hash256::default(), invoice: None, - final_tlc_expiry_delta: None, + final_tlc_expiry_delta: DEFAULT_TLC_EXPIRY_DELTA, tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, timeout: Some(10), max_fee_amount: Some(1000), @@ -572,7 +572,7 @@ fn test_graph_build_route_exceed_max_htlc_value() { amount: 100, // Exceeds max_htlc_value of 50 payment_hash: Hash256::default(), invoice: None, - final_tlc_expiry_delta: Some(100), + final_tlc_expiry_delta: DEFAULT_TLC_EXPIRY_DELTA, tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, timeout: Some(10), max_fee_amount: Some(1000), @@ -599,7 +599,7 @@ fn test_graph_build_route_below_min_htlc_value() { amount: 10, // Below min_htlc_value of 50 payment_hash: Hash256::default(), invoice: None, - final_tlc_expiry_delta: Some(100), + final_tlc_expiry_delta: DEFAULT_TLC_EXPIRY_DELTA, tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, timeout: Some(10), max_fee_amount: Some(1000), @@ -645,7 +645,7 @@ fn test_graph_mark_failed_channel() { amount: 100, payment_hash: Hash256::default(), invoice: None, - final_tlc_expiry_delta: None, + final_tlc_expiry_delta: DEFAULT_TLC_EXPIRY_DELTA, tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, timeout: Some(10), max_fee_amount: Some(1000), @@ -666,7 +666,7 @@ fn test_graph_mark_failed_channel() { amount: 100, payment_hash: Hash256::default(), invoice: None, - final_tlc_expiry_delta: None, + final_tlc_expiry_delta: DEFAULT_TLC_EXPIRY_DELTA, tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, timeout: Some(10), max_fee_amount: Some(1000), @@ -697,7 +697,7 @@ fn test_graph_session_router() { amount: 100, payment_hash: Hash256::default(), invoice: None, - final_tlc_expiry_delta: Some(100), + final_tlc_expiry_delta: DEFAULT_TLC_EXPIRY_DELTA, tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, timeout: Some(10), max_fee_amount: Some(1000), @@ -734,9 +734,8 @@ fn test_graph_mark_failed_node() { amount: 100, payment_hash: Hash256::default(), invoice: None, - final_tlc_expiry_delta: Some(100), + final_tlc_expiry_delta: DEFAULT_TLC_EXPIRY_DELTA, tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, - timeout: Some(10), max_fee_amount: Some(1000), max_parts: None, @@ -753,7 +752,7 @@ fn test_graph_mark_failed_node() { amount: 100, payment_hash: Hash256::default(), invoice: None, - final_tlc_expiry_delta: Some(100), + final_tlc_expiry_delta: DEFAULT_TLC_EXPIRY_DELTA, tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, timeout: Some(10), @@ -774,7 +773,7 @@ fn test_graph_mark_failed_node() { amount: 100, payment_hash: Hash256::default(), invoice: None, - final_tlc_expiry_delta: Some(100), + final_tlc_expiry_delta: DEFAULT_TLC_EXPIRY_DELTA, tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, timeout: Some(10), @@ -793,7 +792,7 @@ fn test_graph_mark_failed_node() { amount: 100, payment_hash: Hash256::default(), invoice: None, - final_tlc_expiry_delta: Some(100), + final_tlc_expiry_delta: DEFAULT_TLC_EXPIRY_DELTA, tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, timeout: Some(10), max_fee_amount: Some(1000), @@ -840,7 +839,7 @@ fn test_graph_payment_self_default_is_false() { amount: 100, payment_hash: Hash256::default(), invoice: None, - final_tlc_expiry_delta: Some(100), + final_tlc_expiry_delta: DEFAULT_TLC_EXPIRY_DELTA, tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, timeout: Some(10), max_fee_amount: Some(1000), diff --git a/src/fiber/tests/network.rs b/src/fiber/tests/network.rs index 52eb88b4..d8f832cd 100644 --- a/src/fiber/tests/network.rs +++ b/src/fiber/tests/network.rs @@ -1,4 +1,5 @@ use super::test_utils::{init_tracing, NetworkNode}; +use crate::fiber::config::DEFAULT_TLC_EXPIRY_DELTA; use crate::fiber::network::SendPaymentData; use crate::fiber::tests::test_utils::gen_rand_keypair; use crate::fiber::tests::test_utils::generate_pubkey; @@ -712,7 +713,9 @@ fn test_send_payment_validate_invoice() { .expiry_time(Duration::from_secs(1024)) .payee_pub_key(public_key) .add_attr(Attribute::FinalHtlcTimeout(5)) - .add_attr(Attribute::FinalHtlcMinimumExpiryDelta(12)) + .add_attr(Attribute::FinalHtlcMinimumExpiryDelta( + DEFAULT_TLC_EXPIRY_DELTA, + )) .add_attr(Attribute::Description("description".to_string())) .build_with_sign(|hash| Secp256k1::new().sign_ecdsa_recoverable(hash, &private_key)) .unwrap(); @@ -837,7 +840,41 @@ fn test_send_payment_validate_invoice() { assert!(result.is_err()); assert!(result .unwrap_err() - .contains("final_tlc_expiry_delta is less than invoice")); + .contains("invalid final_tlc_expiry_delta")); + + // invoice with invalid final_tlc_expiry_delta + let invoice = InvoiceBuilder::new(Currency::Fibb) + .amount(Some(1280)) + .payment_hash(gen_payment_hash) + .fallback_address("address".to_string()) + .expiry_time(Duration::from_secs(1024)) + .payee_pub_key(public_key) + .add_attr(Attribute::FinalHtlcTimeout(5)) + .add_attr(Attribute::FinalHtlcMinimumExpiryDelta(11)) + .add_attr(Attribute::Description("description".to_string())) + .build_with_sign(|hash| Secp256k1::new().sign_ecdsa_recoverable(hash, &private_key)) + .unwrap(); + let invoice_encoded = invoice.to_string(); + let send_command = SendPaymentCommand { + target_pubkey: None, + amount: None, + payment_hash: None, + final_tlc_expiry_delta: None, + tlc_expiry_limit: None, + invoice: Some(invoice_encoded.clone()), + timeout: None, + max_fee_amount: None, + max_parts: None, + keysend: None, + udt_type_script: None, + allow_self_payment: false, + }; + + let result = SendPaymentData::new(send_command, generate_pubkey().into()); + assert!(result.is_err()); + assert!(result + .unwrap_err() + .contains("invalid final_tlc_expiry_delta")); } #[test] @@ -862,5 +899,5 @@ fn test_send_payment_validate_htlc_expiry_delta() { assert!(result.is_err()); assert!(result .unwrap_err() - .contains("final_tlc_expiry_delta is too small")); + .contains("invalid final_tlc_expiry_delta")); } diff --git a/src/tests/store.rs b/src/tests/store.rs index 43da882f..209f7bae 100644 --- a/src/tests/store.rs +++ b/src/tests/store.rs @@ -1,4 +1,5 @@ use crate::fiber::config::AnnouncedNodeName; +use crate::fiber::config::DEFAULT_TLC_EXPIRY_DELTA; use crate::fiber::config::MAX_PAYMENT_TLC_EXPIRY_LIMIT; use crate::fiber::graph::ChannelInfo; use crate::fiber::graph::NetworkGraphStateStore; @@ -221,7 +222,7 @@ fn test_store_payment_session() { amount: 100, payment_hash, invoice: None, - final_tlc_expiry_delta: Some(100), + final_tlc_expiry_delta: DEFAULT_TLC_EXPIRY_DELTA, tlc_expiry_limit: MAX_PAYMENT_TLC_EXPIRY_LIMIT, timeout: Some(10), max_fee_amount: Some(1000), From bb267726520630bf917f0223c3a96e179f13dc38 Mon Sep 17 00:00:00 2001 From: yukang Date: Thu, 14 Nov 2024 21:29:12 +0800 Subject: [PATCH 08/26] fix ui failures --- src/fiber/tests/graph.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/fiber/tests/graph.rs b/src/fiber/tests/graph.rs index dcd057c5..74ace8c7 100644 --- a/src/fiber/tests/graph.rs +++ b/src/fiber/tests/graph.rs @@ -1004,7 +1004,8 @@ fn test_graph_build_route_with_other_node_maybe_better() { target_pubkey: Some(network.keys[0].into()), amount: Some(100), payment_hash: Some(Hash256::default()), - final_htlc_expiry_delta: Some(100), + final_tlc_expiry_delta: None, + tlc_expiry_limit: None, invoice: None, timeout: Some(10), max_fee_amount: Some(1000), @@ -1093,7 +1094,8 @@ fn test_graph_build_route_with_path_limits() { target_pubkey: Some(node99.into()), amount: Some(100), payment_hash: Some(Hash256::default()), - final_htlc_expiry_delta: Some(100), + final_tlc_expiry_delta: None, + tlc_expiry_limit: None, invoice: None, timeout: Some(10), max_fee_amount: Some(1000), @@ -1135,7 +1137,8 @@ fn test_graph_build_route_with_path_limit_fail_with_fee_not_enough() { target_pubkey: Some(node99.into()), amount: Some(100), payment_hash: Some(Hash256::default()), - final_htlc_expiry_delta: Some(100), + final_tlc_expiry_delta: None, + tlc_expiry_limit: None, invoice: None, timeout: Some(10), max_fee_amount: Some(1000), From 30e0fced31883abd582e9b5791a51d110a2cfe55 Mon Sep 17 00:00:00 2001 From: yukang Date: Thu, 14 Nov 2024 23:17:09 +0800 Subject: [PATCH 09/26] considering tlc expiry in find_path --- src/fiber/graph.rs | 23 ++++++++++++------ src/fiber/tests/graph.rs | 50 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/src/fiber/graph.rs b/src/fiber/graph.rs index 6c1d7919..0eb7ba83 100644 --- a/src/fiber/graph.rs +++ b/src/fiber/graph.rs @@ -555,6 +555,8 @@ where amount, payment_data.max_fee_amount, udt_type_script, + payment_data.final_tlc_expiry_delta, + payment_data.tlc_expiry_limit, allow_self_payment, )?; assert!(!route.is_empty()); @@ -638,6 +640,8 @@ where amount: u128, max_fee_amount: Option, udt_type_script: Option