Skip to content

Commit

Permalink
Merge pull request #612 from ava-labs/L01-unused-parameters
Browse files Browse the repository at this point in the history 
L-01 Unused Parameters in calculateReward
  • Loading branch information
@geoff-vball
geoff-vball authored Oct 18, 2024
2 parents 86fe961 + b58287c commit bf4b9ef
Show file tree
Hide file tree
Showing 8 changed files with 29 additions and 57 deletions.
2 changes: 1 addition & 1 deletion abi-bindings/go/validator-manager/ERC20TokenStakingManager/ERC20TokenStakingManager.go
View file

Large diffs are not rendered by default.

28 changes: 14 additions & 14 deletions abi-bindings/go/validator-manager/ExampleRewardCalculator/ExampleRewardCalculator.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion abi-bindings/go/validator-manager/NativeTokenStakingManager/NativeTokenStakingManager.go
View file

Large diffs are not rendered by default.

4 changes: 1 addition & 3 deletions contracts/validator-manager/ExampleRewardCalculator.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ contract ExampleRewardCalculator is IRewardCalculator {
uint64 validatorStartTime,
uint64 stakingStartTime,
uint64 stakingEndTime,
uint64 uptimeSeconds,
uint256, // initialSupply
uint256 // endSupply
uint64 uptimeSeconds
) external view returns (uint256) {
// Equivalent to uptimeSeconds/(validator.endedAt - validator.startedAt) < UPTIME_REWARDS_THRESHOLD_PERCENTAGE/100
// Rearranged to prevent integer division truncation.
Expand Down
8 changes: 2 additions & 6 deletions contracts/validator-manager/PoSValidatorManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,9 +240,7 @@ abstract contract PoSValidatorManager is
validatorStartTime: validator.startedAt,
stakingStartTime: validator.startedAt,
stakingEndTime: validator.endedAt,
uptimeSeconds: uptimeSeconds,
initialSupply: 0,
endSupply: 0
uptimeSeconds: uptimeSeconds
});
$._redeemableValidatorRewards[validationID] += reward;
return (reward > 0);
Expand Down Expand Up @@ -572,9 +570,7 @@ abstract contract PoSValidatorManager is
validatorStartTime: validator.startedAt,
stakingStartTime: delegator.startedAt,
stakingEndTime: delegationEndTime,
uptimeSeconds: $._posValidatorInfo[delegator.validationID].uptimeSeconds,
initialSupply: 0,
endSupply: 0
uptimeSeconds: $._posValidatorInfo[delegator.validationID].uptimeSeconds
});
}

Check notice

Code scanning / Slither

Block timestamp Low

PoSValidatorManager._calculateDelegationReward(Delegator) uses timestamp for comparisons
Dangerous comparisons:
- delegationEndTime <= delegator.startedAt

Expand Down
6 changes: 1 addition & 5 deletions contracts/validator-manager/interfaces/IRewardCalculator.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,12 @@ interface IRewardCalculator {
* @param stakingStartTime The time the staker started staking
* @param stakingEndTime The time the staker stopped staking
* @param uptimeSeconds The total time the validator was validating
* @param initialSupply The total token supply at the start of the staking period
* @param endSupply The total token supply at the end of the staking period
*/
function calculateReward(
uint256 stakeAmount,
uint64 validatorStartTime,
uint64 stakingStartTime,
uint64 stakingEndTime,
uint64 uptimeSeconds,
uint256 initialSupply,
uint256 endSupply
uint64 uptimeSeconds
) external view returns (uint256);
}
8 changes: 2 additions & 6 deletions contracts/validator-manager/tests/ExamplesRewardCalculatorTests.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@ contract ExampleRewardCalculatorTest is Test {
validatorStartTime: DEFAULT_START_TIME,
stakingStartTime: DEFAULT_START_TIME,
stakingEndTime: DEFAULT_END_TIME,
uptimeSeconds: DEFAULT_UPTIME,
initialSupply: 0,
endSupply: 0
uptimeSeconds: DEFAULT_UPTIME
});
assertEq(output, 42e8);
}
Expand All @@ -41,9 +39,7 @@ contract ExampleRewardCalculatorTest is Test {
validatorStartTime: DEFAULT_START_TIME,
stakingStartTime: DEFAULT_START_TIME,
stakingEndTime: DEFAULT_END_TIME,
uptimeSeconds: DEFAULT_UPTIME - 1,
initialSupply: 0,
endSupply: 0
uptimeSeconds: DEFAULT_UPTIME - 1
});
assertEq(output, 0);
}
Expand Down
28 changes: 7 additions & 21 deletions contracts/validator-manager/tests/PoSValidatorManagerTests.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -546,9 +546,7 @@ abstract contract PoSValidatorManagerTest is ValidatorManagerTest {
validatorStartTime: DEFAULT_REGISTRATION_TIMESTAMP,
stakingStartTime: DEFAULT_DELEGATOR_COMPLETE_REGISTRATION_TIMESTAMP,
stakingEndTime: DEFAULT_DELEGATOR_END_DELEGATION_TIMESTAMP,
uptimeSeconds: DEFAULT_DELEGATOR_END_DELEGATION_TIMESTAMP - DEFAULT_REGISTRATION_TIMESTAMP,
initialSupply: 0,
endSupply: 0
uptimeSeconds: DEFAULT_DELEGATOR_END_DELEGATION_TIMESTAMP - DEFAULT_REGISTRATION_TIMESTAMP
});

_completeEndDelegationWithChecks({
Expand Down Expand Up @@ -678,9 +676,7 @@ abstract contract PoSValidatorManagerTest is ValidatorManagerTest {
validatorStartTime: DEFAULT_REGISTRATION_TIMESTAMP,
stakingStartTime: DEFAULT_DELEGATOR_COMPLETE_REGISTRATION_TIMESTAMP,
stakingEndTime: DEFAULT_DELEGATOR_END_DELEGATION_TIMESTAMP,
uptimeSeconds: validationEndTime - DEFAULT_REGISTRATION_TIMESTAMP,
initialSupply: 0,
endSupply: 0
uptimeSeconds: validationEndTime - DEFAULT_REGISTRATION_TIMESTAMP
});

_completeEndDelegationWithChecks({
Expand Down Expand Up @@ -750,9 +746,7 @@ abstract contract PoSValidatorManagerTest is ValidatorManagerTest {
validatorStartTime: DEFAULT_REGISTRATION_TIMESTAMP,
stakingStartTime: DEFAULT_DELEGATOR_COMPLETE_REGISTRATION_TIMESTAMP,
stakingEndTime: DEFAULT_COMPLETION_TIMESTAMP,
uptimeSeconds: DEFAULT_COMPLETION_TIMESTAMP - DEFAULT_REGISTRATION_TIMESTAMP,
initialSupply: 0,
endSupply: 0
uptimeSeconds: DEFAULT_COMPLETION_TIMESTAMP - DEFAULT_REGISTRATION_TIMESTAMP
});

uint256 expectedValidatorFees = expectedTotalReward * DEFAULT_DELEGATION_FEE_BIPS / 10000;
Expand Down Expand Up @@ -803,9 +797,7 @@ abstract contract PoSValidatorManagerTest is ValidatorManagerTest {
validatorStartTime: DEFAULT_REGISTRATION_TIMESTAMP,
stakingStartTime: DEFAULT_DELEGATOR_COMPLETE_REGISTRATION_TIMESTAMP,
stakingEndTime: DEFAULT_DELEGATOR_END_DELEGATION_TIMESTAMP,
uptimeSeconds: DEFAULT_DELEGATOR_END_DELEGATION_TIMESTAMP - DEFAULT_REGISTRATION_TIMESTAMP,
initialSupply: 0,
endSupply: 0
uptimeSeconds: DEFAULT_DELEGATOR_END_DELEGATION_TIMESTAMP - DEFAULT_REGISTRATION_TIMESTAMP
});

uint256 expectedValidatorFees = expectedTotalReward * DEFAULT_DELEGATION_FEE_BIPS / 10000;
Expand Down Expand Up @@ -944,9 +936,7 @@ abstract contract PoSValidatorManagerTest is ValidatorManagerTest {
validatorStartTime: DEFAULT_REGISTRATION_TIMESTAMP,
stakingStartTime: DEFAULT_DELEGATOR_COMPLETE_REGISTRATION_TIMESTAMP,
stakingEndTime: DEFAULT_DELEGATOR_END_DELEGATION_TIMESTAMP,
uptimeSeconds: DEFAULT_DELEGATOR_END_DELEGATION_TIMESTAMP - DEFAULT_REGISTRATION_TIMESTAMP,
initialSupply: 0,
endSupply: 0
uptimeSeconds: DEFAULT_DELEGATOR_END_DELEGATION_TIMESTAMP - DEFAULT_REGISTRATION_TIMESTAMP
});

// Complete delegation1 by delivering the weight update from nonce 4 (delegator2's nonce)
Expand Down Expand Up @@ -979,9 +969,7 @@ abstract contract PoSValidatorManagerTest is ValidatorManagerTest {
validatorStartTime: DEFAULT_REGISTRATION_TIMESTAMP,
stakingStartTime: DEFAULT_REGISTRATION_TIMESTAMP,
stakingEndTime: DEFAULT_COMPLETION_TIMESTAMP,
uptimeSeconds: DEFAULT_COMPLETION_TIMESTAMP - DEFAULT_REGISTRATION_TIMESTAMP,
initialSupply: 0,
endSupply: 0
uptimeSeconds: DEFAULT_COMPLETION_TIMESTAMP - DEFAULT_REGISTRATION_TIMESTAMP
});

_completeEndValidationWithChecks({
Expand Down Expand Up @@ -1422,9 +1410,7 @@ abstract contract PoSValidatorManagerTest is ValidatorManagerTest {
validatorStartTime: completeRegistrationTimestamp,
stakingStartTime: completeRegistrationTimestamp,
stakingEndTime: completionTimestamp,
uptimeSeconds: completionTimestamp - completeRegistrationTimestamp,
initialSupply: 0,
endSupply: 0
uptimeSeconds: completionTimestamp - completeRegistrationTimestamp
});

_completeEndValidationWithChecks({
Expand Down

0 comments on commit bf4b9ef

Please sign in to comment.