LSPS1: Explicitly support LSPS1 payment options #101

ErikDeSmedt · 2024-03-04T12:38:26Z

This PR aims to address a few problems in LSPS1.
I'll provide a few motivating examples of why I believe the changes proposed in this MR are desirable.

1. Use seprate fee for onchain vs lightning payment

Onchain payments are more expensive for the LSP than lightning payments because they create their own utxo. The LSP will have to carry the cost of spending that new utxo at a later point in time.

This PR provides LSP's the option to set a different fee depending on the payment method.

On a related note @2Fast2BCn noted that a similar argument could be made for the expiry_date of an order. The LSP might want to set a different expiry_date depending on the chosen payment option.

See issue: #92
See comment: #95 (comment)

2. Extensibility for additional payment methods

This PR allows to add additional payment options later.

In this section I'll provide a few motivating example.

Dual funding:

Recently, dual-funding got merged to the spec. The spec describes a new channel open protocol in which both the client and server can contribute funds.

This approach provides a superior trust model. The client doesn't have to trust the LSP to open the channel. The benefit is especially clear if client_balance_sat is high. In the current version of LSPS1 the LSP could take the money and run.

BOLT-12 support

Today, BOLT-11 invoices are commonly used to make payments over lightning. Personally, I'd describe myself as a BOLT-12 maxi. This PR makes it easier to add BOLT-12 as a payment option later.

Withhold from first payment

We see a demand for JIT-channels where the client specifies the channel-size. This is currently not possible on LSPS2 or LSPS4.

See issue: #65

Breaking changes: Why (not) to merge this MR?

We are in a bit of tension here. The spec is marked as FOR IMPLEMENTATION but we are seeing some usage on mainnet.

This PR is breaking in the sense that clients will not be able to purchase channel if either the client or server uses this new revision.

However, there will be some hassle for users but merging this MR should not put any funds at risk.

SeverinAlexB

All in all, it's moving into the right direction.

Outdated line L47
How do you imagine dual-funded channels will fit in here? I guess dual-funded channels will have 2 onchain addresses to pay to: client_balance_address and fee_payment_address. Or can the fee realiably be pushed to the LSP on channel open?

SeverinAlexB · 2024-03-12T09:05:10Z

LSPS1/README.md

+> Fees and expiry dates are defined on the level of the payment-option.
+> This allows an LSP to provide the cheapest service for each payment option.


I am aware of @2Fast2BCn's comment but can you give me a concrete example on what variable would change so quickly within a payment option that would make the added complexity of multiple expiry times worth it? I can't think of any so far? Maybe onchain fees?

The problem is that lsps1 suffers a free-option problem for onchain feerates.

A client can use this to his advantage using the following procedure

A client might want to open a channel between now and 6 hours. The client performs `lsps1.create_order` immediately. The client performs `lsps1.create_order` after 6 hours. The client pays for the cheapest order

When a drastic fee-rate occurs the server might be required to open the channel at a loss.
Accepting the order can incur a real risk to the LSP.

To mitigate the risk the server could either

make the order expire quickly

charge a sufficiently high fee

By putting the expiry_time in the payment_option we ensure that

bolt11 payments can use (1)

onchain payments can use (2)

First, as an LSP providing an order with an expiry time of 6hrs, I can use an insurance model to cover such cases.
The LSP adds 1% fees on top of each onchain order. This 1% will be your insurance in case of a unexpected fee spike. If 1 out of 100 orders sees a fee spike, the accumulated insurance will cover these costs.

Second, I don't see any upside for the client to time the channel opening this way. It even has the downside of the client needing to wait for the channel open and needing to monitor the fee rate.

Third, why should an order be valid for such a long time? 1hr is more than sufficient IMO.

The LSP adds 1% fees on top of each onchain order

There is no such thing as an onchain order.

The problem is that the client creates an order and the LSP returns two payment options.

onchain which needs expiry_time > now + 1h and by consequence might need insurance

bolt11 which could use expiry_time > now + 1m and by consequence not need insurance

By putting the expiry_time on the payment option we ensure that bolt11-users don't have to pay for the insurance.

Second, I don't see any upside for the client to time the channel opening this way

At current fee-rates it is not worth the hassle. At higher fee-rates saving some sats might be worth the wait.

Why should an order be valid for such a long time? 1hr is more than sufficient IMO

I'd argue 1hr is too short

Assuming a poisson distribution you can compute a

0.25% probability no blocks are mined in the next hour

1.73% probability 1 block or less is mined in the next hour

6.20% probability 2 blocks or less are mined in the next hour

If the LSP expects

1 confirmation at least 1 out of 400 payments will confirm after expiration. (Not a single block mined)

2 confirmations at least 1 out of 55 payments will confirm after expiration. (0 or 1 blocks mined)

3 confirmations at least 1 out of 16 payments will confirm after expiration. (0, 1 or 2 blocks mined)

4 confirmations at least 1 out of 6 payments will confirm after expiration

5 confirmations at least 2 out of 7 payments will confirm after expiration

6 confirmations at last 4 out of 10 payments will confirm after expiration

The computation assumes all payments make it in the next block

PS: I don't think this part is very important. I don't mind moving the expiry_time up again if you prefer it this way

To be honest, if I put my user hat on I find moving expiry_time to be a per-payment property kind of confusing. We now don't have a time when an order expires, but when said order may be paid with a given payment method.

Still +1 for a order-wide expiry time :)

By putting the expiry_time on the payment option we ensure that bolt11-users don't have to pay for the insurance.

We can define different fees for each method anyway so bolt11 users don't have to pay this insurance, no?

@ErikDeSmedt Any further thoughts on this?

LSPS1/README.md

tnull · 2024-04-24T08:24:45Z

LSPS1/README.md

@@ -119,6 +116,7 @@ The client MUST call `lsps1.get_info` first.
  - `max_channel_balance_sat` <[LSPS0.sat][]> Maximum channel size.
    - MUST be 0 or greater.

+


nit: Seems like an unnecessary change?

LSPS1/README.md

tnull · 2024-04-24T08:42:37Z

LSPS1/README.md

+> Fees and expiry dates are defined on the level of the payment-option.
+> This allows an LSP to provide the cheapest service for each payment option.


To be honest, if I put my user hat on I find moving expiry_time to be a per-payment property kind of confusing. We now don't have a time when an order expires, but when said order may be paid with a given payment method.

tnull · 2024-04-24T08:46:07Z

LSPS1/README.md

+    "onchain_address": "bc1p5uvtaxzkjwvey2tfy49k5vtqfpjmrgm09cvs88ezyy8h2zv7jhas9tu4yr",
+    "min_onchain_payment_confirmations": 1,
+    "min_fee_for_0conf": 253,
+    "onchain_payment": {


I think we simplified the state handling before, but should we drop this field entirely while we're here and leave tracking their payment details to the user until we switch to PAID?

In particular the confirmed field might not make sense in all implementation contexts as not all implementation will necessarily have access to the mempool.

I tried to not make any changes to the state-tracking behavior in this MR.
I did only change wording and separated the tracking for bolt-11 and onchain.

@SeverinAlexB already made similar comments about simplifying state-tracking for bolt-11 payments.

It appears there is a good opportunity for simplification and I'd try to tackle it in this MR

2b4fed9 should fix this

SeverinAlexB

One comment, otherwise close to being done

SeverinAlexB · 2024-05-01T11:58:34Z

LSPS1/README.md

+> Fees and expiry dates are defined on the level of the payment-option.
+> This allows an LSP to provide the cheapest service for each payment option.


Still +1 for a order-wide expiry time :)

By putting the expiry_time on the payment option we ensure that bolt11-users don't have to pay for the insurance.

We can define different fees for each method anyway so bolt11 users don't have to pay this insurance, no?

tnull · 2024-05-29T11:25:29Z

@ErikDeSmedt Gentle ping. Do you intend to address the few remaining open questions?

@2Fast2BCn

This PR aims to address a few problems in LSPS1. I'll provide a few motivating examples of why I believe the changes proposed in this MR are desirable. # 1. Use seprate fee for onchain vs lightning payment Onchain payments are more expensive for the LSP than lightning payments because they create their own utxo. The LSP will have to carry the cost of spending that new utxo at a later point in time. This PR provides LSP's the option to set a different fee depending on the payment method. On a related note @2Fast2BCn [noted](BitcoinAndLightningLayerSpecs#95 (comment)) that a similar argument could be made for the `expiry_date` of an order. The LSP might want to set a different `expiry_date` depending on the chosen payment option. See issue: BitcoinAndLightningLayerSpecs#92 See comment: BitcoinAndLightningLayerSpecs#95 (comment) # 2. Extensibility for additional payment methods This PR allows to add additional payment options later. In this section I'll provide a few motivating example. ## 2.1. Dual funding Recently, dual-funding got merged to the spec. The spec describes a new channel open protocol in which both the client and server can contribute funds. This approach provides a superior trust model. The client doesn't have to trust the LSP to open the channel. The benefit is especially clear if `client_balance_sat` is high. In the current version of LSPS1 the LSP could take the money and run. ## 2.2. BOLT-12 support Today, BOLT-11 invoices are commonly used to make payments over lightning. Personally, I'd describe myself as a BOLT-12 maxi. This PR makes it easier to add BOLT-12 as a payment option later. ### 2.3 Withhold from first payment We see a demand for JIT-channels where the client specifies the channel-size. This is currently not possible on LSPS2 or LSPS4. See issue: BitcoinAndLightningLayerSpecs#65 # Breaking changes: Why (not) to merge this MR? We are in a bit of tension here. The spec is marked as `FOR IMPLEMENTATION` but we are seeing some usage on mainnet. This PR is breaking in the sense that clients will not be able to purchase channel if either the client or server uses this new revision. However, there will be some hassle for users but merging this MR should not put any funds at risk.

Suggested-By: 2Fast2Bcn

Co-authored-by: Elias Rohrer <[email protected]>

@tnull

This commit changes `LSPS1.get_info` response. # Don't wrap data in the options-field All data was wrapped in the `options`-field. This field is useless since [we removed the website from LSPS1](BitcoinAndLightningLayerSpecs#96). @tnull suggested to not use the field anymore. # Drop all payment related fields I had included an `supports_onchain_payment`-field. We don't include this data in `lsps1.get_info` anymore. The client has to create an order and can see the payment options subsequently. Suggested-By: Elias Rohrer <[email protected]>

An LSP should only include the `onchain` payment-option if it is willing to accept `onchain` payments for that order. Previously, we set the `onchain_address` to `null` to indicate that the LSP did not accept `onchain` payments. This is no longer needed.

The client should not rely on the LSP for transaction details of their own payments. Removing this fields simplifies the spec

ErikDeSmedt · 2024-06-14T12:15:39Z

Sorry for making you guys wait.
I've made a few updates and think the PR is in a mergeable state.

SeverinAlexB

One NIT otherwise looks good. Fingers crossed we can finalize soon.

SeverinAlexB · 2024-06-19T11:34:29Z

LSPS1/README.md

-    - `PAID` Lightning payment arrived, preimage released OR full `order_total_sat` on-chain payment arrived.
-    - `REFUNDED` Lightning payment or on-chain payment has been refunded.
+    - `PAID`  When the has been preimage released
+    - `REFUNDED` Lightning payment has been refunded.


Turn REFUNDED into CANCELED. If no payment arrived within the payment expiry time, setting it to REFUNDED would look weird.

I think we had this discussion before.

tnull

Updates look good I think, but do you have any thoughts on an order-wide expiry time?

tnull · 2024-06-19T11:47:20Z

LSPS1/README.md

+> Fees and expiry dates are defined on the level of the payment-option.
+> This allows an LSP to provide the cheapest service for each payment option.


@ErikDeSmedt Any further thoughts on this?

For lightning invoice the `REFUNDED` state existed. This name is not approriate cause in many scenario's the payment has never happened in the first place.

tnull

LGTM. Feel free to land as is.

tnull · 2024-06-20T11:55:57Z

LSPS1/README.md

+    "expires_at": "2025-01-01T00:00:00Z",
+    "fee_total_sat": 8888,
+    "order_total_sat": "200888",
+    "bolt11_invoice": "lnbc252u1p3aht9ysp580g4633gd2x9lc5al0wd8wx0mpn9748jeyz46kqjrpxn52uhfpjqpp5qgf67tcqmuqehzgjm8mzya90h73deafvr4m5705l5u5l4r05l8cqdpud3h8ymm4w3jhytnpwpczqmt0de6xsmre2pkxzm3qydmkzdjrdev9s7zhgfaqxqyjw5qcqpjrzjqt6xptnd85lpqnu2lefq4cx070v5cdwzh2xlvmdgnu7gqp4zvkus5zapryqqx9qqqyqqqqqqqqqqqcsq9q9qyysgqen77vu8xqjelum24hgjpgfdgfgx4q0nehhalcmuggt32japhjuksq9jv6eksjfnppm4hrzsgyxt8y8xacxut9qv3fpyetz8t7tsymygq8yzn05"


nit: This invoice doesn't match the given order_total_sat :P

ErikDeSmedt marked this pull request as ready for review March 4, 2024 13:59

SeverinAlexB added this to the Finalize LSPS1 milestone Mar 12, 2024

SeverinAlexB requested changes Mar 12, 2024

View reviewed changes

2Fast2BCn reviewed Apr 18, 2024

View reviewed changes

LSPS1/README.md Outdated Show resolved Hide resolved

ErikDeSmedt force-pushed the lsps1-multiple-payment-options branch from a7725db to d38f75e Compare April 22, 2024 08:45

tnull reviewed Apr 23, 2024

View reviewed changes

tnull reviewed Apr 24, 2024

View reviewed changes

SeverinAlexB approved these changes May 1, 2024

View reviewed changes

ErikDeSmedt and others added 10 commits June 14, 2024 13:12

Added newline

00aec69

Suggested-By: 2Fast2Bcn

Update LSPS1/README.md

a481b07

Co-authored-by: Elias Rohrer <[email protected]>

Update LSPS1/README.md

e9316ed

Co-authored-by: Elias Rohrer <[email protected]>

Fixes suggested by tnull

2d4e9c7

LSPS1: JSON formatting

0580b0a

LSPS1: Fix examples

250b1d7

Update LSPS1/README.md

3a80d2a

Co-authored-by: Elias Rohrer <[email protected]>

Fix typo

2757f2e

ErikDeSmedt force-pushed the lsps1-multiple-payment-options branch from 1aba755 to 2757f2e Compare June 14, 2024 11:39

ErikDeSmedt added 2 commits June 14, 2024 13:40

LSPS1: Remove onchain payment tracking

2b4fed9

The client should not rely on the LSP for transaction details of their own payments. Removing this fields simplifies the spec

SeverinAlexB reviewed Jun 19, 2024

View reviewed changes

tnull reviewed Jun 19, 2024

View reviewed changes

LSPS1: Rename REFUNDED to CANCELLED

6ac662e

For lightning invoice the `REFUNDED` state existed. This name is not approriate cause in many scenario's the payment has never happened in the first place.

ErikDeSmedt mentioned this pull request Jun 20, 2024

Lsps1 payment options expire on order #117

Closed

ErikDeSmedt force-pushed the lsps1-multiple-payment-options branch from bc04ed5 to 6ac662e Compare June 20, 2024 10:56

LSPS1: Use full lightning invoices

07a50a9

tnull approved these changes Jun 20, 2024

View reviewed changes

SeverinAlexB merged commit 76bc908 into BitcoinAndLightningLayerSpecs:main Jun 21, 2024

kaloudis mentioned this pull request Jun 21, 2024

LSP-101: LSPS1: Explicitly support LSPS1 payment options ZeusLN/zeus#2260

Merged

29 tasks

okjodom mentioned this pull request Jun 22, 2024

Socratic Seminar 8 : Topic Collection BitDevsNBO/bitdevsnbo.org#25

Closed

tnull mentioned this pull request Jun 23, 2024

LSPS1: Fix minor inconsistencies #118

Merged

tnull mentioned this pull request Jul 9, 2024

LSPS1: Fix more inconsistencies #119

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LSPS1: Explicitly support LSPS1 payment options #101

LSPS1: Explicitly support LSPS1 payment options #101

ErikDeSmedt commented Mar 4, 2024

SeverinAlexB left a comment

SeverinAlexB Mar 12, 2024

ErikDeSmedt Mar 12, 2024

SeverinAlexB Mar 13, 2024

ErikDeSmedt Mar 13, 2024 •

edited

Loading

tnull Apr 24, 2024

SeverinAlexB May 1, 2024

tnull Jun 19, 2024

tnull Apr 24, 2024

tnull Apr 24, 2024

tnull Apr 24, 2024

ErikDeSmedt Apr 24, 2024

ErikDeSmedt Jun 14, 2024

SeverinAlexB left a comment

SeverinAlexB May 1, 2024

tnull commented May 29, 2024

ErikDeSmedt commented Jun 14, 2024

SeverinAlexB left a comment

SeverinAlexB Jun 19, 2024

tnull left a comment

tnull Jun 19, 2024

tnull left a comment •

edited

Loading

tnull Jun 20, 2024

		> Fees and expiry dates are defined on the level of the payment-option.
		> This allows an LSP to provide the cheapest service for each payment option.

		@@ -119,6 +116,7 @@ The client MUST call `lsps1.get_info` first.
		- `max_channel_balance_sat` <[LSPS0.sat][]> Maximum channel size.
		- MUST be 0 or greater.

LSPS1: Explicitly support LSPS1 payment options #101

LSPS1: Explicitly support LSPS1 payment options #101

Conversation

ErikDeSmedt commented Mar 4, 2024

1. Use seprate fee for onchain vs lightning payment

2. Extensibility for additional payment methods

Breaking changes: Why (not) to merge this MR?

SeverinAlexB left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ErikDeSmedt Mar 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SeverinAlexB left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tnull commented May 29, 2024

ErikDeSmedt commented Jun 14, 2024

SeverinAlexB left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tnull left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tnull left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ErikDeSmedt Mar 13, 2024 •

edited

Loading

tnull left a comment •

edited

Loading