[LW-9499] New wallet manager migration #833

lucas-barros · 2024-01-11T18:55:28Z

Checklist

JIRA - https://input-output.atlassian.net/browse/LW-9499
Proper tests implemented
Screenshots added.

Proposed solution

Creates migration for newwallet manager

Testing

Unlocked wallet test

Install the extension with main branch
Create/import new wallet
Build this branch, make sure the version in the manifest file is higher then that of the main branch (you might have to manually modify it)
Install the extension with the new build
Open lace and make sure it is working properly (to make sure the migration ran, verify that lock and keyAgentData are not present in localStorage)

Locked wallet test

Repeat the process but lock the wallet after step 2.

Screenshots

Attach screenshots here if implementation involves some UI changes

refactor: use internal wallet address discovery

In order to support multi-wallet/account & shared wallets, walletManager was completely redesigned in SDK. There are a couple new components: - WalletRepository stores key agent data, used accounts, as well as any additional metadata such as 'name'. Lace should use it to get an array of all users wallets. - SignerManager is an abstraction over KeyAgent. Lace now longer needs to maintain the lifecycle of a KeyAgent. Instead, it should use this component to sign transactions.

github-actions · 2024-01-11T19:53:44Z

Allure report

allure-report-publisher generated test report!

smokeTests: ❌ test report for 1c0b289d

	passed	failed	skipped	flaky	total	result
Total	1	30	0	0	31	❌

mkazlauskas

Looks good! Just a question below

apps/browser-extension-wallet/src/lib/scripts/migrations/versions/v1_8_2.ts

lucas-barros · 2024-01-17T12:44:20Z

apps/browser-extension-wallet/src/lib/scripts/migrations/versions/v1_8_2.ts

+          rootPrivateKeyBytes: HexBlob.fromBytes(Buffer.from(data.encryptedRootPrivateKeyBytes)),
+          keyMaterial: HexBlob.fromBytes(Buffer.from(JSON.parse(mnemonic).data))
+        },
+        metadata: {


@mkazlauskas @DominikGuzei is this aligned with input-output-hk/cardano-js-sdk#1050?

Looks good to me

yeah looks correct 👍

lucas-barros · 2024-01-17T12:45:29Z

apps/browser-extension-wallet/src/lib/scripts/migrations/versions/v1_8_2.ts

+
+          await walletRepository.addAccount({
+            accountIndex: keyAgentData.accountIndex,
+            metadata: { name: 'Account #0', lockValue },


@mkazlauskas now that the wallet will have metadata, does it make sense to add lockValue to the wallet level?

I think so. Depends whether we're locking wallet, account or entire Lace. Product question 🤔

So actually I think it makes more sense to only have it per wallet (not account), because it is encrypted with a passphrase that applies to all accounts

mkazlauskas · 2024-01-18T13:06:42Z

apps/browser-extension-wallet/src/lib/scripts/migrations/versions/v1_8_2.ts

+          removeItemFromLocalStorage(LOCK_STORAGE);
+        }
+
+        removeItemFromLocalStorage(KEY_AGENT_DATA_STORAGE);


This should also be obsolete

Suggested change

removeItemFromLocalStorage(KEY_AGENT_DATA_STORAGE);

removeItemFromLocalStorage(KEY_AGENT_DATA_STORAGE);

removeItemFromLocalStorage('wallet');

Updated fe614f5

sonarqubecloud · 2024-01-18T17:14:27Z

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

coveralls · 2024-01-18T17:32:31Z

Changes unknown
when pulling 1c0b289 on feat/lw-9499-migration
into ** on LW-9094-new-wallet-manager**.

DominikGuzei · 2024-01-19T11:43:16Z

@lucas-barros the migration seems to be working for me now 👍 for me the wallet is getting locked after the migration (even if i never locked the wallet before), is this intentional? Btw. it's not locked anymore after closing the popup and re-opening it, so it doesn't seem to be enforced either.

lucas-barros · 2024-01-19T12:26:58Z

@lucas-barros the migration seems to be working for me now 👍 for me the wallet is getting locked after the migration (even if i never locked the wallet before), is this intentional? Btw. it's not locked anymore after closing the popup and re-opening it, so it doesn't seem to be enforced either.

@DominikGuzei did you update the manifest file version to a higher then your current version? If not, likely the migration didnt run, the new build was loaded once (which will "lock" the wallet), then the older version would be reload due to the version being higher. What does your local storage look like after the migration?

mchappell and others added 7 commits December 8, 2023 12:47

chore: update cardano-sdk packages

ec20f46

refactor: update lace based on cardano-sdk breaking changes

81d41e0

refactor: use internal wallet address discovery

chore: bump sdk versions (#782)

035160c

chore: bump sdk version

86ef8d0

refactor(extension): use message sender to get dapp information

d3df0f6

feat: add lock and key angent data migration

fb127d1

github-actions bot added the browser Changes to the browser application. label Jan 11, 2024

Lucas Araujo added 4 commits January 12, 2024 10:12

feat: check if wallet already exists in repository

95da612

feat: activate wallet

e0436a7

feat: handle locked wallet case

0a49a4e

feat: reset lock

48fc319

lucas-barros changed the title ~~feat: add lock and key angent data migration~~ [LW-9499] New wallet manager migration Jan 16, 2024

lucas-barros marked this pull request as ready for review January 16, 2024 00:15

lucas-barros requested a review from a team as a code owner January 16, 2024 00:15

lucas-barros requested a review from mkazlauskas January 16, 2024 00:16

lucas-barros self-assigned this Jan 16, 2024

mkazlauskas approved these changes Jan 16, 2024

View reviewed changes

apps/browser-extension-wallet/src/lib/scripts/migrations/versions/v1_8_2.ts Outdated Show resolved Hide resolved

mkazlauskas requested a review from a team January 16, 2024 09:16

Lucas Araujo and others added 6 commits January 16, 2024 09:20

fix: remove unused

8859572

fixup! refactor(extension): [LW-9094] use new wallet manager

eb48539

fixup! refactor(extension): [LW-9094] use new wallet manager

aa44aba

fixup! refactor(extension): [LW-9094] use new wallet manager

2e1feb6

fixup! refactor(extension): [LW-9094] use new wallet manager

3947c1b

feat: add metadata to wallet

450f961

lucas-barros commented Jan 17, 2024

View reviewed changes

mkazlauskas added 2 commits January 17, 2024 14:55

fixup! refactor(extension): [LW-9094] use new wallet manager

80b1fe0

ci: set higher memory limit for running ci unit tests

b0083fe

Merge branch 'LW-9094-new-wallet-manager' into feat/lw-9499-migration

daa9f96

mkazlauskas requested changes Jan 18, 2024

View reviewed changes

Lucas Araujo added 3 commits January 18, 2024 10:53

fix: wallet name

eef60ea

feat: remove old wallet metadata

fe614f5

feat: remove wallet in rollback

0b2b8ab

lucas-barros requested a review from mkazlauskas January 18, 2024 16:38

fix: add no wallets check

1c0b289

lucas-barros requested a review from DominikGuzei January 18, 2024 18:17

mkazlauskas force-pushed the LW-9094-new-wallet-manager branch from 88123d3 to c68437d Compare January 26, 2024 08:12

lucas-barros requested review from a team January 26, 2024 08:12

mkazlauskas force-pushed the LW-9094-new-wallet-manager branch 10 times, most recently from ccda020 to 1266abb Compare January 29, 2024 07:44

mkazlauskas force-pushed the LW-9094-new-wallet-manager branch 3 times, most recently from 55b223d to d8e3ead Compare February 2, 2024 15:02

Base automatically changed from LW-9094-new-wallet-manager to main February 13, 2024 17:07

rhyslbw marked this pull request as draft September 13, 2024 08:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LW-9499] New wallet manager migration #833

[LW-9499] New wallet manager migration #833

lucas-barros commented Jan 11, 2024 •

edited

Loading

github-actions bot commented Jan 11, 2024 •

edited

Loading

mkazlauskas left a comment

lucas-barros Jan 17, 2024

mkazlauskas Jan 17, 2024

DominikGuzei Jan 17, 2024

lucas-barros Jan 17, 2024

mkazlauskas Jan 17, 2024

mkazlauskas Jan 18, 2024

mkazlauskas Jan 18, 2024

lucas-barros Jan 18, 2024

sonarqubecloud bot commented Jan 18, 2024

coveralls commented Jan 18, 2024

DominikGuzei commented Jan 19, 2024

lucas-barros commented Jan 19, 2024

	removeItemFromLocalStorage(KEY_AGENT_DATA_STORAGE);
	removeItemFromLocalStorage(KEY_AGENT_DATA_STORAGE);
	removeItemFromLocalStorage('wallet');

[LW-9499] New wallet manager migration #833

Are you sure you want to change the base?

[LW-9499] New wallet manager migration #833

Conversation

lucas-barros commented Jan 11, 2024 • edited Loading

Checklist

Proposed solution

Testing

Screenshots

github-actions bot commented Jan 11, 2024 • edited Loading

Allure report

mkazlauskas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Jan 18, 2024

Quality Gate passed

coveralls commented Jan 18, 2024

DominikGuzei commented Jan 19, 2024

lucas-barros commented Jan 19, 2024

lucas-barros commented Jan 11, 2024 •

edited

Loading

github-actions bot commented Jan 11, 2024 •

edited

Loading