Skip to content

Commit

Permalink
Merge pull request #186 from valory-xyz/deposit_limit
Browse files Browse the repository at this point in the history 
refactor: account for staking contract deposit value
  • Loading branch information
@kupermind
kupermind authored Jul 19, 2024
2 parents 5e7c313 + c9a129e commit 3f80781
Show file tree
Hide file tree
Showing 23 changed files with 390 additions and 238 deletions.
18 changes: 18 additions & 0 deletions .gitleaksignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,3 +295,21 @@ cd83fb443a1513890a1da1be0389a2b853dcb93d:scripts/deployment/l2/globals_polygon_m
8a01b571ac0a4138882dacf81572de4753f37f1e:scripts/deployment/l2/globals_polygon_mainnet.json:generic-api-key:2
8a01b571ac0a4138882dacf81572de4753f37f1e:scripts/deployment/l2/globals_base_mainnet.json:generic-api-key:1
8a01b571ac0a4138882dacf81572de4753f37f1e:scripts/deployment/l2/globals_base_mainnet.json:generic-api-key:2
f6aa0cc9dcfba03dacec45e0ea0af309a72f3600:scripts/deployment/l2/globals_arbitrum_one.json:generic-api-key:1
f6aa0cc9dcfba03dacec45e0ea0af309a72f3600:scripts/deployment/l2/globals_arbitrum_sepolia.json:generic-api-key:1
f6aa0cc9dcfba03dacec45e0ea0af309a72f3600:scripts/deployment/l2/globals_base_mainnet.json:generic-api-key:1
f6aa0cc9dcfba03dacec45e0ea0af309a72f3600:scripts/deployment/l2/globals_celo_mainnet.json:generic-api-key:1
f6aa0cc9dcfba03dacec45e0ea0af309a72f3600:scripts/deployment/l2/globals_gnosis_chiado.json:generic-api-key:1
f6aa0cc9dcfba03dacec45e0ea0af309a72f3600:scripts/deployment/l2/globals_optimistic_mainnet.json:generic-api-key:1
f6aa0cc9dcfba03dacec45e0ea0af309a72f3600:scripts/deployment/l2/globals_optimistic_sepolia.json:generic-api-key:1
f6aa0cc9dcfba03dacec45e0ea0af309a72f3600:scripts/deployment/l2/globals_polygon_amoy.json:generic-api-key:1
f6aa0cc9dcfba03dacec45e0ea0af309a72f3600:scripts/deployment/l2/globals_polygon_mainnet.json:generic-api-key:1
f6aa0cc9dcfba03dacec45e0ea0af309a72f3600:scripts/deployment/l2/globals_arbitrum_one.json:generic-api-key:2
f6aa0cc9dcfba03dacec45e0ea0af309a72f3600:scripts/deployment/l2/globals_arbitrum_sepolia.json:generic-api-key:2
f6aa0cc9dcfba03dacec45e0ea0af309a72f3600:scripts/deployment/l2/globals_base_mainnet.json:generic-api-key:2
f6aa0cc9dcfba03dacec45e0ea0af309a72f3600:scripts/deployment/l2/globals_celo_mainnet.json:generic-api-key:2
f6aa0cc9dcfba03dacec45e0ea0af309a72f3600:scripts/deployment/l2/globals_gnosis_chiado.json:generic-api-key:2
f6aa0cc9dcfba03dacec45e0ea0af309a72f3600:scripts/deployment/l2/globals_optimistic_mainnet.json:generic-api-key:2
f6aa0cc9dcfba03dacec45e0ea0af309a72f3600:scripts/deployment/l2/globals_optimistic_sepolia.json:generic-api-key:2
f6aa0cc9dcfba03dacec45e0ea0af309a72f3600:scripts/deployment/l2/globals_polygon_amoy.json:generic-api-key:2
f6aa0cc9dcfba03dacec45e0ea0af309a72f3600:scripts/deployment/l2/globals_polygon_mainnet.json:generic-api-key:2
4 changes: 2 additions & 2 deletions abis/0.8.25/StakingFactory.json
View file

Large diffs are not rendered by default.

83 changes: 53 additions & 30 deletions abis/0.8.25/StakingVerifier.json
View file

Large diffs are not rendered by default.

87 changes: 87 additions & 0 deletions audits/internal6/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,93 @@ contracts/staking/StakingProxy.sol
It is necessary to carry out tests with the option to move this function to implementation.
Perhaps this will solve the problem with proxy recognition on the side gnosisscan.
```
[x] noted

#### Notes: StakingVerifier (commit 261c597388426e4e3a412123f50ee4dbe5e9fa8f)
```
some of the code just follows the same path ("fake if/elese") - since the path can't be changed it just wastes gas.
StakingFactory:
/// @dev Verifies a service staking contract instance.
/// @param instance Service staking proxy instance.
/// @return True, if verification is successful.
function verifyInstance(address instance) public view returns (bool) {
// Get proxy instance params
InstanceParams storage instanceParams = mapInstanceParams[instance];
address implementation = instanceParams.implementation;
// Check that the implementation corresponds to the proxy instance
if (implementation == address(0)) {
return false;
}
// Check for the instance being active
if (!instanceParams.isEnabled) {
return false;
}
// Provide additional checks, if needed
address localVerifier = verifier;
if (localVerifier != address(0)) {
return IStakingVerifier(localVerifier).verifyInstance(instance, implementation);
}
return true;
}
/// @dev Verifies staking proxy instance and gets emissions amount.
/// @param instance Staking proxy instance.
/// @return amount Emissions amount.
function verifyInstanceAndGetEmissionsAmount(address instance) external view returns (uint256 amount) {
// Verify the proxy instance
bool success = verifyInstance(instance);
if (success) {
// If there is a verifier, get the emissions amount
address localVerifier = verifier;
if (localVerifier != address(0)) {
// Get the max possible emissions amount
amount = IStakingVerifier(localVerifier).getEmissionsAmountLimit(instance);
} else {
// Get the proxy instance emissions amount
amount = IStaking(instance).emissionsAmount();
}
}
}
verifyInstanceAndGetEmissionsAmount:
1. verifyInstance(instance)
1.1. if localVerifier != address(0)
1.2. IStakingVerifier(localVerifier).verifyInstance(instance, implementation);
1.3. verifyInstance(address instance, address implementation) external view returns (bool)
if (apy > apyLimit) {
return false;
} and etc
if success (apy checking is success!) then
2. if (localVerifier != address(0)) {
2.1. amount = IStakingVerifier(localVerifier).getEmissionsAmountLimit(instance);
getEmissionsAmountLimit(instance)
->
amount = IStaking(instance).emissionsAmount();
so,
if (success) {
// If there is a verifier, get the emissions amount
address localVerifier = verifier;
if (localVerifier != address(0)) {
// Get the max possible emissions amount
amount = IStakingVerifier(localVerifier).getEmissionsAmountLimit(instance);
} else {
// Get the proxy instance emissions amount
amount = IStaking(instance).emissionsAmount();
}
}
always equal:
if (success) {
amount = IStaking(instance).emissionsAmount();
}
because the current code always produces this result.
```
[x] noted. No code base replacement required.

### Catch up on changes. 15.07.24
Expand Down
45 changes: 20 additions & 25 deletions contracts/staking/StakingFactory.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,9 @@ contract StakingFactory {
}

/// @dev Creates a service staking contract instance.
/// @notice Once the staking instance is created and verified, it is considered valid for its lifetime, or until
/// deliberately restricted / removed. If the DAO changes staking parameters, the outdated staking
/// parameters of instances are respected as they were validated before that.
/// @param implementation Service staking blanc implementation address.
/// @param initPayload Initialization payload.
function createStakingInstance(
Expand Down Expand Up @@ -296,29 +299,22 @@ contract StakingFactory {

emit InstanceRemoved(instance);
}

/// @dev Verifies a service staking contract instance.
/// @param instance Service staking proxy instance.
/// @return True, if verification is successful.
function verifyInstance(address instance) public view returns (bool) {
// Get proxy instance params
function verifyInstance(address instance) external view returns (bool) {
InstanceParams storage instanceParams = mapInstanceParams[instance];
address implementation = instanceParams.implementation;

// Check that the implementation corresponds to the proxy instance
if (implementation == address(0)) {
return false;
}

// Check for the instance being active
if (!instanceParams.isEnabled) {
bool isEnabled = instanceParams.isEnabled;
if (!isEnabled) {
return false;
}

// Provide additional checks, if needed
address localVerifier = verifier;
if (localVerifier != address(0)) {
return IStakingVerifier(localVerifier).verifyInstance(instance, implementation);
return IStakingVerifier(localVerifier).verifyInstance(instance, instanceParams.implementation);
}

return true;
Expand All @@ -328,22 +324,21 @@ contract StakingFactory {
/// @param instance Staking proxy instance.
/// @return amount Emissions amount.
function verifyInstanceAndGetEmissionsAmount(address instance) external view returns (uint256 amount) {
// Verify the proxy instance
bool success = verifyInstance(instance);

if (success) {
// Get the proxy instance emissions amount
amount = IStaking(instance).emissionsAmount();

// If there is a verifier, adjust the amount
// Check if the proxy instance is enabled, since this proves that
// the proxy instance has been created by this factory and verified at the creation time
// An already verified proxy instance should not be re-verified
// DAO governance might have changed verification rules in the mean-time which would render the instance unusable
bool isEnabled = mapInstanceParams[instance].isEnabled;

if (isEnabled) {
// If there is a verifier, get the emissions amount
address localVerifier = verifier;
if (localVerifier != address(0)) {
// Get the max possible emissions amount
uint256 maxEmissions = IStakingVerifier(localVerifier).getEmissionsAmountLimit(instance);
// Limit excessive emissions amount
if (amount > maxEmissions) {
amount = maxEmissions;
}
amount = IStakingVerifier(localVerifier).getEmissionsAmountLimit(instance);
} else {
// Get the proxy instance emissions amount
amount = IStaking(instance).emissionsAmount();
}
}
}
Expand Down
92 changes: 65 additions & 27 deletions contracts/staking/StakingVerifier.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,14 @@ interface IStaking {
/// @return Maximum number of services.
function maxNumServices() external view returns (uint256);

/// @dev Gets time for emissions.
/// @return Time for emissions.
function timeForEmissions() external view returns (uint256);

/// @dev Gets emissions amount.
/// @return Emissions amount.
function emissionsAmount() external view returns (uint256);

/// @dev Gets service staking token.
/// @return Service staking token address.
function stakingToken() external view returns (address);
Expand All @@ -22,6 +30,10 @@ interface IStaking {
/// @dev Gets service registry token utility address.
/// @return Service registry token utility address.
function serviceRegistryTokenUtility() external view returns(address);

/// @dev Minimum service staking deposit value required for staking.
/// @return Minimum service staking deposit.
function minStakingDeposit() external view returns(uint256);
}

/// @dev Provided zero address.
Expand Down Expand Up @@ -52,24 +64,26 @@ contract StakingVerifier {
event OwnerUpdated(address indexed owner);
event SetImplementationsCheck(bool setCheck);
event ImplementationsWhitelistUpdated(address[] implementations, bool[] statuses, bool setCheck);
event StakingLimitsUpdated(uint256 rewardsPerSecondLimit, uint256 timeForEmissionsLimit, uint256 _numServicesLimit,
uint256 emissionsLimit);
event StakingLimitsUpdated(uint256 minStakingDepositLimit, uint256 timeForEmissionsLimit, uint256 numServicesLimit,
uint256 apyLimit);

// One year constant
uint256 public constant ONE_YEAR = 1 days * 365;
// OLAS token address
address public immutable olas;
// Service registry address
address public immutable serviceRegistry;
// Service registry token utility
address public immutable serviceRegistryTokenUtility;

// Rewards per second limit
uint256 public rewardsPerSecondLimit;
// Minimum staking deposit limit
uint256 public minStakingDepositLimit;
// Time for emissions limit
uint256 public timeForEmissionsLimit;
// Limit for the number of services
uint256 public numServicesLimit;
// Emissions per service limit
uint256 public emissionsLimit;
// APY limit in 1e18 format
uint256 public apyLimit;
// Contract owner address
address public owner;
// Flag to check for the implementation address whitelisting status
Expand All @@ -82,35 +96,37 @@ contract StakingVerifier {
/// @param _olas OLAS token address.
/// @param _serviceRegistry Service registry address.
/// @param _serviceRegistryTokenUtility Service registry token utility address.
/// @param _rewardsPerSecondLimit Rewards per second limit.
/// @param _minStakingDepositLimit Minimum staking deposit limit.
/// @param _timeForEmissionsLimit Time for emissions limit.
/// @param _numServicesLimit Limit for the number of services.
/// @param _apyLimit APY limit in 1e18 format.
constructor(
address _olas,
address _serviceRegistry,
address _serviceRegistryTokenUtility,
uint256 _rewardsPerSecondLimit,
uint256 _minStakingDepositLimit,
uint256 _timeForEmissionsLimit,
uint256 _numServicesLimit
uint256 _numServicesLimit,
uint256 _apyLimit
) {
// Zero address check
if (_olas == address(0) || _serviceRegistry == address(0)) {
revert ZeroAddress();
}

// Zero values check
if (_rewardsPerSecondLimit == 0 || _timeForEmissionsLimit == 0 || _numServicesLimit == 0) {
if (_minStakingDepositLimit == 0 || _timeForEmissionsLimit == 0 || _numServicesLimit == 0 || _apyLimit == 0) {
revert ZeroValue();
}

owner = msg.sender;
olas = _olas;
serviceRegistry = _serviceRegistry;
serviceRegistryTokenUtility = _serviceRegistryTokenUtility;
rewardsPerSecondLimit = _rewardsPerSecondLimit;
minStakingDepositLimit = _minStakingDepositLimit;
timeForEmissionsLimit = _timeForEmissionsLimit;
numServicesLimit = _numServicesLimit;
emissionsLimit = _rewardsPerSecondLimit * _timeForEmissionsLimit * _numServicesLimit;
apyLimit = _apyLimit;
}

/// @dev Changes the owner address.
Expand Down Expand Up @@ -225,10 +241,29 @@ contract StakingVerifier {
return false;
}

// Check for the staking parameters
// This is a must have parameter for all staking contracts
uint256 rewardsPerSecond = IStaking(instance).rewardsPerSecond();
if (rewardsPerSecond > rewardsPerSecondLimit) {
// Check for minimum staking deposit
// This lets the verifier limit the max stake per slot for risk mitigation
uint256 minStakingDeposit = IStaking(instance).minStakingDeposit();
if (minStakingDeposit > minStakingDepositLimit) {
return false;
}

// Calculate rewards per year
uint256 rewardsPerYear = IStaking(instance).rewardsPerSecond() * ONE_YEAR;
// Calculate current APY in 1e18 format
uint256 apy = (rewardsPerYear * 1e18) / minStakingDeposit;

// Check for APY
// This lets the verifier limit the max APY a staking contract offers
// The DAO can this way express an upper bound on the APY staking contracts can offer
if (apy > apyLimit) {
return false;
}

// Check for time for emissions
// This lets the verifier enforce an upper bound on the emissions length for risk mitigation
uint256 timeForEmissions = IStaking(instance).timeForEmissions();
if (timeForEmissions > timeForEmissionsLimit) {
return false;
}

Expand Down Expand Up @@ -279,36 +314,39 @@ contract StakingVerifier {
}

/// @dev Changes staking parameter limits.
/// @param _rewardsPerSecondLimit Rewards per second limit.
/// @param _minStakingDepositLimit Minimum staking deposit limit.
/// @param _timeForEmissionsLimit Time for emissions limit.
/// @param _numServicesLimit Limit for the number of services.
/// @param _apyLimit APY limit in 1e18 format.
function changeStakingLimits(
uint256 _rewardsPerSecondLimit,
uint256 _minStakingDepositLimit,
uint256 _timeForEmissionsLimit,
uint256 _numServicesLimit
uint256 _numServicesLimit,
uint256 _apyLimit
) external {
// Check the contract ownership
if (owner != msg.sender) {
revert OwnerOnly(owner, msg.sender);
}

// Zero values check
if (_rewardsPerSecondLimit == 0 || _timeForEmissionsLimit == 0 || _numServicesLimit == 0) {
if (_minStakingDepositLimit == 0 || _timeForEmissionsLimit == 0 || _numServicesLimit == 0 || _apyLimit == 0) {
revert ZeroValue();
}

rewardsPerSecondLimit = _rewardsPerSecondLimit;
minStakingDepositLimit = _minStakingDepositLimit;
timeForEmissionsLimit = _timeForEmissionsLimit;
numServicesLimit = _numServicesLimit;
emissionsLimit = _rewardsPerSecondLimit * _timeForEmissionsLimit * _numServicesLimit;
apyLimit = _apyLimit;

emit StakingLimitsUpdated(_rewardsPerSecondLimit, _timeForEmissionsLimit, _numServicesLimit, emissionsLimit);
emit StakingLimitsUpdated(_minStakingDepositLimit, _timeForEmissionsLimit, _numServicesLimit, _apyLimit);
}

/// @dev Gets emissions amount limit for a specific staking proxy instance.
/// @notice The address field is reserved for the proxy instance, if needed in the next verifier version.
/// @return Emissions amount limit.
function getEmissionsAmountLimit(address) external view returns (uint256) {
return emissionsLimit;
/// @param instance Staking proxy instance.
/// @return amount Emissions amount limit.
function getEmissionsAmountLimit(address instance) external view returns (uint256 amount) {
// Get calculated emissions amount from the instance
amount = IStaking(instance).emissionsAmount();
}
}
Loading

0 comments on commit 3f80781

Please sign in to comment.