diff --git a/src/lib/LibRainDeploy.sol b/src/lib/LibRainDeploy.sol index f5769b5..85ab1f9 100644 --- a/src/lib/LibRainDeploy.sol +++ b/src/lib/LibRainDeploy.sol @@ -14,7 +14,9 @@ import {console2} from "forge-std-1.16.1/src/console2.sol"; library LibRainDeploy { /// Thrown when deployment via Zoltu factory fails. This could be either an /// explicit revert that manifests as non success, or a silent failure that - /// results in the deployed address being empty somehow. + /// results in the deployed address being empty somehow. `deployedAddress` + /// is zero whenever `success` is false: a failed call leaves revert data in + /// the output buffer rather than an address, so it is never read there. error DeployFailed(bool success, address deployedAddress); /// Thrown when a dependency is missing on a network before deployment. @@ -187,7 +189,11 @@ library LibRainDeploy { // Writing 20 bytes at offset 12 (= 32 - 20) right-aligns the address // in scratch space so that mload(0) produces a correctly padded value. success := call(gas(), zoltuFactory, 0, add(creationCode, 0x20), mload(creationCode), 12, 20) - deployedAddress := mload(0) + // The EVM copies revert data into the output region too, so only a + // successful call leaves an address there. A failed call leaves + // `deployedAddress` zero rather than reporting revert bytes as an + // address. + if success { deployedAddress := mload(0) } } if (!success || deployedAddress == address(0) || deployedAddress.code.length == 0) { console2.log("Zoltu deployment failed. Success:", success, "Deployed Address:", deployedAddress); diff --git a/test/concrete/MockAddressRevertingFactory.sol b/test/concrete/MockAddressRevertingFactory.sol new file mode 100644 index 0000000..16938ba --- /dev/null +++ b/test/concrete/MockAddressRevertingFactory.sol @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +/// @title MockAddressRevertingFactory +/// Zoltu factory stand-in whose calls always fail, reverting with exactly the +/// twenty bytes of its own address. A caller that reads its call output buffer +/// without checking the call succeeded sees the address of a contract that has +/// code. +contract MockAddressRevertingFactory { + fallback() external { + bytes20 self = bytes20(address(this)); + assembly ("memory-safe") { + mstore(0, self) + revert(0, 20) + } + } +} diff --git a/test/src/lib/LibRainDeploy.t.sol b/test/src/lib/LibRainDeploy.t.sol index ecb81ec..ec8632d 100644 --- a/test/src/lib/LibRainDeploy.t.sol +++ b/test/src/lib/LibRainDeploy.t.sol @@ -8,6 +8,7 @@ import {IAddressRegistryV1} from "../../../src/interface/IAddressRegistryV1.sol" import {AddressRegistry, ADDRESS_REGISTRY_ROOT} from "../../../src/concrete/AddressRegistry.sol"; import {MockResolvedOwner} from "../../concrete/MockResolvedOwner.sol"; import {MockDirtyWordOwner} from "../../concrete/MockDirtyWordOwner.sol"; +import {MockAddressRevertingFactory} from "../../concrete/MockAddressRevertingFactory.sol"; import {MockDeployable} from "../../concrete/MockDeployable.sol"; import {MockDeployableV2} from "../../concrete/MockDeployableV2.sol"; import {MockReverter} from "../../concrete/MockReverter.sol"; @@ -335,6 +336,30 @@ contract LibRainDeployTest is Test { this.externalDeployZoltu(type(MockReverter).creationCode); } + /// `deployZoltu` MUST report the zero address when the factory call fails, + /// even when the factory reverts with data that reads as an address. The + /// call output buffer holds revert data on the failure path, so anything + /// read from it there is not an address the factory returned. + function testDeployZoltuFailedCallReportsZeroAddress() external { + vm.createSelectFork(LibRainDeploy.ARBITRUM_ONE); + vm.etch(LibRainDeploy.ZOLTU_FACTORY, address(new MockAddressRevertingFactory()).code); + + // The discriminating power of this test is entirely the shape of the + // fixture's revert data: a failing call returning the twenty bytes of + // an address that has code. Asserted rather than assumed, because a + // fixture that reverted with fewer than twenty bytes would leave the + // pre-zeroed output buffer reading as the zero address, and the + // assertion below would then hold whether or not the failure path + // reads that buffer. + (bool factorySuccess, bytes memory factoryRevertData) = LibRainDeploy.ZOLTU_FACTORY.call(""); + assertFalse(factorySuccess); + assertEq(factoryRevertData, abi.encodePacked(bytes20(LibRainDeploy.ZOLTU_FACTORY))); + assertGt(LibRainDeploy.ZOLTU_FACTORY.code.length, 0); + + vm.expectRevert(abi.encodeWithSelector(LibRainDeploy.DeployFailed.selector, false, address(0))); + this.externalDeployZoltu(type(MockDeployable).creationCode); + } + /// `deployToNetworks` MUST revert with `UnexpectedDeployedAddress` when the /// creation code does not deploy to the expected address. function testUnexpectedDeployedAddressReverts() external {