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

feat: implement the get solc task in a hardhat-compatible way #1301

Closed
wants to merge 1 commit into from

Conversation

mmv08
Copy link

@mmv08 mmv08 commented Aug 20, 2024

What 💻

This PR adjusts the get solc job to be implemented in a hardhat compatible manner wrt job params:
https://github.com/NomicFoundation/hardhat/blob/93b30d531d1bb1d766bdf2105cd04fa50fef12aa/packages/hardhat-core/src/builtin-tasks/compile.ts#L561-L563
The original job only accepts quiet and solcVersion params, so there's a discrepancy between hardhat and hardhat-zksync

Why ✋

This is to make it easier to use when someone needs to obtain the solc build when compiling in zksync mode. In the Safe project, we do quite a few compilations from strings for tests, e.g:

        const source = `
        contract Test {
            function sendAndReturnBalance(address payable target, uint256 amount) public returns (uint256) {
                (bool success,) = target.call{ value: amount }("");
                require(success, "Transfer failed");
                return target.balance;
            }
        }`;

and the difference between evm/zkvm behaviours makes us write hacky code

Evidence 📷

The tests passed on my computer; however, the runs seem to be non-deterministic, and every third or so run fails. I'm not sure if it's related to my changes

@mmv08 mmv08 requested a review from a team as a code owner August 20, 2024 15:50
@mmv08 mmv08 changed the title Implement the get solc task in a hardhat-compatible way feat: implement the get solc task in a hardhat-compatible way Aug 20, 2024
@kiriyaga-txfusion
Copy link
Contributor

Hello @mmv08, the plugin requires that parameter to work properly with the forked Era Solidity compiler. Starting from version 1.2.0, this forked compiler will be used by default. We need to obtain the correct version information to ensure the appropriate compiler is selected. However, we will implement a workaround so that this task continues to function even if compilation jobs are not provided. We plan to have this ready and released later this week.

@kiriyaga-txfusion
Copy link
Contributor

Closing this PR and opening a new PR with optional compilationJobs argument.

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

Successfully merging this pull request may close these issues.

2 participants