PoolValidators Manual Review Findings

PVS-01M: Inexistent Removal of Validator Status

Description:

The removeOperator function does not completely omit the operator entry from the contract as the validatorStatuses entry remains unaffected.

Example:

Copy
136/**
137 * @dev See {IPoolValidators-removeOperator}.
138 */
139function removeOperator(address _operator) external override whenNotPaused {
140    require(hasRole(DEFAULT_ADMIN_ROLE, msg.sender) || msg.sender == _operator, "PoolValidators: access denied");
141
142    Operator storage operator = operators[_operator];
143    require(operator.initializeMerkleRoot != "", "PoolValidators: invalid operator");
144    require(!operator.locked, "PoolValidators: operator is locked");
145
146    // clean up operator
147    delete operators[_operator];
148
149    emit OperatorRemoved(msg.sender, _operator);
150}

Recommendation:

We advise the status to also be properly updated as in the current implementation an operator can remove themself, withdraw their collateral and remain Finalized which may be an undesirable logic path.

Alleviation:

The Stakewise team responded that the code is performing according to the specification as the validatorStatuses is meant to represent the current registration status of the validator and shouldn't be cleaned up when a validator is removed. In light of this, we consider this exhibit null.