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.

Sign up for GitHub

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

Wcusdcv3 rewards fix #1016

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
53e5bc0
add regression test, confirmed fails on previous code
tbrent Nov 28, 2023
5513e65
implement fix
tbrent Nov 28, 2023
1d627c7
slightly improve comments
tbrent Nov 28, 2023
5ddfd9e
integrate julian feedback
tbrent Nov 29, 2023
289768d
lint clean
tbrent Nov 29, 2023
7e21b3f
RESCALE_FACTOR uint256
tbrent Nov 29, 2023
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
8 changes: 3 additions & 5 deletions contracts/plugins/assets/compoundv3/CusdcV3Wrapper.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
ICometRewards public immutable rewardsAddr;
IERC20 public immutable rewardERC20;

mapping(address => uint64) public baseTrackingIndex;
mapping(address => uint64) public baseTrackingAccrued;
mapping(address => uint64) public baseTrackingIndex; // uint64 to stay consistent with CometHelpers

Check failure on line 29 in contracts/plugins/assets/compoundv3/CusdcV3Wrapper.sol

View workflow job for this annotation

GitHub Actions / Lint Checks 

Line length must be no more than 100 but current length is 103
tbrent marked this conversation as resolved.
Show resolved Hide resolved
mapping(address => uint256) public baseTrackingAccrued; // uint256 to avoid overflow in L:199
mapping(address => uint256) public rewardsClaimed;

constructor(
Expand Down Expand Up @@ -286,9 +286,7 @@
(, 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
View file
Open in desktop
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
12 changes: 12 additions & 0 deletions test/plugins/individual-collateral/compoundv3/CusdcV3Wrapper.test.ts
View file
Open in desktop
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
Loading