Skip to content

Add liquidity fee split to facilitate integration#706

Merged
pm47 merged 2 commits intomasterfrom
otf-fees
Oct 2, 2024
Merged

Add liquidity fee split to facilitate integration#706
pm47 merged 2 commits intomasterfrom
otf-fees

Conversation

@pm47
Copy link
Member

@pm47 pm47 commented Sep 27, 2024

Even in the "from future htlc" case the mining fee corresponding to the previous channel output will be paid immediately from the channel balance, except if there is no channel.

In the "from future htlc case", this inbound liquidity purchase is going to be followed by one or several IncomingPayment.ReceivedWith.LightningPayment, with a non-null LiquidityAds.FundingFee where LiquidityAds.FundingFee.fundingTxId matches this InboundLiquidityOutgoingPayment.txId. The sum of
LiquidityAds.FundingFee.amount will match InboundLiquidityOutgoingPayment.feePaidFromFutureHtlc.

CC @dpad85

Builds on #705.

@pm47 pm47 requested a review from t-bast September 27, 2024 12:46
pm47 added a commit to ACINQ/phoenixd that referenced this pull request Sep 27, 2024
dpad85 added a commit to ACINQ/phoenix that referenced this pull request Sep 27, 2024
… is shown or not

See ACINQ/lightning-kmp#706

Methods feePaidFromChannelBalance and feePaidFromFutureHtlc have been added
to check whether an inbound liquidity purchase is paid from balance or future
htlc. This is used to know when to display a liquidity fee or not, and to get
the liquidity fee for an incoming payment.

Also fixed some UI issues with descriptions and improved txid and channel id
buttons in technical screen.
Copy link
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

I looked at the last two commits, which look good to me 👍

}
val feeCreditUsed = when (purchase) {
is LiquidityAds.Purchase.WithFeeCredit -> purchase.feeCreditUsed
else -> 0.msat
Copy link
Member

Choose a reason for hiding this comment

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

This case can be slightly confusing: when we're using WithFeeCredit, we're indeed paying the whole purchase fees from our fee credit. However in the FromFutureHtlc and FromFutureHtlcWithPreimage we may also consume some fee credit to partially pay the fees (decided by the LSP to ensure that the fee credit converges to 0), but we will only be able to know the amount of fee credit used once we've received the HTLCs: the fee credit used will be equal to purchase.fees.total - LiquidityAds.FundingFee. I don't know if we want to surface this to the user, it may be cumbersome because it needs to read multiple entries from the DB (the liquidity purchase and the associated HTLCs).

Copy link
Member Author

Choose a reason for hiding this comment

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

Couldn't we add a specific TLV in the htlc to pass this info explicitly?

it may be cumbersome because it needs to read multiple entries from the DB (the liquidity purchase and the associated

We already have to do that in Phoenix, to link automated liquidity events to incoming payments.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's merge this PR and address this point in a follow up if necessary.

Copy link
Member

Choose a reason for hiding this comment

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

Couldn't we add a specific TLV in the htlc to pass this info explicitly?

I don't think we need to add anything, we already have everything we need. We know the total liquidity fee from the purchase DB entry, and we know the fee collected from the HTLCs from the LiquidityAds.FundingFee TLV. The difference between those two values is the fee credit that was used.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, but could we find a way to have this information "statelessly" in the HTLCs, even if it introduces a bit of redundancy ? Now that I think of it, I will definitely need this level of precision for the csv audit, and being unable to process rows one by one is going to be painful.

Earlier I mentioned that we already had to read multiple entries in phoenix, but that's for UI stuff and we don't need absolute precision there.

Copy link
Member

Choose a reason for hiding this comment

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

Actually my comment was completely wrong! I verified the code again, and whenever we use some fee credit (even if we partially pay the fees with it), we will create a LiquidityAds.Purchase.WithFeeCredit. So there's nothing more to add!

pm47 added 2 commits October 1, 2024 13:28
Even in the "from future htlc" case the mining fee corresponding to
the previous channel output will be paid immediately from the channel
balance, except if there is no channel.

In the "from future htlc case", this inbound liquidity purchase is
going to be followed by one or several
`IncomingPayment.ReceivedWith.LightningPayment`, with a non-null
`LiquidityAds.FundingFee` where `LiquidityAds.FundingFee.fundingTxId`
matches this `InboundLiquidityOutgoingPayment.txId`. The sum of
`LiquidityAds.FundingFee.amount` will match
`InboundLiquidityOutgoingPayment.feePaidFromFutureHtlc`.
@pm47
Copy link
Member Author

pm47 commented Oct 1, 2024

Rebased.

@pm47 pm47 merged commit 8704842 into master Oct 2, 2024
@pm47 pm47 deleted the otf-fees branch October 2, 2024 13:23
pm47 added a commit to ACINQ/phoenixd that referenced this pull request Oct 18, 2024
Depending on the `payment_type`, fees for a liquidity purchases may be
paid from the balance, or from future htlcs. In the former case, we must
count the fee amount must be deducted from balance immediately.

A helper method `InboundLiquidityOutgoingPayment.feePaidFromChannelBalance`
has been introduced for that purpose in
ACINQ/lightning-kmp#706 and we must use it.
pm47 added a commit to ACINQ/phoenixd that referenced this pull request Oct 18, 2024
Depending on the `payment_type`, fees for a liquidity purchases may be
paid from the balance, or from future htlcs. In the former case, we must
count the fee amount must be deducted from balance immediately.

A helper method `InboundLiquidityOutgoingPayment.feePaidFromChannelBalance`
has been introduced for that purpose in
ACINQ/lightning-kmp#706 and we must use it.
vincenzopalazzo pushed a commit to vincenzopalazzo/phoenixd that referenced this pull request Nov 7, 2024
Depending on the `payment_type`, fees for a liquidity purchases may be
paid from the balance, or from future htlcs. In the former case, we must
count the fee amount must be deducted from balance immediately.

A helper method `InboundLiquidityOutgoingPayment.feePaidFromChannelBalance`
has been introduced for that purpose in
ACINQ/lightning-kmp#706 and we must use it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants