What if we implement the multichain fork tests

[andreivladbrg]

Background

We have had this idea before but abandoned it for two reasons:

1. A long waiting time for the tests to run due to a high number of API calls.
2. Considerably high maintenance costs.

Problem

The above issue existed in the first place because of our current fork test structure:

• We have fuzz fork tests instead of unit/concrete tests.
  • A key factor in the long waiting time for tests to run.
• We are testing the contracts against multiple tokens (and previously, there were multiple versions of Lockup contracts).
  • A key factor in the high maintenance cost.
  • Also contributes to the long test execution time.
• Multiple time warps (with fuzzed params) which would mess up the fork setup and it complicates things.

New Proposal

What if we implement these multichain fork tests in a more minimal way?
What I mean is to simply test a happy flow with a minimal scenario in a unit/concrete setup, for a single token. Specifically, we would test:

1. createLD
2. cancel
3. createLL
4. withdraw
5. createLT
6. renounce

We can choose whether to run assertions or not. The important part is simply calling these functions against a chain fork to ensure they execute correctly.

Another idea, besides the happy flow, is to add admin tests to verify that the setup is correct across all chains.

Motivation

This approach would be useful and relevant for app integration, eliminating the need for manual testing on all chains before going live. And it would skip a step in case of bad deployments.
These tests would be most beneficial between the deployment of new contracts and the app integrating those new contracts.

About Implementation

Important: This will most likely not be part of the current fork tests and won't be run in the CI, as it serves a very specific use case.

We can add a JSON file format to store admin addresses and new deployments, which would be automatically generated using the multichain-deployer tool.
Then, we can simply read the data using Foundry's VM and JSON tools, similar to how we do in BaseScript.

This approach will result in very low maintenance costs. See below the details block:

Example of flow

The JSON format may vary, but let's assume admins.json:

{
    "1": "0x79Fb3e81aAc012c08501f41296CCC145a1E15844",
    "42161": "0xF34E41a6f6Ce5A45559B1D3Ee92E141a3De96376"
}

sablier_lockup.json:

{
    "1": "0x7C01AA3783577E15fD7e272443D44B92d5b21056",
    "42161": "0x467D5Bf8Cfa1a5f99328fBdCb9C751c78934b725"
}

For the above JSON, we would have the following test:

address internal admin;
ISablierLockup internal lockup;

modifier runMultichainForkTest() {
    vm.createSelectFork("mainnet"); // read from foundry.toml
    admin = getAddressForChain("admin.json");
    lockup = ISablierLockup(getAddressForChain("sablier_lockup.json"));
    _; // run the test on the first chain
    vm.createSelectFork("arbitrum"); // read from foundry.toml
    admin = getAddressForChain("admin.json");
    lockup = ISablierLockup(getAddressForChain("sablier_lockup.json"));
    _; // run the test on the second chain  
}

function getAddressForChain(string memory jsonFile) internal view returns (address) {
    string memory json = vm.readFile(jsonFile);
    string memory chainId = string.concat(".", vm.toString(block.chainid));
    return json.readAddress(chainId);
}

// a single unit test run for each chain
function test_Admin() public runMultichainForkTest {
    assertEq(lockup.admin(), admin);
}

Conclusion

I believe this approach would work well in practice, making the tests more robust and secure.

Curious to hear your thoughts, @sablier-labs/solidity.

[smol-ninja]

Very cool idea Andrei. I will think about it later but leaving some questions for you:

• You mentioned that it would eliminate the need for manual testing on all chains before going live. Isn't the purpose of testing to test the UI and not the contracts?

• Given that all EVMs are built on the similar stack, isn't it safe to assume that the contracts would pass the tests if they pass tests on Ethereum?

• Do you have any example from the past where it failed with some functions on a new EVM chain?

I am considering the idea if the value addition by running multichain fork tests would exceed the efforts into building and maintaining it.

PS: nice reference to the Happy Path.

[andreivladbrg]

Isn't the purpose of testing to test the UI and not the contracts?

well, assuming the same logic is applied to all chains in the UI, maybe it’s not necessary to manually test it for all.

Given that all EVMs are built on the similar stack, isn't it safe to assume that the contracts would pass the tests if they pass tests on Ethereum?

yes, they are, but my idea was to verify whether the deployment was made correctly on each chain rather than testing its functionalities, which i agree—we can test only for ethereum. this is why i mentioned in the OP that we can even skip the assertions. my idea with this is simply to test that each function is callable, which would prove that the contract was deployed correctly.

also, the admin check is important now, given that we have fees—if we mistakenly set it incorrectly, we will generate revenue for another address

Do you have any example from the past where it failed with some functions on a new EVM chain?

the perfect example of this is what made me think of the idea. it’s what we were talking about when you deployed the base sepolia contracts with incorrect linked libraries and had to redeploy them without the deterministic script. basically, the functions that interacted with the Helpers and VestingMath libraries weren’t usable

PS: nice reference to the Happy Path

ty

[smol-ninja]

well, assuming the same logic is applied to all chains in the UI, maybe it’s not necessary to manually test it for all

Fair point.

also, the admin check is important now, given that we have fees—if we mistakenly set it incorrectly

Agree and to avoid such mistakes:

• You have added admin map in the Base script to only rely on run()
• Built multichain deployer script (fantastic work on this) to reduce manual intervention during deployment

So imo, the only manual intervention is manual verification if it fails with the script.

incorrect linked libraries and had to redeploy them without the deterministic script

Good point. To solve that, we have already removed manually linkage of libraries from foundry.toml. All of these steps that we have taken have made it more autonomous than ever.

Given the problems you have mentioned, I don't think having multichain fork tests can help if its created based on the happy path. For example, lets say we accidentally deploy a future version of Lockup contract with older libraries; or we deploy it with incorrect MAX_COUNT, the tests will continue pass which might not help us detect a faulty deployment. So I am not sure if it will be of great help.

---

What if instead of building multichain fork test, lets identify the places where mistakes can be made and automate them as well. For example,

• Remove all instances of run(address) function where admin is required. We currently have both run() and run(address) implemented.
• To avoid accidentally deploying deterministically, lets make a rule to not update version in package.json until the npm release.
• I don't think libraries linking issue will be a problem anymore.
• Lets move Base.s.sol into sol-utils repo. I just checked its identical across all the repos except that Lockup has some logic related to segment count which can also be moved to shared script, and will be ignored by Flow and Airdrops.

Thoughts?