[Rebased] Migrate governance tests from Truffle to Foundry #11415

Mc01 · 2025-05-20T12:18:00Z

Rebased clone of #11402

Description

PR porting old governance integration tests from Truffle as new governance e2e tests in Foundry

Other changes

I needed to fix Migrations.s.sol scripts and additionally added ton of small improvements over the way - including disabling anvil rpc call logging with env var

Tested

With e2e testing.

Related issues

Part of Removal of old tests in Truffle #11378
Fixes Recreate governance integration tests in Foundry #11387
Removes old code touching Optional: Recreate integration tests for incorrectly initialized proxy in Foundry #11388

Backwards compatibility

Previous integration tests in Truffle were not running currently, so it brings some of them back.

Documentation

Internal tooling - no changes here

Co-authored-by: pahor167 <[email protected]>

socket-security · 2025-05-20T12:20:22Z

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	@celo/env-tests@1.0.3

View full report

Mc01

Ported old review comments from @pahor167
For accessing resolved comments -> take a look on older PR: #11402

Mc01 · 2025-05-20T12:22:50Z

packages/protocol/contracts-0.8/common/UsingRegistryV2NoMento.sol

+    keccak256(abi.encodePacked("EpochManagerEnabler"));
+  bytes32 internal constant EPOCH_MANAGER_REGISTRY_ID = keccak256(abi.encodePacked("EpochManager"));
+  bytes32 internal constant ESCROW_REGISTRY_ID = keccak256(abi.encodePacked("Escrow"));
+  // bytes32 internal constant EXCHANGE_REGISTRY_ID = keccak256(abi.encodePacked("Exchange"));


If they have been commented since CR12, they can be removed as they are not being used. Since this file is only used for testing, It should be safe to remove.

Mc01 · 2025-05-20T12:23:34Z

packages/protocol/test-sol/devchain/e2e/utils.sol

+// import { BlockchainParameters } from "@celo-contracts/governance/BlockchainParameters.sol";
+import { ICeloUnreleasedTreasury } from "@celo-contracts/common/interfaces/ICeloUnreleasedTreasury.sol";
+import { ICeloToken } from "@celo-contracts/common/interfaces/ICeloToken.sol";
+// import { DoubleSigningSlasher } from "@celo-contracts/governance/DoubleSigningSlasher.sol";


Not sure why these were added since they are not being used in this file.

Also note:
DownTimeSlasher,DoubleSigningSlasher,BlockchainParameters have been deprecated post L2.

Mc01 · 2025-05-20T12:24:58Z

packages/protocol/migrations_sol/Migration.s.sol

-          uint256 threshold = abi.decode(
-            constitutionJson.parseRaw(string.concat(".", contractName, ".", functionName)),
-            (uint256)
+            string.concat("  Setting constitution thresholds for function: ", loop_.functionName)


Mc01 · 2025-05-20T12:26:28Z

packages/protocol/migrations_sol/constants.sol

@@ -7,7 +7,8 @@ contract MigrationsConstants is TestConstants {
  address constant DEPLOYER_ACCOUNT = 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266;

  // List of contracts that are expected to be in Registry.sol
-  string[27] contractsInRegistry = [
+  // TODO: Probably should be synced with Migration.s.sol


Mc01 · 2025-05-20T12:27:25Z

packages/protocol/test-sol/devchain/e2e/utils.sol

+    // doubleSigningSlasher = DoubleSigningSlasher(
+    //   registryContract.getAddressForOrDie(DOUBLE_SIGNING_SLASHER_REGISTRY_ID)
+    // ); // FIXME: DoubleSigningSlasher is not in UsingRegistry.sol


see comment above as to why they are no longer in registry.

martinvol · 2025-05-21T16:47:37Z

packages/protocol/contracts-0.8/common/UsingRegistryV2NoMento.sol

@@ -95,14 +110,47 @@ contract UsingRegistryV2NoMento {
    return IAccounts(registryContract.getAddressForOrDie(ACCOUNTS_REGISTRY_ID));
  }

+  // FIXME: Deprecated?


I think it's safe to remove, it's a helper anyways

martinvol · 2025-05-21T16:51:05Z

packages/protocol/contracts/common/libraries/StringUtils.sol

would maybe add somewhere else, maybe inside migrations_sol, because this will likely be out of scope in the next audit, but if we ever use it in production following that, then it wont show up in the diff and thus will not be audited

martinvol · 2025-05-21T16:52:00Z

packages/protocol/contracts/governance/GovernanceSlasher.sol

I wouldn't change it, but I try not to do linting in PRs doing something else as they get hard to review, just a nit

martinvol · 2025-05-21T16:52:39Z

packages/protocol/contracts/governance/interfaces/IGovernance.sol

-    uint256 proposalId
-  ) external view returns (address, uint256, uint256, uint256, string memory, uint256, bool);
-
+  // referendum


martinvol · 2025-05-21T16:59:05Z

packages/protocol/migrations_sol/constants.sol

@@ -7,7 +7,8 @@ contract MigrationsConstants is TestConstants {
  address constant DEPLOYER_ACCOUNT = 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266;

  // List of contracts that are expected to be in Registry.sol
-  string[27] contractsInRegistry = [
+  // TODO: Probably should be synced with Migration.s.sol
+  string[26] contractsInRegistry = [


I couldn't figure out how to make the compiler come up with the number of items automatically, maybe we should move to json at some point

martinvol · 2025-05-21T17:00:20Z

packages/protocol/scripts/foundry/start_anvil.sh

@@ -45,7 +45,7 @@ while ! nc -z localhost $ANVIL_PORT; do
 done

 # enabled logging
-cast rpc anvil_setLoggingEnabled true --rpc-url $ANVIL_RPC_URL
+cast rpc anvil_setLoggingEnabled $ANVIL_LOGGING_ENABLED --rpc-url $ANVIL_RPC_URL


Can we keep the default as true?

We can, but I freshly changed it to true based on @pahor167 request 😁

martinvol · 2025-05-21T17:01:47Z

packages/protocol/test-sol/devchain/e2e/common/Governance.t.sol

+    JsonFiles memory files_ = JsonFiles(
+      vm.readFile("./governanceConstitution.json"), // constitution json
+      vm.readFile("./.tmp/selectors/Proxy.json") // proxy selectors
+    );
+    FileProps memory props_ = FileProps(
+      vm.parseJsonKeys(files_.constitutionJson, ""), // contract names
+      vm.parseJsonKeys(files_.constitutionJson, ".Proxy"), // proxy names
+      vm.parseJsonKeys(files_.proxySelectors, "") // proxy sigs
+    );
+
+    // vars for looping
+    LoopVars memory loop_;
+    string memory contractSelectors_;
+    string[] memory functionsWithTypes_;
+    string[] memory functionNames_;
+
+    // loop over contract names
+    for (uint256 i = 0; i < props_.contractNames.length; i++) {
+      loop_.contractName = props_.contractNames[i];
+      if (loop_.contractName.equals("Proxy")) {
+        // skip proxy address
+        continue;
+      } else {
+        // set address from registry
+        loop_.contractAddress = registryContract.getAddressForStringOrDie(loop_.contractName);
+      }
+
+      // load selectors for given contract from file
+      contractSelectors_ = vm.readFile(


Is this block (and beyond) duplicated from Migrations.s.sol?

It overlaps with Migrations.s.sol to some extent -> will check how much I can de-duplicate it tomorrow 👍

Mc01 and others added 15 commits May 20, 2025 14:12

WIP.

369cd6a

Consitution test that fail with anvil.

707e31e

Identify part of migration script that causes tests to fail.

8a89a55

Wip: fixing migration script.

b0d7c1d

Do not rely on ffi mode.

18edc80

Fix constitution values in anvil migration.

eaf989c

Port old governance tests from Truffle.

cccae7a

Finish porting old governance integration tests to Foundry.

9c742cf

Fix unit tests.

871c93c

Fix CI.

beb9e52

Fix remappings for CI.

be8e112

Fix TS deps.

7ead149

Update comment.

c014002

Co-authored-by: pahor167 <[email protected]>

CR.

41d5657

Combine proxy selectors with contract selectors.

4fa52bd

Mc01 requested review from a team as code owners May 20, 2025 12:18

Mc01 requested a review from alvarof2 May 20, 2025 12:18

Mc01 changed the base branch from master to release/core-contracts/13 May 20, 2025 12:18

Mc01 commented May 20, 2025

View reviewed changes

Mc01 mentioned this pull request May 20, 2025

Migrate governance tests from Truffle to Foundry #11402

Closed

martinvol reviewed May 21, 2025

View reviewed changes

[Rebased] Migrate governance tests from Truffle to Foundry #11415

Are you sure you want to change the base?

[Rebased] Migrate governance tests from Truffle to Foundry #11415

Uh oh!

Conversation

Mc01 commented May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Other changes

Tested

Related issues

Backwards compatibility

Documentation

Uh oh!

socket-security bot commented May 20, 2025

Uh oh!

Mc01 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

soloseng May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Mc01 commented May 20, 2025 •

edited

Loading

soloseng May 20, 2025 •

edited

Loading