Divide before multiply

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-cally/blob/main/contracts/src/Cally.sol#L406-L423

Vulnerability details

Impact

Solidity integer division might truncate. As a result, performing multiplication before division can sometimes avoid loss of precision.

Proof of Concept

Cally.sol#L406-423

Cally.getDutchAuctionStrike(uint256,uint32,uint256) (Cally.sol#406-423) performs a multiplication on the result of a division: -progress = (1e18 * delta) / AUCTION_DURATION (Cally.sol#418) -auctionStrike = (progress * progress * startingStrike) / (1e18 * 1e18) (Cally.sol#419)

 function getDutchAuctionStrike(
        uint256 startingStrike,
        uint32 auctionEndTimestamp,
        uint256 reserveStrike
    ) public view returns (uint256 strike) {
        /*
            delta = max(auctionEnd - currentTimestamp, 0)
            progress = delta / auctionDuration
            auctionStrike = progress^2 * startingStrike
            strike = max(auctionStrike, reserveStrike)
        */
        uint256 delta = auctionEndTimestamp > block.timestamp ? auctionEndTimestamp - block.timestamp : 0;
        uint256 progress = (1e18 * delta) / AUCTION_DURATION;
        uint256 auctionStrike = (progress * progress * startingStrike) / (1e18 * 1e18);

        // max(auctionStrike, reserveStrike)
        strike = auctionStrike > reserveStrike ? auctionStrike : reserveStrike;
    }

In general, it's usually a good idea to re-arrange arithmetic to perform multiplication before division, unless the limit of a smaller type makes this dangerous.

Tools Used

manual analysis

Recommended Mitigation Steps

Consider ordering multiplication before division.

[outdoteth]

No exploit is given. The precision here is not critical so we disagree with the severity.

[HardlyDifficult]

This is often considered a best practice, but without details about a potential exploit or how severe the rounding error would be using reasonable inputs - this is a 1 (Low). Converting to a QA report for the warden.