WIP: Staking contract #453
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| function _unlock(uint256 value, address to) internal virtual override { | ||
| payable(to).sendValue(value); | ||
| } |
Check warning
Code scanning / Slither
Dead-code
contracts/staking/StakingManager.sol
Outdated
| // Ensure the registration expiry is in a valid range. | ||
| require( | ||
| registrationExpiry > block.timestamp && block.timestamp + 2 days > registrationExpiry, | ||
| "StakingManager: Invalid registration expiry" |
There was a problem hiding this comment.
Solidity 0.8.26 allows you to use custom errors in require(). They result in smaller contract sizes and are IMO much better for tests because you can assert the exact reason for a failure without risk of introducing a change-detector test if you modify the error string. If you want to stick to 0.8.25 then I'd recommend switching to if (!cond) { revert SpecificError() } where you originally had require(cond, ...).
There was a problem hiding this comment.
Our versioning policy is to use the second-most-recent release to hedge against potential issues in the latest release. That said, we've struggled to choose between revert strings and custom errors in the past, so getting the best of both worlds will be a welcome change.
| // (c) 2024, Ava Labs, Inc. All rights reserved. | ||
| // See the file LICENSE for licensing terms. | ||
|
|
||
| // SPDX-License-Identifier: Ecosystem |
There was a problem hiding this comment.
(nit) Confirm with a lawyer, but I think this should be UNLICENSED per Solidity conventions for non-OSS licenses (not UNLICENSE without the D). There are specific license identifiers for SPDX.
| * | 52 bytes | | ||
| * +----------+ | ||
| */ | ||
| function packSetSubnetValidatorWeightMessage( |
There was a problem hiding this comment.
This can be simplified to:
return abi.encodePacked(
SET_SUBNET_VALIDATOR_WEIGHT_MESSAGE_TYPE_ID,
validationID,
nonce,
weight
);There was a problem hiding this comment.
There are similar improvements that can be made to unpacking, but it was easier to write code so I created a PR, based off this branch, to demonstrate.
Together they save about 39,500 gas on the pack-unpack round trip.
There was a problem hiding this comment.
I only just saw the TODO re gas efficiency 🤦 sorry!
contracts/staking/StakingManager.sol
Outdated
| */ | ||
| function initializeEndValidation( | ||
| bytes32 validationID, | ||
| bool includeUptimeProof, |
There was a problem hiding this comment.
hmm i wonder if this can lead to a fat-fingering situation by providing the warp message but not enabling this flag. sorry if I'm being too nitpicky.
contracts/staking/StakingManager.sol
Outdated
| require(valid, "StakingManager: Invalid warp message"); | ||
|
|
||
| bytes32 validationID; | ||
| if (setWeightMessageType) { |
There was a problem hiding this comment.
did we confirm that both SetWeightTx=0 and InvalidMsg can be signed for this one? I wonder if it makes a better UX to expect a single msg type to be signed/relayed.
| function __ERC20TokenStakingManager_init( | ||
| PoSValidatorManagerSettings calldata settings, | ||
| IERC20 token | ||
| ) internal onlyInitializing { | ||
| __POS_Validator_Manager_init(settings); | ||
| __ERC20TokenStakingManager_init_unchained(token); | ||
| } |
Check warning
Code scanning / Slither
Conformance to Solidity naming conventions
| function __NativeTokenStakingManager_init(PoSValidatorManagerSettings calldata settings) | ||
| internal | ||
| onlyInitializing | ||
| { | ||
| __POS_Validator_Manager_init(settings); | ||
| } |
Check warning
Code scanning / Slither
Conformance to Solidity naming conventions
| function __PoAValidatorManager_init( | ||
| ValidatorManagerSettings calldata settings, | ||
| address initialOwner | ||
| ) internal onlyInitializing { | ||
| __ValidatorManager_init(settings); | ||
| __Ownable_init(initialOwner); | ||
| } |
Check warning
Code scanning / Slither
Conformance to Solidity naming conventions
| } | ||
|
|
||
| // solhint-disable-next-line no-empty-blocks | ||
| function __PoAValidatorManager_init_unchained() internal onlyInitializing {} |
Check warning
Code scanning / Slither
Dead-code
| } | ||
|
|
||
| // solhint-disable-next-line no-empty-blocks | ||
| function __PoAValidatorManager_init_unchained() internal onlyInitializing {} |
Check warning
Code scanning / Slither
Conformance to Solidity naming conventions
| function resendRegisterValidatorMessage(bytes32 validationID) external { | ||
| ValidatorManagerStorage storage $ = _getValidatorManagerStorage(); | ||
| require( | ||
| $._pendingRegisterValidationMessages[validationID].length > 0 | ||
| && $._validationPeriods[validationID].status == ValidatorStatus.PendingAdded, | ||
| "ValidatorManager: invalid validation ID" | ||
| ); | ||
|
|
||
| // Submit the message to the Warp precompile. | ||
| WARP_MESSENGER.sendWarpMessage($._pendingRegisterValidationMessages[validationID]); | ||
| } |
Check warning
Code scanning / Slither
Unused return
| function _initializeEndValidation(bytes32 validationID) internal virtual { | ||
| ValidatorManagerStorage storage $ = _getValidatorManagerStorage(); | ||
|
|
||
| // Ensure the validation period is active. | ||
| Validator memory validator = $._validationPeriods[validationID]; | ||
| require( | ||
| validator.status == ValidatorStatus.Active, "ValidatorManager: validator not active" | ||
| ); | ||
| require(_msgSender() == validator.owner, "ValidatorManager: sender not validator owner"); | ||
|
|
||
| // Check that removing this validator would not exceed the maximum churn rate. | ||
| _checkAndUpdateChurnTracker(validator.weight); | ||
|
|
||
| // Update the validator status to pending removal. | ||
| // They are not removed from the active validators mapping until the P-Chain acknowledges the removal. | ||
| validator.status = ValidatorStatus.PendingRemoved; | ||
|
|
||
| // Set the end time of the validation period, since it is no longer known to be an active validator | ||
| // on the P-Chain. | ||
| validator.endedAt = uint64(block.timestamp); | ||
|
|
||
| // Save the validator updates. | ||
| // TODO: Optimize storage writes here (probably don't need to write the whole value). | ||
| $._validationPeriods[validationID] = validator; | ||
|
|
||
| // Submit the message to the Warp precompile. | ||
| bytes memory setValidatorWeightPayload = ValidatorMessages | ||
| .packSetSubnetValidatorWeightMessage(validationID, validator.messageNonce, 0); | ||
| bytes32 messageID = WARP_MESSENGER.sendWarpMessage(setValidatorWeightPayload); | ||
|
|
||
| // Emit the event to signal the start of the validator removal process. | ||
| emit ValidatorRemovalInitialized(validationID, messageID, validator.weight, block.timestamp); | ||
| } |
Check notice
Code scanning / Slither
Reentrancy vulnerabilities
| function _initializeEndValidation(bytes32 validationID) internal virtual { | ||
| ValidatorManagerStorage storage $ = _getValidatorManagerStorage(); | ||
|
|
||
| // Ensure the validation period is active. | ||
| Validator memory validator = $._validationPeriods[validationID]; | ||
| require( | ||
| validator.status == ValidatorStatus.Active, "ValidatorManager: validator not active" | ||
| ); | ||
| require(_msgSender() == validator.owner, "ValidatorManager: sender not validator owner"); | ||
|
|
||
| // Check that removing this validator would not exceed the maximum churn rate. | ||
| _checkAndUpdateChurnTracker(validator.weight); | ||
|
|
||
| // Update the validator status to pending removal. | ||
| // They are not removed from the active validators mapping until the P-Chain acknowledges the removal. | ||
| validator.status = ValidatorStatus.PendingRemoved; | ||
|
|
||
| // Set the end time of the validation period, since it is no longer known to be an active validator | ||
| // on the P-Chain. | ||
| validator.endedAt = uint64(block.timestamp); | ||
|
|
||
| // Save the validator updates. | ||
| // TODO: Optimize storage writes here (probably don't need to write the whole value). | ||
| $._validationPeriods[validationID] = validator; | ||
|
|
||
| // Submit the message to the Warp precompile. | ||
| bytes memory setValidatorWeightPayload = ValidatorMessages | ||
| .packSetSubnetValidatorWeightMessage(validationID, validator.messageNonce, 0); | ||
| bytes32 messageID = WARP_MESSENGER.sendWarpMessage(setValidatorWeightPayload); | ||
|
|
||
| // Emit the event to signal the start of the validator removal process. | ||
| emit ValidatorRemovalInitialized(validationID, messageID, validator.weight, block.timestamp); | ||
| } |
Check notice
Code scanning / Slither
Block timestamp
| function resendEndValidatorMessage(bytes32 validationID) external { | ||
| ValidatorManagerStorage storage $ = _getValidatorManagerStorage(); | ||
| Validator memory validator = $._validationPeriods[validationID]; | ||
|
|
||
| require( | ||
| validator.status == ValidatorStatus.PendingRemoved, | ||
| "ValidatorManager: validator not pending removal" | ||
| ); | ||
|
|
||
| bytes memory setValidatorWeightPayload = ValidatorMessages | ||
| .packSetSubnetValidatorWeightMessage(validationID, validator.messageNonce, 0); | ||
| WARP_MESSENGER.sendWarpMessage(setValidatorWeightPayload); | ||
| } |
Check warning
Code scanning / Slither
Unused return
Co-authored-by: minghinmatthewlam <[email protected]> Signed-off-by: cam-schultz <[email protected]>
Delegation PoC
| function _getValidatorManagerStorage() | ||
| private | ||
| pure | ||
| returns (ValidatorManagerStorage storage $) | ||
| { | ||
| // solhint-disable-next-line no-inline-assembly | ||
| assembly { | ||
| $.slot := _VALIDATOR_MANAGER_STORAGE_LOCATION | ||
| } | ||
| } |
Check warning
Code scanning / Slither
Assembly usage
| function _checkAndUpdateChurnTracker(uint64 amount) internal { | ||
| ValidatorManagerStorage storage $ = _getValidatorManagerStorage(); | ||
| if ($._maximumHourlyChurn == 0) { | ||
| return; | ||
| } | ||
|
|
||
| ValidatorChurnPeriod memory churnTracker = $._churnTracker; | ||
| uint256 currentTime = block.timestamp; | ||
| if (currentTime - churnTracker.startedAt >= 1 hours) { | ||
| churnTracker.churnAmount = amount; | ||
| churnTracker.startedAt = currentTime; | ||
| } else { | ||
| churnTracker.churnAmount += amount; | ||
| } | ||
|
|
||
| uint8 churnPercentage = uint8((churnTracker.churnAmount * 100) / churnTracker.initialStake); | ||
| require( | ||
| churnPercentage <= $._maximumHourlyChurn, | ||
| "ValidatorManager: maximum hourly churn rate exceeded" | ||
| ); | ||
| $._churnTracker = churnTracker; | ||
| } |
Check notice
Code scanning / Slither
Block timestamp
Track uptimes for delegation period
| function resendDelegatorRegistration(bytes32 delegationID) external { | ||
| _checkPendingRegisterDelegatorMessages(delegationID); | ||
| PoSValidatorManagerStorage storage $ = _getPoSValidatorManagerStorage(); | ||
| // Submit the message to the Warp precompile. | ||
| WARP_MESSENGER.sendWarpMessage($._pendingRegisterDelegatorMessages[delegationID]); | ||
| } |
Check warning
Code scanning / Slither
Unused return
| function completeDelegatorRegistration(uint32 messageIndex, bytes32 delegationID) external { | ||
| PoSValidatorManagerStorage storage $ = _getPoSValidatorManagerStorage(); | ||
|
|
||
| // Unpack the Warp message | ||
| WarpMessage memory warpMessage = _getPChainWarpMessage(messageIndex); | ||
| (bytes32 validationID, uint64 nonce,) = | ||
| ValidatorMessages.unpackSubnetValidatorWeightUpdateMessage(warpMessage.payload); | ||
| _checkPendingRegisterDelegatorMessages(delegationID); | ||
| delete $._pendingRegisterDelegatorMessages[delegationID]; | ||
|
|
||
| Validator memory validator = _getValidator(validationID); | ||
|
|
||
| // The received nonce should be no greater than the highest sent nonce | ||
| require(validator.messageNonce >= nonce, "PoSValidatorManager: invalid nonce"); | ||
| // It should also be greater than or equal to the delegationID's starting nonce | ||
| require( | ||
| $._delegatorStakes[delegationID].startingNonce <= nonce, | ||
| "PoSValidatorManager: nonce does not match" | ||
| ); | ||
|
|
||
| require( | ||
| $._delegatorStakes[delegationID].status == DelegatorStatus.PendingAdded, | ||
| "PoSValidatorManager: delegationID not pending added" | ||
| ); | ||
| // Update the delegation status | ||
| $._delegatorStakes[delegationID].status = DelegatorStatus.Active; | ||
| $._delegatorStakes[delegationID].startedAt = uint64(block.timestamp); | ||
| emit DelegatorRegistered({ | ||
| delegationID: delegationID, | ||
| validationID: validationID, | ||
| nonce: nonce, | ||
| startTime: block.timestamp | ||
| }); | ||
| } |
Check warning
Code scanning / Slither
Unused return
| function initializeEndDelegation( | ||
| bytes32 delegationID, | ||
| bool includeUptimeProof, | ||
| uint32 messageIndex | ||
| ) external { | ||
| PoSValidatorManagerStorage storage $ = _getPoSValidatorManagerStorage(); | ||
| bytes32 validationID = $._delegatorStakes[delegationID].validationID; | ||
|
|
||
| uint64 uptime; | ||
| if (includeUptimeProof) { | ||
| uptime = _getUptime(validationID, messageIndex); | ||
| } | ||
|
|
||
| // TODO: Calculate the delegator's reward, but do not unlock it | ||
|
|
||
| // Ensure the delegator is active | ||
| Delegator memory delegator = $._delegatorStakes[delegationID]; | ||
| require( | ||
| delegator.owner == _msgSender(), "PoSValidatorManager: delegation not owned by sender" | ||
| ); | ||
| require( | ||
| delegator.status == DelegatorStatus.Active, "PoSValidatorManager: delegation not active" | ||
| ); | ||
| uint64 nonce = _getAndIncrementNonce(validationID); | ||
| delegator.status = DelegatorStatus.PendingRemoved; | ||
| delegator.endedAt = uint64(block.timestamp); | ||
| delegator.endingNonce = nonce; | ||
|
|
||
| $._delegatorStakes[delegationID] = delegator; | ||
|
|
||
| Validator memory validator = _getValidator(validationID); | ||
| require(validator.weight > delegator.weight, "PoSValidatorManager: Invalid weight"); | ||
| uint64 newValidatorWeight = validator.weight - delegator.weight; | ||
| _setValidatorWeight(validationID, newValidatorWeight); | ||
|
|
||
| // Submit the message to the Warp precompile. | ||
| bytes memory setValidatorWeightPayload = ValidatorMessages | ||
| .packSetSubnetValidatorWeightMessage(validationID, nonce, newValidatorWeight); | ||
| $._pendingEndDelegatorMessages[delegationID] = setValidatorWeightPayload; | ||
| bytes32 messageID = WARP_MESSENGER.sendWarpMessage(setValidatorWeightPayload); | ||
|
|
||
| emit DelegatorRemovalInitialized({ | ||
| delegationID: delegationID, | ||
| validationID: validationID, | ||
| nonce: nonce, | ||
| validatorWeight: newValidatorWeight, | ||
| endTime: block.timestamp, | ||
| setWeightMessageID: messageID | ||
| }); | ||
| } |
Check notice
Code scanning / Slither
Reentrancy vulnerabilities
| function initializeEndDelegation( | ||
| bytes32 delegationID, | ||
| bool includeUptimeProof, | ||
| uint32 messageIndex | ||
| ) external { | ||
| PoSValidatorManagerStorage storage $ = _getPoSValidatorManagerStorage(); | ||
| bytes32 validationID = $._delegatorStakes[delegationID].validationID; | ||
|
|
||
| uint64 uptime; | ||
| if (includeUptimeProof) { | ||
| uptime = _getUptime(validationID, messageIndex); | ||
| } | ||
|
|
||
| // TODO: Calculate the delegator's reward, but do not unlock it | ||
|
|
||
| // Ensure the delegator is active | ||
| Delegator memory delegator = $._delegatorStakes[delegationID]; | ||
| require( | ||
| delegator.owner == _msgSender(), "PoSValidatorManager: delegation not owned by sender" | ||
| ); | ||
| require( | ||
| delegator.status == DelegatorStatus.Active, "PoSValidatorManager: delegation not active" | ||
| ); | ||
| uint64 nonce = _getAndIncrementNonce(validationID); | ||
| delegator.status = DelegatorStatus.PendingRemoved; | ||
| delegator.endedAt = uint64(block.timestamp); | ||
| delegator.endingNonce = nonce; | ||
|
|
||
| $._delegatorStakes[delegationID] = delegator; | ||
|
|
||
| Validator memory validator = _getValidator(validationID); | ||
| require(validator.weight > delegator.weight, "PoSValidatorManager: Invalid weight"); | ||
| uint64 newValidatorWeight = validator.weight - delegator.weight; | ||
| _setValidatorWeight(validationID, newValidatorWeight); | ||
|
|
||
| // Submit the message to the Warp precompile. | ||
| bytes memory setValidatorWeightPayload = ValidatorMessages | ||
| .packSetSubnetValidatorWeightMessage(validationID, nonce, newValidatorWeight); | ||
| $._pendingEndDelegatorMessages[delegationID] = setValidatorWeightPayload; | ||
| bytes32 messageID = WARP_MESSENGER.sendWarpMessage(setValidatorWeightPayload); | ||
|
|
||
| emit DelegatorRemovalInitialized({ | ||
| delegationID: delegationID, | ||
| validationID: validationID, | ||
| nonce: nonce, | ||
| validatorWeight: newValidatorWeight, | ||
| endTime: block.timestamp, | ||
| setWeightMessageID: messageID | ||
| }); | ||
| } |
Check notice
Code scanning / Slither
Block timestamp
| function resendEndDelegation(bytes32 delegationID) external { | ||
| _checkPendingEndDelegatorMessage(delegationID); | ||
| PoSValidatorManagerStorage storage $ = _getPoSValidatorManagerStorage(); | ||
| WARP_MESSENGER.sendWarpMessage($._pendingEndDelegatorMessages[delegationID]); | ||
| } |
Check warning
Code scanning / Slither
Unused return
| function completeEndDelegation(uint32 messageIndex, bytes32 delegationID) external { | ||
| PoSValidatorManagerStorage storage $ = _getPoSValidatorManagerStorage(); | ||
|
|
||
| // Unpack the Warp message | ||
| WarpMessage memory warpMessage = _getPChainWarpMessage(messageIndex); | ||
| (bytes32 validationID, uint64 nonce,) = | ||
| ValidatorMessages.unpackSubnetValidatorWeightUpdateMessage(warpMessage.payload); | ||
| _checkPendingEndDelegatorMessage(delegationID); | ||
| delete $._pendingEndDelegatorMessages[delegationID]; | ||
|
|
||
| Validator memory validator = _getValidator(validationID); | ||
| // The received nonce should be no greater than the highest sent nonce | ||
| require(validator.messageNonce >= nonce, "PoSValidatorManager: invalid nonce"); | ||
| // It should also be greater than or equal to the delegator's ending nonce | ||
| require( | ||
| $._delegatorStakes[delegationID].endingNonce <= nonce, | ||
| "PoSValidatorManager: nonce does not match" | ||
| ); | ||
|
|
||
| require( | ||
| $._delegatorStakes[delegationID].status == DelegatorStatus.PendingRemoved, | ||
| "PoSValidatorManager: delegation not pending added" | ||
| ); | ||
|
|
||
| // Update the delegator status | ||
| $._delegatorStakes[delegationID].status = DelegatorStatus.Completed; | ||
|
|
||
| // TODO: Unlock the delegator's stake and their reward | ||
|
|
||
| emit DelegationEnded(delegationID, validationID, nonce); | ||
| } |
Check warning
Code scanning / Slither
Unused return
Signed-off-by: Geoff Stuart <[email protected]>
Co-authored-by: Ian Suvak <[email protected]> Signed-off-by: cam-schultz <[email protected]>
Custom errors
Small fixes pre-audit
update vdr manager readme
This is a draft PR for the Staking Contract feature branch. This is to make sure CI is passing, and to have a place for team members to leave comments.
This should roughly track with this milestone https://github.com/ava-labs/teleporter/milestone/2