feat: notify upon incoming UTXOs #33
Conversation
| txId, | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
| if (!addresses) return |
I remember you said there is a bug about it will listen all addresses's utxos if it is empty.
There was a problem hiding this comment.
Yes it is guarded here
kastle/contexts/WalletManagerContext.tsx
Line 652 in a788456
The function is getting hard to read because you have a couple of nested function I will try to keep them at the top of the function.
| return; | ||
| } | ||
|
|
||
| const txId = event.added[0].outpoint.transactionId; |
There was a problem hiding this comment.
Are all added utxos are from the same transaction? I believe it is not.
There was a problem hiding this comment.
I believe UTXOs can come from different addresses by they will be in the same transaction.
contexts/WalletManagerContext.tsx
Outdated
| } | ||
|
|
||
| const txId = event.added[0].outpoint.transactionId; | ||
| const network = settings?.networkId ?? NetworkType.Mainnet; |
There was a problem hiding this comment.
We can use networkId from useRpcClientStateful instead of settings.
There was a problem hiding this comment.
I refactored the code to keep it consistent 782cd77
contexts/WalletManagerContext.tsx
Outdated
| @@ -587,7 +595,7 @@ export function WalletManagerProvider({ children }: { children: ReactNode }) { | |||
| } | |||
|
|
|||
| await saveWalletSettings(walletSettings); | |||
| }; | |||
| }, []); | |||
There was a problem hiding this comment.
Why using callback here? In addition, I think here are missing some dependencies, say, networkId, walletSettings and etc.
There was a problem hiding this comment.
It becomes an issue, that switching network would not change the addresses. The image is in mainnet now but still have test prefix.
There was a problem hiding this comment.
I introduce the useCallback to break the dependency chain and then the rerenders.
But it was more than a hack than the correct solution.
Thanks for removing it ;)
contexts/WalletManagerContext.tsx
Outdated
| rpcClient.subscribeUtxosChanged(addresses); | ||
| return () => { | ||
| rpcClient.unsubscribeUtxosChanged(addresses); | ||
|
|
||
| rpcClient.removeEventListener("utxos-changed", fetchBalance); |
There was a problem hiding this comment.
The removeEventListener is not updated. Fixed infinitely re-render by updating this.
There was a problem hiding this comment.
Please check this commit:
a788456
There was a problem hiding this comment.
Thanks for catching that bug !!!
Also, I have removed the safeQueue because it created more renders than without. I think it was solving the issues of not accounting for the settings loading.
Let me know if I am missing anything. I tested a bit it seems to work fine.
There was a problem hiding this comment.
Yeah, I agree with it. Javascript event loop have already queued the operations. No need to have this.
0x7u
force-pushed
the
feat/notify-incoming-utxos
branch
from
797ad22 to
3e1b117
Compare
Pull Request Checklist
Before Submission
Description
Notify the user upon incoming UTXOs
Additional Notes
By submitting this pull request, I confirm that:
project's
license terms