Skip to content

Sparrow dom/foundry updates - #2957

Open
sparrowDom wants to merge 10 commits into
masterfrom
sparrowDom/foundryUpdates
Open

Sparrow dom/foundry updates#2957
sparrowDom wants to merge 10 commits into
masterfrom
sparrowDom/foundryUpdates

Conversation

@sparrowDom

@sparrowDom sparrowDom commented Aug 1, 2026

Copy link
Copy Markdown
Member

Overview

Foundry does the deploying now, but the Talos crons and the Hardhat task CLI still resolve contracts from hardhat format deployment artifactsdeployments/<network>/<Name>.json, and Foundry writes only addresses, to build/deployments-<chainId>.json — so a redeploy silently left those descriptors pointing at the previous contract.

After each broadcast a script now rewrites them in place, keyed off contractName from run-latest.json, which turns out to be the descriptor filename verbatim. Nothing downstream changes.

The hardhat descriptor files now contain:

  • address (like they used to, but now foundry deploys update id)
  • abi (like they used to, but now foundry deploys update id)
  • storage layout. -> this used to be a separate file but now it is here

The storage layout checks now run on make simulate and make deploy

This PR now also deletes the:

  • storageLayout/
  • and the orphaned solcInputs/

This removed (~139 MB) of data.

Lazy load talos client:
Talos client in signers now lazy loads -> this should trigger only in Talos environment

Code Change Checklist

To be completed before internal review begins:

  • The contract code is complete
  • Executable deployment file
  • Fork tests that test after the deployment file runs
  • Unit tests *if needed
  • The owner has done a full checklist review of the code + tests

sparrowDom and others added 5 commits July 30, 2026 18:31
Three contracts used non-standard gap names (______gap, _____gap, ___gap)
that scripts/check-storage-layout.js did not recognise as gaps, so a
legitimate gap shrink in them would have been reported as a removed
variable. Initializable is inherited widely, so most upgradeable layouts
were affected.

Renaming is storage-neutral -- verified with forge inspect before/after
across OUSD, OETHVault, VaultCore, WOETH, OETHHarvesterSimple and
OUSDResolutionUpgrade: slot, offset, type and the types dictionary are
identical, only labels change. OZ assertStorageUpgradeSafe does flag a
rename, so the committed layout snapshots are relabelled in the same
change; without that the next deploy of anything inheriting Initializable
would fail the upgrade gate.

OUSD keeps its legacy _gap: it declares both _gap and __gap in the same
contract, so they cannot share a name.
The package is an optional peer dependency so CI and external contributors
can install without GitHub Packages auth, but tasks/lib/network.ts imported
it at module scope. That pulled it into hardhat.config.js's load path via
tasks.js -> utils/morpho.js -> utils/resolvers.js, so any `npx hardhat`
command failed with "Cannot find module" in a fresh checkout -- and the
abi.yml release workflow, which installs without auth and then runs
`hardhat deploy`, would fail on the next v* tag.

Move the import inside rpcUrlFor(), the only place it is used, so importing
the module no longer requires the package. Verified: with the package
removed, `hardhat --help` still registers every task and `hardhat compile`
succeeds; with it present, chain and RPC resolution are unchanged and the
action catalog still dumps 43 actions.

Add `pnpm install:talos` for the case where it is genuinely needed (running
actions, KMS signer). It sources a token from the gh CLI, checks the token
can actually read the package -- `gh auth login` does not request
read:packages -- installs via a throwaway npm config, then restores
package.json and pnpm-lock.yaml so the package is never committed as a
dependency, which would break CI.
…t tasks and talos task code can remain unchanged
Replaces the Hardhat-based upgrade gate, which is broken on this branch
(cache/validations.json is stale and VaultCore is now abstract).

The baseline moves into the deployment descriptor, so it is written by the
same step that records the address and the write side cannot silently stop.
The check runs inside _recordDeployment, which is mandatory, and takes the
contract name as type(X).name so a rename cannot point it at the wrong
baseline. A revert there still aborts before broadcast — forge runs the whole
script against the fork before submitting — verified against anvil: no
transactions, no block mined, ledger untouched.

Comparison is @openzeppelin/upgrades-core, which already implements the
__gap shrink arithmetic. Two policies are ours: a rename is ignored only when
the new label derives from the old (assets -> _deprecated_assets), since two
same-typed variables trading labels also reports as two renames; and enum
member data, absent from solc layouts, is ignored — an enum is one byte
regardless of variant count, so no slot moves.

Gap labels are normalised to __gap in memory because OZ's isGap() matches
only __gap/__gap_*, while contracts deployed before the rename still carry
______gap on chain. Stored layouts stay faithful to their source.

Scope is mainnet and Base. foundry.toml gains extra_output = ["storageLayout"]
(extra_output_files alone emitted nothing, which also broke forge inspect).

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GueER8JLB5U4MBLEbFERAK
…y trees

Descriptors become the single record of a deployed contract:
{address, abi, storageLayout}. 103 layouts were read from each deployed
implementation's verified source on chain via cast storage (71 mainnet,
32 Base); the remaining 61 came from the legacy Hardhat snapshots, used only
where the chain fetch could not run — contracts compiled with solc 0.5.x that
the current toolchain cannot rebuild, and stateless contracts whose layout is
empty anyway. 16 descriptors get no layout: 12 proxies, which declare no
storage, and 4 deprecated contracts whose source has left the repo.

Layouts are stored faithfully, exactly as their source produced them, so a bug
in the in-memory gap normalisation never costs a re-fetch. Note this means
some baselines legitimately carry ______gap, which is what is deployed.

Deletes storageLayout/ (216 files) — baselines now live in descriptors, and
the name-keyed snapshots had already rotted into fossils for VaultCore and
VaultAdmin, both abstract and undeployable since #2714. Re-supporting a chain
means re-fetching its layouts; see CLAUDE.md.

Deletes deployments/*/solcInputs/ (217 files, ~139 MB) — only hardhat-deploy's
verification path read them, and verification does not run through Hardhat.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GueER8JLB5U4MBLEbFERAK
sparrowDom and others added 5 commits August 3, 2026 11:25
Conflict in tasks/lib/network.ts resolved in favour of master. #2954 removed
the @oplabs/talos-client dependency from that module outright, replacing
resolveChain/getRpcEnvVar with local CHAIN_IDS/RPC_ENV_VARS maps. This branch
had only deferred the import to call time (4a2a63b), which master's approach
supersedes — there is now no dependency left to defer.

scripts/install-talos-client.sh and the install:talos script stay: signer.ts
and runner.ts still import the package for the KMS signer and nonce queue.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GueER8JLB5U4MBLEbFERAK
network.ts no longer imports @oplabs/talos-client at all — #2954 replaced
resolveChain/getRpcEnvVar with local maps — so the paragraph describing a lazy
require inside rpcUrlFor() was describing code that no longer exists. Only
signer.ts and runner.ts still need the package.

Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GueER8JLB5U4MBLEbFERAK
tasks/lib/signer.ts held the last local requirement: it imported the
package at module scope, and tasks/run.ts imports getSigner at the top
level, so every local action run needed it. Require it lazily instead —
only the Postgres nonce queue uses it, and that is already gated on
DATABASE_URL, which local runs do not set. Same shape as arm-oeth's
src/js/utils/signers.ts.

That makes pnpm install:talos and scripts/install-talos-client.sh dead.
The runner image still installs the package via dockerfile-actions, so
peerDependencies stays.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant