yearn / iearn-finance

Web repository
https://v1.yearn.finance
MIT License
256 stars 294 forks source link

Add slippage calculations and warning to vaults UI #116

Open ghost opened 4 years ago

ghost commented 4 years ago

Some users are experiencing a high amount of slippage when utilizing DAI vault. Previously we utilized a slippage contract to prevent excessive slippage. We should calculate slippage on the front-end and display to the user. We should warn the user if slippage is unreasonably high.

benjaminmbrown commented 4 years ago

Do you have a reference to an example calc of this?

ghost commented 4 years ago

@benjaminmbrown this is how curve does it!

https://github.com/curvefi/curve-ui/blob/3b394439f7665406cb02f0735c324492bacc9e19/common.js

kx9x commented 4 years ago

Specifically in calc_slippage:

async function calc_slippage(deposit) {
    var real_values = [...$("[id^=currency_]")].map((x,i) => +($(x).val()));
    var Sr = real_values.reduce((a,b) => a+b, 0);

    var values = real_values.map((x,i) => cBN(Math.floor(x / c_rates[i]).toString()).toFixed(0,1));
    var token_amount = await swap.methods.calc_token_amount(values, deposit).call();
    var virtual_price = await swap.methods.get_virtual_price().call();
    var Sv = virtual_price * token_amount / 1e36;

    for(let i = 0; i < N_COINS; i++) {
        let coin_balance = parseInt(await swap.methods.balances(i).call()) * c_rates[i];
        if(!deposit) {
            if(coin_balance < real_values[i]) {
                $("#nobalance-warning").show();
                $("#nobalance-warning span").text($("label[for='currency_"+i+"']").text());
            }
            else
                $("#nobalance-warning").hide();
        }
    }
    if (deposit)
        slippage = Sv / Sr
    else
        slippage = Sr / Sv;
    slippage = slippage - 1;
    slippage = slippage || 0
    if(slippage < -0.005) {
        $("#bonus-window").hide();
        $("#highslippage-warning").removeClass('info-message').addClass('simple-error');
        $("#highslippage-warning .text").text("Warning! High slippage");
        $("#highslippage-warning .percent").text((-slippage * 100).toFixed(3));
        $("#highslippage-warning").show();
    }
    else if(slippage > 0) {
        $("#highslippage-warning").hide();
        $("#bonus-window").show();
        $("#bonus-window span").text((slippage * 100).toFixed(3));
    }
    else if(slippage <= 0) {
        $("#bonus-window").hide();
        $("#highslippage-warning").removeClass('simple-error').addClass('info-message');
        $("#highslippage-warning .text").text("Slippage");
        $("#highslippage-warning .percent").text((-slippage * 100).toFixed(3));
        $("#highslippage-warning").show();
    }
    else {
      $("#bonus-window").hide();
      $("#highslippage-warning").hide();
    }
}
ghost commented 3 years ago

Instead of using curve's slippage calculation we should use zap's on-chain 'calc_withdraw' method

jwineman commented 3 years ago

I'm a javascript developer looking to contribute to YFI - is anyone currently working on this? Do you think this would be a good first bug for me to take?

ghost commented 3 years ago

@jwineman this is a great first bug! go for it! recommend using zap's "calc_withdraw" contract method.