Page MenuHomePhabricator

[Cashtab] add publicKey property for each of the Path in the wallet
ClosedPublic

Authored by hungsam on Nov 26 2021, 06:31.

Details

Reviewers
bytesofman
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Commits
rABCb8860f5ce45f: [Cashtab] add publicKey property for each of the Path in the wallet
Summary

Currently, to determine if a transaction is incoming or outgoing, the wallet

  • retrieves the transaction data by calling BCH.RawTransactions.getTxData(txid)
  • looks through the returned transaction data’s “vin” to see if it contains the address of the current wallet
  • If it does, the transaction is outgoing,
  • Else, the transaction is incoming

There is a performance problem with BCH.RawTransactions.getTxData(txid).
It populates the address property of each “vin” by making a separate api call
to retrieve the data of the transaction that generates this input.
If a transaction has hundreds of inputs,
retrieving the data for that transaction will generate hundreds of api calls instead of just one.

This problem can be solved by calling BCH.RawTransaction.getRawTransaction(txid,true),
instead of BCH.RawTransaction.getTxData().

getRawTransaction() only makes one api call, however,
the returned data does not contain a wallet address in the “vin”.
We will not be able to determine if a transaction is incoming or outgoing
by comparing the vin’s address with the current wallet address.
Instead, we have the extract the public key in the scriptSig of each "vin",
and compare it with the public key of this wallet.
If they are the same, the transaction is outgoing.

Hence, we need to retrieve and store public key for each address of this wallet

see
https://bchjs.fullstack.cash/#api-RawTransactions-getRawTransaction
https://bchjs.fullstack.cash/#api-RawTransactions-getTxData

Test Plan
  • make sure there are multiple existing wallets on the local storage
  • npm start
  • make sure the currently active wallet is migrated to include the public key
  • make sure all operations perform as normal
  • switch to another wallet
  • make sure the previously active wallet is saved and includes the public key
  • make sure the currently active wallet is migrated and includes the public key
  • make sure all operations perform as normal

Diff Detail

Repository
rABC Bitcoin ABC
Branch
get-pubkey (branched from master)
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 17518
Build 34862: Build Diffcashtab-tests
Build 34861: arc lint + arc unit

Event Timeline

hungsam requested review of this revision.Nov 26 2021, 06:31
hungsam created this revision.

Failed tests logs:

====== CashTab Unit Tests:  Migrating legacy wallet on testnet ======
Error: expect(received).toStrictEqual(expected) // deep equality

- Expected  - 10
+ Received  + 13

  Object {
    "Path145": Object {
-     "cashAddress": "bitcoincash:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9v0lgx569z",
-     "fundingAddress": "simpleledger:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vryrap6mu",
-     "fundingWif": "L2xvTe6CdNxroR6pbdpGWNjAa55AZX5Wm59W5TXMuH31ihNJdDjt",
-     "legacyAddress": "1511T3ynXKgCwXhFijCUWKuTfqbPxFV1AF",
-     "slpAddress": "simpleledger:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vryrap6mu",
+     "cashAddress": "bchtest:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vtdvpkdz7",
+     "fundingAddress": "slptest:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vset6v6sr",
+     "fundingWif": "cTKuvZ644Sf7xra5z3dPshEECJNaDyBCq7HyBsysQPh1ySSpxtQ1",
+     "legacyAddress": "mjWxk74mLM7TieAsSJArLF7nXqC6oc2mof",
+     "publicKey": "02fe5308d77bcce825068a9e46adc6f032dbbe39167a7b6d05ac563ac71d8b186e",
+     "slpAddress": "slptest:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vset6v6sr",
    },
    "Path1899": Object {
      "cashAddress": "bchtest:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7c9ex06hrx",
      "fundingAddress": "slptest:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7c7dp5qq3m",
      "fundingWif": "cNRFB6MmkNhyhAj1TpGhXdbHgzWg4BsdHbAkKjiz4vt4vwgpC44F",
      "legacyAddress": "mxX888y8yaPpTYh3WhrB9GEkT3cgumYwPw",
+     "publicKey": "02a06bb380cf180d703f6f80796a13555aefff817d1f6f842f1e5c555b15f0fa70",
      "slpAddress": "slptest:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7c7dp5qq3m",
    },
    "Path245": Object {
-     "cashAddress": "bitcoincash:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy54hkry298",
-     "fundingAddress": "simpleledger:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5evac32me",
-     "fundingWif": "KwgNkyijAaxFr5XQdnaYyNMXVSZobgHzSoKKfWiC3Q7Xr4n7iYMG",
-     "legacyAddress": "1EgPUfBgU7ekho3EjtGze87dRADnUE8ojP",
-     "slpAddress": "simpleledger:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5evac32me",
+     "cashAddress": "bchtest:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy539jyxazm",
+     "fundingAddress": "slptest:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5234lu2sx",
+     "fundingWif": "cN3NDtiabeeX1Wzg2CPgLgrb7fsDG8PgWqTnmwAhYWmY6osSta7Q",
+     "legacyAddress": "muCLmiGfH961UuWrTTFNU3KxH9pVQJJx6Z",
+     "publicKey": "03c4a69fd90c8b196683216cffd2943a7b13b0db0812e44a4ff156ac7e03fc4ed7",
+     "slpAddress": "slptest:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5234lu2sx",
    },
    "mnemonic": "apart vacuum color cream drama kind foil history hurt alone ask census",
    "name": "MigrationTestAlpha",
  }
    at Object.<anonymous> (/work/web/cashtab/src/hooks/__tests__/useWallet.test.js:61:20)
====== CashTab Unit Tests:  Migrating legacy wallet on mainnet ======
Error: expect(received).toStrictEqual(expected) // deep equality

- Expected  - 0
+ Received  + 3

@@ -2,24 +2,27 @@
    "Path145": Object {
      "cashAddress": "bitcoincash:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9v0lgx569z",
      "fundingAddress": "simpleledger:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vryrap6mu",
      "fundingWif": "L2xvTe6CdNxroR6pbdpGWNjAa55AZX5Wm59W5TXMuH31ihNJdDjt",
      "legacyAddress": "1511T3ynXKgCwXhFijCUWKuTfqbPxFV1AF",
+     "publicKey": "02fe5308d77bcce825068a9e46adc6f032dbbe39167a7b6d05ac563ac71d8b186e",
      "slpAddress": "simpleledger:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vryrap6mu",
    },
    "Path1899": Object {
      "cashAddress": "bitcoincash:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7cptzgcqy6",
      "fundingAddress": "simpleledger:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7cdsfndq6y",
      "fundingWif": "Kx4FiBMvKK1iXjFk5QTaAK6E4mDGPjmwDZ2HDKGUZpE4gCXMaPe9",
      "legacyAddress": "1J1Aq5tAAYxZgSDRo8soKM2Rb41z3xrYpm",
+     "publicKey": "02a06bb380cf180d703f6f80796a13555aefff817d1f6f842f1e5c555b15f0fa70",
      "slpAddress": "simpleledger:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7cdsfndq6y",
    },
    "Path245": Object {
      "cashAddress": "bitcoincash:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy54hkry298",
      "fundingAddress": "simpleledger:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5evac32me",
      "fundingWif": "KwgNkyijAaxFr5XQdnaYyNMXVSZobgHzSoKKfWiC3Q7Xr4n7iYMG",
      "legacyAddress": "1EgPUfBgU7ekho3EjtGze87dRADnUE8ojP",
+     "publicKey": "03c4a69fd90c8b196683216cffd2943a7b13b0db0812e44a4ff156ac7e03fc4ed7",
      "slpAddress": "simpleledger:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5evac32me",
    },
    "mnemonic": "apart vacuum color cream drama kind foil history hurt alone ask census",
    "name": "MigrationTestAlpha",
  }
    at Object.<anonymous> (/work/web/cashtab/src/hooks/__tests__/useWallet.test.js:117:20)

Each failure log is accessible here:
CashTab Unit Tests: Migrating legacy wallet on testnet
CashTab Unit Tests: Migrating legacy wallet on mainnet

Address unit test failture

  • fix Migrating legacy wallet on mainnet test failure
  • failure in "Migrating legacy wallet on testnet" caused by previous update

as well as this update. Should I wait or go ahead and fix the test with
Path's details on test net?

Failed tests logs:

====== CashTab Unit Tests:  Migrating legacy wallet on testnet ======
Error: expect(received).toStrictEqual(expected) // deep equality

- Expected  - 10
+ Received  + 13

  Object {
    "Path145": Object {
-     "cashAddress": "bitcoincash:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9v0lgx569z",
-     "fundingAddress": "simpleledger:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vryrap6mu",
-     "fundingWif": "L2xvTe6CdNxroR6pbdpGWNjAa55AZX5Wm59W5TXMuH31ihNJdDjt",
-     "legacyAddress": "1511T3ynXKgCwXhFijCUWKuTfqbPxFV1AF",
-     "slpAddress": "simpleledger:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vryrap6mu",
+     "cashAddress": "bchtest:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vtdvpkdz7",
+     "fundingAddress": "slptest:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vset6v6sr",
+     "fundingWif": "cTKuvZ644Sf7xra5z3dPshEECJNaDyBCq7HyBsysQPh1ySSpxtQ1",
+     "legacyAddress": "mjWxk74mLM7TieAsSJArLF7nXqC6oc2mof",
+     "publicKey": "02fe5308d77bcce825068a9e46adc6f032dbbe39167a7b6d05ac563ac71d8b186e",
+     "slpAddress": "slptest:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vset6v6sr",
    },
    "Path1899": Object {
      "cashAddress": "bchtest:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7c9ex06hrx",
      "fundingAddress": "slptest:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7c7dp5qq3m",
      "fundingWif": "cNRFB6MmkNhyhAj1TpGhXdbHgzWg4BsdHbAkKjiz4vt4vwgpC44F",
      "legacyAddress": "mxX888y8yaPpTYh3WhrB9GEkT3cgumYwPw",
+     "publicKey": "02a06bb380cf180d703f6f80796a13555aefff817d1f6f842f1e5c555b15f0fa70",
      "slpAddress": "slptest:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7c7dp5qq3m",
    },
    "Path245": Object {
-     "cashAddress": "bitcoincash:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy54hkry298",
-     "fundingAddress": "simpleledger:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5evac32me",
-     "fundingWif": "KwgNkyijAaxFr5XQdnaYyNMXVSZobgHzSoKKfWiC3Q7Xr4n7iYMG",
-     "legacyAddress": "1EgPUfBgU7ekho3EjtGze87dRADnUE8ojP",
-     "slpAddress": "simpleledger:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5evac32me",
+     "cashAddress": "bchtest:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy539jyxazm",
+     "fundingAddress": "slptest:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5234lu2sx",
+     "fundingWif": "cN3NDtiabeeX1Wzg2CPgLgrb7fsDG8PgWqTnmwAhYWmY6osSta7Q",
+     "legacyAddress": "muCLmiGfH961UuWrTTFNU3KxH9pVQJJx6Z",
+     "publicKey": "03c4a69fd90c8b196683216cffd2943a7b13b0db0812e44a4ff156ac7e03fc4ed7",
+     "slpAddress": "slptest:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5234lu2sx",
    },
    "mnemonic": "apart vacuum color cream drama kind foil history hurt alone ask census",
    "name": "MigrationTestAlpha",
  }
    at Object.<anonymous> (/work/web/cashtab/src/hooks/__tests__/useWallet.test.js:61:20)

Each failure log is accessible here:
CashTab Unit Tests: Migrating legacy wallet on testnet

bytesofman added inline comments.
web/cashtab/src/hooks/__tests__/useWallet.test.js
11 ↗(On Diff #31057)

This needs publicKey field added to each Path... object, similar to the change made on the test at line 64

This revision now requires changes to proceed.Nov 27 2021, 00:21
web/cashtab/src/hooks/__tests__/useWallet.test.js
11 ↗(On Diff #31057)

I did that and it still failed. that was why i only fixed the failure with the mainnet.
Notice the difference in the address prefixes in Path145 and Path245. bitcoincash vs bchtest

fix unit test failure in "Migrating legacy wallet on testnet"

Failed tests logs:

====== CashTab Unit Tests:  Migrating legacy wallet on testnet ======
Error: expect(received).toStrictEqual(expected) // deep equality

- Expected  - 10
+ Received  + 12

  Object {
    "Path145": Object {
-     "cashAddress": "bitcoincash:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9v0lgx569z",
-     "fundingAddress": "simpleledger:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vryrap6mu",
-     "fundingWif": "L2xvTe6CdNxroR6pbdpGWNjAa55AZX5Wm59W5TXMuH31ihNJdDjt",
-     "legacyAddress": "1511T3ynXKgCwXhFijCUWKuTfqbPxFV1AF",
-     "slpAddress": "simpleledger:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vryrap6mu",
+     "cashAddress": "bchtest:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vtdvpkdz7",
+     "fundingAddress": "slptest:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vset6v6sr",
+     "fundingWif": "cTKuvZ644Sf7xra5z3dPshEECJNaDyBCq7HyBsysQPh1ySSpxtQ1",
+     "legacyAddress": "mjWxk74mLM7TieAsSJArLF7nXqC6oc2mof",
+     "publicKey": "02fe5308d77bcce825068a9e46adc6f032dbbe39167a7b6d05ac563ac71d8b186e",
+     "slpAddress": "slptest:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vset6v6sr",
    },
    "Path1899": Object {
      "cashAddress": "bchtest:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7c9ex06hrx",
      "fundingAddress": "slptest:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7c7dp5qq3m",
      "fundingWif": "cNRFB6MmkNhyhAj1TpGhXdbHgzWg4BsdHbAkKjiz4vt4vwgpC44F",
      "legacyAddress": "mxX888y8yaPpTYh3WhrB9GEkT3cgumYwPw",
      "publicKey": "02a06bb380cf180d703f6f80796a13555aefff817d1f6f842f1e5c555b15f0fa70",
      "slpAddress": "slptest:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7c7dp5qq3m",
    },
    "Path245": Object {
-     "cashAddress": "bitcoincash:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy54hkry298",
-     "fundingAddress": "simpleledger:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5evac32me",
-     "fundingWif": "KwgNkyijAaxFr5XQdnaYyNMXVSZobgHzSoKKfWiC3Q7Xr4n7iYMG",
-     "legacyAddress": "1EgPUfBgU7ekho3EjtGze87dRADnUE8ojP",
-     "slpAddress": "simpleledger:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5evac32me",
+     "cashAddress": "bchtest:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy539jyxazm",
+     "fundingAddress": "slptest:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5234lu2sx",
+     "fundingWif": "cN3NDtiabeeX1Wzg2CPgLgrb7fsDG8PgWqTnmwAhYWmY6osSta7Q",
+     "legacyAddress": "muCLmiGfH961UuWrTTFNU3KxH9pVQJJx6Z",
+     "publicKey": "03c4a69fd90c8b196683216cffd2943a7b13b0db0812e44a4ff156ac7e03fc4ed7",
+     "slpAddress": "slptest:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5234lu2sx",
    },
    "mnemonic": "apart vacuum color cream drama kind foil history hurt alone ask census",
    "name": "MigrationTestAlpha",
  }
    at Object.<anonymous> (/work/web/cashtab/src/hooks/__tests__/useWallet.test.js:63:20)

Each failure log is accessible here:
CashTab Unit Tests: Migrating legacy wallet on testnet

fix unit test failure on testnet

I did that and it still failed. that was why i only fixed the failure with the mainnet.
Notice the difference in the address prefixes in Path145 and Path245. bitcoincash vs bchtest

Ah ok. Looks like there is a pre-existing issue where this unit test is not using the correct address formats.

That issue is not related to this diff -- so, it's okay to keep the non-testnet formats in that unit test even though it is "wrong." That needs to be fixed in a separate diff.

Cashtab needs to be fully reviewed and potentially refactored to actually support testnet API. We have these APIs available if you would like to take that on, but that task is distinct from this diff. When Cashtab was forked from mint.bitcoin.com, there was support for testnet...but I am not sure if it has been consistently maintained throughout the development process, as Cashtab has never used it.

This revision now requires changes to proceed.Nov 29 2021, 20:07

rebase to latest master
revert previous fix to 'Mirgrating legacy wallet on testnet' unit test

Failed tests logs:

====== CashTab Unit Tests:  Migrating legacy wallet on testnet ======
Error: expect(received).toStrictEqual(expected) // deep equality

- Expected  - 10
+ Received  + 12

  Object {
    "Path145": Object {
-     "cashAddress": "bitcoincash:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9v0lgx569z",
-     "fundingAddress": "simpleledger:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vryrap6mu",
-     "fundingWif": "L2xvTe6CdNxroR6pbdpGWNjAa55AZX5Wm59W5TXMuH31ihNJdDjt",
-     "legacyAddress": "1511T3ynXKgCwXhFijCUWKuTfqbPxFV1AF",
-     "slpAddress": "simpleledger:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vryrap6mu",
+     "cashAddress": "bchtest:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vtdvpkdz7",
+     "fundingAddress": "slptest:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vset6v6sr",
+     "fundingWif": "cTKuvZ644Sf7xra5z3dPshEECJNaDyBCq7HyBsysQPh1ySSpxtQ1",
+     "legacyAddress": "mjWxk74mLM7TieAsSJArLF7nXqC6oc2mof",
+     "publicKey": "02fe5308d77bcce825068a9e46adc6f032dbbe39167a7b6d05ac563ac71d8b186e",
+     "slpAddress": "slptest:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vset6v6sr",
    },
    "Path1899": Object {
      "cashAddress": "bchtest:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7c9ex06hrx",
      "fundingAddress": "slptest:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7c7dp5qq3m",
      "fundingWif": "cNRFB6MmkNhyhAj1TpGhXdbHgzWg4BsdHbAkKjiz4vt4vwgpC44F",
      "legacyAddress": "mxX888y8yaPpTYh3WhrB9GEkT3cgumYwPw",
      "publicKey": "02a06bb380cf180d703f6f80796a13555aefff817d1f6f842f1e5c555b15f0fa70",
      "slpAddress": "slptest:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7c7dp5qq3m",
    },
    "Path245": Object {
-     "cashAddress": "bitcoincash:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy54hkry298",
-     "fundingAddress": "simpleledger:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5evac32me",
-     "fundingWif": "KwgNkyijAaxFr5XQdnaYyNMXVSZobgHzSoKKfWiC3Q7Xr4n7iYMG",
-     "legacyAddress": "1EgPUfBgU7ekho3EjtGze87dRADnUE8ojP",
-     "slpAddress": "simpleledger:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5evac32me",
+     "cashAddress": "bchtest:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy539jyxazm",
+     "fundingAddress": "slptest:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5234lu2sx",
+     "fundingWif": "cN3NDtiabeeX1Wzg2CPgLgrb7fsDG8PgWqTnmwAhYWmY6osSta7Q",
+     "legacyAddress": "muCLmiGfH961UuWrTTFNU3KxH9pVQJJx6Z",
+     "publicKey": "03c4a69fd90c8b196683216cffd2943a7b13b0db0812e44a4ff156ac7e03fc4ed7",
+     "slpAddress": "slptest:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5234lu2sx",
    },
    "mnemonic": "apart vacuum color cream drama kind foil history hurt alone ask census",
    "name": "MigrationTestAlpha",
  }
    at Object.<anonymous> (/work/web/cashtab/src/hooks/__tests__/useWallet.test.js:63:20)

Each failure log is accessible here:
CashTab Unit Tests: Migrating legacy wallet on testnet

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

Why this change from const to let?

Does not appear to be related to this diff

332 ↗(On Diff #31161)

Because this check is used twice, please make a separate function for it, isLegacyMigrationRequired(), that simply returns this boolean

Add it to utils/Validation.js and include some unit tests.

561 ↗(On Diff #31161)

See comment on line 332 -- also call the new validation function here

606 ↗(On Diff #31161)

Path1899, not Path1800

608 ↗(On Diff #31161)

Also call new validation function here instead of this list of if conditions

This revision now requires changes to proceed.Nov 30 2021, 00:11

Failed tests logs:

====== CashTab Unit Tests:  Migrating legacy wallet on testnet ======
Error: expect(received).toStrictEqual(expected) // deep equality

- Expected  - 10
+ Received  + 12

  Object {
    "Path145": Object {
-     "cashAddress": "bitcoincash:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9v0lgx569z",
-     "fundingAddress": "simpleledger:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vryrap6mu",
-     "fundingWif": "L2xvTe6CdNxroR6pbdpGWNjAa55AZX5Wm59W5TXMuH31ihNJdDjt",
-     "legacyAddress": "1511T3ynXKgCwXhFijCUWKuTfqbPxFV1AF",
-     "slpAddress": "simpleledger:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vryrap6mu",
+     "cashAddress": "bchtest:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vtdvpkdz7",
+     "fundingAddress": "slptest:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vset6v6sr",
+     "fundingWif": "cTKuvZ644Sf7xra5z3dPshEECJNaDyBCq7HyBsysQPh1ySSpxtQ1",
+     "legacyAddress": "mjWxk74mLM7TieAsSJArLF7nXqC6oc2mof",
+     "publicKey": "02fe5308d77bcce825068a9e46adc6f032dbbe39167a7b6d05ac563ac71d8b186e",
+     "slpAddress": "slptest:qq47pcxfn8n7w7jy86njd7pvgsv39l9f9vset6v6sr",
    },
    "Path1899": Object {
      "cashAddress": "bchtest:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7c9ex06hrx",
      "fundingAddress": "slptest:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7c7dp5qq3m",
      "fundingWif": "cNRFB6MmkNhyhAj1TpGhXdbHgzWg4BsdHbAkKjiz4vt4vwgpC44F",
      "legacyAddress": "mxX888y8yaPpTYh3WhrB9GEkT3cgumYwPw",
      "publicKey": "02a06bb380cf180d703f6f80796a13555aefff817d1f6f842f1e5c555b15f0fa70",
      "slpAddress": "slptest:qzagy47mvh6qxkvcn3acjnz73rkhkc6y7c7dp5qq3m",
    },
    "Path245": Object {
-     "cashAddress": "bitcoincash:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy54hkry298",
-     "fundingAddress": "simpleledger:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5evac32me",
-     "fundingWif": "KwgNkyijAaxFr5XQdnaYyNMXVSZobgHzSoKKfWiC3Q7Xr4n7iYMG",
-     "legacyAddress": "1EgPUfBgU7ekho3EjtGze87dRADnUE8ojP",
-     "slpAddress": "simpleledger:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5evac32me",
+     "cashAddress": "bchtest:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy539jyxazm",
+     "fundingAddress": "slptest:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5234lu2sx",
+     "fundingWif": "cN3NDtiabeeX1Wzg2CPgLgrb7fsDG8PgWqTnmwAhYWmY6osSta7Q",
+     "legacyAddress": "muCLmiGfH961UuWrTTFNU3KxH9pVQJJx6Z",
+     "publicKey": "03c4a69fd90c8b196683216cffd2943a7b13b0db0812e44a4ff156ac7e03fc4ed7",
+     "slpAddress": "slptest:qztqe8k4v8ckn8cvfxt5659nhd7dcyvxy5234lu2sx",
    },
    "mnemonic": "apart vacuum color cream drama kind foil history hurt alone ask census",
    "name": "MigrationTestAlpha",
  }
    at Object.<anonymous> (/work/web/cashtab/src/hooks/__tests__/useWallet.test.js:63:20)

Each failure log is accessible here:
CashTab Unit Tests: Migrating legacy wallet on testnet

Need to find out why this unit test is failing.

This revision now requires changes to proceed.Nov 30 2021, 19:49
web/cashtab/src/hooks/useWallet.js
393 ↗(On Diff #31162)

This causes the unit test failure.
Previously, it only generated new account on Path1899.
With this update, it re-generates wallets/accounts on Path245 and Path145 as well, so that they now have public keys.
This code correctly creates wallets/accounts on testnet, with addresses in testnet format. However, the unit test specified addresses in mainnet format.
Simple fix would be to update the unit test with the correct testnet addresses.

Why are the private keys also changing?

The failure in "Migrating legacy wallet on testnet" was caused by different returned data on Path145 and Path245. Path1899 has the correct data as expected (except additional publicKey).

Comparing two unit tests, "Migrating legacy wallet on testnet" vs "Migrating legacy wallet on mainnet".
Looking at the Path1899, we can see that the two test cases expected 2 different private keys.
From this, I am assuming that private key generated for a path on the testnet is expected to be different from the private key for the same path on the mainnet.

From this reasoning, the private keys for Path145 and Path245 on the testnet did not change. The failed unit test specified the wrong expected data.

This can be further confirmed by looking at the mockLegacyWallets.js file, the same wallet mockLegacyWallets.legacyAlpha (which is a mainnet wallet) is used for for both tests on mainnet and testnet.

The "Migrating legacy wallet on testnet" unit test should be using a testnet wallet instead of a mainet wallet. This could be another diff.

This unit test did not fail in all previous diffs because we did not re-create accounts/wallets on the testnet for Path145 and Path245, only Path1899 was created.

Ok -- please correct the mock data for this unit test. Thanks.

rebase to the latest master - that fixes the unit test failure

This revision is now accepted and ready to land.Dec 2 2021, 21:59