-
Notifications
You must be signed in to change notification settings - Fork 45
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
Changes from 6 commits
2f4252e
f672c05
c57ef58
6467fe2
4025996
2ea655f
e3ea35d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,32 @@ impl Mint { | |
} | ||
} | ||
|
||
fn check_amount_less_invoice( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
&self, | ||
amount: Amount, | ||
request_amount: Amount, | ||
unit: CurrencyUnit, | ||
method: PaymentMethod, | ||
) -> Result<(), Error> { | ||
let nut05 = &self.mint_info.nuts.nut05; | ||
|
||
if nut05.disabled { | ||
return Err(Error::MeltingDisabled); | ||
} | ||
let settings = nut05 | ||
.get_settings(&unit, &method) | ||
.ok_or(Error::UnitUnsupported)?; | ||
|
||
if settings | ||
.amountless | ||
.expect("expect a value for amountless invoice") | ||
mubarak23 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
&& amount != request_amount | ||
{ | ||
return Err(Error::AmountLessNotAllowed); | ||
} | ||
Ok(()) | ||
} | ||
|
||
/// Get melt bolt11 quote | ||
#[instrument(skip_all)] | ||
pub async fn get_melt_bolt11_quote( | ||
|
@@ -58,8 +84,12 @@ impl Mint { | |
request, | ||
unit, | ||
options: _, | ||
amount, | ||
} = 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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
Some(mpp_amount) => mpp_amount.amount, | ||
None => { | ||
|
@@ -93,6 +123,14 @@ impl Mint { | |
Error::UnitUnsupported | ||
})?; | ||
|
||
// check amountless (amount in meltbolt11request) is equal to payment_quote.amount | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if mint is setting the amount coming from the option field in |
||
self.check_amount_less_invoice( | ||
amount_less_amount, | ||
payment_quote.amount, | ||
unit.clone(), | ||
PaymentMethod::Bolt11, | ||
)?; | ||
|
||
let quote = MeltQuote::new( | ||
request.to_string(), | ||
unit.clone(), | ||
|
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.
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.
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.
can you explain further on "The ln backends need to be modified to support it." @thesimplekid