-
Notifications
You must be signed in to change notification settings - Fork 74
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/templefrax #178
base: master
Are you sure you want to change the base?
Feat/templefrax #178
Conversation
src/backscratcher/StrategyProxy.sol
Outdated
uint256[] memory _balances = new uint256[](_tokens.length); | ||
|
||
for (uint256 i = 0; i < _tokens.length; i++) { | ||
_balances[i] = IERC20(_tokens[i]).balanceOf(address(proxy)); | ||
} | ||
|
||
proxy.safeExecute(_gauge, 0, abi.encodeWithSignature("getReward()")); | ||
proxy.safeExecute(_gauge, 0, claimRewardsString[_gauge]); |
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.
Would it be helpful to have a default/fallback like so?
proxy.safeExecute(_gauge, 0,
claimRewardsString[_gauge].length == 0 ? abi.encodeWithSignature("getReward()") : claimRewardsString[_gauge]);
Please ignore if you anticipate that claimRewardsString
can always be reliably defined when approveStrategy
is invoked.
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.
We cannot pass "" into the calldata parameter, we could try passing "0x0" but I am confident we can encode the ABI for the string we need.
src/backscratcher/StrategyProxy.sol
Outdated
@@ -194,6 +200,7 @@ contract StrategyProxy { | |||
require(thisStake.liquidity != 0, "kek_id not found"); | |||
|
|||
if (thisStake.liquidity > 0) { | |||
proxy.safeExecute(_gauge, 0, abi.encodeWithSignature("withdrawLocked(bytes32,address)", _kek_id, address(proxy))); |
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.
Doesn't this break in the case where the gauge's withdrawLocked
method only accepts a single kek_id
argument, like with FRAX/USDC?
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.
No, because this one is "withdrawV2", the FRAX/DAI and FRAX/USDC ones use "withdrawV3"
src/backscratcher/StrategyProxy.sol
Outdated
@@ -328,7 +337,7 @@ contract StrategyProxy { | |||
function claimRewards(address _gauge, address _token) external { | |||
require(strategies[_gauge] == msg.sender, "!strategy"); | |||
|
|||
proxy.safeExecute(_gauge, 0, abi.encodeWithSignature("getReward()")); | |||
proxy.safeExecute(_gauge, 0, claimRewardsString[_gauge]); |
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.
Ditto
|
||
uint256 _frax = IERC20(FRAX).balanceOf(address(this)); | ||
|
||
IERC20(FRAX).safeApprove(templeRouter, 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.
I lean towards giving infinite approvals in the constructor, but that's a matter of preference.
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.
Agreed, would save us on a bit of gas too.
No description provided.