feat(ct): rm DeploymentSummary.sol files

[jsvisa]

Description

close #12270

Tests

Additional context

Metadata

[jsvisa]

@smartcontracts PTAL

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.14%. Comparing base (fb62380) to head (436c336).
Report is 8 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #12388      +/-   ##
===========================================
- Coverage    64.32%   64.14%   -0.19%     
===========================================
  Files           52       52              
  Lines         4348     4348              
===========================================
- Hits          2797     2789       -8     
- Misses        1376     1385       +9     
+ Partials       175      174       -1     

Flag	Coverage Δ
cannon-go-tests	64.14% <ø> (-0.19%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1 file with indirect coverage changes

[smartcontracts]

@jsvisa appreciate you taking this one on! So the main issue with this current approach is that compilation will fail unless you generate the summaries locally which isn't ideal. I would prefer to have a dummy summary or something in place that would make it possible to compile the contracts without actually generating the full summaries. Second issue is that generating the summaries in CI takes a long time so we'd prefer not to do that since it will slow CI down quite a bit.

Ideally we don't need to change CI at all - just replace the summaries here with interfaces or something, not really sure, but basically just something to avoid needing to store these massive files in the repository.

[jsvisa]

@smartcontracts thanks for the guides, but I'm not familiar with kontrol, I'll try to find a proper way to generate those files with a simple way, without the heavy kontrol-summary-full.

[smartcontracts]

@smartcontracts thanks for the guides, but I'm not familiar with kontrol, I'll try to find a proper way to generate those files with a simple way, without the heavy kontrol-summary-full.

I think you could make it work as follows.

For DeploymentSummary and DeploymentSumamryFaultProofs you use something like this:

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.13;

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

import { DeploymentSummaryCode } from "./DeploymentSummaryCode.sol";

contract DeploymentSummary is DeploymentSummaryCode {
    // Cheat code address, 0x7109709ECfa91a80626fF3989D68f67F5b1DD12D
    address private constant VM_ADDRESS = address(uint160(uint256(keccak256("hevm cheat code"))));
    Vm private constant vm = Vm(VM_ADDRESS);

    // Addresses required for compilation to work.
    address l1CrossDomainMessengerProxyAddress;
    address superchainConfigProxyAddress;
    address l1ERC721BridgeProxyAddress;
    address l1StandardBridgeProxyAddress;
    address optimismPortalProxyAddress;

    function recreateDeployment() public {
        // Will be generated by Kontrol
        revert("DeploymentSummary: must generate with Kontrol");
    }
}

For DeploymentSummaryCode and DeploymentSumamryFaultProofsCode you use something like this:

// SPDX-License-Identifier: MIT
// This file was autogenerated by running `kontrol load-state-diff`. Do not edit this file manually.

pragma solidity ^0.8.13;

contract DeploymentSummaryCode {
    // Will be generated by Kontrol
}

[jsvisa]

I think you could make it work as follows.

May I ask is that means for the ci or other non-kontrol related tasks, we just echo those dummy code into the four files before forge xxx, instead of the kontrol regenerating process. And for the kontrol tasks, we need to call kontrol-summary-full to regenerate it.

btw, I was also thinking of use remappings to distinguish the dummy or real one, eg:

[profile.ci]
fuzz = { runs = 512 }
reamppings = [
  'test/kontrol/proofs/utils/DeploymentSummary.sol=test/kontrol/proofs/utils/DummySummary.sol',
  'test/kontrol/proofs/utils/DeploymentSummaryFaultProofs.sol=test/kontrol/proofs/utils/DummySummaryFaultProofs.sol',
]

but seems for the local file remapping is not working.

[jsvisa]

@smartcontracts please take another look, thanks.

[jsvisa]

/ping

[smartcontracts]

Hi, sorry for the delay here. I think it would be much easier to commit the dummy files rather than generating them with a script. If we have to generate them with a script then it’s an extra thing for someone to think about every time they want to write a new task or job that compiles the contracts. If we commit the files then it will “just work” while getting rid of the existing files.

[jsvisa]

@smartcontracts I see I got your points, so we can directly replace the 4 files with the dummy files. But then when we're running the kontrol tests, it will rewrite those dummy files, so run git check-out to drop the changes

[smartcontracts]

@smartcontracts I see I got your points, so we can directly replace the 4 files with the dummy files. But then when we're running the kontrol tests, it will rewrite those dummy files, so run git check-out to drop the changes

Yep this is fine. We can make git ignore the file so that it won't be tracked.

[jsvisa]

Yep this is fine. We can make git ignore the file so that it won't be tracked.

But if git ignored those files, then they will not exist in the git worktree, I'm afraid we can't pass the tests.

[smartcontracts]

But if git ignored those files, then they will not exist in the git worktree, I'm afraid we can't pass the tests.

We can use git update-index --assume-unchanged <file> to have it exist in the tree and assume that the user won't change the file locally (so it won't be committed if someone uses git add --all). Since it's technically fine if the user does end up re-generating the file locally it's not a big deal. We can have just clean restore the file to its original state.

[smartcontracts]

Heyo @jsvisa was this closed on purpose?

[jsvisa]

sorry seems by mistake

[smartcontracts]

Thank you!