QA

BathHouse.sol#L132

There is a typo in this comment.

gaurentee -> guarantee

BathHouse.sol#L285

The comment for this function is wrong

    /// @notice Admin-only function to set a Bath Token's timeDelay
    function setBathTokenMarket(address bathToken, address newMarket)
        external
        onlyAdmin
    {
        IBathToken(bathToken).setMarket(newMarket);
    }

BathHouse.sol#L382-L384 (Gas op)

Remove redundent declaration for gas saving

    function isApprovedPair(address pair) public view returns (bool) {
        return pair == approvedPairContract;
    }

BathPair.sol#L122

Should use address(0) here for readability

BathPair.sol#L206-L207 (Gas op)

Can return ++last_stratTrade_id to save on read from storage.

BathPair.sol#L230-L247 (Gas op)

This part of code can be shortenned to

if (info.askId != 0) {
    // if delta > 0 - delta is fill => handle any amount of fill here
    if (askDelta > 0) {
        logFill(askDelta, info.strategist, info.askAsset);
        IBathToken(bathAssetAddress).removeFilledTradeAmount(askDelta);
    }
    if (offer1.pay_amt) {
        IBathToken(bathAssetAddress).cancel(
            info.askId,
            offer1.pay_amt
        );
    }
}

to save gas

BathPair.sol#L250-L267

Same as above comment

BathPair.sol#L303-L319

Since this function is only used for removing a single matching element from the array.

It is better to just make it remove the element inside itself to save gas

    function removeElement(uint256 uid, uint256[] storage array)
        internal
        view
    {
        for (uint256 index = 0; index < array.length; index++) {
            if (uid == array[index]) {
                array[index] = array[array.length - 1];
                return;
            }
        }
        revert("Didnt Find that element in live list, cannot scrub");
    }

BathToken.sol#L346

There should be a check for filledAssetToRebalance != underlying here to revert on malicious case.

code-423n4 / 2022-05-rubicon-findings

QA Report #285

QA