LedgerHQ / ledger-live

Mono-repository for packages related to Ledger Live and its JavaScript ecosystem.
MIT License
434 stars 327 forks source link

[Bug]: Latest version of @ledgerhq/hw-app-eth has breaking changes #4062

Closed fvictorio closed 1 year ago

fvictorio commented 1 year ago

Impacted Library name

@ledgerhq/hw-app-eth

Impacted Library version

6.34.0

Describe the bug

We have a hardhat-ledger plugin in our monorepo, and we also have a CI workflow that tries to build and run our tests with the latest versions of all our dependencies.

The latest version of the eth app library broke this workflow because, as far as I can tell, it has some breaking changes (at the very least the isEIP712Message was removed from the exports; there might be others).

For now we fixed this by pinning the version we depend on. But, assuming you are following semver, I think this was a mistake.

Expected behavior

A new minor version shouldn't include breaking changes.

Additional context

No response

pcaversaccio commented 1 year ago

I also faced this issue this morning and I realised that the PR https://github.com/LedgerHQ/ledger-live/pull/3924 broke it. To be fully straightforward, that shouldn't be a minor changeset...

image

Schoonmoeder commented 1 year ago

Also have the issue of missing functions in EVM-tools while hw-app-eth thinks they should be there

example : sortObjectAlphabetically is removed but not exported in evm removed here https://github.com/LedgerHQ/ledger-live/pull/3924/files#diff-0014e583278c639b19797bb7a95d5ae400802e7f91bb8d44544fd66c7e9fa62f the pr @pcaversaccio talks about.

Snip20230719_1

lambertkevin commented 1 year ago

Sorry about that guys, no breaking change per se in the new release, just a bundling/releasing problem with the new evm-tools lib. PR https://github.com/LedgerHQ/ledger-live/pull/4090 is fixing this, a new patch should come soon without the issue 🙏

fvictorio commented 1 year ago

just a bundling/releasing problem with the new evm-tools lib

Glad to know it was just that, thanks for the update! I'll re-enable the ^ dependency range after this is fixed then.

svarcoder commented 1 year ago

Hi Guys! I installed only two package and my node v is 18 "@ledgerhq/cryptoassets": "^7.0.0", "@ledgerhq/hw-transport-webusb": "^6.27.7",

But it throws same error Module not found: Can't resolve '@ledgerhq/evm-tools/message/EIP712/index'

Can you tell me how to resolve this and which version I will install for this two package?

Screenshot 2023-07-24 at 5 08 49 PM Screenshot 2023-07-24 at 5 08 19 PM Screenshot 2023-07-24 at 5 07 55 PM
lambertkevin commented 1 year ago

Can you all please try with @ledgerhq/hw-app-eth in version 6.34.1 and let me know if it fixes your issues ?

Schoonmoeder commented 1 year ago

For me, it solves the initial problem.

only as I'm using Electron it gives me a USB device in use so I can't connect yet. but believe this is not related to this issue

svarcoder commented 1 year ago

I used 6.34.1 v but now this error throws

Screenshot 2023-07-25 at 1 14 48 PM
lvndry commented 1 year ago

@svarcoder Do you still have the issue ?

toxeus commented 1 year ago

@lvndry for me the latest release has fixed the missing dependency issue. To my surprise a dependency on @ledgerhq/live-network was introduced. I didn't expect @ledgerhq/hw-app-eth to open remote connections. Was that intended?

svarcoder commented 1 year ago

@svarcoder Do you still have the issue ? Yes. Now the issue is resolved.