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

feat: update gonuts dependency #962

Merged
merged 24 commits into from
Jan 30, 2025
Merged

feat: update gonuts dependency #962

merged 24 commits into from
Jan 30, 2025

Conversation

elnosh
Copy link
Contributor

@elnosh elnosh commented Jan 8, 2025

Closes #890
Closes #651

I have made changes for new gonuts version, however not pointing to v0.3.0 but instead to latest commit in main for now since I have changed some things since v0.3.0.

This should handle pending proofs that are stuck in payments and hence not try to use them in other payments.

Another change for this new version is that it does not have an Invoice type anymore and instead MintQuote and MeltQuotes so it has to aggregate those 2 when listing transactions.

Regarding point 4 in #890, I think doing a restore if it finds any pending proofs during startup could be undesirable since the restore does not restore previous transaction history. So any time there are pending proofs and the hub starts, it will wipe out the transaction history. Maybe a cashu-backend specific button could be added in Settings > debug tools where if the user experienced the issue of a unusable wallet due to stuck payments, they could trigger a manual restore?

Regarding this issue #651, I know it's not ideal not having the preimage for incoming payments and one is expected but maybe a dummy/placeholder one could be used so that the hub can mark it as settled and show up in the transaction history and don't go into that loop where it would always check that invoice even though it already got paid.

Any thoughts or comments on the changes and suggestions made here are appreciated :)

@rolznz
Copy link
Contributor

rolznz commented Jan 9, 2025

Thanks for picking this up @elnosh !

Regarding point 4 in #890, I think doing a restore if it finds any pending proofs during startup could be undesirable since the restore does not restore previous transaction history. So any time there are pending proofs and the hub starts, it will wipe out the transaction history. Maybe a cashu-backend specific button could be added in Settings > debug tools where if the user experienced the issue of a unusable wallet due to stuck payments, they could trigger a manual restore?

That sounds better, but I would prefer to not leak individual LNClient-specific code to other parts of the app. It might be best to have a generic "Execute Node Command" button in the debug tools where you can type some text, and then the Cashu implementation can parse it and do what it needs. We originally planned to do something like this but did not yet add it. I will see what we can do and get back to you. CC @rdmitr

Regarding this issue #651, I know it's not ideal not having the preimage for incoming payments and one is expected but maybe a dummy/placeholder one could be used so that the hub can mark it as settled and show up in the transaction history and don't go into that loop where it would always check that invoice even though it already got paid.

This sounds like a good workaround to me!

@rolznz
Copy link
Contributor

rolznz commented Jan 9, 2025

@elnosh for now temporarily we could put the reset logic in this function just to test it, and not block this PR:

func (cs *CashuService) ResetRouter(key string) error {
	return nil
}

It's not so good because it's unrelated to resetting the wallet, but it is executed from the dev tools page.

@elnosh
Copy link
Contributor Author

elnosh commented Jan 10, 2025

made some other changes:

  • saving mnemonic from cashu wallet
  • test cashu wallet restore from the ResetRouter in the debug tools
  • added dummy preimage for mint quotes so that those get marked as settled.

@rolznz
Copy link
Contributor

rolznz commented Jan 15, 2025

Adding for reference, if a user gets a message no mint URL was set, we removed it in a previous PR as we incorrectly set an unstable cashu test mint as the default #535

@rolznz
Copy link
Contributor

rolznz commented Jan 15, 2025

I found an old cashu wallet for Alby Hub with mint https://8333.space:3338 which is operational (I can pay with my old balance) and I can send payments, but when I pay I get "no preimage in melt response". My balance then does not update after paying. Is this expected?

Update: I paid some more and now my balance says 33 sats - but it is not, I can no longer pay.

I managed to restore the proofs at https://wallet.cashu.me/ - nice! afterwards, I could still pay with my hub (I thought the tokens would have become invalid).

@rolznz
Copy link
Contributor

rolznz commented Jan 15, 2025

Starting a brand new hub with a new cashu wallet seems to work well.

Restoring my sats on the old wallet with the incorrect balance changed it from 33 to 11 sats.

With minibits I get the balance issue.
I deposited 10 sats, then paid 1 sat. Now my balance is 12. Maybe just a calculation bug?

@elnosh
Copy link
Contributor Author

elnosh commented Jan 15, 2025

https://8333.space:3338 which is operational (I can pay with my old balance) and I can send payments, but when I pay I get "no preimage in melt response". My balance then does not update after paying. Is this expected?

I've been testing with some mints (including that one) and I think that happens if you try to pay an invoice that was generated from that same mint. Looking at the response from the melt request, the payment is being settled and hence returning PAID but with an empty preimage and then making this check fail.

if meltResponse == nil || meltResponse.Preimage == "" {
	return nil, errors.New("no preimage in melt response")
}

I created issue for that here: cashubtc/nutshell#625
In other cases where I'm trying to pay a different lightning wallet or invoice from different mint, the preimage is there. Instead of checking the preimage we could check the melt state for "PAID" but not sure if you would want the empty preimage in those specific cases when it is returned empty.

@elnosh
Copy link
Contributor Author

elnosh commented Jan 15, 2025

Restoring my sats on the old wallet with the incorrect balance changed it from 33 to 11 sats.

11 should be the correct balance given some previous payments?

With minibits I get the balance issue. I deposited 10 sats, then paid 1 sat. Now my balance is 12. Maybe just a calculation bug?

Sorry, I didn't understand this. Could you explain a bit further? You deposited 10 sats to an empty wallet set with minibits mint and got this?

@rolznz
Copy link
Contributor

rolznz commented Jan 16, 2025

Sorry, I didn't understand this. Could you explain a bit further? You deposited 10 sats to an empty wallet set with minibits mint and got this?

Yes, I deposited 10 sats into an empty wallet with minibits mint.

Then I did a 1 sat payment to [email protected]. I guess the fee was 1 sat.

Instead of decreasing the balance to 8 sats (expected) it increased to 12 sats.

@rolznz
Copy link
Contributor

rolznz commented Jan 16, 2025

I created issue for that here: cashubtc/nutshell#625 In other cases where I'm trying to pay a different lightning wallet or invoice from different mint, the preimage is there. Instead of checking the preimage we could check the melt state for "PAID" but not sure if you would want the empty preimage in those specific cases when it is returned empty.

It seems more than this - because in all my test payments I paid to my LDK hub node. I did not pay to the same mint.

@rolznz
Copy link
Contributor

rolznz commented Jan 16, 2025

11 should be the correct balance given some previous payments?

I think so, yes. It just seems something is broken when we make payments, that the balance increases rather than decreases, or it does not omit already used cashu tokens

@elnosh
Copy link
Contributor Author

elnosh commented Jan 16, 2025

Then I did a 1 sat payment to [email protected]. I guess the fee was 1 sat.
Instead of decreasing the balance to 8 sats (expected) it increased to 12 sats.

It seems more than this - because in all my test payments I paid to my LDK hub node. I did not pay to the same mint.

I haven't been able to replicate those issues :(

I've setup one with minibits mint and have been making payments fine (with preimage in the response). Sent to this address [email protected] and some other nodes and mints and balance has been updating correctly.

@rolznz
Copy link
Contributor

rolznz commented Jan 17, 2025

image

I cannot figure out why mine is like this, I sent you my cashu wallet in case that helps you

update: maybe I was on the wrong branch or some other issue. I could not reproduce it a second time after re-checking out this branch and using a fresh work directory.

@rolznz
Copy link
Contributor

rolznz commented Jan 29, 2025

Added some improvements here: elnosh#1

@rolznz
Copy link
Contributor

rolznz commented Jan 30, 2025

Just waiting on #1007 to be merged.

@rolznz rolznz marked this pull request as ready for review January 30, 2025 11:33
Copy link
Contributor

@rolznz rolznz left a comment

Choose a reason for hiding this comment

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

tACK

@rolznz rolznz merged commit 1c19ddf into getAlby:master Jan 30, 2025
7 of 9 checks passed
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.

Cashu: handle pending proofs Payments to Cashu backend do not appear in transaction history
3 participants