opynfinance / squeeth-monorepo

Squeeth is a new financial primitive in DeFi that gives traders exposure to ETH²
https://squeeth.com/
201 stars 70 forks source link

Clean #970

Closed haythemsellami closed 1 year ago

haythemsellami commented 1 year ago

Task:

Description

Fixes (linear-task)

Type of change

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
continuouscall ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 3, 2023 at 10:22PM (UTC)
haythemsellami commented 1 year ago

I think our break returns in the transfer functions aren't necessary:

Example (but applies to other): This will break out anyways if toExchange = 0 or if we run out of orders, so if we get the final fill in the transfer function, we return 0 and it will break. In a couple of the returns you don't return 0, when it should be done that way, so that needs to be fixed, but not sure we need all these breaks.

for (uint256 i = 0; i < _params.orders.length && toExchange > 0; i++) {
            _checkOrder(_params.orders[i]);
            _useNonce(_params.orders[i].trader, _params.orders[i].nonce);
            require(!_params.orders[i].isBuying, "ZBN19");
            require(_params.orders[i].price <= _params.clearingPrice, "ZBN20");
            bool shouldBreak;
            (shouldBreak, toExchange) = NettingLib.transferOsqthFromMarketMakers(
                oSqth, _params.orders[i].trader, toExchange, _params.orders[i].quantity
            );
            if (shouldBreak) break;
        }```

Yeah that's true forgot to remove this one

aleone commented 1 year ago

Just to be clear @haythemsellami the breaks are still in 3 out of 4 of the transfer for() loops

aleone commented 1 year ago

@KMKoushik might be good if you have a few mins to have a look at events and see if theres something missing you think we want/need for f/e on zen bull netting