Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wcusdcv3 rewards fix #1016

Merged
merged 6 commits into from
Nov 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions contracts/plugins/assets/compoundv3/CusdcV3Wrapper.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ contract CusdcV3Wrapper is ICusdcV3Wrapper, WrappedERC20, CometHelpers {
/// From cUSDCv3, used in principal <> present calculations
uint256 public constant TRACKING_INDEX_SCALE = 1e15;
/// From cUSDCv3, scaling factor for USDC rewards
uint64 public constant RESCALE_FACTOR = 1e12;
uint256 public constant RESCALE_FACTOR = 1e12;

CometInterface public immutable underlyingComet;
ICometRewards public immutable rewardsAddr;
IERC20 public immutable rewardERC20;

mapping(address => uint64) public baseTrackingIndex;
mapping(address => uint64) public baseTrackingAccrued;
mapping(address => uint64) public baseTrackingIndex; // uint64 for consistency with CometHelpers
mapping(address => uint256) public baseTrackingAccrued; // uint256 to avoid overflow in L:199
mapping(address => uint256) public rewardsClaimed;

constructor(
Expand Down Expand Up @@ -257,7 +257,8 @@ contract CusdcV3Wrapper is ICusdcV3Wrapper, WrappedERC20, CometHelpers {

uint256 indexDelta = uint256(trackingSupplyIndex - baseTrackingIndex[account]);
uint256 newBaseTrackingAccrued = baseTrackingAccrued[account] +
safe64((safe104(balanceOf(account)) * indexDelta) / TRACKING_INDEX_SCALE);
(safe104(balanceOf(account)) * indexDelta) /
TRACKING_INDEX_SCALE;

uint256 claimed = rewardsClaimed[account];
uint256 accrued = newBaseTrackingAccrued * RESCALE_FACTOR;
Expand Down Expand Up @@ -286,9 +287,7 @@ contract CusdcV3Wrapper is ICusdcV3Wrapper, WrappedERC20, CometHelpers {
(, uint64 trackingSupplyIndex) = getSupplyIndices();
uint256 indexDelta = uint256(trackingSupplyIndex - baseTrackingIndex[account]);

baseTrackingAccrued[account] += safe64(
(safe104(accountBal) * indexDelta) / TRACKING_INDEX_SCALE
);
baseTrackingAccrued[account] += (safe104(accountBal) * indexDelta) / TRACKING_INDEX_SCALE;
tbrent marked this conversation as resolved.
Show resolved Hide resolved
akshatmittal marked this conversation as resolved.
Show resolved Hide resolved
baseTrackingIndex[account] = trackingSupplyIndex;
}

Expand Down
2 changes: 1 addition & 1 deletion contracts/plugins/assets/compoundv3/ICusdcV3Wrapper.sol
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ interface ICusdcV3Wrapper is IWrappedERC20, IRewardable {

function convertDynamicToStatic(uint256 amount) external view returns (uint104);

function baseTrackingAccrued(address account) external view returns (uint64);
function baseTrackingAccrued(address account) external view returns (uint256);

function baseTrackingIndex(address account) external view returns (uint64);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,15 @@ contract YearnV2CurveFiatCollateral is CurveStableCollateral {
return (low, high, 0);
}

// solhint-disable no-empty-blocks

/// DEPRECATED: claimRewards() will be removed from all assets and collateral plugins
function claimRewards() external virtual override {
// No rewards to claim, everything is part of the pricePerShare
}

// solhint-enable no-empty-blocks

// === Internal ===

/// @return {ref/tok} Actual quantity of whole reference units per whole collateral tokens
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,18 @@ describeFork('Wrapped CUSDCv3', () => {
)
})

it('regression test: able to claim rewards even when they are big without overflow', async () => {
// Nov 28 2023: uint64 math in CusdcV3Wrapper contract results in overflow when COMP rewards are even moderately large

const compToken = <ERC20Mock>await ethers.getContractAt('ERC20Mock', COMP)
expect(await compToken.balanceOf(wcusdcV3.address)).to.equal(0)
await advanceTime(1000)
await enableRewardsAccrual(cusdcV3, bn('2e18')) // enough to revert on uint64 implementation
pmckelvy1 marked this conversation as resolved.
Show resolved Hide resolved
tbrent marked this conversation as resolved.
Show resolved Hide resolved

await expect(wcusdcV3.connect(bob).claimRewards()).to.emit(wcusdcV3, 'RewardsClaimed')
expect(await compToken.balanceOf(bob.address)).to.be.greaterThan(0)
})

it('claims rewards and sends to claimer (claimTo)', async () => {
const compToken = <ERC20Mock>await ethers.getContractAt('ERC20Mock', COMP)
expect(await compToken.balanceOf(wcusdcV3.address)).to.equal(0)
Expand Down