Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: simplify registry #7939

Merged
merged 1 commit into from
Aug 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ title: Registry
tags: [portals, contracts]
---

The registry is a contract deployed on L1, that contains addresses for the `Rollup`, `Inbox` and `Outbox`. It also keeps track of the different versions that have been deployed and let you query prior deployments easily.
The registry is a contract deployed on L1, that contains addresses for the `Rollup`. It also keeps track of the different versions that have been deployed and let you query prior deployments easily.

**Links**: [Interface (GitHub link)](https://github.com/AztecProtocol/aztec-packages/blob/master/l1-contracts/src/core/interfaces/messagebridge/IRegistry.sol), [Implementation (GitHub link)](https://github.com/AztecProtocol/aztec-packages/blob/master/l1-contracts/src/core/messagebridge/Registry.sol).

Expand All @@ -26,26 +26,6 @@ Retrieves the current rollup contract.
| -------------- | ----------- |
| ReturnValue | The current rollup |

## `getInbox()`

Retrieves the current inbox contract.

#include_code registry_get_inbox l1-contracts/src/core/interfaces/messagebridge/IRegistry.sol solidity

| Name | Description |
| -------------- | ----------- |
| ReturnValue | The current Inbox |

## `getOutbox()`

Retrieves the current inbox contract.

#include_code registry_get_outbox l1-contracts/src/core/interfaces/messagebridge/IRegistry.sol solidity

| Name | Description |
| -------------- | ----------- |
| ReturnValue | The current Outbox |

## `getVersionFor(address _rollup)`

Retrieve the version of a specific rollup contract.
Expand All @@ -71,8 +51,6 @@ Retrieve the snapshot of a specific version.
| -------------- | ----------- |
| `_version` | The version number to fetch data for |
| ReturnValue.rollup | The address of the `rollup` for the `_version` |
| ReturnValue.inbox | The address of the `inbox` for the `_version` |
| ReturnValue.outbox | The address of the `outbox` for the `_version` |
| ReturnValue.blockNumber | The block number of the snapshot creation |


Expand All @@ -85,7 +63,5 @@ Retrieves the snapshot for the current version.
| Name | Description |
| -------------- | ----------- |
| ReturnValue.rollup | The address of the `rollup` for the current `_version` |
| ReturnValue.inbox | The address of the `inbox` for the current `_version` |
| ReturnValue.outbox | The address of the `outbox` for the current `_version` |
| ReturnValue.blockNumber | The block number of the snapshot creation |

7 changes: 7 additions & 0 deletions l1-contracts/src/core/interfaces/IRollup.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
// Copyright 2023 Aztec Labs.
pragma solidity >=0.8.18;

import {IInbox} from "../interfaces/messagebridge/IInbox.sol";
import {IOutbox} from "../interfaces/messagebridge/IOutbox.sol";

import {SignatureLib} from "../sequencer_selection/SignatureLib.sol";

interface ITestRollup {
Expand All @@ -15,6 +18,10 @@ interface IRollup {
event L2ProofVerified(uint256 indexed blockNumber, bytes32 indexed proverId);
event ProgressedState(uint256 provenBlockCount, uint256 pendingBlockCount);

function INBOX() external view returns (IInbox);

function OUTBOX() external view returns (IOutbox);

function publishAndProcess(
bytes calldata _header,
bytes32 _archive,
Expand Down
12 changes: 1 addition & 11 deletions l1-contracts/src/core/interfaces/messagebridge/IRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,10 @@ pragma solidity >=0.8.18;

import {DataStructures} from "../../libraries/DataStructures.sol";
import {IRollup} from "../IRollup.sol";
import {IInbox} from "./IInbox.sol";
import {IOutbox} from "./IOutbox.sol";

interface IRegistry {
// docs:start:registry_upgrade
function upgrade(address _rollup, address _inbox, address _outbox) external returns (uint256);
function upgrade(address _rollup) external returns (uint256);
// docs:end:registry_upgrade

// docs:start:registry_get_rollup
Expand All @@ -19,14 +17,6 @@ interface IRegistry {
function getVersionFor(address _rollup) external view returns (uint256);
// docs:end:registry_get_version_for

// docs:start:registry_get_inbox
function getInbox() external view returns (IInbox);
// docs:end:registry_get_inbox

// docs:start:registry_get_outbox
function getOutbox() external view returns (IOutbox);
// docs:end:registry_get_outbox

// docs:start:registry_get_snapshot
function getSnapshot(uint256 _version)
external
Expand Down
4 changes: 0 additions & 4 deletions l1-contracts/src/core/libraries/DataStructures.sol
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,10 @@ library DataStructures {
/**
* @notice Struct for storing address of cross communication components and the block number when it was updated
* @param rollup - The address of the rollup contract
* @param inbox - The address of the inbox contract
* @param outbox - The address of the outbox contract
* @param blockNumber - The block number of the snapshot
*/
struct RegistrySnapshot {
address rollup;
address inbox;
address outbox;
uint256 blockNumber;
}
// docs:end:registry_snapshot
Expand Down
37 changes: 5 additions & 32 deletions l1-contracts/src/core/messagebridge/Registry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import {Ownable} from "@oz/access/Ownable.sol";
// Interfaces
import {IRegistry} from "../interfaces/messagebridge/IRegistry.sol";
import {IRollup} from "../interfaces/IRollup.sol";
import {IInbox} from "../interfaces/messagebridge/IInbox.sol";
import {IOutbox} from "../interfaces/messagebridge/IOutbox.sol";

// Libraries
import {DataStructures} from "../libraries/DataStructures.sol";
Expand All @@ -17,9 +15,7 @@ import {Errors} from "../libraries/Errors.sol";
/**
* @title Registry
* @author Aztec Labs
* @notice Keeps track of addresses of rollup, inbox and outbox as well as historical addresses.
* Used as the source of truth for finding the "head" of the rollup chain. Very important information
* for L1<->L2 communication.
* @notice Keeps track of addresses of current rollup and historical addresses.
*/
contract Registry is IRegistry, Ownable {
uint256 public override(IRegistry) numberOfVersions;
Expand All @@ -29,9 +25,9 @@ contract Registry is IRegistry, Ownable {
mapping(address rollup => uint256 version) internal rollupToVersion;

constructor() Ownable(msg.sender) {
// Inserts a "dead" rollup and message boxes at version 0
// Inserts a "dead" rollup at version 0
// This is simply done to make first version 1, which fits better with the rest of the system
upgrade(address(0xdead), address(0xdead), address(0xdead));
upgrade(address(0xdead));
}

/**
Expand All @@ -53,22 +49,6 @@ contract Registry is IRegistry, Ownable {
return version;
}

/**
* @notice Returns the inbox contract
* @return The inbox contract (of type IInbox)
*/
function getInbox() external view override(IRegistry) returns (IInbox) {
return IInbox(currentSnapshot.inbox);
}

/**
* @notice Returns the outbox contract
* @return The outbox contract (of type IOutbox)
*/
function getOutbox() external view override(IRegistry) returns (IOutbox) {
return IOutbox(currentSnapshot.outbox);
}

/**
* @notice Fetches a snapshot of the registry indicated by `version`
* @dev the version is 0 indexed, so the first snapshot is version 0.
Expand Down Expand Up @@ -104,21 +84,14 @@ contract Registry is IRegistry, Ownable {
* @dev Reverts if the rollup is already registered
*
* @param _rollup - The address of the rollup contract
* @param _inbox - The address of the inbox contract
* @param _outbox - The address of the outbox contract
* @return The version of the new snapshot
*/
function upgrade(address _rollup, address _inbox, address _outbox)
public
override(IRegistry)
onlyOwner
returns (uint256)
{
function upgrade(address _rollup) public override(IRegistry) onlyOwner returns (uint256) {
(, bool exists) = _getVersionFor(_rollup);
if (exists) revert Errors.Registry__RollupAlreadyRegistered(_rollup);

DataStructures.RegistrySnapshot memory newSnapshot =
DataStructures.RegistrySnapshot(_rollup, _inbox, _outbox, block.number);
DataStructures.RegistrySnapshot(_rollup, block.number);
currentSnapshot = newSnapshot;
uint256 version = numberOfVersions++;
snapshots[version] = newSnapshot;
Expand Down
18 changes: 4 additions & 14 deletions l1-contracts/test/Registry.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -22,40 +22,30 @@ contract RegistryTest is Test {
assertEq(registry.numberOfVersions(), 1, "should have 1 version");
DataStructures.RegistrySnapshot memory snapshot = registry.getCurrentSnapshot();
assertEq(snapshot.rollup, DEAD, "should have dead rollup");
assertEq(snapshot.inbox, DEAD, "should have dead inbox");
assertEq(snapshot.outbox, DEAD, "should have dead outbox");
assertEq(address(registry.getRollup()), DEAD);
assertEq(address(registry.getInbox()), DEAD);
assertEq(address(registry.getOutbox()), DEAD);

vm.expectRevert(abi.encodeWithSelector(Errors.Registry__RollupNotRegistered.selector, DEAD));
registry.getVersionFor(DEAD);
}

function testUpgrade() public {
address newRollup = address(0xbeef1);
address inbox = address(0xbeef2);
address newOutbox = address(0xbeef3);
uint256 newVersion = registry.upgrade(newRollup, inbox, newOutbox);
uint256 newVersion = registry.upgrade(newRollup);

assertEq(registry.numberOfVersions(), 2, "should have 2 versions");
DataStructures.RegistrySnapshot memory snapshot = registry.getCurrentSnapshot();
assertEq(snapshot.rollup, newRollup, "should have newRollup");
assertEq(snapshot.inbox, inbox, "should have inbox");
assertEq(snapshot.outbox, newOutbox, "should have newOutbox");

assertEq(address(registry.getRollup()), newRollup);
assertEq(address(registry.getInbox()), inbox);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it worth keeping these assertions in the test, but just calling the INBOX() on the new rollup?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think it make more sense to do related to #7959 then.

assertEq(address(registry.getOutbox()), newOutbox);
assertEq(
registry.getVersionFor(newRollup), newVersion, "should have version newVersion for newRollup"
);
}

function testRevertUpgradeToSame() public {
registry.upgrade(DEAD, DEAD, DEAD);
registry.upgrade(DEAD);
vm.expectRevert(abi.encodeWithSelector(Errors.Registry__RollupAlreadyRegistered.selector, DEAD));
registry.upgrade(DEAD, DEAD, DEAD);
registry.upgrade(DEAD);
}

function testRevertGetVersionForNonExistent() public {
Expand All @@ -68,6 +58,6 @@ contract RegistryTest is Test {
vm.assume(_owner != registry.owner());
vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, _owner));
vm.prank(_owner);
registry.upgrade(DEAD, DEAD, DEAD);
registry.upgrade(DEAD);
}
}
2 changes: 1 addition & 1 deletion l1-contracts/test/Rollup.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ contract RollupTest is DecoderBase {
inbox = Inbox(address(rollup.INBOX()));
outbox = Outbox(address(rollup.OUTBOX()));

registry.upgrade(address(rollup), address(inbox), address(outbox));
registry.upgrade(address(rollup));

// mint some tokens to the rollup
portalERC20.mint(address(rollup), 1000000);
Expand Down
2 changes: 1 addition & 1 deletion l1-contracts/test/portals/FeeJuicePortal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ contract FeeJuicePortal {
returns (bytes32)
{
// Preamble
IInbox inbox = registry.getInbox();
IInbox inbox = registry.getRollup().INBOX();
DataStructures.L2Actor memory actor = DataStructures.L2Actor(l2TokenAddress, 1);

// Hash the message content to be reconstructed in the receiving contract
Expand Down
6 changes: 3 additions & 3 deletions l1-contracts/test/portals/TokenPortal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ contract TokenPortal {
returns (bytes32)
{
// Preamble
IInbox inbox = registry.getInbox();
IInbox inbox = registry.getRollup().INBOX();
DataStructures.L2Actor memory actor = DataStructures.L2Actor(l2Bridge, 1);

// Hash the message content to be reconstructed in the receiving contract
Expand Down Expand Up @@ -69,7 +69,7 @@ contract TokenPortal {
bytes32 _secretHashForL2MessageConsumption
) external returns (bytes32) {
// Preamble
IInbox inbox = registry.getInbox();
IInbox inbox = registry.getRollup().INBOX();
DataStructures.L2Actor memory actor = DataStructures.L2Actor(l2Bridge, 1);

// Hash the message content to be reconstructed in the receiving contract
Expand Down Expand Up @@ -120,7 +120,7 @@ contract TokenPortal {
)
});

IOutbox outbox = registry.getOutbox();
IOutbox outbox = registry.getRollup().OUTBOX();

outbox.consume(message, _l2BlockNumber, _leafIndex, _path);

Expand Down
2 changes: 1 addition & 1 deletion l1-contracts/test/portals/TokenPortal.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ contract TokenPortalTest is Test {
inbox = rollup.INBOX();
outbox = rollup.OUTBOX();

registry.upgrade(address(rollup), address(inbox), address(outbox));
registry.upgrade(address(rollup));

portalERC20.mint(address(rollup), 1000000);
tokenPortal = new TokenPortal();
Expand Down
4 changes: 2 additions & 2 deletions l1-contracts/test/portals/UniswapPortal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ contract UniswapPortal {

// Consume the message from the outbox
{
IOutbox outbox = registry.getOutbox();
IOutbox outbox = registry.getRollup().OUTBOX();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, this adds the gas cost of another external call? will we end up going back on this change / adding a method to the registry anyway when golfing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It needs to be fixed as part of #7959.


outbox.consume(
DataStructures.L2ToL1Msg({
Expand Down Expand Up @@ -209,7 +209,7 @@ contract UniswapPortal {

// Consume the message from the outbox
{
IOutbox outbox = registry.getOutbox();
IOutbox outbox = registry.getRollup().OUTBOX();

outbox.consume(
DataStructures.L2ToL1Msg({
Expand Down
10 changes: 5 additions & 5 deletions l1-contracts/test/portals/UniswapPortal.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ contract UniswapPortalTest is Test {

Rollup internal rollup;
Registry internal registry;

IOutbox internal outbox;
bytes32 internal l2TokenAddress = bytes32(uint256(0x1));
bytes32 internal l2UniswapAddress = bytes32(uint256(0x2));

Expand All @@ -54,7 +56,7 @@ contract UniswapPortalTest is Test {
PortalERC20 portalERC20 = new PortalERC20();
rollup =
new Rollup(registry, new AvailabilityOracle(), IERC20(address(portalERC20)), bytes32(0));
registry.upgrade(address(rollup), address(rollup.INBOX()), address(rollup.OUTBOX()));
registry.upgrade(address(rollup));
portalERC20.mint(address(rollup), 1000000);

daiTokenPortal = new TokenPortal();
Expand All @@ -68,6 +70,8 @@ contract UniswapPortalTest is Test {

// have DAI locked in portal that can be moved when funds are withdrawn
deal(address(DAI), address(daiTokenPortal), amount);

outbox = rollup.OUTBOX();
}

/**
Expand Down Expand Up @@ -169,8 +173,6 @@ contract UniswapPortalTest is Test {
(bytes32[] memory withdrawSiblingPath,) = tree.computeSiblingPath(0);
(bytes32[] memory swapSiblingPath,) = tree.computeSiblingPath(1);

IOutbox outbox = registry.getOutbox();

vm.prank(address(rollup));
outbox.insert(_l2BlockNumber, treeRoot, treeHeight);

Expand Down Expand Up @@ -392,7 +394,6 @@ contract UniswapPortalTest is Test {
// there should be some weth in the weth portal
assertGt(WETH9.balanceOf(address(wethTokenPortal)), 0);
// there the message should be nullified at index 0 and 1
IOutbox outbox = registry.getOutbox();
assertTrue(outbox.hasMessageBeenConsumedAtBlockAndIndex(l2BlockNumber, 0));
assertTrue(outbox.hasMessageBeenConsumedAtBlockAndIndex(l2BlockNumber, 1));
}
Expand Down Expand Up @@ -440,7 +441,6 @@ contract UniswapPortalTest is Test {
// there should be some weth in the weth portal
assertGt(WETH9.balanceOf(address(wethTokenPortal)), 0);
// there should be no message in the outbox:
IOutbox outbox = registry.getOutbox();
assertTrue(outbox.hasMessageBeenConsumedAtBlockAndIndex(l2BlockNumber, 0));
assertTrue(outbox.hasMessageBeenConsumedAtBlockAndIndex(l2BlockNumber, 1));
}
Expand Down
2 changes: 1 addition & 1 deletion l1-contracts/test/sparta/DevNet.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ contract DevNetTest is DecoderBase {
inbox = Inbox(address(rollup.INBOX()));
outbox = Outbox(address(rollup.OUTBOX()));

registry.upgrade(address(rollup), address(inbox), address(outbox));
registry.upgrade(address(rollup));

// mint some tokens to the rollup
portalERC20.mint(address(rollup), 1000000);
Expand Down
2 changes: 1 addition & 1 deletion l1-contracts/test/sparta/Sparta.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ contract SpartaTest is DecoderBase {
inbox = Inbox(address(rollup.INBOX()));
outbox = Outbox(address(rollup.OUTBOX()));

registry.upgrade(address(rollup), address(inbox), address(outbox));
registry.upgrade(address(rollup));

// mint some tokens to the rollup
portalERC20.mint(address(rollup), 1000000);
Expand Down
Loading
Loading