feat: Replace deployImplementations with call to DeployImplementations.run() #12260

maurelian · 2024-10-02T18:32:14Z

feat: Rename setupSuperchain to deploySuperchain

feat: Replace deployImplementations with call to DeployImplementations.run()

maurelian · 2024-10-02T18:32:28Z

feat: Replace deployImplementations with call to DeployImplementations.run() #12260 : 2 dependent PRs (#12261 , #12287 ) 👈
feat: deletes unused FPACOPS scripts #12283
develop

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @maurelian and the rest of your teammates on Graphite

maurelian · 2024-10-02T20:13:17Z

Merge activity

Oct 2, 4:13 PM EDT: Graphite disabled "merge when ready" on this PR due to: a merge conflict with the target branch; resolve the conflict and try again..

semgrep-app · 2024-10-02T20:49:41Z

Semgrep found 2 sol-style-require-reason findings:

packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol
- L441 - Triage
- L442 - Triage

require() must include a reason string

_{Ignore this finding from sol-style-require-reason.}

semgrep-app · 2024-10-03T15:43:41Z

Semgrep found 138 sol-style-require-reason findings:

packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol
- L79 - Triage
- L80 - Triage
- L81 - Triage
- L82 - Triage
- L83 - Triage
- L84 - Triage
- L85 - Triage
- L88 - Triage
- L89 - Triage
- L90 - Triage
- 128 more - Triage

require() must include a reason string

_{Ignore this finding from sol-style-require-reason.}

packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol

packages/contracts-bedrock/scripts/deploy/DeployOwnership.s.sol

maurelian · 2024-10-03T17:12:43Z

packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol

@@ -215,6 +243,15 @@ library ChainAssertions {
        require(factory.owner() == _expectedOwner, "CHECK-DG-20");
    }

+    /// @notice Asserts that the PreimageOracle is setup correctly
+    function checkPreimageOracle(IPreimageOracle _oracle, DeployConfig _cfg) internal view {


This is a singleton, and is not currently exposed in the _proxiesUnstrict() getter, so unlike other functions in this file I'm just passing in the contract rather than a struct with all of the contracts.

I may add a PR on top of this to clean up those getters a bit.

maurelian · 2024-10-03T17:17:46Z

packages/contracts-bedrock/scripts/deploy/Deploy.s.sol

@@ -640,61 +649,6 @@ contract Deploy is Deployer {
        ChainAssertions.checkOptimismPortal({ _contracts: contracts, _cfg: cfg, _isProxy: false });
    }


We still need this for the legacy system tests which OPCM does not support.

maurelian · 2024-10-03T17:27:04Z

packages/contracts-bedrock/scripts/deploy/Deploy.s.sol

-        } else {
-            deployImplementations();
-        }
+        deployImplementations({ _isInterop: cfg.useInterop() });


Passing in this arg is somewhat overkill, as cfg is already available in the deployImplementations function, but my goal is to have all of the cfg.use_() flags visible here rather than buried in function calls.

maurelian · 2024-10-03T17:27:59Z

packages/contracts-bedrock/scripts/deploy/Deploy.s.sol

@@ -311,7 +333,7 @@ contract Deploy is Deployer {
        dsi.set(dsi.recommendedProtocolVersion.selector, ProtocolVersion.wrap(cfg.recommendedProtocolVersion()));

        // Run the deployment script.
-        deploySuperchain.run(dsi, dso);
+        ds.run(dsi, dso);


When I renamed setupSuperchain to deploySuperchain for consistency, I had a shadowed declaration warning.

maurelian · 2024-10-03T17:29:10Z

packages/contracts-bedrock/scripts/deploy/Deploy.s.sol

+            ChainAssertions.checkSystemConfigInterop({ _contracts: contracts, _cfg: cfg, _isProxy: false });
+        } else {
+            ChainAssertions.checkSystemConfig({ _contracts: contracts, _cfg: cfg, _isProxy: false });
+        }


For every deploy_() function which was deleted below, I add (and if necessary created) a corresponding ChainAssertions check here.

…s.run()

…ntations

…deplyImpl

Preimage Oracle DelayedWeth DGF A ChainAssertion was also added for the PreimageOracle

maurelian changed the title ~~feat: Rename setupSuperchain to deploySuperchain~~ feat: Replace deployImplementations with call to DeployImplementations.run() Oct 2, 2024

maurelian mentioned this pull request Oct 2, 2024

feat: delete FPACOPs and unused deploy functions #12261

Merged

maurelian marked this pull request as ready for review October 2, 2024 19:05

maurelian requested a review from a team as a code owner October 2, 2024 19:05

maurelian requested a review from AmadiMichael October 2, 2024 19:05

maurelian assigned mds1 Oct 2, 2024

maurelian marked this pull request as draft October 2, 2024 20:18

maurelian force-pushed the opcm/tests/deploy-impls branch from 5b4da04 to 2426d15 Compare October 2, 2024 20:48

maurelian changed the base branch from develop to opcm/init-superchain-impls October 2, 2024 20:48

maurelian mentioned this pull request Oct 2, 2024

feat: Add ChainAssertions for deploySuperchain contracts #12271

Merged

Base automatically changed from opcm/init-superchain-impls to develop October 2, 2024 21:52

maurelian force-pushed the opcm/tests/deploy-impls branch from 434e651 to 1430998 Compare October 3, 2024 15:54

maurelian changed the base branch from develop to chain-assertions/ux October 3, 2024 15:54

maurelian mentioned this pull request Oct 3, 2024

feat: Add reasons to checkOptimismPortal2 #12282

Open

semgrep-app bot reviewed Oct 3, 2024

View reviewed changes

packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol Outdated Show resolved Hide resolved

semgrep-app bot reviewed Oct 3, 2024

View reviewed changes

packages/contracts-bedrock/scripts/deploy/ChainAssertions.sol Outdated Show resolved Hide resolved

maurelian force-pushed the opcm/tests/deploy-impls branch from 1430998 to 8f88513 Compare October 3, 2024 16:21

maurelian changed the base branch from chain-assertions/ux to delete-fpacops October 3, 2024 16:21

maurelian mentioned this pull request Oct 3, 2024

feat: deletes unused FPACOPS scripts #12283

Merged

maurelian force-pushed the opcm/tests/deploy-impls branch from 8f88513 to 2d47ce4 Compare October 3, 2024 17:09

semgrep-app bot reviewed Oct 3, 2024

View reviewed changes

packages/contracts-bedrock/scripts/deploy/DeployOwnership.s.sol Show resolved Hide resolved

semgrep-app bot reviewed Oct 3, 2024

View reviewed changes

packages/contracts-bedrock/scripts/deploy/DeployOwnership.s.sol Show resolved Hide resolved

maurelian commented Oct 3, 2024

View reviewed changes

Base automatically changed from delete-fpacops to develop October 3, 2024 17:20

maurelian commented Oct 3, 2024

View reviewed changes

maurelian force-pushed the opcm/tests/deploy-impls branch from fab7669 to 7f126ff Compare October 3, 2024 17:32

maurelian added 14 commits October 3, 2024 13:49

feat: Rename setupSuperchain to deploySuperchain

3f61e55

feat: Replace deployImplementations with call to DeployImplementation…

cc6c69a

…s.run()

feat: Add implsUnstrict() to get all impl addresses

96b7e81

feat: Remove deployL1xDM and move its ChainAssertion to deployImpleme…

095cda9

…ntations

feat: Remove deployOptimismPortal2 and move its assertion to deplyImpls

ce8de01

feat: Remove deployOptimismMintableFactory and move its assertion to …

3295a07

…deplyImpl

fix: test file name

ecd6d31

fix: Incorrect implementation contract name in _implUnstrict()

e3af68d

feat: Add checkSystemConfigInterop

facb354

feat: Move unused deploySuperchainConfig to DeployOwnership

2a2d6b2

feat: Delete deployOptimismPortalInterop

89cd8e6

feat: Delete unused FP deploy functions

b86e5b0

Preimage Oracle DelayedWeth DGF A ChainAssertion was also added for the PreimageOracle

feat: Delete deploySystemConfig

e182d0d

feat: remove superfluous Unstrict suffix from contract getters

37dd9b3

maurelian force-pushed the opcm/tests/deploy-impls branch from 7f126ff to 37dd9b3 Compare October 3, 2024 17:49

maurelian marked this pull request as ready for review October 3, 2024 17:51

maurelian mentioned this pull request Oct 3, 2024

feat: Add OPCM to deployImplementations #12287

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Replace deployImplementations with call to DeployImplementations.run() #12260

feat: Replace deployImplementations with call to DeployImplementations.run() #12260

maurelian commented Oct 2, 2024

maurelian commented Oct 2, 2024 •

edited

Loading

maurelian commented Oct 2, 2024

semgrep-app bot commented Oct 2, 2024

semgrep-app bot commented Oct 3, 2024

maurelian Oct 3, 2024

maurelian Oct 3, 2024

maurelian Oct 3, 2024

maurelian Oct 3, 2024

maurelian Oct 3, 2024

		@@ -640,61 +649,6 @@ contract Deploy is Deployer {
		ChainAssertions.checkOptimismPortal({ _contracts: contracts, _cfg: cfg, _isProxy: false });
		}

feat: Replace deployImplementations with call to DeployImplementations.run() #12260

Are you sure you want to change the base?

feat: Replace deployImplementations with call to DeployImplementations.run() #12260

Conversation

maurelian commented Oct 2, 2024

maurelian commented Oct 2, 2024 • edited Loading

maurelian commented Oct 2, 2024

Merge activity

semgrep-app bot commented Oct 2, 2024

semgrep-app bot commented Oct 3, 2024

maurelian Oct 3, 2024

Choose a reason for hiding this comment

maurelian Oct 3, 2024

Choose a reason for hiding this comment

maurelian Oct 3, 2024

Choose a reason for hiding this comment

maurelian Oct 3, 2024

Choose a reason for hiding this comment

maurelian Oct 3, 2024

Choose a reason for hiding this comment

maurelian commented Oct 2, 2024 •

edited

Loading