Page MenuHomePhabricator
Differential D12611

[Cashtab] [Wallet Export] Pt 1/ - Export wallets
Changes PlannedPublic
Actions

Authored by emack on Nov 25 2022, 08:25.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Project
Summary

T1716

This diff combines selected attributes from savedWallet and wallet objects and exports into an external JSON file.

Pt 1/ - Export wallets
Pt 2/ - Import wallets
Pt 3/ - Export wallet contacts data
Pt 4/ - Import wallet contacts data
Pt 5/ - Export combined wallet data
Pt 6/ - Import combined wallet data
Pt 7/ - Export encrypted combined wallet data
Pt 8/ - Import encrypted combined wallet data

Test Plan

npm test
npm start
navigate to Settings and click on the Export Cashtab Data
verify the generated JSON export contains your saved wallets + active wallet, including the following attributes:

  • mnemonic string
  • name string
  • Path1899 object

Diff Detail

Repository
rABC Bitcoin ABC
Branch
walletExport
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 21486
Build 42620: Build Diffcashtab-tests
Build 42619: arc lint + arc unit

Event Timeline

emack created this revision.Nov 25 2022, 08:25
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 25 2022, 08:25
emack requested review of this revision.Nov 25 2022, 08:25
Harbormaster completed remote builds in B21117: Diff 36599.Nov 25 2022, 08:28
bytesofman requested changes to this revision.Nov 28 2022, 05:37
bytesofman added inline comments.
web/cashtab/src/components/Configure/Configure.js
1170 ↗(On Diff #36599)

It's worth having savedWallets as a param here. React functions behave strangely when they use state variables that are not taken in as params (what happens if the state changes while function is executing?)

1172 ↗(On Diff #36599)

Are Path245, Path145, and Path1899 necessary here? They may also be derived.

I think what you have here makes sense because re-deriving everything would be a memory-intensive process. But comment should reflect this, e.g. Store all wallet info except state

1183 ↗(On Diff #36599)

savedWallets includes the active wallet, so we can skip this step

1193 ↗(On Diff #36599)

Please add some info in the comments about why the export is csv encoded (seems like an extra step, now it needs to be decoded later?)

1634 ↗(On Diff #36599)

exportWallet(savedWallets)

This revision now requires changes to proceed.Nov 28 2022, 05:37
emack edited the test plan for this revision. (Show Details)Nov 30 2022, 00:40
emack updated this revision to Diff 36795.Nov 30 2022, 00:45
emack marked 5 inline comments as done.
  • added wallets array within the walletExport object since subsequent stacks will need to differentiate wallet data from contact list data
  • passing in savedWallets as a param to mitigate React behaviors
  • removed Path245 and Path145, but kept Path1899 as the app is used to referencing this to retrieve wallet attributes
  • I've double checked re: active wallets, the export of savedWallets did not contain the active wallet, even though it is displayed under the Saved Wallets dropdown on the frontend. It was only included in the export when explicitly adding the wallet instance to the walletExport object.
  • changed export encoding to json and simplified export logic
  • updated comments
Harbormaster completed remote builds in B21250: Diff 36795.Nov 30 2022, 00:48
emack updated this revision to Diff 37141.Dec 9 2022, 01:23

rebase

Harbormaster completed remote builds in B21486: Diff 37141.Dec 9 2022, 01:26
bytesofman requested changes to this revision.Dec 10 2022, 13:45

Even though we won't migrate to 899, I think we should still finish this stack off before doing this.

Will be complicated to support migrating legacy data in in the future if the active wallet structure changes afterward.

https://reviews.bitcoinabc.org/T2836

More broadly, I think this is lower impact than aliases, dev libraries, getting TransactionBuilder into ecashjs-lib . So, may table this for some time.

This revision now requires changes to proceed.Dec 10 2022, 13:45
emack planned changes to this revision.Dec 11 2022, 02:18

Tabled for now to focus on aliases and dev libraries.

Revision Contents
Changeset List

PathSize
web/
cashtab/
src/
components/
Configure/
37 lines
__tests__/
__snapshots__/

Diff 37141

View Options

web/cashtab/src/components/Configure/Configure.js

Loading...
View Options

web/cashtab/src/components/Configure/__tests__/__snapshots__/Configure.test.js.snap

Loading...