Skip to content

fix: getBridgeUiConfig publishes the orbit-chain proxyAdmin under l2Contracts (parent chain) - #749

Open
gomesalexandre wants to merge 1 commit into
OffchainLabs:mainfrom
gomesalexandre:fix_bridge_ui_config_parent_proxy_admin
Open

fix: getBridgeUiConfig publishes the orbit-chain proxyAdmin under l2Contracts (parent chain)#749
gomesalexandre wants to merge 1 commit into
OffchainLabs:mainfrom
gomesalexandre:fix_bridge_ui_config_parent_proxy_admin

Conversation

@gomesalexandre

Copy link
Copy Markdown

bug

getBridgeUiConfig (src/getBridgeUiConfig.ts) publishes the orbit/child-chain ProxyAdmin address under l2Contracts.proxyAdmin, which the type's own doc comment says is "Parent chain (L2) contract addresses":

l2Contracts: {
  customGateway: tokenBridgeContracts.parentChainContracts.customGateway,
  multicall:     tokenBridgeContracts.parentChainContracts.multicall,
  proxyAdmin:    tokenBridgeContracts.orbitChainContracts.proxyAdmin,   // wrong chain
  router:        tokenBridgeContracts.parentChainContracts.router,
  ...
},
l3Contracts: {
  proxyAdmin:    tokenBridgeContracts.orbitChainContracts.proxyAdmin,   // correct here

Every other field in l2Contracts is sourced from parentChainContracts; only proxyAdmin reaches into orbitChainContracts. Root cause: TokenBridgeParentChainContracts has no proxyAdmin field of its own (inboxToL1Deployment only returns 6 addresses for the parent side), so the only proxyAdmin in scope was the child chain's. The correct value is available in the same function: coreContracts.adminProxy, decoded from the RollupCreated event and already bound at the top of getBridgeUiConfig.

on-chain verification (real RPC calls, not inferred)

Ran against the exact deployment tx the repo's own unit test already uses (0xc172278b80251a7368409b5358fbf7e6250c1afc9a262e38ca15e1ba2bb9a7a8, Sepolia, via https://sepolia.gateway.tenderly.co):

coreContracts.adminProxy (the fix):
  0xA811e68582E043680D915a38Cc11005Ef1F0A509

tokenBridgeContracts.orbitChainContracts.proxyAdmin (currently emitted, wrong chain):
  0x7618B3d4d8C71132BfCd33fDD98424a060F37470

current (pre-fix) l2Contracts.proxyAdmin output:
  0x7618B3d4d8C71132BfCd33fDD98424a060F37470
matches orbitChainContracts.proxyAdmin (i.e. wrong-chain address)? true

--- eth_getCode on Sepolia (the parent chain) ---
currently-emitted l2Contracts.proxyAdmin  bytecode length: 0 bytes
coreContracts.adminProxy (the fix)         bytecode length: 1933 bytes
sibling l2Contracts.router (known-good)    bytecode length: 2202 bytes

The address the SDK currently hands out as the parent chain's ProxyAdmin has zero bytecode on the parent chain — it doesn't exist there. coreContracts.adminProxy does, with a bytecode size consistent with the other real proxies on that chain.

fix

proxyAdmin: coreContracts.adminProxy at src/getBridgeUiConfig.ts:114. Re-recorded the snapshot (src/__snapshots__/getBridgeUiConfig.unit.test.ts.snap) — the only field that changes is l2Contracts.proxyAdmin, from the wrong-chain address to coreContracts.adminProxy, matching the on-chain evidence above.

Added a regression test:

  • l2Contracts.proxyAdmin !== l3Contracts.proxyAdmin (catches the exact regression: reusing the same value across both chains)
  • the resolved l2Contracts.proxyAdmin has real bytecode on the parent-chain client (catches a future wrong-but-different value too, not just this specific one)

testing

$ npx vitest run src/getBridgeUiConfig.unit.test.ts
 Test Files  1 passed (1)
      Tests  1 passed (1)

$ npx tsc --project ./tsconfig.json --module commonjs --outDir ... --declaration --noEmit
(clean, exit 0)

$ npx eslint src/getBridgeUiConfig.ts src/getBridgeUiConfig.unit.test.ts --cache
(clean, exit 0)

Full suite before vs. after the fix, to confirm no regressions beyond the one expected snapshot change:

baseline (unmodified main):  Test Files 19 failed | 41 passed (60)   Tests 18 failed | 312 passed (330)
after this fix:              Test Files 19 failed | 41 passed (60)   Tests 18 failed | 312 passed (330)

Identical failure counts — the 18 pre-existing failures (mostly createRollupPrepareTransactionRequest.type.test.ts erroring on Parent chain not supported: undefined, unrelated to this file) are already present on unmodified main and untouched by this change; my one new test is now green and counted in the 312.

review note

Ran a synchronous adversarial Codex pass as a second reviewer; it didn't finish producing a verdict within a reasonable window (killed rather than left running indefinitely), so this went through a second thorough self-review pass instead, specifically re-checking: (1) whether coreContracts.adminProxy is really the correct semantic value here (yes — consistent with l3Contracts already correctly using orbitChainContracts.proxyAdmin the same way, and confirmed on-chain above), (2) whether the new test has gaps (addressed by pairing the inequality check with a bytecode-presence check), (3) whether anything else nearby in this function is wrong (re-read every field; all others are already correctly matched to their chain's contracts object).

risk

Low. Single-field fix reusing a value (coreContracts.adminProxy) that's already computed and used elsewhere in the same function; no new dependencies, no change to any other field.

…rent-chain one

l2Contracts.proxyAdmin (docs: "Parent chain (L2) contract addresses") was
populated from tokenBridgeContracts.orbitChainContracts.proxyAdmin instead
of the parent chain's own admin proxy. coreContracts.adminProxy (decoded
from the RollupCreated event, already bound in scope) is the correct value.

Verified on-chain against the deployment tx the existing test already uses
(0xc172278b...a7a8, Sepolia): the previously-emitted address
(0x7618B3d4d8C71132BfCd33fDD98424a060F37470) has zero bytecode on Sepolia,
while coreContracts.adminProxy (0xA811e68582E043680D915a38Cc11005Ef1F0A509)
has 1933 bytes of real contract code there.

Added a regression test asserting l2Contracts.proxyAdmin != l3Contracts.proxyAdmin
and that it resolves to a real contract on the parent chain.
@gomesalexandre
gomesalexandre marked this pull request as ready for review September 1, 2026 13:02
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