Skip to content

Commit

Permalink
Integration tests for Anvil migrations (#11002)
Browse files Browse the repository at this point in the history
* test(protocol/migrations_sol): adds existing unit tests

* test(test-sol/integration): adds e2e test for GoldToken

This is a test run, to see how `setUp()` and contract definitions work.

1. run devchain

```sh
./migrations_sol/create_and_migrate_anvil_devchain.sh
```

2. run test against devchain

```sh
forge test \
--match-path test-sol/integration/Integration.t.sol \
--match-contract GoldTokenTest_General \
-vvv \
--fork-url http://127.0.0.1:8546
```

Tests pass

Output:

```sh
[⠰] Compiling...
No files changed, compilation skipped

Running 3 tests for test-sol/integration/Integration.t.sol:GoldTokenTest_General
[PASS] test_decimals() (gas: 10837)
Logs:
  GoldToken address is: 0xfE8CbC1cFA1b3b8256f310bdfd40E60597083448

[PASS] test_name() (gas: 12537)
Logs:
  GoldToken address is: 0xfE8CbC1cFA1b3b8256f310bdfd40E60597083448

[PASS] test_symbol() (gas: 12579)
Logs:
  GoldToken address is: 0xfE8CbC1cFA1b3b8256f310bdfd40E60597083448

Test result: ok. 3 passed; 0 failed; 0 skipped; finished in 135.40ms

Ran 1 test suites: 3 tests passed, 0 failed, 0 skipped (3 total tests)
```

* test(test-sol/integration): adds loop to assert contract is in Registry

* chore(test-sol/integration): adds TODO comment about config file

* test(test-sol/integration): upgrades to solidity 0.8

This is to allow me to use the `.code` property on deployed smart contracts, which is only supported on Solidity 0.8 and above.

* nit(test-sol/integration): updates constructor visibility

In Solidity versions 0.7.0 and later, constructors are implicitly internal, meaning they can only be called within the contract itself or derived contracts, making the visibility keyword redundant.

* nit(test-sol/integration): updates function state mutability

The state mutability of a function can be restricted to view if the function does not modify the state of the blockchain. A view function promises not to alter the state, which allows the Ethereum Virtual Machine (EVM) to optimize how it handles the function.

The function does not modify any state; it only reads from the registry and logs some information.

* test(test-sol/integration): adds MVP assertion that bytecode matches

* fix(test-sol/integration): use implementation address and deployedBytecode

* refactor(test-sol): adds registry array in `constants.sol`

The goal is to use that in both the migration script (`Migration.s.sol`) and the integration test (`Integration.t.sol`).

* refactor(protocol/migration_sol): uses registry array from `constants.sol`

* fix(test-sol/integration): adds back `Test.sol` import

Mistakenly deleted earlier

* refactor(migrations_sol): simplifies script that runs integration tests

* chore(test-sol/integration): commits WIP on failing bytecode test

* test(test-sol): adds helper to remove metadata from bytecode

Also adds docstring for better readability.

* chore(test-sol/integration): excludes failing contracts from bytecode test

* chor(test-sol/integration): adds code comment for readability

* chore(test-sol/integration): removing unused code comments

* fix(workflows/protocol_tests): excludes integration tests

* fix(workflows/protocol_tests): excludes integration tests more explicitly

* nit(test-sol/constants): improves code comment for context

* fix(workflows/protocol_tests): downgrades foundry version

The latest version has a regression that leads to a failing integration test. But, fixing a previous nightly build fixed the error.

* fix(workflows/protocol_tests): excludes integration tests correctly

* chore(test-sol/utils): reverts `deployCodeTo` edit in `ECDSAHelper`

Because I'm downgrading to a previous foundry version, I have to undo the change to this line made in this PR: #10997

* style(protocol): linting

I always forget to run `yarn prettify` in the top-level directory

* docs(workflows/protocol_tests): adds better code comment

To explain what flags and arguments are used.

* refactor(test-sol/integration): rename variables for better readability

* refactor(test-sol/integration): take constants out of `for`-loop

* style(protocol): linting

Using `yarn prettify` in the top-level directory

* fix(test-sol/integration): add contract name back into `for`-loop

* nit(test-sol/integration): rename variable for better readability

* refactor(migrations_sol/constants): creates new `constants.sol`

* refactor(migrations_sol&test-sol): removes `Utils` import

Adds function in `Integration.t.sol` directly to avoid 0.5.x version problems.

* test(test-sol/integration): bumps solidity version to `>=0.8.7 <0.8.20`

* refactor(test-sol/integration): use `.code` property instead of helper function

Because the script is now on 0.8, we can use the `.code` property to fetch the runtime bytecode of a smart contract deployed on the devchain. Previously, we needed to load the bytecode from storage with inline assembly.

* refactor(foundry): simplifies configs to exclude tests

Functionally this should be the same, but the configs and flags are more readable.
  • Loading branch information
arthurgousset committed Jun 5, 2024
1 parent 0e3303d commit 2908c52
Show file tree
Hide file tree
Showing 8 changed files with 166 additions and 55 deletions.
7 changes: 5 additions & 2 deletions .github/workflows/protocol_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ jobs:

- name: Install Foundry
uses: foundry-rs/foundry-toolchain@v1
with:
version: "nightly-f625d0fa7c51e65b4bf1e8f7931cd1c6e2e285e9"

- name: Install forge dependencies
run: forge install
Expand Down Expand Up @@ -94,8 +96,9 @@ jobs:
- name: Run Everything just in case something was missed
# can't use gas limit because some setUp function use more than the limit
run: forge test -vvv #TODO this should ignore integration tests

# Excludes integration tests, because they require an anvil devchain running on the localhost.
run: forge test -vvv --no-match-contract RegistryIntegrationTest

- name: Generate migrations
if: success() || failure()
run: ./migrations_sol/run_integration_tests_in_anvil.sh
10 changes: 7 additions & 3 deletions packages/protocol/foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,16 @@ remappings = [
'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-contracts/=contracts/',
'@celo-migrations/=migrations_sol/'
]

no_match_contract = "RandomTest"
no_match_test = "skip"
no_match_path = "contracts/common/libraries/test/BLS12Passthrough.sol" # tested from celo-blockain repo

# `BLS12Passthrough.sol` is excluded, because it's tested in the celo-blockain repo as
# described here: https://github.com/celo-org/celo-monorepo/pull/10240
# `Random.sol` is excluded, but I'm not sure why. It was already excluded so I'm leaving it here.
no_match_path = "*test/{BLS12Passthrough.sol,RandomTest.sol}"

fs_permissions = [{ access = "read", path = "./out"}, { access = "read", path = "./migrations_sol/migrationsConfig.json"}, { access = "read", path = "./governanceConstitution.json"}]

Expand Down
40 changes: 7 additions & 33 deletions packages/protocol/migrations_sol/Migration.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ pragma solidity >=0.8.7 <0.8.20;
// Note: This script should not include any cheatcode so that it can run in production

import { Script } from "forge-std-8/Script.sol";

import "forge-std/console.sol";
import "forge-std/StdJson.sol";

Expand Down Expand Up @@ -46,6 +47,8 @@ import "@openzeppelin/contracts8/utils/math/Math.sol";

import "@celo-contracts-8/common/UsingRegistry.sol";

import { Constants } from "@celo-migrations/constants.sol";

contract ForceTx {
// event to trigger so a tx can be processed
event VanillaEvent(string);
Expand All @@ -57,7 +60,7 @@ contract ForceTx {
}
}

contract Migration is Script, UsingRegistry {
contract Migration is Script, UsingRegistry, Constants {
using stdJson for string;

/**
Expand Down Expand Up @@ -941,38 +944,9 @@ contract Migration is Script, UsingRegistry {
(bool)
);
if (!skipTransferOwnership) {
// TODO move this list somewhere else

string[23] memory fixedStringArray = [
"Accounts",
// 'Attestations',
// BlockchainParameters ownership transitioned to governance in a follow-up script.?
"BlockchainParameters",
"DoubleSigningSlasher",
"DowntimeSlasher",
"Election",
"EpochRewards",
"Escrow",
"FederatedAttestations",
"FeeCurrencyWhitelist",
"FeeCurrencyDirectory",
"Freezer",
"FeeHandler",
"GoldToken",
"Governance",
"GovernanceSlasher",
"LockedGold",
"OdisPayments",
"Random",
"Registry",
"SortedOracles",
"UniswapFeeHandlerSeller",
"MentoFeeHandlerSeller",
"Validators"
];

for (uint256 i = 0; i < fixedStringArray.length; i++) {
string memory contractToTransfer = fixedStringArray[i];
// BlockchainParameters ownership transitioned to governance in a follow-up script.?
for (uint256 i = 0; i < contractsInRegistry.length; i++) {
string memory contractToTransfer = contractsInRegistry[i];
console.log("Transfering ownership of: ", contractToTransfer);
IProxy proxy = IProxy(registry.getAddressForStringOrDie(contractToTransfer));
proxy._transferOwnership(governanceAddress);
Expand Down
30 changes: 30 additions & 0 deletions packages/protocol/migrations_sol/constants.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
pragma solidity >=0.8.7 <0.8.20;

contract Constants {
// List of contracts that are expected to be in Registry.sol
string[23] contractsInRegistry = [
"Accounts",
"BlockchainParameters",
"DoubleSigningSlasher",
"DowntimeSlasher",
"Election",
"EpochRewards",
"Escrow",
"FederatedAttestations",
"FeeCurrencyWhitelist",
"FeeCurrencyDirectory",
"Freezer",
"FeeHandler",
"GoldToken",
"Governance",
"GovernanceSlasher",
"LockedGold",
"OdisPayments",
"Random",
"Registry",
"SortedOracles",
"UniswapFeeHandlerSeller",
"MentoFeeHandlerSeller",
"Validators"
];
}
4 changes: 0 additions & 4 deletions packages/protocol/migrations_sol/integration_tests.sh

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
#!/usr/bin/env bash
set -euo pipefail


# generate devchain
# Generate and run devchain
echo "Generating and running devchain before running integration tests..."
source $PWD/migrations_sol/create_and_migrate_anvil_devchain.sh

# Run integration tests
source $PWD/migrations_sol/integration_tests.sh

echo "Running integration tests..."
forge test \
-vvv \
--match-contract RegistryIntegrationTest \
--fork-url http://127.0.0.1:$ANVIL_PORT

# helper kill anvil
# kill $(lsof -i tcp:$ANVIL_PORT | tail -n 1 | awk '{print $2}')
Expand Down
117 changes: 109 additions & 8 deletions packages/protocol/test-sol/integration/Integration.t.sol
Original file line number Diff line number Diff line change
@@ -1,19 +1,120 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.5.13;
pragma solidity >=0.8.7 <0.8.20;

import { Test } from "forge-std-8/Test.sol";
import "forge-std-8/console2.sol";

import { Constants } from "@celo-migrations/constants.sol";

import "celo-foundry/Test.sol";
import "@celo-contracts/common/GoldToken.sol";
// import "@celo-contracts/common/test/MockGoldToken.sol";
import "forge-std/console.sol";
import "@celo-contracts/common/interfaces/IRegistry.sol";
import "@celo-contracts/common/interfaces/IProxy.sol";

contract IntegrationTest is Test {
address constant registryAddress = address(0x000000000000000000000000000000000000ce10);
address account1 = actor("account1");
address account2 = actor("account2");
IRegistry registry = IRegistry(registryAddress);

function setUp() public {}

function test_dummy() public {}
/**
* @notice Removes CBOR encoded metadata from the tail of the deployedBytecode.
* @param data Bytecode including the CBOR encoded tail.
* @return Bytecode without the CBOR encoded metadata.
*/
function removeMetadataFromBytecode(bytes memory data) public pure returns (bytes memory) {
// Ensure the data length is at least enough to contain the length specifier
require(data.length >= 2, "Data too short to contain a valid CBOR length specifier");

// Calculate the length of the CBOR encoded section from the last two bytes
uint16 cborLength = uint16(uint8(data[data.length - 2])) *
256 +
uint16(uint8(data[data.length - 1]));

// Ensure the length is valid (not greater than the data array length minus 2 bytes for the length field)
require(cborLength <= data.length - 2, "Invalid CBOR length");

// Calculate the new length of the data without the CBOR section
uint newLength = data.length - 2 - cborLength;

// Create a new byte array for the result
bytes memory result = new bytes(newLength);

// Copy data from the original byte array to the new one, excluding the CBOR section and its length field
for (uint i = 0; i < newLength; i++) {
result[i] = data[i];
}

return result;
}
}

contract RegistryIntegrationTest is IntegrationTest, Constants {
IProxy proxy;

function test_shouldHaveAddressInRegistry() public view {
for (uint256 i = 0; i < contractsInRegistry.length; i++) {
string memory contractName = contractsInRegistry[i];
address contractAddress = registry.getAddressFor(keccak256(abi.encodePacked(contractName)));
console2.log(contractName, "address in Registry is: ", contractAddress);
assert(contractAddress != address(0));
}
}

function test_shouldHaveCorrectBytecode() public {
// Converting contract names to hashes for comparison
bytes32 hashAccount = keccak256(abi.encodePacked("Accounts"));
bytes32 hashElection = keccak256(abi.encodePacked("Election"));
bytes32 hashEscrow = keccak256(abi.encodePacked("Escrow"));
bytes32 hashFederatedAttestations = keccak256(abi.encodePacked("FederatedAttestations"));
bytes32 hashGovernance = keccak256(abi.encodePacked("Governance"));
bytes32 hashSortedOracles = keccak256(abi.encodePacked("SortedOracles"));
bytes32 hashValidators = keccak256(abi.encodePacked("Validators"));

for (uint256 i = 0; i < contractsInRegistry.length; i++) {
// Read name from list of core contracts
string memory contractName = contractsInRegistry[i];
console2.log("Checking bytecode of:", contractName);

// Skipping test for contracts that depend on linked libraries
// This is a known limitation in Foundry at the moment:
// Source: https://github.com/foundry-rs/foundry/issues/6120
bytes32 hashContractName = keccak256(abi.encodePacked(contractName));
if (
hashContractName != hashAccount &&
hashContractName != hashElection &&
hashContractName != hashEscrow &&
hashContractName != hashFederatedAttestations &&
hashContractName != hashGovernance &&
hashContractName != hashSortedOracles &&
hashContractName != hashValidators
) {
// Get proxy address registered in the Registry
address proxyAddress = registry.getAddressForStringOrDie(contractName);
proxy = IProxy(address(uint160(proxyAddress)));

// Get implementation address
address implementationAddress = proxy._getImplementation();

// Get bytecode from deployed contract
bytes memory actualBytecodeWithMetadataOnDevchain = implementationAddress.code;
bytes memory actualBytecodeOnDevchain = removeMetadataFromBytecode(
actualBytecodeWithMetadataOnDevchain
);

// Get bytecode from build artifacts
bytes memory expectedBytecodeWithMetadataFromArtifacts = vm.getDeployedCode(
string(abi.encodePacked(contractName, ".sol"))
);
bytes memory expectedBytecodeFromArtifacts = removeMetadataFromBytecode(
expectedBytecodeWithMetadataFromArtifacts
);

// Compare the bytecodes
assertEq(
actualBytecodeOnDevchain,
expectedBytecodeFromArtifacts,
"Bytecode does not match"
);
}
}
}
}
2 changes: 1 addition & 1 deletion packages/protocol/test-sol/utils/ECDSAHelper.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ contract ECDSAHelper is Test {
bytes32 _s
) public returns (bytes memory) {
address SECP256K1Address = actor("SECP256K1Address");
deployCodeTo("SECP256K1.sol:SECP256K1", SECP256K1Address);
deployCodeTo("out/SECP256K1.sol/SECP256K1.0.5.17.json", SECP256K1Address);
sECP256K1 = ISECP256K1(SECP256K1Address);

string memory header = "\x19Ethereum Signed Message:\n32";
Expand Down

0 comments on commit 2908c52

Please sign in to comment.