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

Melt token with amountless #468

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

mubarak23
Copy link
Contributor

@mubarak23 mubarak23 commented Nov 21, 2024

Description

Base on the current Open PR on nuts: cashubtc/nuts#173

this PR aim at closing issu #460

mint can accept amountless invoice when melting token,

added setting in MeltMethodSettings for amount_less also handle cases where we are trying to melt token and amount_less invoice is provvided

Checklist

crates/cdk/src/mint/melt.rs Outdated Show resolved Hide resolved
crates/cdk/src/nuts/nut05.rs Outdated Show resolved Hide resolved
crates/cdk/src/nuts/nut05.rs Outdated Show resolved Hide resolved
crates/cdk-mintd/src/main.rs Outdated Show resolved Hide resolved
@@ -148,6 +148,7 @@ async fn main() -> anyhow::Result<()> {
CurrencyUnit::Sat,
PaymentMethod::Bolt11,
mint_melt_limits,
Some(true),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Changing it here just singles support for it but it doesn't actually add support for it. The ln backends need to be modified to support it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can you explain further on "The ln backends need to be modified to support it." @thesimplekid

@@ -48,6 +48,32 @@ impl Mint {
}
}

fn check_amount_less_invoice(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This function is mostly a duplicate of the one above it, the check for is amountless is supported can just be added there.

crates/cdk/src/mint/melt.rs Outdated Show resolved Hide resolved
} = melt_request;

let amount_less_amount = amount.unwrap_or_default();
// let amount_less_amount = amount.unwrap_or_else(|| Amount::new(0));

let amount = match melt_request.options {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If this is executed on an invoice without an amount defined it will return an error. If we get an invoice without an amount we need to use the once defined in the request.

@@ -93,6 +123,14 @@ impl Mint {
Error::UnitUnsupported
})?;

// check amountless (amount in meltbolt11request) is equal to payment_quote.amount
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need to check this, shouldn't we be setting the amount?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you mean mint return an amount,

if this is the case, mint does not know the amount of ecash a wallet making the melt qoute request has.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if mint is setting the amount coming from the option field in MeltQuoteBolt11Request then , we need to check to ensure only mint that support amountless payment can handle this.

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