Feature: Liquidity Pool #19
Conversation
…idity-contracts into feat-liq-pool
| $.mpcAddress = mpcAddress_; | ||
| } | ||
|
|
||
| function deposit() external override { |
There was a problem hiding this comment.
Shouldn't we also have a deposit method with pull USDC mechanic? For bridging?
There was a problem hiding this comment.
@mpetrunic could you add more details? Do you mean deposit with transferFrom? Currently bridging mechanic is already in place. Rebalancer transfers funds to the liquidity pool then calls deposit().
There was a problem hiding this comment.
Just create another method that receives argument amount and pulls funds from caller
There was a problem hiding this comment.
All deposits should always take amount input so to prevent depositing profit as collateral
| uint256 amount = COLLATERAL.balanceOf(address(this)); | ||
| require(amount > 0, NoCollateral()); | ||
| IAavePool pool = IAavePool(AAVE_POOL_PROVIDER.getPool()); | ||
| (, uint256 repaidAmount) = _repay(address(COLLATERAL), pool, true); |
There was a problem hiding this comment.
whats the reasoning behind this?
Like, I think it will cause bugs. We have a cron job that calls repayment. If you repay with a deposit, later on when we get repayment, we will consider it profit and not collateral
There was a problem hiding this comment.
Currently the collateral (usdc) cannot be withdrawn as profit. It can be supplied to aave, used for repayments and withdrawn from aave. Please correct us if the flow should be different
There was a problem hiding this comment.
@mpetrunic this really depends on how you are planning to do the offchain accounting. Lets discuss what needs to be accounted, then come up with a robust solution. We can do the accounting inside the contract ofcourse, but that will use more gas.
There was a problem hiding this comment.
conclusion: remove repay with collateral
| // the operation securely. | ||
| (bool success,) = target.call(targetCallData); | ||
| require(success, TargetCallFailed()); | ||
| emit Borrowed(borrowToken, amount, msg.sender, target, targetCallData); |
There was a problem hiding this comment.
this method needs to be really gas optimized, so I would avoid emitting event
|
|
||
| // Admin functions | ||
|
|
||
| function withdraw(address to, uint256 amount) external override onlyRole(LIQUIDITY_ADMIN_ROLE) returns (uint256) { |
There was a problem hiding this comment.
I don't think admin should be allowed to withdraw collateral. Only rebalancer should be able to withdraw to transfer to different chain.
There was a problem hiding this comment.
That role is just called liquidity admin, which is given to rebalancer and hub.
| return withdrawn; | ||
| } | ||
|
|
||
| function withdrawProfit( |
There was a problem hiding this comment.
hmm I don't think this is gonna work. We will most likely always have debt > profit as we are always solving something.
Which means we will never be able to withdraw profit.
From my pov we have two options. We can pause contract and only allow withdrawal once all debt has been repaid.
When borrowing, we submit signed input amount and track profit on the contract.
I'm more in favour of option #1 because of gas and ease of implementation
There was a problem hiding this comment.
If we have debt > profit, then maybe we can assume that there is not enough profit worth withdrawing?
There was a problem hiding this comment.
Conclusion:
- add pause functionality
- prevent withdraw if there is a debt for token we are trying to withdraw
- prevent withdrawing aUSDC token (collateral ownership)
- change interface to just accept list of token addresses (transfer full amount)
- allows usdc withdrawal as profit
Q: can we make LiquidityPool not upgradable
Closes: #4