From c0d48cf8a6dcff3fb394437a447c4c8bc6a0b7df Mon Sep 17 00:00:00 2001 From: David Meister Date: Wed, 29 Jul 2026 13:16:37 +0000 Subject: [PATCH 1/2] fix(deploy): report the zero address when the Zoltu factory call fails The EVM copies revert data into a call's output region, so a failed factory call leaves revert bytes exactly where `deployZoltu` reads `deployedAddress`. Read the output buffer only when the call succeeded, so a failed deploy reports the zero address instead of the first twenty bytes of the factory's revert data. Co-Authored-By: Claude --- src/lib/LibRainDeploy.sol | 10 ++++++++-- test/src/lib/LibRainDeploy.t.sol | 17 +++++++++++++++++ test/src/lib/MockAddressRevertingFactory.sol | 18 ++++++++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 test/src/lib/MockAddressRevertingFactory.sol diff --git a/src/lib/LibRainDeploy.sol b/src/lib/LibRainDeploy.sol index 962a2cb..6b1c0ba 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, as a failed factory call returns no + /// address. error DeployFailed(bool success, address deployedAddress); /// Thrown when a dependency is missing on a network before deployment. @@ -161,7 +163,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/src/lib/LibRainDeploy.t.sol b/test/src/lib/LibRainDeploy.t.sol index c08ec54..be7de7f 100644 --- a/test/src/lib/LibRainDeploy.t.sol +++ b/test/src/lib/LibRainDeploy.t.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.25; import {Test} from "forge-std-1.16.1/src/Test.sol"; import {LibRainDeploy} from "../../../src/lib/LibRainDeploy.sol"; +import {MockAddressRevertingFactory} from "./MockAddressRevertingFactory.sol"; import {MockDeployable} from "./MockDeployable.sol"; import {MockReverter} from "./MockReverter.sol"; @@ -297,6 +298,22 @@ 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 factory reverts with its own address, which has code, so the + // reported address is only zero if the failure path never reads the + // output buffer. + 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 /// deployed address does not match the expected address. function testUnexpectedDeployedAddressReverts() external { diff --git a/test/src/lib/MockAddressRevertingFactory.sol b/test/src/lib/MockAddressRevertingFactory.sol new file mode 100644 index 0000000..4b38fea --- /dev/null +++ b/test/src/lib/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) + } + } +} From 6d4d272ab7b7e1f178d8ae8fa795fde7f80e4d63 Mon Sep 17 00:00:00 2001 From: David Meister Date: Fri, 14 Aug 2026 14:55:18 +0000 Subject: [PATCH 2/2] test(deploy): assert the fixture property the zero-address test depends on MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test's discriminating power is entirely the shape of MockAddressRevertingFactory's revert data — twenty address-shaped bytes from a contract that has code. Nothing asserted that, so a fixture reverting with fewer than twenty bytes would leave the pre-zeroed output buffer reading as address(0) and the test would pass whether or not the failure path reads the buffer. The fixture is now probed directly. The DeployFailed NatSpec claimed a failed factory call 'returns no address'. A failed call can and does return twenty address-shaped bytes — that is the bug. The invariant holds because deployZoltu does not read the buffer on failure, which is what it now says. Co-Authored-By: Claude Opus 5 (1M context) --- src/lib/LibRainDeploy.sol | 4 ++-- test/src/lib/LibRainDeploy.t.sol | 14 +++++++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/lib/LibRainDeploy.sol b/src/lib/LibRainDeploy.sol index b0a3334..85ab1f9 100644 --- a/src/lib/LibRainDeploy.sol +++ b/src/lib/LibRainDeploy.sol @@ -15,8 +15,8 @@ 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. `deployedAddress` - /// is zero whenever `success` is false, as a failed factory call returns no - /// address. + /// 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. diff --git a/test/src/lib/LibRainDeploy.t.sol b/test/src/lib/LibRainDeploy.t.sol index f87878d..ec8632d 100644 --- a/test/src/lib/LibRainDeploy.t.sol +++ b/test/src/lib/LibRainDeploy.t.sol @@ -343,9 +343,17 @@ contract LibRainDeployTest is Test { function testDeployZoltuFailedCallReportsZeroAddress() external { vm.createSelectFork(LibRainDeploy.ARBITRUM_ONE); vm.etch(LibRainDeploy.ZOLTU_FACTORY, address(new MockAddressRevertingFactory()).code); - // The factory reverts with its own address, which has code, so the - // reported address is only zero if the failure path never reads the - // output buffer. + + // 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)));