Fix transaction status screen not updating when transaction gets confirmed #8143
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an issue where the transaction status screen fails to update when a transaction gets confirmed while the user is viewing it. The problem stemmed from missing dependency tracking in the useEffect hook that synchronizes transaction data from the wallet to local state.
Key Change:
- Added
wallet?.getLastTxFetch()to the useEffect dependency array in TransactionStatus.tsx to trigger re-synchronization when wallet transaction data is updated
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
anabelle
force-pushed
the
fix/transaction-status-update-on-confirmation
branch
from
9854d5a to
829cc2b
Compare
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
anabelle
force-pushed
the
fix/transaction-status-update-on-confirmation
branch
from
829cc2b to
a5c117b
Compare
|
@cursoragent review |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
limpbrains
left a comment
limpbrains
left a comment
There was a problem hiding this comment.
Hi Anabelle, thank you for the PR!
Please take a look at the useWalletSubscribe hook we have — it was created for exactly the same reason: to update the React view when something changes inside the wallet.
anabelle
force-pushed
the
fix/transaction-status-update-on-confirmation
branch
from
d68daf6 to
aa0246f
Compare
- Cache wallet last fetch timestamp with useMemo to avoid redundant reads - Trigger transaction data reload when fetch timestamp advances - Add regression unit test covering confirmation refresh behaviour
Address review feedback by using the existing useWalletSubscribe hook instead of manually memoizing getLastTxFetch(). The hook was specifically designed to provide automatic wallet updates when transactions change. Changes: - Import and use useWalletSubscribe(walletID) to get subscribedWallet - Replace wallet state setter to use subscribedWallet instead of finding from wallets array - Replace transaction sync effect to use subscribedWallet and remove lastTxFetch dependency - Update test to mock useWalletSubscribe hook The hook polls getLastTxFetch() every second internally and returns a proxied wallet that triggers re-renders when changes are detected.
anabelle
force-pushed
the
fix/transaction-status-update-on-confirmation
branch
from
aa0246f to
480d9e9
Compare
The test was failing because useWalletSubscribe returns a new proxied wallet when lastTxFetch changes. Updated the mock to create a new Proxy instance when the update function changes lastTxFetch, simulating the hook's behavior.
|
Done! Switched to using useWalletSubscribe as suggested. |
|
Wake the fuck up samurai, we have PRs to merge
[all PRs for @Overtorment] https://github.com/BlueWallet/BlueWallet/pulls/review-requested/Overtorment |
|
Wake the fuck up samurai, we have PRs to merge
[all PRs for @limpbrains] https://github.com/BlueWallet/BlueWallet/pulls/review-requested/limpbrains |
|
reran all tests here: #8172 |
|
squash-merged because git history is not up to a standard. |
|
@anabelle thank you for your contribution! |
4 participants
Problem
Root Cause
Solution
Testing
Fixes #7984.