[evm/runtime]: make IntentGatewayV2 upgradeable via cross-chain governance#988
Conversation
050a11c to
9279bbb
Compare
|
🔴 2. Atomic CREATE2 proxy deployment will revert — initialize owner gate is incompatible with the deploy flow ERC1967Proxy proxy = new ERC1967Proxy{salt: salt}(address(implementation), initData); // initData = initialize(...) Under forge script --broadcast, salted creation is routed through the deterministic CREATE2 factory (0x4e59…4956C). During the proxy constructor, ERC1967Utils.upgradeToAndCall delegatecalls initialize, and delegatecall preserves msg.sender = the CREATE2 factory — not admin. So msg.sender != _owner ⇒ revert, and the whole deployment fails (unless ADMIN is literally set to the factory address, which would make the gate meaningless). This is masked because the tests don't exercise the atomic path — _deployGatewayProxy uses empty init data and calls initialize separately from address(this) (= the owner), with a comment claiming it "mirrors production" (IntentGatewayV2Test.sol:99-106). Production is atomic; the comment is wrong and the real path is untested. Fix (recommended): drop the msg.sender == _owner check entirely. With atomic init, the initData is bound into the CREATE2 initcode hash, so the canonical address can only be produced by deploying the exact correct params — the owner gate adds nothing and breaks the flow. (initializer still guarantees single-init.) Then add a test that deploys through new ERC1967Proxy{salt}(impl, initData) so the atomic path is actually covered. If you instead keep separate (non-atomic) init, the owner gate is fine and front-running is harmless (front-runner isn't the owner), but the script must be changed to empty initData + a separate initialize tx. |
|
Yeah we can just drop the owner gating |
…indowExhausted in on_finalize - indexer: ordersStorageSlot now targets _orders at slot 9. PR #988 removed the _admin storage slot from IntentGatewayV2, shifting _orders down from 10 to 9. Injecting at the stale slot left the escrow override a no-op. - pallet: move the PhantomBidWindowExhausted emission from on_initialize to on_finalize so a bid placed in the window-closing block is already in storage when the indexer aggregates the snapshot. The on_finalize storage reads are reserved in the on_initialize weight.
Makes
IntentGatewayV2upgradeable by the Hyperbridge coprocessor — the same authority that already governsUpdateParams/SweepDust— through a newRequestKind.UpgradeContracthandled inonAccept. The gateway runs behind anERC1967Proxy, initialized atomically at deployment.EVM
ERC1967Proxy. The implementation stays a plain contract (no UUPS); upgrades go throughERC1967Utils.upgradeToAndCallinonAccept, gated bysource == hyperbridge. The branch lives in the inherited base so it can't be dropped by a future upgrade.initwith aninitializer-guardedinitialize;_disableInitializers()locks the raw implementation. The init gate is an immutable_owner(replaces storage_admin, zero slots)._filledstays at storage slot 2.initData._instance()resolves toaddress(this), so cross-chain peers share the gateway's address rather than a stored registry, and the self-referential deployments array is dropped.NewDeploymentremains as a governance override._pausedstorage flag (slot 13, after all proof-sensitive slots); the pause logic is deferred.Coprocessor
RequestKind::UpgradeContract { new_impl, init_data }and aGovernanceOrigin-gatedupgrade_gatewayextrinsic mirroringupdate_params. Wire byte5matches the EVM enum; the payload is encoded to match the Solidityabi.decode(body[1:], (address, bytes)).Determinism
Atomic init makes the proxy address depend on the encoded
Params. Those are byte-identical across chains (host,dispatcher,solverSelectionuniform), so the proxy lands at the same address everywhere — which is exactly what lets_instance()resolve peers toaddress(this). This relies onADMINandParamsstaying uniform across chains, the discipline the deployment already follows.Closes #981.