search

wighawag / hardhat-deploy

hardhat deployment plugin
MIT License
1.17k stars 283 forks source link

Ledger: add path support #497

Closed ifelsedeveloper closed 7 months ago

ifelsedeveloper commented 7 months ago

Add path support for the ledger for andre-t/ledger and etherporject/hardware-wallet. Please check

wighawag commented 7 months ago

Your comment says Add path support for the ledger for andre-t/ledger and etherporject/hardware-wallet.

But the code will only call derivationPath = getDerivationPath(network.config.chainId); for ethersprojectHardwareWalletsModule

ifelsedeveloper commented 7 months ago

I have implemented logic to handle different scenarios when loading the EtherProject. When this module is loaded, I add the specified path, which I have verified on Ubuntu. Additionally, if the 'andre' module is detected, I initialize the LedgerSigner with an alternate path. This setup has been tested and confirmed working with a Ledger device on macOS.

The code functions as follows: If the ethersprojectHardwareWalletsModule is present, it determines the derivationPath based on the network's chain ID. Subsequently, ethersSigner is instantiated as a new LedgerSigner, using the provider and the derived path. In the absence of this module, ethersSigner is simply initialized with the provider and a predefined registeredProtocol."

" if (ethersprojectHardwareWalletsModule) { derivationPath = getDerivationPath(network.config.chainId); ethersSigner = new LedgerSigner( provider, 'default', derivationPath ); } else { ethersSigner = new LedgerSigner(provider, registeredProtocol); } "

wighawag commented 7 months ago

Ok, so it is intended that derivationPath is not used if @anders-t/ethers-ledger is installed (but not @ethersproject/hardware-wallets )

ifelsedeveloper commented 7 months ago

Yes, these two libraries have different constructors. I assume that no one have tested this project with ledger device.

ifelsedeveloper commented 7 months ago

I can make tests on ubuntu, mac os and windows with my ledger device one more time and post results here that it actually works. Please fill free to share any ideas how to improve the code

wighawag commented 7 months ago

Thanks, just wanted to clarify, I ll merge this and publish. Thanks for your PR!

wighawag commented 7 months ago

available in v0.11.44