Page MenuHomePhabricator

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

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 21117
Build 41886: Build Diffcashtab-tests
Build 41885: arc lint + arc unit

Event Timeline

emack requested review of this revision.Nov 25 2022, 08:25
bytesofman added inline comments.
web/cashtab/src/components/Configure/Configure.js
1170

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

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

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

1193

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

exportWallet(savedWallets)

This revision now requires changes to proceed.Nov 28 2022, 05:37
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

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.