Skip to content

add internal_melts_only to MintInfo and cache mint info#658

Closed
gudnuf wants to merge 1 commit intomasterfrom
cache-mint-info
Closed

add internal_melts_only to MintInfo and cache mint info#658
gudnuf wants to merge 1 commit intomasterfrom
cache-mint-info

Conversation

@gudnuf
Copy link
Contributor

@gudnuf gudnuf commented Sep 30, 2025

I'm doing this to prepare for the Loyalty Points PR.

1. Extended MintInfo class with internal_melts_only

For loyalty points we will classify a mint based on whether the internal_melts_only flag is set. Cashu-ts obviously doesn't have this, so I copied the existing MintInfo class then added the necessary methods. This makes it easier to modify the expected mint info as we are running our own mints.

2. Cache mint info in CashuWallet

We already initialize our wallets with mint info, and for loyalty points, we read this internal_melts_only config a lot, so it seemed nice to make getting the mint info sync.

@gudnuf gudnuf requested a review from jbojcic1 September 30, 2025 04:45
@gudnuf gudnuf self-assigned this Sep 30, 2025
@vercel
Copy link

vercel bot commented Sep 30, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
agicash Ready Ready Preview Comment Oct 21, 2025 10:12pm

@supabase
Copy link

supabase bot commented Sep 30, 2025

This pull request has been ignored for the connected project hrebgkfhjpkbxpztqqke because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

This was referenced Sep 30, 2025
@gudnuf gudnuf added the P2 Medium priority label Oct 9, 2025
/**
* Whether the mint only allows internal melts.
*
* NOTE: This flag is not currently defined in the NUTs.
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this something we will suggest them to add?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possibly, we would need to get it added to the spec. I haven't mentioned this idea to anyone else, but once we have a working demo I was going to show them and propose this new flag.

Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need to copy the entire class? can't we just extend the existing one to add what we need?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See my comment at the top of this file. The class is not exported

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see. should we maybe patch the lib instead? or this is safer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know how to patch cashu-ts because its all minified

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would then just call it ExtendedMintInfo so it's always clear it's our extended class

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. We can remove most of this and just extend MintInfo after we upgrade to cashu-ts v3

Copy link
Collaborator

Choose a reason for hiding this comment

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

why is this public? shouldn't public always use getExtendedMintInfo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did this so that it can be sync. getExtendedMintInfo is async

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See where its used here. It would be annoying if this was async.

Copy link
Collaborator

Choose a reason for hiding this comment

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

can it currently happen then you call it and it hasn't been cached yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea because mintInfo is optional, and we want it to be optional so that we can use this class without having to fetch mint info which we do in a couple places.

Copy link
Collaborator

Choose a reason for hiding this comment

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

so how do we ensure that when that isStar method is called, we will have it loaded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess my thinking was that our accounts will always have it loaded unless offline (which just got merged with #672, so I can update this to not check if the account is offline). Then if we try to use the isStar method in a place where its not loaded it will throw while testing. Is there a different patter that I could use? We could just return undefined instead of throwing if mint info is not loaded. Btw cashu-ts@v3 has getMintInfo as sync and throws if not loaded

Copy link
Contributor Author

@gudnuf gudnuf Oct 21, 2025

Choose a reason for hiding this comment

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

I removed it because I'm changing the way that I check for star accounts or whatever we call it. I'm just going to add a flag to the account type instead of calling isStarAccount everytime

@gudnuf
Copy link
Contributor Author

gudnuf commented Jan 27, 2026

@jbojcic1 I'm just gonna close this for now. Adding the account purpose in #766 does this by setting the value when we load mint info in toAccount. We can come back to this is we want better sync access to mint info.

btw the way that I copied and extended MintInfo is this PR shouldn't be needed with cashu-ts v3. I haven't checked, but a while ago they said they will make sure MintInfo is exported so we can directly extend the type if we need

@gudnuf gudnuf closed this Jan 27, 2026
@gudnuf gudnuf deleted the cache-mint-info branch January 29, 2026 23:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Medium priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments