Repository navigation
chore: sync upstream contracts (2026-09-14) - conflicts need resolution - #40
Merged
Merged
Conversation
…ce#2323) * fix(clear-signing): resolve native currency in tokenAmount fields Every `tokenAmount` field in the proposal formatted its value against a `tokenPath` asset id, but LI.FI calldata represents the chain's native currency with a sentinel (address(0) or 0xEeee...EEeE). Neither resolves to ERC-20 metadata, so wallets had no decimals or ticker to format with and fell back to the raw integer: an ERC20->Native swap rendered "Minimum to Receive: 7538051138939978" instead of "0.007538051138939978 ETH". Route all 11 `tokenAmount` construction sites through `tokenAmountParams`, which attaches ERC-7730's `nativeCurrencyAddress` alongside `tokenPath`. A helper rather than a per-site constant: any asset-id path in this diamond can hold a native sentinel, so opting in per-site only leaves room to forget one when a format is added. Covers 103 of the descriptor's 105 `tokenAmount` fields. The remaining two belong to `swapTokensGeneric`, a registry-authored entry the sync preserves rather than owns. Confirmed against the ERC-7730 test fixtures, where the two ERC20->Native swap cases are the ones rendering bare wei today. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(clear-signing): format ERC20-to-native output as native currency swapTokensSingleV3ERC20ToNative and swapTokensMultipleV3ERC20ToNative send address(this).balance and never read the receivingAssetId they are handed, so formatting _minAmountOut against it let crafted calldata label a guaranteed- native amount with any ticker and decimals. Both now use format "amount". Also narrow the native sentinel list to the zero address, the only value LibAsset.isNativeAsset accepts; 0xEeee...EEeE appears in src/ solely as an outbound translation and is never read as native input. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lifinance#2326) Upstream PR #2948 is detached from sync/lifi-erc7730 and cannot be closed by us, but it still matches the workflow's head-ref lookup — so every sync skipped gh pr create and published nothing. A head ref with no open PR gets one that GitHub follows.
lifinance#2308) * feat(immutables): read the immutables src/ declares (EXSC-869) The registry has to account for every immutable in `src/`, so the first thing it needs is a reliable enumeration. This reads source rather than build output, so the CI gate needs no compile, and it recognises only the declaration forms this repo uses — a shape it cannot read shows up as a missing registry entry rather than as a silent pass. Two measurements worth recording, both from real repo data. The ticket says the authoring pass covers "42 facet files / 111 immutable sites". The real figure is **79 declarations in 42 files**. The 111 comes from `grep -rn '\bimmutable\b' src/ | grep -v '^\s*//'`, where the `^` anchor cannot match past `grep -n`'s `file:line:` prefix — so the filter never removed anything and comment lines were counted. Part (ii) is about 30% smaller than scoped. And writing the enumeration found one immutable no single-token type pattern sees: `address payable public immutable POLYMER_FEE_RECEIVER` in `PolymerCCTPFacet.sol`. A two-word type read straight past the pattern, which is precisely the failure a registry gate cannot have — the immutable would never have been asked for. Now covered, and the sweep reports zero declarations the parser cannot read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(immutables): propose registry links, and record why they cannot gate The registry's `config-path` half already exists: `deployRequirements.json` carries 98 `configData` entries across 57 contracts, each naming a config file and a key. So WP-2.3 extends that rather than adding a parallel file. Bridging it to the immutables in `src/` by name does not work, and the measured reason matters more than the code. A `configData` key is a free-form label: `helperFunctions.sh:4903` reads `configData | keys[]` only to iterate, then follows `configFileName` and `keyInConfigFile`. Nothing constrains the spelling. `AcrossFacet` is the worked case — the constructor takes `_wrappedNative`, the contract stores `wrappedNative`, and the requirement is filed under `_wrappedNativeAddress`. Three spellings, one binding. Name matching therefore reports 48 orphaned entries on today's repo, nearly all label drift rather than a missing expectation. A gate that fires 48 times on a correct repo is noise, and this project has spent the day learning what noise costs a gate. So this module is a suggester, labelled as one: it proposes 50 likely links and lists the 29 immutables no entry resembles, as input to the authoring pass. The registry format needs each entry to name the immutable it governs explicitly, which also re-sizes part (ii) — the pass is not 29 additions but a link field on all 98 entries plus those 29. Measured on `origin/main` at ce1b276: 79 immutables in 42 files, 98 entries in 57 contracts, 50 name-matched, 29 unmatched, 48 entries unmatched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(immutables): correct comments the reframe left stale The test header still described `configData` keys as constructor parameter names, which is the assumption the previous commit disproved, and one comment still read an unmatched entry as a lost expectation rather than as something the suggester cannot tell apart from label drift. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(immutables): the registry, its gate, and InfoSec protection for both (D20) Each entry names the immutable it governs and how its expected value is established: `config` (naming a `configData` label, so the deploy requirement and the expectation cannot drift apart), `derived` with a rule, or `unchecked` / `unverifiable` with a reason. `authorityBearing` flags the ones the drift report must call out — D20's marker, with no new approver tier. Errors and warnings are deliberately different things. An immutable nobody has documented yet is a warning, because the authoring pass is still running and the hard fail is ordered after it. A registry that asserts something false is an error in either mode: a link to a `configData` key that does not exist, an entry for an immutable the contract no longer declares, an exemption with no reason, or an unrecognised `source` — that last one because a typo in `source` would otherwise make an immutable silently exempt. The registry is its own file rather than a section inside `deployRequirements.json`, and that is forced by the protection D20 asks for: path-based protection only means anything if the path holds only the protected thing. Verified against the workflow's own matcher — `immutableRegistry.json` and `script/deploy/immutables/` are protected, `deployRequirements.json` and `src/` are not, so routine deploy-requirement edits do not acquire an InfoSec approval they never needed. The gate's own code is protected too: editing the checker to always pass is equivalent to editing the expectation. Real-fleet evidence, all four error paths firing and the benign direction silent: 79 immutables, no entries -> 0 errors, 79 warnings, exit 0 AcrossFacet authored (2) -> 0 errors, 77 warnings, exit 0 --strict with the gap -> exit 1 link to a nonexistent label -> 1 error, exit 1 even in warn-only entry for a renamed immutable -> 1 error exemption with no reason -> 1 error `jq '.AcrossFacet.configData | keys[]'` still returns both labels, so the deploy path reads the requirements file exactly as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(immutables): ask what the enumerator could not read, and close three holes Four review findings, all real. **The parser's blind spot was the important one.** A declaration wrapped across lines is valid Solidity and parses as nothing, so the immutable is never asked for — invisible rather than unanswered, which is the worst failure available to a registry gate. My own docstring claimed an unreadable shape "shows up as a missing entry"; it did not. `findUnreadableImmutableLines` now reports any line mentioning `immutable` that the parser could not read, and the gate treats those as errors in either mode. Demonstrated by wrapping a real declaration in `AcrossFacet`: the count drops 79 to 78 and the gate exits 1; restored, it exits 0 again. **Deleting or renaming a protected file bypassed the InfoSec approval.** The workflow's `[[ ! -f "$FILE" ]]` guard ran before the path matcher, so a deleted path was skipped with a warning, and without `--no-renames` a rename reported only the new, unprotected name. Reproduced both on a scratch repo: deleting immutableRegistry.json -> before: SKIPPED (not found) after: PROTECTED renaming it to harmless.json -> before: free (new name only) after: both paths, old one PROTECTED Pre-existing, and in scope because the protection this PR adds is defeated by it. The matcher is pure string matching, so a path that no longer exists is safe to test. **`authorityBearing: "true"` was silently unflagged.** JSON gives no warning, and `=== true` dropped the string — leaving an authority-bearing immutable out of the one report that must show it. A present non-boolean is now an error. **The config link accepted inherited properties.** `in` walks the prototype, so `configData: "toString"` validated against any object at all. Now `Object.prototype.hasOwnProperty.call`. Second instance of this exact bug in the same day, after the CBOR decoder. Nine tests added; the behavioural ones all fail against the previous commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(immutables): build on lifinance#2213's requirement types now that it has merged lifinance#2213 landed while this branch was open, so the registry no longer restates the `deployRequirements.json` shape: `IContractRequirements` extends its `IDeployRequirementEntry` and adds only the `immutables` section. That keeps `getter` and `legacyGetters` — the fields deciding whether a binding is readable on chain at all — defined once. It is what D20 and the ticket meant by "extend, don't duplicate", and it was not possible until the merge. The stricter type also rejected this file's placeholder fixtures, which is a better outcome than it sounds: `configData: { _spokePool: {} }` is not a shape that exists. The fixture is now AcrossFacet's real pair, including the `getter` lifinance#2213 added. Worth recording for the drift report: `getter` present means the binding is checkable on chain, which is most of the getter-visible/not distinction F5 turns on. The registry's `unverifiable` kind and a missing `getter` describe the same situation from two directions, and the drift report should read them together rather than duplicating the judgement. Merged rather than rebased: a rebase needs a force-push, which is unavailable in this session. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(immutables): close three ways an immutable stayed invisible, and one bypass The gate round found four criticals. Three are fixed here; the fourth is escalated because its fix changes when a security workflow runs. **A second declaration on the same line was invisible.** The reporter asked "did this line parse?", so `address public immutable A; address public immutable HIDDEN;` — valid Solidity, and nothing in CI reformats it — read as covered by the first. `HIDDEN` was not declared, not warned, and not reported unreadable. It now compares the number of `immutable` occurrences on a line against the number parsed from it. **`git ls-files 'src/**/*.sol'` never matched a top-level `src/*.sol`**, because default pathspec still wants a slash after `**`. `src/LiFiDiamond.sol` sits in that hole today. `src/*.sol` matches all 139 files, since `*` crosses slashes here. **A protected path holding a non-ASCII or control byte defeated the matcher.** `git diff --name-only` C-quotes such paths, and the leading quote breaks every `^`-anchored alternative — so adding `.github/workflows/pwné.yml`, a workflow Actions will run, required no Information Security Manager approval. Reproduced and fixed with a NUL-delimited read; verified as bash over five cases: non-ASCII workflow added -> PROTECTED (was: free) newline inside a path -> PROTECTED registry deleted -> PROTECTED registry renamed away -> PROTECTED (old path) benign change only -> no approval required Also from the same round: the natspec-tag exclusion in the reporter was dead — deleting it changed nothing in 57 tests or the fleet run — and worse, it silenced `immutable OWNER /* @dev */ = ...`, which parses as nothing. Removed, and the word inside a string literal is now ignored so a `"immutable"` constant is not a false red. The gate is wired into `validateScripts.yml`, which the PR body already claimed; it had no CI home. Its paths-filter does not fire on `src/**`, so a PR adding a facet does not run it yet — harmless while warn-only, a prerequisite of the `--strict` flip, and a CI-cost trade-off rather than an oversight. Noted in the step. And the fixture claimed a `getter` on `AcrossFacet` that does not exist and could not: `spokePool` is `private`, so there is no on-chain getter to name. The previous commit message repeated it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(healthcheck): enforce the getter annotation for new immutable bindings (EXSC-919) Coverage of `immutable-bindings-match-config` (EXSC-784, lifinance#2213) is opt-in, so a contract added without a `getter` annotation silently reopens the bug class the invariant closes. `immutableGetterCoverage.test.ts` now scans `src/Facets` and `src/Periphery` for public immutable address getters and fails on any that is neither annotated nor recorded in `UNANNOTATED_IMMUTABLE_GETTERS` with a reason; the list may only shrink, since stale and superseded entries also fail. It reads Solidity sources rather than `out/` artifacts because the only CI job running the TypeScript suite is Foundry-free — an artifact-based gate would skip there and enforce nothing. `src/**` joins that job's path filter so a Solidity-only PR still runs it. Exemptions live in TypeScript rather than a new `deployRequirements.json` key, both to keep clear of the `.configData | keys[]` consumer and because a third of them have no arg object to nest inside. Seeding the list recorded 8 permanent exemptions (Executor / LiFiDiamond bindings and the pauser wallet, none of which config holds a value for) and 31 config-backed bindings whose annotation is deferred to EXSC-917: annotating promotes each to an error-severity comparison on every chain at once, which needs a fleet dry run first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(healthcheck): close two silent evasion paths in the getter coverage gate (EXSC-919) Falsifying the gate's coverage claim, rather than only that it fires, surfaced two forms it could not read — each one a getter it would never see, which is the single failure mode a gate like this must not have. Solidity accepts the specifiers in either order, so `address immutable public X;` parsed as nothing. None exist today, so nothing failed; a facet written that way would simply have been invisible. The pattern now reads both orders and tolerates a declaration wrapped across lines. `src/Security` was also unscanned despite holding deployable, registry-tracked contracts — `LiFiTimelockController` already carries `configData` args. It contributes no getters today, so the exemption list is unchanged. Verified the widened pattern finds the same 49 getters with nothing unaccounted, so dropping the line anchor introduced no false positives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(immutables): mask comments and strings before reading a declaration Round-3 review found the enumerator could still be fooled in both directions, and that one fix claimed in f1aa1a6 was never applied. The parser and the reporter approximated Solidity's lexical structure two cheap ways, and each hid a real immutable: - Quote pairs were stripped with a regex that does not know about `\"`. An escaped quote made the quote count odd, so the next pair spanned the following declaration and deleted the keyword. `address public immutable HIDDEN` after `string constant Q = "\"";` compiles and was neither parsed nor reported. - A line whose first characters opened a comment was skipped in the parser and excluded from the reporter, so an inline block comment in front of any declaration hid it in both directions. Counting raw `immutable` occurrences to catch a second declaration then made correct code fail: a trailing `// the immutable owner`, `/// @notice immutable fee` or `/** @dev immutable */` each counted as a mention with no matching parse, and the error told the author to put on one line a declaration that was already on one line. That comment style appears on 152 other statements in `src/`. All three share one cause, so they get one fix: a single pass that blanks comments and string literals in place, with escape handling, before anything counts or parses. Declarations are then read per statement rather than per line, so two or three on one line all parse — previously the line-anchored pattern capped the parsed count at 1, which also made the suite's "both declarations read" control vacuous, since its fixture could never fail. f1aa1a6's message claimed the `src/**/*.sol` pathspec was changed to `src/*.sol`. It was not — that commit never touched the CLI, so the broken pathspec shipped and `src/LiFiDiamond.sol` stayed invisible (138 of 139 files). Applied here. It declares no immutables, so the fleet verdict is unchanged at 79; the blind spot is what closed. Also: the two JSON files the gate validates as hard errors were outside the paths filter of the only job that runs it, so a `deployRequirements.json` edit could introduce a registry error that never ran. Both are now in the filter, and the step's disclosed-gap comment no longer claims `src/**` was the only hole. Fleet: 139 files, 79 declarations, 0 unreadable. Suite 69 pass / 0 fail. * fix(healthcheck): read inline-initialized immutables, and correct the private-immutable wording (EXSC-919) Addresses both CodeRabbit findings on lifinance#2310. Solidity permits assigning at the declaration (`address public immutable X = ...;`), which generates the same getter but did not match a pattern requiring `;` straight after the name. No such declaration exists in the gated trees today — CodeRabbit's evidence to the contrary matched across unrelated statements, since its pattern allowed 200 characters between the declaration and the `=` — so this is robustness, not a live miss. Detection still finds the same 49 getters. The authoring rule also overstated the private/internal case: such an immutable generates no getter, but a contract can still expose the value through an explicit accessor. Reworded, and noted that the gate reads compiler-generated getters only, so a hand-written accessor has to be annotated deliberately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(immutables): never capture a modifier keyword as the immutable name Round-4 review found a regression introduced by the previous commit, in the direction that matters most: an immutable that is neither parsed nor reported. Splitting a line into statements meant the pattern's terminator had to accept end-of-statement rather than `;`. Solidity allows `immutable` before the visibility and allows `override` on a public state variable, so on a declaration that wraps after such a keyword the pattern captured the keyword itself as the name: uint256 immutable public FEE = 1; parsed as a declaration named `public`. That is worse than failing to parse. The phantom made the parsed count equal the mention count, so the reporter stayed silent, and `FEE` appeared nowhere — not as a declaration, not as a warning, not as an unreadable line. Under --strict the gate would fail naming `public`, and a registry entry authored for `public` would clear it while the real immutable stayed unaccounted for. All three shapes compile, and the previous implementation reported all three correctly. The name capture now refuses the state-variable keywords, so these shapes are reported as unreadable again. Also closes two masker behaviours that were asserted in comments but that no test could falsify — a quote or apostrophe inside a comment, which is the commonest way an unbalanced quote enters real Solidity and would otherwise mask a whole file, and `/*/` opening a comment without closing it — plus line-number preservation across a multi-line block comment. Drops the "152 other statements" figure from a test comment: it does not reproduce under any reasonable definition (154 by one, 155 by another, 269 by a third), so it is restated as over 150. Fleet: 139 files, 79 declarations, 0 unreadable. Suite 77 pass / 0 fail. * fix(immutables): reject only the bare keyword, and count mentions the same way Round-5 review found the previous commit's keyword exclusion turns the gate red on code that compiles. `$` is legal in a Solidity identifier but is not a regex word character, so the `\b` on the exclusion read `public$FEE` as the bare keyword `public`: uint256 public immutable public$FEE = 1; compiles; gate reported it red Not an invisible immutable — it was reported, so nothing could slip past the registry — but an unreadable line exits 1 in both modes, so an honest declaration failed the gate. The exclusion now ends in a negative lookahead for an identifier character instead. The mention counter had the same blind spot one layer down: `\bimmutable\b` matches the `immutable` inside a name like `immutable$I`, so one declaration counted as two mentions and the line was reported as hiding something. It now uses the same identifier rule as the name capture, which is the point — the two must agree on where an identifier ends or they disagree about what was read. Adds the coverage round 5 showed was missing. Deleting the boundary altogether previously left the suite green, while that mutant makes `publicKey`, `immutableOwner`, `overrideAddress` and every other reserved-word-prefixed name fail the gate — `immutableOwner` being the name this module's own docstring uses as its example. Nine such names are now tested, plus the four `$` shapes and the four bare keywords that must still be refused. `private` and `internal` join the wrapped-phantom cases: both compile in that position and neither was covered, so dropping either from the alternation left the suite green. Recorded in the docstring that `constant`, `immutable` and `transient` cannot reach that position at all — solc rejects each as a repeated or conflicting mutability — so they are defensive entries rather than reachable ones. Also corrects the operator message. It told the author to put the declaration on one line, which is wrong for the shape where it already is one line and the modifier order is what the parser cannot read; it now names the form to write. Fleet: 139 files, 79 declarations, 0 unreadable. Suite 96 pass / 0 fail. * fix(immutables): stop excluding a legal name, and refuse a malformed registry `transient` was in the exclusion list on the grounds that solc rejects it next to `immutable` as a repeated or conflicting mutability. It does reject it as a data location, which is the only way it could be the modifier the exclusion guards against — but `uint256 public immutable transient` compiles on both the repo floor and the default compiler: solc 0.8.17: Compiler run successful! solc 0.8.29: Compiler run successful! So the exclusion could only ever make a legal declaration unreadable, and an unreadable line exits 1 in both modes. Same false-positive class the previous commit closed for `public$FEE`, reached through the reachability claim instead of the boundary. `constant` and `immutable` stay: solc really does refuse both as a name there (error 3109), verified the same way. A malformed registry now fails instead of degrading. A contract section that is not an object of entries read to the validator as an absent one, so `{"AcrossFacet": null}` reported AcrossFacet's immutables as merely unauthored and exited 0 — and once --strict makes the authored set the thing enforced, that silence is the whole gate. Proven on the real file: exit 1 malformed, exit 0 unchanged. The KNOWN GAP note was also understating itself. A src-only PR skips the unreadable-declaration check too, which is a hard failure in both modes, and the job is additionally skipped on drafts with no push trigger at all — so the gap is not confined to warn-only material. Both workflow headers now name what they gained, per 500-github-actions. Coverage and accuracy: the config-sourced-but-names-no-key branch had no test (registry-schema.ts back to 100% lines), the bare-keyword test asserted the refusal but never that the line was reported, and the coverage suite's headline fixture used a name `src/` does not declare, presenting a 2-of-2 match where the real AcrossFacet pair is 1-of-2 — the exact drift the module documents as its reason for being only a suggester. Corrected numbers measured on this branch: 50 orphaned entries (was 48), `getter` on 8 contracts (was 10), and no array fixture claim, since solc rejects a non-value type as immutable. Suite 109 pass / 0 fail, 100% lines on all three modules. Gate on the real fleet unchanged: 79 declarations, 0 unreadable, 77 warnings, exit 0. * style(immutables): trim history-narrating docstrings per 000-global-standards Goran flagged two docstrings in immutable-declarations.ts as narrating rejected approaches and specific past bug shapes rather than stating the non-obvious why. Pure trim, no behavior change. * fix(safe): import MAX_PROPOSAL_REASON_LENGTH from where it lives Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(healthcheck): stop the coverage gate reading commented and enum-typed declarations (EXSC-919) Comments are stripped before matching, with string literals copied through so a `//` or `/*` inside one cannot swallow the getters declared below it. Enums and user-defined value types declared under `src` are collected and skipped, since both read as contract types but cannot hold a counterparty address. Detection is unchanged: the same 49 getters with the same types. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(immutables): run the registry gate on src/ changes, and test its entry point The gate's own success criterion is a new facet with an undeclared immutable turning CI red, but validateScripts' paths-filter had no src/** entry, so a PR touching only Solidity skipped the job entirely. The check reads source and two JSON files — it needs neither the forge build nor TypeChain — so it moves to its own job gated on src/**, rather than putting a full compile on every Solidity PR. The aggregator now names both jobs; a job in `needs` whose result is never read is reported green whatever it did. The entry point had no tests: mergeRequirements and the exit routing are now exported and covered, and the CLI cases run the real script in a throwaway git repo, because the file set comes from `git ls-files` and the code from process.exit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(immutables): enumerate from the compiler AST instead of parsing source (EXSC-869) The compiler already knows which state variables are immutable, and a regex does not. Swapping to `forge build src --ast` deletes the parser (239 lines) and its test (452), and with them the whole findUnreadableImmutableLines failure class — a shape the reader cannot parse produced nothing, so the immutable was never asked for and never showed as unanswered. What replaces that risk is narrower and has a definite answer: a src/ file the compiler emitted no AST for. findSourcesWithoutAst asks it directly, so an unenumerated file fails the gate instead of quietly contributing zero. Verified rather than assumed. Against the current tree the two enumerations are identical — 79 immutables each, no set difference in either direction, and zero line-number mismatches. Removing one contract's artifact drops the count to 77 and the new guard names the file, which is the failure the deleted class existed to catch. The build writes to out-immutables/ so a developer's out/ is untouched and AST-carrying artifacts cannot reach a workflow caching out/ without one; the gate job gains submodules and pinned foundry under its own cache key. Line numbers come from the AST's BYTE offsets read against the raw buffer: these sources carry multi-byte characters in comments, so character indexing drifts. A test pins that, and fails when the reader is switched back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(immutables): let the caller own the AST build so the gate needs no Foundry (EXSC-869) The CLI cases run the real script, which now compiled for itself — and the TypeScript suite runs on a Foundry-free runner, so they passed locally and went red only in CI with `spawnSync forge ENOENT`. `--out-dir` hands the build to the caller. The workflow compiles in its own step, which also makes a compile failure read as one instead of surfacing inside the gate; the CLI tests point at hand-written artifacts and never shell out. With no flag the gate still builds for itself, which is what a developer wants. The unreadable-declaration case is replaced by its real successor: a tracked src/ file with no AST, asserted to exit 1 and name the file. Verified with forge off PATH, as on the runner: 16 pass, 0 fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(healthcheck): fold the getter gate into the AST enumeration (EXSC-919) The registry gate and this one ask about the same set of immutables, so they now share one enumeration and one CI job instead of each carrying a reader. The regex parser goes with it: `visibility` and the declared type are the compiler's own answers, which retires stripComments, the enum/user-defined-type scan of all of src/, and the address-vs-value-type guessing they existed to support. Verified against the real tree: the AST-derived getter set leaves nothing unaccounted, so the 39 recorded exemptions still describe the code exactly. Both directions of the check were made to fire on real data — dropping one real exemption names that getter, and inventing one names it as undeclared. The exemption list becomes data (getter-exemptions.json) rather than a compiled- in constant. A constant is a claim about one repo, so it read as 39 stale entries inside the gate's own CLI tests — and would in any checkout the gate ran against. It sits beside the gate, where the InfoSec protection on script/deploy/immutables/ already covers it: recording a binding as exempt decides what goes unverified. The gate leaves the TypeScript suite, so ts-unit-tests.yml goes back to its original triggers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(ci): run the immutables gate on every file under script/deploy/immutables (EXSC-919) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(immutables): key the registry on the AST contract name (EXSC-869) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(ci): trigger the immutables gate on the compile inputs it enumerates from (EXSC-919) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(immutables): key the getter gate on the AST contract name (EXSC-919) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(immutables): gate on every input that decides what is compiled (EXSC-869) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ifinance#2325) * feat(deploy): verify .foundry-version on the deploy path (EXSC-932) A developer whose local forge has drifted from .foundry-version passed every existing guard and still produced bytecode a verifier cannot reproduce. The checker was enforced only in CI's setup-foundry action and the pre-commit hook. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(deploy): make the foundry-gate placement assertions non-vacuous The per-entry-point assertions matched a comment that names executeAndParse, and skipped the ordering check entirely for the four entry points with no command-position forge. Deleting the gate left them green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(deploy): prove the foundry gate's placement by running each entry point The line-scanning placement assertions could be evaded two ways: a forge started after a separator (`cd x; forge build`, `a && forge`, `eval "forge …"`) was not seen at all, and a line inside a multi-line string counted as a gate call. Both kept the suite green with the gate removed. Four entry points are now driven for real against a forge stub that records every argv, so the claim comes from bash rather than from a regex. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(deploy): stop the entry-point harness depending on a local .env Two scripts resolve their .s.sol path from a directory .env supplies, so without it they returned before the gate and the assertion passed on a run that never reached it. CI has no .env, so this suite passed locally and would have failed there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(deploy): replace the shell-text classifier with running the scripts Four successive line-scanning attempts each missed a real shape that starts a forge or fakes a gate call: a comment, a multi-line double-quoted string, a heredoc body, and a forge after ;/&&/else/exec. The classifier is gone; five of the six entry points are now driven for real against an argv-recording stub. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(deploy): keep the entry-point harness off the repo's own records Driving diamondUpdateFacet with the gate mutated away reached saveDiamondFacets and rewrote a tracked deployment log. Deployment records are read-only for this project, so the harness stubs the log writers and asserts deployments/ and config/ are unchanged after every run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(deploy): drop the entry-point case that needs a signing key The deployAndStoreCREATE3Factory case stopped on a missing PRIVATE_KEY before its forge call, so it asserted on a run that never reached the gate — and it failed outright with no .env, which is how CI runs this suite. Supplying a key is deliberately not done: it would put a real one in cast's argv on any machine that has one. The seam covers that entry point instead. The four function stubs are removed with it. Four of these scripts re-source helperFunctions.sh inside their own body, which redefined every stub before it could take effect, so the comments claiming they had an effect were false. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Goran Vladika <goran.vladika@gmail.com>
…nel (EXSC-929) (lifinance#2324) * feat(deploy): evaluate the production deploy gate in the proposal funnel (EXSC-704) The gate had two live homes after lifinance#2286 re-homed it: `diamondUpdateFacet.sh`, which runs once per (network, facet) and has five other callers that inherited it incidentally, and `proposeDiamondCut`, which covers only the callers that happen to route through it. Neither covers the generic `sendOrPropose` chokepoint, and both need a caller to remember them. Both call sites are removed and the gate now runs in the Safe proposal funnel — `propose-to-safe.ts` and `propose-to-safe-tron.ts` — which every Safe proposal reaches by construction. The funnel is handed calldata rather than facet names, so the facet set is recovered from the cut: `diamondCut` Add/Replace entries (Remove installs no code), unwrapping a timelock `scheduleBatch` so a pre-wrapped payload cannot slip past, then attributed to a name through the network's production deployment log. An address the log cannot attribute, or a `diamondCut` selector whose arguments do not decode, refuses rather than falling through as "not a cut". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(signing): the deploy gate now lives in the proposal funnel (EXSC-704) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(deploy): gate the calldata that is actually signed, not a re-parse of it The gate read its own `normalizeProposeCalls` / `normalizeTronProposeCalls` result rather than the array the proposal is built from, so the bytes it vouched for and the bytes that get signed came from two separate parses of a caller-supplied file. Both funnels now parse once and the gate reads that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(deploy): prove the gate is wired in the Tron funnel too, and cannot pass on a killed child The Tron funnel's gate call had no test at all: `propose-diamond-cut` was the only thing gating a Tron cut before this, and removing it there left the new call site unproven. Its probe drives the real CLI with a per-run generated key and asserts the refusal lands before the Timelock read, which is the funnel's first RPC. The EVM probe's absence-assertions ("never reached the Safe client") would also have passed on a child killed by a timeout. Both now withhold every signing credential and treat a signalled child as no result. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(deploy): anchor the placement assertions on markers that actually appear Both refusal cases asserted the absence of text the run never prints even without the gate ("Signer Address", "getMinDelay"), so they proved nothing about ordering. Each now asserts the absence of the marker the funnel really emits one step past the gate, checked by deleting the gate call and watching it appear. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(deploy): attribute the cut's _init delegatecall target, and stop claiming universal coverage Two self-review findings. The gate read only `_diamondCut[].facetAddress` and ignored `_init`, which the diamond delegatecalls in the same transaction — code running against the diamond's storage exactly as a facet's does. Every real cut sets it to the facet being added, so attributing it costs nothing legitimate and an `_init` pointing elsewhere no longer passes unexamined. The module and the process doc both claimed every Safe proposal reaches this gate. Five bespoke task scripts call `storeTransactionInMongoDB` directly and do not; none of them encodes a `diamondCut`, which is why coverage is unaffected, but the claim was wrong as written. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(deploy): close the unknown-wrapper bypass and the ambient off-switch (EXSC-929) Gate-review round 1. Two proven bypasses, both in code that looked finished. `LiFiTimelockController` inherits OpenZeppelin's `TimelockController`, so the singular `schedule` is callable by the Safe even though this repo's tooling only ever emits `scheduleBatch`. Only `scheduleBatch` was unwrapped, so a cut handed in under `schedule` produced an empty facet set and skipped the gate with no output at all. `schedule` is now unwrapped, and — because enumerating envelopes cannot be complete — any call whose bytes contain the `diamondCut` selector but yields no decoded cut is refused rather than treated as innocent. The environment predicate read `process.env.ENVIRONMENT`, which no production caller exports: `helperFunctions.sh`, `diamondUpdateFacet.sh` and `scriptMaster.sh` all keep it function-local, so the value came from whatever was in the operator's shell, and `ENVIRONMENT=staging` there turned the gate off silently. Reaching this funnel for a non-testnet network means proposing to a production Safe and signing with the production key — a staging deploy sends straight to the diamond — so the predicate is gone entirely. Testnets stay exempt, and every skip now says so. Also from the round: the TypeScript `sendOrPropose` signs and stores without either funnel, so it carries the gate call inline rather than relying on a claim that every proposal reaches the funnel; an unreadable deployments log now reads as a gate refusal instead of a bare file error; and the attribution layer (`indexDeploymentsByAddress`, `evmHexAddress`) had no tests at all. `docs/MultisigSigningProcess.md` §4.2 and §9 still described two independent gates and the shell-only scope; both now describe the single call site. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(deploy): prove the third funnel's gate call, and stop a refusal assertion passing on the wrong message Two mutation survivors from the fix round. Removing the gate call from the TypeScript `sendOrPropose` only broke the type checker — no test covered it, which is how two funnels shipped wired to nothing earlier in this work. It now has the same real-subprocess probe as the other two. The unreadable-deployments-log case asserted only that the cause survived, which the unwrapped error also satisfies, so the wrapping that makes it read as a gate refusal was untested. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(deploy): refuse a call on its own bytes, not on its siblings' (EXSC-929) Gate-review round 2, and the defect was inside round 1's fix. The unknown-envelope backstop ran once per top-level call, gated on whether a `diamondCut` had decoded anywhere in that call's whole tree. So a timelock batch pairing one readable cut with one unreadable envelope passed: the readable half set the flag, the backstop never ran, and the envelope's facet install was never decoded, never attributed and never compared against main. Reproduced: scheduleBatch([D, X], [cut(A, Add), multiSend(cut(EVIL, Add))]) -> { addresses: [A], undecodable: [] } The scan now runs where an unrecognised selector falls through, so every branch is judged on its own bytes. That also covers nested envelopes, which the per-top-level version could not reach at all. Correction to the previous commit message, which asserted no production caller exports `ENVIRONMENT`: `script/multiNetworkExecution.sh` does, hardcoded to `production`. That does not change the conclusion — the value it exports can never be `staging`, and the ambient case was the problem — but the claim as written was false, and the comment repeating it is gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(deploy): align the selector scan to byte boundaries, and make the store tripwire cover all three funnels (EXSC-929) Gate-review round 3, both findings in round 2's own fixes. The unknown-envelope scan was a plain hex substring search, so it matched at an odd nibble offset too — a `batchSetContractSelectorWhitelist` carrying a DEX address like `0xa1f931c1ca…` was refused for four bytes that were never a selector, and the message sent the operator off to re-encode a cut that did not exist. Only an even offset can be a selector, so only even offsets count now. Latent rather than live: those four bytes appear nowhere in `config/` or `deployments/` (207 files, 3.1M chars), but the failure would have been total and the diagnostic misleading. The probe tripwire added after a probe queued a real proposal matched "Proposal stored", which only the Tron funnel prints. The EVM funnel prints "Transaction successfully stored in MongoDB" and `sendOrPropose` prints "proposed and stored in MongoDB", so seven of nine cases were unprotected by the check that exists precisely because of that incident. Matched case-insensitively on the shared phrase instead. Docs: three statements contradicted the code they shipped beside — the exemption list still named staging, the summary table still said staging is not gated, and §4.2 claimed one call site where there are three. Also states two limits plainly now: the backstop reaches only a verbatim byte-aligned selector, and drain removals are appended after the gate runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(deploy): re-gate the direct-broadcast route the funnel never reaches (EXSC-929) Gate-review round 4. Moving the gate into the proposal funnel silently dropped a production path, and this PR's own acceptance row measured the drop as a success. On main the shell gate's condition was independent of the propose/direct branch, so `ENVIRONMENT=production` with `SEND_PROPOSALS_DIRECTLY_TO_DIAMOND=true` on a mainnet network was gated even though the cut is broadcast straight from the deployer key. The funnel gate is keyed on proposal calldata and never sees that route, so the bring-up window it exists for could install unmerged code on a production diamond with nothing compared against main. `assertDirectBroadcastDeployGate` restores exactly the lost coverage and nothing more: it runs only where `SHOULD_PROPOSE_TO_SAFE` is false, so the two gates are disjoint by construction and the propose route keeps its single evaluation per proposal. Placement is asserted, not just the decision — the refusal has to land before forge broadcasts, and the passing case has to prove it really took the direct branch, or an absence assertion would pass on a run that never got there. Raised as D23 on EXSC-883: whether D9's "never in two places" permits two route-disjoint gates. Shipped in the safe direction rather than parking it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(deploy): make the store tripwire reachable, and pair every absence assertion (EXSC-929) Gate-review round 4, everything outside the direct-broadcast regression. The tripwire added after a probe queued a real proposal was evaluated AFTER the timeout check, and the Tron funnel is documented to hang on an unclosed Mongo connection once an insert succeeds. So a probe that actually wrote was the probe reported as "killed by SIGTERM, so its output proves nothing" — the one outcome the tripwire exists to catch was the one it could not see. Store check first now. Three absence assertions proved nothing. `NEXT_STOP_TRON` was "No Linear ticket supplied", which the harness makes unreachable by supplying the ticket precisely so the run reaches the gate; the marker measured from a passing run is the proposal store refusing an unparseable URI, which is also the last step before a production write. `sendOrPropose` words its key failure differently again, so the EVM marker never appeared on that path either, gate or no gate. Both skip-path cases asserted only absences and would have passed on a run that died earlier. Every absence assertion is now paired with the same marker asserted present in the corresponding pass case, so an unreachable marker fails the suite. `collectInstalledFacetAddresses` read selectors and offsets positionally off a `0x` prefix with nothing checking it had one, so input that was not well-formed calldata was skipped rather than examined — and a skip is a pass. The funnels validate first, but the TypeScript `sendOrPropose` does not. Refused now. `walk`'s boolean return was dead: it fed only itself and was discarded at the top level, left over from the aggregate gating round 2 removed. Its docstring still advertised the value, which is an invitation to reintroduce that bug. Byte-alignment narrows the false-refusal class rather than closing it — an address can carry the selector bytes at an even offset too. Docstring says so and a test pins it, so the two cannot drift. Docs: the gate does not cover "every deploy path by construction"; it covers every path that proposes. The direct-broadcast route and the ungated bash `sendOrPropose` direct branch are both named now, as is the one refusal with no self-service route (a `Replace` cut on a superseded facet name). Correcting round 3's commit message: "seven of nine cases were unprotected" was eight of ten. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(deploy): re-sweep the prose round 4 invalidated, and harden two markers (EXSC-929) Gate-review round 5. Round 4's logic held — the shell gate's guard is equivalent to main's across all 48 combinations of environment, flag and network type, with zero residual regressions — but round 4 re-introduced a construct three comments still describe as retired, and did not re-sweep them. `verify-approvals.ts` said its CLI was "on no deploy path" one commit before the direct-broadcast route started depending on its exit status. The funnel test said the environment-predicate class was "gone", which is true of the funnel and false of the repo, and the PR had deleted the test that pinned the shell condition — so the class came back, a comment said it could not, and its regression test was gone. `deploy-gate-cache.ts` said "once per proposal" for a gate now also invoked on a route that proposes nothing. Two markers could be satisfied without reaching the gate. `NEXT_STOP_SEND_OR_PROPOSE` was the bare "Missing <VAR> in environment" prefix, which `safeScriptHelpers.ts:72` throws on the direct-tx branch — the branch that returns before the gate call. Extended to include the `--ledger` clause, which only the propose route prints, and re-falsified: 2/2 still fail with the gate removed. `spawnCli` discarded `result.error`, so a spawn that never ran left status and signal both null, neither check fired, and every absence assertion in the refusal cases passed on no output at all. The empty-`ENVIRONMENT` case the deleted test carried is restored, alongside `staging2` and `PRODUCTION` — `getPrivateKey` matches "staging" as a substring while the gate matches the exact string, so the gate stays strictly broader than the key it protects, and that is now pinned rather than argued. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(deploy): check the store before the spawn error, not after (EXSC-929) Self-caught while briefing round 6, and it is the previous commit's own fix inverted. `spawnSync` with a `timeout` sets `error` to ETIMEDOUT *and* `signal` to SIGTERM while still returning whatever the child printed, measured: status = null signal = SIGTERM error = ETIMEDOUT: spawnSync bash ETIMEDOUT stdout = "Proposal stored in MongoDB.\n" So the `if (result.error) throw result.error` added in `f927f6f9e` sat in front of the store tripwire and masked exactly the write-then-hang case that round 4 had moved the tripwire in front of `signal` to expose. The Tron funnel is documented to leave its Mongo connection open after a successful insert, so that shape is the expected one, not a corner. The three checks now live in `assertChildIsUsable` with the order stated as the reason the function exists, and four cases pin it — including the one that fails if `error` or `signal` is ever moved back in front. Verified by doing exactly that: only the write-then-hang case goes red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(deploy): label the staging2 case for what it actually proves (EXSC-929) Gate-review round 6. The commit message for `f927f6f9e` got this right and the test comment beside it inverted it, which is the drift the comment-vs-code sweep exists to catch. `getPrivateKey` matches "staging" as a substring, so `ENVIRONMENT=staging2` gets the STAGING key, not the production one — it was grouped under "still gets the production key" with the other three. Its real value is the other half of the argument: the gate compares the exact string, so `staging2` is gated anyway. The gate being broader than the key can only cost a false refusal, where the reverse would be a production deploy nobody checked. Split into its own case, because mutating the gate to substring-match kills that case and only that case — 13 pass 1 fail, verified. `STAGING` takes its place in the group, being the value that literally does hand out the production key. `spawnCli` set no `maxBuffer`, so output past Bun's ~1MB default is truncated and reported as ENOBUFS — which would hide a store breach from the tripwire whatever order the checks run in. The `describe` title still said the environment condition had been retargeted away from `diamondUpdateFacet.sh`, which its own body now contradicts. The surviving `spawnCli` JSDoc omitted `environment`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(deploy): source the task file instead of extracting the function from it (EXSC-929) CodeRabbit's suggestion, and it is simpler than what it replaces. The file holds two function definitions and no top-level code, so sourcing it loads the gate and runs nothing — the `sed`-and-`eval` extraction was guarding against a problem the file does not have. Re-verified that the harness still loads the real function rather than passing because it loads nothing: substring-matching the gate still kills the `staging2` case alone (13 pass 1 fail), and removing the call site still kills all three placement cases (11 pass 3 fail). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Goran Vladika <goran.vladika@gmail.com>
…EXSC-900) (lifinance#2293) * fix(rpc): stop auto-promoting new endpoints, add per-chain failover (EXSC-900) An RPC endpoint added to the RpcEndpoints collection was assigned max(existing priority) + 1, so the most recently added URL always became the chain's primary. fetch-rpcs then wrote only that one URL as live and commented the rest out, and consumers built a plain http() transport from it, so a single throttled endpoint failed every read on the chain. The 2026-09-01 fleet health check hit exactly that: six chains were rate limited off keyless public endpoints with HTTP 429 while a credentialed endpoint sat at the bottom of their list. An audit of the collection found 28 of 88 networks in that state. - add-network-rpc: new endpoints land below every existing one; --priority sets the rank explicitly, and re-adding a known URL no longer promotes it - fetch-rpcs: honour isActive and environment (both were ignored), emit the full ordered list, and warn when a chain's primary carries no credentials - viemScriptHelpers: build a viem fallback() transport over that list - healthCheckAllNetworks: keep the HTTP status in the Slack digest instead of masking it as a count - audit-rpc-priorities: report and repair the collection's ordering - extract the nine duplicated workflow RPC-injection loops into one composite action that masks each URL individually Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(rpc): narrow the connect log back to the primary endpoint (EXSC-900) Putting the fallback list into chain.rpcUrls.default.http widened detect-evm-version's connect log from one URL to every configured URL for the chain, credentials included. Also correct the networkRpcsChecker header, which still told readers add-network-rpc grants highest priority. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(rpc): rank endpoints by reachability before credentials (EXSC-900) Ordering by credentials alone promotes an endpoint that may be expired, revoked or gone. On megaeth all three credentialed endpoints answer 403 or fail DNS, and the chain's only working endpoint is the uncredentialed one — so a credentials-only repair took the chain down. audit-rpc-priorities now probes each endpoint (eth_chainId plus a state read, since a plan that does not cover the chain answers the former and refuses the latter) and ranks reachable above unreachable, credentialed above uncredentialed within each group. --apply implies the probe. Two properties keep the repair from becoming its own source of churn: - unreachable requires every attempt to fail, while one success proves reachability. A single failed request is network weather, and demoting on it reshuffles healthy endpoints. - only a wrong primary is written. Reachability is momentary and stored priority is durable config, so ranking the whole list on a probe made each run disagree with the last about which flaky fallback ranked where. A misordered fallback costs one failed request; the fallback transport moves past it. Priorities are also keyed by position in the stored array rather than by URL — a chain can list one host twice under different keys, and a URL-keyed map collapsed those into a tie the repair could never resolve. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(rpc): state what the endpoint probe does not establish (EXSC-900) The probe shows an endpoint serves state reads from the caller's location. It does not clear it for every method — gravity's primary answers eth_getCode and still refuses eth_call in CI — and reachability can differ by caller IP, so passing here is not a promise it works from a runner. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(rpc): address CodeRabbit findings on endpoint identity and credentials (EXSC-900) - getTransportConfigFromRpcUrl refuses to attach basic auth to a cleartext http URL. The check first sat inside the block's own try/catch, which swallowed it and returned the raw URL — the exact outcome it exists to prevent — so only the URL parse is guarded now. Covered by a test that fails against the swallowed version. - add-network-rpc matches an existing endpoint on URL and environment. URL alone let a staging invocation rewrite the production record's environment and drop that endpoint out of production selection. Records predating the field count as production, the default the writer has always applied. - fetch-rpcs fails instead of falling back to networks.json for a non-production environment: that file holds one public URL per network with no environment split, so serving it would be a silent wrong-environment success. No live instance of either data hazard today — zero endpoints carry credentials over http, and no URL appears under more than one environment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(rpc): pin the endpoint-identity match (EXSC-900) Extract the URL+environment match out of add-network-rpc so the rule that a staging write must not adopt the production record is covered rather than asserted, including the legacy-record-counts-as-production case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(rpc): narrow what the repair writes and what the probe contacts (EXSC-900) Second CodeRabbit round, all three findings valid: - repairOrder returned a credential-ranked order even when every endpoint was unreachable, so --apply could move the primary during a total outage on no evidence. It now returns the input unchanged in that case. The existing all-down test missed this: both its endpoints were credentialed, so the branch never ran. Added the mixed keyed/keyless case, which fails against the previous ranking. - The apply path wrote every repaired rank, persisting probe-derived fallback order over the operator's — contradicting the comment two lines above it saying only a wrong primary is worth a write. It now lifts the primary and touches nothing else, which drops prioritiesFor as dead code. - probeEndpoint fetches URLs directly, a path the transport-level guard does not cover, so it refused nothing. It now declines to contact a credential-bearing non-https URL. Asserted by request count against a real server, since returning false while still sending the request would have leaked the credential anyway. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(rpc): treat a password-only RPC URL as credential-bearing (EXSC-900) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(rpc): keep a chain up when one endpoint is unusable, probe the URL callers use (EXSC-900) Two review findings, both of which only became reachable once this PR made every stored endpoint live rather than just the primary. getFallbackTransportForChain built its transports with an eager .map over every endpoint, so a single credential-bearing http fallback threw and took down a chain whose primary was healthy — the inverse of what failover is for. Unusable endpoints are now skipped with a warning naming the reason, and the throw is reserved for a chain with no usable endpoint left. The reachability probe POSTed a JSON-RPC body to the stored URL. Tron stores the TronGrid root, which serves the native HTTP API and answers JSON-RPC only under /jsonrpc, so tron and tronshasta could never pass. URL shaping moves to rpcEndpoints.ts, the probe now targets the URL callers really use, and results are reported back under the stored URL the audit keys on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(rpc): put the JSON-RPC route on the path and scope reachability to a chain (EXSC-900) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(rpc): route every Tron JSON-RPC caller through the shared normalizer (EXSC-900) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ifinance#2339) The weekly coverage report has failed since the first run that included UpdateScriptBase.t.sol. Its eight cases that compute real cut calldata reach contract-selectors.sh through vm.ffi, and that script reads the facet's methodIdentifiers off the build artifact in out/. forge build and forge test write that tree; forge coverage compiles instrumented sources through a pipeline that never does, and the workflow also passes --force, so under coverage the artifact is absent and the FFI can only fail. Being deterministic, it defeated all ten retries. Guard those cases on the artifact being present, and grant fs_permissions read on ./out/ so the test can make that check. The guard stays out of UpdateScriptBase.getSelectors: that is the deploy path, where a missing artifact must keep failing loudly, since an empty selector list would otherwise encode as a valid no-op diamond cut. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
lifinance#2332) * feat(deploy): gate the zk, direct-build and Tron toolchains (EXSC-933) EXSC-932 gated the EVM toolchain at executeAndParse, which every `forge script` passes through. Four build paths do not. zkEVM: verify-zk-toolchain.sh is the single verdict for the foundry.toml [external.zksync] pins, and assertZkToolchainOrFail is the bash seam that delegates to it and fails closed. It runs inside install_foundry_zksync, which every script reaching for ./foundry-zksync/forge calls first, so no build site can miss it. scriptMaster.sh and diamondUpdateFacet.sh discarded that function's status and now abort on it. Direct `forge build` sites: scriptMaster's startup compile, updateFoundryTomlForGroup (ahead of the foundry.toml rewrite) and ensureStandardArtifactForSalt now call assertFoundryVersionOrFail. Tron: deployments run from pre-built Forge artifacts with nothing checking which forge produced them. assertTronToolchainOrThrow delegates to the same verify-foundry-version.sh and sits in assertTronDeploymentRecordable, which every Tron deploy site calls immediately before deployer.deployContract. It resolves the checker against the working directory so the contracts-tron checkout gates itself with its own pin. deploy-smoke-test's path filter did not list script/helperFunctions.sh, the chokepoint every deploy passes through, so a change to it could merge without the pipeline test running. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(deploy): close the gaps this PR's own gate agents found (EXSC-933) Five findings, two of them the shape where a test passes because of the bug. **An ungated Tron deploy site, invisible to the invariant meant to catch it.** `deploy-safe-tron.ts` broadcasts the Safe singleton and SafeProxyFactory with no recordability assert. The test counted asserts and deploys across all files, so 12 >= 11 held — made up by the function definition in `tronUtils.ts` and an import in a third file — and a file with zero guards could not fail it. Per file now, naming the offender, and both sites are gated. **The unconditional group gate reversed lifinance#2325's stated decision.** `updateFoundryTomlForGroup`'s tolerant mode swallows build failures for the playground runner, and multiNetworkExecution.sh's two callers rely on that return contract; lifinance#2325 examined this and declined for that reason. Scoped to STRICT, where a mismatch still refuses before foundry.toml is rewritten, and the per-network gate in deploySingleContract still refuses the deploy paths. A test covers the tolerant path, which had none. **The zksolc comparison was a substring test**, so pin 1.5.15 accepted a 1.5.155 toolchain. Anchored on the value's closing quote, with the neighbouring versions and the exact pin all covered. **The zksolc leg checks a request, not an observation.** `FOUNDRY_ZKSYNC` is built by helperFunctions.sh from the same foundry.toml line, so on the deploy path it compares the pin against itself, and the success line printed the pin as though it had been observed. The two legs are now labelled differently and the header states what each proves. Reading the compiler from zkout/ metadata is what would make it an observation; that is not claimed here. **The chokepoint test filtered a hardcoded six-file list**, so a brand-new script reaching for the zk forge was invisible — the same shape as the first finding. It walks `script/` now and fails by name on a new ungated file. The count was also wrong: five files invoke the zk forge, scriptMaster.sh delegates. Also: the Tron memo short-circuited before resolving `repoRoot`, so a pass for one checkout authorised another (unreachable today — no call site passes a root); and `assertTronToolchain.ts` claimed to confirm the toolchain that built the artifacts when it checks the one about to run. 2628 pass / 0 fail, tsc clean, eslint 0 errors. * fix(deploy): the gate I added refused every run; and parse the zksolc pin (EXSC-933) Gate round on 049b072, which was itself a fix round. Four findings, one of them a false refusal I introduced in that very commit. **The Tron gate would have blocked the next real Safe deploy.** `deploy-safe-tron.ts` got `assertTronDeploymentRecordable`, which validates constructor arguments against the artifact ABI — and the committed `SafeProxyFactory` build declares no constructor while its own source declares `constructor(address _singleton)`. Loading the real artifact and calling the real function throws `expects 0 constructor arguments, got 1`, on every run, independent of environment. It was also the wrong check: both artifacts are third-party, built by `forge build -C safe/london`, and never written to the deployment log, so recordability is not the applicable question. They carry `assertTronToolchainOrThrow` now, which is what the ticket asked for, and the per-file invariant counts either gate with the reason recorded. Closing a false green by opening a false red, in one file, in one commit. **The zksolc check was still satisfiable by decoration.** Anchoring on the value's quotes only required the pinned string to appear somewhere, so `{ zksolc = "9.9.9", other = "1.5.15" }` and a trailing `# "1.5.15"` comment both read as pinned. The key's value is parsed and compared now, two zksolc keys are refused rather than one of them picked, and the success line prints what was parsed instead of the pin. Both cases are covered; reverting to the anchor fails them. **The chokepoint walk passed on a coincidence.** `verify-zk-toolchain.sh` matched because it names the binary in a comment and `install_foundry_zksync` in a fix hint — reword either and it flips to a false failure with nothing structurally changed. Skipped by path, as the checker rather than a build site. The walk also covers `.ts` now: a zk build started with `spawn()` was invisible to a shell-only scan, which is the same gap this test exists to close. 2630 pass / 0 fail, tsc clean in every touched path, eslint 0 errors. * fix(deploy): require an exact zksolc key, and await Tron gate rejections (EXSC-933) `notzksolc = "<pin>"` counted as a zksolc request. Token-bound the parser. Await the logging-path rejections so a failed matcher cannot pass the suite. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…940) (lifinance#2328) * fix(ci): diff changed files against the merge base in PR gates Both gates computed the PR's changed files with a two-dot diff (origin/main HEAD), which also reports the commits main gained since the branch point. Any PR branched before an unrelated .github/ or src/ change merged was therefore judged on files it never touched. protectSecurityRelevantCode demanded an Information Security Manager approval on such PRs (observed on lifinance#2327); versionControlAndAuditCheck flagged untouched contracts as needing a version bump and an audit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(ci): make the merge-base failure path reachable and its message honest `set -e` aborts on the failing command substitution before an `if [[ -z ]]` guard can run, so the merge-base error message never printed. Move the guard onto the assignment itself. The protect gate's "No files found" abort now also states the case a merge-base diff makes reachable: a PR whose net change is empty. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…t logs (EXSC-695) (lifinance#2331) * feat(deploy): record branch, scoped dirty tree and actor on deployment logs (EXSC-695) Adds gitBranch, dirtyTreeScoped, dirtyTreeTruncated and actor to IDeploymentRecord, captured through the shared captureGitProvenance pass the Safe proposal document already uses. * fix(deploy): never record an unreadable working tree as a clean one * fix(deploy): make the provenance capture and its tests observe what they claim (EXSC-695) Gate-review round: kebab dry-run spelling now resolves instead of silently writing; dirty-tree readability is judged by its own probe rather than the shared error bag; the CLI tests no longer inherit the runner's workflow environment; the capture gains the runner seam its unit tests needed. * fix(deploy): one provenance capture, and keep a dirty tree once recorded (EXSC-695) The record path re-read git status and HEAD, dropping GITHUB_SHA and the sanitized list. A later clean re-log also $set an empty dirty list over the tell-tale. Both go through captureGitProvenance now; empty dirty is insert-only. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(deploy): isolate git identity in provenance CLI tests (EXSC-695) The no-checkout case was asserting actor=human from the laptop's global user.name. CI runners have no identity, so the same probe recorded UNKNOWN and the suite failed. Isolate HOME/GIT_CONFIG so both environments agree. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Goran Vladika <goran.vladika@gmail.com>
…EXSC-943) (lifinance#2337) * fix(health-check): safe-config asserts the Safe owner set both ways (EXSC-943) `safe-config` compared the configured owners against the on-chain set in one direction only: every owner in `config/global.json` had to be an owner of the Safe. It never iterated `getOwners()`, so an owner *added* to the production Safe passed the daily check in silence — every configured owner was still present and the threshold was still at or above its floor. That is a governance change the check is named for. With six owners and a threshold of 3 you need three of six; with an added seventh you need three of seven, and the added key is one of them. Both directions are now asserted, which pins the set exactly and so subsumes an owner-count assertion. Two ways the comparison can be unable to run are reported as errors instead of being rendered as agreement: an empty `safeOwners` list, which is a subset of every on-chain set there is, and a config entry that does not normalise to an address, which would otherwise make an on-chain owner it fails to match look like an intruder. The pre-existing direction's message is corrected — it read "not in SAFE configuration" for an owner that was configured and missing on chain, pointing a responder at the wrong file. Both messages now name a set and a side. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(health-check): close the safe-config gate round's findings (EXSC-943) A blank or absent `safeOwners` entry was dropped by `if (!entry) continue`, which is a second route to the incomplete configured set the unparseable-entry branch exists to refuse — and it skipped that branch entirely. Blanking one of the six config lines therefore turned the daily check red on every production network naming a *legitimate* signer as an unexpected owner of the Safe, sending the responder to the Safe instead of to the blank config line. Falsy entries now take the same route as unparseable ones. A duplicated config entry is also outside set equality: the two sets agree while config claims one more owner than the Safe has, so asserting both directions does not subsume an owner-count check as claimed. Duplicates are now reported. This only adds refusals, so it cannot open a false green; the real config has six distinct entries and is unaffected. The "no SAFE address configured" exit used a raw `consola.warn`, so a network that skipped the check reached the run summary looking like one that passed it. It now goes through `ctx.logWarn`, as the 25 other warn sites in the file do. Two guards were untested and survived mutation: deleting the falsy-entry skip, and deleting the `mismatches` counter that stops a refusal from also printing "owner set matches". Both now have tests, the second asserted on the rendered output because that is where the two lines can contradict each other, with a paired positive so it cannot pass on a run that printed nothing. Comment discipline: the test docstring and four test comments argued the PR's case in past tense, and one implementation comment restated the PR body. All now state the present constraint or are gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(health-check): pin the match-line suppression on every refusal route (EXSC-943) The counter that stops a refusal from also printing "SAFE owner set matches config/global.json" was asserted on one of its three routes. Mutating `report` to `ctx.logError` at the unusable-entry and duplicate call sites left the suite green while both printed the refusal and the match line together — the contradiction the counter exists to prevent. The test now drives all three routes and keeps its paired positive. The duplicate refusal's message claimed "so its owner count does not match <safe>", which is false whenever the counts still agree: three entries naming two distinct owners against a three-owner Safe printed it while both counts were three. It now states what is always true of a duplicate — the list names fewer distinct owners than it has entries. Unusable entries are rendered through `sanitizeProvenanceText`, which subsumes the empty-string special case: a whitespace-only entry reached the operator as an invisible gap between two commas, and the value lands in a terminal and a job log unescaped. Two comments argued against code that is no longer there ("rather than being dropped", "a dropped entry"), which nothing in the file records. Both now state the present constraint. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(health-check): give the invariant registry its docblock back (EXSC-943) `describeConfigEntry` was declared between the registry's JSDoc and `HEALTH_CHECK_INVARIANTS`. TypeScript attaches every leading docblock to the next declaration, so the registry's documentation became documentation on a string-render helper and the registry itself had none. The comment above the duplicate refusal also still said config "claims one more owner than the Safe has", where the message beneath it was generalised because that is false at two or more duplicates. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(health-check): name an unusable owner entry by position, not by rendering `sanitizeProvenanceText` does not strip every invisible character. Measured: a zero-width joiner and a Hangul filler both survive it at zero width, while a zero-width space, a word joiner, a soft hyphen, a BOM and a non-breaking space do not. So a fallback keyed on "did this render to nothing" cannot catch an entry that renders to something invisible, and two such entries are the same text in an operator's terminal. Stripping the joiner would leave the filler, so the description no longer depends on the rendering being visible: it carries the entry's position in `safeOwners` and its length, which identify the line to fix whatever the characters are. That is the same reason `delegatecall-gate.ts` carries a length beside its rendered value. Reported by CodeRabbit on U+200D specifically; the earlier gate round's probe had recorded that case as already handled, and it was not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(health-check): pin the sanitiser on an unusable owner entry Removing `sanitizeProvenanceText` from the entry description survived the suite: nothing asserted that a control character cannot reach the terminal or the job log raw, so a refactor could have dropped it silently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Goran Vladika <goran.vladika@gmail.com>
Co-authored-by: aikido-autofix[bot] <119856028+aikido-autofix[bot]@users.noreply.github.com>
…y (EXSC-878) (lifinance#2336) * feat(deploy): bound the deployer key's power to a documented inventory (EXSC-878) The deployer wallet appears at every stage of a production change: it deploys, writes the deployment record, creates the proposal, holds one Safe owner slot and holds CANCELLER_ROLE on every timelock. R7.1 asks for that set to be enumerated, bounded, and alerted on when it widens. script/deploy/deployer-key-power.ts carries the inventory and the assertion. The config leg walks the whole of config/global.json and config/networks.json rather than a list of known field names, so a field added after the check was written is caught too. The bound leg re-derives the claim the inventory rests on: one owner slot cannot reach SAFE_THRESHOLD, so the key cannot schedule or push an operation. The inventory leg refuses if any power marked as held in production steady state is classed as an integrity power. Two scoped exceptions are recorded in the inventory rather than hidden by it: on testnet and staging the deployer owns the diamond outright, and during production bring-up it owns a production diamond between transferOwnership and the Safe-executed confirmOwnershipTransfer. Timelock execution is carried as pending, because EXECUTOR_ROLE is still address(0). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(deploy): close the deployer-key-power inventory's vacuous cases (EXSC-878) The inventory omitted a power that falsifies its own headline claim, and the config walk could report bounded while observing less than it claimed to. Inventory - Add `safe-deployment`: the deployer can deploy the governance Safe and pick its owner set and threshold. `deploy-safe.ts` unions `--owners` into `globalConfig.safeOwners`, accepts any `--threshold` >= 1, defaults `allowOverride` to true so the "Safe already deployed" guard needs a flag to fire, and rewrites `config/networks.json` with the result; its on-chain check compares `getOwners()`/`getThreshold()` against the same expanded arguments it was handed. `deploy-safe-tron.ts` is the Tron equivalent. Classed `integrity` / `production-mainnet` / `current`, so the "DoS only" claim is now scoped to an already-deployed Safe. - Correct `testnet-diamond-owner` to testnets only. Staging on a mainnet network is a different key: `getPrivateKey` returns `PRIVATE_KEY`, `healthCheck.ts` resolves `ctx.deployerWallet` to `devWallet`, and the `diamond-owner` invariant skips staging. Assertion legs - `networksConfig` now needs a non-empty plain object, matching the existing `globalConfig` guard; `undefined`, `null` and `{}` walked one file and passed. - Match addresses used as object keys, not values only, so an address-keyed grant table is a violation. - Resolve the `41`- and `0x41`-prefixed TronWeb hex forms of the EVM identity. Base58 stays a disclosed residual. - The integrity leg refuses a production-mainnet integrity power unless `ACKNOWLEDGED_PRODUCTION_INTEGRITY_POWERS` names the check that observes it. - Bind the scope into the verdict line: which claim, which two files. Relaxations, and what each starts trusting - `safeOwners` count refuses at >1 instead of !=1; 0 slots is a reduction of power and was a false red. This stops detecting drift in the safe direction: a config with the deployer absent from `safeOwners` is now trusted as intentional, and the `one-signature` row would go stale silently. - The integrity leg no longer asserts the production integrity set is empty. It trusts an explicit id allowlist plus a non-empty detection string, so a disclosure is a second deliberate edit rather than a field on the row being added. Also make the intended `config/global.json declares no deployerWallet` guard reachable: it keyed off `identities.length`, which a surviving `tronWallets.deployerWallet` kept at 1. * docs(deploy): tie the disclosed integrity power to its removal (EXSC-878) `ACKNOWLEDGED_PRODUCTION_INTEGRITY_POWERS` bounds the Safe-deployment power by detection, which is weaker than removal — and an open-ended disclosure is how a temporary carve-out becomes permanent. Each entry now names the work that retires it (EXSC-944), and both the docstring and the docs state that an empty map is the target state, so R7.1's bound is not read as fully held. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(deploy): recognise every spelling of the deployer's address (EXSC-878) `matches()` compared a candidate against the identity as raw lowercase text, so an address written without `0x` was never equal to the prefixed identity and the walk simply did not see it. The bound claims nothing outside the documented slots, so a spelling it fails to recognise is a slot it cannot claim anything about: `pauserWallet: <deployer body, unprefixed>` passed. Both hexadecimal operands are now compared on their body, and `isHexAddressForm` matches the prefix case-insensitively — without that second half a `0X` form still failed the form test and fell through to the verbatim comparison, which is the same escape by a different spelling. Seven spellings now refuse: unprefixed, either digit case, `0X`, and both Tron `41`-hex forms. Only recognition widens, so no configuration that passed for a good reason stops passing: the committed config still passes, an unrelated address still passes, and removing the deployer from `safeOwners` still passes. Reported by CodeRabbit on the unprefixed form; the `0X` half was not in the report and is what made the first fix incomplete. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…s (EXSC-944) (lifinance#2338) * feat(deploy): gate Safe deployment on config, not on its own arguments (EXSC-944) `allowOverride` defaults to false, matching the header the file already carries, so replacing a Safe address config names has to be stated. A production threshold below the shared SAFE_THRESHOLD refuses; testnets, which carry no Safe in committed config, keep a floor of 1. The post-deploy owner check compares getOwners() against config/global.json safeOwners as well as against the requested set, and reports a divergent owner set as a deliberate override. The owner-set divergence ships as detection and loud reporting only. Whether it should refuse outright or require a typed confirmation is items 4-5 of EXSC-944. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(deploy): compare against config before the check that forces equality (EXSC-944) The config comparison ran after the pre-existing requested-vs-actual check, which throws unless the on-chain set equals the arguments this run supplied. So its `absent` direction could not execute at all, and its `beyondConfig` direction only ever restated the `--owners` argument — a value known before any deployment. The whole point of the item was to observe the Safe rather than the request, and in that position it could not. It now runs first, which also makes it meaningful for the one path where the address is not derived from this run at all: the no-`ProxyCreation` fallback takes the Safe address from an operator's keyboard, so the owner set read back is not constrained by the arguments. The floor now requires a whole number. `3.5` and `Infinity` both compared above 3 while being no count of signatures; they get a separate refusal, because telling an operator who passed 1 that their value is not a whole number describes a value that is not the one in front of them. `1e100` is deliberately still allowed here — it is a whole number, and an absurdly large threshold is refused downstream by `threshold > owners.length`. `isDefaultThreshold` matched only the space-separated `--threshold`, so `--threshold=5` printed "Using default threshold of 3" two lines under the new line correctly reporting 5. Pre-existing, but the diff put the two together. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(deploy): pin the normalisers that decide what counts as no Safe Three mutations survived the suite: dropping the `/i` from the zero-address match, and dropping `.trim()` from that match and from the owner comparison. All three are real behaviour changes on inputs that `config/networks.json` does not currently hold, so nothing observed them — and `networks.json` is hand-edited, which is exactly how such a value would arrive. Paired both ways: a lenient reading of the zero address must not make a Safe that exists look absent. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(deploy): pin the owner comparison's trim on both sides Padding only the deployed side left the configured side's `.trim()` unobserved: dropping it survived. Either side can carry the padding — the config side is hand-edited, the deployed side is whatever the node encodes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(deploy): apply the Safe threshold floor to Tron deployments (EXSC-945) (lifinance#2340) * feat(deploy): apply the Safe threshold floor to Tron deployments (EXSC-945) Reuses evaluateSafeThresholdFloor/assertSafeThresholdFloor from safe-deploy-guards, and collapses add-safe-owners-and-threshold's EXPECTED_THRESHOLD onto SAFE_THRESHOLD. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(deploy): name what the setup-only ordering case guards Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(tron): stop truncating the threshold before the floor check sees it The CLI parsed `--threshold` with `parseInt`, so the whole-number refusal added alongside the floor was unreachable from the command line: `3.5` arrived as `3` and passed. Worse in the direction that matters, `--threshold 5.9` arrived as `5` and deployed a Safe weaker than the one the operator asked for, with nothing said, because 5 clears the floor. `Number` keeps the fraction so the guard can judge it. The EVM script already parses this way, which is why its guard is reachable and this one was not. Two inputs change verdict, and both are worth stating: - `--threshold 3abc` was silently accepted as 3 by `parseInt`. It is now NaN and refused. That is the tightening. - `--threshold 0x5` and `--threshold 1e3` now parse as 5 and 1000 rather than 0 and 1. That is a relaxation of the parse, not of the policy: both then face the floor and the owner-count check, and 1000 is refused by the latter. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(tron): kill the truncation route the source-text tests could not see A gate round found that the previous commit shipped ungated, and that its two parse assertions read the *text* of the `Number(args.threshold)` line — so applying `Math.trunc` to the value one line below, where it is handed to `run()`, undid the whole fix with the suite still green. That is a grep-the-source test passing against the obvious rewrite of the same bug. The scan now covers the route rather than the spelling: no `Math.trunc`, `Math.floor`, `Math.round`, `parseInt` or `| 0` may be applied to the threshold anywhere in the file, with a paired assertion that the value reaching the guard is still the parsed one. Two survivors are NOT closed here, and the reason is that the only behavioural proof is unsafe. Wrapping the guard in a condition, or hoisting it past the key read, both survive a suite that asserts placement by source order — and the test that would catch them calls the exported `run()`, where a mutation disabling the guard makes `threshold: 1` proceed into a real Tron deployment with the production key. The test would manufacture the blast radius it exists to prevent. The durable fix is for the guard to return a validated threshold that the deploy path must consume, so an unvalidated number cannot reach it; that is recorded on EXSC-945 rather than bolted on here. Also drops four comments that narrated the previous parse — a counterfactual about code that is gone, which belongs in this message and not in the file. The equivalent EVM line has carried `Number(...)` with no comment at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(tron): match the threshold assignment whole, and assert both of them A bitwise `| 0` survived the scan because the pattern expected the operator to follow `threshold`, where it actually follows the closing paren — and a `toContain` on the prefix passes while the value is truncated after it. The assignment is now matched as a whole line. Asserting only one assignment was wrong: there are two, `run()` taking the number it was handed and the CLI parsing the argument. The whole set is asserted, so a third cannot appear unnoticed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * fix(tron): make the threshold-truncation regex linear (EXSC-944) CodeQL js/redos flagged the nested (?:\s*\)?)* group. A character class matches the same truncation shapes without exponential backtracking. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…s (EXSC-920) (lifinance#2327) * fix(tron): pre-flight the fee limit on the three direct-EOA send paths (EXSC-920) troncast send (raw calldata and signature forms), the tronUtils facet registration and the direct transferOwnership all broadcast without comparing the estimate against the fee limit they run under. Route each through a shared guarded-send seam that reuses assertTronBroadcastAffordable, and move the two Tron energy modules out of script/deploy/safe/executors/ now that most of their callers sit outside the Safe path. * docs(tron): correct the pre-flight docstrings and refusal hints The moved modules' headers still described the Safe paths only, the shared seam claimed every Tron send already goes through it, and the diamondCut hint named a TRX constant with a SUN figure. * fix(troncast): make --feeLimit and --dryRun reach the pre-flight citty resolves the spelling of a multi-word argument that the caller did not type to that argument's `default`, so `--fee-limit 5000` left `args.feeLimit` at "1000" and `--dry-run` left `args.dryRun` at false. The first made the new pre-flight compare against a cap the send does not run under and made its own "re-run with a higher --feeLimit" hint a no-op; the second broadcast a run the operator asked to simulate. Dropping the citty defaults, with the fallbacks already applied in the body, makes both spellings carry the passed value. Also rejects a valueless --fee-limit, which citty passes through as `true` and `Number(true)` would have capped the send at 1 TRX. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(tron): guard the diamondCut path the deploy runbook actually invokes registerFacetToDiamond in tronUtils.ts has no callers anywhere in the repo, so the pre-flight added there protected nothing. The batched diamondCut that the deploy runbook runs lives in register-facets-to-diamond.ts, which estimated the energy, printed the cost and then sent at a hardcoded 5000 TRX without comparing the two — the exact shape EXSC-920 describes. It now goes through sendGuardedTronContractCall, reusing the estimate it already computes. Also on this round: - troncast send's raw-calldata estimate posts `visible: true`, so it needs base58; isValidAddress also accepts `0x…` and `41…`, which the broadcast normalises. A hex address was refused where it used to work. - The signature path's call value is converted from the SUN string without passing through Number first, so a value above MAX_SAFE_INTEGER still reaches the estimate's rounding guard, and a malformed --value refuses with a message instead of a BigInt RangeError. - TRANSFER_OWNERSHIP_FEE_LIMIT_SUN raised to 20 TRX: at mainnet's 210 SUN/energy the previous 10 TRX bought 47,619 energy, and the estimate's 1.2 margin would have consumed the headroom a working send relied on. - The owner-mismatch prompt and .agents/commands/interact-tron.md described post-broadcast failures that the pre-flight now refuses first. - The diamondCut tests move into tronUtils.test.ts, beside the module they cover; typing sendGuardedDiamondCut's tronWeb parameter (instead of `any`) is what surfaced its missing defaultAddress use. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(troncast): name the documented flag spelling in the fee-limit refusal script/troncast/README.md and .agents/commands/interact-tron.md write every flag in kebab form, and so do the shell callers, so the refusal that tells an operator to re-run with a higher limit should name --fee-limit rather than the camel spelling. Both resolve to the same argument now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(tron): the guarded fee limit has to be fixed before the estimate The energy-estimation rules sanctioned deriving a fee limit from an estimate and, one section later, comparing an estimate against a fee limit, without saying that a limit derived from the estimate it is checked against grows with it and can never fail the check. Adds the fixed-limit requirement and marks `estimateEnergyAndFeeLimit()` as the alternative to a pre-flight rather than a companion to one. Also corrects two fee-limit figures in transfer-ownership-to-timelock.test.ts left stale by raising TRANSFER_OWNERSHIP_FEE_LIMIT_SUN to 20 TRX. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(tron): --dry-run broadcast for real on two deploy scripts The same citty defect the last commit fixed in `troncast send` was live in `transfer-ownership-to-timelock.ts` and `register-facets-to-diamond.ts`: for a multi-word argument citty resolves the spelling the caller did NOT type to the argument's `default`, so `--dry-run` left `args.dryRun` at `false` and the ownership transfer went through for real. `--delay-seconds` and `--no-propose` were dropped the same way. Both scripts now declare those flags without a default and read them through `flagIsOn`. `flagIsOn` is new in `cli-flags.ts`, the module that already exists for citty flag ambiguity, so the coercion has one definition rather than three. It counts anything present that is not an explicit `false` as on, because `--dry-run <token>` hands the token through as the flag's value, and reading that as off broadcasts a run the operator asked to simulate. Its tests run real citty over both argument shapes, including the one that carries a default, since a test that builds `args` by hand cannot see this class at all. Also: - `parseValue` now rounds the TRX→SUN multiplication. `4.1tron` stringified as 4099999.9999999995, which is not a SUN amount: TronWeb's integer validator refused it at the broadcast, and the new call-value parse refused it earlier with a message that blamed the operator. - A valueless `--feeLimit`, which citty resolves to `''`, is refused rather than silently applying the 1000 TRX default the operator was overriding. - The owner-mismatch prompt no longer promises a refusal that ALLOW_GAS_ESTIMATE_FALLBACK can override. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(tron): the guarded sends live where a test can reach them in CI Both new suites imported the CLI script they cover, whose chain reaches `demoScriptHelpers.ts` and from there `typechain` — a generated directory the ts-tests job does not build. So `run-ts-tests` failed with `Cannot find module '../../../typechain'` while the same suites passed locally, where a stray `typechain` symlink in the worktree was standing in for it. `sendTransferOwnership` and `sendGuardedFacetRegistration` move into `send-transfer-ownership.ts` and `send-guarded-facet-registration.ts`, which import only the pre-flight and the constants they need. The two suites are renamed to match, so each once again sits beside the module it covers. Verified by hiding the symlink and re-running: 2445 pass / 0 fail with and without `typechain` present, against the 2434 / 2 that CI reported. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(troncast): parse a TRX amount exactly instead of rounding it Rounding the TRX→SUN multiplication fixed the float artefact on ordinary amounts but lost sub-SUN ones: `0.0000004tron` is 0.4 SUN and became `0`, so a nonzero `--value` would have broadcast a zero-value call. Found by CodeRabbit. The TRX form is now shifted digit-wise, which is exact for every amount and needs no float at all: `4.1tron` is 4100000 SUN, `1000000000000000tron` is a literal 22-digit string rather than `1e+21`, an amount finer than one SUN is refused as having no such unit, and a malformed or negative amount is refused instead of reaching the node. That last case also means the call-value estimate's MAX_SAFE_INTEGER guard can finally fire, since the value no longer loses precision before it gets there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(tron): compare the fee limit against a 1.2 margin, not a 10x one (EXSC-920) The guard refused at roughly a tenth of the true threshold, and this PR's own rule text said it should not. `estimateDiamondCutEnergy` applied `DIAMOND_CUT_ENERGY_MULTIPLIER` (10) as its safety margin, and `register-facets-to-diamond.ts` hands that same inflated figure to `sendGuardedFacetRegistration` as the affordability comparison. So a 6,000,000-energy cut — which costs about 720 TRX at 100 SUN/energy, comfortably inside the 5000 TRX limit — was refused. That is a routine multi-facet size, so the failure is a live false refusal rather than a corner case. Meanwhile `.agents/rules/202-tron-scripts.md:41`, added by this same PR, says: "Always apply the devkit's DEFAULT_SAFETY_MARGIN (1.2) so the deploy path and the pre-flight guard agree on what a call costs." Code and doc disagreed by 8.3x, in the same changeset. The shape that is actually right: the margin belongs on the figure the guard compares, and the headroom belongs in `DIAMOND_CUT_FEE_LIMIT_SUN`. Putting a multiplier in both places does not make anything safer — it makes the guard refuse work that the limit can pay for, while printing energy and SUN figures ten times the truth and telling the operator to raise the limit ten times too far. Its test encoded the inflated behaviour as correct, calling a 6,000,000-energy estimate unaffordable against 5000 TRX. Rewritten from the price rather than from the code: at 100 SUN/energy the limit buys 50,000,000 energy, so with the 1.2 margin the break-even raw estimate is about 41,666,667. Affordable is now 6,000,000 and unaffordable 60,000,000, and both figures are derived rather than observed. `DIAMOND_CUT_ENERGY_MULTIPLIER` had no other user and is removed, so nobody re-applies it to a comparison later. Proven not to have disabled the guard: neutralising `assertTronBroadcastAffordable` still kills 27 tests across the tron and troncast suites. 316 pass / 0 fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(troncast): price the selector the broadcast resolves, and allow a loopback estimate Two ways the new Tron fee-limit pre-flight refused a send that had worked. The signature path built the estimate's selector by joining the input types out of `parseFunctionSignature`, which canonicalises nothing and returns whatever was typed. The broadcast resolves `contract[name]` against the fetched ABI and TronWeb formats that selector through ethers, so `transfer(address,uint)` estimated 0x6cb927d8 while the send used 0xa9059cbb: the simulation reverted and the guard hard-refused. The selector and the parameter types now come off the ABI entry `contract[name]` is bound to, so there is one source of truth rather than a conversion table to drift from the encoder. A name the ABI does not carry, a different arity, and an overload the bare name cannot reach are refused by name before the estimate rather than guessed at. The calldata path's HTTPS requirement also refused `--rpcUrl http://localhost:8090`, while the signature path on the same flag was unaffected. The restriction is about an on-path attacker forging `energy_used` to steer the pre-flight past the fee limit, and loopback has none, so plaintext HTTP to localhost, 127.0.0.1 and [::1] is allowed and every other host still needs HTTPS. The refusal now names the rule it applied and the escape hatch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(tron): match the loopback allowance to the host form URL actually reports `URL.hostname` lowercases and brackets, and collapses an expanded IPv6 literal, so the bare `::1` in the allow-list was unreachable and the `toLowerCase()` on the lookup was a no-op. Both dropped, with `[0:0:0:0:0:0:0:1]` and an unparseable endpoint added to the suite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(tron): record that a 429 after retries fail-closes a pause (EXSC-920) A transport failure is the same refusal as an unaffordable estimate, including emergencyPauseBreakGlass. The hatch stays ALLOW_GAS_ESTIMATE_FALLBACK=tron. Co-authored-by: Cursor <cursoragent@cursor.com> * refactor(tron): drop the unreachable registerFacetToDiamond path (EXSC-920) Nothing in the repo calls `registerFacetToDiamond`. The deploy runbook registers facets through `registerFacetsBatch`, so guarding the unreachable copy left two near-identical diamondCut guards in review, only one of which any caller can reach. `estimateDiamondCutEnergy` stays: `register-facets-to-diamond.ts` uses it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(tron): drop IDiamondRegistrationResult (EXSC-920) The deleted registerFacetToDiamond was its only consumer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(script): make the kebab spelling of every multi-word CLI flag reachable (lifinance#2330) * fix(script): make the kebab spelling of every multi-word CLI flag reachable A multi-word citty argument that declares a `default` exposes two keys, and citty's proxy only falls back from the key a command body asks for to the other spelling while the asked-for key is absent. A `default` puts it there unconditionally, so the value a caller typed under the other spelling sits on a key nobody reads: `--dry-run` against `dryRun: { default: false }` read as off and the run broadcast. Drops the `default` from all 48 such arguments across 26 commands and applies each fallback in the command body instead, plus a static guard in validateScripts so the shape cannot come back. * docs(script): carry the removed defaults into the flag descriptions Dropping the citty `default` also drops it from `--help`, so the flags whose fallback is not the obvious one — `--is-production`, `--ledger-live`, `--repo-root`, `--optimizer-runs` — now state it in their description. * fix(script): read a repeated or numeric flag value as on, and widen the guard `flagIsOn` tested for the shapes that mean on, but citty's set of them is open: `--dry-run 5` arrives as the number 5 and `--dry-run --dry-run` as `[true, true]`, both of which read as off — a real broadcast for a doubled or numerically followed `--dry-run`. It now tests for the shapes that mean off instead. The guard missed three of citty's four word separators (`_`, `.`, `/`), could not see an args block assembled from a shared const (the shape repair-deployment-records.ts uses, and where the next flag would land), and was skipped on a manifest-only change — which is exactly a citty bump. Its positional exemption kept, with the real reason: dropping a positional's default makes it required. Also: `--delay-seconds 0` arrives as a falsy 0, so deploy-core-facets keys its fallback on `undefined`; and deploy-safe's --allowOverride docs and error message said the opposite of what the flag does. * fix(script): refuse a repeated boolean flag, and scope the guard's const lookup `flagIsOn` read every array as on, so a doubled negation (`--allowOverride=false --allowOverride=false`) came back ON — the direction that permits the write. Reducing an array has to pick a winner and the safe winner flips per flag, so it now refuses, as `readBooleanFlag` already does. The guard resolved a shared args const by walking the whole file and keeping the last match, so a same-named const inside a function could fail a push over a line the command never reads. It now looks only at module scope, sees through `as const` / `satisfies` on the block, and follows a spread inside an argument's own object. Its cycle guard, whose absence crashes the check outright, is now tested. Also documents check 3 in the workflow header, names the state rather than one of five spellings in deploy-safe's messages, and quotes the empty value in deploy-core-facets' warning. * revert(ci): leave validateScripts.yml untouched Any edit under .github/workflows/ needs an Information Security Manager approval, which is not a reasonable ask for a comment. Check 3 stays documented in validateScripts.ts, which is the file a maintainer reads. * fix(script): refuse an unreadable value where on is the dangerous direction `flagIsOn` resolves anything present to on, which is fail-safe for `--dry-run` and fail-dangerous for a flag that widens what the run touches: `--all-networks 0` fanned out to every active network, `--skip-confirmation 0` skipped the last prompt before a production deploy, `--execute-all 0` entered bulk mode, and `--is-production 0` picked the production diamond log. Those five now read through `readBooleanFlag`, which refuses a value it cannot read. Which reader to pick is now stated on `flagIsOn` itself. The guard also read the FIRST match for `type` and `default`, while object literals are last-write-wins, so `{ ...positionalBase, type: 'boolean', default: false }` was exempted as a positional — a false negative in the one thing the guard exists to catch, introduced by its own spread following. * test(script): prove each strict flag refuses at its own call site Reverting every call site switched to `readBooleanFlag` back to `flagIsOn` left all 2599 tests green: the reader's logic was already covered, so nothing observed the routing. The new test spawns each real CLI, asserts the refusal names that flag, and pairs it with two commands that fail fast on their own arguments to show a readable value is accepted and used. Under the same reversion all 10 cases now fail. Applies the rule to three flags it had missed: `registerOnly` gates the same production prompt as the `skipConfirmation` hardened beside it, and deploy-safe-tron's `allowOverride` and `setupOnly` are the Tron twins of flags already hardened — the first overwrites tron.safeAddress, the second calls setup() on the live Safe down a branch that never reaches the override guard. The test targets a network name no real network uses: a refused run writes nothing, but a run under mutation does, and it must not be able to edit a real diamond log. * fix(script): let the placement test judge what it can, and say what it cannot Six of the spawned commands transitively import generated `typechain/` types, which the CI job running this suite does not build, so they died at module load before reaching the reader and the test failed there while passing locally — it passed only because this worktree had typechain built. Those runs now assert the unbuilt-module error instead, and an afterAll requires at least the four artifact-free flags to have been genuinely judged, so the file cannot go green having judged nothing. With typechain hidden, reverting those four still fails six cases. `properties` also swapped its visited set for a recursion stack: a set is what CodeRabbit asked to change, and it dropped a second legitimate spread of the same literal, losing the last word on a property. * fix(script): never point the placement test at a live network deploy-safe.ts was spawned against mainnet. A run that gets past the reader deploys a Safe and rewrites that network's safeAddress — and getting past the reader is exactly what happens when someone mutation-tests these call sites, which is how a Safe came to be deployed on mainnet from this test. Unsetting the keys in the parent does not help: the spawned child re-reads the repo env file itself. It now uses the same name-with-no-RPC the other command already used, which stops in setupEnvironment before any deployment. Under the mutation that caused it, the test still fails on the reverted call site and writes nothing. * fix(script): treat --ledger-live=false as off, and sweep tasks/ add-safe-owners still truthiness-read ledgerLive after dropping its default, so the string "false" turned Ledger Live on. The citty default guard now also walks tasks/, where generateLedgerClearSigning still had the defect. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> * fix(tron): broadcast the overload the pre-flight priced, and reach --no-propose (EXSC-920) Peer-review findings from lifinance#2327. TronWeb registers `contract[name]` behind a `hasProperty` check but assigns `methodInstances[name]` unguarded, so for an overloaded name the two resolve to opposite ABI entries. The signature path priced one and broadcast the other, and refused the overload the broadcast does reach. Resolution now picks the entry from the typed argument types and the send goes through `contract.methods[selector]`, the key TronWeb registers per overload. The signature-path estimate was handed `args.address` unconverted; `triggerConstantContract` validates with `TronWeb.isAddress`, which is false for the `0x…` form `isValidAddress` accepts, so a hex-addressed send was refused as a failed estimate. mri reads a leading `--no-` as negating the rest of the name, so `--no-propose` never reached `args.noPropose` and the run created the Safe proposal the operator asked to skip. Read via `readOptOutFlag`, which resolves both spellings from argv. Also: the selector-form estimate is now retried like the calldata one, since it is mandatory before the send; and a TRX amount with trailing zeros (`1.5000000tron`) is no longer read as finer than one SUN. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(tron): name the flag spelling operators can actually type (EXSC-920) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(tron): classify a selector-estimate failure by the shape of the throw (EXSC-920) Review-gate findings on the previous commits. TronWeb raises the node's own message — a revert reason, a refused argument — as a plain Error out of `triggerConstantContract`, so the result-shape check never saw a revert and `retryTronEstimate` treated it as retryable: a deterministic refusal cost three attempts and four seconds of real sleep on every production call site. A throw is now retried only when it arrives as an axios failure with no response, a 429, or a 5xx. Proven against TronGrid: an unreachable host takes three attempts, a real revert one. The test fixture that claimed to cover this returned `{ result: { result: false } }`, a shape the transport never hands back; replaced with the errors the real path produces. `readOptOutFlag` also refuses the two spellings contradicting each other (`--propose --noPropose`), which reach different parser keys and so slipped past the per-flag duplicate check that already refuses `--propose --no-propose`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(deploy): drop the citty default on allowOverride after the main merge The 2327 pre-push check treats a defaulted multi-word citty flag as a silent no-op for the other spelling. whenAbsent: false already holds the floor. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…inance#2329) * feat(codehash): derive the legitimate toolchain set from config, never from the deployed code (EXSC-906) D19(a). `compareToAttestedSet` takes a required `scope` with no default, because the compiler version in a metadata trailer is written by whoever proposes the cut — one code byte flipped plus three trailer bytes claiming solc 0.8.99 moved a real facet's verdict from MISMATCH to UNVERIFIABLE. This supplies that scope from repo configuration only. `config/networks.json` carries `targetEvmVersion` for all 73 networks and takes four values; `foundry.toml` pins exactly three compiler pairs. Every active network resolves to a closed set, asserted over the real fleet rather than a fixture, so a config change that would quietly hand authority back to the trailer fails the suite. Two things it deliberately refuses rather than guesses. `"n/a"` is not a missing value — it is `abstract`, `lens` and `zksync`, three active mainnets, saying "not an EVM version" because they compile through zksolc; mapping it to a fallback would grade every deploy there UNVERIFIABLE, the grey fail-open lifinance#2301 was fixed to remove. And when `targetEvmVersion` and `isZkEVM` contradict each other it ERRORs, because neither reading is safe: closed would refuse an honest deploy, open would consult the trailer. Two defects caught in my own first draft, both now pinned by a test that fails without the fix. A hardcoded version-to-profile table mapped every unrecognised hardfork onto `default`, reporting a closed set against the wrong compiler pair — a false GREEN, unreachable today, which is exactly why it needed a test. Matching on `evm_version` instead then admitted the zk profile for every cancun network, since `zksync` also declares cancun; disqualification is keyed on the zksolc pin rather than the profile's name, so a rename cannot re-admit it. Profiles are parsed from `foundry.toml`, not hardcoded: a hardcoded pair keeps passing after someone retunes a profile, which is the drift the gate downstream exists to catch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(codehash): read the zksolc trailer through the existing reader (EXSC-906) D19(b) needs the third trailer format. zksolc writes the same length-word and CBOR envelope solc does — `a2 { ipfs, solc }` with a big-endian length word — so the strip already worked and only the `solc` value differs: solc writes a three-byte release, zksolc a `zksolc:1.5.15;solc:0.8.29;llvm:1.0.2` text triple. Extended the existing reader rather than adding a zk one. WP-1.4 shipped a bug of that exact shape and it took a review round to find: the deploy gate parsed the calldata a second time, so the bytes vouched for and the bytes signed came from two independent reads. One field, one reader. The triple is surfaced whole because D19(b) requires the solc-fork/LLVM sub-version compared UNMASKED — a reader exposing only `solc` would make that comparison silently incomplete while looking finished. It is all-or-nothing and the pattern is anchored, ordered and fully numeric: these bytes are part of the deployed code and therefore chosen by whoever deployed it, so a half-read triple shown to a signer as fact is worse than none. Dropping the anchors kills only one test — the `llvm:notaversion` case — which is what makes those negatives load-bearing rather than decorative. The fixture's layout is the one measured on a real `CalldataVerificationFacet` (10c §3.4); its IPFS digest is filler. It self-validates: the pieces sum to 85 bytes, which is what its own length word declares and what the doc measured independently. A fixture whose declared length disagreed with its contents would be testing nothing. `toolchain` being absent is how a caller tells an EVM lineage from a zk one without consulting the network, and that absence is asserted alongside its presence in the zk case, so it is not a vacuous assertion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(codehash): read zk immutables by supplied ordinal, never by assumed order (EXSC-906) Layer 2 for zkEVM. Immutables are not inlined into runtime code there, so the offset masking in immutable-offsets.ts has nothing to bite on — the values live in ImmutableSimulator (0x…8005) and are addressed by ordinal. The ordinal is an input and its absence is a refusal, which is the whole point. The spike measured index = declaration-ordinal x 32 on three real contracts, and its own caveat is that a constructor assigning immutables out of declaration order breaks that correspondence. Reading the wrong slot then checks one immutable's value against another's expectation — and that PASSES. So a fallback to source declaration order is deliberately absent rather than merely unused (adversarial F11), and the mapping comes from compiler metadata via the caller. Removing the refusal kills exactly one test; indexing by the bare ordinal kills two. Both are the mutations that matter, so those cases are load-bearing. A failed read is reported, never defaulted to zero: 10c measured two genuine zero immutables on SymbiosisFacet (the syBTC path is unsupported on zksync), so zero is a legitimate value here and cannot double as an error. Two names claiming one ordinal is refused for the same reason a wrong index is. The simulator address is pinned and asserted, because it is a system contract: a wrong address reads zeros from an empty account, which is indistinguishable from a contract whose immutables are all unset. Injectable reader, so the policy is tested without a chain — the values in the fixtures are the ones measured on LayerSwapFacet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(codehash): fetch a lineage commit by SHA before calling it unverifiable (EXSC-906) Lineage resolution's first step. The fleet sweep measured 56 of 98 audit commits unreachable from any local ref and retrievable by SHA, so "not in this checkout" is evidence of nothing. Concluding unverifiable without trying the fetch grades honest deploys grey — and a grey that is usually wrong is a grey signers learn to click through, which costs more than the check buys. The opposite failure is worse and is therefore a separate outcome: a commit that genuinely cannot be fetched ERRORs rather than falling through to "absent" (T3/D3). Collapsing the two kills two tests; dropping the retry to a single attempt kills a third. A fetch that exits zero without producing the object is also an ERROR, because it leaves us exactly where we started. Retries are bounded at three so one flaky network moment does not turn an honest deploy unverifiable, while a dead remote cannot spin. A prefix is refused before git is touched at all: `cat-file` would resolve it against whatever this checkout happens to hold, and handing a prefix to `fetch` is an unbounded request to the remote. Presence is asked as `<sha>^{commit}` so a tree or blob sharing the SHA cannot answer for a commit. Injectable git runner, so the policy is tested without a network. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(codehash): classify a cut per element, so an Add cannot ride in on a Remove (EXSC-906) The policy half of the sign-time gate. Adversarial A1: classification is per FacetCut element, never per operation. A batch pairing one Add with one Remove still gates the Add — grading the cut as a whole is exactly how an addition rides in on a deletion. Mutating it to exempt any cut containing a Remove kills three tests. `_init` is a target in its own right because it is delegatecalled in the diamond's own storage context, so its code runs against the diamond exactly as a facet's does. Two rules follow. On an additive cut it is gated alongside the facets, and dropping it from the gated set kills a test. On a removal-only cut it is REFUSED rather than gated: a subtractive cut carrying init calldata is arbitrary code framed as a deletion, and there is no reading under which it is merely a deletion. Allowing it kills two tests. Refusals are returned as a list and none of them is a codehash question — an unknown action, and an Add to the zero address, are both malformed rather than unverifiable. Every refusal is reported, not just the first, because a signer should not discover them one re-run at a time. When anything is refused the gated set is empty, so a caller cannot half-act on a cut this will not sign. It takes the already-decoded cut, not calldata. The gate has to judge the same structure the signer is shown; WP-1.4 shipped the opposite and it took a review round to find — its deploy gate re-parsed the caller's file, so the bytes vouched for and the bytes signed came from two independent reads. Fixture addresses are checksummed through viem rather than hand-written, so a wrong hand-computed checksum cannot turn these into assertions about the fixture. Note for WP-8.1's pruning sweep: `collectInstalledFacetAddresses` in lifinance#2324's funnel-deploy-gate recovers facet addresses from calldata for the deploy gate. Different input and different output, but the same neighbourhood — worth collapsing once both have merged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(codehash): compose the modules into a sign-time verdict (EXSC-906) Classify the cut, then judge every address it would install. Chain reads, attestation lookup and lineage scope are injected, so the policy is tested without an RPC and the wiring can supply the real ones. Three invariants, each proven load-bearing by mutation: MISMATCH and UNVERIFIABLE stay distinct. Both stop a signature, but they are different facts, and collapsing them is what teaches a signer to click through grey — the fleet sweep called that out as constraint 4. Collapsing them kills two tests. Nothing fails open. An address whose code or attestations cannot be read is UNVERIFIABLE and blocks, because "we could not check" and "we checked and it is fine" are the two things a gate exists to keep apart. An infrastructure failure is also kept separate from "no attested build": the second is a missing rebuild, the first could be hiding either answer. Letting grey pass kills three tests. Every target is judged even after one fails, so a signer sees both wrong addresses at once rather than one per re-run. Stopping early kills one test. A Remove is never looked up on chain at all. That is not an optimisation — a verdict on a removal target invites blocking a removal, which criterion (d) forbids, and the address being removed is usually exactly the one that cannot be vouched for. A classification refusal likewise reads nothing: a removal-only cut carrying _init is malformed rather than unverifiable. The excluded-immutable count is carried through and surfaced in the summary, because compareToAttestedSet's own docstring warns a MATCH says nothing about masked bytes. Two fixture corrections worth noting, both of the same kind. An attestation pinning `rawHash` is compared byte for byte and correctly reports zero excluded bytes, so testing the masking path needed an attestation that does not pin — the first fixture was unrealistic in exactly the dimension under test. And tsc caught a missing `solcVersion` that bun's runtime did not: it is declared required-but-nullable rather than optional so a caller cannot omit it by accident, which is that design working. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(codehash): rebuild at the record's commit as the attestation source (EXSC-906) The last missing input to the sign-time gate: attestationsFor now answers from a local rebuild at the deployment record's commit, under every toolchain config/networks.json plus foundry.toml say the network may legitimately have used. The record supplies the commit and the contract identity only — never the compiler pair, which 14 production slots record wrongly — so the gate downstream compares by set membership rather than equality against a record-derived profile. Both sides of the comparison are made to go through one normaliser, because two implementations of strip-then-mask is how the bytes vouched for and the bytes compared come apart. zkEVM takes no offset masking and pins the exact bytes, so the solc-fork/LLVM sub-version is compared unmasked per D19(b). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(codehash): assert the zk discriminator against every active network The branch that decides masking is the profile's zksolc pin, so it is asserted across the 71 active networks in config/networks.json rather than against the two the fixtures name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(codehash): read the rebuild's trailer once, and strip once Two reads of one field is the shape this module's siblings warn about, and the second read was only for the lineage label. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): close three fail-opens the gate review found (EXSC-906) All three sat in the one place the docstrings asserted most confidently and tested least: what happens when a second input differs from the one the code was written against. **A masked MATCH is no longer a MATCH.** `attested-set` states the rule as a rendering instruction — a MATCH says nothing about masked bytes, so a caller that has not run layer 2 "must not render an unqualified green". Rendering is not a gate, and two executable fail-opens rode on the difference. The masking refs come from compiling the record's commit; D3 has the verifier assert commit presence rather than ancestry, so a proposer may reference any fetchable commit; and `findFault` bounds each range without bounding the masked FRACTION. Demonstrated: refs declaring one immutable across the whole 96-byte body produced a MATCH with `blocksSigning: false` on bytes sharing nothing with the rebuild. An attested build carrying no refs against a deployed body masking 32 bytes did the same. No threshold is invented, because any excluded byte is an uncompared byte. It grades UNVERIFIABLE rather than MISMATCH: nothing was found wrong, it was not looked at. Layer 2 supplies the missing check and lifts it. Paired with a test that MATCH still holds when nothing was excluded, so the rule cannot degrade into "always grey" and pass its own test. **The rebuild cache omitted the network.** The record is read per (address, network) and it is the record that names the contract compiled, so the network determines the artifact. Without it, one address holding different contracts on two networks shared an entry and the second got the first's bytecode reported as a MATCH, naming the wrong contract in the verdict a signer reads. Latent, not live — measured across all 71 deployment logs and 71 diamond.json files, 651 facet addresses, zero hold more than one name. But that is a property of how we deploy, not one a cache may assume. Dropping the network again kills the new test; the paired positive proves the cache still reuses a compile for a second address of one contract. **A renamed zk profile leaked into every non-zk network.** My own comment claimed the pin rather than the profile name disqualifies a zksolc profile, "so renaming the profile cannot re-admit it". False: the pin sits outside every profile table and is ATTACHED by name, so a rename detaches it, the renamed profile reads as an ordinary cancun lineage, and zkEVM networks lose theirs. Nothing in foundry.toml identifies that profile other than its name, so the fix is not a different key — it is refusing to build such a map at all, turning a silent widening in one direction and a silent break in the other into one loud failure. Comment corrected to say what the filter does and does not survive. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(codehash): refuse a signature the sign-time codehash gate does not permit Wires verifyCutTargets into confirm-safe-tx.ts. The cut is recovered from the same in-memory calldata the display path decodes, through the same ABI_DIAMOND_CUT, with the scheduleBatch envelope unwrapped. The verdict renders with the transaction details in three visually distinct buckets; the refusal happens inside the one function every sign path funnels through, so it is reached before any signing client is touched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(codehash): keep the rebuild checkouts out of the repo and per process Also extracts the immutable-offsets resolver so its no-commit and no-record branches are covered, and builds the gate's failure state in one literal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(codehash): a hash match with masked bytes now grades UNVERIFIABLE Follows c8ab9eb through the sign-time wiring: the excluded-byte count still has to survive the wiring for the display to say what was not compared. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): close two decode fail-opens the adversarial review found A case-shifted proposal decoded to nothing while installing the same bytes: the walker folded the selector to lowercase for its own comparison, but viem's selector match is case-sensitive, so the diamondCut branch threw and the top-level refusal did not fire. The hex is now folded and validated once. The refusal was keyed on the outer selector, so a cut inside an envelope one level below a scheduleBatch was invisible, and a batch pairing one readable cut with one unreadable frame rendered green. It is now keyed on any frame the decoder could not open, at any depth, and a refused frame no longer discards the cuts that were readable. Also: the commit is shape-checked before it reaches a path or git argv, the record is read once per target, the offsets resolver mirrors the observer's single-lineage refusal, a blocking gate that never ran says so on screen instead of rendering nothing, and the placement and default-checkout-root properties are asserted where they were assumed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): gate the execute route, and stop claiming a pass for bytes nobody read (EXSC-906) Gate review of the wiring. Two majors, both fail-opens, plus the render rule this file states and its own renderer broke. **Execute reached the diamond without the gate.** `Execute` and `Execute with Deployer` call `executeTransaction`, which formats the signatures already on the proposal and hands them to the chain executor — no `signTransaction`, so `createGatedSigner` was never consulted. A proposal at threshold was therefore broadcast by this operator with a red verdict on screen and nothing refusing, and execution is the irreversible step. That is D23's shape exactly: WP-1.4 read D9's "the gate is never in two places" as forbidding a second gate anywhere and left the direct-broadcast route open, and the ruling was two route-disjoint gates. All four execute branches call one local helper, so the assert at its head covers them by construction; the sign-then-execute paths simply assert twice. Removing it fails the placement test. **A green pass was printed about frames the decoder could not open.** `unopened` was discarded unless the calldata happened to carry the cut selector, so an envelope that builds its cut on-chain produced no cut, no refusal, and `✓ MATCH This proposal performs no diamondCut` — an affirmative claim about bytes that were never read, followed by a signature. The narrowing itself is needed, or every ordinary `setFees` proposal would be refused, so the fix is to stop overstating rather than to start blocking: `unopened` is surfaced, the frames are named, and the bucket is `· NO CLAIM` in cyan. Non-blocking is deliberate and is the residual: this decoder cannot distinguish "an ordinary call I do not decode" from "an envelope hiding a payload", and blocking the first would refuse every non-cut proposal. What it no longer does is assert that it checked. **"No cut found" and "checked and clean" no longer look identical.** Both were a green tick, so a signer skimming glyphs could not tell them apart — and REFUSED shared both glyph and colour with MISMATCH, against this file's own stated rule that no two buckets may differ by only one of word, glyph and colour. REFUSED is now ⛔. Both regressions are mutation-covered, and the green tick is asserted to survive for a real MATCH so the rule cannot degrade into never rendering green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): judge the struct that gets signed, and unbreak the scope lookup (EXSC-906) The sign-time gate read the calldata off the stored Mongo document while the signature is produced over the normalised transaction three lines below it, and passed the raw `--network` value into a lookup keyed lowercase. The network half is a live defect: `deriveToolchainScope` throws on an unknown key, so `--network Mainnet` refused an honest signature instead of judging it. A false red, which is the failure that lands on a colleague. The struct half is not. `initializeSafeTransaction` copies `data` across verbatim and transforms only `to`, `value` and `nonce`, so the two are the same bytes today; reading the signed struct is what keeps a later normalisation from splitting the bytes vouched for from the bytes approved. The placement test asserts the positive, so the partial fix — correcting the network and keeping the document read — still fails it. Also: consume each report's own `blocksSigning` rather than re-deriving the rule from its targets (equivalent today, and therefore untested — one decision, one place); reattach the `defaultCheckoutRoot` docblock, which had drifted onto `memoisePerTarget`; and correct a comment that had the address casing backwards (`classifyCut` returns checksummed, so the case-insensitive record lookup is there for the stored side, not the decoded one). Documents the gate in MultisigSigningProcess.md — §4.3, a §5 row, and the residual it does *not* cover in §6 — which lifinance#2324 landing unblocked. * fix(codehash): stop promising the signer main, and make two greppable tests real (EXSC-906) Gate round on c631030. Five things, in descending order of how badly they would have misled a signer. **The green line claimed the wrong thing.** A passing cut rendered "matches an attested build of main". The rebuild is at the commit each deployment record names, and per D3 the verifier asserts that commit is *present*, not that it is an ancestor of main — so code deployed honestly from an unmerged branch reads MATCH. It now says it matched a rebuild at the record's commit and that main membership is unchecked. The docs said the same thing in two places and now carry the residual in §6, next to the Propose-time gate that really is main-anchored. **Both new placement assertions could be satisfied by a comment.** A reviewer reverted both fixes with a comment reciting the correct spelling and the suite stayed green, 10/10. They now assert on a comment-stripped slice of the call site, and a second assertion pins the call-site count at one, since the slice anchors on the first occurrence. **The network fix was in the wrong layer.** `processTxs` already destructures `networkKey`, and pinning the literal `network.toLowerCase()` made that better spelling fail its own test. Normalisation moved inside `evaluateCodehashSignGate`, where no future caller can repeat the bug and where a real test drives it. Lowercasing rather than rejecting: a rejection would refuse honest work, which is the failure being fixed. **Dependencies were built for proposals the gate makes no claim about.** Constructing them reads `foundry.toml` and creates a checkout root, and the caller turns a throw into a refusal — so a broken toolchain config refused a fee change. They are now built lazily, once a cut is present. Both directions are asserted, so a thunk that is never invoked fails too. **Restored the caller-side non-MATCH invariant** the previous commit removed as a duplicate. It was not a duplicate: `blocksSigning` from each report is this layer's input, while "nothing rendered non-MATCH is signable" is its own promise, and resting that on one expression is how a red verdict gets signed. 2770 pass / 0 fail. Also: the call-site and docstring prose still asserted the gate judges the display's variable, which was the guarantee c631030 removed. * fix(codehash): make the two call-site invariants types instead of greps (EXSC-906) Second gate round. The previous commit answered "a comment can satisfy this assertion" by stripping comments; a reviewer then defeated the result three more ways — a string literal reciting the right spelling, a `'/*'` in a string that made the stripper eat the real call site and anchor on a decoy, and an alias (`const signed = tx.safeTransaction.data`) that read the correct field and failed the test anyway. The last one is the tell: a pin on spelling cannot distinguish a correct refactor from a regression, so it was both evadable and a false red. Pinning harder was the wrong answer twice. Both invariants now hold structurally: - `gateInputFor` takes `ISignableProposal`, which names `safeTransaction` and nothing else. Handing over the stored `safeTx` copy does not compile, and which bytes get judged is now driven by real tests instead of a grep. - `deps` is thunk-only rather than thunk-or-value, so the eager `getCodehashDeps()` that refused cut-free proposals is a type error. `tsc` enumerated all 17 call sites. What is left in the placement file is placement: that this file does not build the gate's input itself, and that there is exactly one call site — counted on string- and comment-stripped text, since a second call site hidden inside a fake comment marker was the evasion that reached furthest. Both re-run and fail as intended. Also sweeps the rest of the `main` claim the last commit only half fixed. Five more signer-facing strings and five docstrings still said "a build of main", including the closed-set MISMATCH line — `so it is not a build of main` — which asserts a property nothing checks, in exactly the case D19(a) names as possibly honest. And `safe-decode-utils.ts` still documented the display-anchored contract that c631030 removed. 2773 pass / 0 fail. `tsc` clean for every file this branch touches; eslint 0 errors. * fix(codehash): brand the signed struct, because naming the field was not enough (EXSC-906) Fourth attempt at one invariant — the gate must judge the bytes the signature covers — and the first that a reviewer could not defeat. The three before it: a pin on the call site's spelling (defeated by a comment reciting it); the same pin on comment-stripped text (defeated by a string literal, and by a `'/*'` inside a string that made the stripper delete the real call site and anchor on a decoy); and an interface naming only `safeTransaction` (defeated with no cast at all, because `ISafeTxDocument.safeTx` is the *same type*, so `{ safeTransaction: row.safeTx }` type-checks and judges the stored document). Each version was also a false red: an alias reading the correct field failed the test it satisfied in spirit. `initializeSafeTransaction` — the one function that turns a stored row into the struct Safe hashes — now returns a branded `ISignedSafeTransaction`, and `gateInputFor` requires that brand. The document's copy is no longer assignable. Falsification is a `@ts-expect-error` case: weaken the parameter back to `ISafeTransaction` and `tsc` fails on the unused directive, so the guarantee cannot rot silently. Nothing else needed changing — an intersection is still an `ISafeTransaction` everywhere one is expected. With the invariant in the compiler, the placement file drops its two source-scanning assertions and the string-stripping helper behind them. A second call site is no longer a bug either: it would have to pass the same branded struct. What is left there is ordering, which types cannot express. Three smaller findings from the same round: - `resolveDeps` ran per cut, so a caller whose thunk was not itself memoised would get a checkout root and a connection per cut in a batch against one `close()`. Memoised inside the evaluation, and asserted — nothing had pinned it, and it only held because the caller memoises. - Five `toMatch` regexes had picked up a stray `/g` from a careless sed of mine. Bun's `toMatch` is `lastIndex`-stateful, so the obvious dedupe of a repeated pattern would have made the negatives pass against the same string that satisfies the positives. - `|| undefined` in the selector was justified by a failure mode that cannot occur — `''` is already falsy and short-circuits ahead of the decoder. Removed along with the comment that argued for it. Not fixed, deliberately: `deploy-self-check.ts:151` still says a mismatch is resolved by merging, and sign-time does not check main-membership. It scored below the action threshold, the right remedy wording is a judgement call, and the function has no non-test caller yet. 2773 pass / 0 fail; tsc clean for every file this branch touches; eslint 0 errors. * fix(codehash): check identity at runtime, because a type cannot express it (EXSC-906) Fifth attempt at one invariant: the gate must judge the bytes the signature covers. The fourth was defeated two ways, both cast-free and green. `{ ...tx, safeTransaction: { ...tx.safeTransaction, data: tx.safeTx.data } }` keeps the type-level brand and swaps the calldata, and reads as an innocuous normalisation. And `evaluateCodehashSignGate` still took a plain `{ data, network }`, so a second call site skipped the guarded selector altogether and judged the stored document — while the previous commit argued a second call site was safe *because* it would have to pass the branded struct. That argument was wrong, and arguing it instead of falsifying it is the thing D23 exists to forbid. The check I called self-enforcing was also enforced by nothing: CI type-checks changed production files with `tsc-files`, so a test file's `@ts-expect-error` is not in the program, and full `tsc` is red at baseline. The question was never "what type is this" but "is this the object Safe will hash", which no type can express. `initializeSafeTransaction` now records the struct it produces in a module-private `WeakSet`. There is deliberately no registrar: nothing can add to it but that one function, so there is no forging call for a reviewer to spot, and a test that needs a member goes through the real producer. `gateInputFor` refuses anything else, and `evaluateCodehashSignGate` accepts only `gateInputFor`'s return, branded with an unexported `unique symbol` so no literal satisfies it. Evidence, deliberately in the tools CI actually runs: - the spread and the hand-written brand both refuse under `bun test`, with a paired positive so the check is not blanket; deleting the identity check fails both. - a second call site bypassing the selector is a `tsc-files` error on a changed production file — reproduced against the reviewer's exact mutation. 2775 pass / 0 fail; tsc clean for every file this branch touches; eslint 0 errors. * fix(codehash): bind the verdict to the bytes being signed (EXSC-906) Attempt 5 checked the struct's identity and then discarded the struct, returning a detached `{ data, network }`. The WeakSet was guarding an object nobody kept, so two routes reached the gate with the document's calldata, both cast-free and green: reassigning `input.data` after the check, and `{ ...gateInputFor(tx, key), data: row.safeTx.data.data }` — the spread that beat attempt 4, one level further out. And identity alone was the wrong question anyway. `isSignedStruct` answers "some `initializeSafeTransaction` made this", not "this is the proposal being signed", and every pending row on a network carries a member struct — so grading proposal 0 and signing proposal N passed. Two changes. **The input holds the struct by reference** and the gate reads `struct.data.data` at the moment it judges. A spread now carries the same reference, so swapping the bytes means mutating the struct Safe will sign, which is the correct semantics rather than a bypass. **The verdict is bound to the payload.** A verdict is only ever a statement about specific bytes and nothing enforced that. The report carries `gradedData`; `createGatedSigner` takes a **required** `payloadOf`, so a signer that could omit it would be a signer with the old bug; and a passing verdict on different calldata refuses rather than authorising. The execute funnel passes its payload too. This closes the cross-proposal route and shrinks the mutate-between-grade-and-sign window that had no cover at all. Evidence under `bun test`, which CI runs: removing the binding fails two tests, one of them driven through the funnel against a spy so it asserts the signature was never *reached* rather than that something threw. Each is paired with a positive, so neither can degrade into refusing everything. Also removes a comment in the placement file still presenting the defeated argument as settled — "a second call site would have to pass the same branded struct" — and pointing at a `@ts-expect-error` case that no longer exists. Reasoning a review has already falsified, left in the tree as though current, is the D23 failure itself. 2780 pass / 0 fail; tsc clean for every file this branch touches; eslint 0 errors. * fix(codehash): bind on the signed tuple, and stop the refusal hiding its reason (EXSC-906) The gate round on ab192e4 found no bypass — first clean verdict on this invariant, across seven executed attacks including the two that beat the previous attempt and every production signing route. What it did find: **The refusal was misleading on its commonest path.** A gate that could not evaluate carries no verdict key, and production always passes one, so a broken `foundry.toml` told the operator the calldata had been substituted. Fail-closed, but it sends the next reader after a substitution that never happened. The mismatch still decides — a verdict about another transaction is not a pass — but it no longer chooses the message alone: both facts are reported when both hold. **The binding key was the calldata, not the transaction.** Two re-proposed rows can carry an identical cut at different nonces, which `data` alone cannot tell apart. `proposalKeyOf` covers `to|value|data|operation|nonce`, and `createGatedSigner` takes `keyOf` rather than `payloadOf`. The verdict is still only about the calldata, so binding wider costs nothing and closes the class instead of the instance. **One of my own new tests was vacuous in the dimension it was named for.** It asserted the gate's own reason survives a key mismatch, and passed with `gate.refusals` deleted from the message, because the text it matched also appears in `gate.summary`. Distinct texts now, and each half fails when either field is dropped. **Three more comments arguing for the design this replaced.** One of them argued against reading a mutable source, which is now the deliberate choice: the reference is what is worth holding, precisely because passing the bytes by value is what let them be substituted after the check. Same D23 failure the previous commit fixed in one file and left in three. Also corrects the placement file's claim that no call-site count is asserted there — one is, for signing calls, a few lines above. 2782 pass / 0 fail; tsc clean for every file this branch touches; eslint 0 errors. * fix(codehash): refuse a zk profile with no zksolc pin (EXSC-906) CodeRabbit round. Its review had run on a SHA eleven hours older than HEAD while its check read `pass`, so none of the gate's later work was covered; re-triggered and triaged. **One genuine fail-open.** A `[profile.zksync]` that exists but pins no `zksolc` returned a *closed* scope with `zksolcVersion === undefined`, and `codehash-sign-gate-deps.ts` reads that as "not zk" — so zkEVM bytecode would be normalised under EVM rules, a different trailer format and no immutable masking, leaving the comparison unsound in whichever direction it landed. The mirror case (a pin with no profile to attach it to) was already refused; this direction was not. Separate branch so the message names which of the two states it found, paired positive so the refusal is about the absent pin rather than zk networks, and removing the guard fails the test. **The Critical finding was mostly already closed**, and checking beat assuming: every abandonment path in `walk` is recorded, and the refusal is keyed on any unopened frame *plus* the `diamondCut` selector appearing anywhere in the raw calldata — a byte scan, because a cut nested under an unknown envelope is what an outer-selector check cannot see. Added the two truncated-payload cases it asked for. Its remaining ask — block on *any* undecodable frame — is the NO CLAIM decision, which refuses every non-cut proposal and is an open question for the repo owner rather than a silent choice inside a PR; that thread stays open. The unreadable `_init` case cannot be tested honestly. Probed with calldata whose `_init` word carries `0xff…` in its top twelve bytes: viem masks them and returns a clean address, so `getAddress` never rejects and the branch is unreachable through anything `decodeFunctionData` accepts. It stays as fail-closed defence with no test pretending to cover it. **Docs:** the three verdict buckets were documented and "the gate could not run at all" was not — the distinction the design rests on. §4.3 now states it as a fourth outcome that blocks and renders as a refusal rather than a verdict, and the §5 row no longer implies only a MISMATCH blocks. 2785 pass / 0 fail; tsc clean for every file this branch touches; eslint 0 errors. * fix(codehash): remove the payload-binding opt-out, and correct what a grey line means (EXSC-906) Two rounds landed at once — CodeRabbit's remaining threads and a gate round on acd7eaa, which found an error in the docs sentence that commit had just added. **The payload binding was opt-out by type.** `keyOf` returned `string | undefined` and the assert took `key?: string`, so `key !== undefined` was a door a future call site could walk through by simply not passing one. Both are required now, which also makes the stricter reading true: a non-blocking gate that graded *nothing* no longer authorises a signature. Six test sites updated; the sign-funnel cases carry a matching key so the mismatch check cannot confound the blocking behaviour they were written to assert. **`EXECUTE_CALLS` could not see a new ungated execute path.** It bound the receiver name — `/safeClient\.executeTransaction\(/` — while its sibling `CLIENT_SIGN_CALLS` deliberately matches any receiver. Both `safe` and `deployerSafe` are in scope in that file, so a broadcast through either produced no match and the assertion stayed green over exactly the second route it exists to catch. Widened; adding `safe.executeTransaction(` now fails it. **The infrastructure-failure prose was wrong for two of the three cases it listed.** An unreadable `foundry.toml` stops the gate reaching any verdict, so it is a refusal. An unreachable RPC or attestation store is caught per address and comes back as that address's `UNVERIFIABLE` — the same grey bucket, glyph and colour as an address with no attested build. Both block, so nothing was unsafe; a signer following the old sentence would have been looking for the wrong thing on screen. §4.3 and the §5 row now separate the two, and the grey bucket says it can mean "the check did not conclude" rather than only "a rebuild is missing". The `_init` masking question from the last commit is settled by measurement rather than left as reasoning: dirty high bits in an ABI `address` word are masked by viem and **revert on chain**, verified against a deployed harness, so the gate's view can never diverge from what executes — the worst case is a wasted revert. The same masking applies to facet addresses in the cut tuples, not only `_init`. 2785 pass / 0 fail; tsc clean for every file this branch touches; eslint 0 errors; markdownlint clean. * fix(codehash): pin worktree submodules and skip whole test/script trees (EXSC-906) Forge was auto-installing drifted lib SHAs after a bare worktree add, and `--skip test script` only dropped *.t.sol/*.s.sol — both made production rebuilds unreproducible or UNVERIFIABLE. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…ifinance#2333) * fix(ci): corroborate a stale cron verdict before alerting (EXSC-939) The watchdog read the last scheduled run from the event-filtered runs listing, which GitHub serves from an index that can lag the unfiltered listing by weeks. A lagging answer is indistinguishable from a dropped schedule, so the job paged on crons that were firing on time. A stale verdict is now re-checked against the unfiltered listing, filtering on the schedule event client-side, and the newer timestamp wins. Only the alert path pays for the second request; when neither source has a scheduled run the alert still fires. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(ci): report an exhausted corroboration budget instead of an absence (EXSC-939) A page of the busiest workflow's runs has covered as little as 11h, not the four days the previous page-budget rationale assumed, so a schedule buried under push traffic can outrun the scan. The scan now reports budget exhaustion separately from "no scheduled run", warns when it hits that limit, and gets five pages rather than three; the paging itself moved behind an injected page fetcher so its rules are covered by unit tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ne (EXSC-937) (lifinance#2334) * feat(safe): refuse a delegatecall proposal on the operation field alone (EXSC-937) A `DelegateCall` runs its target's code against the Safe's own storage whatever selector the calldata carries. The sign-time codehash gate decides what to vouch for by decoding calldata — `diamondCut` Add/Replace targets and the cut's `_init` — so a `DelegateCall` carrying no `diamondCut` is invisible to it by design, and is not therefore safe. Same class as the `_init` rule that gate already implements, one level up: a subtractive cut carrying init calldata is arbitrary code framed as a deletion, and this is arbitrary code framed as a call. **Refusing rather than verifying the target, because the measurement said so.** The ticket asked for that measurement before any design, and it changed the answer. Every proposal builder hardcodes `Call` — `propose-to-safe.ts:270` and all six `script/tasks/propose*.ts` batch builders — the Tron path sets no operation and defaults to `Call`, and a repo-wide grep for `operation: 1`, `operation: OperationTypeEnum.DelegateCall` and `operation = 1` outside tests returns nothing. `DelegateCall = 1` exists only as an enum member, one display line, and docstrings. Nothing legitimate produces this shape, so verifying its target would build an address-verification path for traffic that does not exist and hand a proposer a value to be judged against. Refusing costs one comparison. **Absent refuses too.** `operation` is typed required but filled from a Mongo row through a cast in `initializeSafeTransaction`, so it can be missing at runtime, and `confirm-safe-tx.ts` already does `?? 0` elsewhere. A default of `Call` would be a fail-open on precisely this check, so anything that is not exactly `0` refuses — an absent value, an undefined third operation, and a stringly-typed `'0'` each with their own reason, because "we cannot tell" and "this is a delegatecall" are different facts. Modules only, no wiring, deliberately: the refusal belongs at sign time inside `confirm-safe-tx.ts`, which lifinance#2329 rewrites heavily. Wiring lands once that merges rather than as a stacked PR. The interface already carries lifinance#2329's hardest-won lesson — the caller must pass `safeTransaction.data`, the struct the signature covers, never the stored `safeTx` row. 11 tests. Mutation evidence: replacing the `=== Call` check with `!== DelegateCall` (the `?? 0` shape) kills 4; loosening `===` to `==` kills exactly the coercion case. The paired positive is asserted first, because every other case is a refusal and a guard that refused everything would pass them all. * fix(safe): the absent-operation branch was dead, and three claims about it were wrong (EXSC-937) Gate round on cf1758c. It confirmed the refuse-outright design by redoing the measurement independently, and found that the property I advertised loudest does not hold. **"Absent refuses too" cannot fire on the source this module mandates.** `SafeClient.createTransaction` normalises `operation` with `tx.operation || OperationTypeEnum.Call`, so a row that never carried the field reaches `safeTransaction.data` as `0` and passes. The only copy that preserves absence is the stored `safeTx` row, which the module forbids reading — correctly, since the signature covers the normalised struct. So the branch was a floor dressed as a live catch. It is merged into the catch-all now and says so, and the real gap is named rather than implied: the row losing the field happens one frame up and is invisible by the time a signature is offered. That needs its own change to `createTransaction`, which is repo-wide and not this PR's business. Worth stating plainly because the reasoning generalises: this is the one place where the two copies of a field give *opposite* answers, so the unenforced "read the signed struct" contract stops being hygiene and starts deciding the verdict. **A refusal told the signer the opposite of the truth.** `1n` and `'1'` fail the identity test and landed in a branch whose text read "is 1, which is neither Call (0) nor DelegateCall (1)" — a sentence contradicted by the value printed inside it, about an actual delegatecall. Values now render with their type, and a test asserts that wording can never come back. **The refusal reason rendered uncoloured.** The reset closed straight after the badge, so the badge was red and the sentence a signer has to read was not — and the assertion for it was a `toContain` on the escape code, which passed on the badge alone. That was the assertion-observing-nothing in the file. It now pins the reset to the end and asserts nothing resets in the middle. **Renamed off the sign route.** `assertDelegateCallGateAllowsSigning` and "Nothing has been signed" would have steered the wiring into the sign funnel alone, leaving the two execute-only routes — which broadcast a threshold-reached row without consulting a signer — ungated. Now `assertProposalOperationPermitted`, "will not proceed", "signed or executed". Also drops a duplicate `SafeOperationEnum` in favour of the existing `OperationTypeEnum`, which is the declared type of the field being judged, and corrects two false statements: the Tron path sets `Call` explicitly (`propose-to-safe-tron.ts:303`, not "sets none and defaults"), and the EVM proposer's site is `propose-to-safe.ts:286`. 11 tests. Mutation evidence: closing the colour reset early fails the render test; restoring the "neither Call nor DelegateCall" wording fails two. * fix(safe): sanitise the operation value before it reaches a signer's terminal (EXSC-937) Gate round on 78d0f88. Its headline finding was that the gate has no call site, which is deliberate and stated in the PR body — but attacking the colour assertion turned up something that is not: **The refusal interpolated a proposer-controlled value straight into the operator's terminal.** `operation` reaches the struct through a cast, so a MongoDB row can carry a string, and the catch-all branch printed it raw. Verified: an `operation` of `\033[33mYELLOW` puts a live colour switch inside the rendered refusal, recolouring the sentence printed *about* it — and all three of my colour assertions passed while it did. Values go through `sanitizeProvenanceText` now, from `git-provenance.ts`, which imports only `node:child_process` and already strips this class. A paired assertion keeps the value described rather than silently dropped, since stripping it entirely would hide what was refused. **The colour assertion was wrong in both directions**, which is how it missed that. "Starts red, ends reset, no reset in between" is satisfied by a mid-line colour *switch*, and fails a legitimate reason that happens to contain an escape. It now counts escape sequences — exactly two, at the two ends — which is the property actually wanted. Also: `describe()` no longer throws on a value with no `toString` (`Object.create(null)`) or one that throws its own; a refusal has to render even then. An absent field reads "absent" rather than "undefined", and `[]` no longer renders as an empty gap. Two comments recounting the previous round's wrong wording are gone — the same rule the last commit was pulled up on, reintroduced by the fix for it. **Not fixed, and it is a genuine tension between two rounds.** The previous round said to drop the duplicate enum for `safe-utils`' `OperationTypeEnum`, which this did. This round points out `safe-utils.ts` runs `config()` at module scope and imports `mongodb`, so a pure decision module now pulls a dotenv read and a Mongo client at import time. Both findings are right and they conflict. Keeping the shared enum, because a security constant duplicated in two files is the worse failure; extracting it into a light module would touch `safe-utils.ts`, which lifinance#2329 owns and is awaiting review. Recorded for after that merge. 12 tests, 1198 in `script/deploy/safe/`. Mutation evidence: replacing the sanitiser with raw `String(value)` fails the injection test. * fix(safe): make the refused value's rendering lossless enough, bounded, and honest (EXSC-937) Gate round on f42a598 came back clean on the substance — no verdict bypass, and none of OSC hyperlink, clear-screen, bare C1 CSI, carriage return or backspace survives the sanitiser. Four smaller things, three of them in text an operator reads, which is this module's whole job. **Sanitising is lossy, so two malformed rows printed identically.** `01` and `01` both rendered `01 (string)`, and a signer reading the refusal could not tell which row produced it. The type suffix now carries the raw length, so they read `2 chars` and `3 chars`. The verdict was never affected — identity is tested on the raw value before any of this — but a refusal that cannot identify what it refused is worth less than it looks. **"unrenderable" described the wrong failure.** A value made entirely of stripped characters *did* render, to nothing; only a value `String()` cannot handle is unrenderable. Those are different facts and now read differently, with the paired case asserted so the two labels cannot collapse back into one. **A 500-character value printed in full.** Bounded at 80 with an ellipsis, and the type and true length survive the clip — otherwise bounding the value would also hide what it was. **And a comment recounting the previous round's wrong wording survived in the test file**, which the last commit message claimed was no longer true. It said two such comments were gone; one was. Removed, and the assertion it sat on now states the property rather than the history. 16 tests, 1202 in `script/deploy/safe/`. Mutation evidence: dropping the length suffix kills 4, collapsing the two failure labels kills 1, removing the clip kills 1. * fix(safe): clip on a code-point boundary, and report null as null (EXSC-937) Gate round on f4d0503 came back clean on substance — verdicts unaffected (differential fuzz over 30 inputs against the parent, zero divergence), all three mutation claims and both counts verified. Three things left. **The clip could emit a lone surrogate.** `slice(0, 80)` is index-based, so a pair straddling the boundary is cut in half: 79 `A`s then an emoji reproduced a high surrogate with no low half immediately before the ellipsis. That is the same garbled-terminal outcome the sanitising exists to prevent, arriving by a different route. Sliced by code point now, asserted in both directions so no unpaired surrogate can reach a signer. **`null` read as "no printable characters"**, which claims characters were present and stripped. Nothing was there. It reports `null`. **And the comment fix from last round was still a recount.** I removed the sentence naming the previous assertion and replaced it with one that still characterised what a "no reset in between" check cannot see — softened, same shape. Fourth round on this file for the same rule. It now states only the invariant the test checks. Left as disclosed rather than fixed: the length suffix counts UTF-16 code units, so an emoji reads `2 chars`. That is what `String.length` means and the label does not claim human-visible characters; changing it would trade a precise number for a vaguer one. 18 tests, 1204 in `script/deploy/safe/`. Mutation evidence: restoring the index-based slice kills the surrogate test, dropping the null branch kills the null test. * fix(safe): refuse a non-Call operation before any signature or broadcast (EXSC-937) The module was correct and unused. Put the assert on SafeClient so every signer and every execute path hits it, including after confirm-safe-tx is rewritten, and hide Sign/Execute once the signed struct is not Call. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(safe): sanitise the rendered operation and gate the public hash route (EXSC-937) The confirm detail block interpolated the row's `operation` raw into a coloured line, so a proposer-controlled string reached the signer's terminal as a live escape sequence. It now goes through the gate module's own bounded, control-character-free renderer, exported for the purpose. `signTransactionWithHash` is public and reachable without `signTransaction`, so the gate is asserted there too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(safe): correct the gate's docs and tighten its tests (EXSC-937) The module header still described the check as sited at confirm time; it sits on the signing and execution funnel, which the proposal builders reach too. `IDelegateCallVerdict` is readonly. `signTransactionWithHash` documents the precondition it now throws on. The placement test barred one spelling of the raw render and passed on the likelier reintroduction, a bare interpolation; it now counts every read of the field and requires each to be a literal comparison or the sanitiser call. The control-character assertion covers C1 and the line separators, so narrowing the sanitiser to C0 no longer leaves single-character CSI and OSC surviving with the suite green. The hash route gains the shape a stored row can actually carry, a string, alongside the enum value. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): close the bare-hash bypass of the operation gate (EXSC-937) `signHash` takes only a hash, so there is no operation field in scope for the gate to read. Public, it hands a caller a valid signature over a delegatecall with the gate never consulted; its only caller is `signTransactionWithHash`, so private costs nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(safe): sanitise every stored field in the signer's transaction detail block (EXSC-948) (lifinance#2345) * feat(safe): sanitise every proposer-controlled field in the signer prompt (EXSC-948) The Safe transaction detail block interpolated fields straight off the MongoDB proposal row into its own colour codes. A row could therefore erase the screen and repaint a fabricated "Safe Transaction Details" block — benign target, Operation: Call — immediately above the prompt asking whether to sign. The block moves into `buildSafeTxDetailLines`, which renders every stored field through `sanitizeProvenanceText` and appends a notice carrying the stored and shown lengths whenever the two differ, so a hostile row is disclosed rather than quietly cleaned. Output for a normal proposal is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(safe): close three routes a stored row still had into the signer's terminal (EXSC-948) An adversarial round found the sanitised detail block was not the only way a proposal row reaches the prompt. The target address never went through the sanitiser at all: it was declared a trusted pre-rendered string, but `normalizeAddressForNetwork` skips whitespace rather than refusing it and the display helper is a pass-through off Tron, so a leading CR in the stored address survived and rewound the `To:` line over its own label. The proposer address and the Ledger filmstrip took the same raw value. All three now read a sanitised copy. The nonce warnings interpolated the stored string on the grounds that `BigInt()` had already parsed it. `BigInt` skips whitespace too — `BigInt('31\r')` is 31n — so the parse proved nothing about what the string could still do to a line. They now print the parsed value. A missing field rendered blank instead of `undefined`, which both broke the block's byte-identity and left a row with no `data` — still cast to `Hex` and signed — looking exactly like one carrying `0x`. The placement rule was keyed on `tx.` inside a colour code, so it never covered the parked-cleanup fields reached through `ref.`, missed the one nonce warning printed outside a colour code, and five mutations survived it. It now rejects any interpolation of a stored value anywhere in the file against a two-entry allow-list, counts braces so a nested template cannot hide one, and carries fixtures for each shape it must flag. Zero-width characters the sanitiser passes through by design — U+200D and the Hangul fillers — now raise a notice of their own, since stored and shown lengths agree and the value is still not what it looks like. `Default_Ignorable_Code_Point` names that class, so this is not a blocklist to extend each time one is found. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(safe): describe the split the module actually implements (EXSC-948) The header described the caller handing the target address over raw for the block to sanitise. The caller sanitises it and still passes a pre-rendered fragment, because that fragment carries colour codes of its own. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(safe): compose the addresses inside the block so their sanitising is testable (EXSC-948) A second adversarial round defeated the source scanner twice over: a `const d = tx.safeTx.data` alias reaches the row without the text `tx.` ever appearing in the interpolation, and the exclusion guarding the raw reads was an `includes` over the whole line, so a trailing comment naming the sanitiser satisfied it. Both mutations reintroduced the original defect with the suite green. A scanner cannot decide where a value came from, so it is no longer asked to: the target and proposer are passed in as stored and composed inside `buildSafeTxDetailLines`, which the sibling suite reaches by execution. What remains of the scan is a tripwire on the shape of the call, and says so. Pre-sanitising in the caller had also silenced the warning it was meant to raise. The block decides whether to warn by comparing what it was given against what it can print, so a value cleaned on the way in is a value it reports as clean: two proposers differing only by a zero-width space rendered identically with nothing to distinguish them. The two notices were mutually exclusive, so anything stripped hid a surviving invisible character behind a length pair that a whitespace collapse leaves equal. They now combine. A value that is not a string and stringifies to nothing is named rather than rendered as an ordinary blank field. The nonce needs none of this: `txNonce` is a `bigint`, so the compiler already guarantees what the scan was asserting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(safe): close the callback route, and stop naming a repaired address (EXSC-948) A third adversarial round left the composed lines and the disclosure intact but found the block trusting things it had no reason to. The two display callbacks are handed a sanitised address, but what reaches the line is their return value, interpolated raw. Every test passed an identity function, so nothing observed whether a callback honoured its argument at all — and a mutation returning the stored row instead survived the suite. Both return values are now sanitised, which costs nothing on a real address. Sanitising a corrupt address can produce a valid one: a zero-width space inside the hex simply disappears. The repaired address was then looked up in the deployment records and rendered with its name and a working explorer link, so a corrupt row was presented as a known contract — a stronger claim than the row supports, and more convincing than what it rendered before the package. Neither is resolved now for an address that had to be repaired. The Ledger filmstrip previews the screens the device will show, so it takes the target from the signed struct rather than a display copy of the stored row. Three smaller corrections. The counts said "shown N" while a network formatter can replace what it is given with an entirely different rendering — on Tron nothing on the line was N characters long; they now say "printable N" and the comment says what they describe. "empty array" was a claim about the container that `[' ']` and `[null]` both falsify; the remark now states only that the stored value was not a string, and so also covers the values that stringify to something and were previously undisclosed. A `value !== undefined` clause was unreachable, since `String(undefined)` is never empty. The header and the interface docstring described the caller-side split that the previous commit removed. The placement file's title claimed a property its `toContain` assertions cannot establish; it now says it is a smoke check, what defeats it, and where the real coverage lives. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(safe): never show a target's name or link beside an address that is not it (EXSC-948) A fourth adversarial round found the block could print `To: (LiFiDiamond) https://etherscan.io/…` with no address on the line and no warning. A renderer that threw returned an empty fragment, which was neither reported nor treated as a repair, so the decorations that were meant to describe the address survived it. Under the parent that throw crashed the run instead — the previous commit had converted fail-loud into fail-silent on the two address lines of a signing prompt. A fragment that cannot render now says so and takes the name and link with it. The same round showed the previous commit's central invariant was only half observed. Two mutations reintroducing a raw stored value survived a green suite: the proposer formatter's return value, and the target name. Both had a sanitising call and neither had a test. So did a third, replacing the repair test with a change-detector — the only case exercised used a character the sanitiser strips, never one it keeps, which is the branch the module exists for. Four of the tests written in the previous round were not in the file it committed. The count was checked and the names were not. Withholding the name and link is now decided on whether the printable text still identifies what was stored, rather than on whether anything changed. Trimming the ends cannot change which address this is, so a whitespace-padded row keeps its name — losing it was a false red on a row that hashes and signs identically, and the name is the strongest confirmation a signer gets that the target is the contract they expect. Anything that was never a string resolves to nothing at all, absent included. `nonceColor` is typed as a closed set. It is the one field interpolated inside an escape sequence rather than beside one, and the docstring listing the unsanitised fields had omitted it while claiming to be exhaustive. Also: the invisible-character remark counted in the same unit the other one had just been corrected for; the filmstrip takes `value` from the signed struct, as `to` already did; and the placement file's header described a failure mode opposite to the one its `not.toContain` pair actually has. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(safe): guard the fragment coercion, and stop blaming the network for an empty field (EXSC-948) A fifth adversarial round found the previous commit's own new invariant half observed, and one regression it had introduced. The unrenderable branch was asserted on the target line and not on the proposer line, so three mutations restoring the silent blank — including the parent's behaviour verbatim — passed a green suite. That is the same gap the previous commit closed one call site over, one commit later, so both call sites now share `renderAddress` and both are asserted. Moving the sanitiser out of the callback's try/catch narrowed the guard: the coercion inside it throws for a returned object with a throwing `toString`, and `processTxs` has no per-network catch, so that would have cost the operator every remaining network in the run. Both are back under one guard. An empty stored value renders empty because it is empty, and saying the network could not render it named the wrong cause. It was also the only non-hostile shape that no longer matched the original display. Nothing asserted that the formatter and the explorer are handed the sanitised text rather than the stored row — a mutation feeding them the row put the unsanitised string on the line as the address, under a notice still claiming it had been sanitised. The comment on the identity conjuncts claimed they rule out every way the glyphs a reader sees can fail to determine the stored value. They are a byte-level property; confusables and combining marks are neither covered nor now claimed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(safe): check the parked refs are an array before iterating them (EXSC-948) `parkedTaskRefs` is read straight off the stored row and was guarded by a truthiness and length check. A stored document carrying a `length` property satisfies both and then throws on `for...of`, which escapes the builder; the caller has no per-network catch, so it would cost the operator every network left in the run. The function already refuses to trust the element shapes one line further down — the array-ness was the last thing taken on trust, and it falsified both the comment above it and the suite's own "no row shape costs the operator the run". The test added last round observed half of what its name claimed: the formatter and the explorer shared one marker, so the formatter's output satisfied both assertions and nothing constrained what the explorer received. Distinct markers now, and feeding the explorer the rendered fragment instead of the sanitised text is caught — that one is a real defect on Tron, where the fragment carries the hex suffix and the URL would have carried it too. Sharing `renderAddress` between the two address lines did not make both observed: a mutation feeding it the stored row on the proposer line passed. That is the same asymmetry as last round, moved into the helper that was meant to remove it, so both call sites are asserted now. Also covered: the callback-coercion guard restored last round, which nothing would have noticed the removal of; the explorer link suppressed for a target that sanitises to nothing; and the unformatted-address warning's placement outside the value's colour. That warning named the network as the cause when the sanitiser can equally be the one that emptied the fragment, and `renderAddress`'s docstring described the line as blank when it shows the unformatted text. Both reworded. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(safe): observe the parts of the block the suite was stepping over (EXSC-948) A seventh adversarial round found no defect that would mislead a signer or break the run, and named the gaps left in the suite. No behaviour changes here. The provenance wiring was entirely unobserved — the tests never passed a provenance object, so replacing the argument with `undefined` passed. That regression would make every proposal read "not recorded (proposal predates provenance capture)", hiding the commit, branch and dirty-tree disclosure behind a sentence that means the opposite. Only the first parked ref was observed, so truncating the list passed while hiding the deprecation context for every facet after the first. The new malformed-refs test ran its absence assertion against one of its four shapes: a guard accepting an iterable with a length renders `undefined → undefined` rows under a real header for a stored string, and passed. The unformatted-address warning's placement was asserted on the target line and not on the proposer line — the same two-call-site asymmetry twice corrected already, reappearing in the test written to correct it. The try around `formatProvenanceLines` is unobservable and now says so: that function catches its own failures, so nothing reaches this catch and a mutation deleting it survives. It stays because the totality it depends on lives in another module. The test that appeared to cover it is renamed to the property it actually observes — a half-migrated row costs one line, not the prompt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(safe): the provenance guard is load-bearing, and the comment saying otherwise was wrong (EXSC-948) The previous commit documented the try around `formatProvenanceLines` as unobservable belt-and-braces because that function "catches its own failures". It does not, quite: its handler builds its message by stringifying whatever was thrown, so a thrown value that cannot be stringified — a null-prototype object, one whose `toString` throws — makes the handler throw a second time and escape. Measured against the real module: contained for a plain Error, re-thrown for both of those. This catch is the last thing before `processTxs`, which has no per-network catch, so it is what keeps a half-migrated row from costing every network left in the run. The test alongside it modelled a half-migrated row as a throwing plain Error, which is the one shape the other layer handles, so deleting this catch passed. It now covers both, and asserts which layer answered: `formatProvenanceLines` says "block could not be rendered", this module's catch says "could not be rendered". The provenance wiring test asserted one third of the regression it was written for. Its own commit message named the commit, branch and dirty-tree disclosure as what a dropped argument would hide, and it checked only the handle — so a mutation keeping the first line and discarding the rest passed. It checks each of them now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(safe): say when a target name is withheld, and pin the escapes the tests could not see (EXSC-948) From an independent review of this PR by the session that owns lifinance#2340-lifinance#2344. **Withholding the target name inverted its meaning.** When the printable text no longer identifies the stored address, the name and explorer link are dropped — but nothing said so, and a bare address reads to a signer as "not a contract this repo deployed". That is the opposite of what the code concluded: the deployment records did match, and it declined to vouch for the name. A real LiFiDiamond address with a trailing U+200D renders with identical glyphs, is signable, and lost its `(LiFiDiamond)` silently. The omission is now stated, and only when there was a name to withhold. **No test could observe a row-supplied SGR sequence.** The helper strips every `ESC[<n>m` before the control-character assertion, which cannot tell one the row supplied from one this module wrote, and the shared hostile payload deliberately contains none. A sanitiser changed to pass SGR through — the plausible "let the dashboard keep its colours" edit — left both suites green while a row could recolour or `ESC[8m`-conceal its own field. Pinned now with exact renderings for an SGR payload on two fields. **Two fixtures carried a raw `0x1b` byte** against 26 escaped ones, so they read in a diff as strings with no control character at all — unverifiable without `od -c`, and silently gutted by any editor that normalises the file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(safe): the escape claim holds for end-positioned whitespace only (EXSC-948) The header said `\r`, `\n` and U+2028 survive `BigInt()` and `normalizeAddressForNetwork` and reach a line they can rewind. True at either end of those fields, false in the middle: `getAddress` and `BigInt` throw on an interior escape in `initializeSafeTransaction`, before anything is displayed, so for the nonce, value and target that shape is a failed run rather than a spoofed prompt. The fields with no coercion anywhere are named instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(safe): the last catch before the network loop must not be able to throw (EXSC-948) Found by CodeRabbit, outside the diff range. Describing a thrown value is itself a coercion: reading `.message` runs a getter and the sanitiser coerces whatever it is handed. So a provenance row that throws an object whose `toString` throws something unstringifiable — a null-prototype object — got past `formatProvenanceLines`' handler, then past this module's, and out of the builder. `processTxs` has no per-network catch, so that costs the operator every network left in the run. Two commits ago I probed this path with three shapes and concluded it was contained. It was, one level of nesting shallower than the one that escapes. The fallback is now a constant rather than anything derived from the value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(safe): close the remaining proposer-controlled routes into the signer's terminal (EXSC-951, EXSC-949) (lifinance#2349) * feat(safe): close the remaining proposer-controlled routes into the signer's terminal (EXSC-951, EXSC-949) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(safe): drive the 4byte-supplied signature through the route it names Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): decide the periphery check on the stored name, not the shown one (EXSC-951) Sanitising the periphery name before the deployments lookup made the check validate the value being displayed rather than the value being signed. A stored `GasZip<ZWSP>Periphery` sanitises to `GasZipPeriphery`, which is a real mainnet deployment, so the row printed a green "matches deployments" for a name the calldata does not contain. Before this PR the same row correctly said there was no entry. Reproduced by reverting the fix: the spoofed call prints the tick. The lookup now keys on the stored argument while the two messages still show the printable one, so nothing raw reaches the terminal and nothing sanitised reaches a decision. Also corrects three claims that were not true of the code: - The counts of invisible and non-ASCII characters are taken before the clip, not after, so the comment saying they describe "the printable text" was wrong. They cover what the signature covers; the clip remark says what is off screen. Now tested — no case previously exercised a value both clipped and carrying an invisible past the boundary. - The process doc said every shown field is bounded in length. The calldata is deliberately unbounded, which is the one thing a signer must be able to read in full. - The rule this PR added said to put the notice outside the colour code. Every `printableField` call site puts it inside, with no display defect, because the notice carries its own colour and reset. The rule now states that property instead of a placement that was never the point. And records why the frame styling is safe to leave as it is: `t` reaches `String.replace`'s replacement operand, where `$&` and friends substitute, which is harmless only while no styled row carries text off the proposal. * fix(safe): sanitise the two raw routes this PR had claimed were closed (EXSC-951) Both were found by a falsification pass, and both were open on the base rather than introduced here — the defect was the claim, not the code. `decodeDiamondCut` printed the 4byte-supplied function name raw, on the commonest proposal shape, one screen above the sign prompt. The proposer picks the selector and therefore picks which 4byte answer is fetched, and the only requirement on that answer is that it hashes back to the selector — which a signature carrying escape sequences satisfies as readily as one that does not. The identical route was closed in the whitelist formatter and this twin left open, with no diamondCut case in the new tests at all. `formatTimelockScheduleBatch` printed the payload's leading ten characters as `selector=` raw, one line above lines it did sanitise. That is only reachable through the stored-row caller, since `bytes` recovered from calldata is always valid hex, so the test drives the function directly. Each guard has an executed assertion that fails when the guard alone is reverted, and each test pairs the absence of the escape with the presence of the value, so a line that was never printed cannot pass for a line that was sanitised. Hostile fixtures are built with `String.fromCharCode(27)` rather than a literal byte, so they survive a normalising editor and can be read. Still open and tracked on EXSC-951: the `target=` line and its explorer URL, the unbounded decoded display, and five guards that work but carry no assertion. * fix(safe): report a scheduleBatch target that is not an address, never repair it (EXSC-951) The last raw route in the batch display. `target=` was formatted from the stored string and `getTargetSuffix` built an explorer link from the same value, so a row could put text into both. Sanitising it would have been the wrong fix, and the wrong fix in a way this PR has already demonstrated once: cleaning an identifier produces a *different* identifier that still looks valid, which is how the periphery check came to print a green tick for a name the calldata never held. So a target is decided rather than repaired. If it parses as an address for the network it is formatted and keeps its name and explorer link, which is safe because a valid address cannot carry an escape. If it does not parse, it is shown as stored with a notice and loses both the name and the link — there is nothing left to vouch for, and a link built from a non-address is worse than no link. The notice moves into `printable-field.ts` as `fieldNotice` so every disclosure carries the same colour and its own reset, which is what keeps it legible wherever a caller interpolates it, and keeps the escape sequences in one module. Falsified: dropping the validation fails the hostile-target test while the paired witness — a real address still earning its name and link — keeps passing, so the refusal is about the value rather than the feature being off. * fix(safe): bound the decoded display by the shape of what it is showing (EXSC-951) The decoded display bounded nothing, so a 500,000-character `string` argument rendered as two half-million-character lines with no notice at all — nothing was sanitised and nothing clipped, so there was nothing to report — and the flood landed below the name it belonged to, scrolling the target address and the matches-deployments verdict off the screen. No escape sequence required. A blanket bound would have been wrong in the other direction. A hex payload is what the signature covers, so its length is its own disclosure and clipping it hides the thing being approved — the same reason the calldata field stays unbounded. So the bound is chosen by shape: hex payloads whole, everything else at `MAX_FIELD_CHARS`, since a name or label past that is a terminal flood rather than information. The notice now uses the same bound as the text it describes, instead of being measured against a different limit. Both directions are falsified separately: unbounding the name fails the flood test, and making the bound blanket fails the witness that a long hex argument survives whole. The deployments lookup still keys on the whole stored value, so clipping the display cannot move the verdict. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * Revert "feat(safe): close the remaining proposer-controlled routes into the signer's terminal (EXSC-951, EXSC-949) (lifinance#2349)" This reverts commit e84ee5f. --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Goran Vladika <goran.vladika@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ce#2352) Composer routes on Plasma (9745) reverted ContractCallNotAllowed() because the Composer VM proxy 0xA9076E5FD4b9D553af8956Fd5F580De7F6f055CE was missing from the diamond allowlist. Adds the plasma entry to composerWhitelist.json and regenerates the periphery section of whitelist.json.
…rt (EXSC-698) (lifinance#2335) * feat(safe): record every check with its anchor, and keep cannot-run out of green (EXSC-698) Adds the check ledger and its console renderer as pure modules: each result carries expected value, actual value and the anchor it came from, statuses stay four-way, and the report prints N/N against a denominator fixed up front. An unverified check is not a passed check: error and a missing result both block with no acknowledgement path, alongside an integrity mismatch. Triage relaxations are narrowed to a purely subtractive op and never touch an integrity check. No call site is wired to it yet; migrating the existing checks is a later package. * refactor(safe): record what a triaged review relaxed, and digest the detail too (EXSC-698) Self-review findings on the check ledger: the review attestation cleared identically whether an acknowledgement was answered or dropped by --triage, so it now records the triage profile, the count still awaiting acknowledgement and the count relaxed. The digest covers each result's detail, which carries the coercion reason. The renderer matched relaxed rows by object reference across two rollup calls; it now matches on an exported (check, network) key, so a future copy in the rollup cannot silently turn a relaxed row back into a pending one. * fix(safe): render the triage-relaxed note from one label, not two (EXSC-698) The rows and the verdict line each carried their own copy of the relaxed wording, and the copies had drifted: `RELAXED_LABEL` said "relaxed by triage" while the verdict line hardcoded "relaxed by --triage". Two tests were failing on the difference. The verdict line now interpolates the same constant, so the two cannot disagree again — a second literal was the defect, not the text in it. Changing `RELAXED_LABEL` fails both the row assertion and the verdict assertion, which is the evidence they share one source. Also imports `CheckClass` into the renderer, which referenced it without importing it, so the file did not compile. 80 pass in the two ledger suites, 1266 in `script/deploy/safe/`; tsc-files and eslint clean on all four files. * fix(safe): a milder result can no longer erase a mismatch, and nothing renders green (EXSC-698) Gate round on 297ed9e. Two criticals, both reachable through the public API, plus four rules nothing observed. **A recorded mismatch was erased by a later `needs-ack`, and triage then relaxed it.** The supersession guard read `status === 'pass'`, so it blocked only the case its own docstring described. Recording `needs-ack` after a `fail` on the same (check, network) dropped the disagreement from every count, printed `no action`, and made it eligible for a triage relaxation that `isTriageRelaxationAllowed` forbids for a fail — T2 bypassed not by defeating the guard but by changing the status it sees. Semantic only; integrity coerces needs-ack to fail and re-arms. Now any non-`fail` is refused for a network already mismatched. **And the attestation could not tell the two runs apart.** The digest was built from the surviving rollup rows, so "one needs-ack" and "a mismatch, then a needs-ack" hashed identically — a superseded mismatch left no trace in the record either. It digests `ledger.results` now, every stored result, which is what its docstring already claimed. `title` is digested too: it is the only text saying what is being checked, so relabelling a check has to move the hash. **A ledger that verified nothing rendered ALL CHECKS GREEN.** `passed === expected` is `0 === 0`. `createCheckLedger` refuses to build one and its comment calls that the thing the factory exists to prevent — but every consumer takes a plain `ICheckLedger`, and `rollUpChecks`' own docstring anticipates rehydrated documents. `summariseLedger` now refuses, and a check covering no networks is no longer green. **An unrecognised status rendered as an acknowledgeable MISMATCH** while the verdict graded it unverified with no acknowledgement path, and appeared in no numerator. It renders unverified now, the way the verdict grades it. **A semantic mismatch appeared in no term on the section line** — the count was read off `verdict.blocking`, which carries a `fail` only for integrity. Read off the rollups now. **The anchor was the one untrusted interpolation reaching the terminal raw** on the check line, cleaned only on the expanded row. Worth naming why it went unseen: the test named for it never set an anchor, because `recordCheck` validates them — and the first replacement test I wrote had the same hole, so the mutation still survived. It injects through the rehydration path now. Network, section and detail were unobserved too and now have a case each. 1272 pass in `script/deploy/safe/`. Every fix falsified: reverting each of the six kills at least one test. * fix(safe): pin the digest fields that were unobserved, and show coverage on every verdict (EXSC-698) The rest of the gate round on 297ed9e. **Two digest tests passed for the wrong reason.** The reclassification test used a `fail`, so its digest moved through `hardBlocked` flipping true→false rather than through `checkClass` being digested at all — dropping the field entirely left it green. Same shape for the triage test, which moved through the acknowledgement and relaxed counts. Both now have a case where those inputs are held identical and the field under test is the only difference: an all-pass ledger reclassified integrity↔semantic, and `unknown` against `additive` on a needs-ack. `title` gets one too, since relabelling a check is how you make a record describe something else. Dropping any of the three now kills a test; before, two of them survived. **The coverage figure appeared only on the green verdict.** A closing line that shows the denominator when everything passed, and hides it otherwise, inverts the point of the ledger — the 56/57 run is the one whose denominator has to be visible. `BLOCKED`, `ACKNOWLEDGEMENT REQUIRED` and `NO BLOCKING RESULT` all carry it now, asserted on each. Also drops `(T3)` from the blocked line: a signer has no way to resolve an internal ruling id, and the sentence already says there is no acknowledgement path. 1278 pass in `script/deploy/safe/`, eslint clean. Removing the coverage suffix kills 3 tests. * fix(safe): an error may supersede a mismatch — refusing it downgraded the block (EXSC-698) Re-gate on 1b21228 and 2d18c68. It confirmed the retry path, the digest's order-independence, the empty-ledger guard's blast radius and the section sums — and found that my own supersession fix made one pair strictly worse. **A failed retry of a mismatch dropped the verdict from blocked to acknowledgeable.** On a semantic check, `fail` then `error`: the error was refused by the guard, so nothing counted as unverified, `hardBlocked` came back false and the run read ACKNOWLEDGEMENT REQUIRED. The same error with no prior mismatch hard-blocks. So a signer could acknowledge and sign *we saw a disagreement, and we could not re-verify it* — worse than either input, and the failed retry appeared nowhere in the report. `error` is not a milder result: it blocks with no acknowledgement path, so it may supersede. Only `pass` and `needs-ack` are refused now, and the retry path still clears an unverified result, asserted both ways. **The headline fix of the last commit was unobserved.** Reverting the digest to the rollup rows left 1278 passing. The example in that commit message was also wrong: the needs-ack collision it named is no longer producible, because the supersession fix changed which result survives. What still collides under the old source is `pass` against `error` then `pass` — same survivor, same verdict, different history — and that is what the new test pins. **An unrecognised status was still in no numerator.** The last commit fixed the row label only; `errored` counted the literal `'error'`, so the check and section lines showed `pass 0/1` with no term saying why. Counted as unverified now, which is how the verdict already graded it. **And the section line called a semantic mismatch "blocking"** while the verdict below it said acknowledgeable — the contradiction the renderer's own comment forbids. The word is gone; the count stays. Also: the supersession docstring still described the old `pass`-only rule, a digest test's comment and binding named a profile it did not pass, and several comments recounted what a previous version did wrong. **Acceptance rows corrected again.** A4.2 quoted a verdict string this branch no longer prints — `(T3)` is removed and there is a test forbidding it — and still rested on the 74-network run A4.1 retracted one row above. Both rows cited mutation ids with no committed harness, so they were not re-runnable; the named assertions are the citable evidence instead. 1282 pass in `script/deploy/safe/`. Reverting each of the three code fixes kills a test. * fix(safe): keep the superseded mismatch visible, and stop quoting a test count (EXSC-698) Re-gate on 583a223. The guard is clean under exhaustive enumeration — all 128 ordered pairs and triples of the four statuses across both classes, no false-green and no false-block — and each of the three code fixes dies when reverted. Two residuals, and one of them is the structural problem the last three rounds kept trading around. **When an `error` superseded a `fail`, the disagreement vanished from the report.** The verdict stayed blocked, so nothing unsafe could be signed, but every line a human reads showed only `UNVERIFIED … → retry the check`. The operator was told to retry a network that had already disagreed once, which is different guidance. One row per network is what the verdict needs and it cannot hold both facts, so the surviving row now carries the one it replaced: `an earlier attempt disagreed: expected …, observed …, anchor …`. Asserted in both directions, because a note that appeared on every retried network would be worse than none. That is the third shape of the same tension — a `needs-ack` erasing a mismatch, an `error` being refused to protect it, and now the mismatch surviving as a fact on the row that replaced it. Carrying it is what makes the trade unnecessary. **And A4.1's test count went stale for the third time in a day** — inside the commit that corrected it, which is the clearest possible argument against quoting one. The row no longer carries a number; it carries the command that produces it at whatever commit the reader is on. Fixing the mechanism rather than the digit. Also rewrites five comments in the test files as invariants rather than accounts of what a previous version did wrong — three of them added by the commit whose message claimed that sweep was done. Fifth round on this rule here; the pattern is that a fix's own comment explains the bug it fixed, which is exactly what the commit message is for. 1284 pass in `script/deploy/safe/`, tsc-files and eslint clean. Removing the carry kills the visibility test. * fix(safe): the superseded note belongs to the network, and is never accepted from a row (EXSC-698) Gate round on 21f29ec. The carry was right in content, gating and sanitisation — exhaustively: it names the last mismatch on the same (check, network) and nothing else, no sequence produces it without a mismatch in the log, and an ESC, `\r`, `\b` or newline injected into `expected`, `actual` or `anchor` reaches no rendered line. Two defects. **`fail → error → error` dropped the note again.** It was derived from the row it displaced, so the first retry carried it and the second did not — that one displaced an `error`, not the `fail`. Two failed retries against a flaky endpoint is the ordinary case, and the report was back to `UNVERIFIED … → retry the check` with the disagreement invisible. The note is keyed on the network for the whole run now, so it survives any number of retries. **An incoming `supersededMismatch` was trusted.** `recordCheck` validates status, anchor and network, not this field, and the rollup passed it through by spread — so a rehydrated document could assert a disagreement the log never recorded, and the digest names seven fields, not this one, so the attestation could not see it either. It is stripped on the way in and derived or absent. The digest exclusion is right for a derived note; it is exactly why the field must be underivable from outside. Both come from one change, and it is the same insight as the round before: the fact belongs to the network, not to whichever row is being replaced. Also rewrites six comments as invariants rather than accounts of a previous version — one of them added by the commit that claimed the sweep was done. **Two doc corrections, both of the same class as claims already retracted here.** A4.1 still cited mutation labels `M11` and `M3`, which are defined nowhere in the repo or these docs and had no committed harness — unresolvable references, like the 74-network run the row retracts two sentences earlier. And `09-work-packages.md` still recorded WP-4.1 as "not started" while lifinance#2335 exists; it now says modules-on-draft with the migration named as the outstanding part. 1286 pass in `script/deploy/safe/`, tsc-files and eslint clean. Reverting either fix kills a test. * docs(safe): state the invariant, not the previous implementation (EXSC-698) Eighth round on this area came back clean on the note mechanism: exhaustive enumeration of all 64 ordered triples per class shows every `error` survivor following a mismatch carries the most recent one and nothing else, a `pass` can never become the survivor of a mismatched network so it can never carry a stale note, no cross-network or cross-check bleed, and an incoming note cannot survive by raw property, getter, prototype chain or non-string value. The digest is unmoved by a forged note and correctly omits a derived one, because the row that produced it is digested directly. It also confirms no claim in `13-acceptance-suite.md` or `09-work-packages.md` §WP-4.1 is unreproducible from this branch and no reference in them is unresolvable. The one finding is prose, in the test I added to demonstrate the fix: the comment described how the previous derivation failed instead of what now holds. Seventh instance of that habit on this PR, which is enough to name the pattern — a fix's own test wants to explain the bug it closes, and the commit message is where that belongs.
…nce#2358) * fix(safe): resolve repo root without Bun-only import.meta.dir `import.meta.dir` is a Bun API and is undefined under Node, so `join()` threw ERR_INVALID_ARG_TYPE at module load. Since confirm-safe-tx.ts imports this module, the documented `bunx tsx` signing path crashed before parsing any arguments. Uses the portable ESM form already in checkDeploymentAddressConsistency.ts. Every other import.meta.dir in the repo is in a *.test.ts file, which runs under `bun test` where the API exists — which is why tests never caught it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(script): guard shipped modules against Bun-only import.meta.dir A behavioural test cannot catch this class of bug: under `bun test` the offending line works, which is how it shipped. Asserts on source instead, following the repo's *-placement.test.ts convention. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(rules): forbid Bun-only APIs in shipped script modules Records why neither tsc nor the test suite catches this class: tsconfig declares both node and bun types, and tests run under Bun where the APIs exist. Names the portable replacement and the backstop test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(rules): correct the Bun-only API guidance and the guard's matcher The [CONV:NODE-RUNTIME-APIS] bullet told authors to call runMain unconditionally instead of gating on import.meta.main. 13 of the 19 shipped modules carrying that guard are imported by their own *.test.ts for their pure helpers, so following the advice would execute those CLIs during bun test. Replace it with the portable entrypoint comparison, verified equivalent under tsx on Node 18/22 and under bun. Split the bullet per API, since the three fail differently: import.meta.dir is undefined on every Node version, import.meta.main fires on Node 22.23+/24 and only no-ops silently below that, and Bun.* throws at the call site. State the live counts so the rule reads as the repo's direction rather than a claim the tree is already clean. The guard now matches the expression rather than the raw substring and strips comments first, so a module that documents the hazard is not reported as a violation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(rules): point the Bun-only API carve-outs at EXSC-971 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…igin/main at sign time (EXSC-704) (lifinance#2348) * feat(safe): grade the target state against origin/main at sign time (EXSC-704) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(signing): record the pinned target-state check (EXSC-704) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): resolve a Tron facet address against its base58 deployment record (EXSC-704) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): refuse an install whose address no deployment record names (EXSC-704) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(signing): name the unidentified-address refusal (EXSC-704) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): refuse a self-contradicting deployment record instead of taking the first match (EXSC-704) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(safe): update target-state placement pins after the gated signer funnel Main's codehash gate replaced `deployerSafe.signTransaction(...)` with the shared `signTransaction(..., deployerSafe)` funnel; keep the irreversible-call source order assertion pointed at the real spellings. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(safe): print pinned target-state lines without detailLines.push The sanitising detail-block smoke check forbids inline pushes of stored fields onto `detailLines`. Target-state lines are computed verdicts, so join them at the print site instead of mutating the builder's result. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(safe): print target-state after the sanitised detail block Keep the detail-block smoke pin (`consola.info(detailLines.join(...))`) and emit pinned target-state lines as a separate consola pass so we never mutate the builder result with `detailLines.push`. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(safe): retry a failed pinned-state fetch instead of pinning it (EXSC-704) A transient git fetch must not refuse every later proposal in the same process. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(safe): take the deployment record's name from every match, and fall back across networks (EXSC-704) A blank-version sibling was dropped before the contract names were collected, so a differently-named blank row could no longer contradict — the names now come from every match while the versions still ignore blanks. An address is a CREATE2 identity, so where the network under review has no record row the same address on other networks is consulted, name and version having to agree. Measured against the production mirror: 13 installed facet addresses have no row on their own network — robinhood's whole core facet set among them — and all 13 resolve this way, none ambiguously. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): assert the anchor's remote and fetch it by explicit refspec (EXSC-704) `origin` is whatever a clone points at, so the ref alone did not establish where the anchor came from: a fork remote would have let a proposer author the expected state. The read now refuses unless origin is github.com/lifinance/contracts. The fetch also relied on git's opportunistic update of refs/remotes/origin/main, which a clone without a covering refspec skips — leaving a stale anchor with no error. A test falsifies that: in such a clone `git fetch origin main` provably leaves origin/main where it was. The unresolved-version refusal now names its remedy, since the blank is in the deployment record rather than in the cut. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): read the anchor through the remote-tracking ref, and keep the record lookup on its own network (EXSC-704) The anchor was read as `origin/main`, a short name git resolves through refs/, tags and heads before refs/remotes — so a tag of that name in the clone would have been read instead of the ref the fetch had just written, which is the authorship the remote assertion exists to deny. It is now spelled in full. The cross-network fallback added a commit ago is withdrawn. Its premise does not hold: the mirror carries 0xae77c9ad… as CalldataVerificationFacet on 17 networks and LiFuelFeeCollector on opbnb, and each of the 12 robinhood clearances it produced rested on a single uncorroborated row from one other chain — turning a refusal into a pass on evidence weaker than the refusal. It also bought no signability: the codehash sign gate refuses those same addresses on its own per-(address, network) record read. The refusal now names the remedy instead, which is the missing record either way. The name-from-every-match half stands. A remote that could not be read is no longer reported as the wrong remote, and is no longer memoized — an exec that failed says nothing about the remote. The URL pattern also admits GitHub's SSH-over-443 spelling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): say plainly that the check could not run, and shorten both remedies (EXSC-704) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…ifinance#2351) * feat(codehash): the attestation key and the audit bridge (EXSC-952) First of WP-5.2, and deliberately the schema half: the spec requires the `sourceClosureHash` field to exist before the first mint, or early manifests cannot carry the audit bridge. Pure modules with golden tests, so this lands without touching the CI workflow or colliding with the open gate PRs. The key names every input that can change the bytecode. Anything left out lets two different builds share one entry, and the set-membership compare in `attested-set.ts` then answers about a build the deployed code is not. `viaIR` is required rather than optional, because absent and false are the same build only by coincidence. The zk section carries the solc fork and LLVM version alongside zksolc, since those float independently and zksolc alone does not identify the build (F14). `sourceClosureHash` is taken over the source closure only, never the settings, so the same sources built under two profiles bridge to the same audit record — an audit is of source, not of a build (E1). It refuses a closure that names one path twice with different hashes rather than picking a winner, because either choice hashes a closure that was never compiled. Fields are length-prefixed rather than delimited. A delimiter only works while no field can contain it, which is an assumption about every present and future input, and source paths are outside this module's control. Length-prefixing needs no such assumption. Conflicting attestations are returned, not resolved: two entries under one key mean the key does not identify the build, so a caller must refuse rather than take whichever was filed first. A bytecode attested from two different source closures is as disqualifying as two bytecodes under one key. Agreeing duplicates — CI and a local rebuild — are not conflicts. Five properties falsified one at a time, each failing only its own test: dropping `viaIR`, naming only zksolc, removing the absent-zk marker, dropping the length prefix, and ignoring a source-closure disagreement. * fix(codehash): treat hex hashes as case-insensitive in the attestation key (EXSC-952) Also pin the absent-zk marker to the exact 3:evm field, drop the stale delimiter docstring, and use real Foundry profile names in the fixture. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(codehash): name the compiler settings in the attestation key (EXSC-952) The key held `profile` as a stand-in for the settings that decide the bytecode. A profile name survives a retune of the profile it names, so `[profile.default] evm_version = "prague"` or a changed `optimizer_runs` leaves every serialised key byte-identical while every masked hash moves. Two builds then share one key, and the collision sits in the key itself where `findAttestationConflicts` cannot see it. `evmVersion` and the optimizer run count are now named alongside the profile, both read from the same `metadata.settings` the closure hash already reads. `buildSettingsFromMetadata` does that read once for every producer, so CI's mint and a local rebuild cannot disagree about solc's `viaIR: null`, and refuses an artifact that does not state a setting rather than assuming a default. Hashes entering the key are frame-checked with `hex.ts` and compared through a shared `normalizeHash`, which `attested-set.ts` now imports instead of carrying its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): refuse the build shapes the attestation key cannot express (EXSC-952) `buildSettingsFromMetadata` read a missing `optimizer.enabled` as "the optimizer did not run". solc omits `enabled` exactly when `optimizer.details` is supplied, so an optimized build with tuned details keyed as `noopt` and collided with an unoptimized one — the collision the module exists to prevent, reached through the branch that was supposed to make the optimizer explicit. Details cannot be expressed by any key field, so a build carrying them is refused rather than keyed as if absent, and an optimizer that does not state whether it ran is refused too. Hashes entering the key are checked as keccak digests, not merely as framed hex. `frameFault` was written for bytecode, whose length is not known in advance; it accepts `0xdeadbeef` and a 20-byte address in a hash slot, which are the realistic corruptions. `digestFault` adds the length. `evmVersion` is lowercased into the key, since one side reads it from `foundry.toml` and the other from `networks.json`. `sourceClosureHash` refuses an entry with no path. `IAttestationConflict.key` is copied rather than aliasing the caller's attestation. `deploy-self-check.ts` was the third copy of `normalizeHash` and now imports the shared one. Corrects a premise carried in from review: solc emits `viaIR` only when the IR pipeline ran and omits the key otherwise — it is absent in all 193 artifacts of a real `out/` tree, never `null`. `jq '{viaIR}'` renders an absent key as `null`, which is where the `null` reading came from. The mapping was already right; the docstring and the fixture were not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(codehash): identify a build by hashing its settings, not by listing them (EXSC-952) The key named the settings it knew about: evmVersion, viaIR and the optimizer run count. That is a list somebody has to keep complete against every compiler release, and an incomplete one fails silently in the unsafe direction — the omitted setting moves the bytecode, the key does not move, and two builds share an entry. The list was already incomplete at three of roughly seven: optimizer.details, metadata.bytecodeHash, appendCBOR and libraries all change the output and none was named. `buildSettingsHash` hashes `metadata.settings` as the compiler emitted it, so a setting nobody has heard of yet is covered the day it appears. Only `compilationTarget` and `remappings` are stripped, both with a stated reason: the first is the source identity, the second decides what an import resolves to rather than what the compiler does with it, and the resolved sources are already the audit bridge's job. Object keys are sorted and null values dropped, so emitter key order and a JSON reader that fills absent keys cannot split one build across two keys. Three consequences: `profile` leaves the key. `[profile.ci]` inherits `[profile.default]` and differs only in fuzz settings, so the two compile to the same bytes and the name was splitting one build across two keys — a lookup under one then reads a build we made as unattested. It is carried on the attestation as provenance instead, alongside the readable settings, so a refusal still names the settings rather than only their digest. `sourceId` and `repo` join the key, because a contract's name does not identify its source. Two unrelated `IPool` contracts live in `src/Periphery/`, and `Executor` v2.1.0 exists in both repos with different code; all of these shared a key before. Settings carry where they came from. zksolc writes no `metadata` object at all — 280 of 280 artifacts in a real `zkout/` tree — so a zk build's settings come from repo config, which is a claim about what we asked for rather than a report of what ran. `configuredSettingsHash` tags those `config` so they can never answer a lookup for a self-reported build. The zk producer itself is still to come. Verified on the real `out/` tree: 190 artifacts, 190 distinct keys, zero collisions, and the two `IPool` contracts now separate. Every guard mutation-tested, including an over-greedy strip list. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): name the source closure in the attestation key (EXSC-952) The key named a contract's own file and its declared version, and neither moves when something the contract inherits does. Commit 4cfabbf bumped `SwapperV2` alone; the 31 facets that inherit it changed bytecode while keeping their own file, version, settings and solc version — so recompiling AcrossFacet's closure at both commits gives two different masked hashes under a byte-identical key. Both outcomes of that are bad: with both attestations filed the conflict check refuses, blocking signing fleet-wide on any library bump; with the older pruned the survivor silently answers lookups for the other build. `closureHash` is now a key field. It was already computed and already on the attestation, one field short of the key. Any change to any source the build compiled moves it, which is what the key claimed to guarantee and did not. `sourceClosureHash` stays settings-independent, so the audit bridge is unaffected: the same sources under different settings still bridge to the same audit record. The conflict check loses its source-closure arm, which same-key-different-closure made unreachable, and keeps the masked-hash arm. `repo` leaves the key for the same reason — two repos with different code have different closures — and rides along as provenance. Also from this review round: `buildSettingsHash` is no longer exported. Its `source` tag was a free parameter, so a caller could mint an `artifact`-tagged hash from configured values, which is the one thing the tag exists to prevent. The only ways in are now `artifactSettingsHash` and `configuredSettingsHash`, where the tag follows from which reader was used. Both key hashes are checked with `digestFault` in `serialiseAttestationKey`. They were the only hashes in the module reaching it unvalidated, so `'nope'` became a plausible-looking key field. The conflict payload is `structuredClone`d. Both spreads were shallow, so the nested settings object stayed aliased to the caller's attestation while the comment above claimed otherwise. `IArtifactIdentity.settings` is renamed `hashedSettings` and matched on `IMintedAttestation.build`. It was documented as the settings "as emitted" while holding the stripped set, so "the settings" meant two different things in one module. Comments trimmed to the repo's discipline: the module header is three sentences and the design rationale lives in the PR body. Verified: 190 real artifacts, 190 distinct keys, 0 collisions, 169 distinct closures — several contracts legitimately share a closure, which is why the contract name and source id stay in the key. Every guard mutation-tested, including the nested-key-ordering mutation that survived the previous round's suite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): guard the zk sub-join and correct the key's own claims (EXSC-952) Three behaviours had no test behind them, one of which is constructibly wrong. The zk triple is length-prefixed inside the outer join, but nothing pinned that: flattening it to concatenation passed the whole suite, and zksolc 1.5.15 / fork 0.8.29 then collides with zksolc 1.5.1 / fork 50.8.29. The conflict payload's `key` was cloned but only `build` was asserted, and `serialiseAttestationKey` accepted every degenerate string field — harmless for collisions, but a key naming no contract strips every refusal in the module of the name it reports, so the identifying fields are now refused when empty. Two claims in the previous commit were false. The `closureHash` comment said 4cfabbf "bumped SwapperV2 alone" and named "the 31 facets that inherit it": that commit bumped three contracts and deleted a fourth, and 31 is today's count — it was 33 there, with eight since removed and six not yet written. The commit citation is gone rather than corrected; the constraint it was evidence for is the comment, and the evidence belongs in the PR body. `settingsHash` also named `artifactSettingsHash` as its only producer, which is wrong for every `config`-tagged key. The zk arm's second data gap is now stated where a producer will hit it: `closureHash` comes from `metadata.sources`, and zksolc writes no `metadata` at all, so a zk build has no closure source any more than it has a settings source. There is no reader for it here and the PR does not add one. Fixtures moved off CBridgeFacet, which was deleted from this repo, onto AcrossFacetV4, which exists and does inherit SwapperV2; the remappings entry is the one solc actually emits; and the zk fixture no longer hashes a solc-shaped settings object zksolc could not have produced. The `repo` field's justification, over-trimmed in the previous commit, is back: one source path exists in both repos with different content, which is why the closure hash makes the repo name redundant. Also dropped a dangling `(E1)` marker that resolves nowhere in this repo, and documented the `structuredClone` throw. Verified: 190 real artifacts, 190 distinct keys, 0 collisions. All three newly-guarded behaviours mutation-tested and each fails only its own test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): judge empty settings on the canonical form, not the key count (EXSC-952) The refusal counted surviving keys before `canonicalJson` ran, and `canonicalJson` drops null values. So settings of nothing but nulls had keys to count, passed the check, and canonicalised to `{}` — the state the refusal exists to prevent. Reproduced: `{evmVersion: null, optimizer: null}` and `{viaIR: null, libraries: null, metadata: null}` hash identically. Unreachable from solc's `metadata.settings`, but `configuredSettingsHash` takes a caller-shaped object whose form this PR deliberately leaves open, so the zk path is exactly where it would land. Reported by CodeRabbit on the ready flip. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
* feat(safe): RPC quorum for integrity reads (EXSC-870) Adds the agreement layer an integrity read needs before it may be believed: two independent providers, reading the same block, returning the same value. Everything else refuses. Not wired into any call site. WP-4.2 ships the decision module only; wiring waits behind lifinance#2329, as the other built-and-unwired packages do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): an IP-form endpoint cannot establish independence (EXSC-870) A bare IP host became its own provider identity, so an endpoint given as an address could never share an identity with the hostname form of the same node. That is the splitting direction, the only one that can invent a quorum: the endpoint list comes out of a writable store, so adding an IP alias of a node you already control derived a second provider, and one upstream agreed with itself and the read went green. A name may resolve to that very address, so independence is not establishable either way. Every IP host now collapses onto one identity — which also stops several addresses inflating the count between themselves — and a usable IP endpoint refuses the read outright with `provider-identity-unverifiable`, non-transient, ahead of every verdict that reads the provider count. No committed endpoint is an IP literal, so this costs nothing today. The two identity tests asserted the earlier intent, that an address is kept whole rather than sliced to its last two octets. That concern is real and still handled — an address is not a hostname — but it is not a reason to make the address an identity of its own, so they now assert the collapse and the octet-slicing case is stated as the reason an address is treated apart. A trailing-dot host also split from its rooted form, because dropping the empty label is what keeps the last two labels meaningful, and nothing asserted it. Coverage reports now carry the source they measured, as a required argument. `config/networks.json` stores `rpcUrl` as a single string, so a count taken from it reports zero at any quorum above one however many endpoints the fleet really has — a schema property that reads as a fleet measurement once it is a bare number. The operational inventory is the `RpcEndpoints` collection. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): render the coverage source, and give the identity function its doc back The `source` field went in required and assigned and **never rendered**, while its own JSDoc said it was carried into the rendered line. So the thing it exists to prevent — a bare coverage number quoted as a fleet fact — was not prevented at all. The patch that changed the render line was written and never run, and the claim shipped in the PR body regardless. Two tests now assert the source reaches both the green and the refusing line, so the field cannot go dead again. `IP_LITERAL_IDENTITY` and its docblock had been inserted between `providerIdentityForUrl`'s JSDoc and the function, so that `@param`/`@returns` block documented a string constant and the exported function had none. The constant now sits above it, and the `@returns` is corrected: there are two shared sentinels, not one. The refusal's precedence is pinned rather than incidental. It is settled before a fork, before a disagreement and before an outage, because each of those reads a provider count that an unknowable identity makes unsound — and an outage reported in its place would be *retryable*, which is false: another attempt returns the same endpoint list. It stays after `quorum-misconfigured`, a caller defect that must surface whatever the endpoints look like. The ordering docstring claimed shortfall is settled after divergence without naming this exception; it names it now. The condition reads every endpoint consulted, not only those that answered, and a test says so. A check firing only when the bad endpoint happened to respond would refuse one read and green the next on an identical configuration. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(safe): say why the source is required without naming the config keys `200-typescript.md` forbids naming config keys and paths in prose comments, because a comment that does goes stale where the code does not. The reason survives — a source holding one endpoint per network can only ever report none reaching a quorum — without the file and field names, which belong in the PR. The refusal's own block ran to six lines in a file whose other inline comments run to two, and its last two were threat-model rationale. The sentinel's docstring opened by describing what the code does before saying why. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): a root dot does not make one intranet node into two providers A single-label host was returned verbatim, so `rpc-node.` and `rpc-node` derived different identities and one node counted as two independent providers — the splitting direction, which is the only one that can invent a quorum. Returning it from its labels drops the empty one, as the multi-label path already did. Narrow in reach: it needs a host with no domain, so an intranet or local node rather than anything in committed config. It is closed rather than disclosed because the cost is one expression and the failure is a green read from a single upstream. Both directions are pinned. Collapsing every single-label host onto one identity would also close it and could only ever refuse — the safe direction — but it would quietly stop two genuinely distinct intranet nodes from reaching a quorum, so the test asserts that two of them stay two. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): name a declared IP-and-hostname merge after the host (EXSC-870) Co-authored-by: Cursor <cursoragent@cursor.com> * fix(safe): decide the IP refusal by group membership, not by the group's name (EXSC-870) The declared-merge fix named each group after any member with a non-sentinel identity, and the refusal then tested whether the name was the IP sentinel. Two ways out of that followed, and the first is a false green: An empty host derived the identity `''`, which is not a sentinel, so it won the group's name over the IP sentinel in every input order. A bare IP endpoint and any host-less URL sharing a declared providerId therefore counted as a named provider, and a list of one honest endpoint plus those two reported `agreed` on a quorum whose second arm may be the first arm's own address. Both the URLs and the providerId come from the same writable store, so one actor supplies the whole thing. Fixing a false red had opened a false green. Between the two sentinels the name was first-write-wins, so an unparsable endpoint declared to be the IP endpoint refused in one order and agreed in the other. Three changes. A host with no labels reaches a sentinel instead of becoming an identity. Countability is now named positively — a non-empty, non-sentinel identity — rather than as absence from a list of the unverifiable ones already known, which is what let `''` through. And the refusal asks each IP endpoint whether its own group reached a countable identity, which no group name can launder and which does not depend on arrival order. Also aligns the deciding count with the coverage report: `independentProviders` counted sentinels in `evaluateRpcQuorum` while `evaluateQuorumCoverage` excluded them, so the pre-flight report and the live gate disagreed about the same endpoint list. * test(safe): hold the deriver's contract, and stop the docblock overclaiming (EXSC-870) Falsifying the previous commit one change at a time found that the emptiness half of the countability test is observed by nothing: once an empty host derives a sentinel, no input reaches it. Reverting it alone leaves the suite green. Rather than drop it and leave the set named as a denylist again, the property it protects is now asserted where a test can hold it: every identity the deriver returns for a hostile URL is a sentinel or a real host name. That fails if a future sub-two-label path returns something empty, which is the break the emptiness test exists to catch. The docblock said an empty host derives an uncountable identity, which the previous commit made false, and it now says which half is unobservable and why it stays. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…(EXSC-699) (lifinance#2342) * feat(safe): resolve calldata addresses against the deployment record (EXSC-699) A pure sign-time decision module. It grades the addresses a Safe proposal's calldata references against deployment entries a caller supplies, refusing the ones the record cannot account for and erroring — never passing — when the question cannot be answered. Not wired to any production path. `confirm-safe-tx.ts` is owned by lifinance#2329; wiring waits for that to merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): close the fail-open defaults in the calldata address check (EXSC-699) Five policy lookups treated absence from a set as permission: - an index source outside DeploymentIndexSourceEnum's refusal map decided addresses; only the deployment record may decide now, everything else errors - a role outside REFUSAL_BEARING_ROLES warned; a role neither that set nor the new WARN_ONLY_ROLES names now errors - a refusal-bearing reference with no anchor-supplied identity graded identity-unchecked and passed; it now errors, as a never-queried address does - expectation keys were matched lowercased but never normalised, so the checksummed form both _targetState.json and the selector registry use matched nothing; keys are normalised in the module, and an unusable or contradictory key errors - a never-queried removal target errored while a queried-and-unknown one only warned, false-redding the rollback path T2 puts outside the gate A zero-reference verdict said "0 of 0 addresses resolved"; it now names that nothing needed resolving. Three docstring claims the code contradicted are rewritten, and the illustrative address pair is now one facet in both directions: SymbiosisFacet@1.0.0 recorded and absent from the repo files, SymbiosisFacet@2.0.0 in the repo files and absent from the export. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): correct two false claims, in a commit that was meant to fix false claims The `normalizeExpectations` docstring said `_targetState.json` and the selector registry "both key on the checksummed form". Neither holds addresses at all: `_targetState.json` matches zero of `0x[0-9a-fA-F]{40}` and its shape is network → environment → diamond → contractName → version, while the selector registry maps a 4-byte selector to `{ name, signature, source }`. The same sentence was repeated in the expectations field's own doc, which named those two as the anchors that supply the map, and in a test comment. The normalisation is right and stays — a caller assembles the map from whatever names an address, and the helpers that hand one back return the checksummed form while references arrive lowercased. What was wrong was the reason given for it, and it mattered twice: it told a reviewer an anchor exists that does not, and it named a source for a field the whole verdict rests on. The field's doc now says plainly that no committed file supplies it yet and that the wiring package must name an address-bearing anchor. The errors docstring claimed every one of its seven causes applies "for a refusal-bearing reference". Five do not: an unavailable store, an undecidable source, an unreadable call, an unusable expectations key and a role with no policy all error whatever the proposal asks for — the last necessarily so, since it fires exactly when the role is in neither set. Only the never-queried address and the missing identity turn on the role. Split accordingly. Also a false red rather than a false claim: `version` is documented "checked only when present", so an absent one means any version — which a stated one narrows rather than contradicts. Comparing them with `!==` made a name-only anchor and a name-plus-version anchor irreconcilable, which is exactly the merge the docstring describes. Only two stated versions can now disagree, the narrower expectation survives the merge, and both directions are pinned. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(safe): state what holds, not what the code used to do Four comments argued against implementations that are not in the file: the source guard's "not by absence from SOURCE_REFUSALS", the role guard's "rather than a warning", and the assert's status note about no call site existing yet, which rots the moment one lands. The T2 sentence also overclaimed. It said a removal target neither refuses nor errors however little the record says about it, which is true of not knowing the target and false when the record cannot be read at all — that errors under T3. Both halves are now stated, since the difference is exactly what a reader of that carve-out needs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(safe): name the undecidable-version case after the outcome it asserts (EXSC-699) The test was called "still refuses" and asserted only `verdict.error`. Those are different outcomes here — `refuses` means an address contradicts the record, `error` means the check has no answer — and two stated versions disagreeing is the second, not the first. The name claimed the wrong one and nothing observed the difference. Renamed to the outcome it asserts, and `refuses` is now asserted false beside it, so the distinction the module is built on is actually checked rather than implied by a title. Raised by CodeRabbit at 38c8584. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…C-872) (lifinance#2341) * feat(timelock): cancel-decision matrix for the pre-execute check (EXSC-872) Adds a pure decision module that maps the pre-execute re-derivation's signals for one queued timelock operation onto execute / cancel / hold / block, plus a pass-level circuit-breaker, renderers and a broadcast-path assertion. Cancel fires only on a divergence derived from an authoritative anchor and agreed by a provider quorum; every error, unsupported leg, sub-quorum or stored-value divergence lands on a non-destructive verdict instead. Also records the open-executor gap: EXECUTOR_ROLE is granted to address(0) by both timelock deployment scripts, so the breaker's withholding step is gated on the executor posture, and a parse of the committed scripts is asserted as still open. * test(timelock): exercise the fail-closed default on every signal field (EXSC-872) * fix(timelock): fail away from the posture that lets the breaker withhold (EXSC-872) Three findings from a gate round, all the same shape: a check resolving doubt toward the permissive answer. `evaluateExecutorPosture` matched only the canonical zero address and then fell back to `restricted` for anything non-empty. So `0x0`, an unprefixed run of zeros, a bytes32-padded zero, an empty string and outright garbage all read as `restricted` — and `[null]` threw. `restricted` is the only posture under which the circuit breaker may withhold a cancel, so a malformed holder list would have withheld cancels while `EXECUTOR_ROLE` really was open to everyone: the one outcome the F7 coupling exists to prevent. Doubt now resolves away from it — any spelling denoting no address is `open`, anything unreadable as a named address is `unknown`, and only well-formed non-zero addresses give `restricted`. `operationState` was named by the two values that block, so a value it did not recognise — an absent field, a prototype property, something a newer controller returns — reached the execute branch. It is now named by what may proceed, and `ready` is a conjunct of that branch as well, because `execute` is the only irreversible action here and its conditions belong at the point of authorisation. `pending` still passes the guard deliberately: an operation inside its delay is exactly one worth cancelling, it is only not executable. The existing "a pending operation is still cancellable" test is what caught an earlier version of this fix that wrongly blocked both. The quorum test asserted `agreeingProviders` against `MIN_AGREEING_PROVIDERS_FOR_CANCEL`, so mutating that constant from 2 to 1 moved the assertion with it and let a single provider force a cancel. Pinned to the literal instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(timelock): a verified operation still inside its delay is a wait, not a mystery Adding `ready` as a conjunct of the execute branch left a verified but not-yet-matured operation falling through to `unclassified-signals` — block, page, no retry — for a case the guard above it names in words. It is the one combination the matrix explicitly reasons about, reported as one it does not recognise. That path is reached by the intended caller: `execute-pending-timelock-tx.ts` in `--rejectAll` pushes a pending operation into the processed set, so an ordinary queue state paged a human and was dropped from the retry that resolves it. It now holds under `op-not-yet-matured` with a notice and a retry, and the decision matrix in the docs carries the row. Comment discipline, four items. "Named by what may proceed, not by what may not" and "Restated here rather than relied on from the guard above" both argue against implementations that are not in the file. The posture docstring's prose paragraph duplicated its own `@returns` and closed on why the fix matters, which belongs in the PR; the surviving sentence is the constraint, and the blank comment line before `@param` that every other block in the file has is back. A test was named for its technique — "asserted as a number and not as the constant" — rather than for the behaviour it pins, and its comment described a defect in other tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): gate the not-yet-matured hold on the signals it claims passed (EXSC-872) The `op-not-yet-matured` branch was conditioned on `operationState` alone. Each branch above it refuses only the values it names, so a signal carrying anything else — an unrecognised `integrity`, `opIdentity`, `deploymentRecord` or `executability` — passed every guard, failed the execute branch on its `=== 'match'` conditions, and then took the hold with `alert: 'notice'` and `retry: true`. An operation examined by nothing was described as one where "every pre-execute check passed" and handed to the next unattended pass. The four signals are now re-stated on that branch rather than inferred from the refusals above it. They are repeated rather than extracted because the execute branch names its own conditions at the point of authorisation on purpose. Raised by CodeRabbit at b34e120. Its companion finding on the same branch — that a verified pending operation reaches `unclassified-signals` — does not hold at this head: the pending branch precedes that fallback and is the fix that put it there. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
… now (EXSC-697) (lifinance#2344) * feat(safe): tell a signer whether a proposal would revert if executed now (EXSC-697) Adds `script/deploy/safe/executability-simulation.ts`, a pure sign-time decision module: `evaluateExecutability` grades already-decoded payloads and already-made chain reads, `renderExecutability` prints them, and `assertProposalWouldExecute` throws. `toCancelDecisionExecutability` projects a verdict onto the three values the timelock cancel-decision matrix reads. Each finding carries whether it is proven from the calldata alone or predicted from state read now, and the verdict says how many of each it is holding. A simulation that could not run errors and blocks like a revert, with no acknowledgement path (T3). A predicted revert on a Remove cut warns rather than blocks (T2); a proven one blocks. Not wired into any call site. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): walk selectors across the whole proposal, not one payload (EXSC-697) The sequential walk kept its accumulator inside `gradeSelectors`, and that function is re-entered per payload. So two `diamondCut` calls in one proposal — the `multiSend` and `scheduleBatch` shape the module's own `path` examples show — each adding the same selector to the same diamond read clean, because each is clean against the diamond as it stands and nothing simulates the pair. A node's `eth_call` cannot see it either: it simulates each call against the state before the proposal runs. That produced the one outcome this module exists to prevent: a proposal whose second call provably reverts with `FunctionAlreadyExists` printed the green "nothing in this proposal reverts" line. The accumulator now spans the proposal, keyed by diamond — a selector moved on one diamond says nothing about another, and dropping that key turns an ordinary fleet rollout into a false red, so both directions are pinned. The cut-action allowlist was exercised at one value (7), so widening the set to include 3 — the value an off-by-one enum change produces — left the suite green while an unexecutable cut passed. Pinned as a set, both ways. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): a composition proof does not contradict a succeeding eth_call Making the selector walk span the proposal broke the contradiction guard's premise. That guard reads a proven finding as meaning the payload cannot execute whatever the state is, so a succeeding `eth_call` must be simulating something else — true while every proof came from one payload's own bytes. A proof resting on an earlier *call* in the proposal is invisible to that payload's `eth_call`, which a node runs against the state before the proposal does anything, so a succeeding call there agrees with the finding rather than disagreeing with it. The result was that the conflict class the previous commit was written to catch arrived with `error: true` and the line "the simulation and the calldata disagree", and projected to `error` rather than `would-revert` — so the cancel matrix held on an unanswerable instead of reading a revert. The accumulator now records which payload moved each selector, so the two cases are distinguishable: within one payload the cuts run in a single call and that conflict does show up in its own `eth_call`, which keeps the guard meaningful where it was always right. Findings carry `composed` and the guard skips those. The operator-facing source line was wrong for the same reason — it said "by an earlier cut in this same batch" for a conflict spanning two calls. It now names a call or a cut according to which it was. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(safe): pin the accumulator's key, and state the order it depends on Un-normalising only the accumulator's diamond key survived the suite: the immutability comparison still reads the normalised address, so nothing else noticed, and a proposal naming one diamond in two spellings went back to reading clean. One diamond named two ways is now a test. Sharing the accumulator across payloads also made `payloads` order load-bearing where it had only mattered inside one `cuts` array, and the field said nothing about it. A caller that groups by target or sorts by path gets a verdict about a proposal nobody will submit, so the field now says it takes execution order. `gradeSelectors` is not exported, and the tag block added with the accumulator was the only one in the file on a helper — its siblings carry none. Reduced to the constraint. Two test comments argued against the code's own previous shape rather than stating what holds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): compare collected facet addresses case-insensitively (EXSC-697) Co-authored-by: Cursor <cursoragent@cursor.com> * fix(safe): code-check a facet address written with an uppercase hex prefix (EXSC-697) The address test anchored on a lowercase `0x`, and its only caller skips a cut it rejects without recording anything. So a facet given as `0X…` was dropped from the code check silently: no `FacetContainsNoCode` finding and no unchecked note either, which is the one combination that tells the signer nothing. The same escape was closed in `deployer-key-power.ts` for the same reason. Also drops a docblock line that asserted a caller precondition as a fact and restated what the interface's own docstring says twelve lines above. * test(safe): declare findingFor's return type, and cover both casings again (EXSC-697) Two items raised on this PR and deferred while a review was queued. Neither is a behaviour change and neither is falsifiable, which is worth stating rather than implying otherwise: `findingFor` was the one helper in the file without an explicit return type. Annotating it changes nothing tsc infers; the value is that the file reads consistently. The case-insensitivity commit moved the immutability fixture from a lowercase observed value to a checksummed one instead of covering both, so the lowercase side stopped being exercised. It now iterates both. No mutation of the current comparison distinguishes them — `normalise` on a lowercase string is the identity — so this is coverage symmetry against a future change that starts requiring the checksummed form, not a guard with a demonstrated kill. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…ry Tron RPC to /jsonrpc (EXSC-963) (lifinance#2350) * fix(safe): preview the hash mode Ledger screens, and stop routing every Tron RPC to /jsonrpc (EXSC-963) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): gate the Tron route on the TronGrid host, warn before the action select Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(rpc): append /jsonrpc only on TronGrid hosts (EXSC-963) A bare non-TronGrid host still 404s under that route (dRPC, PublicNode). Also drop the unlabelled getTransactionHash log that printed above the hash-mode filmstrip. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(safe): show only what the Flex was observed to show (EXSC-963) The hash-mode preview carried a page counter and a Skip header reproduced from the typed-data flow rather than read off a device. A detail the operator is invited to check against a screen that may not show it teaches them to shrug at the mismatch the preview exists to raise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): assert the preview's contents as a whitelist, and say what it omits (EXSC-963) The previous test named the property "shows nothing that has not been read off the device" but asserted four string literals, so reinstating a counter in other words — a Cancel header, `< 2 / 3 >` — passed it. The screens' text is now compared as a set, which fails on any added row whatever it says. The trimmed screens also drop navigation the device may well show. Left unsaid that omission is a false alarm of its own, so the caveat now tells the signer that navigation around these lines is expected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): correct the gate-placement description of where acknowledgement happens The Confirm-step table still described the target-state gate as sitting before an acknowledgement prompt. This PR removes that prompt, and the only remaining action select runs before the gate rather than after it, so the row described a step that no longer exists and inverted the one that does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…876) (lifinance#2355) * feat(codehash): the attestation key and the audit bridge (EXSC-952) First of WP-5.2, and deliberately the schema half: the spec requires the `sourceClosureHash` field to exist before the first mint, or early manifests cannot carry the audit bridge. Pure modules with golden tests, so this lands without touching the CI workflow or colliding with the open gate PRs. The key names every input that can change the bytecode. Anything left out lets two different builds share one entry, and the set-membership compare in `attested-set.ts` then answers about a build the deployed code is not. `viaIR` is required rather than optional, because absent and false are the same build only by coincidence. The zk section carries the solc fork and LLVM version alongside zksolc, since those float independently and zksolc alone does not identify the build (F14). `sourceClosureHash` is taken over the source closure only, never the settings, so the same sources built under two profiles bridge to the same audit record — an audit is of source, not of a build (E1). It refuses a closure that names one path twice with different hashes rather than picking a winner, because either choice hashes a closure that was never compiled. Fields are length-prefixed rather than delimited. A delimiter only works while no field can contain it, which is an assumption about every present and future input, and source paths are outside this module's control. Length-prefixing needs no such assumption. Conflicting attestations are returned, not resolved: two entries under one key mean the key does not identify the build, so a caller must refuse rather than take whichever was filed first. A bytecode attested from two different source closures is as disqualifying as two bytecodes under one key. Agreeing duplicates — CI and a local rebuild — are not conflicts. Five properties falsified one at a time, each failing only its own test: dropping `viaIR`, naming only zksolc, removing the absent-zk marker, dropping the length prefix, and ignoring a source-closure disagreement. * feat(codehash): bridge an audit to the build that was deployed (EXSC-876) R2.10. `audit/auditLog.json` records 104 audits with five fields and nothing about the compiler, so a contract audited under one compiler and deployed under another is invisible today — and that is routine, since facets ship under the solc-floor profile while their networks declare cancun. Forward-only, per the ruling: no re-audit, no redeploy. Audits stay version-agnostic, because an audit is of source and a different compiler does not invalidate it. What needs review is the compiler set, vetted once per profile against solc and zksolc advisories — so `VETTED_COMPILER_SETS` is the review surface, and editing it is the review event. That map is deliberately not derived from `foundry.toml`. Reading the profile's current pin would make it agree with whatever the repo happens to pin today, which is the opposite of a review record: bumping the pin would silently bless the new compiler. A vetted profile whose versions have since moved therefore reports rather than passing, since the profile name agreeing is not the compiler set agreeing. `sourceClosureHash` carries the other half: it proves the build compiled the audited source over the full transitive closure, which is the unit because a swapped library changes the closure without changing the facet file — the `AcrossV4SwapFacet`/`LibAsset` case. Most audits predate the field, so the common answer is that the bridge cannot be proved; that is reported as its own note rather than conflated with the two closures disagreeing. Everything here is a NOTE and the return type carries no blocking state at all. A per-deploy version gate would brand every london-chain deploy unaudited, which is a false red on ordinary work. The failure this module must avoid is refusing honest deploys; refusing dishonest ones is the codehash gate's job. Notes accumulate rather than short-circuit, so a build that is both unaudited and built under an unvetted compiler says both — one note hiding another is how a signer comes to believe the lesser problem is the only one. Six behaviours falsified one at a time, each failing only its own tests: the no-audit note, the unvetted profile, version drift under a vetted profile, the zk section comparison, treating an absent zk section as a match, and muting a closure disagreement. * fix(codehash): name the zk half in a mismatch detail and refuse a prototype profile A profile named `toString` resolved against Object.prototype and reported a compiler set nobody vetted; the mismatch detail rendered solc alone, so a zk build printed the same string either side of the "but". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): compare closures and versions the way the sibling modules do Hash equality without normalizeHash reports one closure written in two hex spellings as two, and a compiler set compared byte-for-byte reports Cancun against cancun. Both are the false red this module exists not to produce. Pins what the suite could not previously tell from a constant: isFullyBridged in the false direction, the vetting date reaching the signer, and the note set behind "never refuses". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): an unchecked zk toolchain and an unusable closure are their own answers Nothing outside a bytecode trailer records the zk fork and LLVM versions, so every zk build a caller can honestly assemble reported a compiler mismatch that did not exist. It now reports that the zk half could not be checked, which is neither a false red nor a blessing. Two empty strings compare equal, so a build and an audit both recording an empty or sliced closure came back with no notes at all. The closure is validated as a whole digest before it is compared. Drops isFullyBridged: nothing calls it, and it is false for every deployment in the repo, so it could only mislead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): pin the compiler compare on a zk build, and refuse a zero closure The unverified-toolchain note is only safe if the solc and evm halves are still compared on that same build; nothing tested it, so widening the skip to the whole comparison passed the suite. digestFault checks framing and length, so bytes32(0) on both sides — what an unset field encodes to — compared equal and read as a proved bridge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(codehash): drift each zk version independently The suite only ever moved llvmVersion, so a zksolc or fork version that stopped being compared would not have failed anything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…C-956) (lifinance#2347) * feat(script): gate the bash sendOrPropose direct-broadcast route (EXSC-956) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(script): record the Tron address-reader limit of the direct-broadcast gate Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(script): document the direct-broadcast gate test helpers Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(script): keep NODE_ENV unset when spawning the direct-broadcast gate CLI bun test sets NODE_ENV=test, which makes consola silence info/success in the child — so the allow-path cases saw status 0 with empty stdout in CI while the refuse-path (consola.error) still passed. Mirror funnel-deploy-gate.cli.test. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(script): run the direct-broadcast gate without import.meta.main tsx on Node < 22.23 leaves import.meta.main undefined, so the CLI exited 0 without gating. Pair the portable entrypoint with an allow token so a silent pass cannot look like consent. Co-authored-by: Cursor <cursoragent@cursor.com> * test(script): pin the gate entrypoint through a symlinked path The source grep it replaces passes against an entrypoint compare that drops realpath, which no-ops the CLI whenever a path component is a link. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(script): name the real reason the gate cannot use import.meta.main Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(script): count only the allow token's own line on stdout as consent Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(script): stop the CLI cases flaking at bun's default timeout Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(script): observe both streams in the stderr-token case Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…nce#2365) * fix(security): redact RPC endpoints in script logs (EXSC-965) ETH_NODE_URI_* carries the provider key inside the URL, so a log line naming the endpoint writes a live credential into every transcript of every run. troncast leaked the Tron key this way on 2026-09-09. Five sites now go through redactUrls(), and a source scan keeps the class closed: it blanks comments and string bodies first, because the three innocent ways this repo names an endpoint — a doc comment, a help string and an object key — all read as leaks to a plain grep. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(rules): no script logs a full RPC URL (EXSC-965) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(security): close the scan's blind spots and the bash leaks (EXSC-965) The lexer had no regex-literal state, so a class like /['"`]/ opened a phantom string and blanked the rest of the file. Four shipped files were already invisible to the scan, one of them a troncast command. Regex bodies are now blanked like strings, with the division ambiguity resolved on the preceding token and an unterminated literal left untouched. An identifier followed by a colon was read as an object key, which also swallowed the consequent of a ternary — how an endpoint override is written at these call sites. A key now needs a preceding brace or comma too. RPC_URL is prefix-matched: the Tron path reads RPC_URL_TRON, which a word-boundary match never saw. Adds consola's remaining log levels and console's object-dumping methods, and hostOf as a redaction. Bash was leaking the same class the TypeScript fix closed, and the scan reads no .sh: five sites echoed the keyed endpoint, one of them ungated. They now go through redactRpcUrl. getRPCUrl's own stdout returns are untouched — redacting those would break every caller. Also redacts the error object in detect-evm-version, which printed the same URL one line after the redacted one, and adds tasks/** to the ts-unit-tests path filter so a leak there cannot ship with the required check green on a skipped job. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(security): pin the regex decision and redact the break-glass RPC errors (EXSC-965) The previous commit blanked regex literals but still refused one in value position, so `if (x) /['"]/.test(s)` reopened the phantom string it had just closed. Strings are now bounded to their line — a quoted string cannot hold a raw newline — which caps any future mis-decision at the line it happens on rather than the rest of the file. Postfix ++/-- and a closing quote now count as ends of a value. Nine mutations of the division-vs-regex decision survived the last round: the two tests naming it sat on lines the assertions never inspected. Replaced with fixtures where a wrong decision hides the leak, plus false-positive cases for the keyword list and the per-interpolation reset, which after the line-bounding are only observable that way. All nine now die. cast embeds the --rpc-url in its error text, so the break-glass pause path and the readiness check were publishing a keyed endpoint on every RPC failure. emergencyPauseBreakGlass.sh gets a local redactor rather than a dependency on helperFunctions.sh, which it deliberately does not source. Also moves redactRpcUrl out from between redactVerifyCmd and its docblock. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(security): parse the source instead of approximating it (EXSC-965) Three rounds of a hand-written lexer each shipped a hole, and each round's headline fix arrived untested because the fix itself made the old observation channel vacuous. The fourth round left more unpinned mutations than the third, so the mechanism was the problem, not the details. Every distinction this check makes is a question about the grammar — a type annotation versus a value, a binding versus a read, a comment or string versus code, a regex literal versus division. ts.createSourceFile answers all four for free. The repo already pins typescript; the earlier note that its compiler API was unavailable was wrong, an artefact of a worktree with no node_modules where bun auto-installed a different major version. Same verdict on the tree (0 findings, 240 files) and 0 blind on an inject-a-leak sweep in both directions, while the three false positives the lexer produced — an optional type property, an arrow parameter, and a regex reached without a semicolon — are all silent. Deletes blankNonCode, endOfRegex, endsAValue, REGEX_OK_AFTER_WORD, isObjectKey, endOfCall and redactedSpans. Also redacts the retry helper's exhaustion return in both break-glass scripts. Redacting only the per-attempt line left the terminal path — the one that runs during a real incident — returning the raw cast error, which every caller echoes. The success path stays verbatim, since callers parse it. bgRedactUrl had no test at all; both redactors and both retry paths now have one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(security): pin the scanner's sets by value and cover both retry scripts (EXSC-965) declaresRatherThanReads enumerated five declaration kinds and so reported a method shorthand, an accessor, a class property, a named function expression and a label as leaks. Asked structurally now — is this identifier the thing its parent names — with a property access and a shorthand property as the two deliberate exceptions. A fixed list of node kinds is the shape that kept growing a hole. The vocabulary and the log-method list are pinned by VALUE. Iterating the constant alone was self-referential: deleting an entry deleted its own case, so eleven of eighteen log methods and three of six identifiers could be dropped with the suite green. Also covers what nothing observed: a namespaced redactor, redactErrorReason (27 call sites), a non-consola receiver, the tasks/ root, and a finding's line and text. And the readiness script's exhaustion redaction, which no test reached even though it is the one that runs on a schedule in CI — the retry describe now runs against both scripts. Drops the callee guard in the walk: reaching it requires isLogCall, which forces the receiver to be consola/console and the method to be a LOG_METHODS name, so descending into the callee could never report anything. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(security): scope the scan exemption to named identifiers (EXSC-965) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(security): pin the exempted identifiers by value (EXSC-965) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(security): state that redactors are trusted by name (EXSC-965) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(deploy): load the worker without eval (EXSC-965) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(scanner): import typescript by name (EXSC-965) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(security): redact the two ungated bash error paths (EXSC-965) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(security): redact the sibling stderr prints on the same paths (EXSC-965) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(security): redact the pause-state and retry-handler error prints (EXSC-965) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(security): redact the network-info box and the uncaptured stdout prints (EXSC-965) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…SC-688) (lifinance#2291) * feat(audit): source-closure primitive, append-only log guard, content verdict Foundation for the audit gate that asks content instead of provenance (S2/F24). No workflow is wired yet — these are the tested predicates it will call. source-closure.ts identifies a contract by the keccak of its full transitive import closure, read through an injectable reader so the same closure can be computed at an arbitrary historical commit. External dependencies are git submodules whose contents are absent from the parent tree at any commit, so they are identified by their gitlink SHA — which IS present at every commit — and never walked. Verified against the real corpus: a hash computes at 87 of 88 audit entries covering a live facet, 0 unfetchable. audit-log-guard.ts diffs audit/auditLog.json base-vs-head. Without it the content check is self-defeating, since the recorded hash sits in a file the PR author can edit in the same PR. Coverage is compared by id MEMBERSHIP, not length, so swapping one audit id for another cannot pass. verify-audit-content.ts is the verdict. Any single passing audit settles the question; otherwise ERROR outranks FAIL, because an unreachable commit means the gate does not know, and per T3 not-knowing blocks with no acknowledgement path. There is no Revert-title exemption: a clean revert restores byte-identical audited source and passes on content. The corpus has a third state the ruling did not anticipate, raised as D10: 5 of 103 entries carry a non-SHA auditCommitHash (all LiFiDEXAggregator, honest records that the audit was not of a repo commit). ERROR-blocking those would wedge the contract permanently, so they compare against a pinned baseline recorded under a distinct field rather than claiming an audited closure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(audit): gate on audited content and guard the log against rewriting Wires the content-equality verdict into versionControlAndAuditCheck.yml and deletes the provenance test it replaces. The old check asked whether an audit's commit appeared in the PR's commit list. Git history only grows, so that stays true forever: a contract audited at commit X and then edited at commit Y in the same PR merged green (F24). Measured on the current corpus, 47 of 59 audited contracts on main already differ from every commit their audit entries name, at an unchanged @Custom:version — one of them, ChainflipFacet@1.0.1, by a deleted event declaration. The gate now hashes each contract's full transitive import closure at PR head and at every logged audit commit, and blocks unless one of them matches. Legacy entries need no new field: the closure is recomputed at their auditCommitHash, which is the same predicate against the same content (D3). The Revert-title exemption is deleted rather than earned. A clean revert restores byte-identical audited source and therefore passes on content, so the exemption bought nothing except a way to carry unrelated code under a Revert title. audit/auditLog.json becomes append-only in CI, checked on every PR rather than only when a contract changes. Without it the gate is self-defeating: the hash it compares against sits in a file the PR author can edit in the same PR. Exit codes distinguish a mismatch (1) from the gate not knowing (2). Both block — per T3 not-knowing has no acknowledgement path — but one code would hide a broken gate behind what reads as an unaudited contract. Known residual, raised as D13 on EXSC-883: the closure covers shared libraries, so a contract whose own source is byte-identical to its audit can still block when a dependency moves. ERC20Proxy@1.2.0 is the measured case. It does not bite in the normal flow, where the audit commit sits inside the PR, but a long-lived PR that merges main can flip red with nothing about its own contract changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(audit): ERROR-block when the append-only guard cannot read the PR base The guard read the audit log at base and at head and treated a missing read as "nothing recorded yet", passing. A git read returns nothing both when the file is genuinely absent and when the commit could not be read, so an unresolvable base made a security control report success without comparing anything. audit/auditLog.json has existed for the whole life of the repo, so absent-at-base on a real base commit means the read failed. The two states now have separate verdicts, and the decision is a pure function so both are covered by tests. * fix(audit): refuse a verdict when the gate was never told what to check citty drops an unrecognised flag silently, so a renamed or mistyped --contracts-file left the contract list empty. An empty list is a legitimate pass — no contract in the PR needs an audit — so the gate reported success without having checked anything, which is the one failure mode a security gate must not have. The two states are now distinct: a supplied-but-empty list still passes, while no source at all is an ERROR. Same reason the append-only guard separates an unreadable base from an absent file. * test(audit): cover the git seam, and trim history narration from a module header git-source-reader.ts had no tests despite being the git-integration seam a security-blocking gate reads history through, where a fetch-retry bug would make the gate ERROR-block or silently pass. Covered against the real repository rather than a fake, because the properties under test — a squash-merged commit being readable only after a fetch, and an absent file being distinguishable from an unreadable commit — are properties of git, and a fake would assert the assumption instead of testing it. * fix(audit): close a complete gate bypass and three ways the gate could not reach a verdict The append-only guard iterated only the fields an entry already had, so ADDING a field to a recorded entry was zero violations. The content check ranks a recorded sourceClosureHash above the audit commit, so writing that one field onto an entry an auditor already approved made any source pass — with every existing field untouched. Confirmed against the real 103-entry log: two additive edits took FraxFacet from fail to pass. An entry now grows only by being added, never by gaining a field. citty resolves a multi-word argument's camelCase key to its default value, so --audit-log was parsed into args['audit-log'] while the code read args.auditLog and got the default. The workflow line passing it did nothing, and a hand-run against a fixture log silently reported on the real one. Single-word arguments are unaffected, which is why --head and --base worked. Foundry resolves a non-relative, non-remapped specifier against libs, and EcoFacet imports lib/openzeppelin-contracts/.../IERC20.sol that way. The resolver had no direct-path case, so that facet reported closure-incomplete on every commit: a permanent ERROR with nothing an author could do. Direct paths under lib/, node_modules/, src/ and test/ now resolve; the fleet sweep goes from one un-gateable contract to none, with every other verdict unchanged. Comment stripping was two regexes that did not understand string literals, so a /* in one string and a */ in a later one spanned between them and dropped every import in between with nothing recorded as missing — a hash over an incomplete closure, which is the one result this module must never produce. Replaced with a single scan that locates comments and string literals together. * fix(audit): report unreadable inputs through the CLI error path, and fix a test that could not fail An unreadable contract list, an unreadable audit log and a malformed audit log all crashed with an uncategorized stack trace. They now exit through EXIT_ERROR with the path and the underlying reason, which matters because per T3 not-knowing blocks: the gate has no expectation to compare against, and that must not read as an unaudited contract. A malformed log at the PR base is likewise raised rather than returned as undefined — undefined means absent, and absent at base is a pass. The path-binding test only changed one file's content, so its hash differed for the same reason as the test above it and it would have passed against an implementation that ignored paths entirely. It now exchanges two leaves' contents so the path set and the content multiset are identical in both versions, and asserts that equality rather than claiming it. * feat(audit): hash audit-relevant source, not raw bytes (D14) `c37da94a2` normalised SPDX headers across the repo, and because the closure hash was taken over raw bytes that one comment change invalidated content-equality for every contract against every prior audit. An audit has no opinion about a licence header. The closure hash now drops comments, block-comment openers, pragma and blank lines before hashing — mirroring the filter `versionControlAndAuditCheck.yml:148-153` already applies when it decides whether a change needs a version bump, so the content check and the version-bump check cannot disagree about what an audit covers. Mirroring it faithfully means inheriting two gaps: a trailing comment on a code line stays, and a block comment's body lines stay, since neither begins with `//` or `/*`. Both are pinned by tests as deliberate. A stricter normaliser here would be a second definition of "audit-relevant", which is the failure mode worth avoiding more than the gap. The envelope carries `version: 2`: the same closure hashes differently than it did over raw bytes. No migration needed — all 103 existing audit entries predate the field. F24 stays closed: a real code change still moves the normalised hash, and that is tested directly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(audit): split own-source failure from import-closure drift (D14) Normalisation alone does not make the six named contracts green, and measuring showed why. For `ERC20Proxy@1.2.0`, four of six drifting closure files were comment-only and normalise away; `GenericErrors.sol` gained an error (39 to 40 audit-relevant lines) and `LibBytes.sol` gained two (79 to 93). That is real code in a shared library, and normalisation must not hide it. So the verdict splits. Own source differs from the audit: `fail`, blocks — this is not the audited contract. Own source audit-relevant-identical but the closure moved: `closure-drift`, reported and not blocking — a library can change what a facet does, but the contract itself is what was audited. Everything matches: `pass`. The ordering is tested directly: when both differ, the own-source failure wins. Otherwise a real edit to a facet would merge as drift, which is the F24 hole reopened by a different door. Measured against all six contracts D14 names, at their real audit commits: every one is `closure-drift`, blocks=false, and every one drifts through `src/Errors/GenericErrors.sol`. Adding a single line to their own source flips them to `fail`, blocks=true. Still to come for D14: the drift verdict needs its own non-required GitHub status check rather than living inside the required one, and the `LibAllowList` revert case needs demonstrating against its historical commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(audit): wire the D14 verdict split into the gate that actually runs classifyContentVerdict was imported nowhere but its own test, so the shipping path kept doing the single combined-closure comparison and the PR body's evidence described a module CI never called. The closure reader now returns per-file hashes alongside the combined one, which is what lets the gate tell "this contract changed" from "a library it imports changed". Own-source drift still fails; closure drift reports and returns 0. Without the split the gate blocks any contract sharing a library that moved, which on main is most of the audited set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(audit): report closure drift on its own non-required status check Completes D14. The blocking gate returns 0 on drift, so drift needs a status of its own or it is invisible; a status check is per-job, so this is a separate job rather than a step. It re-runs the gate with --fail-on-drift rather than reading a step output, so it cannot report green because an upstream step failed to set a variable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(audit): close six review findings on the content gate (EXSC-688) - The D14 own-source-vs-drift split never ran on the `recorded` path: an entry with a sourceClosureHash pushed straight to failures. Invisible while no entry carries the field, and a hard block on every contract sharing a drifted library the moment EXSC-706 starts writing it. The recorded path now takes the same split when the entry's commit supplies per-file detail, and keeps the definite failure when it does not — a mismatched recorded hash is knowledge the closure changed, not the gate failing to know. - The append-only baseline is HEAD^1, not base.sha. This workflow runs only on pull_request, so HEAD^1 IS the base tip HEAD merged; base.sha is the base as it stood when the PR opened, and an entry landing on main after that and being deleted on the branch is absent from both sides — zero violations, and an entry silently leaves recorded history. - Two throws exited 1 through runMain, delivering the gate-doesn't-know condition as a mismatch and collapsing the very distinction the exit codes exist for. Both now use the consola.error + EXIT_ERROR shape. - `--fail-on-drift` went red for fail and error too, so the drift check was red whenever the required check was, on a check whose purpose is to isolate drift. It now exits 0 on verdicts the required check owns. - A node_modules import became a submodule pointer, and `git ls-tree` on a gitignored directory answers nothing — permanent closure-incomplete with nothing the author could do. `@uniswap/=node_modules/@uniswap/` is in remappings.txt, so this was one import from live. Such imports are now pinned by their declared package.json version, and an undeclared one names the package instead of blaming node_modules. - bun install -> --frozen-lockfile in both jobs this PR adds. Hard failures now also carry the way out of the commonest cause: an audit whose recorded commit predates its own remediation. Whether the auditor must sign off on the post-remediation commit is a process question still open on the PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(audit): treat trailing code after pragma or block comments as relevant (EXSC-688) A line that only started with pragma or /* was dropped wholesale, so `pragma …; function steal()` matched a clean pragma and skipped both the version-bump list and the content hash. Document the post-remediation audit-log flow the gate cannot infer on its own. Co-authored-by: Cursor <cursoragent@cursor.com> * feat(audit): pin content gate to finalCommitHash after remediations Keep auditCommitHash as scope (A); when finalCommitHash (D) is set, CI hashes against D so report scope and deploy pin stay distinct without PDF parsing on the signer path. Co-authored-by: Cursor <cursoragent@cursor.com> * docs(natspec): require /// line NatSpec on LI.FI-owned Solidity House style already uses /// on facets; document the ban on /* block comments so audit-hash false positives from comment bodies stay unreachable. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(ci): drop the leftover auditor-handle check outside the audit loop (EXSC-688) The inner loop already fail-closes on a missing GitHub user. The second block only re-checked the last iteration and could not catch anything new. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(audit): treat per-file closure match as pass when combined hashes disagree Hasher envelope bumps can change `combined` while every file hash stays the same; derive closureMatches from per-file equality and return pass on both the recorded and commit paths. Co-authored-by: Cursor <cursoragent@cursor.com> * test(audit): give the orphaned-commit fetch 30s before timing out (EXSC-688) CI's default 5s is shorter than a cold origin fetch of a squash-merged audit SHA, which kept run-ts-tests red after the recorded-path pass fix. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
… deleting (EXSC-970) (lifinance#2364) * fix(test): withhold credentials from spawned children by setting, not deleting Two placement probes deleted `PRIVATE_KEY` and `PRIVATE_KEY_PRODUCTION` from the child environment, which does not withhold them. Both spawn with no `cwd`, so the child resolves this repo's own env file and re-loads every name the passed environment leaves unset — a deleted credential comes back at its real length. Measured before changing anything, by length alone: 64 characters when deleted, 18 when set to `malformed-in-tests`. The comments said "withheld so a child that runs past the gate cannot reach a signature", which was the opposite of what the code did. This matters most where it looks safest. The blast radius of a spawn test is the FAILING path, and mutation testing manufactures that path deliberately, so a probe whose only barrier is the refusal under test will run a real CLI with a real production key the moment that refusal is mutated away. `strict-flag-placement` spawns `deploy-safe.ts` and `execute-pending-timelock-tx.ts`; `ticket-gate-placement` has a case that deliberately reaches the branch of `unpauseAllDiamonds.ts` which sends directly on mainnet. `funnel-deploy-gate.cli.test.ts` is deliberately NOT changed, and that is the part worth reviewing. Its children spawn with `cwd` set to the `mkdtempSync` fixture repo, which has no env file to resolve, so deleting really does unset — and four of its cases assert the key-absent message that a malformed value replaces. Changing it turned those four red, which is how the distinction was found. It now carries the reason and a `spawn-env:` marker. The new source assertion is what keeps this from regressing: an unannotated delete of a credential name fails, an annotated one passes, and the marker counts only on its own line so one exemption cannot license a file. Whether a spawn's `cwd` can reach an env file is not decidable from the call site — it depends on a value computed elsewhere — so the exemption is annotated rather than inferred, which puts the burden on whoever adds the next one. Falsified in both directions, each restored byte-identical: reintroducing the delete in `strict-flag-placement` fails the guard naming that file, and stripping the marker from `funnel-deploy-gate.cli` fails it naming that one. The pattern is also asserted to match all four spellings and to leave a set, or a delete of a non-credential, alone — `.env` declares only PRIVATE_KEY, PRIVATE_KEY_PRODUCTION, MONGODB_URI and SC_MONGODB_URI, so deleting NODE_ENV or ENVIRONMENT genuinely unsets them. * fix(test): make the credential guard see a multiline delete, and itself Two defects in the guard from the previous commit, one found by review and one by looking into it. **The guard was red.** It enumerates via `git ls-files`, which lists tracked files only — so while the file was untracked the suite was green, and the moment it was committed the guard began reporting itself, because it holds the violation as test data. It now excludes its own path, by exact string rather than a pattern so the exclusion cannot widen to cover a real offender, and a test pins the exclusion at exactly one file. That the pre-commit run was green is the trap worth remembering about any `git ls-files`-driven source assertion: it cannot see itself until it is committed. **A line terminator between `delete` and its operand is legal grammar** — the spec puts no `[no LineTerminator here]` restriction there — so the per-line scan walked straight past `delete\n env.PRIVATE_KEY`. The operand is now joined onto the keyword's line before the per-line pass, which also carries a trailing marker comment with it and so keeps the exemption line-scoped. Both halves are tested: the multiline delete is reported, and the same delete with its marker is not. Falsified: removing the self-exclusion fails two tests including the offender scan, and removing the join fails the multiline case only. Both restored byte-identical. 8 tests, tsc-files and eslint clean. * fix(test): neutralize the store URIs too, and prove the replacement works (EXSC-970) The placement probes set malformed keys but still passed the real store URIs through, and `execute-pending-timelock-tx.ts` opens the timelock queue in its fleet prefetch before it reads any key — so on that path the malformed key never gets a say. Both probes now set the store URIs and the Safe signer key as well, matching the reviewed shape in a739bed. The guard enumerated six credential names; `.env` declares fourteen, including the pauser, refund and withdraw wallet keys and several retired deployer generations. It now matches on substring, so a name added later is covered without an edit. The guard was also source-only, which left the behaviour the whole fix rests on untested: nothing failed if a passed value stopped winning over bun's env-file re-load. A hermetic spawn against a fixture env file now pins both halves — that a deleted name comes back, and that a passed one wins — with no real credential taking part. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(test): correct the credential-name count in the guard's rationale (EXSC-970) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(test): share one withheld-credential list between the probes (EXSC-970) The two placement probes carried the same twenty lines — three key names, two store names, the values and the reason — and a security-relevant list kept in two places is one edit away from covering only one of them. Both now call `withholdCredentials`, which is the single place the next name gets added. Three defects in the guard, all comment-vs-code: - it claimed to match the optional-chained forms and the seed phrase, with no case asserting either; both are now in the paired-present list, and each new case was shown to fail under the matching mutation (drop `MNEMONIC`, drop `?` from the holder class); - it cited a ticket number that does not exist for widening the guard to the RPC and API-key names. Those are outside it by design, not pending, so the rationale stays and the promise goes; - the exact credential-name count had already been wrong twice, which is what a count in a comment does. Dropped. The hermetic fixture directory is now removed after the suite instead of left in tmp on every run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(test): drop two guard comments the refactor and the env store outdate (EXSC-970) One pointed at an expression the probes no longer contain — the assignment moved into `withholdCredentials`. The other asserted which names the env store declares, which is not checkable from a review and does not carry the point: what makes those unsets legitimate is that none of the names holds a credential, whatever the store declares. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(test): close four blind spots in the credential guard (EXSC-970) The guard read one line at a time and matched the raw text, which left three ways past it and one way to trip it wrongly: - two deletes sharing a line were judged together, so a marker at the end of the line carried an unjustified neighbour through - an optional-chained `delete env?.PRIVATE_KEY` matched nothing - `MNEMONIC` was not treated as a credential, though it is a signing seed the env store declares and demo scripts read - prose describing the hazard was reported as committing it, which would have made this file's own guidance unwriteable elsewhere It now reads statements rather than lines, blanks block comments, and drops line comments after the exemption marker has been read — the marker is itself a line comment, so the order matters. Each of the four is pinned by a test that was watched to fail with the fix reverted. The credential list is stated as what it covers and what it does not: the keyed RPC endpoints, provider API keys and Slack webhooks are still inherited, and widening to them is a policy call about which unsets are legitimate fallback tests, tracked as EXSC-988. `funnel-deploy-gate.cli.test.ts` withholds the Safe signer key too, which its siblings already did. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…an take a flag (EXSC-960) (lifinance#2359) * feat(safe): make the ticket channel uniform across the funnels that can take a flag (EXSC-960) `--ticket` existed on `propose-to-safe.ts` alone; the other three funnels that refuse a run for a missing ticket read only `SAFE_PROPOSAL_TICKET`. An operator who passed the flag to one of them got a refusal naming a value they could see they had supplied, which reads as the check being broken rather than as the flag being unwired. `unpauseAllDiamonds.ts`, `propose-to-safe-tron.ts` and `add-safe-owners-and-threshold.ts` now declare it and forward it to `assertTicketPresent`, which has taken the argument since lifinance#2298. No `default` on any of the three: a citty argument with one discards what the caller passed. The remaining path stays environment-only because it cannot be otherwise, which the documentation now says instead of leaving it to look like an omission. The bash `sendOrPropose` takes six positionals and forwards them wholesale, and the TS one is reached from `cleanUpProdDiamond.ts` through several positional hops, so a ticket has nowhere to travel. §7 says which of the two emergency routes offers the flag rather than "where the script offers it", since discovering that during an incident is the cost this is meant to remove. The test spawns the one funnel whose refusal is reachable without proposing, and classifies every caller of the check by the channel it accepts. The classification is a snapshot, so a funnel added later lands in neither list and is named by a case that greps for callers, rather than passing by absence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(safe): stop the spawn probes inheriting a real key and the real store (EXSC-960) The probes deleted PRIVATE_KEY, PRIVATE_KEY_PRODUCTION and the store URIs from the child environment. Bun then loads the repo `.env` inside the child for every name the passed environment leaves unset, so the delete handed all of them back - and this file spawns the one funnel whose step after the ticket check signs on every production mainnet in turn. The worktree `.env` is a symlink to the real one and holds a live production key and the live proposal store, so the only thing standing between a probe and a real proposal was the refusal the file is testing. Mutation testing manufactures exactly that failing path, and did. They are now set to malformed values rather than removed, so a child that got past the check dies before it can act, plus a breach check that treats a run reaching a real Safe or store as no result at all. This was not only a safety gap. Under the old environment the fabricated-ticket mutation escaped the suite entirely: it defeats the refusal in-process, yet all ten cases passed, so the case asserting the absent message was not observing what it claimed. With the environment isolated the same mutation fails that case, and seven of the eight mutations are now caught here rather than six. The breach predicate is keyed on this funnel's success lines because it has to be unsatisfiable by the refusal beside it - a first cut matched the word "proposal", which the refusal itself contains, and failed every case. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(safe): say which routes have no --ticket, not that they cannot have one (EXSC-960) §7 said `diamondEMERGENCYPause.sh` proposes through the bash `sendOrPropose` chokepoint. It does not - it invokes `propose-to-safe.ts` directly, and the source says so deliberately ("intentionally independent of universalCast/sendOrPropose"). The operator-facing fact is simply that the script declares no flag of its own. §4.2 called the flagless routes environment-only "by construction rather than by omission". Neither is closed to a flag: the bash chokepoint could take a seventh positional, and `cleanUpProdDiamond.ts` already threads a signing-options object down the same positional hops the TS `sendOrPropose` sits behind. Both are unwired, not impossible, and a doc that says otherwise tells the next engineer not to try. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(safe): one wording for the --ticket flag across all four funnels (EXSC-960) Two of the three new declarations said "nothing is proposed without one" where `propose-to-safe.ts` says "a proposal is not created without one". The string is `--help` output an operator compares across scripts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(safe): prove the flag reaches the check, not that a bare call is absent (EXSC-960) Mutation testing the previous version: deleting `ticket: args.ticket` from the Tron funnel's only handoff, forwarding `options.privateKey` in place of `options.ticket`, and giving the Tron declaration a fabricating `default` all left the flag inert on a production Safe funnel with all ten cases green. `not.toContain('assertTicketPresent()')` is satisfied by any argument at all, so it never saw them. Each funnel now pins the exact call the check is reached by, plus the Tron handoff, plus the absence of a `default`. `add-safe-owners-and-threshold.ts` joins `unpauseAllDiamonds.ts` as a spawned case - its check follows argument parsing and a config-only network resolution, with nothing live before it. The Tron funnel stays source-checked because its check sits past the timelock reads inside `runPropose`. The caller grep takes `--untracked`: the funnel the fail-closed case exists to name is usually one just written, and without it the case passed at exactly the moment it should fire. Scope is stated rather than implied. This file classifies the funnels carrying an entry-point check; the `script/tasks/propose*.ts` mapping scripts and `parked-tasks.ts` reach the store with no flag and no pre-check, and are refused after a signature. §4.2 now says so instead of naming two flagless routes as though they were all of them, and the rollout playbook no longer tells an agent that only `propose-to-safe.ts` has the flag. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(safe): let the breach check see the funnel that was added to it (EXSC-960) `add-safe-owners-and-threshold.ts` became a spawned probe, but the predicate guarding those probes still only knew `unpauseAllDiamonds.ts` and `propose-to-safe.ts`. A bypass on it prints "Transaction signed:" and "Transaction successfully stored in MongoDB", neither of which the predicate matched, so the one funnel where a probe signs before it stores was the one it could not report. Both lines are added; each is checked against the refusal text beside it, which is what rules out a predicate that fires on every case. The comments around the spawned block still described it as reaching one funnel end to end and the block below it as covering that funnel on its source, and `asserts` carried the caller grep with nothing saying why `propose-to-safe.ts` is exempt from it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(safe): stop §4.2 telling an operator a free refusal costs a signature (EXSC-960) The flagless routes were split into two named by name and "the rest", globbed as `script/tasks/propose*.ts` and described as refused at store time. That glob also catches `proposePeripheryWithWhitelist.ts`, which is not a mapping script and is not refused there: it drives `propose-to-safe.ts` as a child on the inherited environment, so the refusal comes out of `runPropose` before anything is signed. Read as written, §4.2 told an operator the one thing about that route they would act on — whether a missing ticket costs a Ledger tap — and told them wrong. It now names all three routes that front a funnel, and names the store-only set outright instead of globbing at it. That claim is worth exactly its accuracy, so a second fail-closed case pins it: a route into `storeTransactionInMongoDB` that is in neither list fails the run, which nothing else could catch, because calling no entry-point check is what puts a route in the expensive set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(safe): name the block for both sides it now checks (EXSC-960) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): let --ticket reach the store, not just the gate it opens (EXSC-960) The gate and the store resolve the ticket independently. `propose-to-safe.ts` passes its flag on as `provenanceOptions.ticket`; the three funnels wired here passed no provenance at all, so `storeTransactionInMongoDB` consulted only `SAFE_PROPOSAL_TICKET`. A run carrying `--ticket` and nothing exported therefore cleared its own entry check, signed — once per production mainnet on `unpauseAllDiamonds.ts` — and was refused at the insert. Worse where the variable held an older rollout's ticket: the flag won at the gate, the variable won at the store, and the proposal a signer reviewed was linked to the wrong issue with no error anywhere. That is the failure the flag exists to remove, so half-wiring it was worse than leaving the funnels environment-only. `add-safe-owners-and-threshold.ts` threads it through `IProcessNetworkDeps` because both of its store calls sit inside `processNetwork`. No test could have caught it: every spawned probe passes a ticket the parser must refuse, so none reaches a store. The store side is pinned on source instead, counted against the number of store calls so a later one cannot skip the argument. The remaining changes are gate findings on the checks themselves. The caller greps now match a call rather than the bare symbol — `parked-tasks.ts` names `storeTransactionInMongoDB` only in a JSDoc line, which classified a module that never reaches it and went red when that comment was reworded; matching `(` drops it and `proposal-intent.ts` without a hand-maintained exclusion for either. The breach predicate gains the duplicate-proposal and store-failure lines, both of which print after a signature was spent and the live collection was queried, and its summary alternative is anchored on a non-zero count so `All 0 network(s) processed successfully.` cannot report a breach on a run that proposed nothing. The `add-safe-owners-and-threshold.ts` probe passes `--ledger=false`, since no environment value blunts a hardware wallet and that funnel defaults to one. A new case holds §4.2's "and by no other route" to the four declarations, which the per-script cases could not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(safe): stop counting the flagless routes, and name the one they really take (EXSC-960) Three claims in the rewritten §4.2 did not survive checking. It said three flagless routes front the four funnels. There are at least eight — `propose-diamond-cut.ts`, `deploy-and-register-periphery.ts`, `transfer-ownership-to-timelock.ts` and five bash callers of the chokepoint on top of the three named. The text this replaced said "worth naming", which was a selection; turning that into a count made it false. It is a selection again. It said the bash chokepoint forwards its six positionals wholesale. It branches instead — `universalCast sendRaw` off production, `propose-to-safe-tron.ts` on Tron — and for EVM production assembles a named-flag command. That strengthens the point it was making, since `--ticket` would be one more element of that array rather than a seventh positional, so the point is now made from what the script does. It listed `parked-tasks.ts` among the routes refused at store time. That module names `storeTransactionInMongoDB` only in a JSDoc line and never calls it, so an operator was being told a missing ticket there costs a signature on a path that never signs. The rollout playbook had the same shape of error: Phase 2's propose-only mode reaches `propose-to-safe.ts` through `diamondUpdateFacet` / `diamondUpdatePeriphery`, not through `sendOrPropose`. The conclusion held only because those call sites pass no `--ticket` either. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(safe): qualify the two ambiguous script paths, and pin the fence lookup (EXSC-960) `propose-diamond-cut.ts` and `deploy-and-register-periphery.ts` are named bare in §4.2 while living under `shared/` and `deploy/tron/`; the claim about them was right, finding them was not. `storeFn()` had one consumer, the Tron branch of the store-count case. EXSC-984 removes the exemption that branch exists for, and a fence that stopped declaring the name would then be read by nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…EXSC-952) (lifinance#2363) * feat(codehash): grade a codehash match by who built what it matched (EXSC-952) WP-5.2 names the local rebuild as a visible fallback *state* rather than a silent substitution, and this is that grading. `compareToAttestedSet` answers whether the deployed code is a build this repo can vouch for; it does not answer who built it, and the two are different claims. An attestation minted by CI was produced on a machine the signer does not control. A local rebuild was produced by the very host that could be compromised — so a gate rendering both as one green tells the signer their own machine agreed with itself. The case that motivated the shape is neither A-CI nor A-LOCAL. When CI has attested this contract and the deployed code matches only the local rebuild, CI and this host disagree about what the source compiles to. That is not a weaker pass, so `ci-disagrees` is its own grade and, like the local match, is not presentable as attested. Only `ci-attested` is. The disagreement is asked of the whole set rather than of the matching subset, because a CI attestation that exists and *failed to match* is invisible if only the matches are examined. Reading the strongest provenance present would answer "is anything here CI-attested" instead of "was the thing that matched CI-attested" — the same shape of error as keying a lookup on a value the input could be transformed into. Case is folded before comparing, which is safe here for a reason worth stating: hex is case-insensitive by definition, so folding compares a value to itself rather than turning one value into another. Without it an uppercase attestation would fail to match and the verdict would drop to `ci-disagrees` — a false alarm about a build that agrees perfectly. A hash matching nothing grades `unattested` rather than throwing, so a caller that skipped the comparison cannot obtain a pass from this module by accident. Five decisions falsified one at a time, each failing the test that names it and each restored byte-identical: presenting a local rebuild as attested, letting the strongest provenance in the set win, collapsing the disagreement into locally-rebuilt, grading an unmatched hash as a match, and dropping the fold. * fix(codehash): compare provenance hashes in the shared canonical form The local fold lower-cased but did not strip `0x`, so a pair `compareToAttestedSet` calls a match could grade apart here. A CI-attested build stored bare against a prefixed observed hash graded `unattested`, and with a local rebuild carrying the prefixed form it graded `ci-disagrees` — an alarm that CI and the host disagree about a build CI attested exactly. `normalizeHash` from `./hex` is the canonical form the comparison itself uses, so the two cannot disagree about whether two spellings are one hash. Drops a test fully subsumed by the two either side of it, and trims the comments that recounted the PR's rationale rather than the code's invariants. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(codehash): state the provenance invariants without the past tense Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): grade the builds the comparison matched, not a re-derived set `gradeMatchProvenance` re-derived its own match on `maskedHash` alone, where `compareToAttestedSet` requires the masked hash, the deployed length and — on a pinned attestation — the exact bytes. The matched set was therefore a strict superset of the comparison's, so provenance was read off builds the comparison had rejected: a CI attestation that failed to match counted as one that matched, `ci-disagrees` could never fire, and a local-only match was presented to a signer as CI-attested. Reached on zkEVM, where `rawHash` is pinned and the zksolc fork lives only in the trailer that masking removes, so a CI mint on another fork shares the masked hash and differs in the exact bytes. The function is now handed `matched` and consults `all` only to count CI attestations that did not match. There is no second predicate left to disagree with the comparison, so the defect is unexpressible rather than fixed. `provenance` joins `IAttestedBuild` as a required field and `IProvenancedAttestation` is gone: one record now flows into both the comparison and the grading, so the two cannot be given different sets. Required rather than optional because absent and `A-LOCAL` are the same build only by coincidence. `attestationFrom` tags its rebuilds `A-LOCAL`. A matched build whose masked hash fails `digestFault` grades `malformed` and is not presentable: a malformed hash compares as readily as a real one, so without the guard the verdict rested on a comparison between two values that are not hashes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): hand over the matched builds instead of a lineage lookup Taking `matched` as a parameter moved the weak predicate to the caller rather than removing it. `ICodehashComparison` exposed the match only as `matchedLineages`, so the sole way to obtain the builds was to filter the set by those strings — and a lineage is a human label, not a key. `describeLineage` carries no provenance marker, so a CI mint and a local rebuild of one contract@version at one commit under one profile produce the same string: the filter re-admits the build the comparison rejected and the verdict is `ci-attested` on a local-only match, which is the defect the previous commit set out to close. The test there passed only because its two fixtures used hand-written distinct lineages. `compareToAttestedSet` now returns the builds it matched, so no caller has to re-derive them and identity never passes through a display string. Also pins two decisions no test reached. `attestationFrom`'s `A-LOCAL` was written in one struct literal and read nowhere in the suite, so flipping it to `A-CI` — labelling every rebuild this host produced as CI-attested — left all 3664 tests green; it is now asserted on the real producer across every active network, together with the end-to-end property that no local rebuild is presentable as attested. The malformed guard's scope was equally unpinned: widening it from the matched builds to the whole set would let one corrupt record refuse a legitimate CI match. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(codehash): annotate the provenance fixture helpers with their return type Reported by CodeRabbit on the ready flip. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
… fleet dry run (EXSC-936) (lifinance#2360) * feat(immutables): annotate 26 config-backed immutable getters after fleet dry run A fleet dry run read each getter on every active production mainnet (plus Tron for the three rows deployed there) and compared it against the config value the binding was deployed from. 26 of the 31 "config-backed and annotatable" exemptions matched everywhere they are live, so they are now annotated with `getter` in deployRequirements.json and their exemption rows are gone. ChainflipFacet and TokenWrapper needed `legacyGetters`, since most of the fleet still runs the pre-rename builds. Five rows stay exempt with reasons stating the concrete blocker, and one is a real on-chain finding (GenericSwapFacetV3 on stable binds its wrapped native instead of the configured native sentinel). Also rewords the "may only shrink" claim in 601-healthcheck-invariants.md and the mirroring docstring: verify-immutable-registry accepts a new exemption that carries a reason and fails only on an unreasoned, annotated, or dangling one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(immutables): align the exemption criterion with the exemptions actually recorded Three exempt rows do have a config value; they are exempt because the invariant cannot express a legitimately-zero expectation. The stated criterion said the opposite, and the "list may only shrink" claim survived in the gate's own error message. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(immutables): scope the blocking-ticket requirement to review, not the gate Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(immutables): name the gate, not a function, and drop a third false doc claim Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Goran Vladika <goran.vladika@gmail.com>
…ance#2366) * feat(codehash): measure the false-refusal budget in shadow mode (EXSC-977) Runs the merged enforcement-candidate gates report-only over the real fleet corpus, adjudicates every refusal into true positive / named accepted false red / unexplained, and ships the promotion criterion as code. Nothing here proposes, signs, broadcasts or writes: the runner constructs no Safe client, no Mongo connection and no RPC transport. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(codehash): reuse the production inverter, name every shrunken denominator Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): fail closed on an unmeasurable corpus, disclose the layer-2 gap Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(codehash): correct two comments in the false-refusal budget Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): print every coverage note, and say G2's unexplained zero is structural Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(codehash): disclose what each gate's zero actually measures G1 and G4 report a rate no corpus input can move, G5's zero depends on one exclusion, and G3's note asserted a checkout property it never read. Forward the derived scope to compareToAttestedSet and normalise the network key, per the seam verify-cut-targets uses and the lookup lifinance#2329 centralised. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(codehash): G4 is pinned too, and G3's refusals measure the clone The previous commit named G3, G4 and G5 as the gates whose unexplained zero is informative. G4 cannot refuse at all from repo data, and a G3 refusal is a property of the executing checkout, so G5 is the only one left. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(codehash): let each gate's note own its caveat, and stop charging the gate for the harness The header's per-gate list of which zeroes are structural was wrong three times running; the notes sit next to the numbers and are derived, so the list goes away. Encoding the probe cut outside the try keeps a base58 address the harness cannot encode out of the unexplained budget. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(codehash): no coverage note claims anything about another gate Every wrong sentence so far has been a cross-gate superlative — "only", "the one", "the fleet". Each note now describes its own gate and stops there, and G5 names the 18 live facets that would refuse it once attested. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(codehash): derive the zk exclusion, and refuse a row that reproduces nothing Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): grade commit availability against the clone, not against zero Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): correct the funnel-gate residue claim, and disclose what the criterion does not read Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): carry the provenance the corpus actually has, and say it is ungraded Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): give each fails-closed test its own corpus, and derive the re-admitted denominator Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): pin the two decisions no refusal count moves with Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): refuse a corpus whose optimizer settings the model cannot represent Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ned children (EXSC-988) (lifinance#2371) * feat(tests): widen the spawned-child credential guard past the signing keys (EXSC-988) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(tests): correct the guard's header count and the pinned-names rationale Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(tests): make the scan agree with the sweep, and guard every pin (EXSC-988) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(tests): pin the withheld-name set, not a property of its members (EXSC-988) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(tests): judge the unset name, and cover the spelling that unsets by value (EXSC-988) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ebuilding the contract (EXSC-924) (lifinance#2361) * feat(codehash): reconstruct immutables by constructor replay (EXSC-924) WP-2.1 layer 1 masks the byte ranges holding immutables, so its MATCH has to be reported as incomplete: a nonzero excludedByteCount and a reason saying the masked bytes still have to be checked. Re-running the constructor on a local EVM produces runtime code with the immutables baked in, so all of the bytes can be compared and the qualifier goes away. The expected args come from this repo's deploy-requirements registry and config/, never from a chain or an explorer. That is the whole security content. `forge verify-bytecode` sources them from the explorer record or from the tail of the onchain creation code, both of which the proposer writes, so every immutable it checks verifies against itself: on mainnet FeeCollector, whose owner() is 0xC06ebbeF...264B, running it with an owner of 0x...dEaD still reports the creation code as matched. The deployment record's own constructorArgs field is a cross-check rather than a second source, because it was written by the deploy under examination; a disagreement with config refuses instead of picking a side. Offset masking stays as the fallback. Args that cannot be derived from our own record produce UNVERIFIABLE with fallsBackToMasking set, so the caller runs the WP-2.1 path unchanged and its qualified MATCH is what a signer sees. This layer is strictly narrower than the one it fronts: masking required the code outside the immutables to match, this requires that and the immutable bytes, so nothing that failed there can pass here. Against the registry as it stands, 27 of the 47 constructor-taking contracts in src/ derive and 20 fall back. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): survive a missing anvil, and cross-check a nullary constructor Both from CodeRabbit on lifinance#2361. A missing `anvil` escaped `createLocalEvmReplay` as a throw — from the `spawn` call itself under bun, and under Node as an `error` event that terminates the process when nothing listens. Either shape takes down the signer's run over an optional replay whose entire design is to fall back to masking when it cannot decide. The failure is now recorded and reported as the replay's reason, so a caller that has no anvil gets `fallsBackToMasking` rather than an exception. Fixing it exposed a second-order defect worth more than the crash: the startup probe would have spent its whole 100-probe budget waiting for a process that will never exist. `awaitStartup` now aborts as soon as the child reports it cannot run — checked between probes, not once up front, because the report is asynchronous and can land after the first probe. The test went from timing out at 20 s to passing in 1.5 s, which is the same saving a signer would have paid. The nullary path returned before the record cross-check, so the one case where our two repo-controlled sources cannot both be right was the one case that passed: a deployment record carrying `constructorArgs` against a constructor that takes none. That is not a disagreement about a value — it says the record does not describe this build — and it now refuses with that wording. The comparison is extracted so both paths share it rather than agreeing by coincidence. Falsified, each restored byte-identical: removing the `error` listener fails the missing-binary test; skipping the nullary cross-check fails the case that names it; and treating an absent recorded value as disagreement fails a pre-existing test, which is what shows the extraction preserved the old behaviour. * fix(codehash): replay under the deployment's own chain, and refuse code that stores where it landed The replay ran on a bare anvil at chain 31337 and compared every byte, on the premise that the runtime bytes a constructor returns do not depend on the chain it ran on. SupersetFacet falsifies that: `IS_HUB = block.chainid == ARBITRUM_CHAIN_ID` is an immutable, so three bytes of the real Arbitrum deployment can never be reproduced locally. Graded against the code actually at 0x3556edeC on Arbitrum, the layer returned MISMATCH with blocksSigning set and no fallback — a hard block on an honest deployment, on the first contract the derivation covers that reads block.chainid. The chain id is now part of the replay request and the node runs as that chain, so the reconstruction reproduces what the deployment saw. Against the same Arbitrum bytecode the verdict is MATCH on all 12982 bytes outside the trailer, immutables included. The adapter refuses when the node's own eth_chainId disagrees with the request rather than trusting the caller's wiring. Chain id is the part of the deployment context we can reproduce. address(this) is not: EmergencyPauseFacet stores it, and no local deployment lands where the real one did. Code like that is indistinguishable from a tampered immutable, so before blocking a signer the layer replays a second time and reports UNVERIFIABLE with fallsBackToMasking when the two replays disagree. The second replay is spent only on the path that would otherwise block. Falsified against real artifacts and a real anvil: SupersetFacet at 42161 -> MATCH and at 31337 -> MISMATCH against live Arbitrum code; EmergencyPauseFacet -> UNVERIFIABLE naming the two differing replays. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(codehash): name the second replay and the adapter coverage the suite actually has Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(codehash): state what the suite covers without pointing at the PR Test-file headers referred readers to the PR description for the end-to-end evidence, and the nullary-constructor comment narrated a bug from development rather than the invariant it now pins. * fix(codehash): reuse the shared hex check, and name anvil's flag and gas cap correctly composeCreationCode re-derived the framing checks that hex.ts exists to hold; the module already imported strip0x from it. The gas comment claimed 30M was below a limit it is exactly equal to, and --silent is not a flag anvil --help documents. * fix(codehash): drop the replay gas under anvil's cap, and stop the fixture claiming a registry entry it does not match The fixture's arg names are the artifact's, not deployRequirements.json's -- AcrossFacet's real entry annotates _wrappedNativeAddress against a _wrappedNative constructor arg, so the entry the header claimed to mirror is one that refuses. Replay gas sat exactly at anvil's 30M block limit; the largest artifact in src/ deploys for 4.9M. * docs(codehash): state the startup budget the two probe constants encode Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): rotate the replay sender, so msg.sender joins the class the second replay catches Both replays deployed from accounts[0], so an immutable derived from msg.sender was identical in the two and the guard could not see it — the address landed on and the block differ between calls, the sender did not. Nothing in src/ stores msg.sender today; SupersetFacet.IS_HUB and EmergencyPauseFacet's address(this) are the only context reads. Falsified with a constructor that stores CALLER and returns it as its runtime code: two replays now differ, where before the rotation they did not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): derive the lineage from the attested set, and make the refusal unrepresentable as a verdict Two fail-open shapes in the layer's own interface, both raised by @gvladika. A MATCH here tells the caller to skip masking, but nothing tied `creationCode` to anything this repo attested: `matchedLineages` echoed a caller-supplied `lineage` label. Creation code from an unattested build would have matched with layer 1 never running. The attested set is now an input, the replayed code has to normalise into it before any verdict is reached, and the lineage reported is that build's. Normalisation goes through `normalizeRuntimeCode`, the same function both sides of layer 1 use, so the two cannot come to normalise differently. `unverifiable()` returned an ICodehashComparison carrying `blocksSigning: true` that was only safe to read alongside `fallsBackToMasking`. A caller reading the flag alone would block the signer on every refusal, which is the common case. The result is now a discriminated union: `decided: false` carries a reason and no comparison at all, so there is no verdict to misread. Falsified against real artifacts and live Arbitrum bytecode: SupersetFacet still MATCHes with `matchedLineages` taken from the attestation, while the same inputs attested by EmergencyPauseFacet's build, and an empty attested set, both refuse. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * test(codehash): annotate the two test helpers' return types Both relied on inference, which CodeRabbit flagged and which let the ExpectedArgs narrowing in the callers pass vacuously. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): stop the attested-set check claiming a property it does not hold, and test it at production shape The precondition normalises the replayed *runtime* code into the attested set, and normalisation masks the immutables. A constructor is not part of runtime code, so a creation code that hardcodes a different immutable normalises into the set exactly as the attested one does. Compiling SupersetFacet with POOL_MANAGER set to a literal 0xdEad gives the same maskedHash and rawByteLength as the honest build (0x5a36b5b5...ced35, 13035 bytes) and the layer returns MATCH, excludedByteCount 0, blocksSigning false, reporting the honest lineage. Closing it needs a creation-code hash on IAttestedBuild; IRebuiltArtifact carries no creation bytecode, so that belongs to the attestation producer and not here. The docs now say what the check binds — the runtime bytes outside the immutables — instead of what it does not. Every fixture passed immutableReferences: undefined, under which the check is exact-body equality and the three precondition tests pass for the wrong reason. They now run with ranges covering the spliced word, which is the shape every artifact with immutables has. One test names the gap directly so it cannot be closed silently, and two more cover a body that differs outside the masked word and ranges that do not fit the code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(codehash): price the masked immutables per slot instead of rebuilding the contract (EXSC-924) Raised by @gvladika: most of the replay layer served the mechanism rather than the question. Reading each immutable out of the deployed code and comparing it against the value the registry declares needs no local node, no chain-id pin, no second replay and no `address(this)` refusal — and no constructor ABI, argument order or argument encoding either, so the `address`-only type restriction and the `bytes32` gap it created both disappear rather than being extended. The link that made this possible already existed. EXSC-869 shipped `immutableRegistry.json` — immutable name to `configData` label, schema-checked and InfoSec-gated — and `immutable-ast.ts`, which enumerates every immutable from the compiler AST. Only the AST node's own `id` was missing, which is the key Foundry's `immutableReferences` uses. Both must come from one compilation, since an AST id means nothing outside the one that assigned it, so an id with no declaration refuses rather than pricing the ids that happened to line up. Coverage goes up, not down. Replay was all-or-nothing per contract and refused wherever a constructor read its context or called out; per-slot answers each slot on its own: - `LidoWrapper` (3 deployments): all three args derive from config and the replay reverts — its constructor approves wstETH, which has no code on a bare anvil. Graded against its live optimism bytecode, per-slot verifies both immutables and retires all 288 masked bytes. - `SupersetFacet` on arbitrum: `POOL_MANAGER` verified over 256 bytes, `IS_HUB` reported as `derived` with the rule the registry states and its 96 bytes left unaccounted for. Unpriced, never passed. - `EmergencyPauseFacet` (71 deployments) stores `address(this)`, which replay refuses by design and a rule can answer with the address being graded. The layer returns no `ICodehashComparison`. Layer 1 decides whether the code is ours and this prices the bytes it had to mask; the two verdicts in one result were what let a caller read `blocksSigning` without `fallsBackToMasking`. Falsified against live bytecode on two chains, honest and tampered, plus a malformed observation: a value that is not one slot wide refuses, because padding an expectation to the value's own length would compare against the wrong width. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(codehash): stop a Tron config value reading as a mismatch, and close the byte accounting (EXSC-924) Four findings from the review gate on the previous commit. A config value that is not hex was padded and compared: `networks.json` gives `.tron.wrappedNativeAddress` in base58, which is 34 characters and so passed the length-parity and fit checks, producing an expectation no slot can hold and grading a correct deployment `disagrees` — a block, on every Tron chain, with a garbage expectation in the message. An expectation that cannot be expressed in the slot's encoding is not a mismatch, so it is now `unpriceable`. Reached through `frameFault`, the check the other codehash modules already use. `resolveExpectedAddress` reads `keyInConfigFile.startsWith` before testing it, so a `configData` entry missing that field threw out of `priceImmutables` and took the whole contract's grading with it — against a module whose contract is that each slot is answered on its own. `validateImmutableRegistry` does not cover it either: it checks that the label exists, never the shape of the entry it names. Both fields are now checked, and a malformed entry costs one slot. A disagreeing slot's bytes were in neither `pricedByteCount` nor `unpricedByteCount`, so a tampered 64-byte slot beside a verified 32-byte one reported `priced 32, unpriced 0` while layer 1 had masked 96 — and the `unpricedByteCount` doc read as all-clear. `disagreeingByteCount` is now its own counter and the three sum to the masked total. The registry lookup went through bracket access, so an immutable named `toString` would have found a function on the prototype and routed into the declared paths. `registry-schema.ts` guards the same lookup for the same reason. Tests: the fixture's `configData` label was `_wrappedNative` while the shipped `deployRequirements.json` uses `_wrappedNativeAddress`, so the suite never exercised the one entry `immutableRegistry.json` authors. Aligned, and a new test resolves the real registry against the real requirements and config so a label that stops resolving cannot pass. Also asserts the three byte counters sum to what was masked. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Goran Vladika <goran.vladika@gmail.com>
…ifinance#2367) * docs(safe): fallback runbook for the pinned escape commit (EXSC-976) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(safe): the fallback freezes config, and sign-time gates need the signers too (EXSC-976) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(safe): deployments/ is frozen too, and an export beats .env (EXSC-976) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(safe): the gate-layer diff is a candidate list, not an answer (EXSC-976) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(safe): verify the tag, bound the reconcile query, state what survives (EXSC-976) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(safe): make the tag check gate, scope the config diff, date the pinned facts (EXSC-976) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(safe): force-refresh the tag, follow gate call sites transitively (EXSC-976) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(safe): correct the gate inventory, ticket binding, and refusal horizon (EXSC-976) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(safe): list the gates that actually refuse, and fail the config check closed (EXSC-976) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(safe): make the fallback pre-flight checks refuse rather than warn (EXSC-976) Closes the QA round's three findings, all the same shape: the runbook stated a stop condition in prose and then showed a script that fell through to the next step regardless. - §5 now gates the proposal on $DRIFT, defaulting to 1 so pasting the command without having run the check refuses. Two statements rather than `exit 1`, which would close the operator's interactive shell and the tunnel with it. - §5 compares `status`, `safeAddress` and `LiFiTimelockController` as fields rather than diffing the whole network entry. The entry differs on every network at every commit (the baseline predates the `targetEvmVersion` rename), so the wholesale diff refused 100% of honest runs — verified against a real baseline worktree: arbitrum/mainnet/base pass, botanix/tronshasta (retired), injective (absent at baseline) and a typo'd name all stop. - §3 tracks the export-beats-dotenv conflict in $ENVCONFLICT and stops on it. Written out per name rather than looped: `[ -n "${VAR:-}" ]` tests presence without expanding the value, which an indirect lookup cannot do. - §4 states that the baseline never reads `status`, so a retired network and a live one take the identical path with nothing printed either way. - §7 gains the nonce-collision recovery: a collation-aware query to tell a real intent-hash duplicate from the misreported nonce case, and the finding that the baseline already refuses correctly on the `--nonce` branch, which the §5 command never takes. Re-running with an explicit `--nonce` is the recovery. - §2 resolves the three Ledger packages up front, using `node` and not `bun` — bun auto-installs, so `bun -e "require.resolve(...)"` reported success for a nonexistent package and for an absent node_modules alike. - §7's reconciliation filter gains the upper bound it described but did not set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(safe): bracket the jq lookups and gate the proposal on $NET (EXSC-976) CodeRabbit round on b902b2a. `jq ".$NET"` is a syntax error for a network name that is not a bare jq identifier, and `0g` is one — so §5 refused it rather than checking it. The lookups now use `--arg net "$NET"` with `.[$net]`. Verified: 0g passes, and botanix, tronshasta, injective and a typo'd name all still stop. The proposal command took `--network <network>` as a placeholder while the checks above had validated $NET, so a hand-typed name would have proposed against a network nothing verified — a green gate holding over the wrong target. It now passes "$NET". The gate also folds in §3: ENVCONFLICT and DRIFT both default to the refusing value, so pasting the command without having run either check refuses. The Ledger leg is commented out rather than mandatory, since LEDGEROK only bears on the --ledger path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(safe): gate the escape worktree setup and make the Ledger check load-bearing (EXSC-976) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(safe): bind the fallback gates to the tree and the signer they claim to check (EXSC-976) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(safe): check what reuse can inherit, and make the declared signer the one passed (EXSC-976) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(safe): leave SIGNER unset so the refusal is reachable, and exempt the lockfile (EXSC-976) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(safe): bind the fallback gates to the escape path, not only its commit (EXSC-976) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…pen holes closed (EXSC-951, EXSC-949) (lifinance#2362) * feat(safe): close the remaining proposer-controlled routes into the signer's terminal (EXSC-951, EXSC-949) Re-lands lifinance#2349. That PR reads MERGED and none of its code is on `main`: it merged into the stack base `feat/exsc-937-refuse-delegatecall-proposals` at 09:40, was reverted on that branch twelve minutes later in 74b8f40 with no stated reason, and lifinance#2334 then squash-merged the branch to `main` at 10:38 without it. So two packages whose tickets read Done, and whose PR reads merged, had no presence on `main` at all — `printable-field.ts` included. This is the inverse of that revert, applied to `main` with no conflicts: the four shared files were byte-identical between `main` and the branch's post-revert state, so nothing has moved under it. Nothing is re-decided here; the content is what lifinance#2349 carried. The load-bearing piece is the deployments lookup keying on the *stored* periphery name while printing the sanitised one — sanitising can turn a name the record does not hold into one it does, since a zero-width space inside it is simply removed, and the check would then print a ✅ about a value the calldata does not contain. The five suites pass unchanged (322 tests), tsc-files and eslint are clean, and markdownlint reports zero on both prose files. * docs(rules): resolve rule 201's source links from the rule's own directory CodeRabbit on lifinance#2362. The links were written repo-root-relative, so from `.agents/rules/` they resolved to `.agents/rules/script/...` and navigated nowhere. All five in the file, not only the one flagged: four predate this branch and are the same defect in the same paragraph list, so fixing one and leaving the rest would make the correct form look like the exception. Each target is now checked to exist from the rule file's own directory. `../../docs/...` links in the same tree already used this form. * refactor(safe): disclose a repair inside a tuple argument, and fix three review findings - formatDecodedArg dropped every notice for object/tuple args, so a hostile or over-long string nested in a tuple was cleaned silently. renderScalarArg now returns the notice and one disclosure lands after the JSON, never inside it. Removes the double sanitise on the top-level path too. - decodeTransactionData's catch passed the Error itself to printableField, which reports every Error as "stored as an object, not a string". - describeThrown discarded the notice, so a clipped error message said nothing. - Move the renderScalarArg docblock off HEX_PAYLOAD, which had two. - Rule 201 named printable-field as universal while render-check-ledger is still on the bare sanitiser (EXSC-986). * docs(rules): stop rule 201 claiming one module is the only bare-sanitiser holdout provenance-display.ts cleans proposer-supplied fields with the bare helper too, so naming render-check-ledger as the sole exception made the rule assert something a grep disproves. * fix(safe): show a non-hex calldata value as stored on the Ledger panel Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(safe): trim the module header, and pin the tuple-clip bound by value The header ran well past rule 200's one-to-three-sentence cap; the properties it listed are documented at each constant and in MultisigSigningProcess.md. The tuple-clip test built both its input and its expectation from MAX_FIELD_CHARS, so it stayed green for any value of the constant. * fix(safe): bound every decoded argument, not only the ones that look like labels scalarBound decided the bound from the value's shape, so a `string` ABI argument that happened to be 0x-prefixed hex rendered unbounded. It cannot tell a bytes payload from a string that looks like one, and the shape test was reachable through real functions: getPeripheryContract(string), validateCalldata's bridge param, and any hex-shaped string nested in a struct argument. Nothing is lost by bounding it. The bytes under the signature print unclipped as the Data: field, so a clipped argument is still readable in full one screen up. The suite could not have caught this: every crafted long value in it was 'x'.repeat(n), which never matches the hex shape. * refactor(safe): state the bound's reason without recounting the one it replaced * test(safe): assert the guards a mutation pass could remove unnoticed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(rules): name every prompt-printer still on the bare sanitiser Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): assert the decoded 4byte name, and stop two docs overclaiming Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): bound a decoded array, and gate the target link on address-ness Three gaps the review gate found, each reachable from a stored row with no escape sequence in it. `formatDecodedArg` bounded every element and nothing else, so a 20,000-element array rendered 900,001 characters with no notice: the element count is encoded in the calldata, and with each element short no per-element notice fired either. The rendered aggregate now carries its own bound. `toLine` gated the target's name and explorer link on `identityPreserved`, which asks whether sanitising changed the text, not whether the value was ever an address. Neither callback refuses a non-address — the formatter passes an unrecognised shape through and the explorer builder interpolates what it is given — so a plain-ASCII non-address earned a real-looking explorer URL. It is now shown as stored, with a notice and no link, the way the sibling guard in `formatTimelockScheduleBatch` already treated `target=`. Rule 201 justified deferring three modules on their values being machine-reported. Only `render-check-ledger.ts`'s are: `delegatecall-gate.ts` documents its operation value as proposer-controlled, and the provenance block renders stored row text. What those two lack is the disclosure. Both guards are asserted by executing the real render path, and each dies when mutated back to the previous behaviour. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): disclose a non-address target, and sanitise two remote values The address check reached only values that were strings: a row whose `to` is absent or not a string renders a target with no name, no link and no explanation of why. The deployment cache is refreshed from MongoDB, so a facet version is remote row text, and a provider error message reaches the prompt before the operator decides. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): ask the network what an address is, and keep notices distinct The `To:` address check used viem's `isAddress`, but the signing path resolves the field through `normalizeAddressForNetwork`, which accepts base58 on Tron. A Tron row storing base58 was therefore signable and signed over the normalised hex, while the line above the sign prompt called it "not a valid address" and dropped both the target name and the explorer link. The check now makes the same call the signing path already made. `renderScalarArg`'s docblock justified its bound by the unclipped `Data:` field, which exists on the `confirm-safe-tx.ts` path and not on the `execute-pending-timelock-tx.ts` one, where rule 201 forbids it. The bound is unchanged; the docblock now names the path it reasons about and records the gap. Three lines painted a value in the notice's own colour, or wrapped a value containing notices so that a notice's closing reset ended the wrapper mid-line. `colorAroundNotices` re-opens the value's colour after each reset and takes the colour as a closed set that excludes yellow. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): state what the address check actually accepts `isAddressForNetwork`'s docblock said "checksummed hex", but the call it wraps resolves a mixed-case address by recomputing its checksum rather than verifying it, so any hex of the right length is accepted on a non-Tron network. The docblock now says shape and says so explicitly, because a reader who took it for a corruption check would be relying on a guarantee the function does not give. The `network` field's docblock claimed the value never comes from the stored row. It does whenever the operator passes no `--network`: the network list is then `distinct('network')` over the pending rows. What actually holds is that both routes filter to an active `networks.json` key, which is what the field's one use here needs. Two test comments reasoned about what the check used to do rather than about what has to stay true of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…SC-993) (lifinance#2369) * feat(safe): make the check ledger the sign-time composition point (EXSC-698) Wires the first gate verdict onto the ledger and renders it in the confirmation CLI. Before this, recordCheck and renderCheckLedger had no production caller, so summariseLedger's grading rules never ran. * fix(safe): anchor each ledger row on the evidence that decided it (EXSC-993) Three statuses reached the ledger on `A-MAIN` while resting on the deployment record the proposer writes, which routed them around the reporting-only coercion. They now ask a human on `A-MONGO`. `recordCheck` also moves from once per transaction to one worst-of row per network, and a network the run skipped no longer rolls up as missing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): split "nothing to grade" from "could not grade" (EXSC-993) A failed Safe ownership read was recorded as a pass, so a run that examined two networks of three reported 3/3 verified and never printed the reason — green check rows are not expanded by the renderer. It is now an unverified row on A-UNRESOLVED, while the outcomes that genuinely answered stay a pass. The placement test now pins the status each recorder writes, bounds the loop window by brace depth, and matches the render guard by shape; the registry fixtures carry the field shape the emitter really produces. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): grade the row's detail on the finding that decided it (EXSC-993) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(safe): compose the check ledger without rendering it (EXSC-993) Only a `pass` counts as verified, and every status covering a real Add/Replace cut is `needs-ack` whose acknowledgement lands in a separate ledger. A correct rollout therefore closed `0/N verified` while a run that graded nothing closed green. The recording is sound; the verdict is not, so the verdict is withheld until EXSC-994 settles it. Also from review: `expected` no longer claims a comparison against origin/main on rows whose branch never made one, and the comments that recounted how each bug was found are trimmed to the invariant they protect. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(safe): stop describing a check-ledger report the run no longer prints (EXSC-993) The gate table said the target-state verdict was "rendered at the end of the run", which the previous commit stopped doing — the row the doc points a signer at does not exist. Same drift in four comments that survived the drop: a placement-test docblock still scoped to rendering, two comments quoting verdict strings nothing emits, and a registry test comment calling the renderer "the surface the signer actually reads". The note explaining why the ledger is composed and never read moves to the declaration, where a reader is placed to mistake it for dead code; where the render used to be it explained nothing that is still there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(safe): say what each row was actually judged against (EXSC-993) `expected` was derived from the row's anchor, but an anchor records where the evidence came from, not whether a version comparison happened. Four statuses reach `compareSemanticVersions` and nine can decide a row, so keying on `A-LOCAL` corrected the two passing rows and left seven claiming an ordering against origin/main that their branch never established — including `not-previously-targeted`, which covers every first deployment and every new network for a contract already live elsewhere, since the declaration is per network. Carried per status through the same worst-first reduction as `anchor` and `detail`. Each sentence states a requirement rather than describing the row: the three statuses that may proceed share one, so the rank they tie on cannot make the row's truth depend on which element calldata listed first — an Add of a new facet beside a Replace of a live one is the ordinary cut shape. A sentence that diagnosed instead ("a first deployment") would also be wrong wherever origin/main declares nothing for another reason, such as a facet deprecation that removed the entry. Pinned over all thirteen statuses against the module's own constants, so a status added later is a compile error in both the mapping and the table. Also pins that the ledger stays unrendered. Restoring the call site was one line that left the whole suite green, and `summariseLedger` is pinned beside the renderer because the inversion lives there — the render is a thin wrapper, and this file already imports from that module. EXSC-994 deletes this assertion in the commit that lands the corrected verdict. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ce#2373) Mark the five facets where the bridge-specific payload is quoted against the pre-swap minAmount while the amount forwarded is the realized swap output. Each comment states the consequence for that facet and that the behavior is accepted for now. Consequence differs per facet, so the wording does too: - Symbiosis (MetaRouter), Squid (CallBridge/CallBridgeCall): the surplus is left in the external router, unrefunded. - DeBridgeDln: takeAmount is a payout instruction, so the surplus is captured by the solver. - StargateV2, ThorSwap: the full realized amount is forwarded; only the minimum-output guarantee is looser than quoted. No funds are stranded. Comments only - no behavioral, ABI or storage change, so no version bump. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e binding check (EXSC-967) (lifinance#2375) * fix(healthcheck): honour allowToDeployWithZeroAddress in the immutable binding check (EXSC-967) `immutable-bindings-match-config` errored on any zero read and warned on any absent config value, so a binding whose zero is declared could only be left unchecked. The registry flag now decides: with `allowToDeployWithZeroAddress: "true"` and a config file that loaded, an explicit zero and a sparse map's omission both assert the getter reads address(0), and a non-zero binding there is an error. An unreadable config file still warns. Annotates the two AcrossV4SwapFacet sponsored-periphery getters and drops their exemption rows. GenericSwapFacetV3.NATIVE_ADDRESS stays exempt on EXSC-966 alone: stable binds its wrapped native, so annotating it would report a live deployment red before that is settled. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(healthcheck): a non-object config file is not a zero expectation (EXSC-967) `configFileLoaded` accepted any JSON that parsed, so a config file corrupted into a scalar or a list resolved every key to null and the declared-zero branch read that as "the value is zero" — a fleet-wide assertion from an unreadable file. It now requires an object, and the invariant-level test runs from a directory with no `config/`, which fails if the guard is dropped from the decision site. Corrects the justification comment: DeployAcrossV4SwapFacet reads its zero-allowed args with the strict overload, which reverts on a missing key, so `_getOptionalConfigContractAddress` does not explain all five annotated rows. What holds for every one of them is that no deploy can produce a non-zero binding from a key config does not carry. The error message and rule text follow, and 601 now states that such a key is load-bearing for the healthcheck too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(healthcheck): do not resolve an expectation from a config file that did not load (EXSC-967) A numeric path segment indexes a list, so a config file that parsed to an array could still answer a registry key — pairing an address the caller is told not to trust with a `configFileLoaded: false` saying the file is unusable. The collector now resolves against the config only when it is a plain object, so the two always agree. No registry key is numeric today, so nothing changes for the fleet; the state was reachable in principle and is now not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(config): stable's native token is USDT0, not the sunset gUSDT predeploy (EXSC-966) (lifinance#2376) Stable v1.2.0 made USDT0 the native gas token and sunset gUSDT. The chain's docs state the native token identifier is 0x779Ded…3736 and that 0x0000…1000 "isn't used after the upgrade"; the LI.FI backend agrees (/v1/chains nativeToken.address) and deny-lists 0x0000…1000 outright. config/networks.json still carried the pre-upgrade predeploy, which made the live GenericSwapFacetV3 on stable look like it bound the wrong address. It does not — 0x4531eB…15C0 binds 0x779Ded…3736, which is correct. Config was the stale record. With the mismatch resolved, NATIVE_ADDRESS can be annotated and its getter-exemption removed, so immutable-bindings-match-config starts checking the 50 chains whose GenericSwapFacetV3 exposes the getter. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
gvladika
approved these changes
Sep 14, 2026
gvladika
marked this pull request as ready for review
September 14, 2026 09:12
Collaborator
🤖 GitHub Action: Security Alerts Review 🔍✅ No unresolved security alerts! 🎉 |
gvladika
force-pushed
the
sync/upstream-2026-09-14-21
branch
from
September 14, 2026 09:31
0f97ce1 to
c97e841
Compare
gvladika
enabled auto-merge
September 14, 2026 09:34
gvladika
disabled auto-merge
September 14, 2026 09:38
gvladika
enabled auto-merge
September 14, 2026 10:04
melianessa
approved these changes
Sep 14, 2026
3 of 8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Automated sync from
lifinance/contracts@ba03e2bdhit merge conflicts. Conflict markers are committed as-is on this branch - resolve them manually. See the Notion 'Tron deployments' page (section: The contracts-tron Fork) and EXSC-587 for the sync/versioning rules.