Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ relocated.
- **`slither.config.json` filters those abstracts by exact filename, never by
the `src/abstract/` prefix.** Keep it that way when adding an abstract: a
prefix filter would silently exempt a deployable file added there later.
- **A `vm.createSelectFork` failure is not a missing deployment.** It is an
unreachable or rate-limited endpoint. Only `NotDeployedOnNetwork`, from a
network that forked, says anything about the deployment.
- **A `vm.createFork` or `vm.createSelectFork` failure is not a missing
deployment.** It is an unreachable or rate-limited endpoint. Only
`NotDeployedOnNetwork`, from a network that forked, says anything about the
deployment. The deploy and the chain matrix create every fork before selecting
any, so such a failure takes the whole run before any network is checked
rather than stopping partway down the list.
6 changes: 2 additions & 4 deletions src/abstract/RainDeployVerifyChain.sol
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,9 @@ abstract contract RainDeployVerifyChain is RainDeployVerifyBase {
}

string[] memory networks = LibRainDeploy.supportedNetworks();
uint256[] memory forkIds = LibRainDeploy.createForks(vm, networks);
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.
uint256 forkId = vm.createSelectFork(networks[i]);
(forkId);
vm.selectFork(forkIds[i]);
for (uint256 j = 0; j < derived.length; j++) {
checkDeployedOnNetwork(networks[i], derived[j]);
}
Expand Down
48 changes: 38 additions & 10 deletions src/lib/LibRainDeploy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,33 @@ library LibRainDeploy {
return deployedAddress;
}

/// Creates a fork for every network, returning their ids in list order.
/// Nothing is selected here: the caller selects each in turn.
///
/// Creating them all BEFORE the first one is selected is the whole point.
/// Foundry captures the account set of the pre-fork EVM when the first fork
/// is selected, and seeds every fork CREATED after that capture with it, so
/// any address the calling script touched beforehand is carried onto the
/// second and later networks as the empty account the default 31337 EVM has
/// for it. A `dep.code.length` read added to a deploy script for logging is
/// enough. Forks created before the capture read their chain, which is why
/// the first network was always right and every one after it was wrong.
///
/// Closed here rather than by a rule about what a deploy script may read,
/// because the trap is invisible from where a consumer sits: the read is
/// ordinary, the failure names a real address on a network that really has
/// it, and nothing connects the two.
/// @param vm The Vm instance to fork with.
/// @param networks The network names to fork, as `[rpc_endpoints]` aliases.
/// @return The fork id of each network, positionally paired.
function createForks(Vm vm, string[] memory networks) internal returns (uint256[] memory) {
uint256[] memory forkIds = new uint256[](networks.length);
for (uint256 i = 0; i < networks.length; i++) {
forkIds[i] = vm.createFork(networks[i]);
}
return forkIds;
}

/// Returns the list of networks currently supported by Rain deployments.
/// @return The list of supported network names.
function supportedNetworks() internal pure returns (string[] memory) {
Expand Down Expand Up @@ -390,11 +417,9 @@ library LibRainDeploy {
if (readCalls.length == 0) {
revert NoResolvedAddressReads(target);
}
uint256[] memory forkIds = createForks(vm, networks);
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.
uint256 forkId = vm.createSelectFork(networks[i]);
(forkId);
vm.selectFork(forkIds[i]);
console2.log("Checking resolved addresses on network:", networks[i]);
checkResolvedAddresses(networks[i], target, readCalls, expectedAddresses);
}
Expand All @@ -415,8 +440,13 @@ library LibRainDeploy {
/// already-deployed dependency as missing and abort an otherwise-valid
/// deploy. Each network is handled independently: the Zoltu deploy is
/// idempotent (an existing contract is skipped), so a failure on one network
/// leaves the others intact and the script can simply be re-run, which is why
/// no separate all-network pre-flight is needed.
/// leaves the others intact and the script can simply be re-run.
///
/// The forks themselves are all created up front, before any is selected —
/// `createForks` says why it has to be that way round. Endpoint reachability
/// is therefore the one thing that IS all-network: an alias that cannot be
/// forked stops the run before anything is broadcast, rather than partway
/// through it.
/// @param vm The Vm instance to use for forking and broadcasting.
/// @param networks The list of network names to deploy to.
/// @param deployer The deployer address.
Expand Down Expand Up @@ -450,11 +480,9 @@ library LibRainDeploy {
if (derivedAddress != expectedAddress) {
revert UnexpectedDeployedAddress(expectedAddress, derivedAddress);
}
uint256[] memory forkIds = createForks(vm, networks);
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.
uint256 forkId = vm.createSelectFork(networks[i]);
(forkId);
vm.selectFork(forkIds[i]);
console2.log("Deploying to network:", networks[i]);
console2.log("Block number:", block.number);

Expand Down
33 changes: 33 additions & 0 deletions test/src/abstract/RainDeployVerifyChain.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,39 @@ contract RainDeployVerifyChainTest is ExampleDeploySuites, RainDeployVerifyChain
this.testSuitesLiveOnEverySupportedNetwork();
}

/// The matrix MUST create every fork BEFORE it checks anything on any of
/// them, for the reason `LibRainDeploy.createForks` gives: a fork created
/// after the first select is seeded with whatever was read before it, so a
/// deployed address the caller touched first would read as absent on every
/// network but the one the matrix reaches first.
///
/// A run that fails on that FIRST network is what makes the order visible.
/// The matrix never reached the last supported network, so a fork of it
/// exists only if it was created up front.
function testChainMatrixCreatesEveryForkFirst() external {
vm.etch(ADDRESS_REGISTRY_DEPLOYED_ADDRESS, hex"");

vm.expectRevert(
abi.encodeWithSelector(
NotDeployedOnNetwork.selector,
LibRainDeploy.ARBITRUM_ONE,
"address-registry-0-0-1",
ADDRESS_REGISTRY_DEPLOYED_ADDRESS
)
);
this.testSuitesLiveOnEverySupportedNetwork();

// This test forks nothing of its own, so the matrix's forks are ids 0
// upwards in `supportedNetworks()` order. Selecting the last of them at
// all is the assertion; the chain ids say it is a different network
// from the one the failure named, rather than another fork of it.
uint256 lastNetwork = LibRainDeploy.supportedNetworks().length - 1;
vm.selectFork(0);
uint256 firstChainId = block.chainid;
vm.selectFork(lastNetwork);
assertNotEq(block.chainid, firstChainId);
}

/// EVERY suite MUST be checked, not just the first one the matrix
/// reaches. The version missing here is the LAST one, so a matrix that
/// stopped after the first version would pass.
Expand Down
145 changes: 145 additions & 0 deletions test/src/lib/LibRainDeploy.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,67 @@ contract LibRainDeployTest is Test {
assertEq(LibRainDeploy.ZOLTU_FACTORY.codehash, LibRainDeploy.ZOLTU_FACTORY_CODEHASH);
}

/// `createForks` MUST create every fork and select NONE of them, because
/// creating them all before the first select is the whole of what it is for.
///
/// Foundry captures the account set of the pre-fork EVM when the first fork
/// is SELECTED, and seeds every fork created after that capture with it. A
/// `createForks` that selected as it went would put every fork but the first
/// on the wrong side of that capture, which is the defect it exists to
/// remove.
function testCreateForksSelectsNothing() external {
string[] memory networks = new string[](2);
networks[0] = LibRainDeploy.BASE;
networks[1] = LibRainDeploy.ARBITRUM_ONE;

uint256[] memory forkIds = LibRainDeploy.createForks(vm, networks);
assertEq(forkIds.length, 2);

// `activeFork()` reverts when nothing is selected, so the call failing
// is the assertion. Made low level for that reason: a plain `vm` call
// would take the test down with it.
(bool active,) = address(vm).call(abi.encodeWithSignature("activeFork()"));
assertFalse(active, "createForks selected a fork");

// Every id it handed back is a real fork of the network in that
// position, so nothing was skipped or forked twice.
vm.selectFork(forkIds[0]);
assertEq(block.chainid, BASE_CHAIN_ID);
vm.selectFork(forkIds[1]);
assertEq(block.chainid, ARBITRUM_ONE_CHAIN_ID);
}

/// A read the caller made BEFORE any fork existed MUST NOT decide what a
/// `createForks` fork holds, on any network.
///
/// The read here is the one a deploy script makes for logging, and on the
/// default 31337 EVM it answers zero for an address that is on every chain
/// this repo deploys to. Forks created after the first select are seeded
/// with that answer, so the whole difference between a network that reads
/// its chain and one that reports the Zoltu factory missing is the order
/// the forks were created in.
function testCreateForksIgnoresAPreForkRead() external {
// The poisoning read, before anything is forked.
assertEq(LibRainDeploy.ZOLTU_FACTORY.code.length, 0);

string[] memory networks = new string[](2);
networks[0] = LibRainDeploy.BASE;
networks[1] = LibRainDeploy.ARBITRUM_ONE;

uint256[] memory forkIds = LibRainDeploy.createForks(vm, networks);

// The SECOND network is the one that reads the pre-fork answer when the
// forks are created as the loop reaches them. Both are asserted so a
// first network that broke would be seen as well.
vm.selectFork(forkIds[0]);
assertEq(block.chainid, BASE_CHAIN_ID);
assertEq(LibRainDeploy.ZOLTU_FACTORY.codehash, LibRainDeploy.ZOLTU_FACTORY_CODEHASH);

vm.selectFork(forkIds[1]);
assertEq(block.chainid, ARBITRUM_ONE_CHAIN_ID);
assertEq(LibRainDeploy.ZOLTU_FACTORY.codehash, LibRainDeploy.ZOLTU_FACTORY_CODEHASH);
}

/// External wrapper for `deployAndBroadcast` so that
/// `vm.expectRevert` works at the correct call depth.
/// @param networks The list of network names to deploy to.
Expand Down Expand Up @@ -323,6 +384,50 @@ contract LibRainDeployTest is Test {
assertEq(result.codehash, mockDeployableCodeHash());
}

/// A read of a declared dependency BEFORE any fork exists MUST NOT change
/// what `deployToNetworks` sees on any network.
///
/// This is rainlanguage/rain.deploy#157 end to end. A deploy script that
/// logs `dependencies[i].code.length` executes that read on the default
/// 31337 EVM, where the account is empty, and every fork created after the
/// first select is seeded with it — so the first network read its chain and
/// every one after it reverted `MissingDependency` against a dependency
/// that demonstrably has code there. It cost several failed production
/// deploy runs to diagnose, because the check itself is sound and the
/// address it names is real.
///
/// The dependency is the Zoltu factory because it is the one address this
/// repo knows is live on every supported network, so the second network
/// disagreeing with the first can only be the defect and never the chain.
function testDeployToNetworksIgnoresAPreForkDependencyRead() external {
// The read a deploy script makes for logging, before anything forks.
assertEq(LibRainDeploy.ZOLTU_FACTORY.code.length, 0);

string[] memory networks = new string[](2);
networks[0] = LibRainDeploy.BASE;
networks[1] = LibRainDeploy.ARBITRUM_ONE;

address[] memory dependencies = new address[](1);
dependencies[0] = LibRainDeploy.ZOLTU_FACTORY;

address result = this.externalDeployToNetworks(
networks,
address(this),
type(MockDeployable).creationCode,
"test/concrete/MockDeployable.sol:MockDeployable",
mockDeployableAddress(),
mockDeployableCodeHash(),
dependencies
);
assertEq(result, mockDeployableAddress());

// The second network is the one the pre-fork read used to take out, and
// a deploy landed on it.
vm.selectFork(1);
assertEq(block.chainid, ARBITRUM_ONE_CHAIN_ID);
assertEq(result.codehash, mockDeployableCodeHash());
}

/// External wrapper for `deployToNetworks` so that `vm.expectRevert`
/// works at the correct call depth.
/// @param networks The list of network names to deploy to.
Expand Down Expand Up @@ -1251,6 +1356,46 @@ contract LibRainDeployTest is Test {
this.externalCheckResolvedAddressesOnNetworks(networks, address(target), ownerReadCalls(), expected(account));
}

/// `checkResolvedAddressesOnNetworks` MUST create every fork BEFORE it
/// checks anything on any of them, for the reason `createForks` gives: a
/// fork created after the first select is seeded with whatever the caller
/// read before the call, so a target it logged would read as having no code
/// on every network but the first.
///
/// A run that fails on the FIRST network is what makes the order visible
/// from outside. The loop never reached the second network, so a fork of it
/// exists only if it was created up front.
function testCheckResolvedAddressesOnNetworksCreatesEveryForkFirst() external {
bytes32 name = keccak256("testCheckResolvedAddressesOnNetworksCreatesEveryForkFirst");
address account = address(0xf00);
address wrong = address(0xba4);
(, MockResolvedOwner consumer) = deployRegistryAndConsumer(name, account);
vm.makePersistent(address(consumer));

string[] memory networks = new string[](2);
networks[0] = LibRainDeploy.ARBITRUM_ONE;
networks[1] = LibRainDeploy.BASE;

vm.expectRevert(
abi.encodeWithSelector(
LibRainDeploy.UnexpectedResolvedAddress.selector,
LibRainDeploy.ARBITRUM_ONE,
address(consumer),
uint256(0),
wrong,
account
)
);
this.externalCheckResolvedAddressesOnNetworks(networks, address(consumer), ownerReadCalls(), expected(wrong));

// This test forks nothing of its own, so ids 0 and 1 are the call's, in
// the order it was given. Selecting id 1 at all is the assertion; the
// chain id says it is the second network rather than another fork of
// the first.
vm.selectFork(1);
assertEq(block.chainid, BASE_CHAIN_ID);
}

/// `deployToNetworks` MUST deploy when every dependency has code on the
/// network, i.e. a present dependency is not treated as missing.
function testDeployToNetworksPresentDependencyDeploys() external {
Expand Down
Loading