Details

Reviewers

Group Reviewers

Restricted Project

Commits

rABC56b7017f6c1f: [Cashtab] [Simplify wallet structure p1] Show desired end state

Summary

T2663

Wallet state is disorganized due to iterative changes and preservation of artifacts. This stack will better organize how wallet information is stored and ensure legacy wallets are properly migrated.

In part one, nothing is changed -- we just add the desired end state which future diffs will work toward.

Test Plan

Review the new wallet structure and comment if you note any improvements or better approaches.

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 21196
Build 42044: Build Diff	cashtab-tests
Build 42043: arc lint + arc unit

Event Timeline

bytesofman created this revision.Nov 23 2022, 08:48

Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 23 2022, 08:48

bytesofman requested review of this revision.Nov 23 2022, 08:48

Harbormaster completed remote builds in B21063: Diff 36535.Nov 23 2022, 08:54

bytesofman added a child revision: D12592: [Cashtab] [Simplify wallet structure p2] Remove duplicate tokens array.Nov 23 2022, 08:58

emack requested changes to this revision.Nov 23 2022, 12:12

emack added a subscriber: emack.

emack added inline comments.

web/cashtab/src/hooks/useWallet.js
266 ↗	(On Diff #36535)	is there a reason `nonSlpUtxos` does not have its own attribute in this object/

This revision now requires changes to proceed.Nov 23 2022, 12:12

bytesofman requested review of this revision.Nov 23 2022, 17:45

bytesofman added inline comments.

web/cashtab/src/hooks/useWallet.js
266 ↗	(On Diff #36535)	In this case, it happens to have the same name as the variable where it is defined above. To your point, `slpUtxos`, `tokens`, and `parsedTxHistory` should also behave like this, to remove this ambiguity. I think the most appropriate place to do this is toward the end of this stack. Right now, we don't want to change the names of `finalizedSlpUtxos`, `finalTokenArray`, or `chronikTxHistory`, since we will fix those one at a time as this stack progresses. So, I think it's okay to ignore on this diff as we don't want to change it yet. But don't let me land this stack without fixing it later.