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/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 56e8c17..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 @@ -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..681f515 100644 --- a/script/Build.sol +++ b/script/Build.sol @@ -9,21 +9,34 @@ 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 -/// 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 +/// @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 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. /// -/// 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.bytes.sol"; /// One contract's generated files: the rolling snapshot and the released-suites /// lib emitted from its record. @@ -75,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 @@ -84,7 +97,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( @@ -126,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/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/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/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/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/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/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/deploy/LibDecimalFloatDeployProd.t.sol b/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol index 88153e7..3cf2c63 100644 --- a/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol +++ b/test/src/lib/deploy/LibDecimalFloatDeployProd.t.sol @@ -11,43 +11,198 @@ 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 { - function checkProdDeployment(string memory network) internal { - vm.createSelectFork(network); + /// 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. + /// 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, 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" ); } - 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. + /// + /// 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, + /// which is the opposite of useful when the question is WHICH chains are + /// missing a deployment. + function checkProdDeploymentExternal(uint256 forkId) external { + checkProdDeployment(forkId); } - function testProdDeploymentBase() external { - checkProdDeployment("base"); + /// 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); } - function testProdDeploymentBaseSepolia() external { - checkProdDeployment("base_sepolia"); + /// 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. + /// + /// 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)); } - function testProdDeploymentFlare() external { - checkProdDeployment("flare"); + /// 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); + } + 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. 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. + // forge-lint: disable-next-line(unsafe-typecast) + bytes32 offsetWord = bytes32(payload); + if (uint256(offsetWord) != 0x20) { + return vm.toString(err); + } + try this.decodeStringExternal(payload) returns (string memory reason) { + return reason; + } catch { + return vm.toString(err); + } } - function testProdDeploymentPolygon() external { - checkProdDeployment("polygon"); + /// 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); + } + + /// 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 + // never running one. + 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.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); } } 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 {