feat: sync zone contracts with TIP-1091#682
Conversation
|
cyclops audit fast |
|
cc @legion2002 Cyclops audit event published. View workflow run Config: config: |
![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: c27d24bae1
ℹ️ 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".
| mapping(address => bool) internal _validVerifiers; | ||
| address internal _verifier; | ||
| address internal _messenger; | ||
| address public owner; |
There was a problem hiding this comment.
When this ZoneFactory is deployed as a regular Solidity contract, nextZoneId is initialized by the creation code but owner remains address(0) because the constructor that assigned msg.sender was removed. Both createZone and transferOwnership require msg.sender == owner, so any normally deployed local/test stand-in is permanently unusable unless storage slot 3 is patched by hand. If the reference contract is still meant to be deployable, it needs a constructor or initializer that sets owner.
Useful? React with 👍 / 👎.
| messengerFactory.setPortal(OTHER_ZONE_ID, otherPortal); | ||
|
|
||
| messenger = new ZoneMessenger(address(messengerFactory)); | ||
| messenger = ZoneMessenger(ZONE_MESSENGER_ADDRESS); |
There was a problem hiding this comment.
In this setup only ZONE_FACTORY_ADDRESS is etched; unlike tests that call _deployZoneFactory(), ZONE_MESSENGER_ADDRESS still has no code. Binding messenger to that empty address means zoneFactory() decodes empty returndata and relayMessage() calls either no-op or fail expectations instead of testing the constructorless messenger. Etch type(ZoneMessenger).runtimeCode at the fixed messenger address before assigning this handle.
Useful? React with 👍 / 👎.
tempoxyz-bot
left a comment
tempoxyz-bot
left a comment
There was a problem hiding this comment.
👁️ Cyclops Review — No actionable findings.
The consolidated worker reports found the TIP-1091 fixed-address refactor internally consistent. The following non-blocking areas are called out for human confirmation:
Reviewer Callouts
- ⚡ Fixed-address genesis/dependency provisioning: Verify production/dev genesis seeds
ZoneFactory.ownerandZoneFactory.nextZoneIdcorrectly and installs code atZONE_PORTAL_IMPL_ADDRESS,ZONE_VERIFIER_ADDRESS, andZONE_MESSENGER_ADDRESS; consider extendingcrates/node/src/dev.rspreflight checks beyond the factory presence check. - ⚡ Portal proxy deployment semantics: Confirm the production factory/proxy path, node portal discovery, and tests all exercise the intended TIP-1091 protocol-etched proxy deployment model.
- ⚡ Global messenger reentrancy guard: Confirm no legitimate cross-zone relay path needs nested relay execution through the singleton messenger in one transaction.
- ⚡ Legacy receiver/router compatibility: Consider operator-facing warnings for old callback receivers using
zoneFactory.messenger()authentication and forxtask/src/deploy_router.rsnon-fixed factory overrides.
| /// Explorer: https://explore.moderato.tempo.xyz/address/0x5aF2000000000000000000000000000000000000 | ||
| pub(crate) const MODERATO_ZONE_FACTORY: Address = ZONE_FACTORY_ADDRESS; |
There was a problem hiding this comment.
The fixed factory is not active on Moderato yet, while the previous factory remains live. Landing this before the network activation breaks the default create-zone, zone-info, and deploy-router flows. Can we remove this change until the factory precompile is activated on testnet?
| function messenger() external view returns (address); | ||
| function zones(uint32 zoneId) external view returns (ZoneInfo memory); | ||
| function zoneCount() external view returns (uint32); | ||
| function nextZoneId() external view returns (uint32); |
There was a problem hiding this comment.
The existing skills in this repo still call the removed zoneCount() and verifier() methods. Can we update the skills as well?
0xKitsune
left a comment
0xKitsune
left a comment
There was a problem hiding this comment.
Lgtm, just a few comments.
| pub const ZONE_PORTAL_PREFIX: FixedBytes<12> = fixed_bytes!("5AD000000000000000000000"); | ||
| pub const ZONE_PORTAL_IMPL_ADDRESS: Address = | ||
| address!("0x5AD1000000000000000000000000000000000000"); | ||
| pub const ZONE_VERIFIER_ADDRESS: Address = address!("0x5a56000000000000000000000000000000000000"); | ||
| pub const ZONE_MESSENGER_ADDRESS: Address = address!("0x5A4d000000000000000000000000000000000000"); |
There was a problem hiding this comment.
Will this break existing flows on Testnet before these are live?
| returns (bytes4) | ||
| { | ||
| if (msg.sender != zoneFactory.messenger()) { | ||
| if (msg.sender != ZONE_MESSENGER_ADDRESS) { |
There was a problem hiding this comment.
Same thing here, this will break if we redeploy contracts on testnet until the precompiles are live. Can we update to remove this (and related changes) since we will need to redeploy on testnet before the precompiles are live?
3 participants
Summary
The Solidity ZoneFactory remains a deployable local/test stand-in. Tempo's native precompile is responsible for vanity portal addresses and ERC-1167 proxy etching.
This incorporates the delegatecall initialization guard proposed in #656.
TIP: tempoxyz/tempo#6787
Validation