diff --git a/audit/mutation-test-scans.json b/audit/mutation-test-scans.json index b0da986..d5fe3fc 100644 --- a/audit/mutation-test-scans.json +++ b/audit/mutation-test-scans.json @@ -18,5 +18,28 @@ "confirmed": 3, "filed": ["#16", "#17", "#18"] } + }, + { + "timestamp": "2026-08-18T13:31:09Z", + "commit": "ed91bfa5161fa94c5180d57a54e2a4341ba8e876", + "publishedTag": "sol-v0.1.5", + "commitsAheadOfTag": 211, + "scope": "whole repo", + "tool": "adversarial-mutation-test", + "skillVersion": "0.33.0", + "summary": { + "units": 15, + "behaviours": 264, + "killedByExistingTests": 234, + "gapsFilled": 7, + "equivalentMutants": 7, + "unkillableDefensiveGuards": 3, + "notExecutedBySuite": 11, + "nonTerminating": 2, + "candidates": 1, + "confirmed": 1, + "filed": [], + "alreadyFiled": ["#135"] + } } ] diff --git a/test/concrete/MissingDependencyDeploy.sol b/test/concrete/MissingDependencyDeploy.sol new file mode 100644 index 0000000..c2d7ba8 --- /dev/null +++ b/test/concrete/MissingDependencyDeploy.sol @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {RainDeployBroadcast} from "../../src/abstract/RainDeployBroadcast.sol"; +import {DeployCandidate, DeploySuite, RainDeploySuitesBase} from "../../src/abstract/RainDeploySuitesBase.sol"; +import {LibRainDeploy} from "../../src/lib/LibRainDeploy.sol"; +import {MockDeployable} from "./MockDeployable.sol"; + +/// @dev The address the candidate declares must already hold code, and which +/// holds none on any network. +address constant ABSENT_DEPENDENCY = address(0xdeadbee5); + +/// @title MissingDependencyDeploy +/// @notice A deploy script whose candidate declares a dependency that is on no +/// network, so a broadcast that carries the declared list to +/// `deployToNetworks` refuses before it deploys anything and one that carries +/// an empty list deploys. +/// +/// The suite is `MockDeployable`, which is on no supported network, so the +/// broadcast takes the deploying branch — the already-deployed branch skips the +/// dependency check by design and would say nothing about the list. +/// +/// One network, so the refusal names a chain that is the whole target set +/// rather than the first of five. Keyed `second-address-candidate` for the +/// reason `StalePinDeploy` gives. +contract MissingDependencyDeploy is RainDeployBroadcast { + /// @inheritdoc RainDeployBroadcast + function deployNetworks() internal pure override returns (string[] memory networks) { + networks = new string[](1); + networks[0] = LibRainDeploy.ARBITRUM_ONE; + } + + /// @inheritdoc RainDeploySuitesBase + function releasedSuites() internal pure override returns (DeploySuite[] memory suites) { + suites = new DeploySuite[](0); + } + + /// @inheritdoc RainDeploySuitesBase + function candidateSuites() internal pure override returns (DeployCandidate[] memory candidates) { + address[] memory dependencies = new address[](1); + dependencies[0] = ABSENT_DEPENDENCY; + + candidates = new DeployCandidate[](1); + candidates[0] = DeployCandidate({ + snapshot: DeploySuite({ + suite: "second-address-candidate", + creationCode: type(MockDeployable).creationCode, + storedDeployedAddress: LibRainDeploy.zoltuAddress(type(MockDeployable).creationCode), + storedBytecodeHash: keccak256(type(MockDeployable).runtimeCode), + storedRuntimeCode: type(MockDeployable).runtimeCode, + artifactPath: "test/concrete/MockDeployable.sol:MockDeployable", + dependencies: dependencies + }), + sourceCreationCode: type(MockDeployable).creationCode + }); + } +} diff --git a/test/concrete/MockChainDependentOwner.sol b/test/concrete/MockChainDependentOwner.sol new file mode 100644 index 0000000..e480369 --- /dev/null +++ b/test/concrete/MockChainDependentOwner.sol @@ -0,0 +1,37 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +/// @title MockChainDependentOwner +/// @notice Answers the read `MockResolvedOwner` answers with one address on one +/// chain and another everywhere else. +/// +/// This is the deployment the network matrix exists to find: one contract, at +/// one deterministic address, holding a different resolved value on a chain +/// nobody looked at. A deployment that answers the same thing everywhere cannot +/// tell a matrix that forked every network from one that forked the first. +contract MockChainDependentOwner { + /// The address answered on `iChainId`. + address public immutable iOwnerOnChain; + + /// The address answered on every other chain. + address public immutable iOwnerElsewhere; + + /// The chain `iOwnerOnChain` is answered on. + uint256 public immutable iChainId; + + /// @param ownerOnChain The address to answer on `chainId`. + /// @param ownerElsewhere The address to answer everywhere else. + /// @param chainId The chain `ownerOnChain` is answered on. + constructor(address ownerOnChain, address ownerElsewhere, uint256 chainId) { + iOwnerOnChain = ownerOnChain; + iOwnerElsewhere = ownerElsewhere; + iChainId = chainId; + } + + /// The selector `MockResolvedOwner` answers. + /// @return The address for the chain this is called on. + function iOwner() external view returns (address) { + return block.chainid == iChainId ? iOwnerOnChain : iOwnerElsewhere; + } +} diff --git a/test/concrete/MockRevertingAnswerOwner.sol b/test/concrete/MockRevertingAnswerOwner.sol new file mode 100644 index 0000000..38dc3c1 --- /dev/null +++ b/test/concrete/MockRevertingAnswerOwner.sol @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +/// @title MockRevertingAnswerOwner +/// @notice Answers the read `MockResolvedOwner` answers by REVERTING with +/// exactly the bytes a successful answer would have carried. +/// +/// Nothing about the returned bytes distinguishes it from a real answer — same +/// length, same word, same address — so only whether the call SUCCEEDED +/// separates the two. A revert carrying an ABI-encoded address is ordinary: a +/// custom error whose one argument is an address is that shape, and so is any +/// `require` that bubbles a callee's return data. +contract MockRevertingAnswerOwner { + /// The bytes this contract reverts every read with. + bytes internal sAnswer; + + /// @param answer The bytes to revert with. + constructor(bytes memory answer) { + sAnswer = answer; + } + + /// The selector `MockResolvedOwner` answers, reverting with `sAnswer` as + /// the whole of the revert data. + function iOwner() external view { + bytes memory answer = sAnswer; + assembly ("memory-safe") { + revert(add(answer, 0x20), mload(answer)) + } + } +} diff --git a/test/concrete/StalePinDeploy.sol b/test/concrete/StalePinDeploy.sol new file mode 100644 index 0000000..402f164 --- /dev/null +++ b/test/concrete/StalePinDeploy.sol @@ -0,0 +1,49 @@ +// SPDX-License-Identifier: LicenseRef-DCL-1.0 +// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd +pragma solidity =0.8.25; + +import {RainDeployBroadcast} from "../../src/abstract/RainDeployBroadcast.sol"; +import {DeployCandidate, DeploySuite, RainDeploySuitesBase} from "../../src/abstract/RainDeploySuitesBase.sol"; +import {MockDeployableV2} from "./MockDeployableV2.sol"; + +/// @dev The address the candidate records, which its own creation code does not +/// derive. +address constant STALE_PIN_ADDRESS = address(0xdead); + +/// @title StalePinDeploy +/// @notice A deploy script whose candidate records an address its creation code +/// does not derive: the snapshot whose pins went stale while its recorded bytes +/// stayed current. +/// +/// The source anchor has nothing to say about it — the recorded creation code IS +/// the source — so this reaches `deployAndBroadcast` carrying a pin that +/// describes nothing, which is the state the pre-fork address comparison exists +/// for. +/// +/// Keyed `second-address-candidate` so it answers to the `DEPLOYMENT_SUITE` the +/// broadcast test has already set: that variable is process-wide and forge runs +/// tests concurrently, so every write to it stays inside the one test that owns +/// it. +contract StalePinDeploy is RainDeployBroadcast { + /// @inheritdoc RainDeploySuitesBase + function releasedSuites() internal pure override returns (DeploySuite[] memory suites) { + suites = new DeploySuite[](0); + } + + /// @inheritdoc RainDeploySuitesBase + function candidateSuites() internal pure override returns (DeployCandidate[] memory candidates) { + candidates = new DeployCandidate[](1); + candidates[0] = DeployCandidate({ + snapshot: DeploySuite({ + suite: "second-address-candidate", + creationCode: type(MockDeployableV2).creationCode, + storedDeployedAddress: STALE_PIN_ADDRESS, + storedBytecodeHash: keccak256(type(MockDeployableV2).runtimeCode), + storedRuntimeCode: type(MockDeployableV2).runtimeCode, + artifactPath: "test/concrete/MockDeployableV2.sol:MockDeployableV2", + dependencies: new address[](0) + }), + sourceCreationCode: type(MockDeployableV2).creationCode + }); + } +} diff --git a/test/src/abstract/RainDeployBroadcast.t.sol b/test/src/abstract/RainDeployBroadcast.t.sol index eca9c39..0df6c91 100644 --- a/test/src/abstract/RainDeployBroadcast.t.sol +++ b/test/src/abstract/RainDeployBroadcast.t.sol @@ -9,6 +9,8 @@ import {LibRainDeploy} from "../../../src/lib/LibRainDeploy.sol"; import {ExampleDeploy} from "../../concrete/ExampleDeploy.sol"; import {ExampleDeploySingleNetwork} from "../../concrete/ExampleDeploySingleNetwork.sol"; import {SourceMismatchDeploy} from "../../concrete/SourceMismatchDeploy.sol"; +import {StalePinDeploy, STALE_PIN_ADDRESS} from "../../concrete/StalePinDeploy.sol"; +import {MissingDependencyDeploy, ABSENT_DEPENDENCY} from "../../concrete/MissingDependencyDeploy.sol"; import {MockDeployable} from "../../concrete/MockDeployable.sol"; import {MockDeployableV2} from "../../concrete/MockDeployableV2.sol"; @@ -107,6 +109,15 @@ contract RainDeployBroadcastTest is Test { /// Here rather than in a test of its own for the reason the first two are /// here: this leg has to SET `DEPLOYMENT_SUITE`, and a second test doing /// that is a second test racing the first over one process-global variable. + /// + /// ## Carrying the suite's own pins and its own dependencies + /// + /// Two further legs drive declarations whose selected suite differs from the + /// one above in exactly one field: a recorded address its creation code does + /// not derive, and a dependency that is on no network. Both answer to the + /// `DEPLOYMENT_SUITE` this test has already set, so neither adds a writer of + /// it, and both fail where a `run()` that derived the pins or dropped the + /// list would not. function testRunSelectsTheSuiteFromTheEnvBeforeTheKeyNeverDefaultsAndBroadcastsIt() external { // `DEPLOYMENT_SUITE` has to be ABSENT and no cheatcode makes it so, so // the precondition is asserted: a value set outside this test reports @@ -194,6 +205,47 @@ contract RainDeployBroadcastTest is Test { // VALUE of that function; nothing until here asserted that `run()` is // what consults it. assertEq(block.chainid, ARBITRUM_ONE_CHAIN_ID); + + // ## The pins it carries are the ones the suite RECORDS + // + // `deployToNetworks` compares the recorded address against the address + // the creation code derives before it forks anything, and that guard is + // worth exactly nothing if `run()` hands it a value derived here: a + // comparison of a value against itself passes on every suite, stale + // pins included, and the deploy lands wherever the code happens to go + // rather than where the repo's constants say. The suite above records + // pins that are correct, so it cannot tell the two apart; this one + // records an address its own creation code does not derive, and the + // refusal naming both is the whole of the difference. + // + // It answers to the same `DEPLOYMENT_SUITE` already set, so this leg + // adds no second writer of a process-wide variable. + StalePinDeploy stale = new StalePinDeploy(); + vm.expectRevert( + abi.encodeWithSelector( + LibRainDeploy.UnexpectedDeployedAddress.selector, + STALE_PIN_ADDRESS, + LibRainDeploy.zoltuAddress(type(MockDeployableV2).creationCode) + ) + ); + stale.run(); + + // ## And so is the dependency list + // + // A suite's dependencies are the addresses that MUST already hold code + // on a network before it is broadcast there — a constructor that bakes + // in a beacon, a fallback that delegatecalls a facet — so a `run()` that + // dropped them broadcasts a deployment that is born broken, on every + // chain the dispatch reached, at an address `CREATE2` makes permanent. + // Every suite above declares none, so none of them can tell a list that + // was carried from a list that was replaced with an empty one. + MissingDependencyDeploy dependent = new MissingDependencyDeploy(); + vm.expectRevert( + abi.encodeWithSelector( + LibRainDeploy.MissingDependency.selector, LibRainDeploy.ARBITRUM_ONE, ABSENT_DEPENDENCY + ) + ); + dependent.run(); } /// The broadcast MUST refuse a candidate that is not the contract this repo diff --git a/test/src/abstract/RainDeployVerifySnapshot.t.sol b/test/src/abstract/RainDeployVerifySnapshot.t.sol index 1f877b6..c3a2366 100644 --- a/test/src/abstract/RainDeployVerifySnapshot.t.sol +++ b/test/src/abstract/RainDeployVerifySnapshot.t.sol @@ -471,6 +471,86 @@ contract RainDeployVerifySnapshotTest is ExampleDeploySuites, RainDeployVerifySn this.externalCheckInternallyConsistent(suite); } + /// The record is matched on the address a released suite's creation code + /// DERIVES, never on the address that suite RECORDS. + /// + /// The two are the same in a consistent set, which is what makes this worth + /// stating: a suite whose recorded address has gone stale still deployed + /// what its creation code deploys, so it still declares that release — and + /// the stale field is `checkInternallyConsistent`'s to catch, with an error + /// that names it. Matching on the recorded field instead reports a declared + /// release as one nobody declared, and sends the reader to + /// `releasedSuites()` to add an entry that is already there. + /// + /// The suite here is the record's own release with that one field broken, + /// so the derivation is the only thing left that can match it. + function testFrozenSnapshotMatchesTheDerivationNotTheRecordedAddress() external view { + DeploySuite[] memory released = new DeploySuite[](1); + released[0] = consistentSuite(); + released[0].storedDeployedAddress = address(0xdead); + + // It really is the record's release, and it really does record + // something else. + assertEq(LibRainDeploy.zoltuAddress(released[0].creationCode), ADDRESS_REGISTRY_DEPLOYED_ADDRESS); + assertNotEq(released[0].storedDeployedAddress, ADDRESS_REGISTRY_DEPLOYED_ADDRESS); + + this.externalCheckFrozenSnapshotsReleased(recordOfTheGeneratedSnapshot(), released); + } + + /// The inherited internal-consistency check MUST reach EVERY declared + /// suite. + /// + /// Every negative case above drives `checkInternallyConsistent` with one + /// suite handed to it, so all of them pass on a check that only ever looked + /// at the first — and a repo declares one suite until the day it declares + /// several, which is the day a snapshot regenerated for one contract and + /// not the other appears. The suite broken here is not the first, so a loop + /// that stopped there would find nothing to fail on. + /// + /// Broken through the factory rather than by editing a field, because the + /// declaration this contract inherits is the passing case for the inherited + /// tests and cannot be broken in place. The mock is + /// `testZoltuDerivationMismatchReverts`'s, keyed on the creation code of the + /// suite being reached rather than of the first one. + function testSnapshotInternallyConsistentReachesEverySuite() external { + DeploySuite[] memory suites = allSuites(); + assertEq(suites[1].suite, "second-address"); + assertNotEq(keccak256(suites[0].creationCode), keccak256(suites[1].creationCode)); + + vm.mockCall( + LibRainDeploy.ZOLTU_FACTORY, suites[1].creationCode, abi.encodePacked(bytes20(LibRainDeploy.ZOLTU_FACTORY)) + ); + + vm.expectRevert( + abi.encodeWithSelector( + ZoltuDerivationMismatch.selector, + "second-address", + suites[1].storedDeployedAddress, + LibRainDeploy.ZOLTU_FACTORY + ) + ); + this.testSnapshotInternallyConsistent(); + } + + /// The derivation MUST clear the derived address's NONCE, not only its + /// code. + /// + /// `CREATE2` collides on a non-zero nonce exactly as it does on non-empty + /// code, and the nonce is the half that leaves nothing to see: an account + /// with a nonce and no code reads as empty everywhere else, so a derivation + /// that cleared only the code would report a deployment failure for a + /// creation code that deploys perfectly well. The Zoltu factory is + /// permissionless, so whether anything has ever transacted from the address + /// a suite derives is nobody's to control. + function testDerivationClearsTheDerivedNonce() external { + DeploySuite memory suite = consistentSuite(); + vm.setNonce(suite.storedDeployedAddress, 1); + assertEq(vm.getNonce(suite.storedDeployedAddress), 1); + assertEq(suite.storedDeployedAddress.code.length, 0); + + this.externalCheckInternallyConsistent(suite); + } + /// The derivation MUST leave nothing behind. A local deploy that survived /// would be compared against itself by the chain-anchored group, and every /// network would pass whether or not anything is deployed there. diff --git a/test/src/lib/LibRainDeploy.t.sol b/test/src/lib/LibRainDeploy.t.sol index 1a3c258..5a3b0bd 100644 --- a/test/src/lib/LibRainDeploy.t.sol +++ b/test/src/lib/LibRainDeploy.t.sol @@ -10,6 +10,8 @@ import {MockAddressRevertingFactory} from "../../concrete/MockAddressRevertingFa import {MockResolvedOwner} from "../../concrete/MockResolvedOwner.sol"; import {MockDirtyWordOwner} from "../../concrete/MockDirtyWordOwner.sol"; import {MockRawAnswerOwner} from "../../concrete/MockRawAnswerOwner.sol"; +import {MockRevertingAnswerOwner} from "../../concrete/MockRevertingAnswerOwner.sol"; +import {MockChainDependentOwner} from "../../concrete/MockChainDependentOwner.sol"; import {MockDeployable} from "../../concrete/MockDeployable.sol"; import {MockDeployableV2} from "../../concrete/MockDeployableV2.sol"; import {MockReverter} from "../../concrete/MockReverter.sol"; @@ -1009,6 +1011,34 @@ contract LibRainDeployTest is Test { this.externalCheckResolvedAddresses("test_network", address(consumer), readCalls, expected(account)); } + /// A read that REVERTS carrying exactly the bytes a successful answer would + /// have carried MUST be refused. + /// + /// Nothing in the payload separates the two: same length, same word, same + /// address, and it is the address the check is looking for. Only whether + /// the call SUCCEEDED does — which is why the success flag is part of the + /// condition rather than left to the length check, and this is the one case + /// where the length check has nothing to say. It is not a contrived + /// payload: a custom error with one address argument is exactly this shape, + /// and so is any revert that bubbles a callee's return data. + /// + /// The reverting read above carries EMPTY data, so it fails the length + /// check and says nothing about the success flag. + function testCheckResolvedAddressesRevertingAddressAnswerReverts(address account) external { + MockRevertingAnswerOwner target = new MockRevertingAnswerOwner(abi.encode(account)); + + vm.expectRevert( + abi.encodeWithSelector( + LibRainDeploy.ResolvedAddressReadFailed.selector, + "test_network", + address(target), + uint256(0), + abi.encode(account) + ) + ); + this.externalCheckResolvedAddresses("test_network", address(target), ownerReadCalls(), expected(account)); + } + /// A read that answers with one word whose upper 96 bits are dirty has not /// answered with an address, and MUST be reported as /// `ResolvedAddressReadFailed` — the error whose stated subject is a read @@ -1236,6 +1266,42 @@ contract LibRainDeployTest is Test { this.externalCheckResolvedAddressesOnNetworks(networks, address(consumer), ownerReadCalls(), expected(wrong)); } + /// `checkResolvedAddressesOnNetworks` MUST advance past the first network. + /// + /// The passing case above is a deployment that answers the same thing + /// everywhere, which a loop that only ever forked the first network passes + /// just as happily — and the mismatch case above is one network, so it + /// cannot tell them apart either. What separates them is a target that + /// answers differently on a LATER network, which is exactly the deployment + /// this matrix exists for: one chain of five holding a value nobody looked + /// at. + /// + /// The first network is the one the target agrees on, so nothing fails + /// before the loop has to advance, and the failure names the SECOND network + /// rather than a fixed one. + function testCheckResolvedAddressesOnNetworksReachesEveryNetwork() external { + address account = address(0xf00); + address wrong = address(0xba4); + MockChainDependentOwner target = new MockChainDependentOwner(account, wrong, ARBITRUM_ONE_CHAIN_ID); + vm.makePersistent(address(target)); + + string[] memory networks = new string[](2); + networks[0] = LibRainDeploy.ARBITRUM_ONE; + networks[1] = LibRainDeploy.BASE; + + vm.expectRevert( + abi.encodeWithSelector( + LibRainDeploy.UnexpectedResolvedAddress.selector, + LibRainDeploy.BASE, + address(target), + uint256(0), + account, + wrong + ) + ); + this.externalCheckResolvedAddressesOnNetworks(networks, address(target), ownerReadCalls(), expected(account)); + } + /// `deployToNetworks` MUST deploy when every dependency has code on the /// network, i.e. a present dependency is not treated as missing. function testDeployToNetworksPresentDependencyDeploys() external {