Open lambertkevin opened 8 months ago
This issue is relative to upgrade ledger library @ledgerhq/hw-app-eth in the future to include the new feature, @ledgerhq/hw-app-eth
library use subpath exports feature, and that future will not cause issue during extension compile and deploy the bundle to github page during deployment process (likely caused by browserify
library compile es6 syntax to browser compatible version of javascript code).
Currently our extension repo use following library @metamask/eth-ledger-bridge-keyring which will use @ledgerhq/hw-app-eth
, any upgrade of @ledgerhq/hw-app-eth
will be done in that library repo.
during some rough research in google, there is not good solution to fix this issue, we may need to do further detail investigation when we decide to upgrade @ledgerhq/hw-app-eth
to support new EIP712 clear signed feature.
Here is document i found for EIP712 clearing signed feature in ledger
Some more information for EIP-712 typed Sign V4 in both metamask extension and ledger.
I have tests my local metamask extensions with EIP-712 typed sign V4, and i can read the sign content detail in popup:
after i click the sign
button in metamask popup, ledger will receive that typedSignV4 requests, however in ledger screen, the content display as domain hash
which is not clear message to user.
so if just for EIP-712 typed sign feature in metamask extensions, it is not very urgent, but @ledgerhq/hw-app-eth
latest library may contain some more feature update. we need to check their release notes to confirm.
Try to run the gh-pages
branch, and discover it hasn't been updated for a year, and the original gh-pages
branch has already been broken for more than a year. probablly need to talk to maintenaner whether we still need this gh-pages
branch first then we need another story to fix the branch first before doing this story..
After some investigation in the gh-pages
branch, in order to make latest @ledgerhq library work, we need to get rid of browserify
library (used to enable require(module)
in browser), but consider current modern browser in firefox and chrome all support es6, we probably can get rid all old browserify
, gulp
in the branch, and replace with webpack 5
or parceljs
to make the build bundle work. This story isn't a simple task, will require a lot of change to current project library usage.
following are the list of tasks involved.
browserify
and gulp
as build tools in the project. (estimate, 1 day
)parcel
, and configure the package.json
with extra command and support browser version based on following document https://parceljs.org/getting-started/webapp/ (est, 1 day
)@ledgerhq
relative libraries to latest. (ps. @ledgerhq/hw-transport-u2f
is deprecated, do we need to replace it to @ledgerhq/hw-transport-webusb
in this story? ) (est. 2 days
without replace hw-transport-u2f
, 3 - 4 daysfor replacing
hw-transport-u2f`)ledger-bridge.js
with latest code. (est, 1 day
) est 0.5 day
)est 0.5 day
)if we upgrade the @ledgerhq/hw-app-eth
library in gh-pages
branch, we may need to upgrade the library in main
branch, which will be extract work. (est 1 day
)
So total work will take proximately 1 week and half to complete this story.
Unfortunately, after trying to implement the new EIP712 clear signed method from @ledgerhq/hw-app-eth, we discovered that the bundler used in the
gh-pages
branch isn't compatible with subpath exports used by the Ledger librairies in order to provide both commonjs and esmodules versions.Documentation talking about it is very hard to find, I could only find 1 issue talking about it here.
Parcel on its end has recently been updated in May to support this: https://parceljs.org/features/dependency-resolution/#package-exports.
Without a removal of Browserify, it doesn't look possible for Ledger to update its packages to the latest version and include the new features built into it.