fix(deploy): report the zero address when the Zoltu factory call fails

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);

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)
+        }
+    }
+}

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 {