ethers-io / ethers.js

Complete Ethereum library and wallet implementation in JavaScript.
https://ethers.org/
MIT License
7.94k stars 1.85k forks source link

add convenience method for checking if a contract is deployed #1042

Closed thegostep closed 1 month ago

thegostep commented 4 years ago
const someContract = new ethers.Contract( ... )
const isDeployed = await someContract.isDeployed()

if (!isDeployed) {
   // deploy it
}

// continue
garyng2000 commented 4 years ago

this is technically impossible. you can use the same bytecode to deploy it multiple times(we do it all the time for testing or other reason). it would require some 'registry' on the blockchain if you want do something like this, I think shuffle(or is it openzeppelin) use this approach in their 'upgradable' contract.

zemse commented 4 years ago

you can use the same bytecode to deploy it multiple times(

@garyng2000 Did you interpret this as ContractFactory? A Contract has address associated, so we can check if some bytecode exists there signaling it being a contract.

But I fear people might misunderstandingly use this handy method as a safety check to ensure that Contract is connected to the right contract (that supports the abi it has), which can also not be true! To ensure that it supports interface could need it to verify bytecode or EIP-165.

Instead, does having the following make more sense?

provider.isContract(contractAddress: string): boolean

Edit: I just noticed the if else logic in the description. @garyng2000 is answer correct

Edit2: I'm aware that getCode exists. I don't think isContract is required, since ethers.js already throws errors as contract not deployed.

garyng2000 commented 4 years ago

@zemse, if it is about a given contract address, just genCode is generally good enough to know if it has been deployed. Though my experience is that sometimes, we can get the contract address mixed up so would use some other way(in the contract) to test if it is the contract we want. IOW .isContract is a contract function like isContract(someSig).

thegostep commented 4 years ago

I care less about if the deployed bytecode matches the abi I provide than if there is a contract in place.

getCode() !== '0x' does what I need but it seems like it should be wrapped up in a helper that returns a bool.

Maybe provider.isContract()?

garyng2000 commented 4 years ago

a wrapper is fine(though I personally see little value over getCode()) but I am skeptical about the logic. AFAIK, below is typical what happens(pseudo) :

a = deployContract(bytecode) // a => contract address a.getCode() // this would return 0x until it is mined

So straight testing isContract won't tell the state of it(it can be WIP). I think self-destruct also will wipe the code.

IOW, there is more to think about if a contract address is invalid(getCode() return 0x)

ricmoo commented 4 years ago

I don't really see much value in an .isContract method, since it is literally just doing ((await provider.getCode(address)) === "0x"). It seems simple enough and I could imagine someone expecting the call to verify the bytecode matches the contract bytecode, which a contract doesn't have. I have considered a similar thing for the contract factory, but it is not reliable, since the initcode is what the Factory has, not the runtime, and since the initcode is executed there is no way to verify that it is correct (see halting problem ;)).

So, I would think in many cases people would care about what the bytecode is, not that it simple exists and otherwise it is a very simple call to do yourself. The vast majority of use-cases does not need to check either, since the a contract is generally long-lived and deployed long before interacting with it, especially customers.

If this is for testing, the ContractFactory adds the .deployTrasaction on the Contract, so it is easy to await contract.deployTransaction.wait(), which also throws if the contract failed to deploy. So, it should almost never be the case you have a contract connected to an unknown-to-be-deployed contract, unless you have tests running across multiple processes, which sounds painful... :s

If you are looking for deterministic deployments though, you might be interested in rooted or wisps.

Does that all make sense?

garyng2000 commented 4 years ago

@ricmoo, I do have usage scenario where the deployment of the contract are completely separated from where it is used(say multi-node scale out situation), thus the .wait().then() style is rarely used. So logical .isContract(someAddress) of some form is used a lot. Though I am with you that a thin wrapper of .getCode() is probably not worth to be added as a generic library feature. It can be easily implemented via HOF or mixin for those who prefers it.

ricmoo commented 1 month ago

This was included in v6 via contract.waitForDeployment() which is backed by contract.getDeployedCode().

Thanks! :)