Compoundv2 #3
Compoundv2 #3
Conversation
…Still playing around with getPricePerShare logic to show USDC per GRO
src/RedemptionPool.sol
Outdated
| emit Claim(msg.sender, userClaim); | ||
|
|
||
| // Cap the user's claim to the available balance of cUSDC tokens | ||
| if (_amount > userClaim) _amount = userClaim; |
There was a problem hiding this comment.
I think this will create confusing UX. Just revert in case amount provided is > than balance
test/RedemptionPool.t.sol
Outdated
| "USDC balance of CUSDC: %s", | ||
| USDC.balanceOf(address(CUSDC)) | ||
| ); | ||
| console2.log( |
There was a problem hiding this comment.
Please remove console logs from tests :)
src/RedemptionPool.sol
Outdated
| // @param to The address to which the GRO position will be transferred. | ||
| // @param amount The amount of GRO to transfer. | ||
| function transferPosition(address _to, uint256 _amount) public { | ||
| if (_amount > (_userGROBalance[msg.sender] - _userClaims[msg.sender])) |
There was a problem hiding this comment.
I think this one is mixing apples with oranges? _userGROBalance is denominated in GRO tokens, _userClaims is denominated in USDC?
src/RedemptionPool.sol
Outdated
| if (_amount > userClaim) _amount = userClaim; | ||
|
|
||
| // Revert if nothing to claim | ||
| if (_amount == 0) revert RedemptionErrors.InsufficientBalance(); |
There was a problem hiding this comment.
Can be checked at the very top to avoid spending gas if _amount provided is 0
There was a problem hiding this comment.
At the current location it also reverts if userClaim is 0 - if you move it to top then it won't revert when userClaim is 0
There was a problem hiding this comment.
It does, but that requires additional SLOAD. Func parameters should be checked at the very top because they are very cheap to check if you need to revert
src/RedemptionPool.sol
Outdated
| revert RedemptionErrors.USDCRedeemFailed(redeemResult); | ||
|
|
||
| uint256 usdcAmount = (ICERC20(CUSDC).exchangeRateStored() * _amount) / | ||
| 1e20; |
There was a problem hiding this comment.
Why 1e20 ? Can it be moved to constants?
There was a problem hiding this comment.
Because:
USDC 1e6 = (CUSDC 1e8 * exchangeRate 1e18) / 1ePRECISION
PRECISION = 20
yeah - should be in constants
There was a problem hiding this comment.
Again, why don't we transfer all USDC redeemed but trying to calculate it after we redeem?
src/RedemptionPool.sol
Outdated
| if (redeemResult != 0) | ||
| revert RedemptionErrors.USDCRedeemFailed(redeemResult); | ||
|
|
||
| uint256 usdcAmount = (ICERC20(CUSDC).exchangeRateStored() * _amount) / |
There was a problem hiding this comment.
Don't you think we should transfer 100% of USDC balance redeemed?
There was a problem hiding this comment.
Means an additional sload for balanceOf, but probably safer to avoid someone not getting USDC after a rounding error
Now with working tests