V4 #2
V4 #2
Conversation
| @@ -185,15 +188,16 @@ contract SmartVaultYieldManager is ISmartVaultYieldManager, Ownable { | |||
| if (_hypervisorData.hypervisor != _hypervisor) revert InvalidRequest(); | |||
| IHypervisor(_hypervisor).withdraw(_thisBalanceOf(_hypervisor), address(this), address(this), [uint256(0),uint256(0),uint256(0),uint256(0)]); | |||
| _swapToSingleAsset(_hypervisor, _token, uniswapRouter, _hypervisorData.poolFee); | |||
| IERC20(_token).safeTransfer(msg.sender, _thisBalanceOf(_token)); | |||
| } | |||
|
|
|||
| function withdraw(address _hypervisor, address _token) external { | |||
| IERC20(_hypervisor).safeTransferFrom(msg.sender, address(this), IERC20(_hypervisor).balanceOf(msg.sender)); | |||
There was a problem hiding this comment.
super minor, but code might become much more readable with interface wrappings being class defined instead of the member always being wrapped multiple times in every function call.
| @@ -166,7 +169,7 @@ contract SmartVaultYieldManager is ISmartVaultYieldManager, Ownable { | |||
| IERC20(EURA).safeApprove(uniswapRouter, _balance); | |||
| ISwapRouter(uniswapRouter).exactInput(ISwapRouter.ExactInputParams({ | |||
| path: _pathFromEURA, | |||
| recipient: msg.sender, | |||
| recipient: address(this), | |||
| deadline: block.timestamp + 60, | |||
There was a problem hiding this comment.
Do we want to have that configurable possibly (with a require min value of 60 for example). This way we can increase the deadline enforced when unforeseen issues arise, the network being congested, etc. --> it at least allows you to react if something breaks at this point.
There was a problem hiding this comment.
probably a good idea!
contracts/SmartVaultYieldManager.sol
Outdated
| @@ -151,13 +153,14 @@ contract SmartVaultYieldManager is ISmartVaultYieldManager, Ownable { | |||
|
|
|||
| function deposit(address _collateralToken, uint256 _euroPercentage) external returns (address _hypervisor0, address _hypervisor1) { | |||
| // TODO min _euroPercentage of 10% | |||
There was a problem hiding this comment.
why don't we add the require statement as pointed out in the Todo?
There was a problem hiding this comment.
that's done now, was not yet finished developing that out
No description provided.