From 90f0992e4b90fb27def527a44a2c44f7b04d449f Mon Sep 17 00:00:00 2001 From: yukang Date: Wed, 20 Nov 2024 01:03:27 +0800 Subject: [PATCH 1/4] fix router fee rate issue --- src/ckb/contracts.rs | 67 ++++++++++++------- src/fiber/channel.rs | 9 ++- src/fiber/graph.rs | 62 ++++++++++++----- src/fiber/network.rs | 4 +- src/fiber/tests/graph.rs | 43 ++++++++---- src/main.rs | 7 +- src/rpc/README.md | 10 +-- src/rpc/channel.rs | 16 ++--- .../12-node1-to-get-payment-status.bru | 48 +++++++++++++ .../e2e/router-pay/32-node2-list-channels.bru | 40 +++++++++++ .../router-pay/33-node2-update-channel.bru | 43 ++++++++++++ .../e2e/router-pay/34-node1-send-payment.bru | 42 ++++++++++++ .../35-node1-get-payment-status.bru | 48 +++++++++++++ 13 files changed, 367 insertions(+), 72 deletions(-) create mode 100644 tests/bruno/e2e/router-pay/12-node1-to-get-payment-status.bru create mode 100644 tests/bruno/e2e/router-pay/32-node2-list-channels.bru create mode 100644 tests/bruno/e2e/router-pay/33-node2-update-channel.bru create mode 100644 tests/bruno/e2e/router-pay/34-node1-send-payment.bru create mode 100644 tests/bruno/e2e/router-pay/35-node1-get-payment-status.bru diff --git a/src/ckb/contracts.rs b/src/ckb/contracts.rs index 9d85a9f0d..965a69c24 100644 --- a/src/ckb/contracts.rs +++ b/src/ckb/contracts.rs @@ -7,6 +7,7 @@ use once_cell::sync::OnceCell; use regex::Regex; use serde::{Deserialize, Serialize}; use std::{collections::HashMap, vec}; +use thiserror::Error; use tracing::info; use crate::fiber::config::FiberScript; @@ -34,27 +35,42 @@ pub struct ContractsContext { pub contracts: ContractsInfo, } +#[derive(Debug, Error)] +pub enum ContractsContextError { + #[error("Context already initialized")] + ContextAlreadyInitialized, + + #[error("Genesis block transaction #{0} should exist")] + GenesisBlockTransactionNotFound(usize), + + #[error("Genesis block transaction #0 output #{0} should exist")] + GenesisBlockTransaction0OutputNotFound(usize), + + #[error("Genesis block secp256k1 binary cell type script should exist")] + GenesisBlockSecp256k1BinaryCellTypeScriptNotFound, +} + impl ContractsContext { - pub fn new( + pub fn try_new( genesis_block: BlockView, fiber_scripts: Vec, udt_whitelist: UdtCfgInfos, - ) -> Self { + ) -> Result { let mut contract_default_scripts: HashMap = HashMap::new(); let mut script_cell_deps: HashMap> = HashMap::new(); let genesis_tx = genesis_block .transaction(0) - .expect("genesis block transaction #0 should exist"); + .ok_or(ContractsContextError::GenesisBlockTransactionNotFound(0))?; // setup secp256k1 let secp256k1_binary_cell = genesis_tx .output(1) - .expect("genesis block transaction #0 output #1 should exist"); + .ok_or(ContractsContextError::GenesisBlockTransaction0OutputNotFound(1))?; let secp256k1_binary_cell_type_script = secp256k1_binary_cell .type_() .to_opt() - .expect("secp256k1 binary type script should exist"); + .ok_or(ContractsContextError::GenesisBlockSecp256k1BinaryCellTypeScriptNotFound)?; contract_default_scripts.insert( Contract::Secp256k1Lock, Script::new_builder() @@ -65,7 +81,7 @@ impl ContractsContext { let secp256k1_dep_group_tx_hash = genesis_block .transaction(1) - .expect("genesis block transaction #1 should exist") + .ok_or(ContractsContextError::GenesisBlockTransactionNotFound(1))? .hash(); let secp256k1_dep_group_out_point = OutPoint::new_builder() .tx_hash(secp256k1_dep_group_tx_hash) @@ -119,7 +135,11 @@ impl ContractsContext { let output_data = genesis_tx .outputs_data() .get(index as usize) - .expect("contract output data should exist in the genesis tx") + .ok_or( + ContractsContextError::GenesisBlockTransaction0OutputNotFound( + index as usize, + ), + )? .raw_data(); let cell_deps = if matches!(contract, Contract::FundingLock | Contract::CommitmentLock) { @@ -150,13 +170,13 @@ impl ContractsContext { script_cell_deps.insert(name, cell_deps.into_iter().map(CellDep::from).collect()); } - Self { + Ok(Self { contracts: ContractsInfo { contract_default_scripts, script_cell_deps, udt_whitelist, }, - } + }) } fn get_contracts_map(&self) -> &HashMap { @@ -180,7 +200,7 @@ impl ContractsContext { pub(crate) fn get_script(&self, contract: Contract, args: &[u8]) -> Script { self.get_contracts_map() .get(&contract) - .unwrap_or_else(|| panic!("Contract {:?} exists", contract)) + .unwrap_or_else(|| panic!("Contract {:?} should exist", contract)) .clone() .as_builder() .args(args.pack()) @@ -189,14 +209,15 @@ impl ContractsContext { pub(crate) fn get_udt_info(&self, udt_script: &Script) -> Option<&UdtArgInfo> { for udt in &self.get_udt_whitelist().0 { - let _type: ScriptHashType = udt_script.hash_type().try_into().expect("valid hash type"); - if udt.script.code_hash.pack() == udt_script.code_hash() - && udt.script.hash_type == _type - { - let args = format!("0x{:x}", udt_script.args().raw_data()); - let pattern = Regex::new(&udt.script.args).expect("invalid expression"); - if pattern.is_match(&args) { - return Some(udt); + if let Some(_type) = udt_script.hash_type().try_into().ok() { + if udt.script.code_hash.pack() == udt_script.code_hash() + && udt.script.hash_type == _type + { + let args = format!("0x{:x}", udt_script.args().raw_data()); + let pattern = Regex::new(&udt.script.args).expect("invalid expression"); + if pattern.is_match(&args) { + return Some(udt); + } } } } @@ -206,18 +227,18 @@ impl ContractsContext { pub static CONTRACTS_CONTEXT_INSTANCE: OnceCell = OnceCell::new(); -pub fn init_contracts_context( +pub fn try_init_contracts_context( genesis_block: BlockView, fiber_scripts: Vec, udt_whitelist: UdtCfgInfos, -) { +) -> Result<(), ContractsContextError> { CONTRACTS_CONTEXT_INSTANCE - .set(ContractsContext::new( + .set(ContractsContext::try_new( genesis_block, fiber_scripts, udt_whitelist, - )) - .expect("init_contracts_context should only be called once"); + )?) + .map_err(|_| ContractsContextError::ContextAlreadyInitialized) } #[cfg(not(test))] diff --git a/src/fiber/channel.rs b/src/fiber/channel.rs index 014aa9d5e..127c99f23 100644 --- a/src/fiber/channel.rs +++ b/src/fiber/channel.rs @@ -881,7 +881,12 @@ where .expect("public channel exits") .tlc_fee_proportional_millionths .unwrap_or_default(); + info!("expecting fee_rate: {}", fee_rate); let expected_fee = calculate_tlc_forward_fee(forward_amount, fee_rate); + info!( + "forward_fee: {} expected_fee: {}", + forward_fee, expected_fee + ); if forward_fee < expected_fee { error!( "too low forward_fee: {}, expected_fee: {}", @@ -2983,11 +2988,11 @@ impl ChannelActorState { .sum::() } - pub fn get_created_at_in_microseconds(&self) -> u64 { + pub fn get_created_at_in_millis(&self) -> u64 { self.created_at .duration_since(UNIX_EPOCH) .expect("Duration since unix epoch") - .as_micros() as u64 + .as_millis() as u64 } pub fn is_closed(&self) -> bool { diff --git a/src/fiber/graph.rs b/src/fiber/graph.rs index 37da7e5ad..10d252bb8 100644 --- a/src/fiber/graph.rs +++ b/src/fiber/graph.rs @@ -9,6 +9,7 @@ use crate::fiber::path::NodeHeapElement; use crate::fiber::serde_utils::EntityHex; use crate::fiber::types::PaymentHopData; use crate::invoice::CkbInvoice; +use crate::now_timestamp; use ckb_jsonrpc_types::JsonBytes; use ckb_types::packed::{OutPoint, Script}; use serde::{Deserialize, Serialize}; @@ -112,6 +113,16 @@ impl ChannelInfo { pub fn funding_tx_block_number(&self) -> u64 { self.funding_tx_block_number } + + fn get_update_info_with(&self, node: Pubkey) -> Option<&ChannelUpdateInfo> { + if self.node2() == node { + self.node1_to_node2.as_ref() + } else if self.node1() == node { + self.node2_to_node1.as_ref() + } else { + None + } + } } #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] @@ -393,8 +404,10 @@ where &channel, &update ); let update_info = if update.message_flags & 1 == 1 { + debug!("now update node1_to_node2: {:?}", &update); &mut channel.node1_to_node2 } else { + debug!("now update node2_to_node1: {:?}", &update); &mut channel.node2_to_node1 }; @@ -427,6 +440,7 @@ where last_update_message: update.clone(), }); + info!("now new update_info: {:?}", *update_info); self.store.insert_channel(channel.to_owned()); debug!( "Processed channel update: channel {:?}, update {:?}", @@ -449,12 +463,14 @@ where self.channels.values().filter_map(move |channel| { if let Some(info) = channel.node1_to_node2.as_ref() { if info.enabled && channel.node2() == node_id { + debug!("now use node1_to_node2: {:?}", info); return Some((channel.node1(), channel.node2(), channel, info)); } } if let Some(info) = channel.node2_to_node1.as_ref() { if info.enabled && channel.node1() == node_id { + debug!("now use node2_to_node1: {:?}", info); return Some((channel.node2(), channel.node1(), channel, info)); } } @@ -578,17 +594,18 @@ where (0, 0) } else { let channel_info = self - .get_channel(&route[i + 1].channel_outpoint) + .get_channel(&route[i].channel_outpoint) .expect("channel not found"); - let channel_update = &if channel_info.node1() == route[i + 1].target { - channel_info.node2_to_node1.as_ref() - } else { - channel_info.node1_to_node2.as_ref() - } - .expect("channel_update is none"); + let channel_update = channel_info + .get_update_info_with(route[i].target) + .expect("channel_update is none"); let fee_rate = channel_update.fee_rate; let fee = calculate_tlc_forward_fee(current_amount, fee_rate as u128); let expiry = channel_update.htlc_expiry_delta; + eprintln!( + "fee: {:?}, expiry: {:?}, current_amount: {:?}, fee_rate: {:?}", + fee, expiry, current_amount, fee_rate + ); (fee, expiry) }; @@ -712,6 +729,7 @@ where .values() .any(|x| x == &channel_info.out_point()) { + eprintln!("here ......"); continue; } @@ -722,9 +740,15 @@ where let fee = calculate_tlc_forward_fee(next_hop_received_amount, fee_rate as u128); let amount_to_send = next_hop_received_amount + fee; + eprintln!("amount_to_send: {:?}", amount_to_send); // if the amount to send is greater than the amount we have, skip this edge if let Some(max_fee_amount) = max_fee_amount { if amount_to_send > amount + max_fee_amount { + eprintln!( + "amount_to_send {} > amount + max_fee_amount {}", + amount_to_send, + amount + max_fee_amount + ); continue; } } @@ -734,6 +758,13 @@ where || (channel_update.htlc_maximum_value != 0 && amount_to_send > channel_update.htlc_maximum_value) { + eprintln!( + "now .......amount: {:?} capacity: {:?}, channel_update.htlc_maximum_value: {:?}", + amount_to_send, + channel_info.capacity(), + channel_update.htlc_maximum_value + + ); continue; } if amount_to_send < channel_update.htlc_minimum_value { @@ -755,6 +786,7 @@ where ); if probability < DEFAULT_MIN_PROBABILITY { + eprintln!("probability < DEFAULT_MIN_PROBABILITY"); continue; } let agg_weight = @@ -767,6 +799,8 @@ where continue; } } + eprintln!("\n\nfind_path from: {:?}, to: {:?}", from, to); + eprintln!("add use channel_info: {:?}\n\n", channel_info); let node = NodeHeapElement { node_id: from, weight, @@ -912,8 +946,8 @@ pub struct PaymentSession { pub last_error: Option, pub try_limit: u32, pub status: PaymentSessionStatus, - pub created_at: u128, - pub last_updated_at: u128, + pub created_at: u64, + pub last_updated_at: u64, // The channel_outpoint and the tlc_id of the first hop #[serde_as(as = "Option")] pub first_hop_channel_outpoint: Option, @@ -923,10 +957,7 @@ pub struct PaymentSession { impl PaymentSession { pub fn new(request: SendPaymentData, try_limit: u32) -> Self { - let now = std::time::UNIX_EPOCH - .elapsed() - .expect("Duration since unix epoch") - .as_millis(); + let now = now_timestamp(); Self { request, retried_times: 0, @@ -947,10 +978,7 @@ impl PaymentSession { fn set_status(&mut self, status: PaymentSessionStatus) { self.status = status; - self.last_updated_at = std::time::UNIX_EPOCH - .elapsed() - .expect("Duration since unix epoch") - .as_micros(); + self.last_updated_at = now_timestamp(); } pub fn set_inflight_status(&mut self, channel_outpoint: OutPoint, tlc_id: u64) { diff --git a/src/fiber/network.rs b/src/fiber/network.rs index 45cb94451..677c007a2 100644 --- a/src/fiber/network.rs +++ b/src/fiber/network.rs @@ -135,8 +135,8 @@ pub struct AcceptChannelResponse { pub struct SendPaymentResponse { pub payment_hash: Hash256, pub status: PaymentSessionStatus, - pub created_at: u128, - pub last_updated_at: u128, + pub created_at: u64, + pub last_updated_at: u64, pub failed_error: Option, } diff --git a/src/fiber/tests/graph.rs b/src/fiber/tests/graph.rs index ff08837a9..34c3ab7b0 100644 --- a/src/fiber/tests/graph.rs +++ b/src/fiber/tests/graph.rs @@ -97,16 +97,22 @@ impl MockNetworkGraph { ) { let public_key1 = self.keys[node_a]; let public_key2 = self.keys[node_b]; + let node_a_is_node1 = public_key1 < public_key2; let idx = self.edges.len() + 1; let channel_outpoint = OutPoint::from_slice(&[idx as u8; 36]).unwrap(); self.edges.push((node_a, node_b, channel_outpoint.clone())); + let (node_a_key, node_b_key) = if node_a_is_node1 { + (public_key1, public_key2) + } else { + (public_key2, public_key1) + }; let channel_info = ChannelInfo { funding_tx_block_number: 0, funding_tx_index: 0, announcement_msg: ChannelAnnouncement { chain_hash: get_chain_hash(), - node1_id: public_key1.into(), - node2_id: public_key2.into(), + node1_id: node_a_key.into(), + node2_id: node_b_key.into(), channel_outpoint: channel_outpoint.clone(), node1_signature: None, node2_signature: None, @@ -120,12 +126,12 @@ impl MockNetworkGraph { node1_to_node2: None, node2_to_node1: None, }; - self.graph.add_channel(channel_info); + self.graph.add_channel(channel_info.clone()); let channel_update = ChannelUpdate { signature: None, chain_hash: get_chain_hash(), version: 0, - message_flags: 1, + message_flags: if node_a_is_node1 { 1 } else { 0 }, channel_flags: 0, tlc_expiry_delta: 144, tlc_fee_proportional_millionths: fee_rate.unwrap_or(0), @@ -133,13 +139,15 @@ impl MockNetworkGraph { tlc_minimum_value: min_htlc_value.unwrap_or(0), channel_outpoint: channel_outpoint.clone(), }; + eprintln!("add channel_info: {:?}", channel_info); + eprintln!("add channel_update: {:?}", channel_update); self.graph.process_channel_update(channel_update).unwrap(); if let Some(fee_rate) = other_fee_rate { let channel_update = ChannelUpdate { signature: None, chain_hash: get_chain_hash(), version: 0, - message_flags: 0, + message_flags: if node_a_is_node1 { 0 } else { 1 }, channel_flags: 0, tlc_expiry_delta: 144, tlc_fee_proportional_millionths: fee_rate, @@ -147,7 +155,9 @@ impl MockNetworkGraph { tlc_minimum_value: min_htlc_value.unwrap_or(0), channel_outpoint: channel_outpoint.clone(), }; + eprintln!("add rev channel_update: {:?}", channel_update); self.graph.process_channel_update(channel_update).unwrap(); + //eprintln!("add channel_info: {:?}", channel_info); } } @@ -521,9 +531,8 @@ fn test_graph_find_path_err() { #[test] fn test_graph_build_route_three_nodes() { let mut network = MockNetworkGraph::new(3); - network.add_edge(0, 2, Some(500), Some(2)); + network.add_edge(0, 2, Some(500), Some(200000)); network.add_edge(2, 3, Some(500), Some(2)); - let _node0 = network.keys[0]; let node2 = network.keys[2]; let node3 = network.keys[3]; // Test build route from node1 to node3 @@ -551,7 +560,7 @@ fn test_graph_build_route_three_nodes() { assert_eq!(route[1].next_hop, Some(node3.into())); assert_eq!(route[2].next_hop, None); - assert_eq!(route[0].amount, 101); + assert_eq!(route[0].amount, 120); assert_eq!(route[1].amount, 100); assert_eq!(route[2].amount, 100); } @@ -895,6 +904,7 @@ fn test_graph_payment_pay_self_with_one_node() { let payment_data = payment_data.unwrap(); let route = network.graph.build_route(payment_data); + eprintln!("final result {:?}", route); assert!(route.is_ok()); let route = route.unwrap(); assert_eq!(route[1].next_hop, Some(node0.into())); @@ -1052,14 +1062,16 @@ fn test_graph_payment_pay_self_will_ok() { fn test_graph_build_route_with_path_limits() { let mut network = MockNetworkGraph::new(100); // Add edges with min_htlc_value set to 50 + let mut fee_rate = 100000; for i in 0..99 { + fee_rate -= 1000; network.add_edge_with_config( i, i + 1, - Some(500), - Some(500), + Some(5000000), + Some(fee_rate), Some(50), - None, + Some(10000000), None, Some(100), ); @@ -1076,7 +1088,7 @@ fn test_graph_build_route_with_path_limits() { final_htlc_expiry_delta: Some(100), invoice: None, timeout: Some(10), - max_fee_amount: Some(1000), + max_fee_amount: Some(10000000), max_parts: None, keysend: Some(false), udt_type_script: None, @@ -1088,6 +1100,13 @@ fn test_graph_build_route_with_path_limits() { let route = route.unwrap(); assert_eq!(route.len(), 100); assert_eq!(route[98].next_hop, Some(node99.into())); + + // make sure the fee is decreasing + let mut fees = vec![]; + for i in 0..98 { + fees.push(route[i].amount - route[i + 1].amount); + } + assert!(fees.windows(2).all(|x| x[0] >= x[1])); } #[test] diff --git a/src/main.rs b/src/main.rs index 68fc775d6..adcb6b13b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3,7 +3,7 @@ use ckb_resource::Resource; use fnn::actors::RootActor; use fnn::cch::CchMessage; use fnn::ckb::{ - contracts::{get_script_by_contract, init_contracts_context, Contract}, + contracts::{get_script_by_contract, try_init_contracts_context, Contract}, CkbChainActor, }; use fnn::fiber::{channel::ChannelSubscribers, graph::NetworkGraph, network::init_chain_hash}; @@ -100,11 +100,12 @@ pub async fn main() -> Result<(), ExitMessage> { })?; init_chain_hash(genesis_block.hash().into()); - init_contracts_context( + try_init_contracts_context( genesis_block, fiber_config.scripts.clone(), ckb_config.udt_whitelist.clone().unwrap_or_default(), - ); + ) + .map_err(|err| ExitMessage(format!("failed to init contracts context: {}", err)))?; let ckb_actor = Actor::spawn_linked( Some("ckb".to_string()), diff --git a/src/rpc/README.md b/src/rpc/README.md index 13e9c55e1..cc56aa809 100644 --- a/src/rpc/README.md +++ b/src/rpc/README.md @@ -306,8 +306,8 @@ Sends a payment to a peer. * `payment_hash` - Hash256, The payment hash of the payment * `status` - PaymentSessionStatus, The status of the payment -* `created_at` - u128, The time the payment was created at -* `last_updated_at` - u128, The time the payment was last updated at +* `created_at` - u64, The time the payment was created at, in milliseconds from UNIX epoch +* `last_updated_at` - u64, The time the payment was last updated at, in milliseconds from UNIX epoch * `failed_error` - `Option`, The error message if the payment failed @@ -324,8 +324,8 @@ Retrieves a payment. * `payment_hash` - Hash256, The payment hash of the payment * `status` - PaymentSessionStatus, The status of the payment -* `created_at` - u128, The time the payment was created at -* `last_updated_at` - u128, The time the payment was last updated at +* `created_at` - u64, The time the payment was created at, in milliseconds from UNIX epoch +* `last_updated_at` - u64, The time the payment was last updated at, in milliseconds from UNIX epoch * `failed_error` - `Option`, The error message if the payment failed @@ -532,7 +532,7 @@ The channel data structure * `offered_tlc_balance` - u128, The offered balance of the channel * `remote_balance` - u128, The remote balance of the channel * `received_tlc_balance` - u128, The received balance of the channel -* `created_at` - u64, The time the channel was created at +* `created_at` - u64, The time the channel was created at, in milliseconds from UNIX epoch ### Type `RemoveTlcReason` diff --git a/src/rpc/channel.rs b/src/rpc/channel.rs index 92a416326..27b5f753f 100644 --- a/src/rpc/channel.rs +++ b/src/rpc/channel.rs @@ -159,7 +159,7 @@ pub(crate) struct Channel { /// The received balance of the channel #[serde_as(as = "U128Hex")] received_tlc_balance: u128, - /// The time the channel was created at + /// The time the channel was created at, in milliseconds from UNIX epoch #[serde_as(as = "U64Hex")] created_at: u64, } @@ -262,12 +262,12 @@ pub struct GetPaymentCommandResult { pub payment_hash: Hash256, /// The status of the payment pub status: PaymentSessionStatus, - #[serde_as(as = "U128Hex")] - /// The time the payment was created at - created_at: u128, - #[serde_as(as = "U128Hex")] - /// The time the payment was last updated at - pub last_updated_at: u128, + #[serde_as(as = "U64Hex")] + /// The time the payment was created at, in milliseconds from UNIX epoch + created_at: u64, + #[serde_as(as = "U64Hex")] + /// The time the payment was last updated at, in milliseconds from UNIX epoch + pub last_updated_at: u64, /// The error message if the payment failed pub failed_error: Option, } @@ -474,7 +474,7 @@ where remote_balance: state.get_remote_balance(), offered_tlc_balance: state.get_offered_tlc_balance(), received_tlc_balance: state.get_received_tlc_balance(), - created_at: state.get_created_at_in_microseconds(), + created_at: state.get_created_at_in_millis(), }) }) .collect(); diff --git a/tests/bruno/e2e/router-pay/12-node1-to-get-payment-status.bru b/tests/bruno/e2e/router-pay/12-node1-to-get-payment-status.bru new file mode 100644 index 000000000..984ae0f65 --- /dev/null +++ b/tests/bruno/e2e/router-pay/12-node1-to-get-payment-status.bru @@ -0,0 +1,48 @@ +meta { + name: Node1 send get_payment for query status + type: http + seq: 12 +} + +post { + url: {{NODE1_RPC_URL}} + body: json + auth: none +} + +headers { + Content-Type: application/json + Accept: application/json +} + +body:json { + { + "id": "42", + "jsonrpc": "2.0", + "method": "get_payment", + "params": [ + { + "payment_hash": "{{payment_hash}}" + } + ] + } +} + +assert { + res.body.error: isUndefined +} + +script:pre-request { + // sleep for a while + await new Promise(r => setTimeout(r, 2000)); +} + +script:post-response { + // Sleep for sometime to make sure current operation finishes before next request starts. + await new Promise(r => setTimeout(r, 500)); + console.log("get result: ", res.body.result); + if (res.body.result.status != "Success") { + throw new Error("Assertion failed: payment session status expected to be Success"); + } + +} \ No newline at end of file diff --git a/tests/bruno/e2e/router-pay/32-node2-list-channels.bru b/tests/bruno/e2e/router-pay/32-node2-list-channels.bru new file mode 100644 index 000000000..a7a212633 --- /dev/null +++ b/tests/bruno/e2e/router-pay/32-node2-list-channels.bru @@ -0,0 +1,40 @@ +meta { + name: get channels from node2 + type: http + seq: 32 +} + +post { + url: {{NODE2_RPC_URL}} + body: json + auth: none +} + +headers { + Content-Type: application/json + Accept: application/json +} + +body:json { + { + "id": 42, + "jsonrpc": "2.0", + "method": "list_channels", + "params": [ + { + "peer_id": "{{NODE1_PEERID}}" + } + ] + } +} + + +assert { + res.status: eq 200 +} + +script:post-response { + await new Promise(r => setTimeout(r, 1000)); + console.log("step 32 list graph channels: ", res.body.result.channels); + bru.setVar("CHANNEL_ID_TO_UPDATE", res.body.result.channels[0].channel_id); +} diff --git a/tests/bruno/e2e/router-pay/33-node2-update-channel.bru b/tests/bruno/e2e/router-pay/33-node2-update-channel.bru new file mode 100644 index 000000000..1297f7e11 --- /dev/null +++ b/tests/bruno/e2e/router-pay/33-node2-update-channel.bru @@ -0,0 +1,43 @@ +meta { + name: get channels from node2 + type: http + seq: 32 +} + +post { + url: {{NODE2_RPC_URL}} + body: json + auth: none +} + +headers { + Content-Type: application/json + Accept: application/json +} + +body:json { + { + "id": 42, + "jsonrpc": "2.0", + "method": "update_channel", + "params": [ + { + "channel_id": "{{CHANNEL_ID_TO_UPDATE}}", + "tlc_fee_proportional_millionths": "0x2710" + } + ] + } +} + +script:pre-request { + await new Promise(r => setTimeout(r, 3000)); +} + +assert { + res.status: eq 200 +} + +script:post-response { + await new Promise(r => setTimeout(r, 2000)); + console.log("step 33 list channels: ", res.body.result); +} diff --git a/tests/bruno/e2e/router-pay/34-node1-send-payment.bru b/tests/bruno/e2e/router-pay/34-node1-send-payment.bru new file mode 100644 index 000000000..727f63705 --- /dev/null +++ b/tests/bruno/e2e/router-pay/34-node1-send-payment.bru @@ -0,0 +1,42 @@ +meta { + name: Node1 send payment with router + type: http + seq: 34 +} + +post { + url: {{NODE1_RPC_URL}} + body: json + auth: none +} + +headers { + Content-Type: application/json + Accept: application/json +} + +body:json { + { + "id": "42", + "jsonrpc": "2.0", + "method": "send_payment", + "params": [ + { + "target_pubkey": "03032b99943822e721a651c5a5b9621043017daa9dc3ec81d83215fd2e25121187", + "amount": "0x190", + "keysend": true + } + ] + } +} + +assert { + res.body.error: isUndefined +} + +script:post-response { + // Sleep for sometime to make sure current operation finishes before next request starts. + await new Promise(r => setTimeout(r, 2000)); + bru.setVar("payment_hash", res.body.result.payment_hash); + console.log("33 step result: ", res.body); +} diff --git a/tests/bruno/e2e/router-pay/35-node1-get-payment-status.bru b/tests/bruno/e2e/router-pay/35-node1-get-payment-status.bru new file mode 100644 index 000000000..51756953e --- /dev/null +++ b/tests/bruno/e2e/router-pay/35-node1-get-payment-status.bru @@ -0,0 +1,48 @@ +meta { + name: Node1 send get_payment for query status + type: http + seq: 35 +} + +post { + url: {{NODE1_RPC_URL}} + body: json + auth: none +} + +headers { + Content-Type: application/json + Accept: application/json +} + +body:json { + { + "id": "42", + "jsonrpc": "2.0", + "method": "get_payment", + "params": [ + { + "payment_hash": "{{payment_hash}}" + } + ] + } +} + +assert { + res.body.error: isUndefined +} + +script:pre-request { + // sleep for a while + await new Promise(r => setTimeout(r, 2000)); +} + +script:post-response { + // Sleep for sometime to make sure current operation finishes before next request starts. + await new Promise(r => setTimeout(r, 500)); + console.log("get result: ", res.body.result); + if (res.body.result.status != "Success") { + throw new Error("Assertion failed: payment session status expected to be Success"); + } + +} \ No newline at end of file From b5e9db3469e9a9932f8ba05c31db6c93511c5996 Mon Sep 17 00:00:00 2001 From: yukang Date: Wed, 20 Nov 2024 01:07:06 +0800 Subject: [PATCH 2/4] clean up logs --- src/fiber/channel.rs | 5 ----- src/fiber/graph.rs | 24 ------------------------ 2 files changed, 29 deletions(-) diff --git a/src/fiber/channel.rs b/src/fiber/channel.rs index 127c99f23..c589a766b 100644 --- a/src/fiber/channel.rs +++ b/src/fiber/channel.rs @@ -881,12 +881,7 @@ where .expect("public channel exits") .tlc_fee_proportional_millionths .unwrap_or_default(); - info!("expecting fee_rate: {}", fee_rate); let expected_fee = calculate_tlc_forward_fee(forward_amount, fee_rate); - info!( - "forward_fee: {} expected_fee: {}", - forward_fee, expected_fee - ); if forward_fee < expected_fee { error!( "too low forward_fee: {}, expected_fee: {}", diff --git a/src/fiber/graph.rs b/src/fiber/graph.rs index 10d252bb8..d3e20b107 100644 --- a/src/fiber/graph.rs +++ b/src/fiber/graph.rs @@ -404,10 +404,8 @@ where &channel, &update ); let update_info = if update.message_flags & 1 == 1 { - debug!("now update node1_to_node2: {:?}", &update); &mut channel.node1_to_node2 } else { - debug!("now update node2_to_node1: {:?}", &update); &mut channel.node2_to_node1 }; @@ -440,7 +438,6 @@ where last_update_message: update.clone(), }); - info!("now new update_info: {:?}", *update_info); self.store.insert_channel(channel.to_owned()); debug!( "Processed channel update: channel {:?}, update {:?}", @@ -602,10 +599,6 @@ where let fee_rate = channel_update.fee_rate; let fee = calculate_tlc_forward_fee(current_amount, fee_rate as u128); let expiry = channel_update.htlc_expiry_delta; - eprintln!( - "fee: {:?}, expiry: {:?}, current_amount: {:?}, fee_rate: {:?}", - fee, expiry, current_amount, fee_rate - ); (fee, expiry) }; @@ -729,7 +722,6 @@ where .values() .any(|x| x == &channel_info.out_point()) { - eprintln!("here ......"); continue; } @@ -740,15 +732,9 @@ where let fee = calculate_tlc_forward_fee(next_hop_received_amount, fee_rate as u128); let amount_to_send = next_hop_received_amount + fee; - eprintln!("amount_to_send: {:?}", amount_to_send); // if the amount to send is greater than the amount we have, skip this edge if let Some(max_fee_amount) = max_fee_amount { if amount_to_send > amount + max_fee_amount { - eprintln!( - "amount_to_send {} > amount + max_fee_amount {}", - amount_to_send, - amount + max_fee_amount - ); continue; } } @@ -758,13 +744,6 @@ where || (channel_update.htlc_maximum_value != 0 && amount_to_send > channel_update.htlc_maximum_value) { - eprintln!( - "now .......amount: {:?} capacity: {:?}, channel_update.htlc_maximum_value: {:?}", - amount_to_send, - channel_info.capacity(), - channel_update.htlc_maximum_value - - ); continue; } if amount_to_send < channel_update.htlc_minimum_value { @@ -786,7 +765,6 @@ where ); if probability < DEFAULT_MIN_PROBABILITY { - eprintln!("probability < DEFAULT_MIN_PROBABILITY"); continue; } let agg_weight = @@ -799,8 +777,6 @@ where continue; } } - eprintln!("\n\nfind_path from: {:?}, to: {:?}", from, to); - eprintln!("add use channel_info: {:?}\n\n", channel_info); let node = NodeHeapElement { node_id: from, weight, From f3dbb0cf725bdf515119a4cc7979e01d460ed543 Mon Sep 17 00:00:00 2001 From: yukang Date: Wed, 20 Nov 2024 10:42:44 +0800 Subject: [PATCH 3/4] add more tests for fee rate --- src/fiber/graph.rs | 5 ++ src/fiber/tests/graph.rs | 167 +++++++++++++++++++++++++++++++-------- 2 files changed, 140 insertions(+), 32 deletions(-) diff --git a/src/fiber/graph.rs b/src/fiber/graph.rs index d3e20b107..ccc899703 100644 --- a/src/fiber/graph.rs +++ b/src/fiber/graph.rs @@ -532,6 +532,11 @@ where self.history.reset(); } + #[cfg(test)] + pub fn set_source(&mut self, source: Pubkey) { + self.source = source; + } + /// Returns a list of `PaymentHopData` for all nodes in the route, /// including the origin and the target node. pub fn build_route( diff --git a/src/fiber/tests/graph.rs b/src/fiber/tests/graph.rs index 34c3ab7b0..6cd3af894 100644 --- a/src/fiber/tests/graph.rs +++ b/src/fiber/tests/graph.rs @@ -70,6 +70,10 @@ impl MockNetworkGraph { } } + fn set_source(&mut self, source: PublicKey) { + self.graph.set_source(source.into()); + } + pub fn mark_node_failed(&mut self, node: usize) { self.graph.mark_node_failed(self.keys[node].into()); } @@ -139,8 +143,6 @@ impl MockNetworkGraph { tlc_minimum_value: min_htlc_value.unwrap_or(0), channel_outpoint: channel_outpoint.clone(), }; - eprintln!("add channel_info: {:?}", channel_info); - eprintln!("add channel_update: {:?}", channel_update); self.graph.process_channel_update(channel_update).unwrap(); if let Some(fee_rate) = other_fee_rate { let channel_update = ChannelUpdate { @@ -157,7 +159,6 @@ impl MockNetworkGraph { }; eprintln!("add rev channel_update: {:?}", channel_update); self.graph.process_channel_update(channel_update).unwrap(); - //eprintln!("add channel_info: {:?}", channel_info); } } @@ -200,7 +201,7 @@ impl MockNetworkGraph { ); } - pub fn find_route( + pub fn find_path( &self, source: usize, target: usize, @@ -213,7 +214,7 @@ impl MockNetworkGraph { .find_path(source, target, amount, Some(max_fee), None, false) } - pub fn find_route_udt( + pub fn find_path_udt( &self, source: usize, target: usize, @@ -297,18 +298,18 @@ fn test_graph_find_path_basic() { network.add_edge(1, 2, Some(1), Some(2)); let node2 = network.keys[2]; - let route = network.find_route(1, 2, 100, 1000); + let route = network.find_path(1, 2, 100, 1000); assert!(route.is_err()); network.add_edge(1, 2, Some(120), Some(2)); - let route = network.find_route(1, 2, 100, 1000); + let route = network.find_path(1, 2, 100, 1000); assert!(route.is_ok()); let route = route.unwrap(); assert_eq!(route.len(), 1); assert_eq!(route[0].target, node2.into()); assert_eq!(route[0].channel_outpoint, network.edges[1].2); - let route = network.find_route(1, 3, 10, 100); + let route = network.find_path(1, 3, 10, 100); assert!(route.is_err()); } @@ -321,7 +322,7 @@ fn test_graph_find_path_three_nodes() { let node3 = network.keys[3]; // Test route from node 1 to node 3 - let route = network.find_route(1, 3, 100, 1000); + let route = network.find_path(1, 3, 100, 1000); assert!(route.is_ok()); let route = route.unwrap(); assert_eq!(route.len(), 2); @@ -331,7 +332,7 @@ fn test_graph_find_path_three_nodes() { assert_eq!(route[1].channel_outpoint, network.edges[1].2); // Test route from node 1 to node 2 - let route = network.find_route(1, 2, 100, 1000); + let route = network.find_path(1, 2, 100, 1000); assert!(route.is_ok()); let route = route.unwrap(); assert_eq!(route.len(), 1); @@ -339,7 +340,7 @@ fn test_graph_find_path_three_nodes() { assert_eq!(route[0].channel_outpoint, network.edges[0].2); // Test route from node 2 to node 3 - let route = network.find_route(2, 3, 100, 1000); + let route = network.find_path(2, 3, 100, 1000); assert!(route.is_ok()); let route = route.unwrap(); assert_eq!(route.len(), 1); @@ -347,7 +348,7 @@ fn test_graph_find_path_three_nodes() { assert_eq!(route[0].channel_outpoint, network.edges[1].2); // Test route from node 3 to node 1 (should fail) - let route = network.find_route(3, 1, 100, 1000); + let route = network.find_path(3, 1, 100, 1000); assert!(route.is_err()); } @@ -361,7 +362,7 @@ fn test_graph_find_path_fee() { network.add_edge(1, 3, Some(1000), Some(20000)); network.add_edge(3, 4, Some(1000), Some(10000)); - let route = network.find_route(1, 4, 100, 1000); + let route = network.find_path(1, 4, 100, 1000); assert!(route.is_ok()); let route = route.unwrap(); @@ -381,7 +382,7 @@ fn test_graph_find_path_direct_linear() { network.add_edge(3, 4, Some(1000), Some(2)); network.add_edge(4, 5, Some(1000), Some(1)); - let route = network.find_route(1, 5, 100, 1000); + let route = network.find_path(1, 5, 100, 1000); assert!(route.is_ok()); let route = route.unwrap(); @@ -401,14 +402,14 @@ fn test_graph_find_path_cycle() { network.add_edge(2, 3, Some(1000), Some(3)); network.add_edge(3, 1, Some(1000), Some(2)); - let route = network.find_route(1, 3, 100, 1000); + let route = network.find_path(1, 3, 100, 1000); assert!(route.is_ok()); network.add_edge(3, 4, Some(1000), Some(2)); network.add_edge(4, 5, Some(1000), Some(1)); - let route = network.find_route(1, 5, 100, 1000); + let route = network.find_path(1, 5, 100, 1000); assert!(route.is_ok()); } @@ -424,7 +425,7 @@ fn test_graph_find_path_cycle_in_middle() { network.add_edge(4, 5, Some(1000), Some(1)); - let route = network.find_route(1, 5, 100, 1000); + let route = network.find_path(1, 5, 100, 1000); assert!(route.is_ok()); } @@ -436,12 +437,12 @@ fn test_graph_find_path_loop_exit() { network.add_edge(2, 3, Some(1000), Some(3)); network.add_edge(3, 2, Some(1000), Some(2)); - let route = network.find_route(1, 3, 100, 1000); + let route = network.find_path(1, 3, 100, 1000); assert!(route.is_err()); // now add a path from node1 to node2, so that node1 can reach node3 network.add_edge(1, 2, Some(1000), Some(4)); - let route = network.find_route(1, 3, 100, 1000); + let route = network.find_path(1, 3, 100, 1000); assert!(route.is_ok()); } @@ -454,7 +455,7 @@ fn test_graph_find_path_amount_failed() { network.add_edge(3, 4, Some(1000), Some(4)); network.add_edge(4, 5, Some(1000), Some(1)); - let route = network.find_route(1, 5, 1000, 10); + let route = network.find_path(1, 5, 1000, 10); assert!(route.is_err()); } @@ -475,19 +476,15 @@ fn test_graph_find_optimal_path() { network.add_edge(1, 6, Some(500), Some(10000)); network.add_edge(6, 5, Some(500), Some(10000)); - let route = network.find_route(1, 5, 1000, 1000); - assert!(route.is_ok()); - let route = route.unwrap(); - // Check that the algorithm chose the longer path with lower fees + let route = network.find_path(1, 5, 1000, 1000).unwrap(); assert_eq!(route.len(), 4); - assert_eq!(route[0].channel_outpoint, network.edges[1].2); - assert_eq!(route[1].channel_outpoint, network.edges[2].2); - assert_eq!(route[2].channel_outpoint, network.edges[3].2); - assert_eq!(route[3].channel_outpoint, network.edges[4].2); + for (i, edge_index) in (1..=4).enumerate() { + assert_eq!(route[i].channel_outpoint, network.edges[edge_index].2); + } // Test with a smaller amount that allows using the direct path - let small_route = network.find_route(1, 5, 100, 100); + let small_route = network.find_path(1, 5, 100, 100); assert!(small_route.is_ok()); let small_route = small_route.unwrap(); @@ -497,13 +494,119 @@ fn test_graph_find_optimal_path() { assert_eq!(small_route[1].channel_outpoint, network.edges[6].2); } +#[test] +fn test_graph_build_router_is_ok_with_fee_rate() { + let mut network = MockNetworkGraph::new(6); + + // Direct path with high fee + network.add_edge(1, 5, Some(2000), Some(50000)); + + // Longer path with lower total fee + network.add_edge(1, 2, Some(2000), Some(10000)); + // this node has a very low fee rate + network.add_edge(2, 3, Some(2000), Some(1)); + network.add_edge(3, 4, Some(2000), Some(10000)); + network.add_edge(4, 5, Some(2000), Some(10000)); + + // check the fee rate + let source = network.keys[1]; + network.set_source(source); + let node5 = network.keys[5]; + let route = network.graph.build_route(SendPaymentData { + target_pubkey: node5.into(), + amount: 1000, + payment_hash: Hash256::default(), + invoice: None, + final_htlc_expiry_delta: None, + timeout: None, + max_fee_amount: Some(1000), + max_parts: None, + keysend: false, + udt_type_script: None, + preimage: None, + allow_self_payment: false, + }); + assert!(route.is_ok()); + let route = route.unwrap(); + let amounts = route.iter().map(|x| x.amount).collect::>(); + assert_eq!(amounts, vec![1022, 1011, 1010, 1000, 1000]); +} + +#[test] +fn test_graph_build_router_fee_rate_optimize() { + let mut network = MockNetworkGraph::new(10); + + // Direct path with low total fee rate + network.add_edge(1, 6, Some(2000), Some(50000)); + network.add_edge(6, 5, Some(2000), Some(50000)); + + // Longer path with lower total fee + network.add_edge(1, 2, Some(2000), Some(10000)); + network.add_edge(2, 3, Some(2000), Some(20000)); + network.add_edge(3, 4, Some(2000), Some(30000)); + network.add_edge(4, 5, Some(2000), Some(40000)); + + // check the fee rate + let source = network.keys[1]; + network.set_source(source); + let node5 = network.keys[5]; + let route = network.graph.build_route(SendPaymentData { + target_pubkey: node5.into(), + amount: 1000, + payment_hash: Hash256::default(), + invoice: None, + final_htlc_expiry_delta: None, + timeout: None, + max_fee_amount: Some(1000), + max_parts: None, + keysend: false, + udt_type_script: None, + preimage: None, + allow_self_payment: false, + }); + assert!(route.is_ok()); + let route = route.unwrap(); + let amounts = route.iter().map(|x| x.amount).collect::>(); + assert_eq!(amounts, vec![1050, 1000, 1000]); +} + +#[test] +fn test_graph_build_router_no_fee_with_direct_pay() { + let mut network = MockNetworkGraph::new(10); + + network.add_edge(1, 5, Some(2000), Some(50000)); + + // check the fee rate + let source = network.keys[1]; + network.set_source(source); + let node5 = network.keys[5]; + let route = network.graph.build_route(SendPaymentData { + target_pubkey: node5.into(), + amount: 1000, + payment_hash: Hash256::default(), + invoice: None, + final_htlc_expiry_delta: None, + timeout: None, + max_fee_amount: Some(1000), + max_parts: None, + keysend: false, + udt_type_script: None, + preimage: None, + allow_self_payment: false, + }); + assert!(route.is_ok()); + let route = route.unwrap(); + let amounts = route.iter().map(|x| x.amount).collect::>(); + assert_eq!(amounts, vec![1000, 1000]); +} + #[test] fn test_graph_find_path_err() { let mut network = MockNetworkGraph::new(6); let (node1, _node5) = (network.keys[1], network.keys[5]); network.add_edge(1, 2, Some(1000), Some(4)); - let route = network.find_route(1, 1, 100, 1000); + let route = network.find_path(1, 1, 100, 1000); assert!(route.is_err()); let no_exits_public_key = network.keys[0]; @@ -624,7 +727,7 @@ fn test_graph_find_path_udt() { network.add_edge_udt(1, 2, Some(1000), Some(1), udt_type_script.clone()); let node2 = network.keys[2]; - let route = network.find_route_udt(1, 2, 100, 1000, udt_type_script.clone()); + let route = network.find_path_udt(1, 2, 100, 1000, udt_type_script.clone()); assert!(route.is_ok()); let route = route.unwrap(); @@ -632,7 +735,7 @@ fn test_graph_find_path_udt() { assert_eq!(route[0].target, node2.into()); assert_eq!(route[0].channel_outpoint, network.edges[0].2); - let route = network.find_route(1, 3, 10, 100); + let route = network.find_path(1, 3, 10, 100); assert!(route.is_err()); } From 385fa5a2cc9d1c49eb2e3e60825b716c60f6dee4 Mon Sep 17 00:00:00 2001 From: yukang Date: Wed, 20 Nov 2024 10:56:11 +0800 Subject: [PATCH 4/4] set --no-fail-fast for cargo nextest --- .github/workflows/ci.yml | 2 +- Makefile | 2 +- src/fiber/graph.rs | 2 -- 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 82a08432b..29a29942d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -36,7 +36,7 @@ jobs: with: tool: nextest - run: | - RUST_BACKTRACE=full RUST_LOG=trace cargo nextest run + RUST_BACKTRACE=full RUST_LOG=trace cargo nextest run --no-fail-fast fmt: name: Rustfmt diff --git a/Makefile b/Makefile index 5365b7b20..d14e3a6b1 100644 --- a/Makefile +++ b/Makefile @@ -7,7 +7,7 @@ GRCOV_EXCL_LINE = ^\s*(\})*(\))*(;)*$$|\s*((log::|tracing::)?(trace|debug|info|w .PHONY: test test: - RUST_LOG=off cargo nextest run + RUST_LOG=off cargo nextest run --no-fail-fast .PHONY: clippy clippy: diff --git a/src/fiber/graph.rs b/src/fiber/graph.rs index ccc899703..aa5aba6a8 100644 --- a/src/fiber/graph.rs +++ b/src/fiber/graph.rs @@ -460,14 +460,12 @@ where self.channels.values().filter_map(move |channel| { if let Some(info) = channel.node1_to_node2.as_ref() { if info.enabled && channel.node2() == node_id { - debug!("now use node1_to_node2: {:?}", info); return Some((channel.node1(), channel.node2(), channel, info)); } } if let Some(info) = channel.node2_to_node1.as_ref() { if info.enabled && channel.node1() == node_id { - debug!("now use node2_to_node1: {:?}", info); return Some((channel.node2(), channel.node1(), channel, info)); } }