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

NUT-XX: Multinut payments #103

Merged
merged 15 commits into from
May 22, 2024
Merged

NUT-XX: Multinut payments #103

merged 15 commits into from
May 22, 2024

Conversation

callebtc
Copy link
Contributor

@callebtc callebtc commented Mar 23, 2024

Enables https://twitter.com/callebtc/status/1766116631795662921

Accidentally included NUT-08 and NUT-11 header tag edits.

@callebtc callebtc changed the title NUT-14: Draft Multinut payments NUT-14: Multinut payments Mar 23, 2024
@callebtc
Copy link
Contributor Author

I would assume that the on-chain feature by @ngutech21 also includes an amount input in the MeltRequest, is that so? Is there some possible confusion?

Also, I'm wondering if this should be a separate endpoint or not. Since it's only a single element added in the request, I went with the current bolt11 melt endpoint since I thought it fits well. Happy about anyone's thoughts.

@gandlafbtc
Copy link
Collaborator

Is there a time limit on how long these payments can take? Or how/when is a payment considered failed?

@elnosh
Copy link
Contributor

elnosh commented Mar 23, 2024

If I'm constructing a multinut payment from 3 different mints and 2 of those succeed but one fails, the entire multinut payment will fail. But the 2 partial payments from the mints that succeeded, those mints will invalidate the proofs provided if their portion of the melt request succeeds. Will there be a way for wallets to "recover" those proofs from the partial mint payments that succeeded but overall payment fails?

@callebtc
Copy link
Contributor Author

Is there a time limit on how long these payments can take? Or how/when is a payment considered failed?

Yes, very good point! The invoice expiry is the same as usual. However, the MPP payment has a default timeout of around 1-3 minutes in LND/CLN I think. That means that once you initiate the payment from one mint, you have 1-3 minutes to do it with all other mints as well. I think all implementations allow as many retries as you want, if the timeout should be reached and the payment gets rejected (that's my understanding). A comment about that should be added to the document.

@callebtc
Copy link
Contributor Author

callebtc commented Mar 23, 2024

If I'm constructing a multinut payment from 3 different mints and 2 of those succeed but one fails, the entire multinut payment will fail. But the 2 partial payments from the mints that succeeded, those mints will invalidate the proofs provided if their portion of the melt request succeeds. Will there be a way for wallets to "recover" those proofs from the partial mint payments that succeeded but overall payment fails?

Like with normal Lightning payments / melts, a mint should only invalidate the proofs, if the payment was successful. That means, for a failed MPP payments, all nuts across all mints will be "unspent" again (and not pending anymore) and can be reused.

@callebtc callebtc mentioned this pull request Mar 24, 2024
3 tasks
14.md Outdated
{
"request": <str>,
"unit": <str_enum["sat"]>,
"amount": <int> # <-- new
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like the overall idea of extending NUT-05 by adding a single field, instead of copying all the endpoints from NUT-05 and creating a new payment-method.
To make this backwards compatible the amount should be optional

<str|null>

otherwise this would break the api, because the PostMeltQuoteBolt11Request will be used for both endpoints NUT-05 (without mpp field) and NUT-14 (with mpp field).

14.md Outdated

## Settings

The settings indicating that a mint supports this feature are in the `MeltMethodSetting` (see [NUT-05][05]) which is returned in the info endpoint ([NUT-06][06]). The mint MUST indicate `method` and `unit` pairs that support `mpp`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should the mpp field be added to the settings from NUT-05 or would it be better to create new settings for NUT-14? Having explicit settings for NUT-14 would better fit in the overall picture, but would also add some redundancy. Would a mint always return the same min_max amounts, units etc. for both cases mpp=true and mpp=false or could there be differences?
If the mpp field will be part of the NUT-05 settings it has to be optional, otherwise it would break the api.

@ngutech21
Copy link
Collaborator

I would assume that the on-chain feature by @ngutech21 also includes an amount input in the MeltRequest, is that so? Is there some possible confusion?

Yes the PostMeltQuoteOnchainRequest contains an amount field in my implementation. What do you mean by confusion?

@callebtc
Copy link
Contributor Author

Yes the PostMeltQuoteOnchainRequest contains an amount field in my implementation. What do you mean by confusion?

Ok that's what I thought!

One possible source of future confusion between might be between partial MPPs and if we would like to support amountless invoices in the future. For amountless invoices, we would also need to pass an amount field.

Maybe it's not too bad though, since the mint usually needs to treat amountless invoices differently (as opposed to invoices with amount).

Pseudocode of what I'm trying to say:

if meltRequest.amount:
  if invoice.amount:
    pay_mpp(invoice, amount)
  else:
    pay_amountless(invoice, amount)
else:
  pay_normal(invoice)

@ngutech21
Copy link
Collaborator

Yes the PostMeltQuoteOnchainRequest contains an amount field in my implementation. What do you mean by confusion?

Ok that's what I thought!

One possible source of future confusion between might be between partial MPPs and if we would like to support amountless invoices in the future. For amountless invoices, we would also need to pass an amount field.

Here is a solution that can be extended in the future without breaking the Api and being backwards compatible at the same time: Instead of adding an amount we add an optional payment_type. This is more explicit than deriving something from the amount field. In NUT-14 it would look like this:

{
  "request": "lnbc100u1p0n7j7..",
  "unit": "sat",
  "payment_type": {
    "mpp": {
      "amount": 100
    }
  }
}

for no amount bolt11 invoices it would look like this:

{
  "request": "lnbc100u1p0n7j7..",
  "unit": "sat",
  "payment_type": "no_amount"
}

Rust code:

#[derive(Deserialize, Serialize, Debug, Clone)]
pub struct PostMeltQuoteBolt11Request {
    pub request: String,
    pub unit: CurrencyUnit,
    #[serde(skip_serializing_if = "Option::is_none")]
    pub payment_type: Option<Bolt11PaymentType>,
}

#[derive(Deserialize, Serialize, Debug, Clone)]
pub enum Bolt11PaymentType {
    #[serde(rename = "mpp")]
    Mpp { amount: u64 },
    #[serde(rename = "no_amount")]
    NoAmount,
}

We just have to agree on that the payment_type may only contain one entry, since Json does not support arithmetic datatypes.

Would this work well in python and typescript?

@callebtc
Copy link
Contributor Author

This NUT could be extended to support on-chain in the future, #107. Happy to treat it as out of scope for this PR.

@callebtc
Copy link
Contributor Author

Instead of adding an amount we add an optional payment_type.

Interesting approach. The no_amount case also would need an amount input since it refers to an amountless bolt11 invoice that where the payer needs to specify the amount herself.

@callebtc callebtc changed the title NUT-14: Multinut payments NUT-XX: Multinut payments Apr 30, 2024
@callebtc
Copy link
Contributor Author

callebtc commented May 4, 2024

What if we call this payment_option and use the following for this PR:

{
  "request": "lnbc100u1p0n7j7..",
  "unit": "sat",
  "payment_option": {
    "mpp": {
      "amount": 69
    }
  }
}

That means, for a future change where we would like to support amountless invoices, we could use

{
  "request": "lnbc<amountless>p0n7j7..",
  "unit": "sat",
  "payment_option": {
    "amountless": {
      "amount": 100
    }
  }
}

@callebtc
Copy link
Contributor Author

callebtc commented May 11, 2024

As of af13a90, the PostMeltQuoteBolt11Request now reads:

{
  "request": <str>,
  "unit": <str_enum["sat"]>,
  "options": {
    "mpp": {
      "amount": <int>
    }
  }
}

The setting for this NUT (example: NUT-15) would be MultipathPaymentSetting:

{
  "15": {
    [
      {
        "method": "bolt11",
        "unit": "sat",
        "mpp": true
      },
      {
        "method": "bolt11",
        "unit": "usd",
        "mpp": true
      },    
    ]
  }
}

I would like to port these changes to nutshell before merging this PR.

@callebtc callebtc added ready Ready to merge needs implementation Needs a reference implementation labels May 11, 2024
@callebtc callebtc merged commit 8dddf1e into main May 22, 2024
@callebtc callebtc deleted the nut-14-mpp branch May 22, 2024 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs implementation Needs a reference implementation ready Ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants