-
Notifications
You must be signed in to change notification settings - Fork 24
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
On-the-fly channel funding based on splicing and liquidity ads #649
Conversation
01ff74c
to
a0702b2
Compare
a0702b2
to
11d94a6
Compare
src/commonMain/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandler.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review for commit e307660.
src/commonMain/kotlin/fr/acinq/lightning/channel/ChannelData.kt
Outdated
Show resolved
Hide resolved
src/commonMain/kotlin/fr/acinq/lightning/payment/LiquidityPolicy.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review for commit b5c187d.
src/commonMain/kotlin/fr/acinq/lightning/channel/states/WaitForAcceptChannel.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review for commit 22316a5.
// Once the channel funding is complete, we may have enough inbound liquidity to receive the payment without | ||
// an on-chain operation, which is more efficient. We thus reject that payment and wait for the sender to retry. | ||
logger.warning { "cannot accept on-the-fly funding: another funding attempt is already in-progress" } | ||
val failure = OutgoingPaymentPacket.buildWillAddHtlcFailure(nodeParams.nodePrivateKey, msg, TemporaryNodeFailure) | ||
input.send(SendOnTheFlyFundingMessage(failure)) | ||
nodeParams._nodeEvents.emit(LiquidityEvents.Rejected(msg.amount, 0.msat, LiquidityEvents.Source.OffChainPayment, LiquidityEvents.Rejected.Reason.ChannelFundingInProgress)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This constitutes a UX regression compared to the previous protocol, which was fully parallel, but seems acceptable for simplicity. A potential, not too far fetched scenario would be a popular social media account posting a BOLT 12 donation link: there would be some seemingly random failures everytime the channel gets resized.
Perhaps we can devise a more lenient mechanism in a follow up PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think in that case we could keep the will_add_htlc
messages around, and replay them once the current splice is finished. This would create more risks around timeouts though, so we'll need to carefully think about this.
Also, this would be sub-optimal: if we have a splice in progress, since we always buy more inbound liquidity than what we need for the associated payment, the second payment would likely go through without needing an additional splice. By failing it immediately, we give the sender an opportunity to retry (potentially automatically) and get it relayed after signing the splice.
spliceOut = null, | ||
requestRemoteFunding = LiquidityAds.RequestFunds(cmd.requestedAmount, cmd.fundingLease, paymentDetails), | ||
feerate = targetFeerate, | ||
origins = listOf(Origin.OffChainPayment(cmd.preimage, cmd.paymentAmount, totalFees)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was expecting this channel origin to be forwarded to our peer and associated to the splice, but it's not the case. How do we expect the peer to synchronize between channel opens/splices and htlc forwarding? Something like "after an open/splice, send all htlcs that we have sent a WillAddHtlc
for, and didn't receive a WillFailHtlc
"?
Edit: okay the reference is in LiquidityAds.RequestFunds.paymentDetails
. Isn't the channel origin kind of redundant? I'm still not clear on how exactly we deal with multiple parallel payments on the sender's side. I also assume there will be a timeout for replies to WillAddHtlc
(even in the single-payment case).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't the channel origin kind of redundant?
I think the paymentAmount
provided in Origin.OffChainPayment
is useful, and cannot be included in the payment_details
.
I'm still not clear on how exactlt we deal with multiple parallel payments on the sender's side.
Can you detail what you mean by "multiple parallel payments on the sender's side"? Who is the sender here?
I also assume there will be a timeout for replies to WillAddHtlc (even in the single-payment case).
Yes, that is specified in the gist, the LSP times out will_add_htlc
if it doesn't receive an answer or a matching open/splice.
src/commonMain/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandler.kt
Outdated
Show resolved
Hide resolved
src/commonMain/kotlin/fr/acinq/lightning/payment/IncomingPaymentHandler.kt
Show resolved
Hide resolved
src/commonMain/kotlin/fr/acinq/lightning/channel/InteractiveTx.kt
Outdated
Show resolved
Hide resolved
src/commonMain/kotlin/fr/acinq/lightning/channel/InteractiveTx.kt
Outdated
Show resolved
Hide resolved
0d0a30d
to
1242282
Compare
c684dcc
to
172c032
Compare
55f0c2f
to
b895794
Compare
b895794
to
aa4fc3b
Compare
ab9ef6e
to
371ac59
Compare
371ac59
to
7f95eb4
Compare
Rebased without any conflict. |
87a1d54
to
46ef5c0
Compare
baf9259
to
4211e66
Compare
src/commonMain/kotlin/fr/acinq/lightning/payment/LiquidityPolicy.kt
Outdated
Show resolved
Hide resolved
2d91f0a
to
85f2de9
Compare
It is usually the wallet that decides that it needs a channel, but we want the LSP to pay the commit fees to allow the wallet user to empty its wallet over lightning. We previously used a `please_open_channel` message that was sent by the wallet to the LSP, but it doesn't work well with liquidity ads. We remove that message and instead send `open_channel` from the wallet but with a custom channel flag that tells the LSP that they should be paying the commit fees. This only works if the LSP adds funds on their side of the channel, so we couple that with liquidity ads to request funds from the LSP. We also add a `recommended_feerates` message from the LSP which lets the wallet know the on-chain feerates that the LSP will accept for on-chain funding operations, since those feerates are set in the `open_channel` message that is now sent by the wallet.
We previously only used liquidity ads with splicing: we now support it during the initial channel opening flow as well. This lets us add more unit tests, including tests for the case where the node receiving the `open_channel` message is responsible for paying the commitment fees. We also update liquidity ads to use the latest version of the spec from lightning/bolts#1153. This introduces more ways of paying the liquidity fees, to support on-the-fly funding without existing channel balance (not implemented in this commit). Note that we need some backwards-compatibility with the previous liquidity ads types in our state serialization code: when we're in the middle of signing a splice transaction, we may have a legacy liquidity lease in our splice status. We ignore it when finalizing the splice: the only consequence is that we won't store an entry in our DB for that lease, but the channel will otherwise work correctly.
We replace the previous pay-to-open protocol with a new protocol that only relies on liquidity ads for paying fees. We simply transmit HTLCs that cannot be relayed on existing channels with a new message called `will_add_htlc` that contains all the HTLC data. The recipient can verify that the HTLC that would match this promise is valid, and if it wishes to accept that payment, it can trigger a channel open or a splice to purchase the required inbound liquidity. Once that transaction completes, the sender will relay HTLCs matching the proposed `will_add_htlc`, which completes the payment. If the fees for the inbound liquidity purchase couldn't be paid from the previous channel balance, they can be taken from the HTLCs relayed after the funding transaction. When that happens, one side needs to trust that the other will comply. Each side can independently configure the options they're comfortable with, depending on whether they trust their peer or not.
Creating a new channel has an additional cost compared to adding liquidity to an existing channel: the channel will be closed in the future, which will require paying on-chain fees. Node operators can include a `channel-creation-fee-satoshis` in their liquidity ads to cover some of that future cost.
We clarify some of our event types that previously had an `amount` field to detail whether this amount includes fees or not. This impacts: - SwapInEvents.Accepted - StoreIncomingPayment.ViaNewChannel - StoreIncomingPayment.ViaSpliceIn - Origin.OnChainWallet - Origin.OffChainPayment There was an inconsistency in the `ViaSpliceIn` event, where in some cases we used the received amount, and in others the amount with fees.
We forgot to match the `payment_hash` for this payment type, and also didn't check that the `funding_fee` was `0 msat`.
We previously forced wallets to purchase additional inbound liquidity every time an on-chain transaction was created. We now allow wallets to disable automatic liquidity purchases: the LSP will need to add enough funds on their side to cover the commitment fees, which the wallet won't be paying for. But we still make a dummy purchase of 1 sat to ensure that the liquidity ads flow is used and the wallet refunds the mining fees paid by the LSP.
Instead of using a hard-coded value from `WalletParams`, we read the liquidity funding rates from our peer's `init` message.
85f2de9
to
1b7e95b
Compare
* enable feature FundingFeeCredit * emit a liquidity event for each liquidity purchase Independently of whether the purchase was triggered by an on-chain or off-chain payment. Also renamed `LiquidityEvents.Accepted` to `LiquidityEvents.Purchased`. * expose serviceFee in InboundLiquidityOutgoingPayment * add events PaymentEvents.PaymentSent
Adds support for liquidity-ads based protocol for on-the-fly liquidity as specified in lightning/blips#36 and lightning/blips#41, implemented respectively in ACINQ/lightning-kmp#649 and ACINQ/lightning-kmp#660. ### Lightning-kmp update Phoenixd now uses the main branch of `lightning-kmp` (v1.8.0). ### Database update - `LiquidityAds.Lease` is replaced by `LiquidityAds.Purchase`, so we need to update the liquidity table. - the `receivedWith` data have been updated in lightning-kmp, and we need a new `Part.Htlc.V1` object that may contain a `LiquidityAds.FundingFee`. With the `Lease->Purchase` change, we've updated our pattern for versioning database objects. We now have `asDb()` & `asCanonical()` mapping methods and store the type of the db object inside the json (which means we don't need the `type` column anymore, except for convenience). --------- Co-authored-by: pm47 <[email protected]>
Adds support for liquidity-ads based protocol for on-the-fly liquidity as specified in lightning/blips#36 and lightning/blips#41, implemented respectively in ACINQ/lightning-kmp#649 and ACINQ/lightning-kmp#660. ### Lightning-kmp update Phoenixd now uses the main branch of `lightning-kmp` (v1.8.0). ### Database update - `LiquidityAds.Lease` is replaced by `LiquidityAds.Purchase`, so we need to update the liquidity table. - the `receivedWith` data have been updated in lightning-kmp, and we need a new `Part.Htlc.V1` object that may contain a `LiquidityAds.FundingFee`. With the `Lease->Purchase` change, we've updated our pattern for versioning database objects. We now have `asDb()` & `asCanonical()` mapping methods and store the type of the db object inside the json (which means we don't need the `type` column anymore, except for convenience). --------- Co-authored-by: pm47 <[email protected]>
We revamp our protocols for on-the-fly funding to use a unified approach based on liquidity ads. We simplify messages exchanged and reduce the number of custom extensions required on top of the BOLT specifications.
The main changes to the protocol are:
open_channel2
andsplice_init
)interactive-tx
protocolpay_to_open
messages with awill_add_htlc
message, which is a clone ofupdate_add_htlc
without achannel_id
orhtlc_id
:IncomingPaymentHandler
open_channel2
orsplice_init
to purchase enough liquidity:The specification can be found in lightning/blips#36. An implementation of the seller side is available in ACINQ/eclair#2861.
This PR is best reviewed commit-by-commit: each commit is self-contained and contains a piece of the puzzle.
Replaces #618.