diff --git a/src/lib/LibRainDeploy.sol b/src/lib/LibRainDeploy.sol index d63e64f..962a2cb 100644 --- a/src/lib/LibRainDeploy.sol +++ b/src/lib/LibRainDeploy.sol @@ -184,51 +184,18 @@ library LibRainDeploy { return networks; } - /// Checks that the Zoltu factory has the expected codehash and all - /// dependencies have code on each network. Records each dependency's - /// codehash in the provided mapping. - /// @param vm The Vm instance to use for forking. - /// @param networks The list of network names to check. - /// @param dependencies The addresses that must have code on each network. - /// @param depCodeHashes Storage mapping to record dependency codehashes. - function checkDependencies( - Vm vm, - string[] memory networks, - address[] memory dependencies, - mapping(string => mapping(address => bytes32)) storage depCodeHashes - ) internal { - if (networks.length == 0) { - revert NoNetworks(); - } - for (uint256 i = 0; i < networks.length; i++) { - // Capture return value to suppress slither unused-return warning. - uint256 forkId = vm.createSelectFork(networks[i]); - (forkId); - console2.log("Block number:", block.number); - console2.log("Checking dependencies on network:", networks[i]); - - console2.log(" - Zoltu Factory:", ZOLTU_FACTORY); - // Zoltu factory must exist with the expected codehash. - if (ZOLTU_FACTORY.code.length == 0) { - revert MissingDependency(networks[i], ZOLTU_FACTORY); - } - if (ZOLTU_FACTORY.codehash != ZOLTU_FACTORY_CODEHASH) { - revert DependencyChanged(networks[i], ZOLTU_FACTORY, ZOLTU_FACTORY_CODEHASH, ZOLTU_FACTORY.codehash); - } - - for (uint256 j = 0; j < dependencies.length; j++) { - console2.log(" - Dependency:", dependencies[j]); - if (dependencies[j].code.length == 0) { - revert MissingDependency(networks[i], dependencies[j]); - } - depCodeHashes[networks[i]][dependencies[j]] = dependencies[j].codehash; - } - } - } - - /// Verifies that dependencies have not changed since the check phase, - /// then deploys to each network via the Zoltu factory. If code already - /// exists at `expectedAddress`, deployment is skipped for that network. + /// Deploys the given creation code to each network via the Zoltu factory. + /// For each network it forks once, verifies the Zoltu factory and every + /// dependency have code (the factory codehash must also match), then + /// broadcasts the deploy on that same fork. If code already exists at + /// `expectedAddress`, deployment is skipped for that network. Checking and + /// deploying on a single fork reads each dependency exactly once, so a + /// transient RPC inconsistency on a redundant second read cannot report an + /// already-deployed dependency as missing and abort an otherwise-valid + /// deploy. Each network is handled independently: the Zoltu deploy is + /// idempotent (an existing contract is skipped), so a failure on one network + /// leaves the others intact and the script can simply be re-run, which is why + /// no separate all-network pre-flight is needed. /// @param vm The Vm instance to use for forking and broadcasting. /// @param networks The list of network names to deploy to. /// @param deployer The deployer address. @@ -236,8 +203,7 @@ library LibRainDeploy { /// @param contractPath The contract path for verification commands. /// @param expectedAddress The expected deterministic address. /// @param expectedCodeHash The expected code hash of the deployed contract. - /// @param dependencies The dependency addresses to re-verify. - /// @param depCodeHashes Storage mapping of recorded dependency codehashes. + /// @param dependencies The addresses that must have code on each network. /// @return deployedAddress The deployed contract address. function deployToNetworks( Vm vm, @@ -247,60 +213,57 @@ library LibRainDeploy { string memory contractPath, address expectedAddress, bytes32 expectedCodeHash, - address[] memory dependencies, - mapping(string => mapping(address => bytes32)) storage depCodeHashes + address[] memory dependencies ) internal returns (address deployedAddress) { if (networks.length == 0) { revert NoNetworks(); } for (uint256 i = 0; i < networks.length; i++) { - console2.log("Deploying to network:", networks[i]); - // Capture return value to suppress slither unused-return warning. + // createSelectFork returns a fork id that is not needed here; bind + // and reference it so the unused-return lint stays satisfied. uint256 forkId = vm.createSelectFork(networks[i]); (forkId); + console2.log("Deploying to network:", networks[i]); console2.log("Block number:", block.number); - // Re-verify Zoltu factory exists. - if (ZOLTU_FACTORY.code.length == 0) { - revert MissingDependency(networks[i], ZOLTU_FACTORY); - } - // Re-verify Zoltu factory codehash. - if (ZOLTU_FACTORY.codehash != ZOLTU_FACTORY_CODEHASH) { - revert DependencyChanged(networks[i], ZOLTU_FACTORY, ZOLTU_FACTORY_CODEHASH, ZOLTU_FACTORY.codehash); - } - - // Re-verify dependencies have not changed since the check phase. - for (uint256 j = 0; j < dependencies.length; j++) { - if (dependencies[j].code.length == 0) { - revert MissingDependency(networks[i], dependencies[j]); + if (expectedAddress.code.length == 0) { + // Nothing is deployed here yet, so the Zoltu factory and every + // dependency must be present before broadcasting the deploy. + console2.log(" - Zoltu Factory:", ZOLTU_FACTORY); + if (ZOLTU_FACTORY.code.length == 0) { + revert MissingDependency(networks[i], ZOLTU_FACTORY); } - if (dependencies[j].codehash != depCodeHashes[networks[i]][dependencies[j]]) { - revert DependencyChanged( - networks[i], - dependencies[j], - depCodeHashes[networks[i]][dependencies[j]], - dependencies[j].codehash - ); + if (ZOLTU_FACTORY.codehash != ZOLTU_FACTORY_CODEHASH) { + revert DependencyChanged(networks[i], ZOLTU_FACTORY, ZOLTU_FACTORY_CODEHASH, ZOLTU_FACTORY.codehash); + } + for (uint256 j = 0; j < dependencies.length; j++) { + console2.log(" - Dependency:", dependencies[j]); + if (dependencies[j].code.length == 0) { + revert MissingDependency(networks[i], dependencies[j]); + } } - } - vm.startBroadcast(deployer); - if (expectedAddress.code.length == 0) { console2.log(" - Deploying via Zoltu"); + vm.startBroadcast(deployer); deployedAddress = deployZoltu(creationCode); + vm.stopBroadcast(); + if (deployedAddress != expectedAddress) { + revert UnexpectedDeployedAddress(expectedAddress, deployedAddress); + } } else { + // Already deployed on this network. The Zoltu deploy is + // idempotent, so skip it without checking dependencies: an + // already-deployed network needs neither the Zoltu factory nor + // its dependencies present to remain deployed, which keeps a + // rerun a clean no-op here. console2.log(" - Code already exists at expected address, skipping deployment"); deployedAddress = expectedAddress; } console2.log(" - Final Address:", deployedAddress); - if (deployedAddress != expectedAddress) { - revert UnexpectedDeployedAddress(expectedAddress, deployedAddress); - } console2.log(" - Verifying code hash"); if (expectedCodeHash != deployedAddress.codehash) { revert UnexpectedDeployedCodeHash(expectedCodeHash, deployedAddress.codehash); } - vm.stopBroadcast(); console2.log("manual verification command:"); console2.log( @@ -322,7 +285,6 @@ library LibRainDeploy { /// @param expectedAddress The expected deterministic address. /// @param expectedCodeHash The expected code hash of the deployed contract. /// @param dependencies The dependency addresses to check. - /// @param depCodeHashes Storage mapping to record dependency codehashes. /// @return deployedAddress The address of the deployed contract. function deployAndBroadcast( Vm vm, @@ -332,8 +294,7 @@ library LibRainDeploy { string memory contractPath, address expectedAddress, bytes32 expectedCodeHash, - address[] memory dependencies, - mapping(string => mapping(address => bytes32)) storage depCodeHashes + address[] memory dependencies ) internal returns (address deployedAddress) { if (networks.length == 0) { revert NoNetworks(); @@ -342,17 +303,8 @@ library LibRainDeploy { console2.log("Deploying from address:", deployer); - checkDependencies(vm, networks, dependencies, depCodeHashes); deployedAddress = deployToNetworks( - vm, - networks, - deployer, - creationCode, - contractPath, - expectedAddress, - expectedCodeHash, - dependencies, - depCodeHashes + vm, networks, deployer, creationCode, contractPath, expectedAddress, expectedCodeHash, dependencies ); } } diff --git a/test/src/lib/LibRainDeploy.t.sol b/test/src/lib/LibRainDeploy.t.sol index 8a39f30..c08ec54 100644 --- a/test/src/lib/LibRainDeploy.t.sol +++ b/test/src/lib/LibRainDeploy.t.sol @@ -4,30 +4,14 @@ pragma solidity ^0.8.25; import {Test} from "forge-std-1.16.1/src/Test.sol"; import {LibRainDeploy} from "../../../src/lib/LibRainDeploy.sol"; - -/// @title MockDeployable -/// Minimal contract used as a deployment target for Zoltu factory tests. -contract MockDeployable { - /// @notice Placeholder value to ensure the contract has non-trivial code. - uint256 public value = 42; -} - -/// @title MockReverter -/// Contract whose constructor always reverts, used to test DeployFailed with -/// success=false. -contract MockReverter { - constructor() { - revert(); - } -} +import {MockDeployable} from "./MockDeployable.sol"; +import {MockReverter} from "./MockReverter.sol"; /// @title LibRainDeployTest /// Tests for `LibRainDeploy`. External wrappers are used for library functions /// that need `vm.expectRevert` at the correct call depth, and for functions /// that require a storage mapping reference. contract LibRainDeployTest is Test { - mapping(string => mapping(address => bytes32)) internal sDepCodeHashes; - /// External wrapper for `isStartBlock` so that it can be called /// externally in tests. /// @param target The contract address to check. @@ -215,8 +199,7 @@ contract LibRainDeployTest is Test { contractPath, expectedAddress, expectedCodeHash, - dependencies, - sDepCodeHashes + dependencies ); } @@ -228,15 +211,6 @@ contract LibRainDeployTest is Test { this.externalDeployAndBroadcast(networks, 1, hex"", "", address(0), bytes32(0), dependencies); } - /// `checkDependencies` MUST revert with `NoNetworks` when given an empty - /// networks array. - function testCheckDependenciesNoNetworksReverts() external { - string[] memory networks = new string[](0); - address[] memory dependencies = new address[](0); - vm.expectRevert(abi.encodeWithSelector(LibRainDeploy.NoNetworks.selector)); - this.externalCheckDependencies(networks, dependencies); - } - /// `deployToNetworks` MUST revert with `NoNetworks` when given an empty /// networks array. function testDeployToNetworksNoNetworksReverts() external { @@ -246,12 +220,25 @@ contract LibRainDeployTest is Test { this.externalDeployToNetworks(networks, address(this), hex"", "", address(0), bytes32(0), dependencies); } - /// External wrapper for `checkDependencies` so that `vm.expectRevert` - /// works at the correct call depth. - /// @param networks The list of network names to check. - /// @param dependencies The dependency addresses to check. - function externalCheckDependencies(string[] memory networks, address[] memory dependencies) external { - LibRainDeploy.checkDependencies(vm, networks, dependencies, sDepCodeHashes); + /// `deployToNetworks` MUST deploy to every network in the list, forking each + /// independently. Two networks that start without the target both end up with + /// the deterministic contract, and the call returns its address. + function testDeployToNetworksMultipleNetworks() external { + string[] memory networks = new string[](2); + networks[0] = LibRainDeploy.BASE; + networks[1] = LibRainDeploy.ARBITRUM_ONE; + address[] memory dependencies = new address[](0); + + address result = this.externalDeployToNetworks( + networks, + address(this), + type(MockDeployable).creationCode, + "test/src/lib/MockDeployable.sol:MockDeployable", + 0xC24016f209562fc151e5Ab7F88694ED5775feb36, + 0xc1a263a0b50505687a5140c7964ec5c947329e7d03410306fee68cc3620c5483, + dependencies + ); + assertEq(result, 0xC24016f209562fc151e5Ab7F88694ED5775feb36); } /// External wrapper for `deployToNetworks` so that `vm.expectRevert` @@ -262,7 +249,7 @@ contract LibRainDeployTest is Test { /// @param contractPath The contract path for verification commands. /// @param expectedAddress The expected deterministic address. /// @param expectedCodeHash The expected code hash of the deployed contract. - /// @param dependencies The dependency addresses to re-verify. + /// @param dependencies The addresses that must have code on each network. /// @return deployedAddress The deployed contract address. function externalDeployToNetworks( string[] memory networks, @@ -274,15 +261,7 @@ contract LibRainDeployTest is Test { address[] memory dependencies ) external returns (address deployedAddress) { deployedAddress = LibRainDeploy.deployToNetworks( - vm, - networks, - deployer, - creationCode, - contractPath, - expectedAddress, - expectedCodeHash, - dependencies, - sDepCodeHashes + vm, networks, deployer, creationCode, contractPath, expectedAddress, expectedCodeHash, dependencies ); } @@ -321,10 +300,9 @@ contract LibRainDeployTest is Test { /// `deployToNetworks` MUST revert with `UnexpectedDeployedAddress` when the /// deployed address does not match the expected address. function testUnexpectedDeployedAddressReverts() external { - vm.createSelectFork(LibRainDeploy.ARBITRUM_ONE); - address[] memory dependencies = new address[](0); string[] memory networks = new string[](1); networks[0] = LibRainDeploy.ARBITRUM_ONE; + address[] memory dependencies = new address[](0); vm.expectRevert( abi.encodeWithSelector( LibRainDeploy.UnexpectedDeployedAddress.selector, @@ -340,10 +318,9 @@ contract LibRainDeployTest is Test { /// `deployToNetworks` MUST revert with `UnexpectedDeployedCodeHash` when the /// deployed code hash does not match the expected code hash. function testUnexpectedDeployedCodeHashReverts() external { - vm.createSelectFork(LibRainDeploy.ARBITRUM_ONE); - address[] memory dependencies = new address[](0); string[] memory networks = new string[](1); networks[0] = LibRainDeploy.ARBITRUM_ONE; + address[] memory dependencies = new address[](0); address expectedAddress = 0xC24016f209562fc151e5Ab7F88694ED5775feb36; bytes32 wrongCodeHash = bytes32(uint256(1)); vm.expectRevert( @@ -368,7 +345,7 @@ contract LibRainDeployTest is Test { networks, 1, type(MockDeployable).creationCode, - "test/src/lib/LibRainDeploy.t.sol:MockDeployable", + "test/src/lib/MockDeployable.sol:MockDeployable", 0xC24016f209562fc151e5Ab7F88694ED5775feb36, 0xc1a263a0b50505687a5140c7964ec5c947329e7d03410306fee68cc3620c5483, dependencies @@ -394,7 +371,7 @@ contract LibRainDeployTest is Test { networks, address(this), type(MockDeployable).creationCode, - "test/src/lib/LibRainDeploy.t.sol:MockDeployable", + "test/src/lib/MockDeployable.sol:MockDeployable", 0xC24016f209562fc151e5Ab7F88694ED5775feb36, 0xc1a263a0b50505687a5140c7964ec5c947329e7d03410306fee68cc3620c5483, dependencies @@ -402,82 +379,41 @@ contract LibRainDeployTest is Test { assertEq(result, 0xC24016f209562fc151e5Ab7F88694ED5775feb36); } - /// `checkDependencies` MUST record the codehash of each dependency in the - /// storage mapping after verifying it exists. - function testCheckDependenciesRecordsCodehash() external { - string[] memory networks = new string[](1); - networks[0] = LibRainDeploy.ARBITRUM_ONE; - address[] memory dependencies = new address[](1); - dependencies[0] = LibRainDeploy.ZOLTU_FACTORY; - - this.externalCheckDependencies(networks, dependencies); - - assertTrue(sDepCodeHashes[LibRainDeploy.ARBITRUM_ONE][LibRainDeploy.ZOLTU_FACTORY] != bytes32(0)); - } - - /// `checkDependencies` MUST revert with `MissingDependency` when the - /// Zoltu factory has no code on the network. - function testCheckDependenciesMissingZoltuFactoryReverts() external { - vm.createSelectFork(LibRainDeploy.ARBITRUM_ONE); - vm.makePersistent(LibRainDeploy.ZOLTU_FACTORY); - vm.etch(LibRainDeploy.ZOLTU_FACTORY, hex""); - - string[] memory networks = new string[](1); - networks[0] = LibRainDeploy.ARBITRUM_ONE; - address[] memory dependencies = new address[](0); - - vm.expectRevert( - abi.encodeWithSelector( - LibRainDeploy.MissingDependency.selector, LibRainDeploy.ARBITRUM_ONE, LibRainDeploy.ZOLTU_FACTORY - ) - ); - this.externalCheckDependencies(networks, dependencies); - } + /// `deployToNetworks` MUST skip an already-deployed network WITHOUT checking + /// its dependencies. A rerun on a network that no longer needs deployment is + /// a clean no-op even when a dependency is now missing, because the + /// dependency check only guards the deploy path. + function testDeployToNetworksSkipsAlreadyDeployedWithMissingDependency() external { + vm.makePersistent(address(this)); - /// `checkDependencies` MUST revert with `DependencyChanged` when the - /// Zoltu factory exists but has a wrong codehash. - function testCheckDependenciesZoltuFactoryCodehashChangedReverts() external { vm.createSelectFork(LibRainDeploy.ARBITRUM_ONE); - vm.makePersistent(LibRainDeploy.ZOLTU_FACTORY); - vm.etch(LibRainDeploy.ZOLTU_FACTORY, hex"00"); - - string[] memory networks = new string[](1); - networks[0] = LibRainDeploy.ARBITRUM_ONE; - address[] memory dependencies = new address[](0); - - vm.expectRevert( - abi.encodeWithSelector( - LibRainDeploy.DependencyChanged.selector, - LibRainDeploy.ARBITRUM_ONE, - LibRainDeploy.ZOLTU_FACTORY, - LibRainDeploy.ZOLTU_FACTORY_CODEHASH, - keccak256(hex"00") - ) - ); - this.externalCheckDependencies(networks, dependencies); - } + address deployed = this.externalDeployZoltu(type(MockDeployable).creationCode); + assertEq(deployed, 0xC24016f209562fc151e5Ab7F88694ED5775feb36); + vm.makePersistent(deployed); - /// `checkDependencies` MUST revert with `MissingDependency` when a - /// dependency has no code on the network. - function testMissingDependencyReverts() external { string[] memory networks = new string[](1); networks[0] = LibRainDeploy.ARBITRUM_ONE; + // A dependency with no code: it would revert MissingDependency on the + // deploy path, but the target is already deployed so it is never checked. address[] memory dependencies = new address[](1); dependencies[0] = address(0xdead); - vm.expectRevert( - abi.encodeWithSelector( - LibRainDeploy.MissingDependency.selector, LibRainDeploy.ARBITRUM_ONE, address(0xdead) - ) + address result = this.externalDeployToNetworks( + networks, + address(this), + type(MockDeployable).creationCode, + "test/src/lib/MockDeployable.sol:MockDeployable", + 0xC24016f209562fc151e5Ab7F88694ED5775feb36, + 0xc1a263a0b50505687a5140c7964ec5c947329e7d03410306fee68cc3620c5483, + dependencies ); - this.externalCheckDependencies(networks, dependencies); + assertEq(result, 0xC24016f209562fc151e5Ab7F88694ED5775feb36); } - /// `deployToNetworks` MUST revert with `MissingDependency` when the - /// Zoltu factory has been removed since the check phase. - function testDeployToNetworksZoltuFactoryMissingReverts() external { + /// `deployToNetworks` MUST revert with `MissingDependency` when the Zoltu + /// factory has no code on the network. + function testDeployToNetworksMissingZoltuFactoryReverts() external { vm.makePersistent(address(this)); - vm.createSelectFork(LibRainDeploy.ARBITRUM_ONE); vm.makePersistent(LibRainDeploy.ZOLTU_FACTORY); vm.etch(LibRainDeploy.ZOLTU_FACTORY, hex""); @@ -494,11 +430,10 @@ contract LibRainDeployTest is Test { this.externalDeployToNetworks(networks, address(this), hex"", "", address(0), bytes32(0), dependencies); } - /// `deployToNetworks` MUST revert with `DependencyChanged` when the - /// Zoltu factory has a wrong codehash at deploy time. + /// `deployToNetworks` MUST revert with `DependencyChanged` when the Zoltu + /// factory exists but has a wrong codehash. function testDeployToNetworksZoltuFactoryCodehashChangedReverts() external { vm.makePersistent(address(this)); - vm.createSelectFork(LibRainDeploy.ARBITRUM_ONE); vm.makePersistent(LibRainDeploy.ZOLTU_FACTORY); vm.etch(LibRainDeploy.ZOLTU_FACTORY, hex"00"); @@ -519,48 +454,14 @@ contract LibRainDeployTest is Test { this.externalDeployToNetworks(networks, address(this), hex"", "", address(0), bytes32(0), dependencies); } - /// `deployToNetworks` MUST revert with `DependencyChanged` when a - /// dependency's codehash differs from what was recorded during the check - /// phase. - function testDependencyChangedCodehashReverts() external { - // Make the test contract persistent so storage survives fork switches. - vm.makePersistent(address(this)); - - string[] memory networks = new string[](1); - networks[0] = LibRainDeploy.ARBITRUM_ONE; - address[] memory dependencies = new address[](1); - dependencies[0] = LibRainDeploy.ZOLTU_FACTORY; - - // Pre-populate with a wrong codehash to simulate a change between - // the check and deploy phases. - sDepCodeHashes[LibRainDeploy.ARBITRUM_ONE][LibRainDeploy.ZOLTU_FACTORY] = bytes32(uint256(1)); - - vm.expectRevert( - abi.encodeWithSelector( - LibRainDeploy.DependencyChanged.selector, - LibRainDeploy.ARBITRUM_ONE, - LibRainDeploy.ZOLTU_FACTORY, - bytes32(uint256(1)), - LibRainDeploy.ZOLTU_FACTORY_CODEHASH - ) - ); - this.externalDeployToNetworks(networks, address(this), hex"", "", address(0), bytes32(0), dependencies); - } - - /// `deployToNetworks` MUST revert with `MissingDependency` when a - /// dependency has been destroyed (code.length == 0) since the check phase. - function testDependencyMissingAtDeployTimeReverts() external { - // Make the test contract persistent so storage survives fork switches. - vm.makePersistent(address(this)); - + /// `deployToNetworks` MUST revert with `MissingDependency` when a dependency + /// has no code on the network. + function testDeployToNetworksMissingDependencyReverts() external { string[] memory networks = new string[](1); networks[0] = LibRainDeploy.ARBITRUM_ONE; address[] memory dependencies = new address[](1); dependencies[0] = address(0xdead); - // Pre-populate as if the dependency existed during the check phase. - sDepCodeHashes[LibRainDeploy.ARBITRUM_ONE][address(0xdead)] = bytes32(uint256(1)); - vm.expectRevert( abi.encodeWithSelector( LibRainDeploy.MissingDependency.selector, LibRainDeploy.ARBITRUM_ONE, address(0xdead) diff --git a/test/src/lib/MockDeployable.sol b/test/src/lib/MockDeployable.sol new file mode 100644 index 0000000..2309443 --- /dev/null +++ b/test/src/lib/MockDeployable.sol @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity ^0.8.25; + +/// @title MockDeployable +/// Minimal contract used as a deployment target for Zoltu factory tests. +contract MockDeployable { + /// @notice Placeholder value to ensure the contract has non-trivial code. + uint256 public value = 42; +} diff --git a/test/src/lib/MockReverter.sol b/test/src/lib/MockReverter.sol new file mode 100644 index 0000000..32bb09d --- /dev/null +++ b/test/src/lib/MockReverter.sol @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity ^0.8.25; + +/// @title MockReverter +/// Contract whose constructor always reverts, used to test DeployFailed with +/// success=false. +contract MockReverter { + constructor() { + revert(); + } +}