Open smartcontracts opened 3 years ago
Hey @smartcontracts this is definitely a feature I would like to have myself.
I can see 3 options:
One possibility is to add a deployContract method to hardhat-deploy-ethers
that would let you pass an ethers signer (see this PR that aimed to do that in hardhat-deploy itself: https://github.com/wighawag/hardhat-deploy/pull/62). But as a hardhat-deploy users, this is not the most elegant as you need to instantiate the signer, etc...
I was hoping this could be done at the hardhat level and it might well be possible by overriding the hardhat provider. This way not only hardhat-deploy could benefit but every other plugin and use cases.
Otherwise, making it as part of hardhat-deploy could work. I just need to figure out how this would be configured.
Currently hardhat-deploy allow you to name accounts and these account are simply addresses as strings.
you pass that string as is in the from
field.
With that in mind, maybe the simplest would be to change the from
field to be either a string or an object. and the object could specify options whether it is a hardware wallet or not.
But ideally I would like this to be configurable through the named accounts. unfortunately, the current named account structure does not make it easy. One idea was to prepend the address string with some protocol string, like "ledger://0x34fe4ff..."
Very interesting. My initial reaction is that (ideally) plugins should not have to care whether a signer is a plaintext private key or a hardware wallet. Perhaps we could create a separate plugin that allows one to specify a hardware wallet connection in the accounts
field of the hardhat network config. Then we could translate it into a LedgerSigner (seems no ethers Trezor support yet). See example below:
const config = {
networks: {
goerli: {
accounts: [
{
platform: "ledger",
type: "hid",
path: "m/44'/60'/0'/0/0"
}
],
}
}
}
Yes, that would the best way I think. Having it as the lowest level possible
One issue I can see is that hardware wallet do not expose the address without confirmation, so every plugin, tool that fetch the list of account would request user input on the wallet, not ideal.
So maybe the account object should specify the address and if it actually do not match then it errors out, or something
So maybe the account object should specify the address and if it actually do not match then it errors out, or something
I think this is the right way to do it. I'll experiment with this tomorrow and see if I can get it to be relatively reliable.
Are you looking for help on any other issues? would be happy to help out where I can
Not right now, but keen to hear about missing features, and I am available to discuss it like this one for the best way forward :)
I'm experimenting with this now. Unfortunately it's quite a bit of work to do this at a lower level in hardhat. An easy temporary solution would be for hardhat-deploy
to allow passing in a signer to deploy
as opposed to just an address. How do you feel about that? It would allow us to do something along the lines of:
const signer = new LedgerSigner(hre.network.provider, 'default', hre.ethers.utils.defaultPath)
const result = await deploy('MyContract', {
from: signer,
args: [],
log: true,
})
Hey @smartcontracts , check my answer here : https://github.com/wighawag/hardhat-deploy/pull/62 Basically I want to keep hardhat-deploy independent of ethers or any other library on the API side.
But as mentioned in my reply there, this could be added through hardhat-deploy-ethers
: https://github.com/wighawag/hardhat-deploy-ethers
How urgent do you need something like this ?
Hey @smartcontracts I added support for ledger on latest version (0.7.0-beta.51)
Could not test much as ledger support seems to be quite buggy (maybe related to this : https://github.com/ethers-io/ext-signer-ledger/issues/7). but maybe you ll have better chance than me.
I used option 3 which was quite easy to add. you basically add a named account with value like ledger://<address>
see for example here : https://github.com/wighawag/template-ethereum-contracts/blob/d7eee3fc00b7e6a347a5ef2cd7718998254f2401/hardhat.config.ts#L15
hardhat-deploy convert that to the address so you can still refers to this named account as an address, but behind the scene it associate that account to a LedgerSigner. I also added log when an hardware wallet is detected/.
Until a plugin for hardhat can handle it at a low level, this should be good enough for most use case. Let me know what you think
This is fantastic. Testing it out now!
Seemed to work for one transaction but then got some sort of crash. Seems the ethers ledger package is a bit broken.
@wighawag great to see ledger support coming soon. I just want to inform you that for my project I did some fix and which seems to work for my multiple deployments. Here is diff
diff --git a/node_modules/hardhat-deploy/dist/src/helpers.js b/node_modules/hardhat-deploy/dist/src/helpers.js
index 3b588fa..67dce8f 100644
--- a/node_modules/hardhat-deploy/dist/src/helpers.js
+++ b/node_modules/hardhat-deploy/dist/src/helpers.js
@@ -290,6 +290,10 @@ function addHelpers(deploymentManager, partialExtension, network, getArtifact, s
if (options.log || hardwareWallet) {
print(`: deployed at ${deployment.address} with ${receipt === null || receipt === void 0 ? void 0 : receipt.gasUsed} gas\n`);
}
+ if(hardwareWallet){
+ const __eth = await ethersSigner._eth
+ await __eth.transport.device.close()
+ }
return Object.assign(Object.assign({}, deployment), { address, newlyDeployed: true });
}
async function deterministic(name, options) {
@@ -396,7 +400,7 @@ function addHelpers(deploymentManager, partialExtension, network, getArtifact, s
transaction = await provider.getTransaction(deployment.transactionHash);
}
if (transaction) {
- const { ethersSigner } = await getOptionalFrom(options.from);
+ const { ethersSigner, hardwareWallet } = await getOptionalFrom(options.from);
const { artifact } = await getArtifactFromOptions(name, options);
const abi = artifact.abi;
const byteCode = linkLibraries(artifact, options.libraries);
@@ -421,9 +425,17 @@ function addHelpers(deploymentManager, partialExtension, network, getArtifact, s
' not specified in new transaction, cant compare');
}
if (transaction[field] !== newTransaction[field]) {
+ if(hardwareWallet){
+ const __eth = await ethersSigner._eth
+ await __eth.transport.device.close()
+ }
return { differences: true, address: deployment.address };
}
}
+ if(hardwareWallet){
+ const __eth = await ethersSigner._eth
+ await __eth.transport.device.close()
+ }
return { differences: false, address: deployment.address };
}
}
@wighawag I can send PR if you can point me in right direction to do the fix right way, my fix is not something that will go in PR ;P
Thanks @rokso I'll have a look. Note though that latter version of ethers.js hardhware-wallet package might fix the issue anyway.
@wighawag Not sure how ethers.js will fix this, as we will have to call close
explicitly. ethers.js can help by exposing close
method
@rokso looking at the diff, you close after every deploy. ethers.js could do the same at the tx level
@wighawag yeah that works but user will to have open new connection for every transaction. This solution will work for our case as we are opening new connection.
I think the initial solution proposed by @smartcontracts is the most elegant:
const config = {
networks: {
goerli: {
accounts: [
{
platform: "ledger",
type: "hid",
path: "m/44'/60'/0'/0/0"
}
],
}
}
}
Is there an open issue on hardhat? I'd be happy to take a look at implementing this
Hey all, just wanted to check in on the status of this. Has this been solved? I am running into this error locally: /bindings.js:126 err = new Error( ^ Error: Could not locate the bindings file. Tried:
while attempting a setup that uses 'ledger://0x00000000000000addr00000' within namedAccounts
of the config.
I don't think ethers hardware-wallet package has resolved its issues yet : https://github.com/ethers-io/ext-signer-ledger/issues/4
If it's useful I've made a working package for a Ledger Signer for ethers while ricmoo fixes it in EthersJS, the package is @anders-t/ethers-ledger https://www.npmjs.com/package/@anders-t/ethers-ledger
Thanks @anders-torbjornsen I added support for your package in hardhat-deploy@0.9.17
it will first try to load @ethersproject/hardware-wallets but fallback on your if not present
I did not test yet but you should just have "@anders-t/ethers-ledger" as dependency
@anders-torbjornsen @wighawag this is almost working for me but not quite...
I have multiple deploy scripts and I'm able to run the first two scripts, but then I get the following error:
TypeError: cannot open device with path...
I spent some time debugging this and my guess is that its because each new script opens a ledger device but doesn't close it. Once two devices are open, the subsequent open attempt results in the error above.
I haven't dug deep enough to figure out where exactly a fix could reside.
i got error
Error: ERROR processing /aaa/deploy/BUSDToken.js: ProviderError: invalid sender
my BUSDToken.js
module.exports = async ({ getNamedAccounts, deployments, network }) => { const { deployer } = await getNamedAccounts(); const { deploy } = deployments; await deploy("BUSDToken", { from: deployer, log: true, }); }; module.exports.tags = ["BUSDToken", "MockTokens"];
and hardhat.config.js
deployer: { default: 1, 3502: 'ledger://0x52xxxxxxxxxxxxxxxxxxxxxxxx', # Ledger nano wallet },
I didn't have much luck with the ethers LedgerSigner, but figured out a different way to use hardhat with a Ledger (and I believe this will work with Trezor as well)
Frame Wallet supports both Ledger and Trezor, and exposes an http RPC, so the hardhat config can just point to that as a custom network.. e.g.:
// hardhat.config.js
namedAccounts: { deployer: '0x...'},
...
networks: {
goerli: {
url: 'http://127.0.0.1:1248', // this is the RPC endpoint exposed by Frame
chainId: 5,
timeout: 60000, // this is important, because otherwise the request can time out before you've reviewed and confirmed your transaction on the Ledger
}
As hardhat attempts deployments, you'll have to click "Sign" in Frame, and then review the transaction on the Ledger
Any news on this feature ? This should be top priority imho.
@sshmaxime @HartS Hi please check my pull request https://github.com/wighawag/hardhat-deploy/pull/450 There is an issue in the code I have fixed this For now you can use hardhat-deploy-hardware-wallet@0.11.30
Hello! We're hoping to integrate
hardhat-deploy
into https://github.com/ethereum-optimism/contracts. We'd ideally like support for deploying from hardware wallets. It's not a blocker for usinghardhat-deploy
but it would definitely help in the long run. Is this at all possible right now? If not, is this something the Optimism team could help out with?