-
Notifications
You must be signed in to change notification settings - Fork 348
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
feat(registration): Add Chain registrar contract #1064
base: dev
Are you sure you want to change the base?
Conversation
Signed-off-by: Danil <[email protected]> Co-authored-by: Vlad Bochok <[email protected]>
Signed-off-by: Danil <[email protected]> Co-authored-by: Vlad Bochok <[email protected]>
Signed-off-by: Danil <[email protected]> Co-authored-by: Vlad Bochok <[email protected]>
Signed-off-by: Danil <[email protected]>
Signed-off-by: Danil <[email protected]> Co-authored-by: Bence Haromi <[email protected]> Co-authored-by: Grzegorz Prusak <[email protected]> Co-authored-by: Moshe Shababo <[email protected]> Co-authored-by: Akosh Farkash <[email protected]> Co-authored-by: Bruno França <[email protected]> Co-authored-by: Vlad Bochok <[email protected]> Co-authored-by: Roman Brodetski <[email protected]> Co-authored-by: vladbochok <[email protected]> Co-authored-by: Stanislav Bezkorovainyi <[email protected]> Co-authored-by: Danil <[email protected]>
Signed-off-by: Danil <[email protected]> Co-authored-by: otani <[email protected]> Co-authored-by: Zach Kolodny <[email protected]>
Signed-off-by: Danil <[email protected]>
Co-authored-by: Vlad Bochok <[email protected]>
feat: add timestamp asserter contract
…sserter-command fix: add deploy timestamp asserter command
Signed-off-by: Danil <[email protected]>
Signed-off-by: Danil <[email protected]>
Signed-off-by: Danil <[email protected]>
Signed-off-by: Danil <[email protected]>
Signed-off-by: Danil <[email protected]>
Signed-off-by: Danil <[email protected]>
Signed-off-by: Danil <[email protected]>
54e4676
to
9a72fc2
Compare
Signed-off-by: Danil <[email protected]>
Signed-off-by: Danil <[email protected]>
Co-authored-by: Vlad Bochok <[email protected]>
Signed-off-by: Danil <[email protected]>
Signed-off-by: Danil <[email protected]>
Signed-off-by: Danil <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update comments to follow napspec specificaion. Comment all function parameters by /// @param
.
4bd2894
to
b13245c
Compare
Signed-off-by: Danil <[email protected]>
b13245c
to
6ed3ebc
Compare
Signed-off-by: Danil <[email protected]>
d831ef1
to
1fabb69
Compare
Signed-off-by: Danil <[email protected]>
1fabb69
to
8ceaa5b
Compare
Signed-off-by: Danil <[email protected]>
0045c83
to
2b54930
Compare
bd8fe8d
to
0bfe491
Compare
Signed-off-by: Danil <[email protected]>
0bfe491
to
3388f52
Compare
/// @title ChainRegistrar Contract | ||
/// @author Matter Labs | ||
/// @custom:security-contact [email protected] | ||
/// @notice This contract is used for proposing and registering new chains in the zkSync ecosystem. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- zkSync -> ZKsync
- It doesn't register chains, users just propose them
/// @notice This contract is used for proposing and registering new chains in the zkSync ecosystem. | |
/// @notice This contract is used as a public registry where anyone can propose new chain registration in ZKsync ecosystem. |
/// @author Matter Labs | ||
/// @custom:security-contact [email protected] | ||
/// @notice This contract is used for proposing and registering new chains in the zkSync ecosystem. | ||
/// @notice It helps chain administrators retrieve all necessary L1 information about their chain. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// @notice It helps chain administrators retrieve all necessary L1 information about their chain. | |
/// @notice It also helps chain administrators retrieve all necessary L1 information about their chain. |
/// @custom:security-contact [email protected] | ||
/// @notice This contract is used for proposing and registering new chains in the zkSync ecosystem. | ||
/// @notice It helps chain administrators retrieve all necessary L1 information about their chain. | ||
/// @notice Additionally, it assists zkSync administrators in verifying the correctness of registration transactions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think "ZKsync administrators" is a good term, also it is a bit confusing as we rather use ZKsync ecosystem admin
in other places.
/// @dev During the chain proposal, some base tokens must be transferred to this address. | ||
address public l2Deployer; | ||
|
||
/// @notice Address of the ZKsync Bridgehub. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// @notice Address of the ZKsync Bridgehub. | |
/// @notice Address of ZKsync Bridgehub. |
/// @dev Struct for holding the base token configuration of a chain. | ||
struct BaseToken { | ||
/// @notice Gas price multiplier numerator, used to compare the base token price to ether for L1->L2 transactions. | ||
uint128 gasPriceMultiplierNominator; | ||
/// @notice Gas price multiplier denominator, used to compare the base token price to ether for L1->L2 transactions. | ||
uint128 gasPriceMultiplierDenominator; | ||
/// @notice Address of the base token used for gas fees. | ||
address tokenAddress; | ||
/// @notice Address responsible for setting the token multiplier. | ||
address tokenMultiplierSetter; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Natspec
/// @dev Struct for holding the base token configuration of a chain. | |
struct BaseToken { | |
/// @notice Gas price multiplier numerator, used to compare the base token price to ether for L1->L2 transactions. | |
uint128 gasPriceMultiplierNominator; | |
/// @notice Gas price multiplier denominator, used to compare the base token price to ether for L1->L2 transactions. | |
uint128 gasPriceMultiplierDenominator; | |
/// @notice Address of the base token used for gas fees. | |
address tokenAddress; | |
/// @notice Address responsible for setting the token multiplier. | |
address tokenMultiplierSetter; | |
} | |
/// @dev Struct for holding the base token configuration of a chain. | |
/// @param gasPriceMultiplierNominator Gas price multiplier numerator, used to compare the base token price to ether for L1->L2 transactions. | |
/// @param gasPriceMultiplierDenominator Gas price multiplier denominator, used to compare the base token price to ether for L1->L2 transactions. | |
/// @param tokenAddress Address of the base token used for gas fees. | |
/// @param tokenMultiplierSetter Address responsible for setting the token multiplier. | |
struct BaseToken { | |
uint128 gasPriceMultiplierNominator; | |
uint128 gasPriceMultiplierDenominator; | |
address tokenAddress; | |
address tokenMultiplierSetter; | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change it in other places too
/// @param _bridgehub Address of the ZKsync Bridgehub. | ||
/// @param _l2Deployer Address of the L2 deployer. | ||
/// @param _owner Address of the contract owner. | ||
function initialize(address _bridgehub, address _l2Deployer, address _owner) public { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important: anyone can reinitialize the contract. To avoid it you can use Initializable.sol
from OZ lib. Also don't forget to set _disableInitializers
in constructor.
function initialize(address _bridgehub, address _l2Deployer, address _owner) public { | |
function initialize(address _bridgehub, address _l2Deployer, address _owner) external Initializer { |
_transferOwnership(_owner); | ||
} | ||
|
||
/// @notice Proposes a new chain to be registered in the zkSync ecosystem. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ZKsync everywhere please
/// @notice Proposes a new chain to be registered in the zkSync ecosystem. | |
/// @notice Proposes a new chain to be registered in the ZKsync ecosystem. |
revert ChainIsAlreadyDeployed(); | ||
} | ||
|
||
proposedChains[msg.sender][_chainId] = config; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it makes sense to allow user to re-propose config? I would rather restrict it to make avoid any confusion on the final config (as soon as config is onchain it immutable).
What ❔
Add Chain registrar contracts, that allow partners to register their chains and then configure their chain from L1
Why ❔
For making our registration process a bit more transparent and later on make it even easier
Checklist