[LIVE-8890] remove ethereum family code #4336

chabroA · 2023-08-11T14:50:41Z

📝 Description

Remove all ethereum family related code from live, including:

ethereum family folder under LLD
ethereum family folder under LLM
ethereum family folder under LLC
and all direct import or reference to ethereum family code in other files

For easier review, it is recommended to review commit by commit.
Commits have been broken down atomically and ordered and a sensible way.

The biggest commit is a72bfd8 that include the removal of the ethereum family folder from LLD, LLM and LLC. Feels free to skip it or just glance over the list of removed files.

FYI this commit's changes (aec91bd) are not critical since this is just an update of some legacy code that will be removed by #4317 (but it still works as expected)

I smoked tested the PR by launching LLD and LLM in dev, both apps starts without issue and I can navigate to ethereum currencies.
A full non reg test will be performed on the main PR #4285 after this one is merged. This does not make sense to do it at this stage.

This PR also includes changes related to https://ledgerhq.atlassian.net/browse/LIVE-8188 (in this commit 2629450) since this logic was not present in the evm family.

Note 📝:

Speed up / cancel tx
Some logic related to speed up / cancel tx has been commented out instead of removed (cf. b9a7b56) to keep a framework that can be used for the integration of this feature on evm, which should be done shortly.
Also to keep this very PR specific to ethereum code removal and not scope creep it by adding removal of speed up / cancel feature.
We could decide to remove all the speed up / cancel related code present in non family specific code, until we re-integrate the feature within the evm family (which should be done right after this ethereum / evm migration anyway), but then it should be done in it's own dedicated PR.
Evm bridge mock
A test-suite has been 59f99ce voluntarily (instead of removed) until the evm bridge mock is available. cf. Support/LIVE-7931 Evm mock test #4353

❓ Context

Impacted projects: LLC LLD LLM
Linked resource(s): https://ledgerhq.atlassian.net/browse/LIVE-8890 https://ledgerhq.atlassian.net/browse/LIVE-8188

✅ Checklist

Test coverage
Atomic delivery
No breaking changes Well, we are removing a full family so... 🤷‍♂️

📸 Demo

🚀 Expectations to reach

Please make sure you follow these Important Steps.

Pull Requests must pass the CI and be internally validated in order to be merged.

changeset-bot · 2023-08-11T14:50:45Z

⚠️ No Changeset found

Latest commit: 90d7feb

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

vercel · 2023-08-11T14:50:49Z

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

Name	Status	Preview	Comments	Updated (UTC)
ledger-live-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 16, 2023 2:12pm
live-common-tools	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Aug 16, 2023 2:12pm

3 Ignored Deployments

Name	Status	Preview	Updated (UTC)
ledger-live-github-bot	⬜️ Ignored (Inspect)	Visit Preview	Aug 16, 2023 2:12pm
native-ui-storybook	⬜️ Ignored (Inspect)	Visit Preview	Aug 16, 2023 2:12pm
react-ui-storybook	⬜️ Ignored (Inspect)	Visit Preview	Aug 16, 2023 2:12pm

lvndry · 2023-08-16T08:50:44Z

apps/ledger-live-desktop/src/renderer/components/OperationsList/EditOperationPanel.tsx

-        transactionHash: operation.hash,
-      }),
-    );
+    // dispatch(


There's an other draft PR with edit evm transaction right ? If yes we can delete this instead of commenting

There is indeed a draft attempt to port this feature to evm here -> #4217

But since the changes commented out as part of this commit b9a7b56 are not necessarily ethereum (or evm) specific (because present in non ethereum / evm family folders) I decided to only comment them as part of this PR.
Because these changes present in non family specific folders might not be present in the draft PR above (that only port the ethereum family logic to evm).

We could decide to remove all of the speed up / cancel tx code for now, until it's added back again in the evm family, but it should be done in it's own dedicated PR to allow easier revert and since it's outside the scope of this one (and also to avoid growing this already massive PR with even more code removal).
But removing all this code might not be strategically wise since the work to integrate speed up / cancel tx to evm should be (will be) done juste after this evm / ethereum family migration.

Description updated 👍

lvndry · 2023-08-16T08:52:43Z

apps/ledger-live-desktop/src/renderer/modals/Send/fields/RecipientField.react.test.tsx

@@ -125,16 +125,15 @@ const baseMockTransaction: Transaction = {
  recipient: "",


Not related to PR but .react.test.tsx is a bit weird
.tsx already means it's a react file

No strong opinion on that.
But I think this is a convention chosen to differentiate "dom" related tests from non "dom" related tests.

cf. this config for example -> https://github.com/LedgerHQ/ledger-live/blob/develop/libs/domain-service/jest.config.js#L38-L51

But yes, outside of the scope of this PR so won't touch it here / now

libs/ledger-live-common/src/families/evm/walletApiAdapter.test.ts

kept commented for now to keep a framework that can be used for the integration of this feature on evm, which should be done shortly

Untill evm bridge mock is available

PS: this code is legacy and will be removed by this PR #4317

…il evm bridge is available

hzheng-ledger · 2023-08-16T14:13:54Z

apps/ledger-live-desktop/src/renderer/drawers/OperationDetails/index.tsx

+    _transactionRaw: unknown,
+    _transactionHash: unknown,
+  ) => {};
+


line 402-423 in this file, we should remove the alert message for editable operation since we remove the edit tx feature.

hzheng-ledger · 2023-08-16T14:18:30Z

apps/ledger-live-desktop/src/renderer/components/OperationsList/EditOperationPanel.tsx

@@ -1,9 +1,11 @@
+// FIXME: to update when implementing edit transaction on evm
+


if you want to remove the edit tx feature temporarily. you can remove this file completely or just return null.
it is not a major issue. It is up to you to keep it here or not.

lvndry · 2023-08-16T13:39:35Z

apps/ledger-live-mobile/src/components/Nft/NftViewer.tsx

-      transaction = bridge.updateTransaction(defaultTransaction, {
-        tokenIds: [nft?.tokenId],
+    const transaction = bridge.updateTransaction(defaultTransaction, {
+      mode: nft.standard.toLowerCase() as EvmNftTransaction["mode"],


I don't see a real good reason for NFTStandard and EvmNftTransaction["mode"] to be 2 different types. From what I see everytime we use NFTStandard we end up lowering the case so I think we can just use the same type for both

type EvmNftTransaction = { mode: NFTStandard; ... }

and then lowering the case of NFTStandard, or do vice verse doing

type NFTStandard = EvmNftTransaction["mode"]

Seems to make sense, will take a look at it in a dedicated / independent PR 👍

@lvndry cf. #4363 (PS: a lot of changes for a little update 🤷)

hzheng-ledger · 2023-08-16T14:29:06Z

apps/ledger-live-desktop/src/renderer/modals/Send/steps/StepConfirmation.tsx

@@ -1,8 +1,9 @@
+// FIXME: to update when implementing edit transaction on evm
+
 import React from "react";


I think you should remove the following code in
/ledger-live-desktop/src/renderer/modals/Send/steps/StepRecipient.tsx

{stuckAccountAndOperation ? ( <EditOperationPanel operation={stuckAccountAndOperation.operation} account={stuckAccountAndOperation.account} parentAccount={stuckAccountAndOperation.parentAccount} /> ) : null}

chabroA · 2023-08-16T16:18:48Z

Merging this PR with the speed up / cancel code commented (and not fully removed) in the feature branch for better topic segregation. If needed, a new branch starting from the feature branch will be created to remove this code

github-actions bot assigned chabroA Aug 11, 2023

github-actions bot added desktop Has changes in LLD mobile Has changes in LLM common Has changes in live-common labels Aug 11, 2023

add platform and wallet-api adapters to evm family

2629450

chabroA force-pushed the feat/live-8890-remove-ethereum-family-code branch from 1dac527 to c9fff1a Compare August 11, 2023 14:53

vercel bot deployed to Preview – ledger-live-docs August 11, 2023 14:55 View deployment

vercel bot deployed to Preview – live-common-tools August 11, 2023 14:57 View deployment

chabroA marked this pull request as ready for review August 11, 2023 15:04

chabroA requested a review from a team as a code owner August 11, 2023 15:04

chabroA mentioned this pull request Aug 11, 2023

[LIVE-5135] Migrate Ethereum family implementation to EVM family #4285

Merged

4 tasks

ComradeAERGO requested review from a team, Justkant and ComradeAERGO and removed request for Justkant and ComradeAERGO August 11, 2023 16:35

lvndry reviewed Aug 16, 2023

View reviewed changes

chabroA force-pushed the feat/live-8890-remove-ethereum-family-code branch from c9fff1a to fed5759 Compare August 16, 2023 09:15

vercel bot deployed to Preview – live-common-tools August 16, 2023 09:16 View deployment

vercel bot deployed to Preview – ledger-live-docs August 16, 2023 09:17 View deployment

chabroA added 7 commits August 16, 2023 13:33

comment out remaining logic related to edit eth tx

b88d7ca

kept commented for now to keep a framework that can be used for the integration of this feature on evm, which should be done shortly

comment out updateAmountUsingMax test

d46bbec

Untill evm bridge mock is available

update tests to use evm instead of ethereum as family

2625aeb

remove ethereum family files

f8c97d9

removed legacy remainder of lend screen

743fd85

remove ethereum family from generated files

c2bda62

update walletConnect code to not rely on ethereum family anymore

5cc509b

PS: this code is legacy and will be removed by this PR #4317

chabroA force-pushed the feat/live-8890-remove-ethereum-family-code branch from fed5759 to fde4aff Compare August 16, 2023 11:33

chabroA requested review from lvndry and a team August 16, 2023 11:33

github-actions bot added the ledgerjs Has changes in the ledgerjs open source libs label Aug 16, 2023

vercel bot deployed to Preview – ledger-live-docs August 16, 2023 11:35 View deployment

vercel bot deployed to Preview – live-common-tools August 16, 2023 11:37 View deployment

remove ethereum family related code from non coin specific files

1d965f3

chabroA force-pushed the feat/live-8890-remove-ethereum-family-code branch from fde4aff to fae928f Compare August 16, 2023 12:21

vercel bot deployed to Preview – live-common-tools August 16, 2023 12:25 View deployment

vercel bot deployed to Preview – ledger-live-docs August 16, 2023 12:26 View deployment

chabroA added 2 commits August 16, 2023 16:04

disable ethereum and ethereum_classic from mock-bridges test file unt…

79d728c

…il evm bridge is available

update apps polyfill to use ethereum app for evm family

90d7feb

chabroA force-pushed the feat/live-8890-remove-ethereum-family-code branch from fae928f to 90d7feb Compare August 16, 2023 14:09

vercel bot deployed to Preview – ledger-live-docs August 16, 2023 14:11 View deployment

vercel bot deployed to Preview – live-common-tools August 16, 2023 14:12 View deployment

hzheng-ledger reviewed Aug 16, 2023

View reviewed changes

lvndry reviewed Aug 16, 2023

View reviewed changes

hzheng-ledger reviewed Aug 16, 2023

View reviewed changes

chabroA merged commit 0cae97c into feat/live-5135-merge-ethereum-in-evm-family Aug 16, 2023

chabroA deleted the feat/live-8890-remove-ethereum-family-code branch August 16, 2023 16:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LIVE-8890] remove ethereum family code #4336

[LIVE-8890] remove ethereum family code #4336

chabroA commented Aug 11, 2023 •

edited

Loading

changeset-bot bot commented Aug 11, 2023 •

edited

Loading

vercel bot commented Aug 11, 2023 •

edited

Loading

lvndry Aug 16, 2023

chabroA Aug 16, 2023

chabroA Aug 16, 2023

lvndry Aug 16, 2023

chabroA Aug 16, 2023

hzheng-ledger Aug 16, 2023

hzheng-ledger Aug 16, 2023

lvndry Aug 16, 2023

chabroA Aug 16, 2023

chabroA Aug 16, 2023

hzheng-ledger Aug 16, 2023

chabroA commented Aug 16, 2023

		@@ -125,16 +125,15 @@ const baseMockTransaction: Transaction = {
		recipient: "",

		@@ -1,9 +1,11 @@
		// FIXME: to update when implementing edit transaction on evm

		@@ -1,8 +1,9 @@
		// FIXME: to update when implementing edit transaction on evm

		import React from "react";

[LIVE-8890] remove ethereum family code #4336

[LIVE-8890] remove ethereum family code #4336

Conversation

chabroA commented Aug 11, 2023 • edited Loading

📝 Description

❓ Context

✅ Checklist

📸 Demo

🚀 Expectations to reach

changeset-bot bot commented Aug 11, 2023 • edited Loading

⚠️ No Changeset found

vercel bot commented Aug 11, 2023 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chabroA commented Aug 16, 2023

chabroA commented Aug 11, 2023 •

edited

Loading

changeset-bot bot commented Aug 11, 2023 •

edited

Loading

vercel bot commented Aug 11, 2023 •

edited

Loading