nervosnetwork / fiber

21 stars 11 forks source link

After send payment with `allow_self_payment`, the status of invoice and payment is not updated #300

Closed ahonn closed 1 week ago

ahonn commented 3 weeks ago

When I call new_invoice to create an invoice and then call send_payment to make a payment on the same fiber node, the statuses returned by the get_payment and get_invoice queries do not change. However, if I call send_payment to the same invoice again, I encounter an error: "InvalidParameter: Payment session already exists."

The response of the get_payment after send the payment:

{
  "jsonrpc": "2.0",
  "result": {
    "payment_hash": "0x1b7dee974c003b5ee95b9e49088c080e8ba21403377239172aa82be64f007436",
    "status": "Inflight",
    "created_at": "0x192e6f06716",
    "last_updated_at": "0x625d61b12be66",
    "failed_error": null
  },
  "id": 42
}

The logs on the fiber node for the two sent payments are as follows:

fiber-node  |   2024-11-01T08:56:07.702788Z  INFO fnn::fiber::graph: build_route source: Pubkey(PublicKey(048cea664402b5d315e90f725d3fa166392e6142465e81dd0df205cdabc81de55228a3a06e42aa5becfc93a9757113f7e554bf3b76cb3dc87a9f6e513205f2a6)) target: Pubkey(PublicKey(048cea664402b5d315e90f725d3fa166392e6142465e81dd0df205cdabc81de55228a3a06e42aa5becfc93a9757113f7e554bf3b76cb3dc87a9f6e513205f2a6)) amount: 22200000000, payment_hash: Hash256(0x1b7dee974c003b5ee95b9e49088c080e8ba21403377239172aa82be64f007436)
fiber-node  |     at src/fiber/graph.rs:502
fiber-node  |     in ractor::actor::Actor with id: "0.2", name: "Network Qme3a2hpMD1CcNuML6SyNetH8NkiVHNaTz5phvBqfjrgB3"
fiber-node  |
fiber-node  |   2024-11-01T08:56:07.702851Z  INFO fnn::fiber::graph: get_route: nodes visited: 4, edges expanded: 5, time: 38.291µs
fiber-node  |     at src/fiber/graph.rs:753
fiber-node  |     in ractor::actor::Actor with id: "0.2", name: "Network Qme3a2hpMD1CcNuML6SyNetH8NkiVHNaTz5phvBqfjrgB3"
fiber-node  |
fiber-node  |   2024-11-01T08:56:51.075306Z ERROR fnn::fiber::network: Failed to send payment: InvalidParameter("Payment session already exists: Hash256(0x1b7dee974c003b5ee95b9e49088c080e8ba21403377239172aa82be64f007436) with payment session status: Inflight")
fiber-node  |     at src/fiber/network.rs:1582
fiber-node  |     in ractor::actor::Actor with id: "0.2", name: "Network Qme3a2hpMD1CcNuML6SyNetH8NkiVHNaTz5phvBqfjrgB3"
fiber-node  |
fiber-node  |   2024-11-01T08:56:51.075338Z ERROR fnn::rpc::channel: channel request params SendPaymentCommandParams { target_pubkey: None, amount: None, payment_hash: None, final_cltv_delta: None, invoice: Some("fibt222000000001pucv7vxj357gfw0pmm6vkarxxz9hal0wnt0ardstluzg98wd2ccnxuzs6t4vdm2k6gtdvmrqfxgcsm6m7cuz7snuxjs7zzewrshq5te04nsm2zxa98drdjkv0hed2402n8ux6084mjq40p5tl669ajc53jlxg3qg9x0ps4lnxjuf3a58gtxcmv645wsnzp32t56w7xwy4z2fl79h6rl7clkzkjpj55san4x8kztt64z429ppelwu72vf98526pjyr4va4xahd7q0hqm73f4yjt8y0ym4v4e37djgkk040g4at2cnhnt70wtq9y02kts35cvarhy8zeu8mhf4vytayl3laj4umwy92yt9s80m3nc7sueavmaypxr403d7znz6u0gkntf4g7hlf3kvme7rqp8ks62z"), timeout: None, max_fee_amount: None, max_parts: None, keysend: None, udt_type_script: None, allow_self_payment: Some(true) } => error: "InvalidParameter: Payment session already exists: Hash256(0x1b7dee974c003b5ee95b9e49088c080e8ba21403377239172aa82be64f007436) with payment session status: Inflight"
fiber-node  |     at src/rpc/channel.rs:546
chenyukang commented 1 week ago

302 will resolve this issue.

Please note, for self-payment such as A -> B -> A, we don't allow the payment route use same channel for both A -> B and B -> A.

In other words, we need to open another channel from B -> A.

ahonn commented 6 days ago

After some testing, I identified an issue: when both channels A(100)->B(0) and B(100)->A(0) have a remote_balance of 0, self-payment fails with the error message: Send payment error: Failed to send onion packet with error IncorrectOrUnknownPaymentDetails.

It looks like both the remote_balance and local_balance of these two channels need to be sufficient for a successful self-payment, which seems illogical.

Should A(100)->B(0) and B(100)->A(0) allow self-payment? I'm a little confused about this.

chenyukang commented 6 days ago

A(100)->B(0) and B(100)->A(0) should allow self-payment, I will add a test case for it.

chenyukang commented 6 days ago

This is a test cast I just created for the scenario:

https://github.com/chenyukang/fiber/tree/self-pay-test-case

please see the steps here: https://github.com/chenyukang/fiber/tree/self-pay-test-case/tests/bruno/e2e/router-pay

There are two channels between NodeA and NodeB, both with remote balance of 0, and we can send a self payment from NodeA successfully:

e2e/router-pay/29-node2-list-channels (200 OK) - 10 ms
list channels:  [
  {
    channel_id: '0x4017f2b755355cae2f0ae6ab1eb345fd3aa2699f1be0c5b383d77ce5427370b2',
    is_public: true,
    channel_outpoint: '0x63522be63f906f7cf31d3c95ea2454c7133fe4251d79e774f420b64a23f5f1f900000000',
    peer_id: 'QmbvRjJHAQDmj3cgnUBGQ5zVnGxUKwb2qJygwNs2wk41h8',
    funding_udt_type_script: null,
    state: { state_name: 'CHANNEL_READY', state_flags: [] },
    local_balance: '0x277939c85200',
    offered_tlc_balance: '0x0',
    remote_balance: '0x0',
    received_tlc_balance: '0x0',
    created_at: '0x193436ca464'
  },
  {
    channel_id: '0x300eb97bd2c6c3acba303b79b97d2cf448ef8163f729df3dcc8379ebde1c6803',
    is_public: true,
    channel_outpoint: '0x9a684a0b1574f70e06b8db763934a64b2f2b5cd7a01d46b8b3292e987d0ca0fc00000000',
    peer_id: 'QmbvRjJHAQDmj3cgnUBGQ5zVnGxUKwb2qJygwNs2wk41h8',
    funding_udt_type_script: null,
    state: { state_name: 'CHANNEL_READY', state_flags: [] },
    local_balance: '0x0',
    offered_tlc_balance: '0x0',
    remote_balance: '0x377939c85200',
    received_tlc_balance: '0x0',
    created_at: '0x193436c413e'
  }
]
   ✓ assert: res.status: eq 200
e2e/router-pay/30-node1-pay-self-with-node2-succ (200 OK) - 55 ms
30 step result:  {
  jsonrpc: '2.0',
  result: {
    payment_hash: '0x08bb8a1972641c87cb38b4dd2138e3f8a209e230f8063333208fd30f6d7c6942',
    status: 'Inflight',
    created_at: '0x193436cc400',
    last_updated_at: '0x193436cc42e',
    failed_error: null
  },
  id: '42'
}
   ✓ assert: res.body.error: isUndefined
e2e/router-pay/31-node1-get-payment-status (200 OK) - 4 ms
step 31 get result:  {
  payment_hash: '0x08bb8a1972641c87cb38b4dd2138e3f8a209e230f8063333208fd30f6d7c6942',
  status: 'Success',
  created_at: '0x193436cc400',
  last_updated_at: '0x193436cc4df',
  failed_error: null
}
chenyukang commented 6 days ago

run command, in one terminal start all the nodes:

REMOVE_OLD_FIBER=y START_BOOTNODE=y ./tests/nodes/start.sh

in another terminal:

npm exec -- @usebruno/cli@1.20.0 run e2e/router-pay -r --env test
chenyukang commented 6 days ago

any other difference between my testcase and yours? did you consider the fee amount?

ahonn commented 6 days ago

any other difference between my testcase and yours? did you consider the fee amount?

Thank you for your response. I'm a bit unsure at the moment, but I’ll do my best to reproduce the issue. If it happens again, I’ll open a new issue to let you know.