Zksync testing

.gitignore

@@ -32,6 +32,7 @@ yarn-error.log*
 
 # deployments
 deployments
+deployments-zk
 deployed_contracts.json
 managingDAOTX.json
 

DEPLOYMENT_CHECKLIST.md

@@ -1,108 +1,96 @@
 # Deployment Checklist
 
-This checklist is seen as a guide to deploy the stack to a new chain.
-
-## Pre-Deployment
-
-- [ ] Verify that the deployers wallet has enough funds.
-- [ ] Check that the subgraph hoster supports the network OSx is deployed to.
-- [ ] Make sure you are using Node v16
-- [ ] Bump the OSx protocol version in the `ProtocolVersion.sol` file.
-- [ ] Check that version tags are set correctly in the plugin repo deploy scripts `packages/contracts/deploy/new/30_plugins/10_plugin-repos` to ensure synchronized version numbers across all supported networks.
-- [ ] Choose an ENS domain for DAOs
-- [ ] Choose an ENS domain for plugins
-- [ ] Check if there is an official ENS deployment for the chosen chain and if yes:
-  - [ ] Check if there is already an entry for it in `packages/contracts/deploy/helpers.ts`
-  - [ ] Check that the owner of the DAO domain is the deployer
-  - [ ] Check that the owner of the plugin domain is the deployer
-- [ ] Run `yarn` in the repository root to install the dependencies
-- [ ] Run `yarn build` in `packages/contracts` to make sure the contracts compile
-  - [ ] Check that the compiler version in `hardhat.config.ts` is set to at least `0.8.17` and on the [known solidity bugs page](https://docs.soliditylang.org/en/latest/bugs.html) that no relevant vulnerabilities exist that are fixed in later versions. If the latter is not the case, consider updating the compiler pragmas to a safe version and rolling out fixes for affected contracts.
-- [ ] Run `yarn test` in `packages/contracts` to make sure the contract tests succeed
-- [ ] Run `yarn deploy --deploy-scripts deploy/new --network hardhat --reset` to make sure the deploy scripts work
-- [ ] Set `ETH_KEY` in `.env` to the deployers private key
-- [ ] Set the right API key for the chains blockchain explorer in `.env` (e.g. for mainnet it is `ETHERSCAN_KEY`)
-- [ ] Set the chosen DAO ENS domain (in step 1) to `NETWORK_DAO_ENS_DOMAIN` in `.env` and replace `NETWORK` with the correct network name (e.g. for mainnet it is `MAINNET_DAO_ENS_DOMAIN`)
-- [ ] Set the chosen Plugin ENS domain (in step 2) to `NETWORK_PLUGIN_ENS_DOMAIN` in `.env` and replace `NETWORK` with the correct network name (e.g. for mainnet it is `MAINNET_PLUGIN_ENS_DOMAIN`)
-- [ ] Set the subdomain to be used of the managing DAO to `MANAGEMENT_DAO_SUBDOMAIN` in `.env`. If you want to use `management.dao.eth` put only `management`
-- [ ] Set the multisig members of the managing DAO as a comma (`,`) separated list to `MANAGINGDAO_MULTISIG_APPROVERS` in `.env`
-- [ ] Set the amount of minimum approvals the managing DAO needs to `MANAGINGDAO_MULTISIG_MINAPPROVALS` in `.env`
-- [ ] If new plugin builds are released
-  - [ ] Double-check that the build- and release-metadata is published correctly by the deploy script and contracts
+This checklist is seen as a guide to deploy the contracts to a new chain.
+
+## Pre deployment
+
+- [ ] Run `yarn` in the repository root to install the dependencies.
+- [ ] Run `yarn build` in `packages/contracts` to make sure the contracts compile.
+- [ ] Run `yarn test` in `packages/contracts` to make sure the contract tests succeed.
+
+  - To run the tests, edit `packages/contracts/.env` to contain:
+
+    ```env
+    HARDHAT_DAO_ENS_DOMAIN="testdao.eth"
+    HARDHAT_PLUGIN_ENS_DOMAIN="testpluginrepo.eth"
+
+    MANAGEMENT_DAO_SUBDOMAIN="management"
+    ```
+
+- [ ] Edit `packages/contracts/networks.json` and add your custom network to which you want to deploy to.
+
+  If contract verification is not available for your chain:
+
+  - Ensure that the `deploy` key for the new network looks exactly like: <br>
+    `"deploy": ["./deploy/new"]`
+
+  If contract verification is available:
+
+  - Define the `deploy` key like: <br>
+    `"deploy": ["./deploy/new", "./deploy/verification"]`.
+  - Define the `ETHERSCAN_KEY` variable for contract verification on the `.env` file. [Follow the Hardhat guide](https://hardhat.org/hardhat-runner/plugins/nomicfoundation-hardhat-verify) in this case.
+
+- [ ] Define the settings of the ENS domain used by OSx.
+
+  - Define the following ENS names in the `packages/contracts/.env` file, by replacing `SEPOLIA` with the name of the network name you’re deploying to:
+
+    ```env
+    SEPOLIA_DAO_ENS_DOMAIN="my-dao.eth"
+    SEPOLIA_PLUGIN_ENS_DOMAIN="my-dao.eth"
+    ```
+
+    - Ensure that domains end with a suffix like `.eth`
+
+  - If the target chain does not have an official ENS registry:
+    - A new, unofficial ENS registry will be deployed, along with a resolver
+    - No ownership is needed, the Managing DAO will own them
+  - If the target chain does have an official ENS registry:
+    - Ensure that the wallet under `ETH_KEY` owns the domain
+    - If you created the domains via the ENS app, they will be owned by an ENS wrapper which would cause the script to fail
+      - [Open the ENS app](https://app.ens.domains/) and click `unwrap` for each of these domains.
+      - [Example](https://app.ens.domains/morpheusplugin3.eth?tab=more)
+
+- [ ] If desired, update `MANAGEMENT_DAO_SUBDOMAIN` on `packages/contracts/.env`.
+  - In case you want the Managing DAO to use a different subdomain than the default one (`management`):
+  ```env
+  MANAGEMENT_DAO_SUBDOMAIN="root"         # would be root.my-dao.eth
+  ```
+- [ ] Define the the deployment wallet's private key on the `.env` file
+  ```env
+  ETH_KEY="your-private-key-here"          # without the 0x prefix
+  ```
+- [ ] Verify that the deployment wallet has sufficient funds to complete a protocol deployment.
 
 ## Deployment
 
-To deploy run `yarn deploy --network NETWORK` in `packages/contracts` and replace `NETWORK` with the correct network name (e.g. for mainnet it is `yarn deploy --network mainnet`)
-
-## After-Deployment
-
-### Configuration updates
-
-- [ ] Take the addresses from this file `packages/contracts/deployed_contracts.json`
-- [ ] Update `active_contracts.json` with the new deployed addresses
-- [ ] Update `packages/contracts/Releases.md` with the new deployed addresses
-- [ ] Add the managing DAOs' multisig address to `packages/contracts/.env.example` in the format `{NETWORK}_MANAGINGDAO_MULTISIG`
-
-### Verification
-
-- [ ] Take the addresses from this file `packages/contracts/deployed_contracts.json`
-- [ ] Wait for the deployment script finishing verification
-- [ ] Go to the blockchain explorer and verify that each address is verified
-  - [ ] If it is not try to verfiy it with `npx hardhat verify --network NETWORK ADDRESS CONTRUCTOR-ARGS`. More infos on how to use this command can be found here: [https://hardhat.org/hardhat-runner/docs/guides/verifying](https://hardhat.org/hardhat-runner/docs/guides/verifying)
-  - [ ] If it is a proxy try to activate the blockchain explorer's proxy feature
-  - [ ] If the proxies are not verified with the `Similar Match Source Code` feature
-    - [ ] Verify one of the proxies
-    - [ ] Check if the other proxies are now verified with `Similar Match Source Code`
-  - [ ] If it is a `PluginSetup`, check that the implementation is verified.
-
-### Configurations
-
-- [ ] Check that all managing DAO signers are members of the managing DAO multisig and no one else.
-- [ ] Check if the managing DAO is set in the `DAO_ENSSubdomainRegistrar`
-- [ ] Check if the managing DAO is set in the `Plugin_ENSSubdomainRegistrar`
-- [ ] Check if the managing DAO is set in the `DAORegistry`
-- [ ] Check if the `DAO_ENSSubdomainRegistrar` is set in the `DAORegistry`
-- [ ] Check if the managing DAO set in the `PluginRepoRegistry`
-- [ ] Check if the `Plugin_ENSSubdomainRegistrar` is set in the `PluginRepoRegistry`
-- [ ] Check if the `PluginRepoRegistry` is set in the `PluginRepoFactory`
-- [ ] Check if the `PluginRepoRegistry` is set in the `PluginSetupProcessor`
-- [ ] Check if the `DAORegistry` is set in the `DAOFactory`
-- [ ] Check if the `PluginSetupProcessor` is set in the `DAOFactory`
-- [ ] Check that the versions (and eventual `PlaceholderSetup` builds) are published correctly in the `token-voting-repo`, `address-list-voting-repo`, `multisig-repo`, and `admin-repo` and are synchronized across all supported networks.
-
-### Permissions
-
-- [ ] Check that the deployer has not the ROOT permission on the managing DAO
-- [ ] Check if `DAO_ENSSubdomainRegistrar` is approved for all for the DAO' ENS domain. Call `isApprovedForAll` on the ENS registry with the managing DAO as the owner and the `DAO_ENSSubdomainRegistrar` as the operator.
-- [ ] Check if `Plugin_ENSSubdomainRegistrar` is approved for all for the plugin' ENS domain. Call `isApprovedForAll` on the ENS registry with the managing DAO as the owner and the `Plugin_ENSSubdomainRegistrar` as the operator.
-- [ ] Check if the `DAORegistry` has `REGISTER_ENS_SUBDOMAIN_PERMISSION` on `DAO_ENSSubdomainRegistrar`
-- [ ] Check if the `PluginRepoRegistry` has `REGISTER_ENS_SUBDOMAIN_PERMISSION` on `Plugin_ENSSubdomainRegistrar`
-- [ ] Check if the `DAOFactory` has `REGISTER_DAO_PERMISSION` on `DAORegistry`
-- [ ] Check if the `PluginRepoFactory` has `REGISTER_PLUGIN_REPO_PERMISSION` on `PluginRepoRegistry`
-
-### Packages
-
-- [ ] Publish a new version of `@aragon/osx-artifacts` (`./packages/contracts`) to NPM
-- [ ] Publish a new version of `@aragon/osx` (`./packages/contracts/src`) to NPM
-- [ ] Publish a new version of `@aragon/osx-ethers` (`./packages/contracts-ethers`) to NPM
-- [ ] Update the changelog with the new version
-
-### Subgraph
-
-- [ ] Update `packages/subgraph/manifest/data/NETWORK.json` where `NETWORK` is replaced with the deployed network with the new contract addresses. If the file doesn't exist create a new one.
-- [ ] Update the version in `packages/subgraph/package.json`
-- [ ] Update `packages/subgraph/.env` with the correct values
-  - [ ] set `NETWORK_NAME` to the deployed network
-  - [ ] set `SUBGRAPH_NAME` to `osx`
-  - [ ] set `GRAPH_KEY` with the value obtained from the [Satsuma Dashboard](https://app.satsuma.xyz/dashboard)
-  - [ ] set the `SUBGRAPH_VERSION` to the same value as in `packages/subgraph/package.json`
-- [ ] Run `yarn manifest` in `packages/subgraph` to generate the manifest
-- [ ] Run `yarn build` in `packages/subgraph` to build the subgraph
-- [ ] Run `yarn test` in `packages/subgraph` to test the subgraph
-- [ ] Run `yarn deploy` in `packages/subgraph` to deploy the subgraph
-- [ ] Test the new deployed subgraph with the frontend team
-- [ ] Promote the new subgraph to live in the [Satsuma Dashboard](https://app.satsuma.xyz/dashboard)
-
-## Appendix
-
-- Changing the owner of the chosen ENS domains will also revoke the permissions of the `DAO_ENSSubdomainRegistrar` and `Plugin_ENSSubdomainRegistrar`. Therefore if the ownership gets transfered, restore the approval for these 2 contracts.
+When the settings above are correctly set:
+
+```sh
+cd packages/contracts               # if needed
+yarn deploy --network <NETWORK>     # Replace with mainnet, polygon, sepolia, etc
+```
+
+## Post deployment
+
+- After the script has exited, the deployment wallet will be the only one with `ROOT_PERMISSION` on your Managing DAO.
+  - This allows the deployent wallet to manually install plugins on the Managing DAO.
+  - After the required plugins are installed, `ROOT_PERMISSION` has to be revoked on the deployment wallet.
+- Should the script encounter any issues, the deployment should be re-run.
+  - The script will detect and re-use any previously deployed contracts.
+- After the process completes, check out the `packages/contracts/deployed_contracts.json` file to see the deployed contract addresses.
+
+## Other
+
+### Rerunning the deployment script
+
+If you need to restart the redeployment process and want Hardhat to not reuse the existing contracts:
+
+```sh
+rm -R deployments/<network-name>   # replace with the actual name
+```
+
+### Running the deployment script on a testnet
+
+If you want to simulate an L3 deployment within a testnet (sepolia) which has official ENS support, you may want to force the deployment of a new ENS Registry.
+
+Edit the `packages/contracts/deploy/helpers.ts` file and comment out the relevant line from `ENS_ADDRESSES` and `ENS_PUBLIC_RESOLVERS`.

packages/contracts/.env.example

@@ -28,22 +28,22 @@ ARBITRUMSEPOLIA_PLUGIN_ENS_DOMAIN=<ENS-Domain example: plugin.dao.eth>
 LOCALHOST_PLUGIN_ENS_DOMAIN=<ENS-Domain example: plugin.dao.eth>
 HARDHAT_PLUGIN_ENS_DOMAIN=<ENS-Domain example: plugin.dao.eth>
 
-MANAGINGDAO_SUBDOMAIN=<ENS-Subdomain example: "management">
-MANAGINGDAO_MULTISIG_APPROVERS=<list of addresses seperated by coma. example: "0x...01,0x...02">
-MANAGINGDAO_MULTISIG_MINAPPROVALS=<Minimum approvals. example: "1">
-MANAGINGDAO_MULTISIG_LISTEDONLY=<Listed Only. example: "true">
+MANAGEMENT_DAO_SUBDOMAIN=<ENS-Subdomain example: "management">
+MANAGEMENT_DAO_MULTISIG_APPROVERS=<list of addresses seperated by coma. example: "0x...01,0x...02">
+MANAGEMENT_DAO_MULTISIG_MINAPPROVALS=<Minimum approvals. example: "1">
+MANAGEMENT_DAO_MULTISIG_LISTEDONLY=<Listed Only. example: "true">
 
-MAINNET_MANAGINGDAO_MULTISIG=0x0673c13d48023efa609c20e5e351763b99dd67de
-GOERLI_MANAGINGDAO_MULTISIG=0x3263de63e70157c4b607982721026ffaa20e596c
-SEPOLIA_MANAGINGDAO_MULTISIG=0xfcEAd61339e3e73090B587968FcE8b090e0600EF
-POLYGON_MANAGINGDAO_MULTISIG=0x5db93850d843af581d8b87c350aa849a13a88e40
-MUMBAI_MANAGINGDAO_MULTISIG=0x944b067ccdbded94e64826747a5d72d4adcdf50a
-BASESEPOLIA_MANAGINGDAO_MULTISIG=0xBFa3Ea5Bf7C6491b7f24f2a3658fF1d9eAE11c01
-BASEMAINNET_MANAGINGDAO_MULTISIG=0x549B739731dFDfe256f9A3014b30035C05b6D1a6
-ARBITRUM_MANAGINGDAO_MULTISIG=0x02bBc496BEBC9a06C239670Cea663C43ceAd899F
-ARBITRUMSEPOLIA_MANAGINGDAO_MULTISIG=0xfcEAd61339e3e73090B587968FcE8b090e0600EF
+MAINNET_MANAGEMENT_DAO_MULTISIG=0x0673c13d48023efa609c20e5e351763b99dd67de
+GOERLI_MANAGEMENT_DAO_MULTISIG=0x3263de63e70157c4b607982721026ffaa20e596c
+SEPOLIA_MANAGEMENT_DAO_MULTISIG=0xfcEAd61339e3e73090B587968FcE8b090e0600EF
+POLYGON_MANAGEMENT_DAO_MULTISIG=0x5db93850d843af581d8b87c350aa849a13a88e40
+MUMBAI_MANAGEMENT_DAO_MULTISIG=0x944b067ccdbded94e64826747a5d72d4adcdf50a
+BASESEPOLIA_MANAGEMENT_DAO_MULTISIG=0xBFa3Ea5Bf7C6491b7f24f2a3658fF1d9eAE11c01
+BASEMAINNET_MANAGEMENT_DAO_MULTISIG=0x549B739731dFDfe256f9A3014b30035C05b6D1a6
+ARBITRUM_MANAGEMENT_DAO_MULTISIG=0x02bBc496BEBC9a06C239670Cea663C43ceAd899F
+ARBITRUMSEPOLIA_MANAGEMENT_DAO_MULTISIG=0xfcEAd61339e3e73090B587968FcE8b090e0600EF
 
-HARDHAT_MANAGINGDAO_MULTISIG=0xe3ADd897e69010709498738e5116C06B4D81e672 # Changes with each new version
+HARDHAT_MANAGEMENT_DAO_MULTISIG=0xe3ADd897e69010709498738e5116C06B4D81e672 # Changes with each new version
 
 # not using this variable will disable the feature. Anything else will enable it
 TEST_UPDATE_DEPLOY_SCRIPT=

packages/contracts/.gitignore

@@ -10,3 +10,4 @@ docs/osx/03-reference-guide
 #Hardhat files
 cache
 artifacts
+.upgradable

packages/contracts/README-zkSync.md

@@ -0,0 +1,82 @@
+# ZkSync
+
+ZkSync uses a zkEVM and so has different requirements.
+
+1. Ensure contracts are compiled with the zkSolc compiler
+
+- In your `hardhat.config.ts`, ensure the zkSync packages are uncommented, and the non-zkSync contracts that cause conflicts are commented out.
+
+```bash
+# Build your contracts with zkSolc
+yarn build --network zkSyncLocal
+
+# build with solc to ensure artifacts are present
+yarn build
+```
+
+2. Start a zkNode
+
+```bash
+# Start a zkSync node in a second terminal
+yarn node-zkSync
+```
+
+3. Execute tests
+
+```bash
+yard test --network zkLocalTestnet
+```
+
+You may need to configure tests to use custom matchers and wrappers. These are handled by wrapper and matcher files
+
+TODO: add more details on how to configure tests for zkSync
+
+1. Check that I correctly did skip tests as your suggestion/help.
+
+- I've reviewed and made a few small changes but otherwise they look good.
+- The tests log immediately, so have changed the language a bit
+
+2. In tokenVotingSetupZkSync contract, you will notice what I have done and why I got setUpgradePermissions and so on. If we can find a way to write this contract simpler, would be good.
+
+- It looks sensible. I think we should do a thorough review in the morning when we both have fresh eyes.
+
+- I think we need to find a way to revoke the UPGRADE permission inside the uninstallation script
+
+- I removed the permissionIndex from the test. This should always resolve to the final permission in the array so we can just use the permissions array length:
+  - If we have a governance ERC20, then we lengthen the array to 4
+  - We always add +1 to the permissions array if the upgrade permission is needed
+
+3. There are two tests that fail in dao.ts due to insufficient gas on zksync. Either skip it if you feel confident or on the weekends, I will also learn what the fuck they are.
+
+- Yep we need to look at these gas tests. On Monday I wasted ages on these and I'm honestly no closer to understanding wtf is going on. That being said I have said several times I think the gas logic of ZkSync doesn't make this a vulnerability.
+
+4. Take a look at GovernanceERC20Upgradeable and GovernanceWrappedERC20Upgradeable. What struck me is that the contracts they inherit from(i.e GovernanceERC20 and GovernanceWrappedERC20), they don’t have gaps which means that if we in the future decide to add new variable in GovernanceERC20Upgradeable and then in GovernanceERC20, we will mess up the storage. But let’s go with how it is. I really don’t care since this case won’t happen, but I need you to exactly understand what I mean so you’re aware of that too.
+
+- Good spot. So just reiterating:
+  - GovE20Up <- GovE20 (wrapped has similar chain)
+  - We can add new storage vars to the upgradeable version but not to the GovE20, because we have no gap, so it will start writing to storage slots inside GovERC20Up
+- Would a workaround not be to first declare a `__gap` of reserved storage slots on GovERC20upgradeable THEN we add new storage variables (if needed)
+- We would need to be careful this doesn't linearize unexpectedly and write to UUPSUpgradeable Storage.
+- Why do you say "this case won't happen"?
+
+5. In hardhat-deploy script, we shouldn’t deploy Admin plugin if network is zksync.
+
+- I added a skip function to the admin deploy steps, you can take a look.
+
+6. Play with running tests on zksync and hardhat to see how it looks and if I missed anything or not. We can deal with deployment in the next days. Maybe you come across that skipped message sometimes prints ‘f’. You can change it as you see fit.
+
+- I removed the `f` tests
+- I ran into some issues with matchers not firing, I brought these across into our testing lib
+- Ran the whole test suite on 1.4.1, lgtm other than existing comments
+
+7.  On hardhat-deploy script, we should deploy TokenVotingSetupZksync and also Governance contracts as upgradeable versions.
+
+8.  Make psp upgradeable(create PSPZKSYNC), and deploy it on zksync network in hardhat-deploy script. Add some tests to test whether it can be upgraded.
+9.  Maybe we should write tests to check whether my GovernanceERC20Upgradeable and GovernanceWrappedERC20Upgradeable can be upgraded.
+10. I skipped TokenFactory tests completely on all networks as we’re not using it at all(do we use it ?)
+
+11. To run tests on zksync, in hardhat.config.ts, you need to comment some stuff and to run tests on hardhat, you need to uncomment and comment other stuff. I asked about this and hardhat team said to use multiple configuration files. Just know that this is required to comment/uncomment.
+
+- I made a quick attempt to add `hardhat.config-zk.ts`
+- I then used this in `yarn hardhat --config hardhat.config-zk.ts test --network zkLocalTestnet` but I got some weird errors related to the wrapper not being instantiated
+- We can come back to this, but I don't think it's essential just yet