Skip to content

Commit 7411acc

Browse files
authored
chore: move governance out of core (#8823)
Slight extension on #8818 to separate gov and instance more clearly. Gov does not have dependencies on core, but core can depend on the gov. Removes the registry value from the `Rollup` does not address that an upgrade would make the fee contract unbacked, that is to be handled separately in #8756. Updates testing for the `Registry` contract to use the [Branching Tree Technique](https://www.youtube.com/watch?v=0-EmbNVgFA4) from Paul. Found it quite neat. If using `bulloak` as well quite convenient to build the setups. The TL;DR is, build a `.tree` file outlining the test for a function, and then use `bulloak scaffold` to prepare a testing file and then fill in the logic. Makes it quite nice to follow the logic. # Example time ```.tree 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 ``` ```solidity // 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)); } } ``` ```solidity // 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); } } ```
1 parent 2d5ae66 commit 7411acc

32 files changed

+391
-141
lines changed

docs/docs/reference/developer_references/smart_contract_reference/portals/data_structures.md

+1-3
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,11 @@ A message that is sent from L2 to L1.
5858

5959
A snapshot of the registry values.
6060

61-
#include_code registry_snapshot l1-contracts/src/core/libraries/DataStructures.sol solidity
61+
#include_code registry_snapshot l1-contracts/src/governance/libraries/DataStructures.sol solidity
6262

6363
| Name | Type | Description |
6464
| -------------- | ------- | ----------- |
6565
| `rollup` | `address` | The address of the rollup contract for the snapshot. |
66-
| `inbox` | `address` | The address of the inbox contract for the snapshot. |
67-
| `outbox` | `address` | The address of the outbox contract for the snapshot. |
6866
| `blockNumber` | `uint256` | The block number at which the snapshot was created. |
6967

7068

docs/docs/reference/developer_references/smart_contract_reference/portals/registry.md

+9-9
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@ tags: [portals, contracts]
55

66
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.
77

8-
**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).
8+
**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).
99

1010
## `numberOfVersions()`
1111

1212
Retrieves the number of versions that have been deployed.
1313

14-
#include_code registry_number_of_versions l1-contracts/src/core/interfaces/messagebridge/IRegistry.sol solidity
14+
#include_code registry_number_of_versions l1-contracts/src/governance/interfaces/IRegistry.sol solidity
1515

1616
| Name | Description |
1717
| -------------- | ----------- |
@@ -20,17 +20,17 @@ Retrieves the number of versions that have been deployed.
2020
## `getRollup()`
2121
Retrieves the current rollup contract.
2222

23-
#include_code registry_get_rollup l1-contracts/src/core/interfaces/messagebridge/IRegistry.sol solidity
23+
#include_code registry_get_rollup l1-contracts/src/governance/interfaces/IRegistry.sol solidity
2424

2525
| Name | Description |
2626
| -------------- | ----------- |
2727
| ReturnValue | The current rollup |
2828

2929
## `getVersionFor(address _rollup)`
3030

31-
Retrieve the version of a specific rollup contract.
31+
Retrieve the version of a specific rollup contract.
3232

33-
#include_code registry_get_version_for l1-contracts/src/core/interfaces/messagebridge/IRegistry.sol solidity
33+
#include_code registry_get_version_for l1-contracts/src/governance/interfaces/IRegistry.sol solidity
3434

3535
| Name | Description |
3636
| -------------- | ----------- |
@@ -42,10 +42,10 @@ Will revert with `Registry__RollupNotRegistered(_rollup)` if the rollup have not
4242

4343
## `getSnapshot(uint256 _version)`
4444

45-
Retrieve the snapshot of a specific version.
45+
Retrieve the snapshot of a specific version.
4646

47-
#include_code registry_snapshot l1-contracts/src/core/libraries/DataStructures.sol solidity
48-
#include_code registry_get_snapshot l1-contracts/src/core/interfaces/messagebridge/IRegistry.sol solidity
47+
#include_code registry_snapshot l1-contracts/src/governance/libraries/DataStructures.sol solidity
48+
#include_code registry_get_snapshot l1-contracts/src/governance/interfaces/IRegistry.sol solidity
4949

5050
| Name | Description |
5151
| -------------- | ----------- |
@@ -58,7 +58,7 @@ Retrieve the snapshot of a specific version.
5858

5959
Retrieves the snapshot for the current version.
6060

61-
#include_code registry_get_current_snapshot l1-contracts/src/core/interfaces/messagebridge/IRegistry.sol solidity
61+
#include_code registry_get_current_snapshot l1-contracts/src/governance/interfaces/IRegistry.sol solidity
6262

6363
| Name | Description |
6464
| -------------- | ----------- |

l1-contracts/README.md

+26-2
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,29 @@ Currently not running any proofs _nor_ access control so blocks can be submitted
5656

5757
---
5858

59+
# Branching Tree Technique
60+
61+
For writing tests we will be using the [Branching Tree Technique](https://www.youtube.com/watch?v=0-EmbNVgFA4).
62+
The approach is simple, for a function that is to be tested (all functions) you should write a `.tree` file first.
63+
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.
64+
65+
```bash
66+
bulloak scaffold -w **/*.tree
67+
```
68+
69+
To check that the tests are following the expected format, you can run the `check` command.
70+
71+
```bash
72+
bulloak check **/*.tree
73+
```
74+
75+
We assume that you already have `bulloak` installed, if not you can install it as `cargo install bulloak`.
76+
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.
77+
78+
For examples, see the tests for the registry.
79+
80+
---
81+
5982
# Linter
6083

6184
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
7396
# Slither & Slitherin
7497

7598
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:
99+
76100
```bash
77101
yarn slither
78102
```
79103

80-
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.
104+
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.
81105

82106
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.
83107

84-
> We are not running the `naming-convention` detector because we have our own rules for naming which is enforced by the linter.
108+
> We are not running the `naming-convention` detector because we have our own rules for naming which is enforced by the linter.

l1-contracts/src/core/FeeJuicePortal.sol

+3-2
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ pragma solidity >=0.8.27;
55
import {IERC20} from "@oz/token/ERC20/IERC20.sol";
66
import {IFeeJuicePortal} from "@aztec/core/interfaces/IFeeJuicePortal.sol";
77
import {IInbox} from "@aztec/core/interfaces/messagebridge/IInbox.sol";
8-
import {IRegistry} from "@aztec/core/interfaces/messagebridge/IRegistry.sol";
8+
import {IRegistry} from "@aztec/governance/interfaces/IRegistry.sol";
9+
import {IRollup} from "@aztec/core/interfaces/IRollup.sol";
910

1011
import {Constants} from "@aztec/core/libraries/ConstantsGen.sol";
1112
import {DataStructures} from "@aztec/core/libraries/DataStructures.sol";
@@ -74,7 +75,7 @@ contract FeeJuicePortal is IFeeJuicePortal, Ownable {
7475
returns (bytes32)
7576
{
7677
// Preamble
77-
IInbox inbox = registry.getRollup().INBOX();
78+
IInbox inbox = IRollup(registry.getRollup()).INBOX();
7879
DataStructures.L2Actor memory actor = DataStructures.L2Actor(l2TokenAddress, 1);
7980

8081
// Hash the message content to be reconstructed in the receiving contract

l1-contracts/src/core/Rollup.sol

-4
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import {IProofCommitmentEscrow} from "@aztec/core/interfaces/IProofCommitmentEsc
66
import {IInbox} from "@aztec/core/interfaces/messagebridge/IInbox.sol";
77
import {IOutbox} from "@aztec/core/interfaces/messagebridge/IOutbox.sol";
88
import {IFeeJuicePortal} from "@aztec/core/interfaces/IFeeJuicePortal.sol";
9-
import {IRegistry} from "@aztec/core/interfaces/messagebridge/IRegistry.sol";
109
import {IRollup, ITestRollup} from "@aztec/core/interfaces/IRollup.sol";
1110
import {IVerifier} from "@aztec/core/interfaces/IVerifier.sol";
1211

@@ -56,7 +55,6 @@ contract Rollup is Leonidas, IRollup, ITestRollup {
5655
uint256 public constant PROOF_COMMITMENT_MIN_BOND_AMOUNT_IN_TST = 1000;
5756

5857
uint256 public immutable L1_BLOCK_AT_GENESIS;
59-
IRegistry public immutable REGISTRY;
6058
IInbox public immutable INBOX;
6159
IOutbox public immutable OUTBOX;
6260
IProofCommitmentEscrow public immutable PROOF_COMMITMENT_ESCROW;
@@ -85,15 +83,13 @@ contract Rollup is Leonidas, IRollup, ITestRollup {
8583
IVerifier public epochProofVerifier;
8684

8785
constructor(
88-
IRegistry _registry,
8986
IFeeJuicePortal _fpcJuicePortal,
9087
bytes32 _vkTreeRoot,
9188
address _ares,
9289
address[] memory _validators
9390
) Leonidas(_ares) {
9491
blockProofVerifier = new MockVerifier();
9592
epochProofVerifier = new MockVerifier();
96-
REGISTRY = _registry;
9793
FEE_JUICE_PORTAL = _fpcJuicePortal;
9894
PROOF_COMMITMENT_ESCROW = new MockProofCommitmentEscrow();
9995
INBOX = IInbox(address(new Inbox(address(this), Constants.L1_TO_L2_MSG_SUBTREE_HEIGHT)));

l1-contracts/src/core/libraries/DataStructures.sol

-12
Original file line numberDiff line numberDiff line change
@@ -67,18 +67,6 @@ library DataStructures {
6767
}
6868
// docs:end:l2_to_l1_msg
6969

70-
// docs:start:registry_snapshot
71-
/**
72-
* @notice Struct for storing address of cross communication components and the block number when it was updated
73-
* @param rollup - The address of the rollup contract
74-
* @param blockNumber - The block number of the snapshot
75-
*/
76-
struct RegistrySnapshot {
77-
address rollup;
78-
uint256 blockNumber;
79-
}
80-
// docs:end:registry_snapshot
81-
8270
/**
8371
* @notice Struct for storing flags for block header validation
8472
* @param ignoreDA - True will ignore DA check, otherwise checks

l1-contracts/src/core/libraries/Errors.sol

-4
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,6 @@ library Errors {
7272
error Rollup__TryingToProveNonExistingBlock(); // 0x34ef4954
7373
error Rollup__UnavailableTxs(bytes32 txsHash); // 0x414906c3
7474

75-
// Registry
76-
error Registry__RollupNotRegistered(address rollup); // 0xa1fee4cf
77-
error Registry__RollupAlreadyRegistered(address rollup); // 0x3c34eabf
78-
7975
//TxsDecoder
8076
error TxsDecoder__InvalidLogsLength(uint256 expected, uint256 actual); // 0x829ca981
8177
error TxsDecoder__TxsTooLarge(uint256 expected, uint256 actual); // 0xc7d44a62

l1-contracts/src/core/messagebridge/Registry.sol l1-contracts/src/governance/Registry.sol

+10-9
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,10 @@
22
// Copyright 2023 Aztec Labs.
33
pragma solidity >=0.8.27;
44

5-
import {IRegistry} from "@aztec/core/interfaces/messagebridge/IRegistry.sol";
6-
import {IRollup} from "@aztec/core/interfaces/IRollup.sol";
5+
import {IRegistry} from "@aztec/governance/interfaces/IRegistry.sol";
76

8-
import {DataStructures} from "@aztec/core/libraries/DataStructures.sol";
9-
import {Errors} from "@aztec/core/libraries/Errors.sol";
7+
import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol";
8+
import {Errors} from "@aztec/governance/libraries/Errors.sol";
109

1110
import {Ownable} from "@oz/access/Ownable.sol";
1211

@@ -29,11 +28,11 @@ contract Registry is IRegistry, Ownable {
2928
}
3029

3130
/**
32-
* @notice Returns the rollup contract
33-
* @return The rollup contract (of type IRollup)
31+
* @notice Returns the address of the rollup contract
32+
* @return The rollup address
3433
*/
35-
function getRollup() external view override(IRegistry) returns (IRollup) {
36-
return IRollup(currentSnapshot.rollup);
34+
function getRollup() external view override(IRegistry) returns (address) {
35+
return currentSnapshot.rollup;
3736
}
3837

3938
/**
@@ -109,11 +108,13 @@ contract Registry is IRegistry, Ownable {
109108
snapshots[version] = newSnapshot;
110109
rollupToVersion[_rollup] = version;
111110

111+
emit InstanceAdded(_rollup, version);
112+
112113
return version;
113114
}
114115

115116
function _getVersionFor(address _rollup) internal view returns (uint256 version, bool exists) {
116117
version = rollupToVersion[_rollup];
117-
return (version, version > 0);
118+
return (version, version > 0 || snapshots[0].rollup == _rollup);
118119
}
119120
}

l1-contracts/src/core/interfaces/messagebridge/IRegistry.sol l1-contracts/src/governance/interfaces/IRegistry.sol

+4-3
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
// SPDX-License-Identifier: Apache-2.0
22
pragma solidity >=0.8.27;
33

4-
import {DataStructures} from "../../libraries/DataStructures.sol";
5-
import {IRollup} from "../IRollup.sol";
4+
import {DataStructures} from "@aztec/governance/libraries/DataStructures.sol";
65

76
interface IRegistry {
7+
event InstanceAdded(address indexed instance, uint256 indexed version);
8+
89
// docs:start:registry_upgrade
910
function upgrade(address _rollup) external returns (uint256);
1011
// docs:end:registry_upgrade
1112

1213
// docs:start:registry_get_rollup
13-
function getRollup() external view returns (IRollup);
14+
function getRollup() external view returns (address);
1415
// docs:end:registry_get_rollup
1516

1617
// docs:start:registry_get_version_for
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
// Copyright 2023 Aztec Labs.
3+
pragma solidity >=0.8.18;
4+
5+
/**
6+
* @title Data Structures Library
7+
* @author Aztec Labs
8+
* @notice Library that contains data structures used throughout Aztec governance
9+
*/
10+
library DataStructures {
11+
// docs:start:registry_snapshot
12+
/**
13+
* @notice Struct for storing address of cross communication components and the block number when it was updated
14+
* @param rollup - The address of the rollup contract
15+
* @param blockNumber - The block number of the snapshot
16+
*/
17+
struct RegistrySnapshot {
18+
address rollup;
19+
uint256 blockNumber;
20+
}
21+
// docs:end:registry_snapshot
22+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
// Copyright 2023 Aztec Labs.
3+
pragma solidity >=0.8.18;
4+
5+
/**
6+
* @title Errors Library
7+
* @author Aztec Labs
8+
* @notice Library that contains errors used throughout the Aztec governance
9+
* Errors are prefixed with the contract name to make it easy to identify where the error originated
10+
* when there are multiple contracts that could have thrown the error.
11+
*/
12+
library Errors {
13+
// Registry
14+
error Registry__RollupNotRegistered(address rollup); // 0xa1fee4cf
15+
error Registry__RollupAlreadyRegistered(address rollup); // 0x3c34eabf
16+
}

l1-contracts/test/Registry.t.sol

-62
This file was deleted.

0 commit comments

Comments
 (0)