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

Deploy command fails when trying to deploy custom factory contract #85

Closed
2 tasks done
0xDEnYO opened this issue May 22, 2023 · 13 comments
Closed
2 tasks done

Deploy command fails when trying to deploy custom factory contract #85

0xDEnYO opened this issue May 22, 2023 · 13 comments

Comments

@0xDEnYO
Copy link

0xDEnYO commented May 22, 2023

Component

Other (please describe)

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (31fcf5a 2023-05-19T00:10:33.861185000Z)

What command(s) is the bug in?

zkforge zk-create

Operating System

macOS (Apple Silicon)

Describe the bug

I am trying to deploy our custom version of a factory contract.
It imports from solmate which I have added as a submodule. The compiler runs fine, the artifact is produced and looks good.

However, when I try to deploy using this command, I get an error:
Command:
../foundry-zksync/target/debug/zkforge zk-create src/is-system/CREATE3Factory.sol:CREATE3Factory --private-key ...

Output:

Error:
Error getting bytecode from contract: invalid type: null, expected a string

Here the artifact:
image

I believe the error happens in zkCreate.rs in function get_bytecode_from_contract

@sammyshakes
Copy link

i am having trouble reproducing this error. Can you provide a simplified example of your contract so i can try it, or possibly a repo with an example?

@0xDEnYO
Copy link
Author

0xDEnYO commented May 23, 2023

This is the contract I am trying to deploy:

// SPDX-License-Identifier: AGPL-3.0
pragma solidity ^0.8.13;

import { CREATE3 } from "lib/solmate/src/utils/CREATE3.sol";

import { ICREATE3Factory } from "../ICREATE3Factory.sol";

/// @title Factory for deploying contracts to deterministic addresses via CREATE3
/// @author zefram.eth
/// @notice Enables deploying contracts using CREATE3. Each deployer (msg.sender) has
/// its own namespace for deployed addresses.
contract CREATE3Factory is ICREATE3Factory {
    /// @inheritdoc	ICREATE3Factory
    function deploy(
        bytes32 salt,
        bytes memory creationCode
    ) external payable override returns (address deployed) {
        // hash salt with the deployer address to give each deployer its own namespace
        salt = keccak256(abi.encodePacked(msg.sender, salt));
        return CREATE3.deploy(salt, creationCode, msg.value);
    }

    /// @inheritdoc	ICREATE3Factory
    function getDeployed(
        address deployer,
        bytes32 salt
    ) external view override returns (address deployed) {
        // hash salt with the deployer address to give each deployer its own namespace
        salt = keccak256(abi.encodePacked(deployer, salt));
        return CREATE3.getDeployed(salt);
    }
}

and here's the interface it imports:

// SPDX-License-Identifier: AGPL-3.0
pragma solidity >=0.6.0;

/// @title Factory for deploying contracts to deterministic addresses via CREATE3
/// @author zefram.eth
/// @notice Enables deploying contracts using CREATE3. Each deployer (msg.sender) has
/// its own namespace for deployed addresses.
interface ICREATE3Factory {
    /// @notice Deploys a contract using CREATE3
    /// @dev The provided salt is hashed together with msg.sender to generate the final salt
    /// @param salt The deployer-specific salt for determining the deployed contract's address
    /// @param creationCode The creation code of the contract to deploy
    /// @return deployed The address of the deployed contract
    function deploy(
        bytes32 salt,
        bytes memory creationCode
    ) external payable returns (address deployed);

    /// @notice Predicts the address of a deployed contract
    /// @dev The provided salt is hashed together with the deployer address to generate the final salt
    /// @param deployer The deployer account that will call deploy()
    /// @param salt The deployer-specific salt for determining the deployed contract's address
    /// @return deployed The address of the contract that will be deployed
    function getDeployed(
        address deployer,
        bytes32 salt
    ) external view returns (address deployed);
}

@mm-zk
Copy link
Collaborator

mm-zk commented May 23, 2023

Hey @0xDEnYO - I've tried reproducing your issue - but I couldn't:
https://github.com/mm-zk/sample-fzksync-project/tree/0523_bug

I've run:

../../matter/foundry-zksync/target/debug/zkforge zk-build

and then:

../../matter/foundry-zksync/target/debug/zkforge zk-create src/is-system/CREATE3Factory.sol:CREATE3Factory --rpc-url http://localhost:3050 --chain 270 

to deploy on local network - and it worked.
Can you double-check that you've re-compiled your solidity (with zk-build)?

@mm-zk
Copy link
Collaborator

mm-zk commented May 23, 2023

but I did find one scenario, when I get the same error -- when contract name differs.

../../matter/foundry-zksync/target/debug/zkforge zk-create src/is-system/CREATE3Factory.sol:WRONGNAMECREATE3Factory --rpc-url http://localhost:3050 --chain 270 

@0xDEnYO
Copy link
Author

0xDEnYO commented May 23, 2023

ild

Yeah I mean I did compile it, otherwise there wouldnt be an artifact, right?

When I try deploying locally I get the same error as when trying to deploy to testnet

image

Contract name seems correct to me.
Maybe some settings somewhere? Maybe it's looking in the wrong directory for the artifact?

@mm-zk
Copy link
Collaborator

mm-zk commented May 23, 2023

I've just merged: #86 - that should show better error messages.

@mm-zk
Copy link
Collaborator

mm-zk commented May 23, 2023

found the problem:

Your file is called Create3Factory, and you call:

zk-create src/is-system/CREATE3Factory.sol:CREATE3Factory 

instead, you should call:

zk-create src/is-system/Create3Factory.sol:CREATE3Factory 

@0xDEnYO
Copy link
Author

0xDEnYO commented May 23, 2023

Oh my god, that is such a silly error. My apologies for wasting your time with this.... :(
With this correction I am able to deploy to local network. Thank you !!

But deploying to testnet still fails.
My testnet account has funds in both Goerli and your testnet:
image

If you have any advice what could cause this error, I would appreciate it.

image

This is what my .env file looks like (using a GOERLI RPC url for ETH_RPC_URL):
image

@sammyshakes
Copy link

We tried to keep as much Foundry convention as possible to allow for future integrations. in this case ETH_RPC_URL is the variable the replaces --rpc-url. Which would be the direct rpc url you are interacting with (zkSync testnet).

so change from:

ETH_RPC_URL=https://eth-goerli...

to:

ETH_RPC_URL=https://testnet.era.zksync.dev
  • The ZKSYNC_RPC_URL env variable is currently only used for deposits, which uses zk-deposit command, and it replaces --l2-url in command line

@mm-zk
Copy link
Collaborator

mm-zk commented May 23, 2023

@sammyshakes - good point. we should probably change it, so that ETH_RPC_URL is always pointing at 'zksync era' - and we should introduce another variable (L1_URL ?) for the case of deposit/withdrawal.

@0xDEnYO
Copy link
Author

0xDEnYO commented May 23, 2023

We tried to keep as much Foundry convention as possible to allow for future integrations. in this case ETH_RPC_URL is the variable the replaces --rpc-url. Which would be the direct rpc url you are interacting with (zkSync testnet).

so change from:

ETH_RPC_URL=https://eth-goerli...

to:

ETH_RPC_URL=https://testnet.era.zksync.dev
  • The ZKSYNC_RPC_URL env variable is currently only used for deposits, which uses zk-deposit command, and it replaces --l2-url in command line

this works fine now. Thanks a lot for your help.
From my side this issue is closed but I leave it open for your additional point.

@sammyshakes
Copy link

@sammyshakes - good point. we should probably change it, so that ETH_RPC_URL is always pointing at 'zksync era' - and we should introduce another variable (L1_URL ?) for the case of deposit/withdrawal.

yes can definitely do something like this

@mm-zk
Copy link
Collaborator

mm-zk commented May 30, 2023

Done in #94

@mm-zk mm-zk closed this as completed May 30, 2023
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

3 participants