Feature: add zksync support to v1.5.0 #742

mmv08 · 2024-03-01T15:28:06Z

This PR:

Implements Add zkSync Support To Repository #766 by adding the zksync support for deployment and testing contracts
It is based on Adjust 1.5.0 for zkSync #651. However, Matter Labs has published many updates in the meantime, so there are many changes
Add zksync tests to the CI

Importants changes

The zksync team did a great job of adding a lot of interoperability features with hardhat's environment ethers object, however, there are certain features we rely on that require zksync-specific adjustments, such as compiling code from string, contract deployment, create2 address calculation. Functions specific to that had to be adjusted
we had incorrect test assumptions: for example, we assumed that hre.ethers.getContractAt would always return a contract instance connected to the first account returned from hre.ethers.getSigners. This didn't hold in zksync environment as they use the 9th account

Notes

This PR doesn't include changes to Safe Migration contract tests for zksync, as we decided to merge this first and add the support to 1.4.1 and fix the migration contracts there, then cherry-pick it back to 1.5.0

Proof of deployment:

> hardhat deploy-contracts --network zkSyncSepolia

Yul codegen is only supported for solc >= 0.8. Flag forceEVMLA will automatically be set to true by default.
Yul codegen is only supported for solc >= 0.8. Flag forceEVMLA will automatically be set to true by default.
The zksolc compiler version in your Hardhat config file (1.5.2) is not the latest. We recommend using the latest version 1.5.3.
Nothing to compile
No need to generate any newer typings.
reusing "SimulateTxAccessor" at 0x1aaa95173dAA2F9B18df1277485CAd842Bb5EeFe
reusing "SafeProxyFactory" at 0x9c8588F1Cfbf54845c06C6924E90b20AB0912B6F
reusing "TokenCallbackHandler" at 0x94AA81a0E2e857A0e5978b7E4218DdAE2d611bCf
reusing "CompatibilityFallbackHandler" at 0x7e940bC046e2a069F67e40f00A9d5749Abf59f70
reusing "CreateCall" at 0xded498F0eEEAe67E2E339a42e5c926335fDDCcf7
reusing "MultiSend" at 0xE25a6DFf1cbD91d195D87ECb90C96517fc98F581
reusing "MultiSendCallOnly" at 0x0E5006a4179dF454031800eC575F413c83d7B025
reusing "SignMessageLib" at 0x97273487793848a571054bf3a5FDc9c0982648B9
reusing "Safe" at 0xB23798CfB614f3A1a6d0820c08e004922475CD76
reusing "SafeL2" at 0x0ED239feb23632cE272A0AE5f8bfd843377897DF
reusing "SafeToL2Migration" at 0x4c81Ce72dD304E27739A5607f0Ba7207eD415360
reusing "SafeMigration" at 0x80dd94053ED6075992771A2Df501eFF101fBc281
Verification status for CompatibilityFallbackHandler: SUCCESS
Verification status for CreateCall: SUCCESS
Verification status for MultiSend: SUCCESS
Verification status for MultiSendCallOnly: SUCCESS
Verification status for Safe: SUCCESS
Verification status for SafeL2: SUCCESS
Verification status for SafeMigration: SUCCESS
Verification status for SafeProxyFactory: SUCCESS
Verification status for SafeToL2Migration: SUCCESS
Verification status for SignMessageLib: SUCCESS
Verification status for SimulateTxAccessor: SUCCESS
Verification status for TokenCallbackHandler: SUCCESS

Running zk verification on block explorer

Verifying CompatibilityFallbackHandler at 0x7e940bC046e2a069F67e40f00A9d5749Abf59f70...
Your verification ID is: 23855
Contract successfully verified on ZKsync block explorer!

Verifying CreateCall at 0xded498F0eEEAe67E2E339a42e5c926335fDDCcf7...
Your verification ID is: 23856
Contract successfully verified on ZKsync block explorer!

Verifying MultiSend at 0xE25a6DFf1cbD91d195D87ECb90C96517fc98F581...
Your verification ID is: 23857
Contract successfully verified on ZKsync block explorer!

Verifying MultiSendCallOnly at 0x0E5006a4179dF454031800eC575F413c83d7B025...
Your verification ID is: 23858
Contract successfully verified on ZKsync block explorer!

Verifying Safe at 0xB23798CfB614f3A1a6d0820c08e004922475CD76...
Your verification ID is: 23859
Contract successfully verified on ZKsync block explorer!

Verifying SafeL2 at 0x0ED239feb23632cE272A0AE5f8bfd843377897DF...
Your verification ID is: 23860
Contract successfully verified on ZKsync block explorer!

Verifying SafeMigration at 0x80dd94053ED6075992771A2Df501eFF101fBc281...
Your verification ID is: 23861
Contract successfully verified on ZKsync block explorer!

Verifying SafeProxyFactory at 0x9c8588F1Cfbf54845c06C6924E90b20AB0912B6F...
Your verification ID is: 23862
Contract successfully verified on ZKsync block explorer!

Verifying SafeToL2Migration at 0x4c81Ce72dD304E27739A5607f0Ba7207eD415360...
Your verification ID is: 23863
Contract successfully verified on ZKsync block explorer!

Verifying SignMessageLib at 0x97273487793848a571054bf3a5FDc9c0982648B9...
Your verification ID is: 23864
Contract successfully verified on ZKsync block explorer!

Verifying SimulateTxAccessor at 0x1aaa95173dAA2F9B18df1277485CAd842Bb5EeFe...
Your verification ID is: 23865
Contract successfully verified on ZKsync block explorer!

Verifying TokenCallbackHandler at 0x94AA81a0E2e857A0e5978b7E4218DdAE2d611bCf...
Your verification ID is: 23866
Contract successfully verified on ZKsync block explorer!
Execution time: 51.41s

ElvisKrop · 2024-03-01T15:41:17Z

@mmv08 sorry, for delay 😞
It seems "edits from maintainers" feature is disabled on protofire organization level, therefore I was not able to give you access quickly.

coveralls · 2024-04-23T16:32:18Z

Pull Request Test Coverage Report for Build 10726230198

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 93.478%

Totals
Change from base Build 10451906756:	0.0%
Covered Lines:	386
Relevant Lines:	398

💛 - Coveralls

mmv08 · 2024-04-25T16:10:02Z

This is put on pause again because of a bug with zksync-hardhat-node, it was reported to the zksync team: zkSync-Community-Hub/zksync-developers#482

@nlordell and I are also tried a few workarounds:

Running the in-memory node locally, it worked, but it failed for all the tests with the mock contract because, for some reason, the invocation count didn't update 🤷 perhaps this should be reported to zksync team too
Running the "full" dockerized node -> connection failed with ECONNRESET, so the testing suite couldn't be run.

nlordell · 2024-04-26T07:13:35Z

I feel like the invocation count not updating is something we are doing wrong (as other storage updates were working as expected), and I am super curious and will likely try to debug at some point :P

mmv08 · 2024-04-26T15:04:42Z

testing with the "full" local node was also not possible matter-labs/local-setup#19

mmv08 · 2024-08-26T16:00:41Z

package.json

        "build:ts": "rimraf dist && tsc -p tsconfig.prod.json",
        "build:ts:dev": "rimraf dist && tsc -p tsconfig.json",
-        "test": "hardhat test && npm run test:L1",
+        "test:zk": "HARDHAT_ENABLE_ZKSYNC=1 hardhat test",
+        "test": "hardhat test --network hardhat && npm run test:L1 && npm run test:L2 && npm run test:zk",


We previously didn't have a command for testing the SafeL2 contract

mmv08 · 2024-08-26T16:03:39Z

test/accessors/SimulateTxAccessor.spec.ts

@@ -6,7 +6,7 @@ import { buildContractCall } from "../../src/utils/execution";
 describe("SimulateTxAccessor", () => {
    const setupTests = deployments.createFixture(async ({ deployments }) => {
        await deployments.fixture();
-        const signers = await ethers.getSigners();
+        const signers = await hre.ethers.getSigners();


the change here (and a similar change in other files) is unrelated to zksync: it was in the original PR and I decided to keep it because this way the invocation is more specific and less likely to cause a conflict with a global import like this import { ethers } from 'ethers'

…cy."

.env.sample

.gitignore

.husky/pre-commit

hardhat.config.ts

src/tasks/deploy_contracts.ts

nlordell · 2024-08-27T12:34:09Z

src/utils/deploy.ts

+ * @returns The deployer account address or private key.
+ */
+export const getDeployerAccount = async (hre: HardhatRuntimeEnvironment) => {
+    const { deployer } = await hre.getNamedAccounts();


ubernit: it feels weird to get the deployer named account if we discard it right after. Why not if (!hre.network.zksync) { ... } else { ... } pattern here?

I pushed a small change to the code. I moved the happy path outside of the branching code because the branching will only execute rarely (there are only few networks with the zksync environment)

nlordell · 2024-08-29T12:13:06Z

src/utils/proxies.ts

+        const bytecodeHash = getZkSyncBytecodeHashFromDeployerCallHeader(proxyCreationCode);
+        const input = new ethers.AbiCoder().encode(["address"], [singleton]);
+        return zk.utils.create2Address(factoryAddress, bytecodeHash, salt, input);
+    }


nlordell · 2024-08-29T12:15:02Z

test/core/Safe.Execution.spec.ts

+            // await expect(await executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)], { gasLimit: 5500000 })).to.emit(
+            //     nativeTokenReceiver,
+            //     "BreadReceived",
+            // );


nit: left-in commented out code.

I left it on purpose because I was looking for a way to make it work in ZkSync, this test should be considered unfinished

nlordell · 2024-08-29T12:17:03Z

test/core/Safe.Execution.spec.ts

+            );
+
+            // Reverted reason seems not properly returned by zkSync local node, though it is in fact GS010 when using debug_traceTransaction
+            if (hre.network.zksync) {


Weird that only this case doens't properly find the revert reason, but in other cases it does. Do we know why?

I updated the comment

nlordell · 2024-08-29T12:22:41Z

test/libraries/SafeMigration.spec.ts

+    before(function () {
+        /**
+         * ## Migration tests are not working yet for zkSync: this test depends on the EVM bytecode
+         * which is not supported on zkSync. Tests will be adjusted.


Should we create a GH issue to track this?

I think it's covered by #767

nlordell

LGTM - just some small comments, but approving to reduce back-and-forth.

nlordell · 2024-08-29T12:27:07Z

test/core/Safe.Execution.spec.ts

@@ -1,5 +1,5 @@
 import { expect } from "chai";
-import hre, { deployments, ethers } from "hardhat";
+import hre, { ethers } from "hardhat";


Should we also get rid if the { ethers } here?

…pecs.

…solc

mmv08 mentioned this pull request Mar 1, 2024

Adjust 1.5.0 for zkSync #651

Closed

nlordell assigned mmv08 Apr 12, 2024

mmv08 force-pushed the v1.5.0-zksync branch 2 times, most recently from e9e3213 to cca1b20 Compare April 23, 2024 12:18

mmv08 added the onhold Blocked by external dependency label Apr 25, 2024

nlordell mentioned this pull request Jun 21, 2024

Add zkSync Support To Repository #766

Closed

3 tasks

Sulthanmh approved these changes Jul 15, 2024

View reviewed changes

mmv08 force-pushed the v1.5.0-zksync branch 2 times, most recently from 83873da to 823d0c6 Compare July 15, 2024 09:35

mmv08 force-pushed the v1.5.0-zksync branch 6 times, most recently from 0107628 to 84b9af3 Compare August 23, 2024 13:26

feat: add zksync support to the project

2891ee8

mmv08 force-pushed the v1.5.0-zksync branch from ef9ff42 to 2891ee8 Compare August 26, 2024 15:22

mmv08 changed the title ~~V1.5.0 zksync~~ Feature: add zksync support to v1.5.0 Aug 26, 2024

mmv08 added 3 commits August 26, 2024 17:51

Add zksync support to proxy address calculation functions

1d1cc31

fix: remove unnecessary await

fa637f6

Update tsconfig.json to include src/utils directory

633c130

mmv08 commented Aug 26, 2024

View reviewed changes

mmv08 added 3 commits August 26, 2024 18:07

Remove unused types and simplify contract instantiation.

f287fbe

Remove unnecessary await parentheses in Safe.setup call.

79f02c8

"Refactor imports to use ethers directly instead of hre for consisten…

bfa8fe5

…cy."

mmv08 requested review from akshay-ap and remedcu August 27, 2024 11:37