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

Script: support matching --sig argument with function hex selector #4325

Closed
2 tasks done
timurguvenkaya opened this issue Feb 10, 2023 · 6 comments
Closed
2 tasks done
Labels
C-forge Command: forge Cmd-forge-script Command: forge script T-bug Type: bug

Comments

@timurguvenkaya
Copy link

timurguvenkaya commented Feb 10, 2023

Component

Forge

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

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (249538f 2023-02-10T00:23:08.24269Z)

What command(s) is the bug in?

forge script DeployTest -s d61d5b11 --broadcast -vvv --force

Operating System

macOS (Intel)

Describe the bug

I'm trying to do multichain deployment using CREATE2 with forge script

I use helper which takes fork id and deploys everything.

When I run everything I get Script ran successfully with Sig is invalid during the broadcast.

I'm using anvil on different ports & different forks to test deployment locally.

Example code

// SPDX-License-Identifier: MIT
pragma solidity 0.8.18;

import {Script, StdUtils} from "forge-std/Script.sol";

import {console2} from "forge-std/console2.sol";

contract TestContract {
    function test() public view returns (uint256) {
        return 1;
    }
}

contract DeployTest is Script {
    bytes32 internal constant TEST_SALT = bytes32(uint256(3));

  uint256 public local = vm.createFork("http://127.0.0.1:8545");
  uint256 public localBSCTest = vm.createFork("http://127.0.0.1:8546");
        
    address create2addrSBT =
        computeCreate2Address(
            TEST_SALT,
            hashInitCode(type(TestContract).creationCode)
        );

    function deployTest() public {
        forkDeployTest(local);
        forkDeployTest(localBSCTest); 
    }

    function forkDeployTest(uint256 fork) public {
        uint256 deployerPrivateKey;
        address owner;

        deployerPrivateKey = vm.envUint("LOCAL_DEPLOYER_KEY");
        owner = vm.envAddress("LOCAL_SAFE_OWNER");

        console2.log("Deploying TestContract on fork", fork);

        vm.selectFork(fork);
        vm.startBroadcast(deployerPrivateKey);

        TestContract testContract = new TestContract{salt: TEST_SALT}();

        console2.log("Test Contract address", address(testContract));

        require(
            create2addrSBT == address(testContract),
            "Address mismatch Test Contract"
        );

        vm.stopBroadcast();
    }
}

Anvil:

anvil --fork-url https://rpc.ankr.com/eth_goerli
anvil --fork-url https://rpc.ankr.com/bsc_testnet_chapel -p 8546

Output:

Script ran successfully.
Gas used: 151175

== Logs ==
  Deploying TestContract on fork 0
  Test Contract address 0x4C04E31cd475dF0135ceaE24D20Fb3F25b9A472f
  Deploying TestContract on fork 1
  Test Contract address 0x4C04E31cd475dF0135ceaE24D20Fb3F25b9A472f
Multi chain deployment is still under development. Use with caution.

## Setting up (2) EVMs.

==========================

Chain 5

Estimated gas price: 3.049130518 gwei

Estimated total gas used for script: 109692

Estimated amount required: 0.000334465224780456 ETH

==========================

==========================

Chain 97

Estimated gas price: 2 gwei

Estimated total gas used for script: 109692

Estimated amount required: 0.000219384 ETH

==========================
Error:
Sig is invalid.

It works when I only leave one fork in deployTest() or run manually forkDeployTest() twice for different forks

Not sure whether the issue is on my end

@timurguvenkaya timurguvenkaya added the T-bug Type: bug label Feb 10, 2023
@mattsse
Copy link
Member

mattsse commented Feb 10, 2023

@joshieDo this happens when the signature does not contain a (, unclear what's going on here or what the assumptions wrt sig are here.

ptal

@timurguvenkaya
Copy link
Author

timurguvenkaya commented Feb 11, 2023

Rerun with #4326

It returns

Failed to compute file name: Signature d61d5b11 is invalid.

It works if I change deployTest to run

@mattsse
Copy link
Member

mattsse commented Feb 11, 2023

ah that's the raw hex signature of the function.
I see, we expect a name like run() because this is generally easier to use, but we def need to support the raw hex sig as well.

can you please try using deployTest()

@mattsse
Copy link
Member

mattsse commented Feb 11, 2023

It works if I change deployTest to run

great so this is indeed a bug with how we match signatures.

@mattsse mattsse changed the title Sig is invalid error when deploying to multiple chains Script: support matching --sig argument with function hex selector Feb 11, 2023
@timurguvenkaya
Copy link
Author

Yep
It also works if I use deployTest() directly

forge script DeployTest -s "deployTest()" --broadcast -vvv --force

@onbjerg onbjerg added C-anvil Command: anvil C-forge Command: forge Cmd-forge-script Command: forge script and removed C-anvil Command: anvil labels Feb 27, 2023
@klkvr
Copy link
Member

klkvr commented Jul 1, 2024

This was likely fixed with script refactoring. Can't reproduce anymore

@klkvr klkvr closed this as completed Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-forge Command: forge Cmd-forge-script Command: forge script T-bug Type: bug
Projects
Status: Completed
Development

No branches or pull requests

4 participants