Merge pull request #530 from ava-labs/delegation-comments

Expand delegation comments

contracts/staking/PoSValidatorManager.sol

@@ -173,7 +173,9 @@ abstract contract PoSValidatorManager is IPoSValidatorManager, ValidatorManager
         $._pendingRegisterDelegatorMessages[delegationID] = setValidatorWeightPayload;
         bytes32 messageID = WARP_MESSENGER.sendWarpMessage(setValidatorWeightPayload);
 
-        // Store the delegation information
+        // Store the delegation information. Set the delegator status to pending added,
+        // so that it can be properly started in the complete step, even if the delivered
+        // nonce is greater than the nonce used to initialize registration.
         $._delegatorStakes[delegationID] = Delegator({
             status: DelegatorStatus.PendingAdded,
             owner: delegatorAddress,
@@ -216,14 +218,23 @@ abstract contract PoSValidatorManager is IPoSValidatorManager, ValidatorManager
 
         Validator memory validator = _getValidator(validationID);
 
-        // The received nonce should be no greater than the highest sent nonce
+        // The received nonce should be no greater than the highest sent nonce. This should never
+        // happen since the staking manager is the only entity that can trigger a weight update
+        // on the P-Chain.
         require(validator.messageNonce >= nonce, "PoSValidatorManager: invalid nonce");
-        // It should also be greater than or equal to the delegationID's starting nonce
+
+        // The nonce should also be greater than or equal to the delegationID's starting nonce. This allows
+        // a weight update using a higher nonce (which implicitly includes the delegation's weight update)
+        // to be used to complete registration for an earlier delegation. This is necessary because the P-Chain
+        // is only willing to sign the latest weight update.
         require(
             $._delegatorStakes[delegationID].startingNonce <= nonce,
             "PoSValidatorManager: nonce does not match"
         );
 
+        // Ensure the delegator is pending added. Since anybody can call this function once
+        // delegator registration has been initialized, we need to make sure that this function is only
+        // callable after that has been done.
         require(
             $._delegatorStakes[delegationID].status == DelegatorStatus.PendingAdded,
             "PoSValidatorManager: delegationID not pending added"
@@ -257,12 +268,17 @@ abstract contract PoSValidatorManager is IPoSValidatorManager, ValidatorManager
         // Ensure the delegator is active
         Delegator memory delegator = $._delegatorStakes[delegationID];
         require(
-            delegator.owner == _msgSender(), "PoSValidatorManager: delegation not owned by sender"
+            delegator.status == DelegatorStatus.Active, "PoSValidatorManager: delegation not active"
         );
+        // Only the delegation owner can end the delegation.
         require(
-            delegator.status == DelegatorStatus.Active, "PoSValidatorManager: delegation not active"
+            delegator.owner == _msgSender(), "PoSValidatorManager: delegation not owned by sender"
         );
         uint64 nonce = _getAndIncrementNonce(validationID);
+
+        // Set the delegator status to pending removed, so that it can be properly removed in
+        // the complete step, even if the delivered nonce is greater than the nonce used to
+        // initialize the removal.
         delegator.status = DelegatorStatus.PendingRemoved;
         delegator.endedAt = uint64(block.timestamp);
         delegator.endingNonce = nonce;
@@ -307,14 +323,23 @@ abstract contract PoSValidatorManager is IPoSValidatorManager, ValidatorManager
         delete $._pendingEndDelegatorMessages[delegationID];
 
         Validator memory validator = _getValidator(validationID);
-        // The received nonce should be no greater than the highest sent nonce
+        // The received nonce should be no greater than the highest sent nonce. This should never
+        // happen since the staking manager is the only entity that can trigger a weight update
+        // on the P-Chain.
         require(validator.messageNonce >= nonce, "PoSValidatorManager: invalid nonce");
-        // It should also be greater than or equal to the delegator's ending nonce
+
+        // The nonce should also be greater than or equal to the delegationID's ending nonce. This allows
+        // a weight update using a higher nonce (which implicitly includes the delegation's weight update)
+        // to be used to complete delisting for an earlier delegation. This is necessary because the P-Chain
+        // is only willing to sign the latest weight update.
         require(
             $._delegatorStakes[delegationID].endingNonce <= nonce,
             "PoSValidatorManager: nonce does not match"
         );
 
+        // Ensure the delegator is pending removed. Since anybody can call this function once
+        // end delegation has been initialized, we need to make sure that this function is only
+        // callable after that has been done.
         require(
             $._delegatorStakes[delegationID].status == DelegatorStatus.PendingRemoved,
             "PoSValidatorManager: delegation not pending added"

contracts/staking/interfaces/IPoSValidatorManager.sol

@@ -122,15 +122,20 @@ interface IPoSValidatorManager is IValidatorManager {
      * @notice Completes the delegator registration process by returning 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.
      * Any P-Chain acknowledgement with a nonce greater than or equal to the nonce used to initialize registration of the
-     * delegator is valid, as long as that nonce has been sent by the contract.
+     * 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 called.
+     * Note: only the specified delegation will be marked as registered, even if the validator weight update
+     * message implicitly includes multiple weight changes.
      * @param messageIndex The index of the Warp message to be received providing the acknowledgement.
      * @param delegationID The ID of the delegation being registered.
      */
     function completeDelegatorRegistration(uint32 messageIndex, bytes32 delegationID) external;
 
     /**
      * @notice Begins the process of removing a delegator from a validation period. The delegator must have been previously
-     * registered with the given validationID.
+     * registered with the given validationID. For the purposes of computing delegation rewards, the delegation period is
+     * considered ended when this function is called. In order to be eligible for rewards, an uptime proof must be provided.
+     * Note that this function can only be called by the address that registered the delegation.
      * @param delegationID The ID of the delegation being removed.
      * @param includeUptimeProof Whether or not an uptime proof is provided for the validation period.
      * If no uptime proof is provided, the validation uptime for the delegation period will be assumed to be 0.
@@ -154,7 +159,10 @@ interface IPoSValidatorManager is IValidatorManager {
      * @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.
      * Any P-Chain acknowledgement with a nonce greater than or equal to the nonce used to initialize the end of the
-     * delegator's delegation is valid, as long as that nonce has been sent by the contract.
+     * 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.
+     * Note: only the specified delegation will be marked as completed, even if the validator weight update
+     * message implicitly includes multiple weight changes.
      * @param messageIndex The index of the Warp message to be received providing the acknowledgement.
      * @param delegationID The ID of the delegation being removed.
      */