From 977d43c42d1a8551f70874d61ea8ead65e5c0689 Mon Sep 17 00:00:00 2001 From: Paul Razvan Berg Date: Fri, 25 Nov 2022 15:30:23 +0200 Subject: [PATCH] refactor: return base if exponent is 1 in "pow" test: update "pow" tests to account for new behavior --- src/SD59x18.sol | 6 ++- src/UD60x18.sol | 6 ++- test/sd59x18/fixed-point/pow/pow.t.sol | 51 ++++++++++++++++++++++---- test/sd59x18/fixed-point/pow/pow.tree | 17 +++++---- test/ud60x18/fixed-point/pow/pow.t.sol | 51 ++++++++++++++++++++++---- test/ud60x18/fixed-point/pow/pow.tree | 11 ++++-- 6 files changed, 113 insertions(+), 29 deletions(-) diff --git a/src/SD59x18.sol b/src/SD59x18.sol index f2bb2e28..49414e81 100644 --- a/src/SD59x18.sol +++ b/src/SD59x18.sol @@ -688,7 +688,11 @@ function pow(SD59x18 x, SD59x18 y) pure returns (SD59x18 result) { if (x.isZero()) { result = y.isZero() ? UNIT : ZERO; } else { - result = exp2(mul(log2(x), y)); + if (y.eq(UNIT)) { + result = x; + } else { + result = exp2(log2(x).mul(y)); + } } } diff --git a/src/UD60x18.sol b/src/UD60x18.sol index 5ee2e530..7fa7a9d9 100644 --- a/src/UD60x18.sol +++ b/src/UD60x18.sol @@ -485,7 +485,11 @@ function pow(UD60x18 x, UD60x18 y) pure returns (UD60x18 result) { if (x.isZero()) { result = y.isZero() ? UNIT : ZERO; } else { - result = exp2(mul(log2(x), y)); + if (y.eq(UNIT)) { + result = x; + } else { + result = exp2(mul(log2(x), y)); + } } } diff --git a/test/sd59x18/fixed-point/pow/pow.t.sol b/test/sd59x18/fixed-point/pow/pow.t.sol index c2b0d075..4be85e9a 100644 --- a/test/sd59x18/fixed-point/pow/pow.t.sol +++ b/test/sd59x18/fixed-point/pow/pow.t.sol @@ -34,7 +34,7 @@ contract SD59x18__PowTest is SD59x18__BaseTest { function testCannotPow__BaseNegative() external BaseNotZero { SD59x18 x = sd(-0.000000000000000001e18); - SD59x18 y = sd(1e18); + SD59x18 y = sd(2e18); vm.expectRevert(abi.encodeWithSelector(PRBMathSD59x18__LogInputTooSmall.selector, x)); pow(x, y); } @@ -60,10 +60,39 @@ contract SD59x18__PowTest is SD59x18__BaseTest { _; } - function testCannotPow__ExponentGreaterThanMaxPermitted() external BaseNotZero BasePositive ExponentNotZero { + function exponentOneSets() internal returns (Set[] memory) { + delete sets; + sets.push(set({ x: 1e18, y: 1e18, expected: 1e18 })); + sets.push(set({ x: E, y: 1e18, expected: E })); + sets.push(set({ x: PI, y: 1e18, expected: PI })); + return sets; + } + + function testPow__ExponentOne() + external + parameterizedTest(exponentOneSets()) + BaseNotZero + BasePositive + ExponentNotZero + { + SD59x18 actual = pow(s.x, s.y); + assertEq(actual, s.expected); + } + + modifier ExponentNotOne() { + _; + } + + function testCannotPow__ExponentGreaterThanMaxPermitted() + external + BaseNotZero + BasePositive + ExponentNotZero + ExponentNotOne + { SD59x18 x = MAX_PERMITTED.add(sd(1)); - SD59x18 y = sd(1e18); - vm.expectRevert(abi.encodeWithSelector(PRBMathSD59x18__Exp2InputTooBig.selector, sd(192e18))); + SD59x18 y = sd(1e18 + 1); + vm.expectRevert(abi.encodeWithSelector(PRBMathSD59x18__Exp2InputTooBig.selector, sd(192e18 + 192))); pow(x, y); } @@ -101,6 +130,7 @@ contract SD59x18__PowTest is SD59x18__BaseTest { BaseNotZero BasePositive ExponentNotZero + ExponentNotOne ExponentLessThanOrEqualToMaxPermitted { SD59x18 actual = pow(s.x, s.y); @@ -115,7 +145,7 @@ contract SD59x18__PowTest is SD59x18__BaseTest { sets.push(set({ x: 0.24e18, y: 11e18, expected: 0.000000152168114316e18 })); sets.push(set({ x: 0.5e18, y: 0.7373e18, expected: 0.59986094064056398e18 })); sets.push(set({ x: 0.799291e18, y: 69e18, expected: 0.000000193481602919e18 })); - sets.push(set({ x: 1e18, y: 1e18, expected: 1e18 })); + sets.push(set({ x: 1e18, y: 2e18, expected: 1e18 })); sets.push(set({ x: 1e18, y: PI, expected: 1e18 })); sets.push(set({ x: 2e18, y: 1.5e18, expected: 2_828427124746190097 })); sets.push(set({ x: E, y: E, expected: 15_154262241479263804 })); @@ -131,12 +161,16 @@ contract SD59x18__PowTest is SD59x18__BaseTest { sets.push( set({ x: 340282366920938463463374607431768211455e18, - y: 1e18, - expected: 340282366920938457799636748773271041925_182187238234989391 + y: 1e18 + 1, + expected: 340282366920938487979097481391762860220_000000000004665573 }) ); sets.push( - set({ x: MAX_PERMITTED, y: 1e18, expected: 6277101735386680659358266643954607672760_949507286104301595e18 }) + set({ + x: MAX_PERMITTED, + y: 1e18 - 1, + expected: 6277101735386679823624773486129835356722228023657461399187e18 + }) ); return sets; } @@ -147,6 +181,7 @@ contract SD59x18__PowTest is SD59x18__BaseTest { BaseNotZero BasePositive ExponentNotZero + ExponentNotOne ExponentLessThanOrEqualToMaxPermitted { SD59x18 actual = pow(s.x, s.y); diff --git a/test/sd59x18/fixed-point/pow/pow.tree b/test/sd59x18/fixed-point/pow/pow.tree index 6c01e416..f2afae57 100644 --- a/test/sd59x18/fixed-point/pow/pow.tree +++ b/test/sd59x18/fixed-point/pow/pow.tree @@ -11,10 +11,13 @@ pow.t.sol ├── when the exponent is zero │ └── it should return one └── when the exponent is not zero - ├── when the exponent is greater than the maximum permitted - │ └── it should revert - └── when the exponent is less than or equal to the maximum permitted - ├── when the exponent is negative - │ └── it should return the correct value - └── when the exponent is positive - └── it should return the correct value + ├── when the exponent is one + │ └── it should return the base + └── when the exponent is not one + ├── when the exponent is greater than the maximum permitted + │ └── it should revert + └── when the exponent is less than or equal to the maximum permitted + ├── when the exponent is negative + │ └── it should return the correct value + └── when the exponent is positive + └── it should return the correct value diff --git a/test/ud60x18/fixed-point/pow/pow.t.sol b/test/ud60x18/fixed-point/pow/pow.t.sol index 513926e5..27d2400e 100644 --- a/test/ud60x18/fixed-point/pow/pow.t.sol +++ b/test/ud60x18/fixed-point/pow/pow.t.sol @@ -65,10 +65,40 @@ contract UD60x18__PowTest is UD60x18__BaseTest { _; } - function testCannotPow__ExponentGreaterThanOrEqualToMaxPermitted() external BaseNotZero ExponentNotZero { + function exponentOneSets() internal returns (Set[] memory) { + delete sets; + sets.push(set({ x: 1e18, y: 1e18, expected: 1e18 })); + sets.push(set({ x: E, y: 1e18, expected: E })); + sets.push(set({ x: PI, y: 1e18, expected: PI })); + return sets; + } + + function testPow__ExponentOne() + external + parameterizedTest(exponentOneSets()) + BaseNotZero + BaseGreaterThanOrEqualToOne + ExponentNotZero + ExponentLessThanOrEqualToMaxPermitted + { + UD60x18 actual = pow(s.x, s.y); + assertEq(actual, s.expected); + } + + modifier ExponentNotOne() { + _; + } + + function testCannotPow__ExponentGreaterThanOrEqualToMaxPermitted() + external + BaseNotZero + BaseGreaterThanOrEqualToOne + ExponentNotZero + ExponentNotOne + { UD60x18 x = MAX_PERMITTED.add(ud(1)); - UD60x18 y = ud(1e18); - vm.expectRevert(abi.encodeWithSelector(PRBMathUD60x18__Exp2InputTooBig.selector, ud(192e18))); + UD60x18 y = ud(1e18 + 1); + vm.expectRevert(abi.encodeWithSelector(PRBMathUD60x18__Exp2InputTooBig.selector, ud(192e18 + 192))); pow(x, y); } @@ -78,11 +108,11 @@ contract UD60x18__PowTest is UD60x18__BaseTest { function powSets() internal returns (Set[] memory) { delete sets; - sets.push(set({ x: 1e18, y: 1e18, expected: 1e18 })); + sets.push(set({ x: 1e18, y: 2e18, expected: 1e18 })); sets.push(set({ x: 1e18, y: PI, expected: 1e18 })); sets.push(set({ x: 2e18, y: 1.5e18, expected: 2_828427124746190097 })); - sets.push(set({ x: E, y: E, expected: 15_154262241479263804 })); sets.push(set({ x: E, y: 1.66976e18, expected: 5_310893029888037563 })); + sets.push(set({ x: E, y: E, expected: 15_154262241479263804 })); sets.push(set({ x: PI, y: PI, expected: 36_462159607207910473 })); sets.push(set({ x: 11e18, y: 28.5e18, expected: 478290249106383504726311660571_903531944106436935 })); sets.push( @@ -94,12 +124,16 @@ contract UD60x18__PowTest is UD60x18__BaseTest { sets.push( set({ x: 340282366920938463463374607431768211455e18, - y: 1e18, - expected: 340282366920938457799636748773271041925_182187238234989391 + y: 1e18 + 1, + expected: 340282366920938487979097481391762860220_000000000004665573 }) ); sets.push( - set({ x: MAX_PERMITTED, y: 1e18, expected: 6277101735386680659358266643954607672760_949507286104301595e18 }) + set({ + x: MAX_PERMITTED, + y: 1e18 - 1, + expected: 6277101735386679823624773486129835356722228023657461399187e18 + }) ); return sets; } @@ -109,6 +143,7 @@ contract UD60x18__PowTest is UD60x18__BaseTest { parameterizedTest(powSets()) BaseNotZero ExponentNotZero + ExponentNotOne ExponentLessThanOrEqualToMaxPermitted { UD60x18 actual = pow(s.x, s.y); diff --git a/test/ud60x18/fixed-point/pow/pow.tree b/test/ud60x18/fixed-point/pow/pow.tree index 3261e1da..31f593d3 100644 --- a/test/ud60x18/fixed-point/pow/pow.tree +++ b/test/ud60x18/fixed-point/pow/pow.tree @@ -11,7 +11,10 @@ pow.t.sol ├── when the exponent is zero │ └── it should return one └── when the exponent is not zero - ├── when the exponent is greater than the maximum permitted - │ └── it should revert - └── when the exponent is less than or equal to the maximum permitted - └── it should return the correct value + ├── when the exponent is one + │ └── it should return the base + └── when the exponent is not one + ├── when the exponent is greater than the maximum permitted + │ └── it should revert + └── when the exponent is less than or equal to the maximum permitted + └── it should return the correct value