Skip to content

Commit

Permalink
Merge pull request #411 from chenyukang/fix-direct-channel-bug
Browse files Browse the repository at this point in the history 
fix a bug in direct channel
  • Loading branch information
@quake
quake authored Dec 20, 2024
2 parents c963248 + e8e2814 commit d7b8494
Show file tree
Hide file tree
Showing 4 changed files with 172 additions and 43 deletions.
9 changes: 7 additions & 2 deletions src/fiber/graph.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -900,12 +900,17 @@ where
}

// if this is a direct channel, try to load the channel actor state for balance
if from == self.source {
if from == self.source || to == self.source {
if let Some(state) = self
.store
.get_channel_state_by_outpoint(&channel_info.out_point())
{
if amount_to_send > state.to_local_amount {
let balance = if from == self.source {
state.to_local_amount
} else {
state.to_remote_amount
};
if amount_to_send > balance {
continue;
}
}
Expand Down
74 changes: 35 additions & 39 deletions src/fiber/tests/channel.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@ use crate::fiber::channel::{
use crate::fiber::config::MAX_PAYMENT_TLC_EXPIRY_LIMIT;
use crate::fiber::graph::PaymentSessionStatus;
use crate::fiber::network::{DebugEvent, SendPaymentCommand};
use crate::fiber::tests::test_utils::create_n_nodes_with_index_and_amounts_with_established_channel;
use crate::fiber::tests::test_utils::{
create_3_nodes_with_established_channel, create_n_nodes_with_established_channel,
create_nodes_with_established_channel, NetworkNodeConfigBuilder,
};
use crate::fiber::tests::test_utils::*;
use crate::fiber::types::{
Hash256, PaymentHopData, PeeledOnionPacket, TlcErrorCode, NO_SHARED_SECRET,
};
Expand Down Expand Up @@ -1134,8 +1130,8 @@ async fn test_network_send_payment_amount_is_too_large() {
init_tracing();

let _span = tracing::info_span!("node", node = "test").entered();
let node_a_funding_amount = 100000000000 + 4200000000;
let node_b_funding_amount = 4200000000 + 2;
let node_a_funding_amount = 100000000000 + MIN_RESERVED_CKB;
let node_b_funding_amount = MIN_RESERVED_CKB + 2;

let (node_a, node_b, _new_channel_id) =
create_nodes_with_established_channel(node_a_funding_amount, node_b_funding_amount, true)
Expand Down Expand Up @@ -4189,8 +4185,8 @@ async fn test_send_payment_with_multiple_edges_in_middle_hops() {
let (nodes, _channels) = create_n_nodes_with_index_and_amounts_with_established_channel(
&[
((0, 1), (100000000000, 100000000000)),
((1, 2), (4200000000 + 900, 5200000000)),
((1, 2), (4200000000 + 1000, 5200000000)),
((1, 2), (MIN_RESERVED_CKB + 900, 5200000000)),
((1, 2), (MIN_RESERVED_CKB + 1000, 5200000000)),
((2, 3), (100000000000, 100000000000)),
],
4,
Expand Down Expand Up @@ -4256,8 +4252,8 @@ async fn test_send_payment_with_all_failed_middle_hops() {
let (nodes, _channels) = create_n_nodes_with_index_and_amounts_with_established_channel(
&[
((0, 1), (100000000000, 100000000000)),
((1, 2), (4200000000 + 900, 4200000000 + 1000)),
((1, 2), (4200000000 + 910, 4200000000 + 1000)),
((1, 2), (MIN_RESERVED_CKB + 900, MIN_RESERVED_CKB + 1000)),
((1, 2), (MIN_RESERVED_CKB + 910, MIN_RESERVED_CKB + 1000)),
((2, 3), (100000000000, 100000000000)),
],
4,
Expand Down Expand Up @@ -4323,8 +4319,8 @@ async fn test_send_payment_with_multiple_edges_can_succeed_in_retry() {
let (nodes, _channels) = create_n_nodes_with_index_and_amounts_with_established_channel(
&[
((0, 1), (100000000000, 100000000000)),
((1, 2), (4200000000 + 1000, 5200000000)),
((1, 2), (4200000000 + 900, 6200000000)),
((1, 2), (MIN_RESERVED_CKB + 1000, 5200000000)),
((1, 2), (MIN_RESERVED_CKB + 900, 6200000000)),
((2, 3), (100000000000, 100000000000)),
],
4,
Expand Down Expand Up @@ -4391,8 +4387,8 @@ async fn test_send_payment_with_final_hop_multiple_edges_in_middle_hops() {
&[
((0, 1), (100000000000, 100000000000)),
((1, 2), (100000000000, 100000000000)),
((2, 3), (4200000000 + 900, 5200000000)),
((2, 3), (4200000000 + 1000, 5200000000)),
((2, 3), (MIN_RESERVED_CKB + 900, 5200000000)),
((2, 3), (MIN_RESERVED_CKB + 1000, 5200000000)),
],
4,
true,
Expand Down Expand Up @@ -4458,8 +4454,8 @@ async fn test_send_payment_with_final_all_failed_middle_hops() {
&[
((0, 1), (100000000000, 100000000000)),
((1, 2), (100000000000, 100000000000)),
((2, 3), (4200000000 + 900, 4200000000 + 1000)),
((2, 3), (4200000000 + 910, 4200000000 + 1000)),
((2, 3), (MIN_RESERVED_CKB + 900, MIN_RESERVED_CKB + 1000)),
((2, 3), (MIN_RESERVED_CKB + 910, MIN_RESERVED_CKB + 1000)),
],
4,
true,
Expand Down Expand Up @@ -4516,8 +4512,8 @@ async fn test_send_payment_with_final_multiple_edges_can_succeed_in_retry() {
&[
((0, 1), (100000000000, 100000000000)),
((1, 2), (100000000000, 100000000000)),
((2, 3), (4200000000 + 1000, 5200000000)),
((2, 3), (4200000000 + 900, 6200000000)),
((2, 3), (MIN_RESERVED_CKB + 1000, 5200000000)),
((2, 3), (MIN_RESERVED_CKB + 900, 6200000000)),
],
4,
true,
Expand Down Expand Up @@ -4571,7 +4567,7 @@ async fn test_send_payment_with_first_hop_failed_with_fee() {
// even 1000 > 999, but it's not enough for fee, and this is the direct channel
// so we can check the actual balance of channel
// the payment will fail
((0, 1), (4200000000 + 1000, 5200000000)),
((0, 1), (MIN_RESERVED_CKB + 1000, 5200000000)),
((1, 2), (100000000000, 100000000000)),
((2, 3), (100000000000, 100000000000)),
],
Expand Down Expand Up @@ -4622,8 +4618,8 @@ async fn test_send_payment_succeed_with_multiple_edges_in_first_hop() {
// the send payment should be succeed
let (nodes, _channels) = create_n_nodes_with_index_and_amounts_with_established_channel(
&[
((0, 1), (4200000000 + 900, 5200000000)),
((0, 1), (4200000000 + 1001, 5200000000)),
((0, 1), (MIN_RESERVED_CKB + 900, 5200000000)),
((0, 1), (MIN_RESERVED_CKB + 1001, 5200000000)),
((1, 2), (100000000000, 100000000000)),
((2, 3), (100000000000, 100000000000)),
],
Expand Down Expand Up @@ -4679,8 +4675,8 @@ async fn test_send_payment_with_first_hop_all_failed() {
// path finding will fail in the first time of send payment
let (nodes, _channels) = create_n_nodes_with_index_and_amounts_with_established_channel(
&[
((0, 1), (4200000000 + 900, 4200000000 + 1000)),
((0, 1), (4200000000 + 910, 4200000000 + 1000)),
((0, 1), (MIN_RESERVED_CKB + 900, MIN_RESERVED_CKB + 1000)),
((0, 1), (MIN_RESERVED_CKB + 910, MIN_RESERVED_CKB + 1000)),
((1, 2), (100000000000, 100000000000)),
((2, 3), (100000000000, 100000000000)),
],
Expand Down Expand Up @@ -4733,8 +4729,8 @@ async fn test_send_payment_will_succeed_with_direct_channel_info_first_hop() {
// so it will try the channel with smaller capacity and the payment will succeed
let (nodes, channels) = create_n_nodes_with_index_and_amounts_with_established_channel(
&[
((0, 1), (4200000000 + 2000, 4200000000 + 1000)),
((0, 1), (4200000000 + 1005, 4200000000 + 1000)),
((0, 1), (MIN_RESERVED_CKB + 2000, MIN_RESERVED_CKB + 1000)),
((0, 1), (MIN_RESERVED_CKB + 1005, MIN_RESERVED_CKB + 1000)),
((1, 2), (100000000000, 100000000000)),
((2, 3), (100000000000, 100000000000)),
],
Expand Down Expand Up @@ -4801,8 +4797,8 @@ async fn test_send_payment_will_succeed_with_retry_in_middle_hops() {
&[
((0, 1), (100000000000, 100000000000)),
((1, 2), (100000000000, 100000000000)),
((2, 3), (4200000000 + 2000, 4200000000 + 1000)),
((2, 3), (4200000000 + 1005, 4200000000 + 1000)),
((2, 3), (MIN_RESERVED_CKB + 2000, MIN_RESERVED_CKB + 1000)),
((2, 3), (MIN_RESERVED_CKB + 1005, MIN_RESERVED_CKB + 1000)),
],
4,
true,
Expand Down Expand Up @@ -4865,8 +4861,8 @@ async fn test_send_payment_will_fail_with_last_hop_info_in_add_tlc_peer() {
&[
((0, 1), (100000000000, 100000000000)),
((1, 2), (100000000000, 100000000000)),
((2, 3), (4200000000 + 2000, 4200000000 + 1000)),
((2, 3), (4200000000 + 1005, 4200000000 + 1000)),
((2, 3), (MIN_RESERVED_CKB + 2000, MIN_RESERVED_CKB + 1000)),
((2, 3), (MIN_RESERVED_CKB + 1005, MIN_RESERVED_CKB + 1000)),
],
4,
true,
Expand Down Expand Up @@ -4933,8 +4929,8 @@ async fn test_send_payment_will_fail_with_invoice_not_generated_by_target() {
&[
((0, 1), (100000000000, 100000000000)),
((1, 2), (100000000000, 100000000000)),
((2, 3), (4200000000 + 2000, 4200000000 + 1000)),
((2, 3), (4200000000 + 1005, 4200000000 + 1000)),
((2, 3), (MIN_RESERVED_CKB + 2000, MIN_RESERVED_CKB + 1000)),
((2, 3), (MIN_RESERVED_CKB + 1005, MIN_RESERVED_CKB + 1000)),
],
4,
true,
Expand Down Expand Up @@ -4994,8 +4990,8 @@ async fn test_send_payment_will_succeed_with_valid_invoice() {
&[
((0, 1), (100000000000, 100000000000)),
((1, 2), (100000000000, 100000000000)),
((2, 3), (4200000000 + 2000, 4200000000 + 1000)),
((2, 3), (4200000000 + 1005, 4200000000 + 1000)),
((2, 3), (MIN_RESERVED_CKB + 2000, MIN_RESERVED_CKB + 1000)),
((2, 3), (MIN_RESERVED_CKB + 1005, MIN_RESERVED_CKB + 1000)),
],
4,
true,
Expand Down Expand Up @@ -5064,8 +5060,8 @@ async fn test_send_payment_will_fail_with_no_invoice_preimage() {
&[
((0, 1), (100000000000, 100000000000)),
((1, 2), (100000000000, 100000000000)),
((2, 3), (4200000000 + 2000, 4200000000 + 1000)),
((2, 3), (4200000000 + 1005, 4200000000 + 1000)),
((2, 3), (MIN_RESERVED_CKB + 2000, MIN_RESERVED_CKB + 1000)),
((2, 3), (MIN_RESERVED_CKB + 1005, MIN_RESERVED_CKB + 1000)),
],
4,
true,
Expand Down Expand Up @@ -5136,8 +5132,8 @@ async fn test_send_payment_will_fail_with_cancelled_invoice() {
&[
((0, 1), (100000000000, 100000000000)),
((1, 2), (100000000000, 100000000000)),
((2, 3), (4200000000 + 2000, 4200000000 + 1000)),
((2, 3), (4200000000 + 1005, 4200000000 + 1000)),
((2, 3), (MIN_RESERVED_CKB + 2000, MIN_RESERVED_CKB + 1000)),
((2, 3), (MIN_RESERVED_CKB + 1005, MIN_RESERVED_CKB + 1000)),
],
4,
true,
Expand Down Expand Up @@ -5207,7 +5203,7 @@ async fn test_send_payment_will_succeed_with_large_tlc_expiry_limit() {

let (nodes, _channels) = create_n_nodes_with_index_and_amounts_with_established_channel(
&[
((0, 1), (4200000000 + 2000, 4200000000 + 1000)),
((0, 1), (MIN_RESERVED_CKB + 2000, MIN_RESERVED_CKB + 1000)),
((1, 2), (100000000000, 100000000000)),
((2, 3), (100000000000, 100000000000)),
],
Expand Down
131 changes: 129 additions & 2 deletions src/fiber/tests/payment.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ async fn test_send_payment_for_direct_channel_and_dry_run() {

let (nodes, channels) = create_n_nodes_with_index_and_amounts_with_established_channel(
&[
((0, 1), (4200000000 + 10000000000, 4200000000)),
((0, 1), (4200000000, 4200000000 + 10000000000)),
((0, 1), (MIN_RESERVED_CKB + 10000000000, MIN_RESERVED_CKB)),
((0, 1), (MIN_RESERVED_CKB, MIN_RESERVED_CKB + 10000000000)),
],
2,
true,
Expand Down Expand Up @@ -115,3 +115,130 @@ async fn test_send_payment_for_direct_channel_and_dry_run() {
assert_eq!(node_0_balance, 10000000000);
assert_eq!(node_1_balance, 0);
}

#[tokio::test]
async fn test_send_payment_for_pay_self() {
init_tracing();
let _span = tracing::info_span!("node", node = "test").entered();
// from https://github.com/nervosnetwork/fiber/issues/362

let (nodes, channels) = create_n_nodes_with_index_and_amounts_with_established_channel(
&[
((0, 1), (MIN_RESERVED_CKB + 10000000000, MIN_RESERVED_CKB)),
((1, 2), (MIN_RESERVED_CKB + 10000000000, MIN_RESERVED_CKB)),
((2, 0), (MIN_RESERVED_CKB + 10000000000, MIN_RESERVED_CKB)),
],
3,
true,
)
.await;
let [mut node_0, node_1, node_2] = nodes.try_into().expect("3 nodes");

let node_1_channel0_balance = node_1.get_local_balance_from_channel(channels[0]);
let node_1_channel1_balance = node_1.get_local_balance_from_channel(channels[1]);
let node_2_channel1_balance = node_2.get_local_balance_from_channel(channels[1]);
let node_2_channel2_balance = node_2.get_local_balance_from_channel(channels[2]);

// sleep for a while
tokio::time::sleep(tokio::time::Duration::from_secs(1)).await;

// now node_0 -> node_2 will be ok only with node_1, so the fee is larger than 0
let res = node_0
.send_payment(SendPaymentCommand {
target_pubkey: Some(node_2.pubkey.clone()),
amount: Some(60000000),
payment_hash: None,
final_tlc_expiry_delta: None,
tlc_expiry_limit: None,
invoice: None,
timeout: None,
max_fee_amount: None,
max_parts: None,
keysend: Some(true),
udt_type_script: None,
allow_self_payment: false,
dry_run: true,
})
.await;

assert!(res.unwrap().fee > 0);

// node_0 -> node_0 will be ok for dry_run if `allow_self_payment` is true
let res = node_0
.send_payment(SendPaymentCommand {
target_pubkey: Some(node_0.pubkey.clone()),
amount: Some(60000000),
payment_hash: None,
final_tlc_expiry_delta: None,
tlc_expiry_limit: None,
invoice: None,
timeout: None,
max_fee_amount: None,
max_parts: None,
keysend: Some(true),
udt_type_script: None,
allow_self_payment: true,
dry_run: false,
})
.await;

eprintln!("res: {:?}", res);
assert!(res.is_ok());

// sleep for a while
tokio::time::sleep(tokio::time::Duration::from_secs(1)).await;
let res = res.unwrap();
let payment_hash = res.payment_hash;
node_0
.assert_payment_status(payment_hash, PaymentSessionStatus::Success, Some(1))
.await;

let node_0_balance1 = node_0.get_local_balance_from_channel(channels[0]);
let node_0_balance2 = node_0.get_local_balance_from_channel(channels[2]);

assert_eq!(node_0_balance1, 10000000000 - 60000000 - res.fee);
assert_eq!(node_0_balance2, 60000000);

eprintln!(
"node1 left: {:?}, right: {:?}",
node_1.get_local_balance_from_channel(channels[0]),
node_1.get_local_balance_from_channel(channels[1])
);

let node_1_new_channel0_balance = node_1.get_local_balance_from_channel(channels[0]);
let node_1_new_channel1_balance = node_1.get_local_balance_from_channel(channels[1]);
let node_2_new_channel1_balance = node_2.get_local_balance_from_channel(channels[1]);
let node_2_new_channel2_balance = node_2.get_local_balance_from_channel(channels[2]);

let node1_fee = (node_1_new_channel0_balance - node_1_channel0_balance)
- (node_1_channel1_balance - node_1_new_channel1_balance);
assert!(node1_fee > 0);

let node2_fee = (node_2_new_channel1_balance - node_2_channel1_balance)
- (node_2_channel2_balance - node_2_new_channel2_balance);
assert!(node2_fee > 0);
assert_eq!(node1_fee + node2_fee, res.fee);

// node_0 -> node_2 will be ok with direct channel2,
// since after payself this channel now have enough balance, so the fee is 0
let res = node_0
.send_payment(SendPaymentCommand {
target_pubkey: Some(node_2.pubkey.clone()),
amount: Some(60000000),
payment_hash: None,
final_tlc_expiry_delta: None,
tlc_expiry_limit: None,
invoice: None,
timeout: None,
max_fee_amount: None,
max_parts: None,
keysend: Some(true),
udt_type_script: None,
allow_self_payment: false,
dry_run: true,
})
.await;

eprintln!("res: {:?}", res);
assert_eq!(res.unwrap().fee, 0);
}
1 change: 1 addition & 0 deletions src/fiber/tests/test_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ use crate::{
};

static RETAIN_VAR: &str = "TEST_TEMP_RETAIN";
pub(crate) const MIN_RESERVED_CKB: u128 = 4200000000;

#[derive(Debug)]
pub struct TempDir(ManuallyDrop<OldTempDir>);
Expand Down

0 comments on commit d7b8494

Please sign in to comment.