add cashu token melting to mutiny-node #1026

elnosh · 2024-02-12T12:34:04Z

following up on @oleonardolima outline on #1021.

mutiny-core/src/error.rs

mutiny-core/src/lib.rs

mutiny-wasm/Cargo.toml

mutiny-wasm/src/error.rs

benthecarman

really close, just a couple things

mutiny-core/src/lib.rs

oleonardolima

utACK

benthecarman

Can you squash the commits?

Just some nits, otherwise lgtm

benthecarman · 2024-02-15T15:31:10Z

mutiny-core/src/lib.rs

+                .clone()
+                .bolt11


nit: if you do .bolt11.clone() instead of .clone().bolt11 then it'll only clone the invoice instead of the whole struct.

benthecarman · 2024-02-15T15:31:51Z

mutiny-core/src/lib.rs

+            let invoice_amount = token.proofs.total_amount() as f64 * (1.0 - 3.0 / 100.0);
+
+            let mut mutiny_invoice = self
+                .create_invoice(invoice_amount as u64, vec!["Cashu Token Melt".to_string()])


Can you make "Cashu Token Melt" a const at the top of this file, we have one for SWAP as well

mutiny-core/src/lib.rs

benthecarman · 2024-02-15T15:35:04Z

mutiny-wasm/src/lib.rs

+        Ok(JsValue::from_serde(
+            &self.inner.melt_cashu_token(token).await?,


We need to convert this into mutiny-wasm's version of MutinyInvoice

Have to do something like this.

let result = self.inner.melt_cashu_token(token).await?; let invoices: Vec<MutinyInvoice> = result.into_iter().map(|i| i.into()).collect(); Ok(JsValue::from_serde(&invoices)?)

benthecarman · 2024-02-15T17:14:54Z

mutiny-core/src/lib.rs

+            log_debug!(
+                self.logger,
+                "will need to add extra outputs for overpaid fees"
+            );


sorry, can we log the amount here as well.

should be something like

let amt = post_melt_bolt11_response.change.iter().map(|s|s.amount).sum();

you can add it to the log by just putting "{amt}"

benthecarman

lgtm

TonyGiorgio · 2024-02-15T17:52:11Z

Cargo.lock

+]
+
+[[package]]
+name = "libsqlite3-sys"


Eww, this adds another 4MB? Does mocha really need to pull in sqlite?

TonyGiorgio · 2024-02-15T17:54:29Z

Cargo.lock

+]
+
+[[package]]
+name = "moksha-wallet"


This pulls in a lot of dependencies. What is in moksha wallet that isn't in core that we need?

moksha-core is providing the basic crypto implementation and primitives. Moksha-wallet adds a higher abstraction layer for working with a mint and provides methods for minting, melting, exporting etc. Since v1 cashu is using quotes for all the apis. The wallet hides this complexity and also adds multiple implementations for desktop/web httpclients and databases for storing the tokens. If you are not interested in storing tokens and just want to redeem them for a bolt11 invoice, using moksha-core might be the right choice.

TonyGiorgio · 2024-02-15T17:56:57Z

mutiny-core/src/lib.rs

+        &self,
+        token_v3: TokenV3,
+    ) -> Result<Vec<MutinyInvoice>, MutinyError> {
+        let mint_client = CrossPlatformHttpClient::new();


If moksha_wallet is just for an HTTP client, can we get rid of it by just pulling in our current http library?

you are right. I think we can get rid of moksha-wallet and make the http calls to the mint without it.

TonyGiorgio · 2024-02-15T17:58:07Z

mutiny-core/src/lib.rs

+                None => return Err(MutinyError::EmptyMintURLError),
+            };
+
+            let invoice_amount = token.proofs.total_amount() as f64 * (1.0 - 3.0 / 100.0);


What's the purpose of 1.0 - 3.0 / 100.0 ?

the amount for the invoice needs to be a bit lower than the actual amount of the cashu token to account for potential fees. Not sure on best way to calculate it but landed on that number after discussing with @benthecarman

TonyGiorgio · 2024-02-15T18:01:12Z

mutiny-core/src/lib.rs

+            log_debug!(
+                self.logger,
+                "will need to add extra outputs for {amt} for overpaid fees"
+            );


I'm a little confused why this debug is here. Should this be a warning? And does it always occur?

@benthecarman commented to add that log that could be for a follow-up. Follow up could be to add outputs param to the post_melt_bolt call in order to get the overpaid fees if any. Or if we create an invoice for "right" amount, we could get rid of this.

mutiny-core/src/lib.rs

benthecarman · 2024-02-19T14:05:03Z

looks like you have some rebase conflicts in the cargo.lock, I would just revert the changes in there and then compile again, let cargo fix it for you

benthecarman · 2024-02-19T14:07:54Z

mutiny-core/src/lib.rs

+                    .mint_post(
+                        &mint_url.join("/v1/melt/quote/bolt11")?,
+                        json!(quote_request),
+                    )


can make this a dedicated function on the cashu client. This way you can just pass the mint url and a PostMeltQuoteBolt11Request. Will make things easier to use in the future and we could eventually mock it for tests too

benthecarman · 2024-02-19T14:08:03Z

mutiny-core/src/lib.rs

+            let post_melt_bolt11_response: PostMeltBolt11Response = self
+                .cashu_client
+                .mint_post(&mint_url.join("v1/melt/bolt11")?, json!(melt_request))
+                .await?;


should do the same for this function

benthecarman · 2024-02-19T14:08:42Z

mutiny-core/src/lib.rs

+
+            let total_proofs_amount = token.proofs.total_amount();
+            // create invoice for 1% less than proofs amount
+            let mut invoice_amount = total_proofs_amount as f64 * (1.0 - 1.0 / 100.0);


this would be more readable as
let mut invoice_amount = total_proofs_amount as f64 * 0.99;

TonyGiorgio

Looks really good! Nice work.

The only question/comment I have on is around the multiple token thing. Is that common? If so, I wonder if all of the errors in the for loop should just result in a continue and if a new PartialCashuError could be introduced. If it's not very common, then I think it's fine to leave it as is.

elnosh · 2024-02-20T12:06:10Z

thanks! I believe the EmptyMintURLError in the for loop should not be very common. In NUT-00 https://github.com/cashubtc/nuts/blob/main/00.md https://github.com/cashubtc/nuts/blob/main/00.md#03---methods the mint url is not optional in the token so I'm thinking it should always be there.

The only question/comment I have on is around the multiple token thing. Is that common? If so, I wonder if all of the errors in the for loop should just result in a continue and if a new PartialCashuError could be introduced. If it's not very common, then I think it's fine to leave it as is.

benthecarman · 2024-02-21T10:14:36Z

mutiny-core/Cargo.toml

@@ -56,6 +56,7 @@ fedimint-mint-client = { git = "https://github.com/fedimint/fedimint", rev = "6a
 fedimint-ln-client = { git = "https://github.com/fedimint/fedimint", rev = "6a923ee10c3a578cd835044e3fdd94aa5123735a" }
 fedimint-bip39 = { git = "https://github.com/fedimint/fedimint", rev = "6a923ee10c3a578cd835044e3fdd94aa5123735a" }
 fedimint-ln-common = { git = "https://github.com/fedimint/fedimint", rev = "6a923ee10c3a578cd835044e3fdd94aa5123735a" }
+moksha-core = { git = "https://github.com/ngutech21/moksha", rev = "0b9a32d216ba0782a4aa2cdb19a87589e16d9089" }


I updated waila to the commit 18d99977965662d46ccec29fecdb0ce493745917, can you do the same here so their in sync, then we should be good to go

elnosh marked this pull request as draft February 12, 2024 12:35

oleonardolima mentioned this pull request Feb 12, 2024

feat: add cashu token melting (redeem) to mutiny-node #1021

Closed

benthecarman requested changes Feb 12, 2024

View reviewed changes

elnosh marked this pull request as ready for review February 12, 2024 18:14

benthecarman reviewed Feb 12, 2024

View reviewed changes

mutiny-core/src/lib.rs Outdated Show resolved Hide resolved

mutiny-core/src/lib.rs Show resolved Hide resolved

elnosh force-pushed the receive-cashu branch from 3446201 to 693c31f Compare February 13, 2024 09:43

oleonardolima approved these changes Feb 13, 2024

View reviewed changes

elnosh mentioned this pull request Feb 15, 2024

redeem cashu token MutinyWallet/mutiny-web#885

Closed

benthecarman reviewed Feb 15, 2024

View reviewed changes

elnosh force-pushed the receive-cashu branch from 693c31f to 5715450 Compare February 15, 2024 16:46

elnosh requested a review from benthecarman February 15, 2024 16:51

benthecarman reviewed Feb 15, 2024

View reviewed changes

elnosh force-pushed the receive-cashu branch from 5715450 to 547c246 Compare February 15, 2024 17:21

benthecarman approved these changes Feb 15, 2024

View reviewed changes

benthecarman requested a review from TonyGiorgio February 15, 2024 17:37

TonyGiorgio requested changes Feb 15, 2024

View reviewed changes

elnosh force-pushed the receive-cashu branch from 9e988e3 to 31b88eb Compare February 17, 2024 15:19

benthecarman reviewed Feb 17, 2024

View reviewed changes

mutiny-core/src/lib.rs Outdated Show resolved Hide resolved

mutiny-core/src/lib.rs Outdated Show resolved Hide resolved

mutiny-core/src/lib.rs Show resolved Hide resolved

elnosh force-pushed the receive-cashu branch from 018ce0d to 73031be Compare February 18, 2024 22:06

benthecarman reviewed Feb 19, 2024

View reviewed changes

elnosh force-pushed the receive-cashu branch from 73031be to 0ca90b0 Compare February 20, 2024 11:04

TonyGiorgio approved these changes Feb 20, 2024

View reviewed changes

TonyGiorgio requested a review from benthecarman February 20, 2024 11:42

benthecarman approved these changes Feb 20, 2024

View reviewed changes

benthecarman requested changes Feb 21, 2024

View reviewed changes

elnosh added 2 commits February 21, 2024 21:56

add cashu melting method

b43b810

update moksha

908851d

elnosh force-pushed the receive-cashu branch from 0ca90b0 to 908851d Compare February 21, 2024 22:11

TonyGiorgio requested a review from benthecarman February 22, 2024 11:16

benthecarman approved these changes Feb 22, 2024

View reviewed changes

benthecarman merged commit a9816e9 into MutinyWallet:master Feb 22, 2024
9 checks passed

elnosh deleted the receive-cashu branch April 12, 2024 16:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add cashu token melting to mutiny-node #1026

add cashu token melting to mutiny-node #1026

elnosh commented Feb 12, 2024

benthecarman left a comment

oleonardolima left a comment

benthecarman left a comment

benthecarman Feb 15, 2024

benthecarman Feb 15, 2024

benthecarman Feb 15, 2024

benthecarman Feb 15, 2024

benthecarman left a comment

TonyGiorgio Feb 15, 2024

TonyGiorgio Feb 15, 2024

ngutech21 Feb 16, 2024

TonyGiorgio Feb 15, 2024

elnosh Feb 15, 2024

TonyGiorgio Feb 15, 2024

elnosh Feb 15, 2024

TonyGiorgio Feb 15, 2024

elnosh Feb 15, 2024

benthecarman commented Feb 19, 2024

benthecarman Feb 19, 2024

benthecarman Feb 19, 2024

benthecarman Feb 19, 2024

TonyGiorgio left a comment

elnosh commented Feb 20, 2024

benthecarman Feb 21, 2024

		Ok(JsValue::from_serde(
		&self.inner.melt_cashu_token(token).await?,

add cashu token melting to mutiny-node #1026

add cashu token melting to mutiny-node #1026

Conversation

elnosh commented Feb 12, 2024

benthecarman left a comment

Choose a reason for hiding this comment

oleonardolima left a comment

Choose a reason for hiding this comment

benthecarman 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

Choose a reason for hiding this comment

benthecarman 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

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

benthecarman commented Feb 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TonyGiorgio left a comment

Choose a reason for hiding this comment

elnosh commented Feb 20, 2024

Choose a reason for hiding this comment