ethers.js
ethers.js copied to clipboard
Fix LedgerSigner for EIP1559
The current LedgerSigner does not pass the correct TX parameters for EIP1559, which results in the gas price being zero and therefore the transaction never being sent.
I've just realised #1971 does the same.
Tested locally, and it worked for me.
This PR is very useful, anything that I can do to help get it over the line?
Hi, @ricmoo how can we help to get this one merged and released? Since the London fork on the xDai network, a few days ago, it is not possible to send a tx using a ledger wallet 🤕
My problem was related to this article. In case someone else experience a similar problem: https://www.ledger.com/blog/metamask-and-ledger-integration-fixed-new-partnership-between-metamask-and-ledger-to-ensure-ease-of-use-and-security-for-our-shared-customers
Tested this locally and it worked for me too
Please, can you merge this if it works ?
@ricmoo I have tested this PR, and it appears to work. Others have also reported testing this, and it appears ready to merge. Can we merge this for the next release? This bug is causing trouble with my hardhat scripts requiring me to use the old transaction format awaiting this PR. This PR has been ready for over a year, and I would like to know if you're awaiting anything before merging this in. If you need help or need to see something more before merging in, I would gladly offer it to get this code in the next release.
I was not planing to re-add this into the main package in v5, as I'm trying to minimize changes in preparation for v6 (in v6 this is a separate package, which makes things much simpler). It should be fairly simple to break out the files into a new package, or even just include them directly in specific projects as an additional single file.
The CI gets complicated for this, as the needed USB/HID libraries kept changing, which required brew install
-ing a bunch of development headers, kernel libraries, etc. on MacOS targets and apt-get install
-ing packages on Linux. And depending on the environment, changing group ownership of /dev/
items to make it work in user space.
By keeping it a separate package in v6, it will be easier to isolate it and reduce the number of dependencies needed for the core.
Hey @ricmoo, thanks for replying. I am looking forward to the V6 release and greatly appreciate the work you have put into this project. The EthersJS library is such a nicer alternative to the Web3 library. I can understand wanting to minimize changes before a big release, but this is a minimal change and ensures the V5 has uniform EIP-1559 support. This does a version bump on two dependencies and then passes the missing params to the base transaction for Ledger. Without this change, ledger support is effectively neutered as a Signer in Ethersjs. If the concern is integration testing and needing to update tests to give full coverage here, I would be happy to look at helping. Still, I would appreciate it if this change was made available as soon as possible, as those of us who use Ethers with hardhat cannot take advantage of EIP-1559. Regardless if the LedgerSigner is moved into a separate package for V6, this work will still have to be done, right?
it's really a good feature for our hardhat users