prb-math icon indicating copy to clipboard operation
prb-math copied to clipboard

`SD59x18.exp` reverts on hugely negative numbers

Open danielattilasimon opened this issue 2 years ago • 1 comments

What version of PRBMath are you using?

v4.0.1

What version of Solidity are you using?

0.8.19

Operating System

None

Describe the bug

SD59x18.exp correctly returns 0 for inputs less than (roughly) -41.45e18, however it starts to throw PRBMath_SD59x18_Exp2_InputTooBig when the input gets hugely negative. This is because of the unchecked multiplication in exp() overflowing into positive values:

function exp(SD59x18 x) pure returns (SD59x18 result) {
    int256 xInt = x.unwrap();

    // This check prevents values greater than 192e18 from being passed to {exp2}.
    if (xInt > uEXP_MAX_INPUT) {
        revert Errors.PRBMath_SD59x18_Exp_InputTooBig(x);
    }

    unchecked {
        // Inline the fixed-point multiplication to save gas.
        int256 doubleUnitProduct = xInt * uLOG2_E;                // <== overflow
        result = exp2(wrap(doubleUnitProduct / uUNIT));
    }
}

A potential fix would be to compare the input with the smallest (most negative) number that can be safely multiplied by uLOG2_E, and return 0 if it's smaller. Alternatively, exp() could return 0 for inputs smaller than -41.45e18, which are expected to be truncated to zero by exp2() anyway.

danielattilasimon avatar Aug 02 '23 08:08 danielattilasimon

Great suggestion! I don't have the bandwidth to implement this at the moment, but I could dedicate a little time to review a PR.

PaulRBerg avatar Aug 02 '23 12:08 PaulRBerg

@PaulRBerg I opened a pr to fix this.

0x2me avatar May 02 '24 14:05 0x2me

Fixed in https://github.com/PaulRBerg/prb-math/pull/229, thanks @0x2me

PaulRBerg avatar May 06 '24 14:05 PaulRBerg