chore: counterfactual chain agnostic deploy#1469
Open
tbwebb22 wants to merge 68 commits into
Open
Conversation
* feat: vanilla CCTP counterfactual deposit route
Adds `CounterfactualDepositVanillaCCTP`, a per-bridge implementation for the
upgradeable counterfactual system that bridges via Circle CCTP v2 directly
(`ITokenMessengerV2`) — no Across periphery — so USDC mints natively on the
destination. One branch on `hookData`:
- empty -> `depositForBurn` (plain CCTPv2; fast or standard via
`minFinalityThreshold` + `cctpMaxFeeBps`)
- present -> `depositForBurnWithHook` (HyperCore: burn to HyperEVM domain 19 with
`mintRecipient` = Circle's CctpForwarder and
an opaque hook envelope, routed on to
HyperCore via CoreDepositWallet)
`mintRecipient`/`destinationCaller`/`hookData` are opaque route-leaf params, so
the contract carries no HyperCore-specific logic. With no periphery quote sig,
the EIP-712 fee signature binds the route + amount + fee directly (mirroring the
SpokePool impl); replay protection is the short `signatureDeadline`.
Adds Foundry coverage (plain CCTP, HyperCore hook, standard transfer, fee/sig/
expiry/source-chain/cross-proxy/amount/route-binding cases) and documents the
route + HyperCore wiring in DESIGN.md.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor: drop unused sourceDomain from CounterfactualDepositVanillaCCTP
Per review (tbwebb22): the vanilla path calls ITokenMessengerV2.depositForBurn
directly, which infers the source domain from the deploy chain, so the
`sourceDomain` immutable is never read. Remove it and its constructor param.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Move every chain-specific value (bridge endpoints, CCTP domain / OFT EID, fee signer, token addresses) out of the leaf-implementation immutables and the merkle leaf params, and onto the per-chain CounterfactualBeacon as public immutables. Leaf implementations resolve the beacon from the proxy's ERC-1967 beacon slot (CounterfactualImplementationBase) and read what they need at runtime; the input token is fixed per implementation. Result: leaf implementations are byte-identical across chains (one CREATE2 address everywhere) and a single leaf is valid on every chain, so initialRoot carries one leaf per route instead of one per source chain. Drops sourceChainId and the SourceChainMismatch check; an unconfigured route reverts RouteNotConfigured. - CounterfactualBeacon: add immutable chain config (CounterfactualChainConfig) with named getters (signer, spokePool, wrappedNativeToken, cctp*, oft*, usdc, usdt); implementation/upgradeRoot stay mutable. Config changes are UUPS upgrades. - CounterfactualBeaconBootstrap: chain-identical no-arg UUPS impl so the beacon proxy deploys to the same address on every chain (bootstrap -> upgrade). - SpokePool impl is now abstract with per-token variants (Usdc, Native) and a distinct EIP-712 domain name each, preventing cross-variant signature replay. Tron variant resolves USDT. - Signer read from beacon.signer() (rotatable via beacon upgrade). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CounterfactualTestBase now exposes _deployBeacon(CounterfactualChainConfig) (the beacon config is immutable, so each test builds it from its mocks) and _baseConfig(). Bridge tests construct no-arg impls, set endpoints/tokens/signer on the beacon, and drop sourceChainId + the token field from leaf params. SpokePool tests split into Usdc/Native variants (distinct EIP-712 domain names) and add cross-variant and RouteNotConfigured coverage; Tron test uses the USDT variant. 91 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- DeployCounterfactualBeacon (new): bootstrap -> ERC1967Proxy (chain-invariant init calldata) -> chain-specific CounterfactualBeacon impl -> upgradeToAndCall -> dispatcher -> setImplementation, keeping the proxy address identical per chain. - CounterfactualConfig._buildChainConfig() resolves the CounterfactualChainConfig from constants/deployed-addresses (signer, spokePool, wrappedNativeToken, cctp*, oft*, usdc, usdt), tolerating missing values as address(0). - Impl deploy scripts are now no-arg; SpokePool deploys Usdc + Native variants; add a VanillaCCTP deploy script. - CheckCounterfactualDeployments reads chain config from the beacon's getters (impls no longer expose them); impl checks become bytecode presence checks. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rewrite Address Determinism (one leaf per route, not per source chain), Route leaves (no sourceChainId/token; token fixed per impl), calldata injection (#3 chain-specific values from the beacon), Upgrade Mechanism (Route Trees are now chain-invariant; no sourceChainId defense-in-depth), Execution Fees (signer read from beacon.signer()), Vanilla CCTP, and the Trust Model (admin also owns the immutable, upgrade-gated chain config). Add a Chain Configuration section documenting the beacon getters, the immutable-config model, and the bootstrap->upgrade deploy for proxy parity. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the per-token SpokePool variants (…Usdc / …Native) with a single input-token-agnostic CounterfactualDepositSpokePool. The leaf carries the beacon getter selector for its input token (`inputTokenGetter`); bytes4(0) means a native deposit (wrapped via beacon.wrappedNativeToken()). The impl resolves it via a guarded staticcall to the beacon, so one implementation serves any registered token. Because inputTokenGetter is part of params, it is committed in routeParamsHash (which the SpokePool EIP-712 fee signature binds) — so a signature for one token never validates for another, and cross-chain replay is independently prevented by the chainId in the EIP-712 domain. This removes the need for per-token variants and per-variant EIP-712 domain names; the Tron variant once again only overrides transfer semantics and inherits the mainline name. CCTP / Vanilla CCTP / OFT are unchanged (USDC-only, read beacon.usdc() directly). Updates the SpokePool deploy script (one impl), DeployAll/Check scripts, tests (adds arbitrary-token and cross-token-signature coverage), and DESIGN.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- OFT impl resolves input token from periphery.TOKEN() (token-agnostic): every currently deployed SponsoredOFTSrcPeriphery is USDT0, so hardcoding beacon.usdc() bricked the OFT route on every chain. - Re-add deploy-time CCTP/OFT periphery guards in DeployAllCounterfactual so the beacon does not bake address(0) and silently brick routes. - Require Tron USDT in _buildChainConfig so the Tron SpokePool route cannot be deployed with usdt=0. - Transfer beacon ownership in the all-in-one transferRoles flow so the beacon admin does not stay on the deployer EOA. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- DeployAllCounterfactual: verify beacon proxy actually resolves the expected dispatcher (guarded staticcall) before skipping the beacon stack sub-script, so an interrupted previous deploy doesn't leave the proxy on the bootstrap or with a stale target. - ExtractDeployedFoundryAddresses: special-case the beacon deploy script so the ERC1967Proxy is recorded as `CounterfactualBeacon` (canonical address) and the per-chain implementation deploy is skipped; without this, the generic ERC1967Proxy→SpokePool rewrite would misfile the beacon proxy and overwrite SpokePool. - CheckCounterfactualDeployments: add manual review of the beacon owner and pendingOwner against config.toml and multisigs.json, mirroring the AdminWithdrawManager owner review so a deployer-EOA beacon admin is surfaced instead of silently passing. - CounterfactualConfig: require `spokePool != 0` in `_buildChainConfig` so a missing deployed-addresses.json entry fails at deploy time rather than baking address(0) into an immutable on the beacon implementation. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- DeployCounterfactualDepositSpokePool + DeployAllCounterfactual: branch on Tron (chainid 728126428) to deploy `CounterfactualDepositSpokePoolTr` instead of the mainline impl, so Tron USDT's non-standard `transfer` return value doesn't revert execution-fee payouts on the Foundry deploy path. The canonical Tron production deploy remains the TS script under `script/tron/counterfactual/`; this branch is the defensive Foundry path. - DeployAllCounterfactual: on the beacon-stack skip path, compare every field of the live beacon's immutable chain config against the values the current resolvers produce. Per-field mismatches are logged and a summary warning explains the UUPS-upgrade remediation, so a previously deployed beacon with a stale config no longer hides behind an "ALREADY DEPLOYED" message. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Matt flagged that the proxy address chains off this Bootstrap's CREATE2 address, which chains off its exact compiled creation code. Any solc bump, optimizer change, or AST-altering edit would move the Bootstrap → move the beacon proxy → move every counterfactual proxy. Added a "Bytecode pinning" paragraph to the natspec calling out the chain of dependence, the mitigation (deploy from a saved canonical bytecode artifact on later chains rather than recompiling), and an explicit "treat as frozen" cue so any future edit surfaces the bytecode-pinning workflow. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
mrice32's review: the SpokePool leaf was branching on `inputTokenGetter == bytes4(0)` to detect native, which couples the merkle commitment to "this chain has a native gas token". Switch to a value-based decision so the same leaf can serve both flavors: - Add `nativeToken()` getter to ICounterfactualBeacon / CounterfactualBeacon and a matching `nativeToken` field on `CounterfactualChainConfig`. - Replace the bytes4(0) selector special-case in CounterfactualDepositSpokePool with `address constant NATIVE_SENTINEL` (the Aave/Compound-style `0xEeee…EEeE`). The leaf always resolves inputTokenGetter via the staticcall and branches on `resolved == NATIVE_SENTINEL`. - CounterfactualConfig: `_resolveNativeToken` defaults to NATIVE_SENTINEL, with a per-chain ERC-20 override at `.NATIVE_TOKEN.<chainId>` for hypothetical chains whose canonical gas-token route is an ERC-20. - CheckCounterfactualDeployments: assert beacon's `nativeToken` matches the same resolver. - Tests: NATIVE_GETTER now names `nativeToken.selector`; setUp sets `cfg.nativeToken = NATIVE_SENTINEL`; new `testNativeGetterResolving- ToErc20UsesErc20Path` exercises the alt-chain ERC-20 case (same leaf, beacon returns ERC-20 → leaf takes the transferFrom path, no msg.value). - DESIGN.md updated. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…o taylor/counterfactual-chain-agnostic-deploy-cctp-fix Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
…#1456) * feat: chain-agnostic counterfactual leaves via beacon-provided config Move every chain-specific value (bridge endpoints, CCTP domain / OFT EID, fee signer, token addresses) out of the leaf-implementation immutables and the merkle leaf params, and onto the per-chain CounterfactualBeacon as public immutables. Leaf implementations resolve the beacon from the proxy's ERC-1967 beacon slot (CounterfactualImplementationBase) and read what they need at runtime; the input token is fixed per implementation. Result: leaf implementations are byte-identical across chains (one CREATE2 address everywhere) and a single leaf is valid on every chain, so initialRoot carries one leaf per route instead of one per source chain. Drops sourceChainId and the SourceChainMismatch check; an unconfigured route reverts RouteNotConfigured. - CounterfactualBeacon: add immutable chain config (CounterfactualChainConfig) with named getters (signer, spokePool, wrappedNativeToken, cctp*, oft*, usdc, usdt); implementation/upgradeRoot stay mutable. Config changes are UUPS upgrades. - CounterfactualBeaconBootstrap: chain-identical no-arg UUPS impl so the beacon proxy deploys to the same address on every chain (bootstrap -> upgrade). - SpokePool impl is now abstract with per-token variants (Usdc, Native) and a distinct EIP-712 domain name each, preventing cross-variant signature replay. Tron variant resolves USDT. - Signer read from beacon.signer() (rotatable via beacon upgrade). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: migrate counterfactual tests to chain-agnostic beacon config CounterfactualTestBase now exposes _deployBeacon(CounterfactualChainConfig) (the beacon config is immutable, so each test builds it from its mocks) and _baseConfig(). Bridge tests construct no-arg impls, set endpoints/tokens/signer on the beacon, and drop sourceChainId + the token field from leaf params. SpokePool tests split into Usdc/Native variants (distinct EIP-712 domain names) and add cross-variant and RouteNotConfigured coverage; Tron test uses the USDT variant. 91 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(scripts): migrate counterfactual deploy scripts to beacon config - DeployCounterfactualBeacon (new): bootstrap -> ERC1967Proxy (chain-invariant init calldata) -> chain-specific CounterfactualBeacon impl -> upgradeToAndCall -> dispatcher -> setImplementation, keeping the proxy address identical per chain. - CounterfactualConfig._buildChainConfig() resolves the CounterfactualChainConfig from constants/deployed-addresses (signer, spokePool, wrappedNativeToken, cctp*, oft*, usdc, usdt), tolerating missing values as address(0). - Impl deploy scripts are now no-arg; SpokePool deploys Usdc + Native variants; add a VanillaCCTP deploy script. - CheckCounterfactualDeployments reads chain config from the beacon's getters (impls no longer expose them); impl checks become bytecode presence checks. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: update counterfactual DESIGN for chain-agnostic leaves Rewrite Address Determinism (one leaf per route, not per source chain), Route leaves (no sourceChainId/token; token fixed per impl), calldata injection (#3 chain-specific values from the beacon), Upgrade Mechanism (Route Trees are now chain-invariant; no sourceChainId defense-in-depth), Execution Fees (signer read from beacon.signer()), Vanilla CCTP, and the Trust Model (admin also owns the immutable, upgrade-gated chain config). Add a Chain Configuration section documenting the beacon getters, the immutable-config model, and the bootstrap->upgrade deploy for proxy parity. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: make SpokePool counterfactual input-token-agnostic Replace the per-token SpokePool variants (…Usdc / …Native) with a single input-token-agnostic CounterfactualDepositSpokePool. The leaf carries the beacon getter selector for its input token (`inputTokenGetter`); bytes4(0) means a native deposit (wrapped via beacon.wrappedNativeToken()). The impl resolves it via a guarded staticcall to the beacon, so one implementation serves any registered token. Because inputTokenGetter is part of params, it is committed in routeParamsHash (which the SpokePool EIP-712 fee signature binds) — so a signature for one token never validates for another, and cross-chain replay is independently prevented by the chainId in the EIP-712 domain. This removes the need for per-token variants and per-variant EIP-712 domain names; the Tron variant once again only overrides transfer semantics and inherits the mainline name. CCTP / Vanilla CCTP / OFT are unchanged (USDC-only, read beacon.usdc() directly). Updates the SpokePool deploy script (one impl), DeployAll/Check scripts, tests (adds arbitrary-token and cross-token-signature coverage), and DESIGN.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(counterfactual): address codex review on chain-agnostic leaves - OFT impl resolves input token from periphery.TOKEN() (token-agnostic): every currently deployed SponsoredOFTSrcPeriphery is USDT0, so hardcoding beacon.usdc() bricked the OFT route on every chain. - Re-add deploy-time CCTP/OFT periphery guards in DeployAllCounterfactual so the beacon does not bake address(0) and silently brick routes. - Require Tron USDT in _buildChainConfig so the Tron SpokePool route cannot be deployed with usdt=0. - Transfer beacon ownership in the all-in-one transferRoles flow so the beacon admin does not stay on the deployer EOA. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(counterfactual): address second codex review pass - DeployAllCounterfactual: verify beacon proxy actually resolves the expected dispatcher (guarded staticcall) before skipping the beacon stack sub-script, so an interrupted previous deploy doesn't leave the proxy on the bootstrap or with a stale target. - ExtractDeployedFoundryAddresses: special-case the beacon deploy script so the ERC1967Proxy is recorded as `CounterfactualBeacon` (canonical address) and the per-chain implementation deploy is skipped; without this, the generic ERC1967Proxy→SpokePool rewrite would misfile the beacon proxy and overwrite SpokePool. - CheckCounterfactualDeployments: add manual review of the beacon owner and pendingOwner against config.toml and multisigs.json, mirroring the AdminWithdrawManager owner review so a deployer-EOA beacon admin is surfaced instead of silently passing. - CounterfactualConfig: require `spokePool != 0` in `_buildChainConfig` so a missing deployed-addresses.json entry fails at deploy time rather than baking address(0) into an immutable on the beacon implementation. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(counterfactual): address third codex review pass - DeployCounterfactualDepositSpokePool + DeployAllCounterfactual: branch on Tron (chainid 728126428) to deploy `CounterfactualDepositSpokePoolTr` instead of the mainline impl, so Tron USDT's non-standard `transfer` return value doesn't revert execution-fee payouts on the Foundry deploy path. The canonical Tron production deploy remains the TS script under `script/tron/counterfactual/`; this branch is the defensive Foundry path. - DeployAllCounterfactual: on the beacon-stack skip path, compare every field of the live beacon's immutable chain config against the values the current resolvers produce. Per-field mismatches are logged and a summary warning explains the UUPS-upgrade remediation, so a previously deployed beacon with a stale config no longer hides behind an "ALREADY DEPLOYED" message. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(counterfactual): pin Bootstrap bytecode for chain-invariant proxy Matt flagged that the proxy address chains off this Bootstrap's CREATE2 address, which chains off its exact compiled creation code. Any solc bump, optimizer change, or AST-altering edit would move the Bootstrap → move the beacon proxy → move every counterfactual proxy. Added a "Bytecode pinning" paragraph to the natspec calling out the chain of dependence, the mitigation (deploy from a saved canonical bytecode artifact on later chains rather than recompiling), and an explicit "treat as frozen" cue so any future edit surfaces the bytecode-pinning workflow. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(counterfactual): native vs ERC-20 decided by getter return value mrice32's review: the SpokePool leaf was branching on `inputTokenGetter == bytes4(0)` to detect native, which couples the merkle commitment to "this chain has a native gas token". Switch to a value-based decision so the same leaf can serve both flavors: - Add `nativeToken()` getter to ICounterfactualBeacon / CounterfactualBeacon and a matching `nativeToken` field on `CounterfactualChainConfig`. - Replace the bytes4(0) selector special-case in CounterfactualDepositSpokePool with `address constant NATIVE_SENTINEL` (the Aave/Compound-style `0xEeee…EEeE`). The leaf always resolves inputTokenGetter via the staticcall and branches on `resolved == NATIVE_SENTINEL`. - CounterfactualConfig: `_resolveNativeToken` defaults to NATIVE_SENTINEL, with a per-chain ERC-20 override at `.NATIVE_TOKEN.<chainId>` for hypothetical chains whose canonical gas-token route is an ERC-20. - CheckCounterfactualDeployments: assert beacon's `nativeToken` matches the same resolver. - Tests: NATIVE_GETTER now names `nativeToken.selector`; setUp sets `cfg.nativeToken = NATIVE_SENTINEL`; new `testNativeGetterResolving- ToErc20UsesErc20Path` exercises the alt-chain ERC-20 case (same leaf, beacon returns ERC-20 → leaf takes the transferFrom path, no msg.value). - DESIGN.md updated. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(counterfactual): address third codex review pass - CheckCounterfactualDeployments: branch the SpokePool-impl bytecode check on Tron (728126428 → CounterfactualDepositSpokePoolTr) so a correct Tron deployment doesn't read as missing, mirroring the DeployCounterfactualDepositSpokePool branch and the address-extractor contract name. - CheckCounterfactualDeployments: assert `beacon.implementation()` equals the deployed CounterfactualDeposit dispatcher, so a beacon deploy that stopped after `upgradeToAndCall` but before `setImplementation(dispatcher)` no longer silently passes. - DeployAllCounterfactual `_warnIfBeaconConfigStale`: include `nativeToken` in the diff so a `.NATIVE_TOKEN.<chainId>` override added/removed/changed after the beacon is deployed surfaces in the stale-config warning. - CounterfactualConfig `_resolveNativeToken` (and the readiness mirror): fall back to `address(0)` when the chain has no `.WRAPPED_NATIVE_TOKENS.<chainId>` entry (e.g. Lens 232, Tempo 4217), so the sentinel-but-no-wrapper footgun (commit to native branch then revert at `_requireConfigured(beacon.wrappedNativeToken())`) is closed — leaves naming `nativeToken.selector` cleanly RouteNotConfigured at the resolve step there. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * feat: OFT counterfactual supports multiple tokens via a periphery selector The OFT leaf now carries a `peripheryGetter` — the 4-byte selector of the beacon getter for the SponsoredOFTSrcPeriphery to use. Each OFT periphery is single-token (immutable TOKEN()), so naming the periphery selects the input token, which the impl reads from the resolved periphery. Supporting another OFT token is a beacon upgrade adding another periphery getter — no leaf/impl change. - Beacon: add oftUsdcPeriphery alongside oftSrcPeriphery (per-token OFT peripheries; leaf names which via selector). Config resolver + DeployAll stale-check + Check updated. - Move the guarded selector→address staticcall into CounterfactualImplementationBase as the shared _resolveBeaconAddress; SpokePool and OFT both use it (SpokePool's private _resolveInputToken removed). - OFT test: two single-token peripheries wired; new test proves a second token routes through its periphery via OFT_USDC_GETTER while the primary periphery is untouched. - DESIGN.md: OFT route is now selector-driven like SpokePool. maxExecutionFee intentionally stays in the leaf params (per-chain move deferred). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs: tighten counterfactual code comments Make the NatSpec and inline comments across the counterfactual contracts, deploy scripts, and tests more concise — cut redundancy and over-explanation while keeping the correctness/security rationale (fee-before-periphery ordering, RouteNotConfigured gating, EIP-712 binding, bytecode pinning, prank-before-external-call test gotchas). Comments only; no code changes (build + 94 tests unchanged). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: drop fictional oftUsdcPeriphery — USDC isn't an OFT token USDC bridges via CCTP, not LayerZero OFT, so a "USDC OFT periphery" doesn't exist; oftUsdcPeriphery was a wrongly-named placeholder added to demo multi-token. Remove it from the beacon (struct/immutable/getter), config resolver, deploy/check scripts, and DESIGN. The selector mechanism stays: the OFT leaf carries `peripheryGetter`, the beacon exposes oftSrcPeriphery (USDT0), and real additional OFT tokens are onboarded by adding a periphery getter via a beacon upgrade. The OFT test now proves the selector is genuinely consulted (RouteNotConfigured when the named getter is unset) instead of routing a fake USDC periphery. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat: per-(chain,token) maxExecutionFee via a leaf selector + audit note Move the execution-fee cap out of the leaf as a hardcoded value and onto the per-chain beacon, named by a `bytes4 maxExecutionFeeGetter` the leaf carries (mirroring inputTokenGetter/peripheryGetter). The impl resolves it with the new `_resolveBeaconUint`. For CCTP/Vanilla/OFT it's the executionFee cap; for SpokePool it's the fixed component of the combined cap (`maxFeeBps` stays in the leaf). Add example per-(token,bridge) cap getters on the beacon (immutable uint256): usdcCctpMaxExecutionFee, usdtOftMaxExecutionFee, usdcSpokePoolMaxExecutionFee, usdtSpokePoolMaxExecutionFee, wethSpokePoolMaxExecutionFee. Vanilla CCTP reuses the USDC CCTP getter. Wired into the config resolver (optional config.toml reads, default 0), the DeployAll stale-check, and the Check script. Add a note on CounterfactualBeacon that its immutable config values are pure configuration — a new implementation changing only those is not subject to audit; only logic changes are. Tests updated to set caps on the beacon and name them via selectors. 94 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * refactor: split CounterfactualBeacon into base (logic) + derived (config) Move root/implementation management, UUPS, and Ownable2Step admin into a new abstract CounterfactualBeaconBase. CounterfactualBeacon now derives from it and holds only the chain-specific immutables (endpoints, tokens, fee signer, fee caps) plus the constructor wiring. This keeps the audit boundary clean: the base is the reviewable logic, while a config-only change is a new derived contract that touches no logic. Static references to inherited members (initialize, implementation, NotAContract, WrongBeacon) are repointed to CounterfactualBeaconBase, since inherited members are not reachable via the derived contract type. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * bind OFT & CCTP sigs to routeParams * fix: guard nativeToken=sentinel requires wrappedNativeToken at deploy _buildChainConfig now rejects a config where nativeToken is the native sentinel but wrappedNativeToken is unset. _resolveNativeToken upholds this for the default path, but returns a .NATIVE_TOKEN.<chainId> override verbatim, so an operator override could otherwise bake a sentinel that bricks at execution (the native branch reverts on a zero wrappedNativeToken). Fail loud at deploy instead, matching the existing spokePool/Tron-USDT guards. Addresses a review comment on PR #1456. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix merkle tree hashing description in design doc (#1468) Co-authored-by: Taylor Webb <taylor@Taylors-MacBook-Pro.local> * fix: fix cctp maxFee (#1463) * fix cctp maxFee * add maxFee and minFinalityThreshold to signature check --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: droplet-rl <284132418+droplet-rl@users.noreply.github.com> Co-authored-by: Taylor Webb <tbwebb22@gmail.com> Co-authored-by: Taylor Webb <84364476+tbwebb22@users.noreply.github.com> Co-authored-by: Taylor Webb <taylor@Taylors-MacBook-Pro.local>
tbwebb22
requested review from
bmzig,
mrice32,
nicholaspai and
pxrl
as code owners
…actual-chain-agnostic-deploy-cctp-fix
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4a2b83a0f9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
224
to
227
| string.concat(SCRIPT_DIR, "DeployCounterfactualBeacon.s.sol"), | ||
| "DeployCounterfactualBeacon", | ||
| "", | ||
| ' --sig "run()"', | ||
| profile |
There was a problem hiding this comment.
On a fresh chain, or after changing beacon config, this path calls DeployCounterfactualBeacon without first creating the DeployCounterfactualBeaconImpl.s.sol broadcast that DeployCounterfactualBeacon._run() requires via getLatestBroadcastDeployment(...). That child script will revert before deploying/upgrading the proxy, and _runForgeScript appends || true, so the orchestrator can continue with the beacon stack missing or unwired instead of performing the documented “deploy all” flow.
Useful? React with 👍 / 👎.
Contributor
Author
Comment on lines
+176
to
+180
| cfg.usdcCctpMaxExecutionFee = type(uint256).max; | ||
| // Bps cap (not token units) on the submitter-chosen Circle fast-transfer fee (vanilla CCTP); | ||
| // 0 if unset ⇒ standard transfers only on this chain. | ||
| cfg.usdcCctpMaxFeeBps = _resolveFeeCap("usdcCctpMaxFeeBps"); | ||
| cfg.usdtOftMaxExecutionFee = _resolveFeeCap("usdtOftMaxExecutionFee"); | ||
| cfg.usdcSpokePoolMaxExecutionFee = _resolveFeeCap("usdcSpokePoolMaxExecutionFee"); | ||
| cfg.usdtSpokePoolMaxExecutionFee = _resolveFeeCap("usdtSpokePoolMaxExecutionFee"); | ||
| cfg.wethSpokePoolMaxExecutionFee = _resolveFeeCap("wethSpokePoolMaxExecutionFee"); | ||
| cfg.usdcCctpMaxFeeBps = 10_000; | ||
| cfg.usdtOftMaxExecutionFee = type(uint256).max; |
There was a problem hiding this comment.
For every beacon implementation deployed with this helper, the new cap getters return effectively unbounded values (uint256.max or 10,000 bps) even though the leaf contracts now rely on these beacon getters after removing the per-leaf maxExecutionFee. If the fee signer/backend produces an excessive executionFee or vanilla CCTP maxFeeCctp, the on-chain cap no longer limits it and the clone’s funds can be paid or burned as fees; these values should come from explicit config or remain committed in the route.
Useful? React with 👍 / 👎.
tbwebb22
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70d1ccca43
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // impl the beacon is deployed, and this script never upgrades a live beacon — bail without changes. | ||
| // (Re-pointing to a newer impl/dispatcher is an upgrade, performed out of band by the beacon owner.) | ||
| address currentImpl = address(uint160(uint256(vm.load(deployedProxy, ERC1967Utils.IMPLEMENTATION_SLOT)))); | ||
| if (currentImpl != bootstrap) { |
There was a problem hiding this comment.
In the recovery case where an earlier broadcast already ran upgradeToAndCall but failed before deploying/calling setImplementation, currentImpl is no longer the bootstrap while beacon.implementation() is still zero or stale. DeployAllCounterfactual explicitly treats that as not wired and invokes this script again, but this guard returns before deploying the dispatcher or calling setImplementation, so the final code-presence summary can report success while every counterfactual BeaconProxy still resolves a bad implementation. Continue through the dispatcher wiring when the proxy is on a real impl but the beacon target is not the predicted dispatcher.
Useful? React with 👍 / 👎.
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f79477af43
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| console.log("ERROR: no CounterfactualBeacon impl broadcast for chain %d.", block.chainid); | ||
| console.log("Run DeployCounterfactualBeaconImpl with --broadcast first."); | ||
| } | ||
| require(beaconImpl != address(0), "no impl broadcast for this chain: run DeployCounterfactualBeaconImpl first"); |
There was a problem hiding this comment.
For an already-deployed/wired beacon, rerunning this deploy-only script, or calling run(true) just to transfer ownership, should not need a freshly recorded DeployCounterfactualBeaconImpl broadcast. This require runs before the script inspects the existing proxy state, so operators who have not just run the new impl script get no impl broadcast... even though no implementation upgrade is needed for the no-op/recovery path. Resolve the live proxy state first and only require beaconImpl when the proxy is still on the bootstrap.
Useful? React with 👍 / 👎.
| console.log("Beacon already deployed and wired; impl slot:", currentImpl); | ||
| console.log("Deploy-only script makes no changes. Upgrades are performed separately by the owner."); | ||
| console.log("============================================"); | ||
| return; |
There was a problem hiding this comment.
When run(true) is used after a previous run already deployed and wired the beacon but left ownership on the deployer, this early return exits before the doTransferOwnership block below. That makes the documented multisig handoff a no-op in the recovery/second-step case, leaving the deployer EOA as the beacon admin that can upgrade or retarget all counterfactual proxies. Allow the ownership-transfer logic to run before returning when doTransferOwnership is true.
Useful? React with 👍 / 👎.
Signed-off-by: Faisal Usmani <faisal.of.usmani@gmail.com>
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
4 participants
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.
No description provided.