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

[ACP-99] Emit Justification Info #188

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
61 changes: 39 additions & 22 deletions ACPs/99-validatorsetmanager-contract/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,15 +120,34 @@ For a full implementation, please see the [Reference Implementation](#reference-
* validator management, as specified in ACP-77.
*/
abstract contract ACP99Manager {
/// @notice Emitted when an initial validator is registered.
event RegisteredInitialValidator(bytes32 indexed validationID, bytes20 indexed nodeID, uint64 weight);
/// @notice Emitted when a validator registration to the L1 is initiated.
/**
* @notice Emitted when an initial validator is registered.
* @notice The field index is the index of the initial validator in the conversion data.
* This is used along with the subnetID as the ACP-118 justification for
* signature requests from P-Chain validators over a L1ValidatorRegistrationMessage
Comment on lines +126 to +127
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* This is used along with the subnetID as the ACP-118 justification for
* signature requests from P-Chain validators over a L1ValidatorRegistrationMessage
* This is used along with the subnetID as the ACP-118 justification in
* signature requests to P-Chain validators over a L1ValidatorRegistrationMessage

It took me a few tries to parse this sentence. I think this is slightly more accurate, but feel free to disagree.

* when removing the validator
*/
event RegisteredInitialValidator(
bytes32 indexed validationID,
bytes20 indexed nodeID,
bytes32 indexed subnetID,
uint64 weight,
uint32 index
);
/**
* @notice Emitted when a validator registration to the L1 is initiated.
* @notice The field registerL1ValidatorMessage is the serialized RegisterL1ValidatorMessage
* used to register the validator on the P-Chain. This is also used as the
* ACP-118 justification for signature requests from P-Chain validators
* over a L1ValidatorRegistrationMessage when removing the validator.
*/
event InitiatedValidatorRegistration(
bytes32 indexed validationID,
bytes20 indexed nodeID,
bytes32 registrationMessageID,
uint64 registrationExpiry,
uint64 weight
uint64 weight,
bytes registerL1ValidatorMessage
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These bytes are already emitted by the precompile as part of the sendWarpMessage event here, right? Is there a significant UX improvement to also emitting them here?

);
/// @notice Emitted when a validator registration to the L1 is completed.
event CompletedValidatorRegistration(bytes32 indexed validationID, uint64 weight);
Expand All @@ -152,11 +171,9 @@ abstract contract ACP99Manager {
function subnetID() public view virtual returns (bytes32 id);

/// @notice Returns the validator details for a given validation ID.
function getValidator(bytes32 validationID)
public
view
virtual
returns (Validator memory validator);
function getValidator(
bytes32 validationID
) public view virtual returns (Validator memory validator);

/// @notice Returns the total weight of the current L1 validator set.
function l1TotalWeight() public view virtual returns (uint64 weight);
Expand Down Expand Up @@ -208,10 +225,9 @@ abstract contract ACP99Manager {
* @param messageIndex The index of the L1ValidatorRegistrationMessage to be received providing the acknowledgement.
* @return validationID The ID of the registered validator.
*/
function completeValidatorRegistration(uint32 messageIndex)
public
virtual
returns (bytes32 validationID);
function completeValidatorRegistration(
uint32 messageIndex
) public virtual returns (bytes32 validationID);

/**
* @notice Initiates validator removal by issuing a L1ValidatorWeightMessage with the weight set to zero.
Expand All @@ -221,7 +237,9 @@ abstract contract ACP99Manager {
*
* @param validationID The ID of the validator to remove.
*/
function _initiateValidatorRemoval(bytes32 validationID) internal virtual;
function _initiateValidatorRemoval(
bytes32 validationID
) internal virtual;

/**
* @notice Completes validator removal by consuming an RegisterL1ValidatorMessage from the P-Chain acknowledging
Expand All @@ -230,11 +248,11 @@ abstract contract ACP99Manager {
* Emits a {CompletedValidatorRemoval} on success.
*
* @param messageIndex The index of the RegisterL1ValidatorMessage.
* @return validationID The ID of the validator that was removed.
*/
function completeValidatorRemoval(uint32 messageIndex)
public
virtual
returns (bytes32 validationID);
function completeValidatorRemoval(
uint32 messageIndex
) public virtual returns (bytes32 validationID);

/**
* @notice Initiates a validator weight update by issuing an L1ValidatorWeightMessage with a nonzero weight.
Expand Down Expand Up @@ -262,10 +280,9 @@ abstract contract ACP99Manager {
* @return validationID The ID of the validator, retreived from the L1ValidatorWeightMessage.
* @return nonce The nonce of the validator, retreived from the L1ValidatorWeightMessage.
*/
function completeValidatorWeightUpdate(uint32 messageIndex)
public
virtual
returns (bytes32 validationID, uint64 nonce);
function completeValidatorWeightUpdate(
uint32 messageIndex
) public virtual returns (bytes32 validationID, uint64 nonce);
}
```

Expand Down