From 89ee286c43f3d7a5c53f847df20f37eb266eed21 Mon Sep 17 00:00:00 2001 From: David Meister Date: Sun, 27 Sep 2026 18:37:24 +0000 Subject: [PATCH 1/6] audit: take deploy logic and network config from upstream MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follows the repo audit for places that re-derive what `rain-deploy` already owns. Upstream is the single source of truth; each of these had a local second spelling of it that could drift. Prod checks walk `LibRainDeploy.supportedNetworks()` `LibDecimalFloatDeployProd.t.sol` held its own hardcoded network list, so a network added upstream was a network the prod check silently skipped. It now walks the upstream list, asserts the list is non-empty (an empty one would pass every assertion by never running one), and aggregates per-network failures instead of aborting on the first, so one run names every network that is wrong rather than the alphabetically-first. `reasonOf` decodes the revert payload to text so a CI reader gets "arbitrum: " rather than a hex blob, falling back to hex for anything that is not a 4-byte selector plus an ABI encoded string rather than reverting inside the handler and losing the other networks with it. Deterministic deployment goes through the factory Four sites hand-rolled `create(0, ...)` where `LibRainDeploy.deployZoltu` already deploys deterministically, three of them then etching the runtime across to the pinned address by hand. That is what the factory does, and weaker: an etch writes the runtime wherever it is told, so it keeps passing when the creation code and the pinned address have diverged. The factory landing somewhere other than the pin is now a failure. `test/abstract/LogTest.sol` keeps its raw `create` and says why: it wants a SECOND copy at an address that is deliberately not the pin, so that tests taking a log-tables address as an argument can distinguish a passed argument from the pinned constant. `deployZoltu` cannot serve, because for this creation code it lands on the pin, which `setUp` has occupied. Etherscan config carries chain ids Five `[etherscan]` entries had neither `chain` nor `url`, which is what `EtherscanEntryUnresolvable("arbitrum")` was reporting. All nine now carry `chain`. This was misread as a missing CI secret twice before the config was read. Generated path is derived, not respelled `script/Build.sol` spelled `src/generated` for the log tables while every other write in the file takes it from `recordRoot()`. It was the one path upstream did not own: it would keep writing to the old place if `LIB_FS_ROOT` moved, and would ignore a `recordRoot()` override, which exists so `cutRelease()` can run against a record other than the repo's. Now `GENERATED_LOG_TABLES_LEAF` is a leaf name joined to `recordRoot()`. `LibEtchLogTables`'s docstring named `script/Deploy.sol` as a consumer. It is not one and must not be: a chain without the tables is a chain the suite must not deploy to. The four real callers are named instead. `forge test --force`: 42 suites, 75 passed, 3 failed — the 3 are `testSuitesLiveOnEverySupportedNetwork`, `testSupportedNetworkChainIdsAreBound` and the prod walk, all failing only on missing `*_RPC_URL` env vars, which CI supplies. Identical to the pre-change baseline. `forge fmt --check` and `forge lint -D warnings` clean. Co-Authored-By: Claude Opus 5 (1M context) --- foundry.toml | 28 +++++--- script/Build.sol | 16 +++-- script/lib/LibEtchLogTables.sol | 43 ++++++++--- test/abstract/LogTest.sol | 12 ++++ ...alFloatDeploy.checkLogTablesDeployed.t.sol | 30 ++++---- .../lib/deploy/LibDecimalFloatDeploy.t.sol | 11 +-- .../deploy/LibDecimalFloatDeployProd.t.sol | 71 +++++++++++++++---- 7 files changed, 155 insertions(+), 56 deletions(-) diff --git a/foundry.toml b/foundry.toml index 56e8c17..b56234d 100644 --- a/foundry.toml +++ b/foundry.toml @@ -86,7 +86,7 @@ forge-std = "1.16.2" recursive_deps = false # One alias per network `script/Deploy.sol` broadcasts to, which is -# `LibRainDeploy.supportedNetworks()` — all seven. A network the script +# `LibRainDeploy.supportedNetworks()`. A network the script # selects but this section does not name is not a skip: `vm.createSelectFork` # reverts with "invalid rpc url: " and takes the whole deploy down, # after it has already broadcast to the networks ahead of it in the list. @@ -106,17 +106,23 @@ polygon = "${POLYGON_RPC_URL}" # fails with no API key configured for the chain. One entry per # `[rpc_endpoints]` alias, because the deploy goes to all of them. # -# "chain" is stated on ethereum and hyperevm because foundry does not know -# those alias strings as chain names — its own names for them are "mainnet" -# and "hyperliquid" — so without it the entry cannot be matched to the chain -# being verified against. The other five aliases foundry resolves itself. -# `forge config` prints the resolved chain per entry. +# "chain" is stated on EVERY entry, not only the aliases foundry cannot resolve +# itself. `rain-deploy`'s `checkEtherscanEntriesResolvable` requires it, because +# the set foundry resolves is foundry's own table and moves under a toolchain +# bump — and foundry resolves the whole section rather than the single entry a +# `--verify` needs, so one unresolvable entry fails the verify for every network. +# Stating a chain an alias already resolves to resolves it to the same chain, so +# the strict form only ever agrees with the loose one. +# +# The ids are verified rather than trusted: `testSupportedNetworkChainIdsAreBound` +# forks each network and compares. `forge config` prints the resolved chain per +# entry. [etherscan] -arbitrum = { key = "${CI_DEPLOY_ARBITRUM_ETHERSCAN_API_KEY}" } -base = { key = "${CI_DEPLOY_BASE_ETHERSCAN_API_KEY}" } -base_sepolia = { key = "${CI_DEPLOY_BASE_SEPOLIA_ETHERSCAN_API_KEY}" } +arbitrum = { key = "${CI_DEPLOY_ARBITRUM_ETHERSCAN_API_KEY}", chain = 42161 } +base = { key = "${CI_DEPLOY_BASE_ETHERSCAN_API_KEY}", chain = 8453 } +base_sepolia = { key = "${CI_DEPLOY_BASE_SEPOLIA_ETHERSCAN_API_KEY}", chain = 84532 } ethereum = { key = "${CI_DEPLOY_ETHEREUM_ETHERSCAN_API_KEY}", chain = 1 } -flare = { key = "${CI_DEPLOY_FLARE_ETHERSCAN_API_KEY}" } +flare = { key = "${CI_DEPLOY_FLARE_ETHERSCAN_API_KEY}", chain = 14 } hyperevm = { key = "${CI_DEPLOY_HYPEREVM_ETHERSCAN_API_KEY}", chain = 999 } # Robinhood Chain (4663) is not indexed by Etherscan V2. Its Blockscout speaks # the Etherscan API, so the entry points there (the key is ignored). That @@ -125,4 +131,4 @@ hyperevm = { key = "${CI_DEPLOY_HYPEREVM_ETHERSCAN_API_KEY}", chain = 999 } # sourcify --chain 4663 ...`. robinhood = { key = "${CI_DEPLOY_ROBINHOOD_ETHERSCAN_API_KEY}", chain = 4663, url = "https://robinhoodchain.blockscout.com/api" } bsc = { key = "${CI_DEPLOY_BSC_ETHERSCAN_API_KEY}", chain = 56 } -polygon = { key = "${CI_DEPLOY_POLYGON_ETHERSCAN_API_KEY}" } +polygon = { key = "${CI_DEPLOY_POLYGON_ETHERSCAN_API_KEY}", chain = 137 } diff --git a/script/Build.sol b/script/Build.sol index 3095a17..0bba171 100644 --- a/script/Build.sol +++ b/script/Build.sol @@ -9,21 +9,29 @@ import {LibRainDeploySnapshot} from "rain-deploy-0.1.11/src/lib/LibRainDeploySna import {DeployCandidate} from "../src/abstract/RainDeploySuitesBase.sol"; import {DecimalFloatDeploySuites} from "../src/abstract/DecimalFloatDeploySuites.sol"; -/// @dev Committed path of the generated log tables. The `.pointers.sol` suffix +/// @dev Committed LEAF NAME of the generated log tables, under `recordRoot()`. +/// +/// The directory is not spelled here. Every other write in this script takes it +/// from `recordRoot()`, and spelling `src/generated` again would make this the +/// one path upstream does not own: it would keep writing to the old place if +/// `LIB_FS_ROOT` moved, and would ignore a `recordRoot()` override, which exists +/// so `cutRelease()` can be exercised against a record other than the repo's own. +/// +/// The `.pointers.sol` suffix /// is the one the deploy pins and importers reference. The file is pure /// log-table data with no contract instance behind it, so it carries no /// bytecode-hash constant and is written directly rather than through /// `LibFs.buildFileForContract`, which heads every file it writes with the hash /// of an instance and reverts on the codeless `address(0)` this build has. /// -/// It sits at the root of `src/generated/` rather than in a snapshot directory +/// It sits at the root of the record rather than in a snapshot directory /// because it is not a deploy record: it holds the table BYTES, which are a /// pure function of `LibLogTable` and carry no address, code hash or tag. The /// deploy record of the data contract those bytes are wrapped in is /// `src/generated/candidate/LogTables.sol`, and only tag-shaped directories /// under this root are releases — a loose file here is one neither /// `frozenSnapshotPaths` nor `release-guard` reads as a snapshot. -string constant GENERATED_LOG_TABLES = "src/generated/LogTables.pointers.sol"; +string constant GENERATED_LOG_TABLES_LEAF = "LogTables.pointers.sol"; /// One contract's generated files: the rolling snapshot and the released-suites /// lib emitted from its record. @@ -84,7 +92,7 @@ contract Build is BuildScript, DecimalFloatDeploySuites { function regenerateLogTables() internal { //forge-lint: disable-next-line(unsafe-cheatcode) vm.writeFile( - GENERATED_LOG_TABLES, + string.concat(recordRoot(), "/", GENERATED_LOG_TABLES_LEAF), string.concat( LibCodeGen.filePrefix(), LibCodeGen.bytesConstantString( diff --git a/script/lib/LibEtchLogTables.sol b/script/lib/LibEtchLogTables.sol index c9917db..e0f3253 100644 --- a/script/lib/LibEtchLogTables.sol +++ b/script/lib/LibEtchLogTables.sol @@ -5,25 +5,48 @@ pragma solidity ^0.8.25; import {Vm} from "forge-std-1.16.2/src/Vm.sol"; import {LibDataContract} from "rain-datacontract-0.1.9/src/lib/LibDataContract.sol"; import {LibDecimalFloatDeploy} from "src/lib/deploy/LibDecimalFloatDeploy.sol"; +import {LibRainDeploy} from "rain-deploy-0.1.11/src/lib/LibRainDeploy.sol"; /// @notice Shared logic for planting the log-tables data contract at its -/// Zoltu-deterministic address inside a forge VM. Used by `script/Deploy.sol` -/// (so the decimal-float simulation pass can pass the constructor's codehash -/// check before log-tables exists on-chain) and by tests that need +/// Zoltu-deterministic address inside a forge VM, for tests that need /// `DecimalFloat` operations to work without a real on-chain deploy. +/// +/// NOT used by `script/Deploy.sol`, which does not import this and must not: +/// its docstring states that nothing plants the tables locally to get a +/// simulation through, because a chain without them is a chain the suite must +/// not be deployed to — `DecimalFloat`'s constructor reverts there, and +/// `LibRainDeploy.deployToNetworks` raises `MissingDependency` rather than +/// papering over it. An earlier version of this comment claimed the broadcast +/// as a consumer; the broadcast is entirely `RainDeployBroadcast.run()` and has +/// no etch hook to call this from. +/// +/// Callers are `test/abstract/LogTest.sol`, +/// `test/src/concrete/DecimalFloat.constructor.t.sol`, +/// `test/src/abstract/DecimalFloatDeployChain.t.sol` and +/// `test/src/abstract/DecimalFloatDeploySnapshot.t.sol`. library LibEtchLogTables { /// @notice Deploys the log-tables data contract to a temporary address, /// copies its runtime code, and etches that runtime at /// `LibDecimalFloatDeploy.ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS` so the /// codehash matches `LibDecimalFloatDeploy.LOG_TABLES_DATA_CONTRACT_HASH`. + /// Deployed through the Zoltu factory rather than a raw `create` into a + /// temporary address followed by an etch across. The factory is what puts + /// the contract at `ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS` in the first place, + /// so using it makes the address a consequence of the creation code instead + /// of something asserted after the fact — and the temp-address dance existed + /// only because a raw `create` lands at a nonce-dependent address. + /// + /// The returned address is checked against the pin. A deployment that does + /// not land on it means the creation code and the committed snapshot have + /// diverged, which is worth a failure here rather than an etch that hides it + /// by writing the runtime wherever the pin says regardless. function etchLogTables(Vm vm) internal { + LibRainDeploy.etchZoltuFactory(vm); bytes memory tables = LibDecimalFloatDeploy.combinedTables(); - bytes memory creationCode = LibDataContract.contractCreationCode(tables); - address temp; - assembly ("memory-safe") { - temp := create(0, add(creationCode, 0x20), mload(creationCode)) - } - require(temp != address(0), "log tables etch deploy failed"); - vm.etch(LibDecimalFloatDeploy.ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS, temp.code); + address deployed = LibRainDeploy.deployZoltu(LibDataContract.contractCreationCode(tables)); + require( + deployed == LibDecimalFloatDeploy.ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS, + "log tables deployed off their pinned Zoltu address" + ); } } diff --git a/test/abstract/LogTest.sol b/test/abstract/LogTest.sol index 847b6ce..9ba095e 100644 --- a/test/abstract/LogTest.sol +++ b/test/abstract/LogTest.sol @@ -25,6 +25,18 @@ abstract contract LogTest is Test { ); } + /// A SECOND copy of the tables, at a nonce-dependent address that is + /// deliberately not `ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS` — `setUp` has + /// already put a copy there. + /// + /// The raw `create` is the point and not an un-migrated call site. Tests + /// that take a log-tables address as an argument use this one so that + /// passing it is distinguishable from reading the pinned constant: if both + /// addresses were the pin, a function that ignored its argument entirely + /// would still pass. `LibRainDeploy.deployZoltu` cannot serve here, because + /// the address it lands on is a function of the creation code alone, so for + /// this creation code it is the pin — which `setUp` has occupied, making the + /// second deployment fail rather than yield a distinct address. function logTables() internal returns (address) { if (sTables == address(0)) { bytes memory tables = LibDecimalFloatDeploy.combinedTables(); diff --git a/test/src/lib/deploy/LibDecimalFloatDeploy.checkLogTablesDeployed.t.sol b/test/src/lib/deploy/LibDecimalFloatDeploy.checkLogTablesDeployed.t.sol index 52e28ad..656f4cb 100644 --- a/test/src/lib/deploy/LibDecimalFloatDeploy.checkLogTablesDeployed.t.sol +++ b/test/src/lib/deploy/LibDecimalFloatDeploy.checkLogTablesDeployed.t.sol @@ -5,6 +5,7 @@ pragma solidity =0.8.25; import {Test} from "forge-std-1.16.2/src/Test.sol"; import {LibDecimalFloatDeploy} from "src/lib/deploy/LibDecimalFloatDeploy.sol"; import {LibDataContract} from "rain-datacontract-0.1.9/src/lib/LibDataContract.sol"; +import {LibRainDeploy} from "rain-deploy-0.1.11/src/lib/LibRainDeploy.sol"; import {LogTablesNotDeployed} from "rain-math-float-0.2.4/src/error/ErrDecimalFloat.sol"; /// Direct tests for `LibDecimalFloatDeploy.checkLogTablesDeployed`. These @@ -42,15 +43,19 @@ contract LibDecimalFloatDeployCheckLogTablesDeployedTest is Test { } /// Correct table runtime at the expected address → no revert. + /// + /// Through the factory, which lands the deployment on the pin as a + /// consequence of the creation code. An earlier version deployed to a + /// nonce-dependent address and etched the runtime across to the pin, which + /// is what the factory does — reproduced by hand, and weaker for it: an + /// etch writes the runtime wherever it is told, so it would keep passing if + /// the creation code and the pinned address had diverged. The factory not + /// landing on the pin is a failure here instead. function testCheckLogTablesDeployedSucceedsWhenPresent() external { + LibRainDeploy.etchZoltuFactory(vm); bytes memory tables = LibDecimalFloatDeploy.combinedTables(); - bytes memory creationCode = LibDataContract.contractCreationCode(tables); - address temp; - assembly ("memory-safe") { - temp := create(0, add(creationCode, 0x20), mload(creationCode)) - } - require(temp != address(0), "log tables deploy failed in test setup"); - vm.etch(LibDecimalFloatDeploy.ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS, temp.code); + address deployed = LibRainDeploy.deployZoltu(LibDataContract.contractCreationCode(tables)); + assertEq(deployed, LibDecimalFloatDeploy.ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS, "tables off their pinned address"); // Should not revert. this.callCheckLogTablesDeployed(); } @@ -59,16 +64,13 @@ contract LibDecimalFloatDeployCheckLogTablesDeployedTest is Test { /// with same-length zeroes flips the codehash and the call reverts. /// Confirms the test is keyed on the codehash check, not anything else. function testCheckLogTablesDeployedMutation() external { + LibRainDeploy.etchZoltuFactory(vm); bytes memory tables = LibDecimalFloatDeploy.combinedTables(); - bytes memory creationCode = LibDataContract.contractCreationCode(tables); - address temp; - assembly ("memory-safe") { - temp := create(0, add(creationCode, 0x20), mload(creationCode)) - } - vm.etch(LibDecimalFloatDeploy.ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS, temp.code); + address deployed = LibRainDeploy.deployZoltu(LibDataContract.contractCreationCode(tables)); + assertEq(deployed, LibDecimalFloatDeploy.ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS, "tables off their pinned address"); this.callCheckLogTablesDeployed(); - bytes memory zeros = new bytes(temp.code.length); + bytes memory zeros = new bytes(deployed.code.length); vm.etch(LibDecimalFloatDeploy.ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS, zeros); vm.expectRevert(); this.callCheckLogTablesDeployed(); diff --git a/test/src/lib/deploy/LibDecimalFloatDeploy.t.sol b/test/src/lib/deploy/LibDecimalFloatDeploy.t.sol index 527666d..3a781f3 100644 --- a/test/src/lib/deploy/LibDecimalFloatDeploy.t.sol +++ b/test/src/lib/deploy/LibDecimalFloatDeploy.t.sol @@ -54,10 +54,13 @@ contract LibDecimalFloatDeployTest is Test { function testExpectedCodeHashLogTables() external { bytes memory logTables = LibDataContract.contractCreationCode(LibDecimalFloatDeploy.combinedTables()); - address deployedAddress; - assembly ("memory-safe") { - deployedAddress := create(0, add(logTables, 0x20), mload(logTables)) - } + // Through the factory, matching `testDeployAddressLogTables` above + // rather than hand-rolling a second spelling of the same deployment in + // the same file. Only the codehash is under test here, so where the + // deployment lands is incidental — but the runtime code a creation code + // produces is not a function of the deployer, so there is nothing a raw + // `create` tests that this does not. + address deployedAddress = LibRainDeploy.deployZoltu(logTables); assertEq(deployedAddress.codehash, LibDecimalFloatDeploy.LOG_TABLES_DATA_CONTRACT_HASH); assertTrue(address(deployedAddress).code.length > 0, "Deployed address has no code"); diff --git a/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol b/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol index 88153e7..208ed32 100644 --- a/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol +++ b/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol @@ -31,23 +31,68 @@ contract LibDecimalFloatDeployProdTest is Test { ); } - function testProdDeploymentArbitrum() external { - checkProdDeployment("arbitrum"); + /// EVERY supported network, taken from `LibRainDeploy.supportedNetworks()` + /// rather than named here. + /// + /// The five names this file used to list were a subset: `bsc`, `ethereum`, + /// `hyperevm` and `robinhood` are supported, are configured in + /// `[rpc_endpoints]` and `[etherscan]`, and were never checked. A hardcoded + /// list cannot notice that, which is why the list is the source and this + /// walks it — a network added upstream is covered here without an edit, and + /// one removed stops being checked without a stale test failing. + /// + /// `testSupportedNetworksAreFullyConfigured` guards the config against the + /// same list, so config and coverage now derive from one place. + /// Each network is checked through an external call so a revert on one does + /// not abort the walk. The five test functions this replaced reported per + /// network; a bare loop would hide every network after the first failure, + /// which is the opposite of useful when the question is WHICH chains are + /// missing a deployment. + function checkProdDeploymentExternal(string memory network) external { + checkProdDeployment(network); } - function testProdDeploymentBase() external { - checkProdDeployment("base"); + /// The revert reason as text, so the failure message names what went wrong + /// rather than handing a CI reader a hex blob to decode. + /// + /// A failed assertion and a cheatcode error both carry a 4 byte selector + /// then an ABI encoded string, so both decode the same way. Anything with a + /// different shape falls back to hex rather than reverting inside the + /// handler and losing every other network's result with it. + function reasonOf(bytes memory err) internal pure returns (string memory) { + if (err.length < 68) { + return vm.toString(err); + } + bytes memory payload = new bytes(err.length - 4); + for (uint256 i = 0; i < payload.length; i++) { + payload[i] = err[i + 4]; + } + // The offset word of an ABI encoded string is always 0x20; a payload + // that does not start with it is not one. + // + // Truncating to the first word is the entire point of the cast: the + // rest of the payload is the string this is deciding whether to decode. + // forge-lint: disable-next-line(unsafe-typecast) + bytes32 offsetWord = bytes32(payload); + if (uint256(offsetWord) != 0x20) { + return vm.toString(err); + } + return abi.decode(payload, (string)); } - function testProdDeploymentBaseSepolia() external { - checkProdDeployment("base_sepolia"); - } - - function testProdDeploymentFlare() external { - checkProdDeployment("flare"); - } + function testProdDeploymentEverySupportedNetwork() external { + string[] memory networks = LibRainDeploy.supportedNetworks(); + // A list that came back empty would pass every assertion below by + // never running one. + assertTrue(networks.length > 0, "no supported networks"); - function testProdDeploymentPolygon() external { - checkProdDeployment("polygon"); + string memory failures = ""; + for (uint256 i = 0; i < networks.length; i++) { + try this.checkProdDeploymentExternal(networks[i]) {} + catch (bytes memory err) { + failures = string.concat(failures, "\n", networks[i], ": ", reasonOf(err)); + } + } + assertEq(failures, "", failures); } } From a7ddd440814f37f91ecca2ca0e7e861eb2593751 Mon Sep 17 00:00:00 2001 From: David Meister Date: Sun, 27 Sep 2026 18:44:52 +0000 Subject: [PATCH 2/6] audit: name the config guard that actually exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prod walk's docstring credited `testSupportedNetworksAreFullyConfigured` for guarding the network config. No such test exists — the name was written into the comment without being checked. The real guard is upstream's `testSupportedNetworkChainIdsAreBound` in `RainDeployVerifyChain`, inherited here through `test/src/abstract/DecimalFloatDeployChain.t.sol`. It forks each network that states a chain id, which is why it is one of the suites that needs `*_RPC_URL` to run. Co-Authored-By: Claude Opus 5 (1M context) --- test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol b/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol index 208ed32..fcf5c01 100644 --- a/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol +++ b/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol @@ -41,8 +41,9 @@ contract LibDecimalFloatDeployProdTest is Test { /// walks it — a network added upstream is covered here without an edit, and /// one removed stops being checked without a stale test failing. /// - /// `testSupportedNetworksAreFullyConfigured` guards the config against the - /// same list, so config and coverage now derive from one place. + /// Upstream's `testSupportedNetworkChainIdsAreBound`, inherited through + /// `test/src/abstract/DecimalFloatDeployChain.t.sol`, guards the config + /// against the same list, so config and coverage now derive from one place. /// Each network is checked through an external call so a revert on one does /// not abort the walk. The five test functions this replaced reported per /// network; a bare loop would hide every network after the first failure, From 7734ce5c6a24229d9de31a741194a4f4f9c293d0 Mon Sep 17 00:00:00 2001 From: David Meister Date: Sun, 27 Sep 2026 18:50:46 +0000 Subject: [PATCH 3/6] audit: restore the config guard reference, and stop doubling the network name Retracts a7ddd44, which was wrong. That commit claimed `testSupportedNetworksAreFullyConfigured` does not exist and removed the docstring's reference to it. It does exist. It is `RainDeployVerifySnapshot.testSupportedNetworksAreFullyConfigured`, inherited here through `test/src/abstract/DecimalFloatDeploySnapshot.t.sol`. The grep behind that claim searched only this repo's `src` and `test`, not `dependencies/`, so an inherited test read as a missing one. The CI run on this branch is what exposed it: the name appears in main's failure list as `[FAIL: EtherscanEntryUnresolvable("arbitrum")]`. The docstring now names both upstream guards and says which is which: `testSupportedNetworksAreFullyConfigured` reads `foundry.toml` and needs no fork, so it catches a missing `[rpc_endpoints]` or `[etherscan]` entry locally; `testSupportedNetworkChainIdsAreBound` forks and needs `*_RPC_URL`. That first one is the discriminating test for this PR's `[etherscan]` change, and it runs locally: base's foundry.toml -> [FAIL: EtherscanEntryUnresolvable("arbitrum")] this branch's -> [PASS] Separately, CI printed `flare: flare: DecimalFloat not deployed`. The walk prefixes every collected line with the network and `checkProdDeployment` named it again. The prefix stays with the walk, because a cheatcode failure reverts before any assertion and can only be named by the caller, so the assertion messages drop it. `forge test --force`: 42 suites, 75 passed, 3 failed on missing `*_RPC_URL` as before. fmt and lint clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../deploy/LibDecimalFloatDeployProd.t.sol | 32 ++++++++++++------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol b/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol index fcf5c01..70ed953 100644 --- a/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol +++ b/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol @@ -11,23 +11,25 @@ import {LibRainDeploy} from "rain-deploy-0.1.11/src/lib/LibRainDeploy.sol"; /// contract are deployed on every supported production network with the expected /// addresses and code hashes. contract LibDecimalFloatDeployProdTest is Test { + /// The assertion messages do NOT name the network. The walk below prefixes + /// every line it collects with the network, and naming it here too produced + /// `flare: flare: DecimalFloat not deployed` in CI. The prefix belongs to the + /// walk rather than here, because a cheatcode failure — an unset + /// `*_RPC_URL`, say — reverts before any assertion and so can only be named + /// by the caller. function checkProdDeployment(string memory network) internal { vm.createSelectFork(network); address logTables = LibDecimalFloatDeploy.ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS; - assertTrue(logTables.code.length > 0, string.concat(network, ": log tables not deployed")); + assertTrue(logTables.code.length > 0, "log tables not deployed"); assertEq( - logTables.codehash, - LibDecimalFloatDeploy.LOG_TABLES_DATA_CONTRACT_HASH, - string.concat(network, ": log tables code hash mismatch") + logTables.codehash, LibDecimalFloatDeploy.LOG_TABLES_DATA_CONTRACT_HASH, "log tables code hash mismatch" ); address decimalFloat = LibDecimalFloatDeploy.ZOLTU_DEPLOYED_DECIMAL_FLOAT_ADDRESS; - assertTrue(decimalFloat.code.length > 0, string.concat(network, ": DecimalFloat not deployed")); + assertTrue(decimalFloat.code.length > 0, "DecimalFloat not deployed"); assertEq( - decimalFloat.codehash, - LibDecimalFloatDeploy.DECIMAL_FLOAT_CONTRACT_HASH, - string.concat(network, ": DecimalFloat code hash mismatch") + decimalFloat.codehash, LibDecimalFloatDeploy.DECIMAL_FLOAT_CONTRACT_HASH, "DecimalFloat code hash mismatch" ); } @@ -41,9 +43,17 @@ contract LibDecimalFloatDeployProdTest is Test { /// walks it — a network added upstream is covered here without an edit, and /// one removed stops being checked without a stale test failing. /// - /// Upstream's `testSupportedNetworkChainIdsAreBound`, inherited through - /// `test/src/abstract/DecimalFloatDeployChain.t.sol`, guards the config - /// against the same list, so config and coverage now derive from one place. + /// Two upstream tests guard the CONFIG against the same list, so config and + /// coverage now derive from one place: + /// + /// - `testSupportedNetworksAreFullyConfigured`, in + /// `RainDeployVerifySnapshot` and inherited through + /// `test/src/abstract/DecimalFloatDeploySnapshot.t.sol`. It reads + /// `foundry.toml` and needs no fork, so it is the one that catches a + /// missing `[rpc_endpoints]` or `[etherscan]` entry locally. + /// - `testSupportedNetworkChainIdsAreBound`, in `RainDeployVerifyChain` and + /// inherited through `test/src/abstract/DecimalFloatDeployChain.t.sol`. It + /// forks each network that states a chain id, so it needs `*_RPC_URL`. /// Each network is checked through an external call so a revert on one does /// not abort the walk. The five test functions this replaced reported per /// network; a bare loop would hide every network after the first failure, From ae5bf9ab0eff7a04c008803f84087999aee5d2a7 Mon Sep 17 00:00:00 2001 From: David Meister Date: Sun, 27 Sep 2026 18:56:57 +0000 Subject: [PATCH 4/6] audit: catch a malformed revert payload instead of predicting it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit finding on #31, and it is real. `reasonOf` checked the payload length and the offset word, then called `abi.decode(payload, (string))`. A payload can carry offset `0x20` and still declare a string length its data does not cover, and `abi.decode` reverts on that. `reasonOf` runs inside the walk's `catch`, so that revert does not fall back to hex — it propagates out of `testProdDeploymentEverySupportedNetwork` and takes every other network's result with it. That is the exact failure the hex fallback exists to prevent, reintroduced by the fallback's own implementation. The decode now goes through `this.decodeStringExternal`, so a malformed payload is caught rather than predicted. Bounds arithmetic would have to be right about every malformed shape; `try` is right about all of them by construction. The offset check stays. The `try` would catch its revert, but a custom error whose first word happens to be a valid offset could decode into a garbage string instead of falling back, and a shape check is the only thing that rejects that. Three tests, all passing: testReasonOfDecodesRevertString a well formed Error(string) still decodes to its text testReasonOfSurvivesMalformedStringPayload offset 0x20 + length type(uint256).max falls back to hex rather than reverting testMalformedStringPayloadDoesRevertAbiDecode the same payload really does revert abi.decode, so the test above is not vacuous The third exists because the second alone could pass for the wrong reason: if the payload were quietly decodable, the fallback assertion would hold while proving nothing about the `try`. `forge test --force`: 42 suites, 81 tests, 78 passed, 3 failed on missing `*_RPC_URL` as before. fmt and lint clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../deploy/LibDecimalFloatDeployProd.t.sol | 56 ++++++++++++++++++- 1 file changed, 53 insertions(+), 3 deletions(-) diff --git a/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol b/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol index 70ed953..ef27fbb 100644 --- a/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol +++ b/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol @@ -70,7 +70,22 @@ contract LibDecimalFloatDeployProdTest is Test { /// then an ABI encoded string, so both decode the same way. Anything with a /// different shape falls back to hex rather than reverting inside the /// handler and losing every other network's result with it. - function reasonOf(bytes memory err) internal pure returns (string memory) { + /// + /// The decode goes through an external call so that a malformed payload is + /// CAUGHT rather than merely predicted. The cheap shape checks below cannot + /// be complete: a payload can carry the right offset word and still declare + /// a length its data does not cover, and `abi.decode` reverts on that. This + /// function runs inside the walk's `catch`, so a revert here does not fall + /// back — it propagates and takes every other network's result with it, + /// which is the exact failure the fallback exists to prevent. Bounds + /// arithmetic would have to be right about every malformed shape; `try` is + /// right about all of them by construction. + function decodeStringExternal(bytes memory payload) external pure returns (string memory) { + return abi.decode(payload, (string)); + } + + /// See `decodeStringExternal` for why the decode is an external call. + function reasonOf(bytes memory err) internal view returns (string memory) { if (err.length < 68) { return vm.toString(err); } @@ -79,7 +94,10 @@ contract LibDecimalFloatDeployProdTest is Test { payload[i] = err[i + 4]; } // The offset word of an ABI encoded string is always 0x20; a payload - // that does not start with it is not one. + // that does not start with it is not one. Kept as a shape check even + // though the `try` below would catch the resulting revert, because a + // custom error whose first word happens to be a valid offset could + // otherwise decode into a garbage string instead of falling back. // // Truncating to the first word is the entire point of the cast: the // rest of the payload is the string this is deciding whether to decode. @@ -88,7 +106,39 @@ contract LibDecimalFloatDeployProdTest is Test { if (uint256(offsetWord) != 0x20) { return vm.toString(err); } - return abi.decode(payload, (string)); + try this.decodeStringExternal(payload) returns (string memory reason) { + return reason; + } catch { + return vm.toString(err); + } + } + + /// A well formed `Error(string)` payload decodes to its text, which is the + /// whole reason the walk reports reasons rather than hex. + function testReasonOfDecodesRevertString() external view { + bytes memory err = abi.encodeWithSignature("Error(string)", "arbitrum: nope"); + assertEq(reasonOf(err), "arbitrum: nope"); + } + + /// A payload carrying the right offset word but a length its data does not + /// cover. `abi.decode` reverts on this. Before the decode was moved behind + /// an external call that revert propagated out of the walk's `catch` and + /// took every other network's result with it, so this asserts the fallback + /// rather than the decode. + function testReasonOfSurvivesMalformedStringPayload() external view { + bytes memory err = abi.encodePacked(bytes4(0x08c379a0), uint256(0x20), type(uint256).max); + assertEq(err.length, 68, "payload should be exactly the minimum accepted length"); + assertEq(reasonOf(err), vm.toString(err), "malformed payload should fall back to hex"); + } + + /// The other half of the test above, and the reason it is not vacuous: the + /// same payload really does revert `abi.decode`. Without this, a change that + /// made the payload decodable would leave the fallback test passing for the + /// wrong reason and prove nothing about the `try`. + function testMalformedStringPayloadDoesRevertAbiDecode() external { + bytes memory payload = abi.encodePacked(uint256(0x20), type(uint256).max); + vm.expectRevert(); + this.decodeStringExternal(payload); } function testProdDeploymentEverySupportedNetwork() external { From e436afa41f74456d59181ba74f5f0419f1f32884 Mon Sep 17 00:00:00 2001 From: David Meister Date: Sun, 27 Sep 2026 19:20:55 +0000 Subject: [PATCH 5/6] audit: create every fork before selecting one, and bind the union MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from a final sweep for remaining upstream duplication. Forks are created before any is selected `checkProdDeployment` called `vm.createSelectFork(network)` inside the walk. `LibRainDeploy.createForks` documents why that is wrong: foundry captures the pre-fork account set when the first fork is SELECTED, and seeds every fork created after that capture with it, so such a fork reads an address the caller already touched as the empty account a bare 31337 EVM has for it. A `createSelectFork` loop creates fork N after the capture for every N past the first. The loop was correct only by accident — nothing in this contract touches the pinned addresses before the first select, so the captured set was empty of them. The accident is one edit deep: the sibling `DecimalFloatDeployChainTest` already has exactly the `setUp` that would break it, etching the tables at the pin. With that, eight of nine networks would report `DecimalFloat not deployed` while naming real addresses on chains that hold them. Now two passes: create every fork, then select and check each. Not `LibRainDeploy.createForks`, which is the same creation in one call but reverts on the first unreachable endpoint and so reports no network after it. Creating them one at a time through an external call keeps an outage attributed to its own network, the way a missing deployment already is. The chain test binds `RainDeployVerify`, not `RainDeployVerifyChain` Upstream calls `RainDeployVerify` "the ONE contract a deploy repo binds" and reserves putting a new check directly on the union: "A new check therefore goes into a contract this already inherits, or into this." This repo bound the two halves separately, whose union is the same set TODAY, so a check added to `RainDeployVerify` itself would never arrive and nothing would say so. `DecimalFloatDeploySnapshotTest` keeps binding `RainDeployVerifySnapshot` alone, so that half is now bound twice. Deliberate: the snapshot group forks nothing, and `--match-contract` selects at a contract boundary, so its own contract is what lets a job with no RPC credentials run it. Four extra network-free tests is cheaper than losing either the credential-free run or a future union-level check. The manual deploy workflow describes the real mechanism Its header credited `script/Deploy.sol`'s "DEPLOYMENT_SUITE switch". That file holds no switch — it is a declaration plus `RainDeployBroadcast`, and upstream's `RainDeployBroadcast.run()` resolves the value through `suiteByName` against the keys `DecimalFloatDeploySuites` declares. Same class of error as the `LibEtchLogTables` docstring earlier in this branch: a comment naming a consumer that is not one. It also now says why the suite keys are hand-listed as `options` — a workflow input cannot read them out of Solidity — and that an unlisted suite cannot be dispatched, which fails visibly rather than deploying the wrong thing. Swept and found nothing: Zoltu factory address and codehash, network names and chain ids, snapshot and lib paths, etch and deployZoltu call sites, broadcast suite dispatch, the `RainDeploySuitesBase` shim (required — the generated libs' relative import is emitted upstream), crates and test_js. `forge test --force`: 42 suites, 85 tests, 82 passed, 3 failed on missing `*_RPC_URL` as before. fmt and lint clean. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/manual-sol-artifacts.yaml | 21 +++++-- .../abstract/DecimalFloatDeployChain.t.sol | 25 ++++++-- .../deploy/LibDecimalFloatDeployProd.t.sol | 59 +++++++++++++++++-- 3 files changed, 90 insertions(+), 15 deletions(-) diff --git a/.github/workflows/manual-sol-artifacts.yaml b/.github/workflows/manual-sol-artifacts.yaml index c42e107..21b6a6b 100644 --- a/.github/workflows/manual-sol-artifacts.yaml +++ b/.github/workflows/manual-sol-artifacts.yaml @@ -1,9 +1,20 @@ name: Manual sol artifacts -# The on-chain deploy, human-dispatched, run BEFORE tagging a release. Two -# suites, matching `script/Deploy.sol`'s DEPLOYMENT_SUITE switch: deploy the -# log-tables data contract once per chain, then the DecimalFloat concrete (its -# constructor reads the log tables at the Zoltu address). Never broadcasts on -# merge. +# The on-chain deploy, human-dispatched, run BEFORE tagging a release. Deploy +# the log-tables data contract once per chain, then the DecimalFloat concrete +# (its constructor reads the log tables at the Zoltu address). Never broadcasts +# on merge. +# +# The `suite` input below is passed as DEPLOYMENT_SUITE. `script/Deploy.sol` +# holds no switch on it — it is a declaration plus `RainDeployBroadcast`, and +# upstream's `RainDeployBroadcast.run()` resolves the value through +# `suiteByName` against the keys `src/abstract/DecimalFloatDeploySuites.sol` +# declares. A mistyped key reports the valid ones from there. +# +# Those keys are hand-listed as `options` because a workflow input cannot read +# them out of Solidity. They are the one place in this repo that restates the +# declaration, so a suite added there needs a line added here; an unlisted suite +# simply cannot be dispatched, which fails visibly rather than deploying the +# wrong thing. on: workflow_dispatch: inputs: diff --git a/test/src/abstract/DecimalFloatDeployChain.t.sol b/test/src/abstract/DecimalFloatDeployChain.t.sol index 61cbe53..63c1fd2 100644 --- a/test/src/abstract/DecimalFloatDeployChain.t.sol +++ b/test/src/abstract/DecimalFloatDeployChain.t.sol @@ -2,19 +2,34 @@ // SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd pragma solidity =0.8.25; -import {RainDeployVerifyChain} from "rain-deploy-0.1.11/src/abstract/RainDeployVerifyChain.sol"; +import {RainDeployVerify} from "rain-deploy-0.1.11/src/abstract/RainDeployVerify.sol"; import {DecimalFloatDeploySuites} from "src/abstract/DecimalFloatDeploySuites.sol"; import {LibEtchLogTables} from "script/lib/LibEtchLogTables.sol"; /// @title DecimalFloatDeployChainTest -/// @notice Binds this repo's declaration to `RainDeployVerifyChain`: every -/// frozen release of the log tables and of `DecimalFloat` is live, with the -/// code it froze, on every supported network. +/// @notice Binds this repo's declaration to `RainDeployVerify`: every frozen +/// release of the log tables and of `DecimalFloat` is live, with the code it +/// froze, on every supported network. /// /// The live CURRENT pins are checked by `LibDecimalFloatDeployProdTest`, which /// is a different claim: the candidate is what the NEXT release will be, and a /// released suite is a deployment that already happened. -contract DecimalFloatDeployChainTest is DecimalFloatDeploySuites, RainDeployVerifyChain { +/// +/// `RainDeployVerify` rather than `RainDeployVerifyChain`, because upstream +/// states it is "the ONE contract a deploy repo binds" and reserves the right to +/// put a new check directly on it: a check on the union reaches a repo that +/// bound only the two halves never, and nothing red-lines the repo that misses +/// it. Binding the union here is what makes a version bump deliver it. +/// +/// `DecimalFloatDeploySnapshotTest` still binds `RainDeployVerifySnapshot` +/// alone, which is why that half is bound twice. That is deliberate and is the +/// cheap side of the trade: the snapshot group forks nothing, so binding it in +/// its own contract is what lets a job with no RPC credentials select it with +/// `--match-contract` — selection happens at a contract boundary, so a single +/// combined contract could not offer it. The duplicated runs are network-free +/// and fast; losing the credential-free run, or losing a future union-level +/// check, would both cost more. +contract DecimalFloatDeployChainTest is DecimalFloatDeploySuites, RainDeployVerify { /// Plants the log tables for the same reason the snapshot suite does: the /// derivations all run on the local EVM before anything forks, and /// `DecimalFloat`'s constructor reverts without the tables in place. diff --git a/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol b/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol index ef27fbb..3cf2c63 100644 --- a/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol +++ b/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol @@ -17,8 +17,11 @@ contract LibDecimalFloatDeployProdTest is Test { /// walk rather than here, because a cheatcode failure — an unset /// `*_RPC_URL`, say — reverts before any assertion and so can only be named /// by the caller. - function checkProdDeployment(string memory network) internal { - vm.createSelectFork(network); + /// Takes a fork ID rather than a network name, and SELECTS ONLY. The fork + /// must already exist. See `testProdDeploymentEverySupportedNetwork` for why + /// creating it here would be wrong. + function checkProdDeployment(uint256 forkId) internal { + vm.selectFork(forkId); address logTables = LibDecimalFloatDeploy.ZOLTU_DEPLOYED_LOG_TABLES_ADDRESS; assertTrue(logTables.code.length > 0, "log tables not deployed"); @@ -59,8 +62,14 @@ contract LibDecimalFloatDeployProdTest is Test { /// network; a bare loop would hide every network after the first failure, /// which is the opposite of useful when the question is WHICH chains are /// missing a deployment. - function checkProdDeploymentExternal(string memory network) external { - checkProdDeployment(network); + function checkProdDeploymentExternal(uint256 forkId) external { + checkProdDeployment(forkId); + } + + /// Creates one fork, external so an unreachable endpoint is reported against + /// its own network instead of aborting the run. + function createForkExternal(string memory network) external returns (uint256) { + return vm.createFork(network); } /// The revert reason as text, so the failure message names what went wrong @@ -141,6 +150,27 @@ contract LibDecimalFloatDeployProdTest is Test { this.decodeStringExternal(payload); } + /// EVERY fork is created before ANY fork is selected, in two passes. + /// + /// `LibRainDeploy.createForks` documents why: foundry captures the pre-fork + /// account set when the first fork is SELECTED, and seeds every fork created + /// after that capture with it, so such a fork reads any address the caller + /// had already touched as the empty account a bare 31337 EVM has for it. A + /// single `createSelectFork` loop creates fork N after the capture for every + /// N above the first. + /// + /// This loop is correct today only by accident: nothing here touches the + /// pinned addresses before the first select, so nothing is in the captured + /// set to carry forward. A `setUp` that etched the tables — the obvious thing + /// for someone to add — would put them in it and silently turn eight of the + /// nine networks into `DecimalFloat not deployed`, naming real addresses on + /// chains that really hold them. Two passes remove the accident. + /// + /// Not `LibRainDeploy.createForks` itself, which is this same creation in one + /// call. It creates the forks in a plain loop, so the first unreachable + /// endpoint reverts and the remaining networks are never reported. Creating + /// them one at a time through an external call keeps an outage attributed to + /// its own network, the same way a missing deployment is. function testProdDeploymentEverySupportedNetwork() external { string[] memory networks = LibRainDeploy.supportedNetworks(); // A list that came back empty would pass every assertion below by @@ -148,12 +178,31 @@ contract LibDecimalFloatDeployProdTest is Test { assertTrue(networks.length > 0, "no supported networks"); string memory failures = ""; + + // Pass 1: create every fork. Nothing is selected yet. + uint256[] memory forkIds = new uint256[](networks.length); + bool[] memory created = new bool[](networks.length); for (uint256 i = 0; i < networks.length; i++) { - try this.checkProdDeploymentExternal(networks[i]) {} + try this.createForkExternal(networks[i]) returns (uint256 forkId) { + forkIds[i] = forkId; + created[i] = true; + } catch (bytes memory err) { + failures = string.concat(failures, "\n", networks[i], ": ", reasonOf(err)); + } + } + + // Pass 2: select each in turn and check it. A network whose fork could + // not be created has already been reported above. + for (uint256 i = 0; i < networks.length; i++) { + if (!created[i]) { + continue; + } + try this.checkProdDeploymentExternal(forkIds[i]) {} catch (bytes memory err) { failures = string.concat(failures, "\n", networks[i], ": ", reasonOf(err)); } } + assertEq(failures, "", failures); } } From d83fba205049b9e85be573100b1b494fc76a3c71 Mon Sep 17 00:00:00 2001 From: David Meister Date: Mon, 28 Sep 2026 11:35:54 +0000 Subject: [PATCH 6/6] audit: name the log tables file after what it holds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `src/generated/LogTables.pointers.sol` -> `src/generated/LogTables.bytes.sol`. There are no pointers in it. It holds five `bytes` constants of log and anti-log table data: LOG_TABLES, LOG_TABLES_SMALL, LOG_TABLES_SMALL_ALT, ANTI_LOG_TABLES, ANTI_LOG_TABLES_SMALL. The name came from the rainlang convention, where a generated file of this shape holds function pointers — `RainlangReferenceExternPointers.sol` has `SUB_PARSER_FUNCTION_POINTERS` and friends. Same generator, `LibCodeGen.bytesConstantString`; the filename followed the generator's usual payload rather than this one's. Nothing enforced it: `rain-sol-codegen` never mentions `.pointers`, and no other `.pointers.sol` exists in any dependency. It had been there since 03edf4f, the initial commit. `.bytes.sol` matches `test/src/lib/table/LibLogTable.bytes.t.sol`, which already tested it under that word, and distinguishes it from `src/generated/candidate/LogTables.sol` — the deploy record of the data contract these bytes are wrapped in, which is a different thing with nearly the same name. Done now because the repo is already cutting a version for the moved `DecimalFloat` address, so a consumer re-pins once rather than twice. The blast radius is smaller than it looks: rainlang vendors this file but never imports its path, reaching the tables through `LibDecimalFloatDeploy`. `Build.sol`'s docstring also claimed the leaf was a name "the deploy pins and importers reference". The deploy pins addresses, not filenames. It now says importers reference it, and records what the old name got wrong. Pure rename: git records `R` with no content change, and re-running `script/Build.sol` regenerates the file byte-identical at the new leaf. `forge test --force`: 42 suites, 85 tests, 82 passed, 3 failed on missing `*_RPC_URL` as before. fmt and lint clean. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 2 +- README.md | 4 ++-- foundry.toml | 2 +- script/Build.sol | 19 ++++++++++++------- src/abstract/DecimalFloatDeploySuites.sol | 2 +- ...ables.pointers.sol => LogTables.bytes.sol} | 0 src/lib/deploy/LibDecimalFloatDeploy.sol | 2 +- .../LibDecimalFloatDeployCandidate.t.sol | 2 +- test/src/lib/table/LibLogTable.bytes.t.sol | 4 ++-- 9 files changed, 21 insertions(+), 16 deletions(-) rename src/generated/{LogTables.pointers.sol => LogTables.bytes.sol} (100%) diff --git a/CLAUDE.md b/CLAUDE.md index 3af31ff..76e0641 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -36,7 +36,7 @@ generated deploy records and the deploy scripts + tests. The pure math frozen `src/generated//` snapshot; a normal PR never bumps it. - `DecimalFloat`'s constructor reverts unless the log tables are at their Zoltu address: tables deploy/build/broadcast FIRST, always. -- `src/generated/LogTables.pointers.sol` is table BYTES (a pure function of +- `src/generated/LogTables.bytes.sol` is table BYTES (a pure function of `LibLogTable`), not a deploy record — it stays at the generated root, never in a tag dir. diff --git a/README.md b/README.md index 3d9ac56..e863fdc 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ concrete `DecimalFloat` contract, the rolling `src/generated/candidate/` snapshots of its deterministic Zoltu deploy records (address, codehash, creation and runtime bytecode), the alias lib `src/lib/deploy/LibDecimalFloatDeploy.sol` over those pins, the generated log-tables bytes -(`src/generated/LogTables.pointers.sol`), and the deploy scripts + tests. +(`src/generated/LogTables.bytes.sol`), and the deploy scripts + tests. The **library** half — `LibDecimalFloat`, `LibDecimalFloatImplementation`, `LibFormatDecimalFloat`, `LibParseDecimalFloat`, `LibLogTable` and the errors — @@ -33,7 +33,7 @@ consumers that need the deployed address, codehash or the deploy pins `checkLogTablesDeployed()`. - `src/lib/LibReleasedSuites.sol` (+ the per-contract `Lib*Released.sol`) — the generated record of every released suite. NEVER edit by hand. -- `src/generated/LogTables.pointers.sol` — the AOT-compiled log/anti-log table +- `src/generated/LogTables.bytes.sol` — the AOT-compiled log/anti-log table bytes, regenerated (not hand-written) by `script/Build.sol`. The bytes are a pure function of `LibLogTable`, so this snapshot is version-invariant. NEVER edit by hand. diff --git a/foundry.toml b/foundry.toml index b56234d..1911cb1 100644 --- a/foundry.toml +++ b/foundry.toml @@ -43,7 +43,7 @@ libs = ["dependencies"] # Write access is scoped to exactly the files `script/Build.sol` generates: # everything under `src/generated/` (the candidate snapshots, the frozen tag -# dirs `cutRelease()` adds, `LogTables.pointers.sol`) and the three generated +# dirs `cutRelease()` adds, `LogTables.bytes.sol`) and the three generated # released-suites libs in `src/lib/` — never hand-written source. It reads the # release version from this file when `cutRelease()` runs, and # `DecimalFloatDeploySnapshotTest`'s inherited frozen-record walk reads diff --git a/script/Build.sol b/script/Build.sol index 0bba171..681f515 100644 --- a/script/Build.sol +++ b/script/Build.sol @@ -17,10 +17,15 @@ import {DecimalFloatDeploySuites} from "../src/abstract/DecimalFloatDeploySuites /// `LIB_FS_ROOT` moved, and would ignore a `recordRoot()` override, which exists /// so `cutRelease()` can be exercised against a record other than the repo's own. /// -/// The `.pointers.sol` suffix -/// is the one the deploy pins and importers reference. The file is pure -/// log-table data with no contract instance behind it, so it carries no -/// bytecode-hash constant and is written directly rather than through +/// The leaf is what importers reference, so changing it is a downstream edit. +/// It was `LogTables.pointers.sol` until this branch, which named the file after +/// a payload it does not hold: there are no pointers in it, only five `bytes` +/// constants of table data. The name came from the rainlang convention where +/// `LibCodeGen.bytesConstantString` output IS function pointers, and followed +/// the generator's usual payload rather than this one's. +/// +/// The file is pure log-table data with no contract instance behind it, so it +/// carries no bytecode-hash constant and is written directly rather than through /// `LibFs.buildFileForContract`, which heads every file it writes with the hash /// of an instance and reverts on the codeless `address(0)` this build has. /// @@ -31,7 +36,7 @@ import {DecimalFloatDeploySuites} from "../src/abstract/DecimalFloatDeploySuites /// `src/generated/candidate/LogTables.sol`, and only tag-shaped directories /// under this root are releases — a loose file here is one neither /// `frozenSnapshotPaths` nor `release-guard` reads as a snapshot. -string constant GENERATED_LOG_TABLES_LEAF = "LogTables.pointers.sol"; +string constant GENERATED_LOG_TABLES_LEAF = "LogTables.bytes.sol"; /// One contract's generated files: the rolling snapshot and the released-suites /// lib emitted from its record. @@ -83,7 +88,7 @@ contract Build is BuildScript, DecimalFloatDeploySuites { return names; } - /// Rewrites `src/generated/LogTables.pointers.sol` from `LibLogTable`. + /// Rewrites `src/generated/LogTables.bytes.sol` from `LibLogTable`. /// /// The bytes are a pure function of that library's mathematics, so this /// file is a single snapshot rather than a per-tag directory: there is no @@ -134,7 +139,7 @@ contract Build is BuildScript, DecimalFloatDeploySuites { /// a freeze copies whatever this hook leaves behind, so regenerating them /// anywhere but here would let a release freeze a snapshot cut from stale /// tables. They are written from `LibLogTable` while the snapshot below - /// is cut from the COMPILED-IN `LogTables.pointers.sol`, so a run that + /// is cut from the COMPILED-IN `LogTables.bytes.sol`, so a run that /// actually moves the tables converges on the second run — which is what /// the currency check in CI reports rather than hides. /// - `writeSnapshot` runs each creation code through the Zoltu factory to diff --git a/src/abstract/DecimalFloatDeploySuites.sol b/src/abstract/DecimalFloatDeploySuites.sol index 74cca42..63e4fca 100644 --- a/src/abstract/DecimalFloatDeploySuites.sol +++ b/src/abstract/DecimalFloatDeploySuites.sol @@ -52,7 +52,7 @@ abstract contract DecimalFloatDeploySuites is RainDeploySuitesBase { /// There is no Solidity contract behind it — the creation code is /// `LibDataContract`'s data-contract wrapper around the bytes /// `LibDecimalFloatDeploy.combinedTables()` concatenates out of - /// `src/generated/LogTables.pointers.sol` — so `sourceCreationCode` is that + /// `src/generated/LogTables.bytes.sol` — so `sourceCreationCode` is that /// same pure expression rather than a `type(X).creationCode`, and the /// artifact path is empty because there is no source file for an explorer /// to verify against. diff --git a/src/generated/LogTables.pointers.sol b/src/generated/LogTables.bytes.sol similarity index 100% rename from src/generated/LogTables.pointers.sol rename to src/generated/LogTables.bytes.sol diff --git a/src/lib/deploy/LibDecimalFloatDeploy.sol b/src/lib/deploy/LibDecimalFloatDeploy.sol index dfcc2ba..40b4c8b 100644 --- a/src/lib/deploy/LibDecimalFloatDeploy.sol +++ b/src/lib/deploy/LibDecimalFloatDeploy.sol @@ -8,7 +8,7 @@ import { LOG_TABLES_SMALL_ALT, ANTI_LOG_TABLES, ANTI_LOG_TABLES_SMALL -} from "../../generated/LogTables.pointers.sol"; +} from "../../generated/LogTables.bytes.sol"; import { DEPLOYED_ADDRESS as LOG_TABLES_CANDIDATE_ADDRESS, BYTECODE_HASH as LOG_TABLES_CANDIDATE_HASH diff --git a/test/src/lib/deploy/LibDecimalFloatDeployCandidate.t.sol b/test/src/lib/deploy/LibDecimalFloatDeployCandidate.t.sol index 2c3bee1..ad43b3a 100644 --- a/test/src/lib/deploy/LibDecimalFloatDeployCandidate.t.sol +++ b/test/src/lib/deploy/LibDecimalFloatDeployCandidate.t.sol @@ -37,7 +37,7 @@ import {DecimalFloat} from "src/concrete/DecimalFloat.sol"; contract LibDecimalFloatDeployCandidateTest is Test { /// SOURCE -> CANDIDATE, log tables: the candidate records the data-contract /// creation code this repo's own `combinedTables()` produces, so the pins - /// describe the tables in `src/generated/LogTables.pointers.sol` rather + /// describe the tables in `src/generated/LogTables.bytes.sol` rather /// than a stale payload. function testLogTablesCandidateCreationCodeMatchesSource() external pure { assertEq( diff --git a/test/src/lib/table/LibLogTable.bytes.t.sol b/test/src/lib/table/LibLogTable.bytes.t.sol index f336999..58b586c 100644 --- a/test/src/lib/table/LibLogTable.bytes.t.sol +++ b/test/src/lib/table/LibLogTable.bytes.t.sol @@ -10,11 +10,11 @@ import { LOG_TABLES_SMALL_ALT, ANTI_LOG_TABLES, ANTI_LOG_TABLES_SMALL -} from "src/generated/LogTables.pointers.sol"; +} from "src/generated/LogTables.bytes.sol"; /// @title LibLogTableBytesTest /// @notice Verifies that toBytes encoding of each table matches the -/// AOT-compiled constants in LogTables.pointers.sol. +/// AOT-compiled constants in LogTables.bytes.sol. contract LibLogTableBytesTest is Test { /// toBytes(logTableDec()) matches the generated LOG_TABLES constant. function testToBytesLogTableDec() external pure {