RGB-Tools / rgb-lightning-node

MIT License
17 stars 19 forks source link

Maker execute fails with Error 403 (No Route Found) for High Amounts in Sats #37

Closed bitwalt closed 2 weeks ago

bitwalt commented 3 weeks ago

I've encountered a critical bug when attempting to perform a swap involving a higher amount in sats (e.g., 10,000 sats). The swap fails with the error 403 (No route found), even though there is sufficient liquidity (outbound_balance_msat and inbound_balance_msat) in the channel .

Steps to Reproduce:

  1. Attempt a swap with a "high" amount in sats, such as 10,000 sats.
  2. Observe that the maker_execute fails with the error 403 (No route found) despite there being enough liquidity in the channel.

Expected Behavior: The swap should succeed without any errors if there is enough liquidity in the channel.

Actual Behavior: The swap fails with a 403 (No route found) error.

Additional Information: I tried replicating this issue by editing the swap_roundtrip_buy_same_channel.rs test. I set qty_from to 10,000,000 msat, and the test failed, confirming the issue.

zoedberg commented 2 weeks ago

This is not a critical bug. It's more about lack of documentation and information returned by the APIs.

The outbound_balance_msat and inbound_balance_msat fields do not take into account the htlc_maximum_msat, which has an upper bound of 10% of the channel capacity (hard-coded in LDK). The only way to change this upper bound would be to manually change this value in our fork. I'm not sure we should change that value though, since it's there as a safety measure to limit the node exposure to HTLCs. But to improve the UX, we could expose the next_outbound_htlc_limit_msat and next_outbound_htlc_minimum_msat values in the list channels, so that the upper bound would be explicit and programs using RLN could deny users setting higher values when performing offchain transfers.

In the context of the swaps (and RGB payments) there's another amount to take into account though. Since to transfer RGB assets we need a non-dust bitcoin amount, when swapping the maker adds 3000 sats on both routes (as non-dust bitcoin amount for RGB payments). This way no side will pay extra bitcoins for the swap to happen. This approach has a drawback though: the amount (3000) is not explicit, which might lead to misunderstandings like the one you had (you expected let qty_from = 10_000_000 to work because it's not larger than 10% of the channel capacity, but the max value you can use is actually let qty_from = 10_000_000 - 3_000_000;). We could mitigate this issue by exposing this value in the /nodeinfo API (so that you can do next_outbound_htlc_limit_msat - rgb_htlc_min_msat to retrieve the max swappable amount). We considered also other solutions, like adding this amount into the swap string or changing this amount to a "fee" that gets paid by the part sending the assets, but after several considerations we think the proposed solution is the best one. Let us know if you agree with this or have other suggestions/opinions.

bitwalt commented 2 weeks ago

Hi @zoedberg, thank you very much for your feedback and clarifications. I didn't know about htlc_maximum_msat limit, so I believe it is important to maintain it so as to not decrease the security of the node.

I also agree with you regarding the improvements you proposed, they could definitely improve the overall user experience.

zoedberg commented 2 weeks ago

@bitwalt I've pushed c487484 which adds next_outbound_htlc_limit_msat, next_outbound_htlc_minimum_msat and rgb_htlc_min_msat. Thanks for reporting this and providing feedback