add internal_melts_only to MintInfo and cache mint info#658
add internal_melts_only to MintInfo and cache mint info#658
internal_melts_only to MintInfo and cache mint info#658Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
4b48af0 to
0fcc1e1
Compare
| /** | ||
| * Whether the mint only allows internal melts. | ||
| * | ||
| * NOTE: This flag is not currently defined in the NUTs. |
There was a problem hiding this comment.
is this something we will suggest them to add?
There was a problem hiding this comment.
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.
app/lib/cashu/mint-info.ts
Outdated
There was a problem hiding this comment.
do we need to copy the entire class? can't we just extend the existing one to add what we need?
There was a problem hiding this comment.
See my comment at the top of this file. The class is not exported
There was a problem hiding this comment.
I see. should we maybe patch the lib instead? or this is safer?
There was a problem hiding this comment.
I don't know how to patch cashu-ts because its all minified
There was a problem hiding this comment.
I would then just call it ExtendedMintInfo so it's always clear it's our extended class
There was a problem hiding this comment.
Done. We can remove most of this and just extend MintInfo after we upgrade to cashu-ts v3
app/lib/cashu/utils.ts
Outdated
There was a problem hiding this comment.
why is this public? shouldn't public always use getExtendedMintInfo?
There was a problem hiding this comment.
I did this so that it can be sync. getExtendedMintInfo is async
There was a problem hiding this comment.
See where its used here. It would be annoying if this was async.
There was a problem hiding this comment.
can it currently happen then you call it and it hasn't been cached yet?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
so how do we ensure that when that isStar method is called, we will have it loaded?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
a490162 to
40972cc
Compare
|
@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 |
I'm doing this to prepare for the Loyalty Points PR.
1. Extended MintInfo class with
internal_melts_onlyFor loyalty points we will classify a mint based on whether the
internal_melts_onlyflag 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_onlyconfig a lot, so it seemed nice to make getting the mint info sync.