From 3f289c80871be38a2430a89348798881719c2fd4 Mon Sep 17 00:00:00 2001 From: cam-schultz Date: Tue, 21 Jan 2025 16:31:59 -0600 Subject: [PATCH] lint + doccomments --- .../validator-manager/PoAValidatorManager.sol | 22 +++---- .../validator-manager/PoSValidatorManager.sol | 61 ++++++++++++------- .../validator-manager/ValidatorManager.sol | 30 ++++----- .../interfaces/IPoSValidatorManager.sol | 5 +- 4 files changed, 65 insertions(+), 53 deletions(-) diff --git a/contracts/validator-manager/PoAValidatorManager.sol b/contracts/validator-manager/PoAValidatorManager.sol index 82395ab7b..09594dd8a 100644 --- a/contracts/validator-manager/PoAValidatorManager.sol +++ b/contracts/validator-manager/PoAValidatorManager.sol @@ -5,7 +5,7 @@ pragma solidity 0.8.25; -import {ValidatorManager, ValidatorManagerSettings} from "./ValidatorManager.sol"; +import {ValidatorManager} from "./ValidatorManager.sol"; import {IPoAValidatorManager, PChainOwner} from "./interfaces/IPoAValidatorManager.sol"; import {ICMInitializable} from "@utilities/ICMInitializable.sol"; import {OwnableUpgradeable} from @@ -18,14 +18,17 @@ import {OwnableUpgradeable} from */ contract PoAValidatorManager is IPoAValidatorManager, OwnableUpgradeable { // solhint-disable private-vars-leading-underscore + /// @custom:storage-location erc7201:avalanche-icm.storage.PoAValidatorManager struct PoAValidatorManagerStorage { ValidatorManager _manager; } + // solhint-enable private-vars-leading-underscore // keccak256(abi.encode(uint256(keccak256("avalanche-icm.storage.PoAValidatorManager")) - 1)) & ~bytes32(uint256(0xff)); bytes32 public constant POA_VALIDATOR_MANAGER_STORAGE_LOCATION = 0x81773fca73a14ca21edf1cadc6ec0b26d6a44966f6e97607e90422658d423500; + // solhint-disable ordering function _getPoAValidatorManagerStorage() private pure @@ -43,10 +46,7 @@ contract PoAValidatorManager is IPoAValidatorManager, OwnableUpgradeable { } } - function initialize( - ValidatorManager manager, - address initialOwner - ) external initializer { + function initialize(ValidatorManager manager, address initialOwner) external initializer { __PoAValidatorManager_init(manager, initialOwner); } @@ -60,7 +60,10 @@ contract PoAValidatorManager is IPoAValidatorManager, OwnableUpgradeable { } // solhint-disable-next-line no-empty-blocks - function __PoAValidatorManager_init_unchained(ValidatorManager manager) internal onlyInitializing { + function __PoAValidatorManager_init_unchained(ValidatorManager manager) + internal + onlyInitializing + { PoAValidatorManagerStorage storage $ = _getPoAValidatorManagerStorage(); $._manager = manager; } @@ -97,12 +100,9 @@ contract PoAValidatorManager is IPoAValidatorManager, OwnableUpgradeable { } /** - * @notice See {ACP99Manager-completeValidatorRemoval}. + * @notice Completes validator removal by forwarding to the validator manager. */ - function completeValidatorRemoval(uint32 messageIndex) - public - returns (bytes32) - { + function completeValidatorRemoval(uint32 messageIndex) public returns (bytes32) { return _getPoAValidatorManagerStorage()._manager.completeValidatorRemoval(messageIndex); } diff --git a/contracts/validator-manager/PoSValidatorManager.sol b/contracts/validator-manager/PoSValidatorManager.sol index 8b48a430d..6ae6f31f7 100644 --- a/contracts/validator-manager/PoSValidatorManager.sol +++ b/contracts/validator-manager/PoSValidatorManager.sol @@ -14,10 +14,12 @@ import { PoSValidatorInfo, PoSValidatorManagerSettings } from "./interfaces/IPoSValidatorManager.sol"; -import {ACP99Manager, Validator, ValidatorStatus, PChainOwner} from "./ACP99Manager.sol"; +import {Validator, ValidatorStatus, PChainOwner} from "./ACP99Manager.sol"; import {IRewardCalculator} from "./interfaces/IRewardCalculator.sol"; -import {IWarpMessenger, WarpMessage} from - "@avalabs/subnet-evm-contracts@1.2.0/contracts/interfaces/IWarpMessenger.sol"; +import { + IWarpMessenger, + WarpMessage +} from "@avalabs/subnet-evm-contracts@1.2.0/contracts/interfaces/IWarpMessenger.sol"; import {ReentrancyGuardUpgradeable} from "@openzeppelin/contracts-upgradeable@5.0.2/utils/ReentrancyGuardUpgradeable.sol"; import {ContextUpgradeable} from @@ -212,7 +214,8 @@ abstract contract PoSValidatorManager is if (!_isPoSValidator(validationID)) { revert ValidatorNotPoS(validationID); } - ValidatorStatus status = _getPoSValidatorManagerStorage()._manager.getValidator(validationID).status; + ValidatorStatus status = + _getPoSValidatorManagerStorage()._manager.getValidator(validationID).status; if (status != ValidatorStatus.Active) { revert InvalidValidatorStatus(status); } @@ -403,19 +406,28 @@ abstract contract PoSValidatorManager is } /** - * @notice See {ACP99Manager-completeValidatorRemoval}. + * Completes validator removal by dispatching to the ValidatorManager to update the validator status, + * and unlocking stake. The ValidatorManager's completeValidatorRemoval function may be called directly, + * in which case this function requires the validationID to be passed as a parameter. + * + * @param validationID The ID of the validator to remove. Only required if the validator removal has already + * been completed directly through the ValidatorManager + * @param messageIndex The index of the ICM message to be received providing the acknowledgement from the P-Chain. + * This is forwarded to the ValidatorManager to be parsed. */ - function completeValidatorRemoval(bytes32 validationID, uint32 messageIndex) - public - nonReentrant - returns (bytes32) - { + function completeValidatorRemoval( + bytes32 validationID, + uint32 messageIndex + ) public nonReentrant returns (bytes32) { PoSValidatorManagerStorage storage $ = _getPoSValidatorManagerStorage(); Validator memory validator = $._manager.getValidator(validationID); // Check if the validator has been already been removed from the validator manager. - if (validator.status != ValidatorStatus.Completed && validator.status != ValidatorStatus.Invalidated) { - $._manager.completeValidatorRemoval(messageIndex); + if ( + validator.status != ValidatorStatus.Completed + && validator.status != ValidatorStatus.Invalidated + ) { + validationID = $._manager.completeValidatorRemoval(messageIndex); validator = $._manager.getValidator(validationID); } @@ -791,8 +803,9 @@ abstract contract PoSValidatorManager is // initiate the removal. $._delegatorStakes[delegationID].status = DelegatorStatus.PendingRemoved; - ($._delegatorStakes[delegationID].endingNonce,) = - $._manager.initiateValidatorWeightUpdate(validationID, validator.weight - delegator.weight); + ($._delegatorStakes[delegationID].endingNonce,) = $ + ._manager + .initiateValidatorWeightUpdate(validationID, validator.weight - delegator.weight); uint256 reward = _calculateAndSetDelegationReward(delegator, rewardRecipient, delegationID); @@ -909,7 +922,8 @@ abstract contract PoSValidatorManager is $._manager.getValidator(delegator.validationID).status != ValidatorStatus.Completed && validator.receivedNonce < delegator.endingNonce ) { - (bytes32 validationID, uint64 nonce) = $._manager.completeValidatorWeightUpdate(messageIndex); + (bytes32 validationID, uint64 nonce) = + $._manager.completeValidatorWeightUpdate(messageIndex); if (delegator.validationID != validationID) { revert InvalidValidationID(validationID); } @@ -1029,14 +1043,17 @@ abstract contract PoSValidatorManager is PChainOwner memory disableOwner, uint64 weight ) public returns (bytes32) { - return _getPoSValidatorManagerStorage()._manager.initiateValidatorRegistration( - nodeID, blsPublicKey, registrationExpiry, remainingBalanceOwner, disableOwner, weight - ); + return _getPoSValidatorManagerStorage()._manager.initiateValidatorRegistration({ + nodeID: nodeID, + blsPublicKey: blsPublicKey, + registrationExpiry: registrationExpiry, + remainingBalanceOwner: remainingBalanceOwner, + disableOwner: disableOwner, + weight: weight + }); } - function completeValidatorRegistration(uint32 messageIndex) - public - returns (bytes32) { - return _getPoSValidatorManagerStorage()._manager.completeValidatorRegistration(messageIndex); + function completeValidatorRegistration(uint32 messageIndex) public returns (bytes32) { + return _getPoSValidatorManagerStorage()._manager.completeValidatorRegistration(messageIndex); } } diff --git a/contracts/validator-manager/ValidatorManager.sol b/contracts/validator-manager/ValidatorManager.sol index bcb033430..7b7b50b04 100644 --- a/contracts/validator-manager/ValidatorManager.sol +++ b/contracts/validator-manager/ValidatorManager.sol @@ -25,7 +25,6 @@ import {Initializable} from "@openzeppelin/contracts-upgradeable@5.0.2/proxy/utils/Initializable.sol"; import {ICMInitializable} from "@utilities/ICMInitializable.sol"; - /** * @dev Describes the current churn period */ @@ -142,7 +141,6 @@ contract ValidatorManager is Initializable, ContextUpgradeable, ACP99Manager { function initialize(ValidatorManagerSettings calldata settings) external initializer { __ValidatorManager_init(settings); } - // solhint-disable-next-line func-name-mixedcase function __ValidatorManager_init(ValidatorManagerSettings calldata settings) @@ -181,7 +179,10 @@ contract ValidatorManager is Initializable, ContextUpgradeable, ACP99Manager { } modifier onlyAdmin() { - require(msg.sender == _getValidatorManagerStorage()._admin, "ValidatorManager: unauthorized caller"); + require( + msg.sender == _getValidatorManagerStorage()._admin, + "ValidatorManager: unauthorized caller" + ); _; } @@ -281,14 +282,14 @@ contract ValidatorManager is Initializable, ContextUpgradeable, ACP99Manager { PChainOwner memory disableOwner, uint64 weight ) public onlyAdmin returns (bytes32) { - return _initiateValidatorRegistration( - nodeID, - blsPublicKey, - registrationExpiry, - remainingBalanceOwner, - disableOwner, - weight - ); + return _initiateValidatorRegistration({ + nodeID: nodeID, + blsPublicKey: blsPublicKey, + registrationExpiry: registrationExpiry, + remainingBalanceOwner: remainingBalanceOwner, + disableOwner: disableOwner, + weight: weight + }); } /** @@ -544,12 +545,7 @@ contract ValidatorManager is Initializable, ContextUpgradeable, ACP99Manager { } /** - * @notice Completes the process of ending a validation period by receiving an acknowledgement from the P-Chain - * that the validation ID is not active and will never be active in the future. - * Note: that this function can be used for successful validation periods that have been explicitly - * ended by calling {_initiateValidatorRemoval} or for validation periods that never began on the P-Chain due to the - * {registrationExpiry} being reached. - * @return (Validation ID, Validator instance) representing the completed validation period. + * @notice See {ACP99Manager-completeValidatorRemoval}. */ function completeValidatorRemoval(uint32 messageIndex) public diff --git a/contracts/validator-manager/interfaces/IPoSValidatorManager.sol b/contracts/validator-manager/interfaces/IPoSValidatorManager.sol index 10da95300..4feb2bc96 100644 --- a/contracts/validator-manager/interfaces/IPoSValidatorManager.sol +++ b/contracts/validator-manager/interfaces/IPoSValidatorManager.sol @@ -7,7 +7,6 @@ pragma solidity 0.8.25; import {ValidatorManager} from "../ValidatorManager.sol"; import {IRewardCalculator} from "./IRewardCalculator.sol"; -import {ACP99Manager} from "../ACP99Manager.sol"; /** * @dev Delegator status @@ -193,7 +192,7 @@ interface IPoSValidatorManager { /** * @notice Completes the delegator registration process by submitting an acknowledgement of the registration of a - * validationID from the P-Chain. After this function is called, the validator's weight is updated in the contract state. + * validationID from the P-Chain. This function may be called after {ACP99Manager-completeValidatorWeightUpdate}. * Any P-Chain acknowledgement with a nonce greater than or equal to the nonce used to initiate registration of the * delegator is valid, as long as that nonce has been sent by the contract. For the purposes of computing delegation rewards, * the delegation is considered active after this function is completed. @@ -276,7 +275,7 @@ interface IPoSValidatorManager { /** * @notice Completes the process of ending a delegation by receiving an acknowledgement from the P-Chain. - * After this function is called, the validator's weight is updated in the contract state. + * This function may be called after {ACP99Manager-completeValidatorWeightUpdate}. * Any P-Chain acknowledgement with a nonce greater than or equal to the nonce used to initiate the end of the * delegator's delegation is valid, as long as that nonce has been sent by the contract. This is because the validator * weight change pertaining to the delegation ending is included in any subsequent validator weight update messages.