Skip to content

refactor(bridge-core): make protocol contract addresses baked-only (SDK is source of truth) - #24

Open
AvivYossef-starkware wants to merge 1 commit into
mainfrom
bake-protocol-addresses
Open

refactor(bridge-core): make protocol contract addresses baked-only (SDK is source of truth)#24
AvivYossef-starkware wants to merge 1 commit into
mainfrom
bake-protocol-addresses

Conversation

@AvivYossef-starkware

@AvivYossef-starkware AvivYossef-starkware commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Why

The fixed protocol contract/token addresses are network-deterministic facts the SDK already bakes per-network — but each stayed env-overridable, so a consuming app could shadow them from its own env. When one drifted (a stale VITE_ANONYMIZER_ADDRESS pinned to the retired 9-felt OutboundAnonymizer), it silently retargeted the pool withdraw / CCTP burn and broke cash-out. This makes the bridge the single source of truth for those addresses so no app env can shadow them.

What

Baked-only in config.ts (dropped the e.vars.X || override):

  • poolAddress
  • anonymizerAddress (OutboundAnonymizer) + inboundAnonymizerAddress
  • cctp.snTokenMessengerMinter + cctp.snMessageTransmitter
  • the shared EVM CCTP TokenMessengerV2
  • strkToken + depositToken.address (native USDC)

Address values are unchanged — only the override path is removed.

Kept env-overridable (deploy/infra/policy, not fixed protocol facts): ozClassHash (required, declared per network), all RPC/prover/indexer URLs, the Iris URL, depositToken symbol/decimals/mintEntrypoint, and the per-chain EVM CCTP source-registry USDC/RPC rows.

⚠️ Removed escape hatch (reviewer note)

This intentionally removes the documented Override via ANONYMIZER_ADDRESS for a fork capability for the listed addresses. A fork / local-redeploy workflow (e.g. testing against a freshly deployed anonymizer during contract dev) now has to fork the SDK rather than set an env var. That is the accepted cost of the hard cut.

Scope deferred

Per-chain EVM CCTP source-registry USDC token addresses stay overridable in this PR (entangled with per-chain RPC overrides + the empty-string-footgun tests). A follow-up could bake those too.

Does NOT fix prod cash-out by itself

This only removes the ability to shadow. Production cash-out is fixed once a new SDK is released and the app bumps to it (both deferred). Until then prod still resolves whatever its deploy-env injects.

Tests

  • Inverted the "override wins" cases to "override IGNORED", incl. the exact stale anonymizer address that broke cash-out.
  • Added a funds-safety block pinning that each baked field ignores its now-legacy env var.
  • Dropped the now-inert address fixtures from vitest.setup / apps/bridge vitest.config; simulate the 0x0 inbound placeholder via the live-config Proxy.
  • Full suite green: bridge-core 976, apps/bridge 69; typecheck clean on both.

🤖 Generated with Claude Code


This change is Reviewable

…DK is source of truth)

The fixed protocol contract/token addresses are network-deterministic facts the SDK
already bakes per-network, yet each stayed env-overridable. That let a consuming app
shadow them from its own env — and when one drifted (a stale VITE_ANONYMIZER_ADDRESS
pinned to the retired 9-felt OutboundAnonymizer) it silently retargeted the pool
withdraw/CCTP burn and broke cash-out.

Make these baked-only in config.ts (drop the `e.vars.X ||` override), so the SDK is
their single source of truth and no app env can shadow them:
  - poolAddress
  - anonymizerAddress (OutboundAnonymizer) + inboundAnonymizerAddress
  - cctp.snTokenMessengerMinter + cctp.snMessageTransmitter
  - the shared EVM CCTP TokenMessengerV2
  - strkToken + depositToken.address (native USDC)

Kept env-overridable (deploy/infra/policy, not fixed protocol facts): ozClassHash
(required, must be declared per network), all RPC/prover/indexer URLs, the Iris URL,
depositToken symbol/decimals/mintEntrypoint, and the per-chain EVM CCTP source-registry
USDC/RPC rows (a follow-up could bake those token addresses too).

Tests: invert the "override wins" cases to "override IGNORED" and add a funds-safety
block pinning that each baked field ignores its now-legacy env var (incl. the exact
stale anonymizer address that broke cash-out). Drop the now-inert address fixtures from
vitest.setup; simulate the 0x0 inbound placeholder via the live-config Proxy. Full
suite green (976 tests), typecheck clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

Copy link
Copy Markdown
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@AvivYossef-starkware
AvivYossef-starkware marked this pull request as ready for review July 16, 2026 13:59
@cursor

cursor Bot commented Jul 16, 2026

Copy link
Copy Markdown

PR Summary

High Risk
Changes which on-chain contracts bridge flows target (withdraw/burn/CCTP); wrong addresses strand funds, though this PR hardens against env drift rather than changing baked values.

Overview
Makes fixed protocol contract and token addresses in bridge-core baked-only so app env vars can no longer shadow them. Addresses are unchanged; only the e.vars.X || override path is removed for pool, outbound/inbound anonymizers, Starknet CCTP messenger/transmitter, shared EVM TokenMessengerV2, STRK, and native USDC deposit token.

This closes the fund-safety hole where a stale ANONYMIZER_ADDRESS (e.g. the retired 9-felt contract) could retarget pool withdraw / CCTP burn and break cash-out. Infra-style settings (RPC URLs, ozClassHash, deposit token symbol/decimals, per-chain EVM USDC/RPC rows) stay env-overridable.

Tests now assert legacy env overrides are ignored, including the exact stale mainnet anonymizer. Vitest fixtures drop those address vars; unclaimedReturns simulates an unconfigured inbound via the live config proxy. Move From Pool shows a generic anonymizer error instead of naming VITE_ANONYMIZER_ADDRESS.

Fork/local-redeploy workflows that relied on env overrides for these addresses must change the SDK instead.

Reviewed by Cursor Bugbot for commit 63a1eba. Bugbot is set up for automated code reviews on this repo. Configure here.

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