diff --git a/README.md b/README.md index 6776143..9c2eb72 100644 --- a/README.md +++ b/README.md @@ -21,7 +21,8 @@ Approach: - Zoltu deterministic deployment proxy: same address on every supported network. - Caller-provided supported-network and dependency lists. - Hard guards against deploying to networks where dependencies are missing. -- Pre-calculated addresses asserted post-deploy: silent failures fail loudly. +- Pre-calculated addresses asserted against the creation code before deploying, + and against the chain after: silent failures fail loudly. - Bytecode integrity checks (e.g. via the Rain Extrospection lib) supported post-deploy. diff --git a/src/lib/LibRainDeploy.sol b/src/lib/LibRainDeploy.sol index 962a2cb..39c5175 100644 --- a/src/lib/LibRainDeploy.sol +++ b/src/lib/LibRainDeploy.sol @@ -147,6 +147,20 @@ library LibRainDeploy { vm.etch(ZOLTU_FACTORY, ZOLTU_FACTORY_BYTECODE); } + /// Derives the address the Zoltu factory deploys the given creation code + /// to. The factory is CREATE2 over its calldata with a zero salt, so the + /// address is a pure function of the creation code and is identical on + /// every network. + /// @param creationCode The creation code to derive the address for. + /// @return The address the creation code deploys to. + function zoltuAddress(bytes memory creationCode) internal pure returns (address) { + return address( + uint160( + uint256(keccak256(abi.encodePacked(bytes1(0xff), ZOLTU_FACTORY, bytes32(0), keccak256(creationCode)))) + ) + ); + } + /// Deploys the given creation code via the Zoltu factory. /// Handles the return data and errors appropriately. /// @param creationCode The creation code to deploy. @@ -185,6 +199,11 @@ library LibRainDeploy { } /// Deploys the given creation code to each network via the Zoltu factory. + /// `expectedAddress` MUST be the address the Zoltu factory derives for + /// `creationCode`, which is checked before any network is forked, so an + /// expected address that disagrees with the creation code fails loudly + /// rather than matching some other contract already deployed there and + /// skipping every network. /// 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 @@ -201,7 +220,8 @@ library LibRainDeploy { /// @param deployer The deployer address. /// @param creationCode The creation code to deploy. /// @param contractPath The contract path for verification commands. - /// @param expectedAddress The expected deterministic address. + /// @param expectedAddress The expected deterministic address, which MUST be + /// the address the Zoltu factory derives for `creationCode`. /// @param expectedCodeHash The expected code hash of the deployed contract. /// @param dependencies The addresses that must have code on each network. /// @return deployedAddress The deployed contract address. @@ -218,6 +238,16 @@ library LibRainDeploy { if (networks.length == 0) { revert NoNetworks(); } + // The Zoltu factory deploys the given creation code to a single + // deterministic address on every network, so an expected address that + // disagrees with the creation code can never hold that code. Checked + // up front, before any fork, because otherwise a network that already + // has some other contract at the expected address takes the skip + // branch and reports success without ever deploying. + address derivedAddress = zoltuAddress(creationCode); + if (derivedAddress != expectedAddress) { + revert UnexpectedDeployedAddress(expectedAddress, derivedAddress); + } for (uint256 i = 0; i < networks.length; i++) { // createSelectFork returns a fork id that is not needed here; bind // and reference it so the unused-return lint stays satisfied. @@ -282,7 +312,8 @@ library LibRainDeploy { /// @param deployerPrivateKey The private key to use for broadcasting. /// @param creationCode The creation code to deploy. /// @param contractPath The contract path for verification commands. - /// @param expectedAddress The expected deterministic address. + /// @param expectedAddress The expected deterministic address, which MUST be + /// the address the Zoltu factory derives for `creationCode`. /// @param expectedCodeHash The expected code hash of the deployed contract. /// @param dependencies The dependency addresses to check. /// @return deployedAddress The address of the deployed contract. diff --git a/test/src/lib/MockDeployable.sol b/test/concrete/MockDeployable.sol similarity index 93% rename from test/src/lib/MockDeployable.sol rename to test/concrete/MockDeployable.sol index 2309443..06790a8 100644 --- a/test/src/lib/MockDeployable.sol +++ b/test/concrete/MockDeployable.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: LicenseRef-DCL-1.0 // SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd -pragma solidity ^0.8.25; +pragma solidity =0.8.25; /// @title MockDeployable /// Minimal contract used as a deployment target for Zoltu factory tests. diff --git a/test/concrete/MockDeployableV2.sol b/test/concrete/MockDeployableV2.sol new file mode 100644 index 0000000..8c92819 --- /dev/null +++ b/test/concrete/MockDeployableV2.sol @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +/// @title MockDeployableV2 +/// Deployment target whose creation code differs from `MockDeployable`, so the +/// two derive different Zoltu addresses. Stands in for a contract whose +/// bytecode has changed while its pinned constants have not. +contract MockDeployableV2 { + /// @notice Placeholder value to ensure the contract has non-trivial code. + uint256 public value = 43; + + /// @notice Second placeholder value so the runtime code also differs from + /// `MockDeployable`. + uint256 public other = 99; +} diff --git a/test/src/lib/MockReverter.sol b/test/concrete/MockReverter.sol similarity index 92% rename from test/src/lib/MockReverter.sol rename to test/concrete/MockReverter.sol index 32bb09d..2081d63 100644 --- a/test/src/lib/MockReverter.sol +++ b/test/concrete/MockReverter.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: LicenseRef-DCL-1.0 // SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd -pragma solidity ^0.8.25; +pragma solidity =0.8.25; /// @title MockReverter /// Contract whose constructor always reverts, used to test DeployFailed with diff --git a/test/src/lib/LibRainDeploy.t.sol b/test/src/lib/LibRainDeploy.t.sol index c08ec54..4991305 100644 --- a/test/src/lib/LibRainDeploy.t.sol +++ b/test/src/lib/LibRainDeploy.t.sol @@ -4,14 +4,39 @@ pragma solidity ^0.8.25; import {Test} from "forge-std-1.16.1/src/Test.sol"; import {LibRainDeploy} from "../../../src/lib/LibRainDeploy.sol"; -import {MockDeployable} from "./MockDeployable.sol"; -import {MockReverter} from "./MockReverter.sol"; +import {MockDeployable} from "../../concrete/MockDeployable.sol"; +import {MockDeployableV2} from "../../concrete/MockDeployableV2.sol"; +import {MockReverter} from "../../concrete/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 { + /// The address the Zoltu factory deploys `MockDeployable` to. Derived from + /// the mock's creation code by the same formula the factory applies, so it + /// follows the compiler that builds the mock. `testDeployZoltu` pins the + /// derivation against the live factory on a fork. + /// @return The deterministic address for `MockDeployable`. + function mockDeployableAddress() internal pure returns (address) { + return LibRainDeploy.zoltuAddress(type(MockDeployable).creationCode); + } + + /// The code hash `MockDeployable` has once deployed, i.e. `keccak256` over + /// the runtime code its creation code leaves behind. Derived from the mock + /// rather than pinned, for the same reason as `mockDeployableAddress`. + /// @return The deployed code hash for `MockDeployable`. + function mockDeployableCodeHash() internal pure returns (bytes32) { + return keccak256(type(MockDeployable).runtimeCode); + } + + /// The address the Zoltu factory deploys `MockDeployableV2` to, derived the + /// same way as `mockDeployableAddress`. + /// @return The deterministic address for `MockDeployableV2`. + function mockDeployableV2Address() internal pure returns (address) { + return LibRainDeploy.zoltuAddress(type(MockDeployableV2).creationCode); + } + /// External wrapper for `isStartBlock` so that it can be called /// externally in tests. /// @param target The contract address to check. @@ -212,7 +237,7 @@ contract LibRainDeployTest is Test { } /// `deployToNetworks` MUST revert with `NoNetworks` when given an empty - /// networks array. + /// networks array, before any other input is checked. function testDeployToNetworksNoNetworksReverts() external { string[] memory networks = new string[](0); address[] memory dependencies = new address[](0); @@ -233,12 +258,12 @@ contract LibRainDeployTest is Test { networks, address(this), type(MockDeployable).creationCode, - "test/src/lib/MockDeployable.sol:MockDeployable", - 0xC24016f209562fc151e5Ab7F88694ED5775feb36, - 0xc1a263a0b50505687a5140c7964ec5c947329e7d03410306fee68cc3620c5483, + "test/concrete/MockDeployable.sol:MockDeployable", + mockDeployableAddress(), + mockDeployableCodeHash(), dependencies ); - assertEq(result, 0xC24016f209562fc151e5Ab7F88694ED5775feb36); + assertEq(result, mockDeployableAddress()); } /// External wrapper for `deployToNetworks` so that `vm.expectRevert` @@ -277,7 +302,12 @@ contract LibRainDeployTest is Test { function testDeployZoltu() external { vm.createSelectFork(LibRainDeploy.ARBITRUM_ONE); address deployed = this.externalDeployZoltu(type(MockDeployable).creationCode); - assertEq(deployed, 0xC24016f209562fc151e5Ab7F88694ED5775feb36); + // Pinned literal, deliberately not `mockDeployableAddress()`. The live + // factory on the fork is the oracle here, so an expected value taken + // from the derivation would only check `zoltuAddress` against itself. + // It is the address the factory returns for the creation code solc + // 0.8.25 emits for `MockDeployable`, which itself pins `=0.8.25`. + assertEq(deployed, 0x1fa1bBf9Cf73B1aCCc1a3D9de5896E81Cd567854); } /// `deployZoltu` MUST revert with `DeployFailed` when the Zoltu factory @@ -298,16 +328,14 @@ contract LibRainDeployTest is Test { } /// `deployToNetworks` MUST revert with `UnexpectedDeployedAddress` when the - /// deployed address does not match the expected address. + /// creation code does not deploy to the expected address. function testUnexpectedDeployedAddressReverts() external { string[] memory networks = new string[](1); networks[0] = LibRainDeploy.ARBITRUM_ONE; address[] memory dependencies = new address[](0); vm.expectRevert( abi.encodeWithSelector( - LibRainDeploy.UnexpectedDeployedAddress.selector, - address(0xdead), - 0xC24016f209562fc151e5Ab7F88694ED5775feb36 + LibRainDeploy.UnexpectedDeployedAddress.selector, address(0xdead), mockDeployableAddress() ) ); this.externalDeployToNetworks( @@ -321,13 +349,11 @@ contract LibRainDeployTest is Test { string[] memory networks = new string[](1); networks[0] = LibRainDeploy.ARBITRUM_ONE; address[] memory dependencies = new address[](0); - address expectedAddress = 0xC24016f209562fc151e5Ab7F88694ED5775feb36; + address expectedAddress = mockDeployableAddress(); bytes32 wrongCodeHash = bytes32(uint256(1)); vm.expectRevert( abi.encodeWithSelector( - LibRainDeploy.UnexpectedDeployedCodeHash.selector, - wrongCodeHash, - 0xc1a263a0b50505687a5140c7964ec5c947329e7d03410306fee68cc3620c5483 + LibRainDeploy.UnexpectedDeployedCodeHash.selector, wrongCodeHash, mockDeployableCodeHash() ) ); this.externalDeployToNetworks( @@ -345,12 +371,12 @@ contract LibRainDeployTest is Test { networks, 1, type(MockDeployable).creationCode, - "test/src/lib/MockDeployable.sol:MockDeployable", - 0xC24016f209562fc151e5Ab7F88694ED5775feb36, - 0xc1a263a0b50505687a5140c7964ec5c947329e7d03410306fee68cc3620c5483, + "test/concrete/MockDeployable.sol:MockDeployable", + mockDeployableAddress(), + mockDeployableCodeHash(), dependencies ); - assertEq(deployed, 0xC24016f209562fc151e5Ab7F88694ED5775feb36); + assertEq(deployed, mockDeployableAddress()); } /// `deployToNetworks` MUST skip deployment and return the expected address @@ -360,7 +386,7 @@ contract LibRainDeployTest is Test { vm.createSelectFork(LibRainDeploy.ARBITRUM_ONE); address deployed = this.externalDeployZoltu(type(MockDeployable).creationCode); - assertEq(deployed, 0xC24016f209562fc151e5Ab7F88694ED5775feb36); + assertEq(deployed, mockDeployableAddress()); vm.makePersistent(deployed); string[] memory networks = new string[](1); @@ -371,12 +397,12 @@ contract LibRainDeployTest is Test { networks, address(this), type(MockDeployable).creationCode, - "test/src/lib/MockDeployable.sol:MockDeployable", - 0xC24016f209562fc151e5Ab7F88694ED5775feb36, - 0xc1a263a0b50505687a5140c7964ec5c947329e7d03410306fee68cc3620c5483, + "test/concrete/MockDeployable.sol:MockDeployable", + mockDeployableAddress(), + mockDeployableCodeHash(), dependencies ); - assertEq(result, 0xC24016f209562fc151e5Ab7F88694ED5775feb36); + assertEq(result, mockDeployableAddress()); } /// `deployToNetworks` MUST skip an already-deployed network WITHOUT checking @@ -388,7 +414,7 @@ contract LibRainDeployTest is Test { vm.createSelectFork(LibRainDeploy.ARBITRUM_ONE); address deployed = this.externalDeployZoltu(type(MockDeployable).creationCode); - assertEq(deployed, 0xC24016f209562fc151e5Ab7F88694ED5775feb36); + assertEq(deployed, mockDeployableAddress()); vm.makePersistent(deployed); string[] memory networks = new string[](1); @@ -402,12 +428,12 @@ contract LibRainDeployTest is Test { networks, address(this), type(MockDeployable).creationCode, - "test/src/lib/MockDeployable.sol:MockDeployable", - 0xC24016f209562fc151e5Ab7F88694ED5775feb36, - 0xc1a263a0b50505687a5140c7964ec5c947329e7d03410306fee68cc3620c5483, + "test/concrete/MockDeployable.sol:MockDeployable", + mockDeployableAddress(), + mockDeployableCodeHash(), dependencies ); - assertEq(result, 0xC24016f209562fc151e5Ab7F88694ED5775feb36); + assertEq(result, mockDeployableAddress()); } /// `deployToNetworks` MUST revert with `MissingDependency` when the Zoltu @@ -427,7 +453,15 @@ contract LibRainDeployTest is Test { LibRainDeploy.MissingDependency.selector, LibRainDeploy.ARBITRUM_ONE, LibRainDeploy.ZOLTU_FACTORY ) ); - this.externalDeployToNetworks(networks, address(this), hex"", "", address(0), bytes32(0), dependencies); + this.externalDeployToNetworks( + networks, + address(this), + type(MockDeployable).creationCode, + "", + mockDeployableAddress(), + bytes32(0), + dependencies + ); } /// `deployToNetworks` MUST revert with `DependencyChanged` when the Zoltu @@ -451,7 +485,15 @@ contract LibRainDeployTest is Test { keccak256(hex"00") ) ); - this.externalDeployToNetworks(networks, address(this), hex"", "", address(0), bytes32(0), dependencies); + this.externalDeployToNetworks( + networks, + address(this), + type(MockDeployable).creationCode, + "", + mockDeployableAddress(), + bytes32(0), + dependencies + ); } /// `deployToNetworks` MUST revert with `MissingDependency` when a dependency @@ -467,6 +509,122 @@ contract LibRainDeployTest is Test { LibRainDeploy.MissingDependency.selector, LibRainDeploy.ARBITRUM_ONE, address(0xdead) ) ); - this.externalDeployToNetworks(networks, address(this), hex"", "", address(0), bytes32(0), dependencies); + this.externalDeployToNetworks( + networks, + address(this), + type(MockDeployable).creationCode, + "", + mockDeployableAddress(), + bytes32(0), + dependencies + ); + } + + /// `zoltuAddress` MUST derive the address the Zoltu factory actually + /// deploys the given creation code to, and creation code that differs MUST + /// derive a different address. + function testZoltuAddressMatchesFactoryDeploy() external { + vm.createSelectFork(LibRainDeploy.ARBITRUM_ONE); + assertEq( + LibRainDeploy.zoltuAddress(type(MockDeployable).creationCode), + this.externalDeployZoltu(type(MockDeployable).creationCode) + ); + assertEq( + LibRainDeploy.zoltuAddress(type(MockDeployableV2).creationCode), + this.externalDeployZoltu(type(MockDeployableV2).creationCode) + ); + assertNotEq( + LibRainDeploy.zoltuAddress(type(MockDeployable).creationCode), + LibRainDeploy.zoltuAddress(type(MockDeployableV2).creationCode) + ); + } + + /// `deployToNetworks` MUST revert with `UnexpectedDeployedAddress` when the + /// creation code does not deploy to `expectedAddress`, even when a contract + /// with the expected code hash already sits at that address on every + /// network and would otherwise be skipped as already deployed. + function testDeployToNetworksStaleExpectedAddressReverts() external { + vm.makePersistent(address(this)); + + vm.createSelectFork(LibRainDeploy.ARBITRUM_ONE); + address deployed = this.externalDeployZoltu(type(MockDeployable).creationCode); + assertEq(deployed, mockDeployableAddress()); + vm.makePersistent(deployed); + + string[] memory networks = new string[](1); + networks[0] = LibRainDeploy.ARBITRUM_ONE; + address[] memory dependencies = new address[](0); + + vm.expectRevert( + abi.encodeWithSelector( + LibRainDeploy.UnexpectedDeployedAddress.selector, mockDeployableAddress(), mockDeployableV2Address() + ) + ); + // The new contract's creation code paired with the old contract's + // address and code hash. + this.externalDeployToNetworks( + networks, + address(this), + type(MockDeployableV2).creationCode, + "test/concrete/MockDeployableV2.sol:MockDeployableV2", + mockDeployableAddress(), + mockDeployableCodeHash(), + dependencies + ); + + // The new contract was not deployed anywhere. + assertEq(mockDeployableV2Address().code.length, 0); + } + + /// `deployToNetworks` MUST check `expectedAddress` against the creation + /// code before it forks anything, so the mismatch is reported without any + /// network being reachable at all. + function testDeployToNetworksStaleExpectedAddressRevertsBeforeForking() external { + string[] memory networks = new string[](1); + // Not a configured RPC alias, so forking it is itself an error. + networks[0] = "unconfigured_network"; + address[] memory dependencies = new address[](0); + + vm.expectRevert( + abi.encodeWithSelector( + LibRainDeploy.UnexpectedDeployedAddress.selector, address(0xdead), mockDeployableAddress() + ) + ); + this.externalDeployToNetworks( + networks, address(this), type(MockDeployable).creationCode, "", address(0xdead), bytes32(0), dependencies + ); + } + + /// `deployToNetworks` MUST revert with `UnexpectedDeployedAddress` when the + /// factory reports an address other than the one derived from the creation + /// code, so the chain is checked and not only the derivation. + function testDeployToNetworksFactoryReportsOtherAddressReverts() external { + vm.createSelectFork(LibRainDeploy.ARBITRUM_ONE); + // The factory reports the address of a contract that does have code, + // but not the one the creation code derives. + vm.mockCall( + LibRainDeploy.ZOLTU_FACTORY, + type(MockDeployable).creationCode, + abi.encodePacked(bytes20(LibRainDeploy.ZOLTU_FACTORY)) + ); + + string[] memory networks = new string[](1); + networks[0] = LibRainDeploy.ARBITRUM_ONE; + address[] memory dependencies = new address[](0); + + vm.expectRevert( + abi.encodeWithSelector( + LibRainDeploy.UnexpectedDeployedAddress.selector, mockDeployableAddress(), LibRainDeploy.ZOLTU_FACTORY + ) + ); + this.externalDeployToNetworks( + networks, + address(this), + type(MockDeployable).creationCode, + "test/concrete/MockDeployable.sol:MockDeployable", + mockDeployableAddress(), + mockDeployableCodeHash(), + dependencies + ); } }