Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Incorrect Period Transition Logic in Reward Distribution

TxGuard

Incorrect Period Transition Logic in Reward Distribution

Summary

The updatePeriod function contains a critical flaw in the logic used to calculate the start of the next reward period. This error can lead to gaps in reward distribution, resulting in users missing out on potential rewards. The issue arises from the incorrect calculation of the next period's start time, which can significantly impact user experience and financial outcomes.

Code Issue

The problematic line of code is as follows:

uint256 nextPeriodStart = ((currentTime / periodDuration) + 2) * periodDuration;
​
​

Concrete Example

  • Period Duration: 7 days (604800 seconds)

  • Deployment Time: 1717100000 (Wed May 29 2024 12:13:20 UTC)

  • First Period Start Calculation:

    • Incorrect Calculation:

      • ((1717100000 / 604800) + 2) * 604800 = 1717718400 (Wed Jun 05 2024 00:00:00 UTC)

    • Correct Calculation:

      • ((1717100000 / 604800) + 1) * 604800 = 1717113600 (Wed May 29 2024 16:00:00 UTC)

Impact Visualization

[Period 1] [Gap] [Period 2]
|----------|------------------------------|----------|
Start End Start End
​
​

Impact

User Experience

  • Users may not receive rewards during the gap period, leading to dissatisfaction and loss of trust in the system. This can result in decreased user engagement and potential withdrawal from the platform.

Financial Impact

  • Users could lose out on significant rewards, especially in systems where timely reward claims are critical. The financial implications can be substantial, particularly in high-frequency trading or yield farming scenarios where every moment counts.

Exploitation Risk

  • If users are aware of the timing issues, they may exploit the system by strategically timing their actions to benefit from the gaps. This could lead to unfair advantages and further erode trust in the system.

PoC

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import "forge-std/Test.sol";
​
import "../contracts/core/governance/gauges/BaseGauge.sol";
import "../contracts/interfaces/core/governance/gauges/IGauge.sol";
import "./mocks/MockERC20.sol";
import "./mocks/MockVeToken.sol";
​
/**
* @title TestGauge
* @notice Mock implementation of BaseGauge for testing purposes
* @dev Implements required abstract functions with fixed values for testing
*/
contract TestGauge is BaseGauge {
constructor(
address _rewardToken,
address _stakingToken,
address _controller,
uint256 _maxEmission,
uint256 _periodDuration
)
BaseGauge(
_rewardToken,
_stakingToken,
_controller,
_maxEmission,
_periodDuration
)
{}
​
function _getBaseWeight(address) internal pure override returns (uint256) {
return 1000; // Fixed weight for testing
}
}
​
/**
* @title MockGaugeController
* @notice Mock controller implementation for testing gauge interactions
* @dev Simulates basic controller functionality required for tests
*/
contract MockGaugeController {
MockVeToken public veRAACToken;
​
constructor(address _veToken) {
veRAACToken = MockVeToken(_veToken);
}
​
function getGaugeWeight(address) external pure returns (uint256) {
return 1000;
}
​
function getVotingPower(address user) external view returns (uint256) {
return veRAACToken.getLockedAmount(user);
}
}
​
/**
* @title BaseGaugeTest
* @notice Comprehensive test suite for BaseGauge contract
* @dev Tests are organized into phases focusing on different aspects of functionality
*/
contract BaseGaugeTest is Test {
// Test contracts
TestGauge public gauge;
MockERC20 public rewardToken;
MockERC20 public stakingToken;
MockVeToken public veToken;
MockGaugeController public controller;
​
// Test addresses
address public admin = address(this);
address public user1 = address(0x1);
address public user2 = address(0x2);
​
// Constants
uint256 constant PERIOD_DURATION = 7 days;
uint256 constant MAX_EMISSION = 1000e18;
uint256 constant WEEK = 7 days;
uint256 constant BASIS_POINTS = 10000;
​
// Events
event DirectionVoted(
address indexed user,
uint256 direction,
uint256 votingPower
);
event EmissionUpdated(uint256 emission);
event PeriodUpdated(uint256 timestamp, uint256 avgWeight);
​
function setUp() public {
console.log("Setting up test environment...");
​
// Deploy mock tokens
rewardToken = new MockERC20();
stakingToken = new MockERC20();
veToken = new MockVeToken();
controller = new MockGaugeController(address(veToken));
​
console.log("Deployed mock tokens:");
console.log("- Reward Token:", address(rewardToken));
console.log("- Staking Token:", address(stakingToken));
console.log("- veToken:", address(veToken));
console.log("- Controller:", address(controller));
​
// Deploy gauge with initial configuration
gauge = new TestGauge(
address(rewardToken),
address(stakingToken),
address(controller),
MAX_EMISSION,
PERIOD_DURATION
);
console.log("Deployed TestGauge:", address(gauge));
​
// Setup initial token balances
rewardToken.mint(address(gauge), 10000e18);
stakingToken.mint(user1, 1000e18);
stakingToken.mint(user2, 1000e18);
veToken.mint(user1, 100e18);
veToken.mint(user2, 100e18);
​
console.log("Initial token balances set up");
console.log(
"- Gauge reward balance:",
rewardToken.balanceOf(address(gauge))
);
console.log("- User1 staking balance:", stakingToken.balanceOf(user1));
console.log("- User2 staking balance:", stakingToken.balanceOf(user2));
​
// Approve tokens for testing
vm.startPrank(user1);
stakingToken.approve(address(gauge), type(uint256).max);
vm.stopPrank();
​
vm.startPrank(user2);
stakingToken.approve(address(gauge), type(uint256).max);
vm.stopPrank();
​
// Grant roles
vm.startPrank(address(this));
bytes32 adminRole = gauge.DEFAULT_ADMIN_ROLE();
gauge.grantRole(adminRole, address(this));
gauge.grantRole(gauge.CONTROLLER_ROLE(), address(controller));
gauge.grantRole(gauge.CONTROLLER_ROLE(), address(this));
gauge.grantRole(gauge.FEE_ADMIN(), address(this));
vm.stopPrank();
​
console.log("Setup complete - roles granted and approvals set");
}
​
function testNextPeriodStartCalculation() public {
uint256 periodDuration = 7 days;
uint256 deploymentTime = 1717100000; // Example deployment time
vm.warp(deploymentTime);
​
// Set initial weight to trigger period creation
gauge.setInitialWeight(1000);
​
// Calculate expected next period start
uint256 expectedNextPeriodStart = ((deploymentTime / periodDuration) +
1) * periodDuration;
​
// Check the actual period start time
uint256 actualNextPeriodStart = gauge.getCurrentPeriodStart();
​
// Log the values for debugging
console.log("Expected Next Period Start:", expectedNextPeriodStart);
console.log("Actual Next Period Start:", actualNextPeriodStart);
​
assertEq(
actualNextPeriodStart,
expectedNextPeriodStart,
"Next period start time is incorrect"
);
}
}
​
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
​
contract MockERC20 is ERC20 {
uint8 private _decimals = 18;
​
constructor() ERC20("Mock Token", "MOCK") {}
​
function mint(address to, uint256 amount) external {
_mint(to, amount);
}
​
function setDecimals(uint8 decimals_) external {
_decimals = decimals_;
}
​
function decimals() public view override returns (uint8) {
return _decimals;
}
​
function approve(address spender, uint256 amount) public virtual override returns (bool) {
_approve(_msgSender(), spender, amount);
return true;
}
}
​
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import "../../contracts/interfaces/core/tokens/IveRAACToken.sol";
​
/**
* @notice A mock implementation of IveRAACToken for testing
*/
contract MockVeToken is IveRAACToken {
uint256 public totalSupplyVal = 10000e18; // Just an arbitrary total supply
mapping(address => uint256) public balances;
mapping(address => LockPosition) public lockPositions;
mapping(address => mapping(address => uint256)) public allowances;
mapping(address => uint256) private _votingPower;
address public minterAddress;
function setBalance(address user, uint256 amount) external {
balances[user] = amount;
lockPositions[user] = LockPosition({
amount: amount,
end: block.timestamp + 365 days,
power: amount
});
}
function balanceOf(address user) external view returns (uint256) {
return balances[user];
}
function totalSupply() external view returns (uint256) {
return totalSupplyVal;
}
function getVotingPower(address user) external view returns (uint256) {
return lockPositions[user].power;
}
​
function getVotingPower(address user, uint256) external view returns (uint256) {
return lockPositions[user].power;
}
function getTotalVotingPower() external view returns (uint256) {
return totalSupplyVal;
}
​
function getTotalVotingPower(uint256) external view returns (uint256) {
return totalSupplyVal;
}
function getLockPosition(address user) external view returns (LockPosition memory) {
return lockPositions[user];
}
​
function setTotalSupply(uint256 amount) external {
totalSupplyVal = amount;
}
​
// Required interface functions
function lock(uint256 amount, uint256 duration) external override {
balances[msg.sender] = amount;
lockPositions[msg.sender] = LockPosition({
amount: amount,
end: block.timestamp + duration,
power: amount
});
}
​
function extend(uint256 newDuration) external override {
LockPosition storage position = lockPositions[msg.sender];
position.end = block.timestamp + newDuration;
}
​
function increase(uint256 amount) external override {
LockPosition storage position = lockPositions[msg.sender];
position.amount += amount;
position.power += amount;
balances[msg.sender] += amount;
}
​
function withdraw() external override {
LockPosition storage position = lockPositions[msg.sender];
require(block.timestamp >= position.end, "Lock not expired");
uint256 amount = position.amount;
position.amount = 0;
position.power = 0;
balances[msg.sender] = 0;
// In a real implementation, we would transfer tokens here
}
​
function setMinter(address minter) external override {
minterAddress = minter;
}
​
function calculateVeAmount(uint256 amount, uint256) external pure returns (uint256) {
return amount; // Simplified calculation for testing
}
​
function transfer(address to, uint256 amount) external override returns (bool) {
require(balances[msg.sender] >= amount, "Insufficient balance");
balances[msg.sender] -= amount;
balances[to] += amount;
return true;
}
​
function transferFrom(address from, address to, uint256 amount) external override returns (bool) {
require(allowances[from][msg.sender] >= amount, "Insufficient allowance");
require(balances[from] >= amount, "Insufficient balance");
allowances[from][msg.sender] -= amount;
balances[from] -= amount;
balances[to] += amount;
return true;
}
​
function approve(address spender, uint256 amount) external returns (bool) {
allowances[msg.sender][spender] = amount;
return true;
}
​
// Additional helper functions for testing
function getLockedAmount(address user) external view returns (uint256) {
return lockPositions[user].amount;
}
​
// Helper function to mint tokens for testing
function mint(address to, uint256 amount) external {
require(msg.sender == minterAddress || minterAddress == address(0), "Not minter");
balances[to] += amount;
totalSupplyVal += amount;
// Also update lock position for voting power
LockPosition storage position = lockPositions[to];
position.amount += amount;
position.power += amount;
if (position.end == 0) {
position.end = block.timestamp + 365 days;
}
}
​
// Helper function to set locked amount for testing
function setLockedAmount(address user, uint256 amount) external {
LockPosition storage position = lockPositions[user];
position.amount = amount;
position.power = amount;
if (position.end == 0) {
position.end = block.timestamp + 365 days;
}
}
​
// Additional helper for tests
function setVotingPower(address account, uint256 amount) external {
_votingPower[account] = amount;
}
}
​

Recommendations

  1. Correct the Period Calculation Logic: Update the calculation of nextPeriodStart to ensure that it accurately reflects the end of the current period and the start of the next one. The corrected line should be:

uint256 nextPeriodStart = ((currentTime / periodDuration) + 1) * periodDuration;
​
​

  1. Thorough Testing: Implement comprehensive tests to cover various scenarios, including edge cases where the current time is close to the end of a period. This will help ensure that the period transition logic works as intended.

  2. User Communication: Inform users about the changes made to the period calculation logic and the importance of timely reward distribution. Transparency can help rebuild trust in the system.

  3. Monitoring and Alerts: Set up monitoring for reward distribution events to quickly identify and address any future discrepancies in reward calculations.

Updates

Lead Judging Commences

inallhonesty Lead Judge 3 months ago
Submission Judgement Published
Validated
Assigned finding tags:

BaseGauge::updatePeriod uses ((currentTime / periodDuration) + 2) calculation causing entire reward periods to be skipped, resulting in permanent loss of user rewards

inallhonesty Lead Judge 3 months ago
Submission Judgement Published
Validated
Assigned finding tags:

BaseGauge::updatePeriod uses ((currentTime / periodDuration) + 2) calculation causing entire reward periods to be skipped, resulting in permanent loss of user rewards

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.