Pm/zap token to pt #230
Pm/zap token to pt #230
Conversation
|
|
||
| // Due to rounding issues of some tokens, we find the | ||
| // relevant token balance of this contract | ||
| _ctx[i] = IERC20(_zap.roots[i]).balanceOf(address(this)); |
There was a problem hiding this comment.
why don't we use the _zap.amounts[i] here? It will save one external call and avoid using the unaccounted token balance that may present in the contract.
There was a problem hiding this comment.
So the _ctx array is necessary as it is meant to be used to keep a context of what occurred in the childZap from the zapIn() function which called this.
As you can see in the excerpt below, for the _childZap it's irrelevant and we just pass an undeclared uint256[3], however the result of the _childZap is inserted at the index relevant to the _zap amount.
As an example, we have a principal token based on the curve pool token LUSD3CRV_f which is a metapool(?) that combines LUSD and the 3CRV token. The 3CRV token is based on DAI, USDT and USDC. Contextualising this, the user might only have DAI to purchase the PT with and so they would initially _zapCurveLpIn on the _childZap which contains the amount of DAI they have plus other info. As a result there would be an amount of 3CRV tokens that would be recorded as a result in the ctx array, which is then passed to the _zapCurveLpIn on the _zap which would result in receiving LUSD3CRV_f tokens which are subsequently used to purchase the PT's.
| address(_zap.curvePool).functionCallWithValue( | ||
| abi.encodeWithSelector(_zap.funcSig, _ctx, 0), | ||
| msg.value | ||
| ); |
There was a problem hiding this comment.
I think there will be a big mistake of passing 0 as the min_mint_amount, It creates the arbitrage opportunity and it will screw the trade which will give users a very less amount than expected. Although below call where we are purchasing the PT amt we are passing the minimum PT amount that can counter the arbitrage swap although I still recommend having a minimum amount avoid any opening in the contract.
There was a problem hiding this comment.
Yes that can be done. Might however introduce some UX issues as there is not an absolute deterministic solution for calculating the curve swap correctly (calc_token_amount is intended as loosely accurate -> https://github.com/curvefi/curve-contract/blob/master/contracts/pool-templates/a/SwapTemplateA.vy#L347-L355) but can leave it up to the frontend to determine that issue
| ); | ||
| // When a childZap happens, we add the amount of lpTokens gathered | ||
| // from it to the relevant root index of the "main" zap | ||
| ctx[_childZap.parentIdx] += _amount; |
There was a problem hiding this comment.
Is there any way to validate this input? Or we are completely relying on the user input. The only worrisome I have is that I don't want to give an opportunity to the external users to get unwanted gains by just changing some parameters.
There was a problem hiding this comment.
We could use the coins method on the curve contract maybe? It takes an integer value returning the address of the erc20 at that index. We could validate if the returned address from that call matches the childZap LP token? But we also would run into the issue of different interfaces for the coins method so that would require passing func sig for that from off-chain.
| toInternalBalance: false | ||
| }), | ||
| _info.minPtAmount, | ||
| _info.deadline |
There was a problem hiding this comment.
Where we are approving the assets to the vault contract?
There was a problem hiding this comment.
Could you clarify this? We have the setApprovalsFor function which would enable the admin to do so?
| // The contract address of the curve pools lpToken | ||
| IERC20 lpToken; | ||
| // This is the index of the target root we are swapping for | ||
| int128 rootTokenIdx; |
There was a problem hiding this comment.
How we are going to validate the index of the token that the user is going to get.
There was a problem hiding this comment.
We could do it by the coins method on the curve pool I mentioned in another comment above but may be more trouble than it's worth if func sig must be passed from off-chain
No description provided.