Summary
An error in the formula tracking gauge weights will surface when someone calls GaugeController.vote(...) a second time after acquiring more veRAAC. As a result, users will receive less rewards than they should have earned.
Finding Description
GaugeController.sol's vote function will update weights after each vote via the _updateGaugeWeight helper, where the "votingPower" is the user's veRAACToken balance:
function vote(address gauge, uint256 weight) external override whenNotPaused {
...
uint256 votingPower = veRAACToken.balanceOf(msg.sender);
if (votingPower == 0) revert NoVotingPower();
uint256 oldWeight = userGaugeVotes[msg.sender][gauge];
userGaugeVotes[msg.sender][gauge] = weight;
_updateGaugeWeight(gauge, oldWeight, weight, votingPower);
emit WeightUpdated(gauge, oldWeight, weight);
}
Source: GaugeController.sol#L190-L203
_updateGaugeWeight tracks the total weight * votingPower for a gauge across all users. To do this, it first attempts to subtract the current user's previous vote before adding in the latest:
function _updateGaugeWeight(
address gauge,
uint256 oldWeight,
uint256 newWeight,
uint256 votingPower
) internal {
Gauge storage g = gauges[gauge];
uint256 oldGaugeWeight = g.weight;
uint256 newGaugeWeight = oldGaugeWeight - (oldWeight * votingPower / WEIGHT_PRECISION)
+ (newWeight * votingPower / WEIGHT_PRECISION);
g.weight = newGaugeWeight;
g.lastUpdateTime = block.timestamp;
}
Specifically the issue is oldWeight * votingPower. Since "votingPower" is just the user's veRAACToken balance, it may have increased since their last vote. When their balance has grown, oldWeight * votingPower will result in subtracting MORE than the gauge weight that their vote had originally added.
As a result, the transaction will revert due to underflow or the gauge weight will end up lower than it should be.
Gauge weights impact token rewards. This flow begins with GaugeController.distributeRewards(..):
function distributeRewards(
address gauge
) external override nonReentrant whenNotPaused {
...
uint256 reward = _calculateReward(gauge);
if (reward == 0) return;
IGauge(gauge).notifyRewardAmount(reward);
emit RewardDistributed(gauge, msg.sender, reward);
}
Source: GaugeController.sol#L323-L334
In the _calculateReward helper, we see how the gauge weight calculated above will impact rewards. Note how a smaller g.weight will cause the return value to decrease:
function _calculateReward(address gauge) internal view returns (uint256) {
Gauge storage g = gauges[gauge];
uint256 totalWeight = getTotalWeight();
if (totalWeight == 0) return 0;
uint256 gaugeShare = (g.weight * WEIGHT_PRECISION) / totalWeight;
uint256 typeShare = (typeWeights[g.gaugeType] * WEIGHT_PRECISION) / MAX_TYPE_WEIGHT;
uint256 periodEmission = g.gaugeType == GaugeType.RWA ? _calculateRWAEmission() : _calculateRAACEmission();
return (periodEmission * gaugeShare * typeShare) / (WEIGHT_PRECISION * WEIGHT_PRECISION);
}
Source: GaugeController.sol#L360-L372
That value (rewards for the gauge) is then shared with the gauge itself via the gauge.notifyRewardAmount(reward) in distributeRewards.
Inside the BaseGauge we see that notifyRewardAmount will update the gauge's rewardRate storage variable:
function notifyRewardAmount(uint256 amount) external override onlyController updateReward(address(0)) {
if (amount > periodState.emission) revert RewardCapExceeded();
rewardRate = notifyReward(periodState, amount, periodState.emission, getPeriodDuration());
...
}
Source: BaseGauge.sol#L354-L357
That in turn will result in a smaller value returned by getRewardPerToken:
function getRewardPerToken() public view returns (uint256) {
if (totalSupply() == 0) {
return rewardPerTokenStored;
}
return rewardPerTokenStored + (
(lastTimeRewardApplicable() - lastUpdateTime) * rewardRate * 1e18 / totalSupply()
);
}
Source: BaseGauge.sol#L568-L576
getRewardPerToken() is used by another view function earned(account) which is called after every transaction via the updateReward modifier. This saves the calculated user rewards to userStates[account].rewards.
When getReward() is called by a user, the amount stored in userStates[account].rewards is sent to them:
function getReward() external virtual nonReentrant whenNotPaused updateReward(msg.sender) {
...
UserState storage state = userStates[msg.sender];
uint256 reward = state.rewards;
if (reward > 0) {
state.rewards = 0;
...
rewardToken.safeTransfer(msg.sender, reward);
emit RewardPaid(msg.sender, reward);
}
}
Source: BaseGauge.sol#L327-L347
Impact Explanation
vote transactions revert.
Voting will not influence rewards correctly, resulting in all staking users being paid less than they should be.
Impact 1) is the lesser concern but manifests first in testing. Since the formula is attempting to subtract more than was originally added, the transaction will revert with an underflow for whales (relative to the balance of other voters) or when only a few users have voted. The POC below shows how the transaction no longer reverts after some other users have submitted their votes.
Impact 2) The POC below shows a loss of 45.5% rewards for all users staking with the impacted gauge. The amount of loss will be higher or lower depending on how much the user's balance has increased between vote calls. Doubling their balance resulted in ~25% loss of rewards, and the POC is using 6x the original balance which results in the 45.5% loss.
Likelihood Explanation
This seems very likely to occur. Since veRAACToken requires a lock up of at least 1 year, some users will enter the system with a conservative investment and then later increase their exposure. Someone may even start by using very small values ("dust") in order to first experience the application end to end - then once they deposit real money, rewards will be significantly off from the expected value.
Proof of Concept
The following git patch adds two "it" test blocks. The first "it" block demonstrates the issue, then impact 1) where transactions revert, followed by impact 2) where rewards are lower than they should be. The second "it" block is used to calculate the expected reward rate for comparison.
The values in this POC should be reasonable & realistic amounts (or maybe even too small) so not to exaggerate the issue.
Run with npm run test:unit:governance.
diff --git a/test/unit/core/governance/gauges/GaugeController.test.js b/test/unit/core/governance/gauges/GaugeController.test.js
index 17bac09..89a9b78 100644
--- a/test/unit/core/governance/gauges/GaugeController.test.js
+++ b/test/unit/core/governance/gauges/GaugeController.test.js
@@ -2,6 +2,7 @@ import { time } from "@nomicfoundation/hardhat-network-helpers";
import { expect } from "chai";
import hre from "hardhat";
const { ethers } = hre;
+import { PANIC_CODES } from "@nomicfoundation/hardhat-chai-matchers/panic.js";
describe("GaugeController", () => {
let gaugeController;
@@ -87,6 +88,90 @@ describe("GaugeController", () => {
beforeEach(async () => {
await veRAACToken.mint(user1.address, ethers.parseEther("1000"));
await veRAACToken.mint(user2.address, ethers.parseEther("500"));
+
+
+
+ await rewardToken.mint(rwaGauge.getAddress(), ethers.parseEther("1000000"));
+
+
+ await rwaGauge.setBoostParameters(25000, 10000, 7 * 24 * 3600);
+
+
+ await veRAACToken.connect(user1).approve(rwaGauge.getAddress(), ethers.MaxUint256);
+ await rwaGauge.connect(user1).stake(ethers.parseEther("20"));
+ });
+
+ it.only("Bug and impact", async () => {
+
+ await gaugeController.connect(user1).vote(rwaGauge.getAddress(), 5000);
+
+
+ await veRAACToken.mint(user1.address, ethers.parseEther("5000"));
+
+
+
+
+ await expect(
+ gaugeController.connect(user1).vote(rwaGauge.getAddress(), 5000)
+ ).to.be.revertedWithPanic(PANIC_CODES.ARITHMETIC_OVERFLOW);
+
+
+ for(let i = 0; i < 5; i++) {
+ const newUser = (await ethers.getSigners())[i + 10];
+ await veRAACToken.mint(newUser.address, ethers.parseEther("1000"));
+ await gaugeController.connect(newUser).vote(rwaGauge.getAddress(), 5000);
+ }
+
+
+ {
+ await gaugeController.connect(user1).vote(rwaGauge.getAddress(), 5000);
+ const weight = await gaugeController.getGaugeWeight(rwaGauge.getAddress());
+
+ expect(weight).to.be.eq(2990n * 10n ** 18n);
+
+
+ }
+
+
+ {
+
+ await gaugeController.distributeRewards(rwaGauge.getAddress());
+
+ await time.increase(60 * 60 * 24 * 7);
+ }
+
+
+ {
+ const tx = await rwaGauge.connect(user1).getReward();
+
+ await expect(tx).to.emit(rewardToken, "Transfer").withArgs(rwaGauge.getAddress(), user1.address, 310461666666);
+ }
+ });
+
+ it.only("FYI: versus the expected state", async () => {
+
+
+
+ await veRAACToken.mint(user1.address, ethers.parseEther("5000"));
+
+
+ await gaugeController.connect(user1).vote(rwaGauge.getAddress(), 5000);
+ for(let i = 0; i < 5; i++) {
+ const newUser = (await ethers.getSigners())[i + 10];
+ await veRAACToken.mint(newUser.address, ethers.parseEther("1000"));
+ await gaugeController.connect(newUser).vote(rwaGauge.getAddress(), 5000);
+ }
+
+
+ const weight = await gaugeController.getGaugeWeight(rwaGauge.getAddress());
+ expect(weight).to.be.eq(5490n * 10n ** 18n);
+
+ await gaugeController.distributeRewards(rwaGauge.getAddress());
+ await time.increase(60 * 60 * 24 * 7);
+
+ const tx = await rwaGauge.connect(user1).getReward();
+
+ await expect(tx).to.emit(rewardToken, "Transfer").withArgs(rwaGauge.getAddress(), user1.address, 570044999999);
});
it("should calculate correct initial weights", async () => {
Recommendation
This git patch adds state (userGaugePower) tracking the user's power at the time of their last vote. This allows us to update the formula in _updateGaugeWeight to correctly subtract the user's old information before adding their current vote.
When applied, the POC test above fails (since it no longer reverts).
@@ -78,6 +78,7 @@ contract GaugeController is IGaugeController, AccessControl, ReentrancyGuard, Pa
*/
/// @notice Maps user addresses to their gauge votes
mapping(address => mapping(address => uint256)) public userGaugeVotes;
+ mapping(address => mapping(address => uint256)) public userGaugePower;
/// @notice Last vote timestamp for each user
mapping(address => uint256) public lastVoteTime;
/// @notice Required delay between votes
@@ -196,8 +197,10 @@ contract GaugeController is IGaugeController, AccessControl, ReentrancyGuard, Pa
uint256 oldWeight = userGaugeVotes[msg.sender][gauge];
userGaugeVotes[msg.sender][gauge] = weight;
+ uint256 oldVotingPower = userGaugePower[msg.sender][gauge];
+ userGaugePower[msg.sender][gauge] = votingPower;
- _updateGaugeWeight(gauge, oldWeight, weight, votingPower);
+ _updateGaugeWeight(gauge, oldWeight, weight, oldVotingPower, votingPower);
emit WeightUpdated(gauge, oldWeight, weight);
}
@@ -214,12 +217,13 @@ contract GaugeController is IGaugeController, AccessControl, ReentrancyGuard, Pa
address gauge,
uint256 oldWeight,
uint256 newWeight,
+ uint256 oldVotingPower,
uint256 votingPower
) internal {
Gauge storage g = gauges[gauge];
uint256 oldGaugeWeight = g.weight;
- uint256 newGaugeWeight = oldGaugeWeight - (oldWeight * votingPower / WEIGHT_PRECISION)
+ uint256 newGaugeWeight = oldGaugeWeight - (oldWeight * oldVotingPower / WEIGHT_PRECISION)
+ (newWeight * votingPower / WEIGHT_PRECISION);
g.weight = newGaugeWeight;
