-
Notifications
You must be signed in to change notification settings - Fork 1
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
Feat/cctp #13
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Readability and simplification
src/velo-fed/VeloFarmerV3.sol
Outdated
if(l2Token == address(DOLA) || l2Token == address(nUSDC)) revert RestrictedToken(); | ||
|
||
IERC20(l2Token).approve(address(bridge), amount); | ||
bridge.withdrawTo(address(l2Token), treasury, amount, 0, ""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bridge.withdrawTo(address(l2Token), treasury, amount, 0, ""); | |
bridge.withdrawTo(l2Token, treasury, amount, 0, ""); |
Unnecessary parse
src/velo-fed/VeloFarmerV3.sol
Outdated
function withdrawLiquidityAndSwapToDOLA(uint dolaAmount) external { | ||
uint usdcAmount = withdrawLiquidity(dolaAmount); | ||
|
||
swapUSDCNativetoDOLA(usdcAmount); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make access control explicit
src/velo-fed/VeloFarmerV3.sol
Outdated
function withdrawLiquidity(uint dolaAmount) public onlyChair returns (uint) { | ||
uint lpTokenPrice = getLpTokenPrice(); | ||
uint liquidityToWithdraw = dolaAmount *1e18 / lpTokenPrice; | ||
uint ownedLiquidity = dolaGauge.balanceOf(address(this)); | ||
|
||
if (liquidityToWithdraw > ownedLiquidity) liquidityToWithdraw = ownedLiquidity; | ||
dolaGauge.withdraw(liquidityToWithdraw); | ||
|
||
LP_TOKEN_NATIVE.approve(address(router), liquidityToWithdraw); | ||
(uint amountUSDC, uint amountDola) = router.removeLiquidity(address(nUSDC), address(DOLA), true, liquidityToWithdraw, 0, 0, address(this), block.timestamp); | ||
|
||
uint totalDolaReceived = amountDola + (amountUSDC *DOLA_USDC_CONVERSION_MULTI); | ||
|
||
if ((dolaAmount *(PRECISION - maxSlippageBpsLiquidity) / PRECISION) > totalDolaReceived) { | ||
revert LiquiditySlippageTooHigh(); | ||
} | ||
|
||
return amountUSDC; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider making internal and adding withdrawLiquidity(uint dolaAmount) external onlyChair returns (uint);
function.
src/velo-fed/VeloFarmerV3.sol
Outdated
function depositAll() external { | ||
deposit(DOLA.balanceOf(address(this)), nUSDC.balanceOf(address(this))); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make access control explicit
src/velo-fed/VeloFarmerV3.sol
Outdated
function deposit(uint dolaAmount, uint usdcAmount) public onlyChair { | ||
uint lpTokenPrice = getLpTokenPrice(); | ||
|
||
DOLA.approve(address(router), dolaAmount); | ||
nUSDC.approve(address(router), usdcAmount); | ||
(uint dolaSpent, uint usdcSpent, uint lpTokensReceived) = router.addLiquidity(address(DOLA), address(nUSDC), true, dolaAmount, usdcAmount, 0, 0, address(this), block.timestamp); | ||
require(lpTokensReceived > 0, "No LP tokens received"); | ||
|
||
uint totalDolaValue = dolaSpent + (usdcSpent *DOLA_USDC_CONVERSION_MULTI); | ||
|
||
uint expectedLpTokens = totalDolaValue *1e18 / lpTokenPrice *(PRECISION - maxSlippageBpsLiquidity) / PRECISION; | ||
if (lpTokensReceived < expectedLpTokens) revert LiquiditySlippageTooHigh(); | ||
|
||
uint lpBalance = LP_TOKEN_NATIVE.balanceOf(address(this)); | ||
LP_TOKEN_NATIVE.approve(address(dolaGauge), lpBalance); | ||
dolaGauge.deposit(lpBalance); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider making internal and adding deposit(uint dolaAmount, uint usdcAmount) external onlyChair returns (uint);
function.
uint public maxSlippageBpsDolaToUsdc; | ||
uint public maxSlippageBpsUsdcToDola; | ||
uint public maxSlippageBpsUsdcNativeToDola; | ||
uint public maxSlippageBpsDolaToUsdcNative; | ||
uint public maxSlippageBpsUsdcToUsdcNative; | ||
uint public maxSlippageBpsUsdcNativeToUsdc; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove half of these?
src/velo-fed/VeloFarmerV3.sol
Outdated
uint public constant PRECISION = 10_000; | ||
|
||
IGauge public constant dolaGauge = IGauge(0x853CAcEc83e4183eF78d6b64ccca3de365861CaF); | ||
IERC20 public constant LP_TOKEN_NATIVE = IERC20(0xA56a25Dee5B3199A9198Bbd48715EE3D0ed98378); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can rename this to just LP_TOKEN
src/velo-fed/VeloFarmerV3.sol
Outdated
error OnlyChair(); | ||
error OnlyGov(); | ||
error OnlyPendingGov(); | ||
error OnlyGovOrGuardian(); | ||
error MaxSlippageTooHigh(); | ||
error NotEnoughTokens(); | ||
error LiquiditySlippageTooHigh(); | ||
error RestrictedToken(); | ||
|
||
constructor( | ||
address gov_, | ||
address chair_, | ||
address l2chair_, | ||
address treasury_, | ||
address guardian_, | ||
address bridge_, | ||
address optiFed_, | ||
address cctp_, | ||
uint[] memory maxSlippageBps, | ||
uint maxSlippageBpsLiquidity_ | ||
) | ||
{ | ||
gov = gov_; | ||
chair = chair_; | ||
l2chair = l2chair_; | ||
treasury = treasury_; | ||
guardian = guardian_; | ||
bridge = IL2ERC20Bridge(bridge_); | ||
optiFed = optiFed_; | ||
cctp = ICCTP(cctp_); | ||
maxSlippageBpsDolaToUsdc = maxSlippageBps[0]; | ||
maxSlippageBpsUsdcToDola = maxSlippageBps[1]; | ||
maxSlippageBpsUsdcNativeToDola = maxSlippageBps[2]; | ||
maxSlippageBpsDolaToUsdcNative = maxSlippageBps[3]; | ||
maxSlippageBpsUsdcToUsdcNative = maxSlippageBps[4]; | ||
maxSlippageBpsUsdcNativeToUsdc = maxSlippageBps[5]; | ||
maxSlippageBpsLiquidity = maxSlippageBpsLiquidity_; | ||
} | ||
|
||
modifier onlyGov() { | ||
if (msg.sender != address(ovmL2CrossDomainMessenger) || | ||
ovmL2CrossDomainMessenger.xDomainMessageSender() != gov | ||
) revert OnlyGov(); | ||
_; | ||
} | ||
|
||
modifier onlyPendingGov() { | ||
if (msg.sender != address(ovmL2CrossDomainMessenger) || | ||
ovmL2CrossDomainMessenger.xDomainMessageSender() != pendingGov | ||
) revert OnlyPendingGov(); | ||
_; | ||
} | ||
|
||
modifier onlyChair() { | ||
if ((msg.sender != address(ovmL2CrossDomainMessenger) || | ||
ovmL2CrossDomainMessenger.xDomainMessageSender() != chair) && | ||
msg.sender != l2chair | ||
) revert OnlyChair(); | ||
_; | ||
} | ||
|
||
modifier onlyGovOrGuardian() { | ||
if ((msg.sender != address(ovmL2CrossDomainMessenger) || | ||
(ovmL2CrossDomainMessenger.xDomainMessageSender() != gov) && | ||
ovmL2CrossDomainMessenger.xDomainMessageSender() != guardian) | ||
) revert OnlyGovOrGuardian(); | ||
_; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can probably simplify these to a onlyRole(address)
modifier, onlyGovOrRole(address)
modifier and OnlyRole(address)
error message.
@notice Burns `dolaAmount` of DOLA held in this contract | ||
@param dolaAmount Amount of DOLA to burn | ||
*/ | ||
function contraction(uint dolaAmount) public { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can make this external
for small gas savings.
Update VeloFarmer to use CCTP USDC bridge for transferring USDC to Optimism