Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
💡 Enable Vercel Agent with $100 free credit for automated AI reviews |
| } | ||
| } | ||
|
|
||
| export const accountOfflineToast = { |
There was a problem hiding this comment.
should this maybe go into accounts feature?
| inputValue: Money, | ||
| convertedValue: Money | undefined, | ||
| ) => { | ||
| if (!sendAccount.isOnline) { |
There was a problem hiding this comment.
I wonder if this belongs to getQuote instead and then we could just throw the domain error instead maybe?
| className="w-[100px]" | ||
| onClick={() => reverseTransaction({ transaction })} | ||
| onClick={() => { | ||
| if (!account.isOnline) { |
There was a problem hiding this comment.
would it make more sense to still let it try and then handle network error? or that is problematic because network error only happens when background processing?
There was a problem hiding this comment.
I think that works, in this case it will throw the network error when wallet.getKeys is called from CashuTokenSwapService.create. I will try. If we do this, we just have to make sure the whatever we are creating that depends on the mint being online throws before anything is actually created in our db.
There was a problem hiding this comment.
I was thinking if we should maybe do something like that plus have some dedicated error and then we could handle it by showing the toast for it from the global handler instead of checking in bunch of different places
But maybe we can also keep like this for now. It's more work but leaves it more open to change later
gudnuf
left a comment
gudnuf
left a comment
There was a problem hiding this comment.
I addressed all of the nits, but I need to think more on how to handle offline mints when the user attempts to send/receive and then I'll move the offline account toast if that's still needed.
I will probably do what you've suggested. The other idea I had is that the CashuMint class lets us pass a custom request, so we could throw the DomainError from there so that any cashu wallet method will throw the DomainError that the mint is offline, but I'm not sure if that's a good idea yet.
| return filteredData; | ||
| }, | ||
| [select?.currency, select?.type, user], | ||
| [select, user], |
There was a problem hiding this comment.
we should do individual properties and instead of entire select. because if caller does this:
const accounts. = useAccounts({ isOnline: true })
the instance of select object will be different on every render even though the individual property value is the same and that will make this useCallback useless
| <Badge>Default</Badge> | ||
| </div> | ||
| )} | ||
| {account.isDefault || |
There was a problem hiding this comment.
nit but I would format this as:
{(account.isDefault || !account.isOnline) && (...)}
lets just leave it as is for now. approved but pls handle last comments and this before merge |
|
closing to trigger ci |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
2 participants
closes #595
If fetching any of the mint data fails with a
NetworkError, we flag the account as offline. Now the account will be loaded into the app withisOnline: false, so we can handle offline mints in the UI by adding "Offline" badges and disabling usage of the account.I also filter out any pending token-swaps, send-swaps, receive-quotes, and send-quotes where the accounId is offline.