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

fix: Redundant Logic #854

Draft
wants to merge 1 commit into
base: consensus-fix-review
Choose a base branch
from
Draft
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
56 changes: 46 additions & 10 deletions l2-contracts/contracts/ConsensusRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,20 @@ contract ConsensusRegistry is IConsensusRegistry, Initializable, Ownable2StepUpg
/// @dev Fails if node owner already exists.
/// @dev Fails if a validator/attester with the same public key already exists.
/// @param _nodeOwner The address of the new node's owner.
/// @param _isValidatorActive A flag stating if the validator starts activated.
/// @param _validatorWeight The voting weight of the validator.
/// @param _validatorPubKey The BLS12-381 public key of the validator.
/// @param _validatorPoP The proof-of-possession (PoP) of the validator's public key.
/// @param _isAttesterActive A flag stating if the attester starts activated.
/// @param _attesterWeight The voting weight of the attester.
/// @param _attesterPubKey The ECDSA public key of the attester.
function add(
address _nodeOwner,
bool _isValidatorActive,
uint32 _validatorWeight,
BLS12_381PublicKey calldata _validatorPubKey,
BLS12_381Signature calldata _validatorPoP,
bool _isAttesterActive,
uint32 _attesterWeight,
Secp256k1PublicKey calldata _attesterPubKey
) external onlyOwner {
Expand All @@ -77,8 +81,8 @@ contract ConsensusRegistry is IConsensusRegistry, Initializable, Ownable2StepUpg
nodeOwners.push(_nodeOwner);
nodes[_nodeOwner] = Node({
attesterLatest: AttesterAttr({
active: true,
removed: false,
active: _isAttesterActive,
weight: _attesterWeight,
pubKey: _attesterPubKey
}),
Expand All @@ -90,8 +94,8 @@ contract ConsensusRegistry is IConsensusRegistry, Initializable, Ownable2StepUpg
}),
attesterLastUpdateCommit: attestersCommit,
validatorLatest: ValidatorAttr({
active: true,
removed: false,
active: _isValidatorActive,
weight: _validatorWeight,
pubKey: _validatorPubKey,
proofOfPossession: _validatorPoP
Expand All @@ -111,19 +115,21 @@ contract ConsensusRegistry is IConsensusRegistry, Initializable, Ownable2StepUpg

emit NodeAdded({
nodeOwner: _nodeOwner,
isValidatorActive: _isValidatorActive,
validatorWeight: _validatorWeight,
validatorPubKey: _validatorPubKey,
validatorPoP: _validatorPoP,
isAttesterActive: _isAttesterActive,
attesterWeight: _attesterWeight,
attesterPubKey: _attesterPubKey
});
}

/// @notice Deactivates a node, preventing it from participating in committees.
/// @notice Deactivates an attester, preventing it from participating in attester committees.
/// @dev Only callable by the contract owner or the node owner.
/// @dev Verifies that the node owner exists in the registry.
/// @param _nodeOwner The address of the node's owner to be inactivated.
function deactivate(address _nodeOwner) external onlyOwnerOrNodeOwner(_nodeOwner) {
function deactivate_attester(address _nodeOwner) external onlyOwnerOrNodeOwner(_nodeOwner) {
_verifyNodeOwnerExists(_nodeOwner);
(Node storage node, bool deleted) = _getNodeAndDeleteIfRequired(_nodeOwner);
if (deleted) {
Expand All @@ -132,17 +138,32 @@ contract ConsensusRegistry is IConsensusRegistry, Initializable, Ownable2StepUpg

_ensureAttesterSnapshot(node);
node.attesterLatest.active = false;
_ensureValidatorSnapshot(node);
node.validatorLatest.active = false;

emit NodeDeactivated(_nodeOwner);
emit AttesterDeactivated(_nodeOwner);
}

/// @notice Activates a previously inactive node, allowing it to participate in committees.
/// @notice Deactivates a validator, preventing it from participating in validator committees.
/// @dev Only callable by the contract owner or the node owner.
/// @dev Verifies that the node owner exists in the registry.
/// @param _nodeOwner The address of the node's owner to be inactivated.
function deactivate_validator(address _nodeOwner) external onlyOwnerOrNodeOwner(_nodeOwner) {
_verifyNodeOwnerExists(_nodeOwner);
(Node storage node, bool deleted) = _getNodeAndDeleteIfRequired(_nodeOwner);
if (deleted) {
return;
}

_ensureValidatorSnapshot(node);
node.validatorLatest.active = false;

emit ValidatorDeactivated(_nodeOwner);
}

/// @notice Activates a previously inactive attester, allowing it to participate in attester committees.
/// @dev Only callable by the contract owner.
/// @dev Verifies that the node owner exists in the registry.
/// @param _nodeOwner The address of the node's owner to be activated.
function activate(address _nodeOwner) external onlyOwnerOrNodeOwner(_nodeOwner) {
function activate_attester(address _nodeOwner) external onlyOwner {
_verifyNodeOwnerExists(_nodeOwner);
(Node storage node, bool deleted) = _getNodeAndDeleteIfRequired(_nodeOwner);
if (deleted) {
Expand All @@ -151,10 +172,25 @@ contract ConsensusRegistry is IConsensusRegistry, Initializable, Ownable2StepUpg

_ensureAttesterSnapshot(node);
node.attesterLatest.active = true;

emit AttesterActivated(_nodeOwner);
}

/// @notice Activates a previously inactive validator, allowing it to participate in validator committees.
/// @dev Only callable by the contract owner.
/// @dev Verifies that the node owner exists in the registry.
/// @param _nodeOwner The address of the node's owner to be activated.
function activate_validator(address _nodeOwner) external onlyOwner {
_verifyNodeOwnerExists(_nodeOwner);
(Node storage node, bool deleted) = _getNodeAndDeleteIfRequired(_nodeOwner);
if (deleted) {
return;
}

_ensureValidatorSnapshot(node);
node.validatorLatest.active = true;

emit NodeActivated(_nodeOwner);
emit ValidatorActivated(_nodeOwner);
}

/// @notice Removes a node from the registry.
Expand Down
18 changes: 14 additions & 4 deletions l2-contracts/contracts/interfaces/IConsensusRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,18 @@ interface IConsensusRegistry {

event NodeAdded(
address indexed nodeOwner,
bool isValidatorActive,
uint32 validatorWeight,
BLS12_381PublicKey validatorPubKey,
BLS12_381Signature validatorPoP,
bool isAttesterActive,
uint32 attesterWeight,
Secp256k1PublicKey attesterPubKey
);
event NodeDeactivated(address indexed nodeOwner);
event NodeActivated(address indexed nodeOwner);
event AttesterDeactivated(address indexed nodeOwner);
event ValidatorDeactivated(address indexed nodeOwner);
event AttesterActivated(address indexed nodeOwner);
event ValidatorActivated(address indexed nodeOwner);
event NodeRemoved(address indexed nodeOwner);
event NodeDeleted(address indexed nodeOwner);
event NodeValidatorWeightChanged(address indexed nodeOwner, uint32 newWeight);
Expand All @@ -126,16 +130,22 @@ interface IConsensusRegistry {

function add(
address _nodeOwner,
bool _isValidatorActive,
uint32 _validatorWeight,
BLS12_381PublicKey calldata _validatorPubKey,
BLS12_381Signature calldata _validatorPoP,
bool _isAttesterActive,
uint32 _attesterWeight,
Secp256k1PublicKey calldata _attesterPubKey
) external;

function deactivate(address _nodeOwner) external;
function deactivate_attester(address _nodeOwner) external;

function activate(address _nodeOwner) external;
function deactivate_validator(address _nodeOwner) external;

function activate_attester(address _nodeOwner) external;

function activate_validator(address _nodeOwner) external;

function remove(address _nodeOwner) external;

Expand Down
Loading