NomicFoundation / hardhat-ignition

Hardhat Ignition is a declarative deployment system that enables you to deploy your smart contracts without navigating the mechanics of the deployment process.
https://hardhat.org/ignition
MIT License
108 stars 26 forks source link

Fully-qualified artifact name resolution seems to not actually work #778

Closed luismasuelli closed 1 month ago

luismasuelli commented 5 months ago

What happened?

When I have two contracts with the same name (let's say "MyContract", in different .sol files in different paths) the resolution in the m.contract("MyContract", [], ...) future prompts to use fully-qualified artifact names, but then when I try that it is not accepted/found by ignition. So there's no way to discriminate between artifacts with the same name (I agree that, however, having artifacts with the same name is not necessarily a good practice, but they might come somehow from different libraries).

Minimal reproduction steps

Hardhat version: 2.22.5. Hardhat Ignition version: 0.15.4 (at least by checking the package-lock.json). Also using: @openzeppelin/contracts.

  1. I created a contract named MyOwnedERC20, located at contracts/MyOwnedERC20.sol. I don't believe the contents matter but for the purpose of this discovery the contents are these:
// SPDX-License-Identifier: MIT
pragma solidity 0.8.24;
import "@openzeppelin/contracts/access/Ownable.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";

contract MyOwnedERC20 is ERC20, Ownable {
    constructor() ERC20("My Token", "MYTK") Ownable(msg.sender) {}
    function mint(address account, uint256 amount) onlyOwner public {
        _mint(account, amount);
    }
}
  1. Out of curiosity, I copied the contract exactly into a new directory (yes, two copies of the same file): contracts/subdirectory/MyOwnedERC20.sol.

  2. Then I built: npx hardhat compile.

  3. Then, create a hardhat ignition module. In this example, ignition/modules/MyOwnedERC20.js. The contents are simple:

const { buildModule } = require("@nomicfoundation/hardhat-ignition/modules");

module.exports = buildModule("MyOwnedERC20", (m) => {
  // const contract = m.contract(
  //     "contracts/subdirectory/MyOwnedERC20.sol:MyOwnedERC20", [], {id: "MyOwnedERC20_1"}
  // );
  const contract = m.contract(
      "MyOwnedERC20", [], {id: "MyOwnedERC20_1"}
  );

  return { contract };
});

Notice how there are 3 lines intentionally commented. We'll make use of it in the next steps.

  1. Try it: npx hardhat ignition deploy ignition/modules/MyOwnedERC20.js. You'll see an error:
Error HH701: There are multiple artifacts for contract "MyOwnedERC20", please use a fully qualified name.

Please replace MyOwnedERC20 for one of these options wherever you are trying to read its artifact:

contracts/MyOwnedERC20.sol:MyOwnedERC20
contracts/subdirectory/MyOwnedERC20.sol:MyOwnedERC20

For more info go to https://hardhat.org/HH701 or run Hardhat with --show-stack-traces

(this is a standard hardhat error and it is understandable, so no problem here)

  1. Edit the MyOwnedERC20.js deployment file. Comment the m.contract invocation and uncomment the other m.contract invocation:
const { buildModule } = require("@nomicfoundation/hardhat-ignition/modules");

module.exports = buildModule("MyOwnedERC20", (m) => {
  const contract = m.contract(
      "contracts/subdirectory/MyOwnedERC20.sol:MyOwnedERC20", [], {id: "MyOwnedERC20_1"}
  );
  // const contract = m.contract(
  //     "MyOwnedERC20", [], {id: "MyOwnedERC20_1"}
  // );

  return { contract };
});
  1. Try it: npx hardhat ignition deploy ignition/modules/MyOwnedERC20.js. You'll see another error now:

Hardhat Ignition 🚀

Resuming existing deployment from ./ignition/deployments/chain-31337

Deploying [ MyOwnedERC20 ]

Batch #1
  Executing MyOwnedERC20#MyOwnedERC20_1...

Error in plugin hardhat-ignition: Artifact path not found for contracts/subdirectory/MyOwnedERC20.sol:MyOwnedERC20

For more info run Hardhat with --show-stack-traces

I expect: the fully-qualified artifact path to work.

Search terms

duplicate artifact name

luismasuelli commented 5 months ago

Just a suggestion for this to work. I just found and made this change and it worked:

  1. I went to this file: @nomicfoundation/hardhat-ignition/dist/src/hardhat-artifact-resolver.js and found the _resolvePath function, which its TRANSPILED code looks like this:
async _resolvePath(contractName) {  
    const artifactPaths = await this._hre.artifacts.getArtifactPaths();  
    const artifactPath = artifactPaths.find((p) => path_1.default.parse(p).name === contractName);  
    return artifactPath;  
}
  1. Then I replaced it by adding some lines (these 3 lines are retained as default logic, however):
    async _resolvePath(contractName) {
        // My addition starts here:
        const parts = contractName.split(":");
        if (parts.length != 1) {
            contractName = parts.pop();
            const filename = parts.join(":");
            const projectArtifactsPath = this._hre.config.paths.artifacts;
            const fullArtifactPath = path_1.default.resolve(projectArtifactsPath, filename, contractName + ".json");
            const artifactPaths = await this._hre.artifacts.getArtifactPaths();
            if (artifactPaths.indexOf(fullArtifactPath) >= 0) {
                return fullArtifactPath;
            } else {
                return undefined;
            }
        }
        // My addition ends here.
        const artifactPaths = await this._hre.artifacts.getArtifactPaths();
        const artifactPath = artifactPaths.find((p) => path_1.default.parse(p).name === contractName);
        return artifactPath;
    }

Then this made my attempt on using FQ contract names... work.

Perhaps you could try this (I'm sorry. I did not do a full test or properly add test cases for this, so this is why I made no PR and just suggesting this to test: perhaps there are things that I'm not covering / considering by doing this).

kanej commented 5 months ago

Thanks @luismasuelli, this is a bug. I have been able to reproduce it locally. We will take a look at your resolve path suggestion.

blockedby commented 2 months ago

please, up