diff --git a/docs/docs/reference/developer_references/smart_contract_reference/portals/data_structures.md b/docs/docs/reference/developer_references/smart_contract_reference/portals/data_structures.md index 662e409c124..57c16ca3367 100644 --- a/docs/docs/reference/developer_references/smart_contract_reference/portals/data_structures.md +++ b/docs/docs/reference/developer_references/smart_contract_reference/portals/data_structures.md @@ -58,13 +58,11 @@ A message that is sent from L2 to L1. A snapshot of the registry values. -#include_code registry_snapshot l1-contracts/src/core/libraries/DataStructures.sol solidity +#include_code registry_snapshot l1-contracts/src/governance/libraries/DataStructures.sol solidity | Name | Type | Description | | -------------- | ------- | ----------- | | `rollup` | `address` | The address of the rollup contract for the snapshot. | -| `inbox` | `address` | The address of the inbox contract for the snapshot. | -| `outbox` | `address` | The address of the outbox contract for the snapshot. | | `blockNumber` | `uint256` | The block number at which the snapshot was created. | diff --git a/docs/docs/reference/developer_references/smart_contract_reference/portals/registry.md b/docs/docs/reference/developer_references/smart_contract_reference/portals/registry.md index 337fd7e4bad..1a129fe83c7 100644 --- a/docs/docs/reference/developer_references/smart_contract_reference/portals/registry.md +++ b/docs/docs/reference/developer_references/smart_contract_reference/portals/registry.md @@ -5,13 +5,13 @@ tags: [portals, contracts] 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). +**Links**: [Interface (GitHub link)](https://github.com/AztecProtocol/aztec-packages/blob/master/l1-contracts/src/governance/interfaces/IRegistry.sol), [Implementation (GitHub link)](https://github.com/AztecProtocol/aztec-packages/blob/master/l1-contracts/src/governance/Registry.sol). ## `numberOfVersions()` Retrieves the number of versions that have been deployed. -#include_code registry_number_of_versions l1-contracts/src/core/interfaces/messagebridge/IRegistry.sol solidity +#include_code registry_number_of_versions l1-contracts/src/governance/interfaces/IRegistry.sol solidity | Name | Description | | -------------- | ----------- | @@ -20,7 +20,7 @@ Retrieves the number of versions that have been deployed. ## `getRollup()` Retrieves the current rollup contract. -#include_code registry_get_rollup l1-contracts/src/core/interfaces/messagebridge/IRegistry.sol solidity +#include_code registry_get_rollup l1-contracts/src/governance/interfaces/IRegistry.sol solidity | Name | Description | | -------------- | ----------- | @@ -28,9 +28,9 @@ Retrieves the current rollup contract. ## `getVersionFor(address _rollup)` -Retrieve the version of a specific rollup contract. +Retrieve the version of a specific rollup contract. -#include_code registry_get_version_for l1-contracts/src/core/interfaces/messagebridge/IRegistry.sol solidity +#include_code registry_get_version_for l1-contracts/src/governance/interfaces/IRegistry.sol solidity | Name | Description | | -------------- | ----------- | @@ -42,10 +42,10 @@ Will revert with `Registry__RollupNotRegistered(_rollup)` if the rollup have not ## `getSnapshot(uint256 _version)` -Retrieve the snapshot of a specific version. +Retrieve the snapshot of a specific version. -#include_code registry_snapshot l1-contracts/src/core/libraries/DataStructures.sol solidity -#include_code registry_get_snapshot l1-contracts/src/core/interfaces/messagebridge/IRegistry.sol solidity +#include_code registry_snapshot l1-contracts/src/governance/libraries/DataStructures.sol solidity +#include_code registry_get_snapshot l1-contracts/src/governance/interfaces/IRegistry.sol solidity | Name | Description | | -------------- | ----------- | @@ -58,7 +58,7 @@ Retrieve the snapshot of a specific version. Retrieves the snapshot for the current version. -#include_code registry_get_current_snapshot l1-contracts/src/core/interfaces/messagebridge/IRegistry.sol solidity +#include_code registry_get_current_snapshot l1-contracts/src/governance/interfaces/IRegistry.sol solidity | Name | Description | | -------------- | ----------- | diff --git a/l1-contracts/README.md b/l1-contracts/README.md index ddae20c94ed..149ae82027c 100644 --- a/l1-contracts/README.md +++ b/l1-contracts/README.md @@ -56,6 +56,29 @@ Currently not running any proofs _nor_ access control so blocks can be submitted --- +# Branching Tree Technique + +For writing tests we will be using the [Branching Tree Technique](https://www.youtube.com/watch?v=0-EmbNVgFA4). +The approach is simple, for a function that is to be tested (all functions) you should write a `.tree` file first. +Then the tree file can be used to generate a solidity tests file using [Bulloak](https://github.com/alexfertel/bulloak) by running the `scaffold` command. + +```bash +bulloak scaffold -w **/*.tree +``` + +To check that the tests are following the expected format, you can run the `check` command. + +```bash +bulloak check **/*.tree +``` + +We assume that you already have `bulloak` installed, if not you can install it as `cargo install bulloak`. +Also, we suggest using [Ascii Tree Generator](https://marketplace.visualstudio.com/items?itemName=aprilandjan.ascii-tree-generator), since the pipes can be a bit of a pain otherwise. + +For examples, see the tests for the registry. + +--- + # Linter We use an extended version of solhint (https://github.com/LHerskind/solhint) to include custom rules. These custom rules relate to how errors should be named, using custom errors instead of reverts etc, see `.solhint.json` for more specifics about the rules. @@ -73,12 +96,13 @@ yarn lint # Slither & Slitherin We use slither as an automatic way to find blunders and common vulnerabilities in our contracts. It is not part of the docker image due to its slowness, but it can be run using the following command to generate a markdown file with the results: + ```bash yarn slither ``` -When this command is run in CI, it will fail if the markdown file generated in docker don't match the one in the repository. +When this command is run in CI, it will fail if the markdown file generated in docker don't match the one in the repository. We assume that you already have slither installed. You can install it with `pip3 install slither-analyzer==0.10.0 slitherin==0.5.0`. It is kept out of the bootstrap script as it is not a requirement for people who just want to run tests or are uninterested in the contracts. -> We are not running the `naming-convention` detector because we have our own rules for naming which is enforced by the linter. \ No newline at end of file +> We are not running the `naming-convention` detector because we have our own rules for naming which is enforced by the linter. diff --git a/l1-contracts/src/core/FeeJuicePortal.sol b/l1-contracts/src/core/FeeJuicePortal.sol index 55443034163..f3210d008c3 100644 --- a/l1-contracts/src/core/FeeJuicePortal.sol +++ b/l1-contracts/src/core/FeeJuicePortal.sol @@ -5,7 +5,8 @@ pragma solidity >=0.8.27; import {IERC20} from "@oz/token/ERC20/IERC20.sol"; import {IFeeJuicePortal} from "@aztec/core/interfaces/IFeeJuicePortal.sol"; import {IInbox} from "@aztec/core/interfaces/messagebridge/IInbox.sol"; -import {IRegistry} from "@aztec/core/interfaces/messagebridge/IRegistry.sol"; +import {IRegistry} from "@aztec/governance/interfaces/IRegistry.sol"; +import {IRollup} from "@aztec/core/interfaces/IRollup.sol"; import {Constants} from "@aztec/core/libraries/ConstantsGen.sol"; import {DataStructures} from "@aztec/core/libraries/DataStructures.sol"; @@ -74,7 +75,7 @@ contract FeeJuicePortal is IFeeJuicePortal, Ownable { returns (bytes32) { // Preamble - IInbox inbox = registry.getRollup().INBOX(); + IInbox inbox = IRollup(registry.getRollup()).INBOX(); DataStructures.L2Actor memory actor = DataStructures.L2Actor(l2TokenAddress, 1); // Hash the message content to be reconstructed in the receiving contract diff --git a/l1-contracts/src/core/Rollup.sol b/l1-contracts/src/core/Rollup.sol index 43f2d75ac6b..6a45af67aec 100644 --- a/l1-contracts/src/core/Rollup.sol +++ b/l1-contracts/src/core/Rollup.sol @@ -6,7 +6,6 @@ import {IProofCommitmentEscrow} from "@aztec/core/interfaces/IProofCommitmentEsc import {IInbox} from "@aztec/core/interfaces/messagebridge/IInbox.sol"; import {IOutbox} from "@aztec/core/interfaces/messagebridge/IOutbox.sol"; import {IFeeJuicePortal} from "@aztec/core/interfaces/IFeeJuicePortal.sol"; -import {IRegistry} from "@aztec/core/interfaces/messagebridge/IRegistry.sol"; import {IRollup, ITestRollup} from "@aztec/core/interfaces/IRollup.sol"; import {IVerifier} from "@aztec/core/interfaces/IVerifier.sol"; @@ -56,7 +55,6 @@ contract Rollup is Leonidas, IRollup, ITestRollup { uint256 public constant PROOF_COMMITMENT_MIN_BOND_AMOUNT_IN_TST = 1000; uint256 public immutable L1_BLOCK_AT_GENESIS; - IRegistry public immutable REGISTRY; IInbox public immutable INBOX; IOutbox public immutable OUTBOX; IProofCommitmentEscrow public immutable PROOF_COMMITMENT_ESCROW; @@ -85,7 +83,6 @@ contract Rollup is Leonidas, IRollup, ITestRollup { IVerifier public epochProofVerifier; constructor( - IRegistry _registry, IFeeJuicePortal _fpcJuicePortal, bytes32 _vkTreeRoot, address _ares, @@ -93,7 +90,6 @@ contract Rollup is Leonidas, IRollup, ITestRollup { ) Leonidas(_ares) { blockProofVerifier = new MockVerifier(); epochProofVerifier = new MockVerifier(); - REGISTRY = _registry; FEE_JUICE_PORTAL = _fpcJuicePortal; PROOF_COMMITMENT_ESCROW = new MockProofCommitmentEscrow(); INBOX = IInbox(address(new Inbox(address(this), Constants.L1_TO_L2_MSG_SUBTREE_HEIGHT))); diff --git a/l1-contracts/src/core/libraries/DataStructures.sol b/l1-contracts/src/core/libraries/DataStructures.sol index 13911e4d7c2..8827cededee 100644 --- a/l1-contracts/src/core/libraries/DataStructures.sol +++ b/l1-contracts/src/core/libraries/DataStructures.sol @@ -67,18 +67,6 @@ library DataStructures { } // docs:end:l2_to_l1_msg - // docs:start:registry_snapshot - /** - * @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 blockNumber - The block number of the snapshot - */ - struct RegistrySnapshot { - address rollup; - uint256 blockNumber; - } - // docs:end:registry_snapshot - /** * @notice Struct for storing flags for block header validation * @param ignoreDA - True will ignore DA check, otherwise checks diff --git a/l1-contracts/src/core/libraries/Errors.sol b/l1-contracts/src/core/libraries/Errors.sol index cc24c729720..a6b3d8a68af 100644 --- a/l1-contracts/src/core/libraries/Errors.sol +++ b/l1-contracts/src/core/libraries/Errors.sol @@ -72,10 +72,6 @@ library Errors { error Rollup__TryingToProveNonExistingBlock(); // 0x34ef4954 error Rollup__UnavailableTxs(bytes32 txsHash); // 0x414906c3 - // Registry - error Registry__RollupNotRegistered(address rollup); // 0xa1fee4cf - error Registry__RollupAlreadyRegistered(address rollup); // 0x3c34eabf - //TxsDecoder error TxsDecoder__InvalidLogsLength(uint256 expected, uint256 actual); // 0x829ca981 error TxsDecoder__TxsTooLarge(uint256 expected, uint256 actual); // 0xc7d44a62 diff --git a/l1-contracts/src/core/messagebridge/Registry.sol b/l1-contracts/src/governance/Registry.sol similarity index 88% rename from l1-contracts/src/core/messagebridge/Registry.sol rename to l1-contracts/src/governance/Registry.sol index c0c0c6aed08..b734e28c33e 100644 --- a/l1-contracts/src/core/messagebridge/Registry.sol +++ b/l1-contracts/src/governance/Registry.sol @@ -2,11 +2,10 @@ // Copyright 2023 Aztec Labs. pragma solidity >=0.8.27; -import {IRegistry} from "@aztec/core/interfaces/messagebridge/IRegistry.sol"; -import {IRollup} from "@aztec/core/interfaces/IRollup.sol"; +import {IRegistry} from "@aztec/governance/interfaces/IRegistry.sol"; -import {DataStructures} from "@aztec/core/libraries/DataStructures.sol"; -import {Errors} from "@aztec/core/libraries/Errors.sol"; +import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol"; +import {Errors} from "@aztec/governance/libraries/Errors.sol"; import {Ownable} from "@oz/access/Ownable.sol"; @@ -29,11 +28,11 @@ contract Registry is IRegistry, Ownable { } /** - * @notice Returns the rollup contract - * @return The rollup contract (of type IRollup) + * @notice Returns the address of the rollup contract + * @return The rollup address */ - function getRollup() external view override(IRegistry) returns (IRollup) { - return IRollup(currentSnapshot.rollup); + function getRollup() external view override(IRegistry) returns (address) { + return currentSnapshot.rollup; } /** @@ -109,11 +108,13 @@ contract Registry is IRegistry, Ownable { snapshots[version] = newSnapshot; rollupToVersion[_rollup] = version; + emit InstanceAdded(_rollup, version); + return version; } function _getVersionFor(address _rollup) internal view returns (uint256 version, bool exists) { version = rollupToVersion[_rollup]; - return (version, version > 0); + return (version, version > 0 || snapshots[0].rollup == _rollup); } } diff --git a/l1-contracts/src/core/interfaces/messagebridge/IRegistry.sol b/l1-contracts/src/governance/interfaces/IRegistry.sol similarity index 83% rename from l1-contracts/src/core/interfaces/messagebridge/IRegistry.sol rename to l1-contracts/src/governance/interfaces/IRegistry.sol index c20d0868390..6c9fd5d1a45 100644 --- a/l1-contracts/src/core/interfaces/messagebridge/IRegistry.sol +++ b/l1-contracts/src/governance/interfaces/IRegistry.sol @@ -1,16 +1,17 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity >=0.8.27; -import {DataStructures} from "../../libraries/DataStructures.sol"; -import {IRollup} from "../IRollup.sol"; +import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol"; interface IRegistry { + event InstanceAdded(address indexed instance, uint256 indexed version); + // docs:start:registry_upgrade function upgrade(address _rollup) external returns (uint256); // docs:end:registry_upgrade // docs:start:registry_get_rollup - function getRollup() external view returns (IRollup); + function getRollup() external view returns (address); // docs:end:registry_get_rollup // docs:start:registry_get_version_for diff --git a/l1-contracts/src/governance/libraries/DataStructures.sol b/l1-contracts/src/governance/libraries/DataStructures.sol new file mode 100644 index 00000000000..b631facb96b --- /dev/null +++ b/l1-contracts/src/governance/libraries/DataStructures.sol @@ -0,0 +1,22 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2023 Aztec Labs. +pragma solidity >=0.8.18; + +/** + * @title Data Structures Library + * @author Aztec Labs + * @notice Library that contains data structures used throughout Aztec governance + */ +library DataStructures { + // docs:start:registry_snapshot + /** + * @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 blockNumber - The block number of the snapshot + */ + struct RegistrySnapshot { + address rollup; + uint256 blockNumber; + } + // docs:end:registry_snapshot +} diff --git a/l1-contracts/src/governance/libraries/Errors.sol b/l1-contracts/src/governance/libraries/Errors.sol new file mode 100644 index 00000000000..c9bf0cba3e0 --- /dev/null +++ b/l1-contracts/src/governance/libraries/Errors.sol @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2023 Aztec Labs. +pragma solidity >=0.8.18; + +/** + * @title Errors Library + * @author Aztec Labs + * @notice Library that contains errors used throughout the Aztec governance + * Errors are prefixed with the contract name to make it easy to identify where the error originated + * when there are multiple contracts that could have thrown the error. + */ +library Errors { + // Registry + error Registry__RollupNotRegistered(address rollup); // 0xa1fee4cf + error Registry__RollupAlreadyRegistered(address rollup); // 0x3c34eabf +} diff --git a/l1-contracts/test/Registry.t.sol b/l1-contracts/test/Registry.t.sol deleted file mode 100644 index d93192fab7c..00000000000 --- a/l1-contracts/test/Registry.t.sol +++ /dev/null @@ -1,62 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// Copyright 2023 Aztec Labs. -pragma solidity >=0.8.27; - -import {Test} from "forge-std/Test.sol"; -import {Ownable} from "@oz/access/Ownable.sol"; -import {Registry} from "@aztec/core/messagebridge/Registry.sol"; -import {Errors} from "@aztec/core/libraries/Errors.sol"; -import {DataStructures} from "@aztec/core/libraries/DataStructures.sol"; - -contract RegistryTest is Test { - address internal constant DEAD = address(0xdead); - - Registry internal registry; - - function setUp() public { - registry = new Registry(address(this)); - } - - function testConstructorSetup() public { - assertEq(registry.numberOfVersions(), 1, "should have 1 version"); - DataStructures.RegistrySnapshot memory snapshot = registry.getCurrentSnapshot(); - assertEq(snapshot.rollup, DEAD, "should have dead rollup"); - assertEq(address(registry.getRollup()), DEAD); - - vm.expectRevert(abi.encodeWithSelector(Errors.Registry__RollupNotRegistered.selector, DEAD)); - registry.getVersionFor(DEAD); - } - - function testUpgrade() public { - address newRollup = address(0xbeef1); - 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(address(registry.getRollup()), newRollup); - assertEq( - registry.getVersionFor(newRollup), newVersion, "should have version newVersion for newRollup" - ); - } - - function testRevertUpgradeToSame() public { - registry.upgrade(DEAD); - vm.expectRevert(abi.encodeWithSelector(Errors.Registry__RollupAlreadyRegistered.selector, DEAD)); - registry.upgrade(DEAD); - } - - function testRevertGetVersionForNonExistent() public { - address rollup = address(0xbeef1); - vm.expectRevert(abi.encodeWithSelector(Errors.Registry__RollupNotRegistered.selector, rollup)); - registry.getVersionFor(rollup); - } - - function testRevertUpgradeAsNonOwner(address _owner) public { - vm.assume(_owner != registry.owner()); - vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, _owner)); - vm.prank(_owner); - registry.upgrade(DEAD); - } -} diff --git a/l1-contracts/test/Rollup.t.sol b/l1-contracts/test/Rollup.t.sol index f1ea19b3daf..cc26edb8257 100644 --- a/l1-contracts/test/Rollup.t.sol +++ b/l1-contracts/test/Rollup.t.sol @@ -8,12 +8,11 @@ import {DataStructures} from "@aztec/core/libraries/DataStructures.sol"; import {Constants} from "@aztec/core/libraries/ConstantsGen.sol"; import {SignatureLib} from "@aztec/core/libraries/crypto/SignatureLib.sol"; -import {Registry} from "@aztec/core/messagebridge/Registry.sol"; +import {Registry} from "@aztec/governance/Registry.sol"; import {Inbox} from "@aztec/core/messagebridge/Inbox.sol"; import {Outbox} from "@aztec/core/messagebridge/Outbox.sol"; import {Errors} from "@aztec/core/libraries/Errors.sol"; import {Rollup} from "@aztec/core/Rollup.sol"; -import {IFeeJuicePortal} from "@aztec/core/interfaces/IFeeJuicePortal.sol"; import {IRollup} from "@aztec/core/interfaces/IRollup.sol"; import {FeeJuicePortal} from "@aztec/core/FeeJuicePortal.sol"; import {Leonidas} from "@aztec/core/Leonidas.sol"; @@ -69,13 +68,7 @@ contract RollupTest is DecoderBase { feeJuicePortal.initialize( address(registry), address(portalERC20), bytes32(Constants.FEE_JUICE_ADDRESS) ); - rollup = new Rollup( - registry, - IFeeJuicePortal(address(feeJuicePortal)), - bytes32(0), - address(this), - new address[](0) - ); + rollup = new Rollup(feeJuicePortal, bytes32(0), address(this), new address[](0)); inbox = Inbox(address(rollup.INBOX())); outbox = Outbox(address(rollup.OUTBOX())); diff --git a/l1-contracts/test/governance/registry/Base.t.sol b/l1-contracts/test/governance/registry/Base.t.sol new file mode 100644 index 00000000000..e76ef145355 --- /dev/null +++ b/l1-contracts/test/governance/registry/Base.t.sol @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.27; + +import {Test} from "forge-std/Test.sol"; + +import {Registry} from "@aztec/governance/Registry.sol"; + +contract RegistryBase is Test { + Registry internal registry; + + function setUp() public { + registry = new Registry(address(this)); + } +} diff --git a/l1-contracts/test/governance/registry/getCurentSnapshotTest.t.sol b/l1-contracts/test/governance/registry/getCurentSnapshotTest.t.sol new file mode 100644 index 00000000000..4e9e1d652db --- /dev/null +++ b/l1-contracts/test/governance/registry/getCurentSnapshotTest.t.sol @@ -0,0 +1,27 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.27; + +import {RegistryBase} from "./Base.t.sol"; + +import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol"; + +contract GetCurrentSnapshotTest is RegistryBase { + function test_GivenOneListedRollup() external { + // it should return the newest + DataStructures.RegistrySnapshot memory snapshot = registry.getCurrentSnapshot(); + assertEq(snapshot.blockNumber, block.number); + assertEq(snapshot.rollup, address(0xdead)); + assertEq(registry.numberOfVersions(), 1); + } + + function test_GivenMultipleListedRollups() external { + // it should return the newest + address rollup = address(uint160(uint256(bytes32("new instance")))); + registry.upgrade(rollup); + + DataStructures.RegistrySnapshot memory snapshot = registry.getCurrentSnapshot(); + assertEq(snapshot.blockNumber, block.number); + assertGt(snapshot.blockNumber, 0); + assertEq(snapshot.rollup, rollup); + } +} diff --git a/l1-contracts/test/governance/registry/getCurentSnapshotTest.tree b/l1-contracts/test/governance/registry/getCurentSnapshotTest.tree new file mode 100644 index 00000000000..0b2a3b826a7 --- /dev/null +++ b/l1-contracts/test/governance/registry/getCurentSnapshotTest.tree @@ -0,0 +1,5 @@ +GetCurrentSnapshotTest +├── given one listed rollup +│ └── it should return the newest +└── given multiple listed rollups + └── it should return the newest \ No newline at end of file diff --git a/l1-contracts/test/governance/registry/getRollup.t.sol b/l1-contracts/test/governance/registry/getRollup.t.sol new file mode 100644 index 00000000000..28d336f32e1 --- /dev/null +++ b/l1-contracts/test/governance/registry/getRollup.t.sol @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.27; + +import {RegistryBase} from "./Base.t.sol"; + +contract GetRollupTest is RegistryBase { + function test_GivenOneListedRollup() external { + // it should return the newest + assertEq(registry.getRollup(), address(0xdead)); + } + + function test_GivenMultipleListedRollups() external { + // it should return the newest + address rollup = address(uint160(uint256(bytes32("new instance")))); + registry.upgrade(rollup); + assertEq(registry.getRollup(), rollup); + } +} diff --git a/l1-contracts/test/governance/registry/getRollup.tree b/l1-contracts/test/governance/registry/getRollup.tree new file mode 100644 index 00000000000..6064511d509 --- /dev/null +++ b/l1-contracts/test/governance/registry/getRollup.tree @@ -0,0 +1,5 @@ +GetRollupTest +├── given one listed rollup +│ └── it should return the newest +└── given multiple listed rollups + └── it should return the newest \ No newline at end of file diff --git a/l1-contracts/test/governance/registry/getSnapshot.t.sol b/l1-contracts/test/governance/registry/getSnapshot.t.sol new file mode 100644 index 00000000000..2c3ee39e8b7 --- /dev/null +++ b/l1-contracts/test/governance/registry/getSnapshot.t.sol @@ -0,0 +1,32 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.27; + +import {RegistryBase} from "./Base.t.sol"; + +import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol"; + +contract GetSnapshotTest is RegistryBase { + modifier givenMultipleListedRollups() { + _; + } + + function test_When_versionExists() external givenMultipleListedRollups { + // it should return the snapshot + + DataStructures.RegistrySnapshot memory snapshot = registry.getSnapshot(0); + assertEq(snapshot.blockNumber, block.number); + assertEq(snapshot.rollup, address(0xdead)); + assertEq(registry.numberOfVersions(), 1); + } + + function test_When_versionDoesNotExists(uint256 _version) external givenMultipleListedRollups { + // it should return empty snapshot + + uint256 version = bound(_version, 1, type(uint256).max); + + DataStructures.RegistrySnapshot memory snapshot = registry.getSnapshot(version); + assertEq(snapshot.blockNumber, 0); + assertEq(snapshot.rollup, address(0)); + assertEq(registry.numberOfVersions(), 1); + } +} diff --git a/l1-contracts/test/governance/registry/getSnapshot.tree b/l1-contracts/test/governance/registry/getSnapshot.tree new file mode 100644 index 00000000000..ed095a9473e --- /dev/null +++ b/l1-contracts/test/governance/registry/getSnapshot.tree @@ -0,0 +1,6 @@ +GetSnapshotTest +└── given multiple listed rollups + ├── when _version exists + │ └── it should return the snapshot + └── when _version does not exists + └── it should return empty snapshot \ No newline at end of file diff --git a/l1-contracts/test/governance/registry/getVersionFor.t.sol b/l1-contracts/test/governance/registry/getVersionFor.t.sol new file mode 100644 index 00000000000..e9a6a05b0c5 --- /dev/null +++ b/l1-contracts/test/governance/registry/getVersionFor.t.sol @@ -0,0 +1,47 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.27; + +import {RegistryBase} from "./Base.t.sol"; +import {Errors} from "@aztec/governance/libraries/Errors.sol"; + +contract GetVersionForTest is RegistryBase { + address internal rollup; + + modifier givenNoAdditionalListedRollups() { + _; + } + + function test_When_rollupIs0xdead() external givenNoAdditionalListedRollups { + // it should return 0 + assertEq(registry.getVersionFor(address(0xdead)), 0); + } + + function test_RevertWhen__rollupNot0xdead(address _rollup) + external + givenNoAdditionalListedRollups + { + // it should revert + vm.assume(_rollup != address(0xdead)); + vm.expectRevert(abi.encodeWithSelector(Errors.Registry__RollupNotRegistered.selector, _rollup)); + registry.getVersionFor(_rollup); + } + + modifier givenMultipleListedRollups() { + rollup = address(0xbeef); + registry.upgrade(rollup); + _; + } + + function test_When_rollupIsListed() external givenMultipleListedRollups { + // it should return the rollup version + assertEq(registry.getVersionFor(address(0xdead)), 0); + assertEq(registry.getVersionFor(address(0xbeef)), 1); + } + + function test_RevertWhen__rollupIsNotListed(address _rollup) external givenMultipleListedRollups { + // it should revert + vm.assume(_rollup != address(0xdead) && _rollup != address(0xbeef)); + vm.expectRevert(abi.encodeWithSelector(Errors.Registry__RollupNotRegistered.selector, _rollup)); + registry.getVersionFor(_rollup); + } +} diff --git a/l1-contracts/test/governance/registry/getVersionFor.tree b/l1-contracts/test/governance/registry/getVersionFor.tree new file mode 100644 index 00000000000..25e2d07b3aa --- /dev/null +++ b/l1-contracts/test/governance/registry/getVersionFor.tree @@ -0,0 +1,11 @@ +GetVersionForTest +├── given no additional listed rollups +│ ├── when _rollup is 0xdead +│ │ └── it should return 0 +│ └── when _rollup not 0xdead +│ └── it should revert +└── given multiple listed rollups + ├── when _rollup is listed + │ └── it should return the rollup version + └── when _rollup is not listed + └── it should revert \ No newline at end of file diff --git a/l1-contracts/test/governance/registry/isRollupRegistered.t.sol b/l1-contracts/test/governance/registry/isRollupRegistered.t.sol new file mode 100644 index 00000000000..b426dedf039 --- /dev/null +++ b/l1-contracts/test/governance/registry/isRollupRegistered.t.sol @@ -0,0 +1,41 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.27; + +import {RegistryBase} from "./Base.t.sol"; + +contract IsRollupRegisteredTest is RegistryBase { + address internal rollup; + + modifier givenNoAdditionalListedRollups() { + _; + } + + function test_When_rollupIs0xdead() external givenNoAdditionalListedRollups { + // it should return true + assertTrue(registry.isRollupRegistered(address(0xdead))); + } + + function test_When_rollupNot0xdead(address _rollup) external givenNoAdditionalListedRollups { + // it should return false + vm.assume(_rollup != address(0xdead)); + assertFalse(registry.isRollupRegistered(_rollup)); + } + + modifier givenMultipleListedRollups() { + rollup = address(0xbeef); + registry.upgrade(rollup); + _; + } + + function test_When_rollupIsListed() external givenMultipleListedRollups { + // it should return true + assertTrue(registry.isRollupRegistered(address(0xdead))); + assertTrue(registry.isRollupRegistered(address(0xbeef))); + } + + function test_When_rollupIsNotListed(address _rollup) external givenMultipleListedRollups { + // it should return false + vm.assume(_rollup != address(0xdead) && _rollup != address(0xbeef)); + assertFalse(registry.isRollupRegistered(_rollup)); + } +} diff --git a/l1-contracts/test/governance/registry/isRollupRegistered.tree b/l1-contracts/test/governance/registry/isRollupRegistered.tree new file mode 100644 index 00000000000..b5b06372076 --- /dev/null +++ b/l1-contracts/test/governance/registry/isRollupRegistered.tree @@ -0,0 +1,11 @@ +IsRollupRegisteredTest +├── given no additional listed rollups +│ ├── when _rollup is 0xdead +│ │ └── it should return true +│ └── when _rollup not 0xdead +│ └── it should return false +└── given multiple listed rollups + ├── when _rollup is listed + │ └── it should return true + └── when _rollup is not listed + └── it should return false \ No newline at end of file diff --git a/l1-contracts/test/governance/registry/upgrade.t.sol b/l1-contracts/test/governance/registry/upgrade.t.sol new file mode 100644 index 00000000000..534465031b6 --- /dev/null +++ b/l1-contracts/test/governance/registry/upgrade.t.sol @@ -0,0 +1,58 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.27; + +import {RegistryBase} from "./Base.t.sol"; + +import {Ownable} from "@oz/access/Ownable.sol"; + +import {IRegistry} from "@aztec/governance/interfaces/IRegistry.sol"; +import {Errors} from "@aztec/governance/libraries/Errors.sol"; +import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol"; + +contract UpgradeTest is RegistryBase { + function test_RevertWhen_CallerIsNotOwner(address _caller) external { + // it should revert + vm.assume(_caller != address(this)); + vm.prank(_caller); + vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, _caller)); + registry.upgrade(address(uint160(uint256(bytes32("new instance"))))); + } + + modifier whenCallerIsOwner() { + _; + } + + function test_RevertWhen_RollupAlreadyInSet() external whenCallerIsOwner { + // it should revert + address rollup = registry.getRollup(); + + vm.expectRevert( + abi.encodeWithSelector(Errors.Registry__RollupAlreadyRegistered.selector, rollup) + ); + registry.upgrade(rollup); + } + + function test_WhenRollupNotAlreadyInSet(address _rollup) external whenCallerIsOwner { + // it should add the rollup to state + // it should emit a {InstanceAdded} event + vm.assume(_rollup != address(0xdead)); + + { + DataStructures.RegistrySnapshot memory snapshotBefore = registry.getCurrentSnapshot(); + assertEq(snapshotBefore.blockNumber, block.number); + assertEq(snapshotBefore.rollup, address(0xdead)); + assertEq(registry.numberOfVersions(), 1); + } + + vm.expectEmit(true, true, false, false, address(registry)); + emit IRegistry.InstanceAdded(_rollup, 1); + registry.upgrade(_rollup); + + assertEq(registry.numberOfVersions(), 2); + + DataStructures.RegistrySnapshot memory snapshot = registry.getCurrentSnapshot(); + assertEq(snapshot.blockNumber, block.number); + assertGt(snapshot.blockNumber, 0); + assertEq(snapshot.rollup, _rollup); + } +} diff --git a/l1-contracts/test/governance/registry/upgrade.tree b/l1-contracts/test/governance/registry/upgrade.tree new file mode 100644 index 00000000000..ac72d6892c3 --- /dev/null +++ b/l1-contracts/test/governance/registry/upgrade.tree @@ -0,0 +1,9 @@ +UpgradeTest +├── when caller is not owner +│ └── it should revert +└── when caller is owner + ├── when rollup already in set + │ └── it should revert + └── when rollup not already in set + ├── it should add the rollup to state + └── it should emit a {InstanceAdded} event \ No newline at end of file diff --git a/l1-contracts/test/portals/TokenPortal.sol b/l1-contracts/test/portals/TokenPortal.sol index 831e3f40f14..b7024677640 100644 --- a/l1-contracts/test/portals/TokenPortal.sol +++ b/l1-contracts/test/portals/TokenPortal.sol @@ -4,9 +4,10 @@ import {IERC20} from "@oz/token/ERC20/IERC20.sol"; import {SafeERC20} from "@oz/token/ERC20/utils/SafeERC20.sol"; // Messaging -import {IRegistry} from "@aztec/core/interfaces/messagebridge/IRegistry.sol"; +import {IRegistry} from "@aztec/governance/interfaces/IRegistry.sol"; import {IInbox} from "@aztec/core/interfaces/messagebridge/IInbox.sol"; import {IOutbox} from "@aztec/core/interfaces/messagebridge/IOutbox.sol"; +import {IRollup} from "@aztec/core/interfaces/IRollup.sol"; import {DataStructures} from "@aztec/core/libraries/DataStructures.sol"; // docs:start:content_hash_sol_import import {Hash} from "@aztec/core/libraries/crypto/Hash.sol"; @@ -40,7 +41,7 @@ contract TokenPortal { returns (bytes32) { // Preamble - IInbox inbox = registry.getRollup().INBOX(); + IInbox inbox = IRollup(registry.getRollup()).INBOX(); DataStructures.L2Actor memory actor = DataStructures.L2Actor(l2Bridge, 1); // Hash the message content to be reconstructed in the receiving contract @@ -69,7 +70,7 @@ contract TokenPortal { bytes32 _secretHashForL2MessageConsumption ) external returns (bytes32) { // Preamble - IInbox inbox = registry.getRollup().INBOX(); + IInbox inbox = IRollup(registry.getRollup()).INBOX(); DataStructures.L2Actor memory actor = DataStructures.L2Actor(l2Bridge, 1); // Hash the message content to be reconstructed in the receiving contract @@ -120,7 +121,7 @@ contract TokenPortal { ) }); - IOutbox outbox = registry.getRollup().OUTBOX(); + IOutbox outbox = IRollup(registry.getRollup()).OUTBOX(); outbox.consume(message, _l2BlockNumber, _leafIndex, _path); diff --git a/l1-contracts/test/portals/TokenPortal.t.sol b/l1-contracts/test/portals/TokenPortal.t.sol index 207fedc454f..630b5106de3 100644 --- a/l1-contracts/test/portals/TokenPortal.t.sol +++ b/l1-contracts/test/portals/TokenPortal.t.sol @@ -5,7 +5,7 @@ import "forge-std/Test.sol"; // Rollup Processor import {Rollup} from "@aztec/core/Rollup.sol"; import {Constants} from "@aztec/core/libraries/ConstantsGen.sol"; -import {Registry} from "@aztec/core/messagebridge/Registry.sol"; +import {Registry} from "@aztec/governance/Registry.sol"; import {DataStructures} from "@aztec/core/libraries/DataStructures.sol"; import {Errors} from "@aztec/core/libraries/Errors.sol"; import {Hash} from "@aztec/core/libraries/crypto/Hash.sol"; @@ -60,8 +60,7 @@ contract TokenPortalTest is Test { function setUp() public { registry = new Registry(address(this)); portalERC20 = new PortalERC20(); - rollup = - new Rollup(registry, IFeeJuicePortal(address(0)), bytes32(0), address(this), new address[](0)); + rollup = new Rollup(IFeeJuicePortal(address(0)), bytes32(0), address(this), new address[](0)); inbox = rollup.INBOX(); outbox = rollup.OUTBOX(); diff --git a/l1-contracts/test/portals/UniswapPortal.sol b/l1-contracts/test/portals/UniswapPortal.sol index 2ea18dabd1d..9d8688b255e 100644 --- a/l1-contracts/test/portals/UniswapPortal.sol +++ b/l1-contracts/test/portals/UniswapPortal.sol @@ -2,8 +2,9 @@ pragma solidity >=0.8.27; import {IERC20} from "@oz/token/ERC20/IERC20.sol"; -import {IRegistry} from "@aztec/core/interfaces/messagebridge/IRegistry.sol"; +import {IRegistry} from "@aztec/governance/interfaces/IRegistry.sol"; import {IOutbox} from "@aztec/core/interfaces/messagebridge/IOutbox.sol"; +import {IRollup} from "@aztec/core/interfaces/IRollup.sol"; import {DataStructures} from "@aztec/core/libraries/DataStructures.sol"; import {DataStructures as PortalDataStructures} from "./DataStructures.sol"; import {Hash} from "@aztec/core/libraries/crypto/Hash.sol"; @@ -103,7 +104,7 @@ contract UniswapPortal { // Consume the message from the outbox { - IOutbox outbox = registry.getRollup().OUTBOX(); + IOutbox outbox = IRollup(registry.getRollup()).OUTBOX(); outbox.consume( DataStructures.L2ToL1Msg({ @@ -209,7 +210,7 @@ contract UniswapPortal { // Consume the message from the outbox { - IOutbox outbox = registry.getRollup().OUTBOX(); + IOutbox outbox = IRollup(registry.getRollup()).OUTBOX(); outbox.consume( DataStructures.L2ToL1Msg({ diff --git a/l1-contracts/test/portals/UniswapPortal.t.sol b/l1-contracts/test/portals/UniswapPortal.t.sol index a2e2a2dea12..ca30d52df47 100644 --- a/l1-contracts/test/portals/UniswapPortal.t.sol +++ b/l1-contracts/test/portals/UniswapPortal.t.sol @@ -4,7 +4,7 @@ import "forge-std/Test.sol"; // Rollup Processor import {Rollup} from "@aztec/core/Rollup.sol"; -import {Registry} from "@aztec/core/messagebridge/Registry.sol"; +import {Registry} from "@aztec/governance/Registry.sol"; import {DataStructures} from "@aztec/core/libraries/DataStructures.sol"; import {DataStructures as PortalDataStructures} from "./DataStructures.sol"; import {Hash} from "@aztec/core/libraries/crypto/Hash.sol"; @@ -52,8 +52,7 @@ contract UniswapPortalTest is Test { vm.selectFork(forkId); registry = new Registry(address(this)); - rollup = - new Rollup(registry, IFeeJuicePortal(address(0)), bytes32(0), address(this), new address[](0)); + rollup = new Rollup(IFeeJuicePortal(address(0)), bytes32(0), address(this), new address[](0)); registry.upgrade(address(rollup)); daiTokenPortal = new TokenPortal(); diff --git a/l1-contracts/test/sparta/Sparta.t.sol b/l1-contracts/test/sparta/Sparta.t.sol index 1dfd393df44..0eb15f5772f 100644 --- a/l1-contracts/test/sparta/Sparta.t.sol +++ b/l1-contracts/test/sparta/Sparta.t.sol @@ -8,7 +8,6 @@ import {DataStructures} from "@aztec/core/libraries/DataStructures.sol"; import {Constants} from "@aztec/core/libraries/ConstantsGen.sol"; import {SignatureLib} from "@aztec/core/libraries/crypto/SignatureLib.sol"; -import {Registry} from "@aztec/core/messagebridge/Registry.sol"; import {Inbox} from "@aztec/core/messagebridge/Inbox.sol"; import {Outbox} from "@aztec/core/messagebridge/Outbox.sol"; import {Errors} from "@aztec/core/libraries/Errors.sol"; @@ -40,7 +39,6 @@ contract SpartaTest is DecoderBase { bool shouldRevert; } - Registry internal registry; Inbox internal inbox; Outbox internal outbox; Rollup internal rollup; @@ -75,16 +73,11 @@ contract SpartaTest is DecoderBase { initialValidators[i - 1] = validator; } - registry = new Registry(address(this)); portalERC20 = new PortalERC20(); - rollup = new Rollup( - registry, IFeeJuicePortal(address(0)), bytes32(0), address(this), initialValidators - ); + rollup = new Rollup(IFeeJuicePortal(address(0)), bytes32(0), address(this), initialValidators); inbox = Inbox(address(rollup.INBOX())); outbox = Outbox(address(rollup.OUTBOX())); - registry.upgrade(address(rollup)); - merkleTestUtil = new MerkleTestUtil(); txsHelper = new TxsDecoderHelper(); diff --git a/yarn-project/ethereum/src/deploy_l1_contracts.ts b/yarn-project/ethereum/src/deploy_l1_contracts.ts index 511c9369c52..6fe962560bc 100644 --- a/yarn-project/ethereum/src/deploy_l1_contracts.ts +++ b/yarn-project/ethereum/src/deploy_l1_contracts.ts @@ -208,7 +208,6 @@ export const deployL1Contracts = async ( logger.info(`Deployed Gas Portal at ${feeJuicePortalAddress}`); const rollupAddress = await deployer.deploy(contractsToDeploy.rollup, [ - getAddress(registryAddress.toString()), getAddress(feeJuicePortalAddress.toString()), args.vkTreeRoot.toString(), account.address.toString(),