Sync release/core-contracts/12 with master (#11033)

* test: proof of concept 2e2 test using anvil devchain (#11020) * chore(test-sol/FeeCurrencyDirectory)): use `@celo-...` remapping not `../..` * test(test-sol/FeeCurrencyDirectory)): MVP e2e test MVP demo of an e2e test using the devchain. This is not the pattern I'll suggest, but good to see the test passes end-to-end. * chore(foundry.toml): adds `@test-sol` remapping * chore(test-sol/e2e): adds MVP `utils.sol` Idea is to have a Devchain class that can be inherited by Test contracts to have access to the deployed contracts on the devchain. * test(FeeCurrencyDirectory): MVP test using `Devchain` class * chore(migrations_sol): adds MVP script to run e2e tests * style(test-sol/e2e): linting * refactor(test-sol/FeeCurrencyDirectory): moves file to `.../e2e/` * chore(migrations_sol): use `test-sol/e2e/*` path * chore(test-sol/FeeCurrencyDirectory): match contract name in unit and e2e test * style(test-sol/FeeCurrencyDirectory): linting * chore(e2e/utils): removes unused imports and more Cleans up Adds `TODO` comments * test(test-sol/e2e): renames contract with "E2E..." I'm temporarily adding the "E2E..." prefix to the contract so I can exclude this test and the integration tests during the CI run. In a separate PR, I'll refactor the tests into a directory structure like: ``` test-sol/unit/ test-sol/e2e/ test-sol/integration/ ``` That way we could run tests with something like this: ``` # unit tests forge test --match-path "*test-sol/unit/*" # e2e tests forge test --match-path "*test-sol/e2e/*" # integration tests forge test --match-path "*test-sol/integration/*" ``` * chore(workflows/protocol_tests): excludes e2e test from workflow I'm temporarily adding the "E2E..." prefix to the contract so I can exclude this test and the integration tests during the CI run. In a separate PR, I'll refactor the tests into a directory structure like: ``` test-sol/unit/ test-sol/e2e/ test-sol/integration/ ``` That way we could run tests with something like this: ``` # unit tests forge test --match-path "*test-sol/unit/*" # e2e tests forge test --match-path "*test-sol/e2e/*" # integration tests forge test --match-path "*test-sol/integration/*" ``` * style(test-sol/e2e): linting * chore(e2e): temporarily renames contract with "E2E..." In subsequent PRs, I'll rename this more accurately. * feat: add ReserveSpenderMultiSig to anvil migrations (#11028) * feat(migrations_sol/migrationsConfig): adds `reserveSpenderMultiSig` configs * feat(migrations_sol): adds `migrateReserveSpenderMultiSig` Also adds contract calls in `migrateReserve` function. Not tested, and not sure this works yet. I might be missing some changes. * chore(test-sol/utils): adds `ReserveSpenderMultiSig.t.sol` From the code comment: The purpose of this file is not to provide test coverage for `ReserveSpenderMultiSig.sol`. This is an empty test to force foundry to compile `ReserveSpenderMultiSig.sol`, and include its artifacts in the `/out` directory. `ReserveSpenderMultiSig.sol` is needed in the migrations script, but because it's on Solidity 0.5 it can't be imported there directly. If there is a better way to force foundry to compile `ReserveSpenderMultiSig.sol` without this file, this file can confidently be deleted. * feat(migrations_sol/HelperInterFaces): adds `IReserveSpenderMultiSig` The helper interface is used as a workaround to allow the migrations script (`Migrations.s.sol`) to be on Solidity 0.8, while the ReserveSpenderMultiSig contract is on Solidity 0.5. The migration script only needs to initialize the contract, which is why the helper interface only defines an `initialize` function. * chore(migrations_sol): initialize `ReserveSpenderMultiSig` * chore(migrations_sol): adds code comment for readability * chore(migrations_sol): improves code comment, moves code block up * chore(migrations_sol): fix typo * style(migrations_sol): linting * test: refactor foundry test directory into unit, e2e, and integration tests (#11023) * chore(foundry.toml): adds `@mento-core/...` remapping Also moves existing mappings into groups for better readability * refactor(test-sol/common): moves files to `unit/common/` Also updates imports respectively using remappings. * refactor(test-sol/identity): moves files to `unit/identity/` Also updates imports using remappings where necessary. * refactor(test-sol/stability): moves files to `unit/stability/` Also updates imports using remappings where necessary. * refactor(test-sol/voting): moves files to `unit/voting/` Also updates imports using remappings where necessary. * refactor(test-sol/governance): moves files to `unit/governance/` Also updates imports using remappings where necessary. * chore(test-sol): update missing remappings With these changes all contracts compile and resolve correctly. * chore(workflows/protocol_tests): updates paths to `test-sol/unit` * chore(workflows/protocol_tests): adds integration and e2e tests * style(workflows/protocol_tests): refactors `forge test` for better readability * chore(migrations_sol): moves scripts to `scripts/foundry/ Moves all bash scripts from `migrations_sol` to `scripts/foundry/`, and updates paths where necessary. We already have a directory for `scripts/truffle/` and `scripts/bash/`, so this makes it easier to find foundry-related bash scripts. * test(governance/mock): move `MockGovernance` to `unit/` folder * refactor(foundry.toml): rename `migrations-sol/` remapping * refactor(workflows): refactor "run everything" step Runs all tests in the `unit/` directory instead of all test files in the repo. This makes sense from my perspective, because e2e tests (in the `e2e/` directory) and integration tests (in the `integration/` directory) require a connection to an anvil devchain serving at localhost. The intent of this command is to ensure that no unit tests are forgotten, since unit tests are run explicitly by going through the directories above. But, the intention is not to run all tests in the repo generally. * style(workflows/protocol-devchain-anvil): splits migration into 2 steps This helps the script becoming more readable, and ensure error logs are clearly associated with a workflow step rather than a single bash script. * chore(workflows): defines `ANVIL_PORT` env variable in workflow Previously, the `$ANVIL_PORT` variable was exported and passed around as an env variable in a bash script. But that required generating anvil migrations and running a devchain repeatedly. Instead, the workflow does that once and different steps can access the devchain. But, in that case the env variable is not passed around, so it has to be defined at the workflow level. Source: https://docs.github.com/en/actions/learn-github-actions/variables * chore(workflows/protocol_tests): removes code comment * feat(scripts/foundry): adds `stop_anvil.sh` * refactor(scripts/foundry): use `stop_anvil.sh` to stop anvil * feat(protocol/package.json): adds `anvil-devchain:...` (`start` and `stop`) * refactor(scripts/foundry): use `stop_anvil.sh` to stop anvil * refactor(migrations_sol): update `@migrations-sol` remapping Previous the remapping was called `@celo-migrations`. * docs(migrations_sol/README): renames README file Also changes title to match NPM package name. This is a matter of personal preference. IMO this is a small improvement in readability for 3rd party users. * docs(scripts/foundry): adds more links to `package.json` Adds `repository` and `directory` to `package.json`. This ensures npmjs.org displays hyperlinks to the github repository. * docs(migrations_sol/CONTRIBUTING): adds MVP dev docs We can complete this doc with additional information regarding the anvil devchain and migrations going forward. * docs(migrations_sol/README): adds "how we work" section This is helpful for 3rd party developers that found this package on npmjs.org. The links help developers take action and help improve the anvil devchain. * Soloseng/CHORE-update-workflow (#11029) * update workflow to run on push to release branches * ++ mintgoldschedule to migration test constants * update GH WF --------- Co-authored-by: Arthur Gousset <46296830+arthurgousset@users.noreply.github.com>
celo-org · Jun 12, 2024 · 1adfa6c · 1adfa6c
1 parent 95af1c6
commit 1adfa6c
Show file tree

Hide file tree

Showing 73 changed files with 382 additions and 111 deletions.
diff --git a/.github/workflows/celo-monorepo.yml b/.github/workflows/celo-monorepo.yml
@@ -8,6 +8,7 @@ on:
   push:
     branches:
       - master
+      - 'release/**'
   pull_request:
     branches:
       - master

diff --git a/.github/workflows/containers.yaml b/.github/workflows/containers.yaml
@@ -8,6 +8,7 @@ on:
       - 'packages/celotool/**'
     branches:
       - master
+      - 'release/**'
   pull_request:
     paths:
       - 'dockerfiles/**'

diff --git a/.github/workflows/protocol-devchain-anvil.yml b/.github/workflows/protocol-devchain-anvil.yml
@@ -3,10 +3,12 @@ on:
   push:
     branches:
       - master
+      - 'release/**'
 
 env:
   # Increment these to force cache rebuilding
   FOUNDRY_CACHE_KEY: 1
+  ANVIL_PORT: 8546
 
 jobs:
   build:
@@ -86,8 +88,16 @@ jobs:
         id: date
         run: echo "::set-output name=date::$(date +'%Y-%m-%d')"
 
-      - name: Generate migrations
-        run: ./migrations_sol/run_integration_tests_in_anvil.sh
+      - name: Generate migrations and run devchain
+        if: success() || failure()
+        run: ./scripts/foundry/create_and_migrate_anvil_devchain.sh
+
+      - name: Run integration tests against local anvil devchain
+        if: success() || failure()
+        run: |
+          forge test -vvv \
+          --match-path "test-sol/integration/*" \
+          --fork-url http://127.0.0.1:${{ env.ANVIL_PORT }}
 
       - name: Sanitize ref name
         id: sanitize-ref-name

diff --git a/.github/workflows/protocol_tests.yml b/.github/workflows/protocol_tests.yml
@@ -3,6 +3,7 @@ on:
   push:
     branches:
       - master
+      - 'release/**'
   pull_request:
     branches:
       - master
@@ -11,6 +12,7 @@ on:
 env:
   # Increment these to force cache rebuilding
   FOUNDRY_CACHE_KEY: 2
+  ANVIL_PORT: 8546
 
 jobs:
   check:
@@ -61,30 +63,47 @@ jobs:
       - name: Compile Contracts
         run: forge --version && forge compile
 
-      - name: Run tests common
+      - name: Run unit tests common
         # can't use gas limit because some setUp function use more than the limit
-        run: forge test -vvv --match-path "test-sol/common/*" # --block-gas-limit 50000000
+        run: |
+          forge test -vvv \
+          --match-path "test-sol/unit/common/*"
 
-      - name: Run tests governance/network
+      - name: Run unit tests governance/network
         if: success() || failure()
-        run: forge test -vvv --block-gas-limit 50000000 --match-path "test-sol/governance/network/*"
+        run: |
+          forge test -vvv \
+          --match-path "test-sol/unit/governance/network/*" \
+          --block-gas-limit 50000000
 
-      - name: Run tests governance/validators
+      - name: Run unit tests governance/validators
         if: success() || failure()
-        run: forge test -vvv --block-gas-limit 50000000 --match-path "test-sol/governance/validators/*"
+        run: | 
+          forge test -vvv \
+          --match-path "test-sol/unit/governance/validators/*" \
+          --block-gas-limit 50000000
 
-      - name: Run tests governance/voting
+      - name: Run unit tests governance/voting
         # can't use gas limit because some setUp function use more than the limit
         if: success() || failure()
-        run: forge test -vvv --match-path "test-sol/governance/voting/*" --block-gas-limit 50000000
+        run: | 
+          forge test -vvv \
+          --match-path "test-sol/unit/governance/voting/*" \
+          --block-gas-limit 50000000
 
-      - name: Run tests stability
+      - name: Run unit tests stability
         if: success() || failure()
-        run: forge test -vvv --block-gas-limit 50000000 --match-path "test-sol/stability/*"
+        run: |
+          forge test -vvv \
+          --match-path "test-sol/unit/stability/*" \
+          --block-gas-limit 50000000
 
-      - name: Run tests identity
+      - name: Run unit tests identity
         if: success() || failure()
-        run: forge test -vvv --block-gas-limit 50000000 --match-path "test-sol/identity/*"
+        run: |
+          forge test -vvv \
+          --match-path "test-sol/unit/identity/*" \
+          --block-gas-limit 50000000
 
       - name: Fail if there are tests without folder
         if: success() || failure()
@@ -94,11 +113,28 @@ jobs:
             exit 1
           fi
 
-      - name: Run Everything just in case something was missed
+      - name: Run all unit tests in case some were missed (excl. integration and e2e tests)
         # can't use gas limit because some setUp function use more than the limit
-        # Excludes integration tests, because they require an anvil devchain running on the localhost.
-        run: forge test -vvv --no-match-contract RegistryIntegrationTest
+        # Excludes e2e and integration tests, because they require a connection to an anvil devchain
+        # serving at localhost.
+        run: |
+          forge test -vvv \
+          --match-path "test-sol/unit/*"
       
-      - name: Generate migrations
+      - name: Generate migrations and run devchain
         if: success() || failure()
-        run: ./migrations_sol/run_integration_tests_in_anvil.sh
+        run: ./scripts/foundry/create_and_migrate_anvil_devchain.sh
+
+      - name: Run integration tests against local anvil devchain
+        if: success() || failure()
+        run: |
+          forge test -vvv \
+          --match-path "test-sol/integration/*" \
+          --fork-url http://127.0.0.1:${{ env.ANVIL_PORT }}
+      
+      - name: Run e2e tests against local anvil devchain
+        if: success() || failure()
+        run: |
+          forge test -vvv \
+          --match-path "test-sol/e2e/*" \
+          --fork-url http://127.0.0.1:${{ env.ANVIL_PORT }}
diff --git a/packages/protocol/foundry.toml b/packages/protocol/foundry.toml
@@ -4,18 +4,20 @@ out = 'out'
 test = 'test-sol'
 libs = ['lib', 'node_modules']
 remappings = [
+    '@celo-contracts/=contracts/',
+    '@celo-contracts-8=contracts-0.8/',
+    '@mento-core/=lib/mento-core',
     'openzeppelin-solidity/=lib/openzeppelin-contracts/',
-    'solidity-bytes-utils/=lib/solidity-bytes-utils/',
-    'forge-std/=lib/celo-foundry/lib/forge-std/src/',
-    'ds-test/=lib/celo-foundry/lib/forge-std/lib/ds-test/src/',
+    '@openzeppelin/contracts8/=lib/openzeppelin-contracts8/contracts/',
     'celo-foundry/=lib/celo-foundry/src/',
-    '@summa-tx/memview.sol/=lib/memview.sol',
     'celo-foundry-8/=lib/celo-foundry-8/src/',
+    '@test-sol/=test-sol/',
+    '@migrations-sol/=migrations_sol/',
+    'forge-std/=lib/celo-foundry/lib/forge-std/src/',
     'forge-std-8/=lib/celo-foundry-8/lib/forge-std/src/',
-    '@celo-contracts-8=contracts-0.8/',
-    '@openzeppelin/contracts8/=lib/openzeppelin-contracts8/contracts/',
-    '@celo-contracts/=contracts/',
-    '@celo-migrations/=migrations_sol/'
+    '@summa-tx/memview.sol/=lib/memview.sol',
+    'solidity-bytes-utils/=lib/solidity-bytes-utils/',
+    'ds-test/=lib/celo-foundry/lib/forge-std/lib/ds-test/src/',
 ]
 
 no_match_test = "skip"

diff --git a/packages/protocol/migrations_sol/CONTRIBUTING.md b/packages/protocol/migrations_sol/CONTRIBUTING.md
@@ -0,0 +1,57 @@
+# Anvil migrations 
+
+The "Anvil migrations" are a set of scripts that generate a local anvil-based devchain. 
+This devchain is useful for testing and development of the Celo protocol.
+
+## Usage
+
+### Start devchain
+
+```sh
+$ yarn anvil-devchain:start
+``` 
+
+Starts a new anvil devchain serving at localhost (default port 8546).
+
+For example:
+
+```sh
+$ yarn anvil-devchain:start
+
+yarn run v1.22.22
+$ ./scripts/foundry/create_and_migrate_anvil_devchain.sh
+# ...
+Total elapsed time: 193 seconds
+✨  Done in 193.09s.
+```
+
+You can now run commands against the local devchain.
+
+For example:
+
+```sh
+cast block-number \
+--rpc-url http://127.0.0.1:8546
+266
+```
+
+### Stop devchain
+
+```sh
+$ yarn anvil-devchain:stop
+```
+
+Terminates any anvil nodes serving at localhost.
+
+For example:
+
+```sh
+# in packages/protocol/ directory
+$ yarn anvil-devchain:stop
+
+yarn run v1.22.22
+$ ./scripts/foundry/stop_anvil.sh
+Connection to localhost port 8546 [tcp/*] succeeded!
+Killed Anvil
+✨  Done in 0.11s.
+```
diff --git a/packages/protocol/migrations_sol/HelperInterFaces.sol b/packages/protocol/migrations_sol/HelperInterFaces.sol
@@ -49,3 +49,17 @@ interface IExchangeInitializer {
 interface IExchange {
   function activateStable() external;
 }
+
+interface IReserveSpenderMultiSig {
+  /**
+    @dev Contract constructor sets initial owners and required number of confirmations.
+    @param _owners List of initial owners.
+    @param _required Number of required confirmations for external transactions.
+    @param _internalRequired Number of required confirmations for internal transactions. 
+  */
+  function initialize(
+    address[] calldata _owners,
+    uint256 _required,
+    uint256 _internalRequired
+  ) external;
+}
diff --git a/packages/protocol/migrations_sol/Migration.s.sol b/packages/protocol/migrations_sol/Migration.s.sol
@@ -43,12 +43,12 @@ import "@celo-contracts/stability/interfaces/ISortedOracles.sol";
 import "@celo-contracts-8/common/interfaces/IGasPriceMinimumInitializer.sol";
 import "@celo-contracts-8/common/interfaces/IMintGoldScheduleInitializer.sol";
 
-import "./HelperInterFaces.sol";
+import "@migrations-sol/HelperInterFaces.sol";
 import "@openzeppelin/contracts8/utils/math/Math.sol";
 
 import "@celo-contracts-8/common/UsingRegistry.sol";
 
-import { Constants } from "@celo-migrations/constants.sol";
+import { Constants } from "@migrations-sol/constants.sol";
 
 contract ForceTx {
   // event to trigger so a tx can be processed
@@ -212,6 +212,7 @@ contract Migration is Script, UsingRegistry, Constants {
     migrateGoldToken(json);
     migrateSortedOracles(json);
     migrateGasPriceMinimum(json);
+    migrateReserveSpenderMultiSig(json);
     migrateReserve(json);
     migrateStableToken(json);
     migrateExchange(json);
@@ -329,9 +330,31 @@ contract Migration is Script, UsingRegistry, Constants {
     );
   }
 
-  function migrateReserve(string memory json) public {
-    // Reserve spend multisig not migrated
+  function migrateReserveSpenderMultiSig(string memory json) public {
+    address[] memory owners = new address[](1);
+    owners[0] = deployerAccount;
+
+    uint256 required = abi.decode(json.parseRaw(".reserveSpenderMultiSig.required"), (uint256));
+    uint256 internalRequired = abi.decode(
+      json.parseRaw(".reserveSpenderMultiSig.internalRequired"),
+      (uint256)
+    );
 
+    // Deploys and adds the ReserveSpenderMultiSig to the Registry for ease of reference.
+    // The ReserveSpenderMultiSig is not in the Registry on Mainnet, but it's useful to keep a
+    // reference of the deployed contract, so it's in the Registry on the devchain.
+    deployProxiedContract(
+      "ReserveSpenderMultiSig",
+      abi.encodeWithSelector(
+        IReserveSpenderMultiSig.initialize.selector,
+        owners,
+        required,
+        internalRequired
+      )
+    );
+  }
+
+  function migrateReserve(string memory json) public {
     uint256 tobinTaxStalenessThreshold = abi.decode(
       json.parseRaw(".reserve.tobinTaxStalenessThreshold"),
       (uint256)
@@ -374,9 +397,14 @@ contract Migration is Script, UsingRegistry, Constants {
     // TODO this should be a transfer from the deployer rather than a deal
     vm.deal(reserveProxyAddress, initialBalance);
 
-    address reserveSpenderMultiSig = deployerAccount;
-    IReserve(reserveProxyAddress).addSpender(reserveSpenderMultiSig);
-    console.log("reserveSpenderMultiSig set to:", reserveSpenderMultiSig);
+    // Adds ReserveSpenderMultiSig to Reserve
+    bool useSpender = abi.decode(json.parseRaw(".reserveSpenderMultiSig.required"), (bool));
+    address spender = useSpender
+      ? registry.getAddressForString("ReserveSpenderMultiSig")
+      : deployerAccount;
+
+    IReserve(reserveProxyAddress).addSpender(spender);
+    console.log("reserveSpenderMultiSig added as Reserve spender");
   }
 
   function deployStable(

diff --git a/packages/protocol/migrations_sol/README.md b/packages/protocol/migrations_sol/README.md
@@ -0,0 +1,30 @@
+# @celo/devchain-anvil
+
+Anvil state with Celo core contracts for local testing and development.
+
+# Usage
+
+```bash
+npm install --save-dev @celo/devchain-anvil
+anvil --state <path_to_devchain.json>
+```
+
+## How we work
+
+We are a GitHub-first team, which means we have a strong preference for communicating via GitHub. 
+Please use GitHub to:
+
+🐞 [File a bug report](https://github.com/celo-org/celo-monorepo/issues/new/choose)
+
+💬 [Ask a question](https://github.com/celo-org/celo-monorepo/discussions)
+
+✨ [Suggest a feature](https://github.com/celo-org/celo-monorepo/issues/new/choose)
+
+🧑‍💻 [Contribute!](https://github.com/celo-org/celo-monorepo/tree/master/packages/protocol/migrations_sol/CONTRIBUTING.md)
+
+🚔 [Report a security vulnerability](https://github.com/celo-org/celo-monorepo/issues/new/choose)
+
+> [!TIP]
+> 
+> Please avoid messaging us via Slack, Telegram, or email. We are more likely to respond to you on 
+> GitHub than if you message us anywhere else. We actively monitor GitHub, and will get back to you shortly 🌟
diff --git a/packages/protocol/migrations_sol/celo-anvil-README.md b/packages/protocol/migrations_sol/celo-anvil-README.md
diff --git a/packages/protocol/migrations_sol/constants.sol b/packages/protocol/migrations_sol/constants.sol
@@ -2,7 +2,7 @@ pragma solidity >=0.8.7 <0.8.20;
 
 contract Constants {
   // List of contracts that are expected to be in Registry.sol
-  string[23] contractsInRegistry = [
+  string[24] contractsInRegistry = [
     "Accounts",
     "BlockchainParameters",
     "DoubleSigningSlasher",
@@ -19,6 +19,7 @@ contract Constants {
     "Governance",
     "GovernanceSlasher",
     "LockedGold",
+    "MintGoldSchedule",
     "OdisPayments",
     "Random",
     "Registry",