[Cashtab] [Chronik Websocket Notifications] [Part 4] Unsubscribe to Hash160s when active wallet changes
ClosedPublic
Actions

Authored by bytesofman on May 30 2022, 17:34.

Details

Reviewers

emack

Group Reviewers

Restricted Project

Commits

rABCbfb235fbfb79: [Cashtab] [Chronik Websocket Notifications] [Part 4] Unsubscribe to Hash160s…

Summary

T2447

Depends on D11534

Part 4 of chronik websocket subscriptions for Cashtab

When the user switches active wallets, unsubscribe from previous addresses.

Test Plan

npm start
Observe subscriptions to hash160s of active wallet
Navigate to settings and change the active wallet
Observe unsubscriptions in the dev console
Send a tx to the new active wallet. Observe a msg in the console.

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bytesofman created this revision.May 30 2022, 17:34

Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 30 2022, 17:34

bytesofman requested review of this revision.May 30 2022, 17:34

Harbormaster completed remote builds in B19182: Diff 33725.May 30 2022, 17:37

bytesofman mentioned this in D11530: [Cashtab] [Chronik Websocket Notifications] [Part 1] Connect to websocket.May 31 2022, 16:37

bytesofman edited the summary of this revision. (Show Details)May 31 2022, 17:51

bytesofman added a parent revision: D11534: [Cashtab] [Chronik Websocket Notifications] [Part 3] Subscribe to Hash160s.

bytesofman added a child revision: D11536: [Cashtab] [Chronik Websocket Notifications] [Part 5] Parse XEC txs from websocket msg.

emack requested changes to this revision.Jun 1 2022, 02:43

emack added a subscriber: emack.

emack added inline comments.

web/cashtab/src/hooks/useWallet.js
964 ↗	(On Diff #33725)	Similar to D11534, need to wrap this unsubscribe call in a try catch block

This revision now requires changes to proceed.Jun 1 2022, 02:43

Issue is related to chronik-client not handling incoming block msgs; try...catch is not necessary.

As the websocket gets into prod, we may see more edge cases of how it will fail. The app needs to be built to always handle a failed websocket. So, it's okay to not handle this (as yet unseen) error for now.

At the moment, the failure condition from this series of diffs is "incoming tx notifications happen the old way and not with the websocket, or incoming transaction notifications are broken." This is an acceptable failure condition.

emack accepted this revision.Jun 6 2022, 23:46

This revision is now accepted and ready to land.Jun 6 2022, 23:46

Rebase

Harbormaster completed remote builds in B19271: Diff 33841.Jun 7 2022, 01:19

Closed by commit rABCbfb235fbfb79: [Cashtab] [Chronik Websocket Notifications] [Part 4] Unsubscribe to Hash160s… (authored by bytesofman). · Explain WhyJun 7 2022, 21:06

This revision was automatically updated to reflect the committed changes.

bytesofman added a commit: rABCbfb235fbfb79: [Cashtab] [Chronik Websocket Notifications] [Part 4] Unsubscribe to Hash160s….