Add divergence check and max slippage #105
Conversation
There was a problem hiding this comment.
It looks good, Wukong, but we'll need some changes, nonetheless.
The frontend is preparing a confirmation window where the user can select the maximum slippage he's allowing from the expected price he's being shown. However, this will not be the oracle price (like it is the case now on the trading page), but the expected price from the AMM. This means, a divergence check between AMM and oracle price is not what we need.
What we need instead is to allow the user to pass the minReturn value, similar to how it is on the oracle based amm. In case of the swap, you can simply pass on the value to the AMM contracts. they will handle it automatically. in case of the margin trade it's a bit more complicated.
one option is to let the user pass the minimum position size in the collateral currency.
there are certainly other options as well. feel free to propose one if you find another one better suited.
ghost
linked an issue
Fix conflict Move maxSlippage direction to hh
| address destToken, | ||
| uint256 sourceTokenAmount, | ||
| uint256 minReturn | ||
| ) public view; |
| function() external { | ||
| revert("loan token logic - fallback not allowed"); | ||
| // Due to contract size issue, need to keep the error message to 32 bytes length / we remove the revert function | ||
| // Remove revert function is fine as this fallback function is not payable, but the trade off is we cannot have the custom message for the fallback function error | ||
| // revert("loan token-fallback not allowed"); | ||
| } |
There was a problem hiding this comment.
Nothing is executed in this function. Should it be removed?
| @@ -707,7 +726,7 @@ contract LoanTokenLogicStandard is LoanTokenSettingsLowerAdmin { | |||
| uint256 leverageAmount, | |||
| uint256 loanTokenSent, | |||
| uint256 collateralTokenSent, | |||
| address collateralTokenAddress /// address(0) means rBTC. | |||
| address collateralTokenAddress // address(0) means ETH | |||
There was a problem hiding this comment.
Misleading comment since on line 740 this variable is assigned the value wrbtcTokenAddress
| uint256 spreadValue = sourceToDestSwapRate > rate ? sourceToDestSwapRate - rate : rate - sourceToDestSwapRate; | ||
|
|
||
| if (spreadValue != 0) { | ||
| if (rate > sourceToDestSwapRate) { |
There was a problem hiding this comment.
Why is the case where rate < sourceToDestSwapRate is not considered anymore?
No description provided.