-
Notifications
You must be signed in to change notification settings - Fork 35
feat: add system config consensus to deprecate clique #72
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,65 @@ | ||||||
| // SPDX-License-Identifier: MIT | ||||||
| pragma solidity =0.8.24; | ||||||
|
|
||||||
| import "@openzeppelin/contracts/access/Ownable.sol"; | ||||||
|
|
||||||
| /** | ||||||
| * @dev Example "SystemSignerRegistry" storing `(startBlock, signer)` pairs. | ||||||
| * The getSigners() function returns parallel arrays for block numbers and addresses, | ||||||
| * | ||||||
| */ | ||||||
| contract SystemSignerRegistry is Ownable { | ||||||
|
||||||
| struct Signer { | ||||||
| uint64 startBlock; | ||||||
|
||||||
| address signer; | ||||||
| } | ||||||
|
|
||||||
| Signer[] private signers; | ||||||
|
||||||
| Signer[] private signers; | |
| mapping (address => bool) public isAuthorizedSigner; |
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.
Need to consider how l2geth will read this contract.
- If it just polls the latest state asking "am I authorized?", then a mapping is suitable.
- If it just polls the latest state asking "what is the list of current authorized signers?", then an array is suitable.
- If it fetches events, then I'd still go with mapping.
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.
will we have multiple authorized signer in the same time?
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.
No, we should not have multiple authorized signers at the same time in this implementation: a new key will replace the old one even if specifying the same blockNum, although for this I need to make the list in DESC order. (if we go with the easier, optimism-like implementation, then we have multiple authorized sequencers actually, i.e. see this other comment).
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.
Having multiple makes key rotation a bit more convenient. But if we want to keep it simple, we can also just have one (currentAuthorizedSigner).
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 we want to make this upgradable?