wighawag / hardhat-deploy

hardhat deployment plugin
MIT License
1.19k stars 292 forks source link

Cannot deploy contracts as facets if artifacts are sitting inside imports #367

Closed prakashsanker closed 2 years ago

prakashsanker commented 2 years ago

Describe the bug I am trying to deploy a diamond with facets. This is the deployment code

await hre.deployments.diamond.deploy(getDeploymentName("SomeName"), {
    from: deployer.address,
    owner: deployer.address,
    log: true,
    facets: [
      { name: getDeploymentName("ContractName"), contract: "ContractName", args: [] },
    ],
    defaultOwnershipFacet: false,
    defaultCutFacet: false,
    execute: isDiamondUpgrade
      ? undefined
      : {
          contract: "DiamondInit",
          methodName: "init",
          args: [
            domainConfig.domain,
            xappConnectionManagerAddress,
            tokenRegistry.address,
            WRAPPED_ETH_MAP.get(+chainId) ?? constants.AddressZero,
            relayerFeeRouter.address,
            promiseRouter.address,
          ],
        },
    // deterministicSalt: keccak256(utils.toUtf8Bytes("connextDiamondProxyV1")),
  });

The artifact is sitting in an imports folder and the contract ContractName is not found.

I dug deep into this and realized it's happening because of this line https://github.com/wighawag/hardhat-deploy/blob/master/src/DeploymentsManager.ts#L215.

If you go deeper, the error is intentional - you hit https://github.com/wighawag/hardhat-deploy/blob/master/src/DeploymentsManager.ts#L215 and then ultimately hit _getArtifactPathFromFiles in hardhat which throws the error. (Function pasted below)

  private _getArtifactPathFromFiles(
    contractName: string,
    files: string[]
  ): string {
    const matchingFiles = files.filter((file) => {
      return path.basename(file) === `${contractName}.json`;
    });

    if (matchingFiles.length === 0) {
      return this._handleWrongArtifactForContractName(contractName, files);
    }

    if (matchingFiles.length > 1) {
      const candidates = matchingFiles.map((file) =>
        this._getFullyQualifiedNameFromPath(file)
      );

      throw new HardhatError(ERRORS.ARTIFACTS.MULTIPLE_FOUND, {
        contractName,
        candidates: candidates.join(os.EOL),
      });
    }

    return matchingFiles[0];
  }

The error happens because files has only the artifacts path and the code path checks to see if the contract is sitting in that folder or not. It's not, so it throws an error.

This however means that the code path will never look at the imports folder (which is what is communicated in the documentation). The code works if I dive in and hardcode the imports folder as the artifacts folder.

To Reproduce Steps to reproduce the behavior:

  1. Have a diamond contract and precompile the contract
  2. Put the contract artifact in an imports folder
  3. Have a deployment script in /deploy and run npx hardhat deploy
  4. See error

Expected behavior Everything should work as with the non extended artifacts case. The imports code path should be hit.

versions

snake-poison commented 2 years ago

I've also ran into this bug but for the external.artifacts use case. I similarly went down the rabbit hole and noticed that the issue is with getExtendedArtifactFromFolders , specifically this line where an uncaught exception bubbles up before getting to the logic where the additional import paths are loaded https://github.com/wighawag/hardhat-deploy/blob/32a80276a9d473e43e38b3b07c300e35967b07e8/src/utils.ts#L71

this bug is not limited to the diamond functions and surfaces whenever getExtendedArtifacts is invoked (as is the case with deploy)

my workaround is :

const deployAlt: typeof deploy = async (name, options) =>{
        const artifact = await deployments.getArtifact(name)
        const deployOptions = Object.assign({}, options, {contract: {abi: artifact.abi, bytecode:artifact.bytecode}})
        return await deploy(name, deployOptions)
    }