Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

renepay: accomodate fees in the payment flow #6893

Merged
merged 1 commit into from Jan 29, 2024

Conversation

Lagrang3
Copy link
Collaborator

@Lagrang3 Lagrang3 commented Nov 23, 2023

Min. Cost Flow (MCF) does not resolve the correct amount in channel forwarding
because the algorithm dismisses the amounts paid as fees to the routing
nodes. The root of this problem is that flow conservation is assumed for
simplicity such that the sender is the only source and the recepient is the
only sink. This in reality is not true, because also forwarding nodes are
sinks since they consume fees.

When real amounts including fees are then added to the payment routes by
the function flow_complete, some channels might be requested to forward an
amount which could be greater than known_max - htlc_total.
For remote channels this issue is likely inadverted because we are anyways
caping the probability of success down to 5% in our linearization scheme,
see this line,
ie. 5% of the channel probable liquidity is reserved which could be in most
cases sufficient to let fees go through.
However, if the conditional capacity (ie. know_max - known_min) is small
then its 5% may not be sufficient to let fees go through, specially if the MCF
has saturated the available liquidity of that particular channel.
The easiest way this might happen is with local channels, because
known_max-knon_min = 0 therefore there is 0 breath space for fees, furthermore
the MCF is always favorable to send as much flow as possible through local
channels because their fee and uncertainty cost is zero.
This was reported in issue #6599.

A temporary fix #6601 was merged that puts a fee reserve of 1% in local
channels.

This commit is a more robust solution that works-around the problem by reducing
a portion of the amount in those routes that exceed the liquidity bound.
Then the deficit in the deliver amount is either sent along the other routes
or sent through a new set of flows.

Liquidity bound in a channel

The liquidity bound in a channel is defined as known_max - htlc_total, it
represents the maximum amount we can forward through this channel given our
current knowledge. This is implemented in this function:

static struct amount_msat channel_liquidity(
		const struct gossmap *gossmap,
		struct chan_extra_map *chan_extra_map,
		const struct gossmap_chan *chan,
		const int dir)

Maximum forward

Bolt 7 specifies that a channel should forward an amount out if the
fee is greater equal to

base_fee + floor(out*proportional_fee/1000000)

If there is a limit in to the amount we can pay to a node to forward a payment on
our behalf (through a specific channel) there will be a limit to the amount that
the channel can forward for us. Since we would like to send as much as possible we
need to solve the following problem:
given the value in, find the maximum value out such that:

in - out >= base_fee + floor(out*proportional_fee/1000000)

Which is equivalent to

out <= ((in - base_fee)*1000000 + (out*proportional_fee) % 1000000)/(proportional_fee+1000000)

Let's denote the bound in the right hand side as B(in,out),

B(in,out) = ((in - base_fee)*1000000 + (out*proportional_fee) % 1000000)/(proportional_fee+1000000)

which depends on out and that makes the fee equation difficult to invert.
But consider another simpler expression B_simple(in)

B_simple(in)  = floor((in - base_fee)*1000000/(proportional_fee + 1000000))

One can quickly verify that B_simple(in) <= B(in,out) so that one could use B_simple(in) as an upper bound for out.
In fact we can even show that

B_simple(in) <= B(in,out) < B_simple(in) + 2

for every value of out.

Therefore if we set

out = B_simple(in) = floor((in - base_fee)*1000000/(proportional_fee + 1000000))

we will automatically satisfy Bolt7 inequality.
We can also try if out+1 also satisfies the inequality, if it does then that
number is the maximum value we can forward.

The function

static struct amount_msat channel_maximum_forward(
		const struct gossmap_chan *chan,
		const int dir,
		struct amount_msat in)

computes the maximum value of out such that

in - out >= base_fee + floor(out*proportional_fee/1000000)

using the technique we explained here.

Liquidity bound of a route

Another component of this PR is the liquidity bound of a given route.
It is defined as the maximum amount the last node can receive considering the
liquidity bound of each channel and the fees that are paid.
It can be computed greedly in a single pass from the source to the destination.
The implementation is in the function:

static struct amount_msat flow_maximum_deliverable(
		const struct flow *flow,
		const struct gossmap *gossmap,
		struct chan_extra_map *chan_extra_map)

For completeness we also constrain the liquidity of the route by taking into account the htlc_max.

Changes

  1. the function flow_complete allocates amounts to send over the set of routes
    computed by the MCF algorithm, but it does not allocate more than liquidity
    bound of the route. For this reason minflow returns a set of routes that
    satisfy the liquidity bounds but it is not guaranteed that the total payment
    reaches the destination therefore there could a deficit in the delivery:
    deficit = amount_to_deliver - delivering.

  2. in the function add_payflows after minflow returns a set of routes we
    call flows_fit_amount that tries to a allocate the deficit in the routes
    that the MCF have computed.

  3. if the resulting flows pass all payment constraints then we update
    amount_to_deliver = amount_to_deliver - delivering, and the loop
    repeats as long as amount_to_deliver is not zero.

In other words, the excess amount, beyond the liquidity bound,
in the routes is removed and then we try to allocate it
into known routes, otherwise we do a whole MCF again just for the
remaining amount.

Task list

  • implementing the solution
  • write tests

@rustyrussell
Copy link
Contributor

Notes:

  1. make check-includes failed. Easy fix.
  2. Notes in GH are very transitory, so make sure the commit has this text on what's changing, and the code has comments on what is now there.

@Lagrang3
Copy link
Collaborator Author

3fc82bf: fixed some conflicts with the previous changes to the local gossmods construction introduced in #6869.
Now we are ready for review.

@Lagrang3
Copy link
Collaborator Author

Added an error check in uncertainty_network_update_from_listpeerchannels and rebased and squashed commits.

@Lagrang3 Lagrang3 marked this pull request as ready for review December 16, 2023 10:48
@rustyrussell
Copy link
Contributor

I tried to rebase this, but it was non-trivial and the tests failed. So I'd ask you to do it please?

Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trivial feedback only... this approach looks much nicer than my random hack!!!

Rebase needed though


/* Try to increase the value we send until we fail to fulfill the
* fee inequality. It takes only one iteration though. */
for(int i=0;i<10;++i)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: I prefer size_t i if it's a simple loop like this

= amount_msat_min(
channel_liquidity(gossmap,chan_extra_map,flow->path[0],flow->dirs[0]),
channel_max_htlc(flow->path[0],flow->dirs[0]));
for(int i=1;i<tal_count(flow->path);++i)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here too...

* probability. If the deficit is very small with respect to the amount
* each flow carries then optimization here will not make much
* difference. */
for(int i=0;i<tal_count(flows) && !amount_msat_zero(deficit);++i)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here too...

    Min. Cost Flow does not take into account fees when computing a flow
    with liquidity constraints.
    This is a work-around solution that reduces the amount on every route to
    respect the liquidity bound. The deficity in the delivered amount is
    solved by running MCF once again.

    Changes:

    1. the function `flow_complete` allocates amounts to send over the set of routes
       computed by the MCF algorithm, but it does not allocate more than liquidity
       bound of the route. For this reason `minflow` returns a set of routes that
       satisfy the liquidity bounds but it is not guaranteed that the total payment
       reaches the destination therefore there could a deficit in the delivery:
       `deficit = amount_to_deliver - delivering`.

    2. in the function `add_payflows` after `minflow` returns a set of routes we
       call `flows_fit_amount` that tries to a allocate the `deficit` in the routes
       that the MCF have computed.

    3. if the resulting flows pass all payment constraints then we update
        `amount_to_deliver = amount_to_deliver - delivering`, and the loop
        repeats as long as `amount_to_deliver` is not zero.

    In other words, the excess amount, beyond the liquidity bound,
    in the routes is removed and then we try to allocate it
    into known routes, otherwise we do a whole MCF again just for the
    remaining amount.

    Fixes issue ElementsProject#6599
@Lagrang3
Copy link
Collaborator Author

I have rebased and fixed the conflicts. If the CI test pass, this will be ready for merging.

@rustyrussell
Copy link
Contributor

rustyrussell commented Jan 28, 2024

Flake is unrelated, opened PR #7021 for that.

@rustyrussell rustyrussell merged commit b0054aa into ElementsProject:master Jan 29, 2024
38 checks passed
@Lagrang3 Lagrang3 deleted the renepay_fee_reserve branch January 29, 2024 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants