⚡️ [FEAT] : Integrate Trustchain logic in LedgerSync LLD - V1 #7131

mcayuelas-ledger · 2024-06-18T08:12:21Z

✅ Checklist

npx changeset was attached.
Covered by automatic tests.
Impact of the changes:
- Trustchain Store is persistent
- Integration of Trustchain
  - Add Member
  - Remove Member
  - Destroy Trustchain

📝 Description

Integrates the trustchain lib and store integration on the actual UI of the trustchain flows

Arch reworked
Hooks dedicated

❓ Context

JIRA or GitHub link: LIVE-12631

🧐 Checklist for the PR Reviewers

The code aligns with the requirements described in the linked JIRA or GitHub issue.
The PR description clearly documents the changes made and explains any technical trade-offs or design decisions.
There are no undocumented trade-offs, technical debt, or maintainability issues.
The PR has been tested thoroughly, and any potential edge cases have been considered and handled.
Any new dependencies have been justified and documented.
Performance considerations have been taken into account. (changes have been profiled or benchmarked if necessary)

…ns-screen-Ws

vercel · 2024-06-18T08:12:24Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

5 Skipped Deployments

Name	Status	Preview	Updated (UTC)
ledger-live-docs	⬜️ Ignored (Inspect)	Visit Preview	Jul 9, 2024 9:02am
ledger-live-github-bot	⬜️ Ignored (Inspect)	Visit Preview	Jul 9, 2024 9:02am
native-ui-storybook	⬜️ Ignored (Inspect)	Visit Preview	Jul 9, 2024 9:02am
react-ui-storybook	⬜️ Ignored (Inspect)	Visit Preview	Jul 9, 2024 9:02am
web-tools	⬜️ Ignored (Inspect)	Visit Preview	Jul 9, 2024 9:02am

gre

some feedbacks

gre · 2024-06-18T08:18:38Z

...ive-desktop/src/newArch/WalletSync/Flows/Activation/03-ActivationOrSynchroWithTrustchain.tsx

-      !hasError && stuffHandledByTrustchain();
-    }, 3000);
-  }, [hasError, stuffHandledByTrustchain]);
+    !hasError && stuffHandledByTrustchain();


improvements i see:

we need to handle possible error thrown by stuffHandledByTrustchain and display them to user (using the Generic Error rendering which should tell the user the precise error instead of the UnsecuredError )

Q: is there a way, as a user, to interrupt the flow? because we don't handle the unsubscribe of these, and it's possible that runWithDevice never ends which can introduce race conditions

gre · 2024-06-18T08:21:55Z

apps/ledger-live-desktop/src/newArch/WalletSync/Flows/ManageBackup/useDeleteData.ts

+  const memberCredentials = useSelector(memberCredentialsSelector);
+
+  if (!trustchain || !memberCredentials) {
+    throw new Error("trustchain or memberCredentials is missing");


dangerous to throw in a hook.
to me, instead we should return a callback that will do nothing if trustchain is falsy, because in that case there will be no more things to delete.

gre · 2024-06-18T08:30:36Z

apps/ledger-live-desktop/src/newArch/WalletSync/Flows/Synchronize/02-QRCodeStep.tsx

+        setUrl(url);
+      },
+      onDisplayDigits: digits => {
+        dispatch(setQrCodePinCode(digits)); // Should we assert that it is a string of 3 digits ?


actually today it's "3" but it could be something else, this is a config in the protocol of QR Code, it could be changed in future and UI just need to adapt to the number of digits ideally: https://github.com/LedgerHQ/ledger-live/blob/develop/libs/trustchain/src/qrcode/index.ts#L155

gre · 2024-06-18T08:33:04Z

apps/ledger-live-desktop/src/newArch/WalletSync/useTrustchainSdk.ts

+  const trustchain = useSelector(trustchainSelector);
+  const memberCredentials = useSelector(memberCredentialsSelector);
+
+  if (!trustchain || !memberCredentials) {


i think we should be able to useLiveAuthenticate with these falsy, it hsouldn't throw from the hook.
there will be error cases where we will need to reset the trustchain (e.g. when member is ejected)

as auth(trustchain: Trustchain, memberCredentials: MemberCredentials): Promise<JWT>;
We can't :/

gre · 2024-07-08T08:07:24Z

libs/trustchain/src/qrcode/index.ts

@@ -34,7 +34,7 @@ export async function createQRCodeHostInstance({
 }): Promise<void> {
  const ephemeralKey = await crypto.randomKeypair();
  const publisher = crypto.to_hex(ephemeralKey.publicKey);
-  const url = `${getEnv("TRUSTCHAIN_API").replace("http", "ws")}/v1/qr?host=${publisher}`;
+  const url = `${getEnv("TRUSTCHAIN_API")}/v1/qr?host=${publisher}`.replace(/^http/, "ws");


can you undo this change? the develop's version is correct

gre · 2024-07-08T08:07:48Z

libs/trustchain/src/store.ts

 };

 export const getInitialStore = (): TrustchainStore => {
  return {
    trustchain: null,
    memberCredentials: null,
+    jwt: null,


we should undo this too, no need to store the jwt anymore

apps/ledger-live-desktop/src/renderer/storage.ts

gre · 2024-07-08T08:20:12Z

don't forget to update & document the PR description

apps/ledger-live-desktop/src/newArch/WalletSync/hooks/useTrustchainSdk.ts

LucasWerey · 2024-07-09T08:32:50Z

...ive-desktop/src/newArch/WalletSync/Flows/Activation/03-ActivationOrSynchroWithTrustchain.tsx

+    return addMemberMutation.isError ? (
+      <ErrorDisplay error={addMemberMutation.error} withExportLogs onRetry={onRetry} />
+    ) : (
+      <FollowStepsOnDevice modelId={device.modelId as DeviceModelId} />


Suggested change

<FollowStepsOnDevice modelId={device.modelId as DeviceModelId} />

<FollowStepsOnDevice modelId={device.modelId} />

And the import

--------- Co-authored-by: Come Grellard <come.grellard@ledger.fr> Co-authored-by: Gaëtan Renaudeau <renaudeau.gaetan@gmail.com>

cgrellard-ledger and others added 22 commits June 14, 2024 10:38

wip

01eddb1

Fix useFlow

46f46db

WIP 2

60182e3

wip3

6bad373

wip4 - share with qr code integrated in LLD

aefcd2c

trustchain store persistent

4201b9c

reset trustchain store when deleting all data

71097ce

gotoactivation on click on qr code hint

880209e

💄 Fix Final step display in Sync

c2713fd

💄 Fix Final step display in Sync

6a5c86e

WIP

d35f2cb

wip

8b4067a

Fix useFlow

e61a6f5

WIP 2

8ad1ef8

wip3

f2e1bd4

wip4 - share with qr code integrated in LLD

5adcb1b

trustchain store persistent

6073ae4

reset trustchain store when deleting all data

a185569

gotoactivation on click on qr code hint

698a9e5

💄 Fix Final step display in Sync

330f410

💄 Fix Final step display in Sync

e747a9e

Merge branch 'feat/integrate-trustchain-logic' into feat/device-actio…

57c0656

…ns-screen-Ws

live-github-bot bot added desktop Has changes in LLD mobile Has changes in LLM translations Translation files have been touched labels Jun 18, 2024

github-actions bot assigned mcayuelas-ledger Jun 18, 2024

gre reviewed Jun 18, 2024

View reviewed changes

DeviceAction

29ac532

vercel bot deployed to Preview – web-tools June 18, 2024 08:45 View deployment

vercel bot deployed to Preview – react-ui-storybook July 4, 2024 15:45 View deployment

mcayuelas-ledger added 3 commits July 5, 2024 09:36

WIP migrate without JWT

cf55473

Merge branch 'develop' into feat/integrate-trustchain-logic

184afa7

Migrate to new JWT cycle

38f3091

mcayuelas-ledger force-pushed the feat/integrate-trustchain-logic branch from 7181e83 to 38f3091 Compare July 5, 2024 12:17

vercel bot had a problem deploying to Preview – web-tools July 5, 2024 12:19 Failure

mcayuelas-ledger added 2 commits July 5, 2024 15:32

Fix device triggering

9bdec28

First try fixing tests

83bf551

gre reviewed Jul 8, 2024

View reviewed changes

Merge branch 'develop' into feat/integrate-trustchain-logic

55807ad

vercel bot had a problem deploying to Preview – web-tools July 8, 2024 09:31 Failure

Fix review

4a39257

vercel bot had a problem deploying to Preview – web-tools July 8, 2024 09:35 Failure

chore: improve member name

8776e24

mcayuelas-ledger commented Jul 8, 2024

View reviewed changes

apps/ledger-live-desktop/src/newArch/WalletSync/hooks/useTrustchainSdk.ts Show resolved Hide resolved

mcayuelas-ledger and others added 3 commits July 9, 2024 10:14

✅ Fix integration tests

606e306

changeset

82564e7

Merge branch 'develop' into feat/integrate-trustchain-logic

de66853

vercel bot had a problem deploying to Preview – web-tools July 9, 2024 08:26 Failure

Fix lint

abb81aa

LucasWerey reviewed Jul 9, 2024

View reviewed changes

Review

de552f3

gre approved these changes Jul 9, 2024

View reviewed changes

mcayuelas-ledger requested a review from LucasWerey July 9, 2024 10:51

LucasWerey approved these changes Jul 9, 2024

View reviewed changes

mcayuelas-ledger merged commit 08e85c3 into develop Jul 9, 2024
54 of 56 checks passed

mcayuelas-ledger deleted the feat/integrate-trustchain-logic branch July 9, 2024 12:10

gre added a commit that referenced this pull request Jul 9, 2024

⚡️ [FEAT] : Integrate Trustchain logic in LedgerSync LLD - V1 (#7131)

16975d9

--------- Co-authored-by: Come Grellard <come.grellard@ledger.fr> Co-authored-by: Gaëtan Renaudeau <renaudeau.gaetan@gmail.com>

KVNLS pushed a commit that referenced this pull request Jul 19, 2024

⚡️ [FEAT] : Integrate Trustchain logic in LedgerSync LLD - V1 (#7131)

4e5f322

--------- Co-authored-by: Come Grellard <come.grellard@ledger.fr> Co-authored-by: Gaëtan Renaudeau <renaudeau.gaetan@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

⚡️ [FEAT] : Integrate Trustchain logic in LedgerSync LLD - V1 #7131

⚡️ [FEAT] : Integrate Trustchain logic in LedgerSync LLD - V1 #7131

mcayuelas-ledger commented Jun 18, 2024 •

edited by jira bot

Loading

vercel bot commented Jun 18, 2024 •

edited

Loading

gre left a comment

gre Jun 18, 2024

gre Jun 18, 2024

gre Jun 18, 2024

gre Jun 18, 2024

mcayuelas-ledger Jun 18, 2024

gre Jul 8, 2024

gre Jul 8, 2024

gre commented Jul 8, 2024

LucasWerey Jul 9, 2024

LucasWerey Jul 9, 2024

	<FollowStepsOnDevice modelId={device.modelId as DeviceModelId} />
	<FollowStepsOnDevice modelId={device.modelId} />

⚡️ [FEAT] : Integrate Trustchain logic in LedgerSync LLD - V1 #7131

⚡️ [FEAT] : Integrate Trustchain logic in LedgerSync LLD - V1 #7131

Conversation

mcayuelas-ledger commented Jun 18, 2024 • edited by jira bot Loading

✅ Checklist

📝 Description

❓ Context

🧐 Checklist for the PR Reviewers

vercel bot commented Jun 18, 2024 • edited Loading

gre 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

gre commented Jul 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcayuelas-ledger commented Jun 18, 2024 •

edited by jira bot

Loading

vercel bot commented Jun 18, 2024 •

edited

Loading