Page MenuHomePhabricator

[Cashtab] Hide notification when switching apps/wallets
ClosedPublic

Authored by kieran709 on Mar 9 2022, 17:02.

Details

Summary

When switching between wallets, if switching to a wallet that has more XEC, the xecReceivedNotification fires showing the difference between the balances of the two wallets. This behaviour also happens for tokens in some situations. To fix this, the hasUpdated state variable will be initalized to false on component load, set to true during the useAsyncTimeoutFunction, and reset to false during the activateWallet function. Related to T2295.

Test Plan

cd web/cashtab
npm start
switch to a wallet that has a higher balance of XEC
ensure xecReceivedNotification does not fire
switch to a wallet that has a lower balance of XEC
ensure xecReceivedNotification doesn't fire
from another wallet, send XEC and ensure that xecNotification does fire

Diff Detail

Repository
rABC Bitcoin ABC
Branch
no-notification-on-activate
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 18485
Build 36765: Build Diffcashtab-tests
Build 36764: arc lint + arc unit

Event Timeline

Tail of the build log:

Build 'Bitcoin ABC Diffs / Diff Testing' #43689, branch 'refs/tags/phabricator/diff/32643'
Triggered 2022-03-09 17:02:35 by 'Phabricator Staging (phabricator-staging)'
Started 2022-03-09 17:02:38 on agent 'buildagent1'
Finished 2022-03-09 17:02:44 with status FAILURE 'Snapshot dependency failed to start: Automated Deployments / Bitcoin ABC Infra / Bitcoin-ABC Infra Checkout'
VCS revisions: 'PhabBitcoinAbcStagingRo' (Git, instance id 18): 'N/A' (checkout rules: '+:. => ./bitcoin-abc')
TeamCity URL https://build.bitcoinabc.org/viewLog.html?buildId=364281&buildTypeId=BitcoinABC_BitcoinAbcStaging 
TeamCity server version is 2019.2.4 (build 72059), server timezone: GMT (UTC)

[17:02:35]E: bt15 (running for 8s)
[17:02:35]i: TeamCity server version is 2019.2.4 (build 72059)
[17:02:35] : Collecting changes in 2 VCS roots
[17:02:35] :	 [Collecting changes in 2 VCS roots] VCS Root details
[17:02:35] :		 [VCS Root details] "phab-bitcoin-abc-staging-ro" {instance id=18, parent internal id=14, parent id=PhabBitcoinAbcStagingRo, description: "https://reviews.bitcoinabc.org/source/bitcoin-abc-staging.git#refs/heads/master"}
[17:02:35] :		 [VCS Root details] "abc-infrastructure" {instance id=7, parent internal id=7, parent id=AutomatedDeployments_BitcoinAbcDeveloperTools_AbcInfrastructure, description: "ssh://vcs@reviews.bitcoinabc.org:2221/source/infrastructure.git#refs/heads/master"}
[17:02:38] : The build is removed from the queue to be prepared for the start
[17:02:44]W: This build has not been started because some of the builds it depends on failed to start
[17:02:44]E: Snapshot dependency "Automated Deployments / Bitcoin ABC Infra / Bitcoin-ABC Infra Checkout" failed
[17:02:44] : Build finished

Changes planned as outlined in summary.

bytesofman requested changes to this revision.Mar 9 2022, 17:42
  • It's not clear why this is being wrapped in a function. The existing code is designed so that this will run every time a state item changes. I can see a case for wrapping it in useEffect, so that it is only changing when balance changes. But I don't think that's related to this diff.
  • This is still missing another common case:

a) user makes a bunch of txs with Cashtab wallet on one device
b) user opens same Cashtab wallet on another device
c) user sees notification for significant tx received that did not happen, it's just the balance being reconciled

Designing a check for "is this the first time balances has been updated on wallet load?" would catch both this case and the changed wallet case. This might be as simple as previousUtxos not being null or undefined. You could also set a flag variable like updateHasRan that is initialized as false but set to true the first time update is called.

Responding to review feedback.

Failed tests logs:

====== CashTab Unit Tests: useBCH hook sends XEC throws error attempting to encrypt a message with an invalid address ======
Error: expect(received).toStrictEqual(expected) // deep equality

- Expected  - 2
+ Received  + 2

  Object {
-   "error": "Unsupported address format : INVALIDADDRESS",
-   "success": false,
+   "message": "Not Found",
+   "status": 404,
  }
    at Object.<anonymous> (/work/web/cashtab/src/hooks/__tests__/useBCH.test.js:222:29)
    at processTicksAndRejections (node:internal/process/task_queues:94:5)

Each failure log is accessible here:
CashTab Unit Tests: useBCH hook sends XEC throws error attempting to encrypt a message with an invalid address

added comment to force new test.

bytesofman added inline comments.
web/cashtab/src/hooks/useWallet.js
1003 ↗(On Diff #32706)

&& hasUpdated is equivalent to && hasUpdated === true --> so use the shorter one, i.e. remove === true

1117 ↗(On Diff #32706)

if (!hasUpdated) {setHasUpdated(true)}

This revision now requires changes to proceed.Mar 14 2022, 22:06

Responding to review feedback.

This revision is now accepted and ready to land.Mar 15 2022, 16:37