Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

factoryDepths for zksync is incorrectly built #537

Open
novaknole opened this issue Apr 17, 2024 · 2 comments
Open

factoryDepths for zksync is incorrectly built #537

novaknole opened this issue Apr 17, 2024 · 2 comments

Comments

@novaknole
Copy link
Contributor

novaknole commented Apr 17, 2024

Describe the bug
As we know, zksync introduces factoryDepths field in the transaction to make sure bytecodes are known.

Assume the following contracts:

contract Test2 { uint public x = 35; }

contract Test1 {
   address public addr;

   function start() public {
       addr = address(new Test2());
   }
}

contract Main {
    address public addr;

    function start() {
          addr = address(new Test1());
    }
}

Inside hardhat-deploy script, let's assume we deploy the Main contract.

await deploy("Main", {
    contract: MainArtifact,
    from: wallet.address,
    args: [],
    log: true,
 });

What actually happens is before the actual deploy transaction is passed to the zksync operator, hardhat-deploy builds the factoryDepths. see. The problem is in this function, because for our contracts, factoryDepths array would only contain a single bytecode(Test1Bytecode), because it doesn't recursively goes through with artifacts. The deployment of Main works fine, but once you do the following steps after deployment:

  1. get Main's address and call start on it. It will deploy Test1 and store it on addr. Get this addr value.
  2. Then call start on that addr which would deploy Test2.
  3. Call x on the Test2.

This will fail with the code hash is not known. This is actually expected, because deployment transaction's factoryDepths didn't contain the bytecode of Test2.

If you look into the original package from zksync, you will realize that factoryDepths is correctly constructed. See. You can see that it recursively goes through artifacts to generate the correct factoryDepths. In this case, factoryDepths would contain bytecodes of Test1 and Test2 and we wouldn't get the same problems anymore.

versions

  • hardhat-deploy 0.11.26 (but it also fails with latest version)
  • hardhat 2.12.4
  • nodejs 18.19.1

I wonder if this problem has been missed and why factoryDepths construction is not recursive in hardhat-deploy package.

@mikemcdonald made the PR with the extractFactoryDepths written in a wrong way in my opinion. What's your thoughts ? @wighawag

@wighawag
Copy link
Owner

@novaknole thanks for pointing this out,

I unfortunately do not know the intricacies as you seems to understand and I don't have the bandwidth to look at it. Do you think you could make a PR to fix it ?

@novaknole
Copy link
Contributor Author

@wighawag hey Ronan, I just made a PR. Please take a look #541

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants