Skip to content

Performance: fix - stop the persist transform mutating live wallet state - #2314

Open
JohnathanWhite wants to merge 4 commits into
bitpay:developfrom
JohnathanWhite:fix/persist-transform-and-nonce-gate
Open

Performance: fix - stop the persist transform mutating live wallet state#2314
JohnathanWhite wants to merge 4 commits into
bitpay:developfrom
JohnathanWhite:fix/persist-transform-and-nonce-gate

Conversation

@JohnathanWhite

Copy link
Copy Markdown
Collaborator

The bindWalletKeys inbound transform ran delete wallet.transactionHistory on wallet objects it does not own. redux-persist passes inbound transforms the live store slice by reference, so this mutated in-memory Redux state: the tx-history cache was wiped on every persist flush, moments after the reducer wrote it. The read-side cache check in GetTransactionHistory was therefore dead, and history was re-downloaded on every wallet or account view - one request per wallet on account views.

Restoring that cache re-arms a safety gate that had been dead as a side effect of the same bug: send.ts blocks an ETH send when hasConfirmingTxs is set, to avoid a nonce collision with a pending transaction. Nothing recomputes that flag unless a details screen is mounted or the user pulls to refresh, so a cached value can be arbitrarily old. Trusting a stale value locks the user out of sending; discarding it disables the protection for exactly the case it exists for. So the flag now carries a fetchedAt stamp and, when stale, is re-verified against the network before the send is allowed or blocked. On a fetch failure it fails closed.

For ERC20 sends the re-verification targets the linked native wallet, because the flag for a token wallet is derived from the linked wallet's history - nonces are account-level - and re-fetching the token wallet would recompute from the same stale cache. Load-more pages carry the previous flag and its age forward rather than reporting false, which would clobber a correct value after an ordinary scroll.

The bindWalletKeys inbound transform ran `delete wallet.transactionHistory` on
wallet objects it does not own. redux-persist passes inbound transforms the live
store slice by reference, so this mutated in-memory Redux state: the tx-history
cache was wiped on every persist flush, moments after the reducer wrote it. The
read-side cache check in GetTransactionHistory was therefore dead, and history was
re-downloaded on every wallet or account view - one request per wallet on account
views.

Restoring that cache re-arms a safety gate that had been dead as a side effect of
the same bug: send.ts blocks an ETH send when `hasConfirmingTxs` is set, to avoid
a nonce collision with a pending transaction. Nothing recomputes that flag unless
a details screen is mounted or the user pulls to refresh, so a cached value can be
arbitrarily old. Trusting a stale value locks the user out of sending; discarding
it disables the protection for exactly the case it exists for. So the flag now
carries a fetchedAt stamp and, when stale, is re-verified against the network
before the send is allowed or blocked. On a fetch failure it fails closed.

For ERC20 sends the re-verification targets the linked native wallet, because the
flag for a token wallet is derived from the linked wallet's history - nonces are
account-level - and re-fetching the token wallet would recompute from the same
stale cache. Load-more pages carry the previous flag and its age forward rather
than reporting false, which would clobber a correct value after an ordinary scroll.
@gabrielbazan7
gabrielbazan7 force-pushed the fix/persist-transform-and-nonce-gate branch from 613c126 to ba081c9 Compare August 25, 2026 14:26
@gabrielbazan7

Copy link
Copy Markdown
Collaborator

follow up: JohnathanWhite#10

…once-gate-followup

persist transform and nonce gate followup
@JohnathanWhite
JohnathanWhite marked this pull request as ready for review August 25, 2026 14:38
@gabrielbazan7

Copy link
Copy Markdown
Collaborator

Already implemented here: #2241. We can close this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants