diff --git a/.gas-snapshot b/.gas-snapshot index f02cf562..a5babd3c 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -525,11 +525,11 @@ SafeTransferLibTest:testTransferWithStandardERC20() (gas: 36702) SafeTransferLibTest:testTransferWithStandardERC20(address,uint256,bytes) (runs: 256, μ: 36060, ~: 37605) SignedWadMathTest:testFailWadDivOverflow(int256,int256) (runs: 256, μ: 347, ~: 329) SignedWadMathTest:testFailWadDivZeroDenominator(int256) (runs: 256, μ: 296, ~: 296) -SignedWadMathTest:testFailWadMulEdgeCase() (gas: 252) -SignedWadMathTest:testFailWadMulEdgeCase2() (gas: 328) -SignedWadMathTest:testFailWadMulOverflow(int256,int256) (runs: 256, μ: 362, ~: 319) +SignedWadMathTest:testFailWadMulEdgeCase() (gas: 286) +SignedWadMathTest:testFailWadMulEdgeCase2() (gas: 309) +SignedWadMathTest:testFailWadMulOverflow(int256,int256) (runs: 256, μ: 354, ~: 319) SignedWadMathTest:testWadDiv(uint256,uint256,bool,bool) (runs: 256, μ: 5697, ~: 5714) -SignedWadMathTest:testWadMul(uint256,uint256,bool,bool) (runs: 256, μ: 5765, ~: 5781) +SignedWadMathTest:testWadMul(uint256,uint256,bool,bool) (runs: 256, μ: 5745, ~: 5712) WETHInvariants:invariantTotalSupplyEqualsBalance() (runs: 256, calls: 3840, reverts: 1838) WETHTest:testDeposit() (gas: 63535) WETHTest:testDeposit(uint256) (runs: 256, μ: 63337, ~: 65880) diff --git a/src/test/SafeCastLib.t.sol b/src/test/SafeCastLib.t.sol index cf627c23..52a6c070 100644 --- a/src/test/SafeCastLib.t.sol +++ b/src/test/SafeCastLib.t.sol @@ -25,7 +25,7 @@ contract SafeCastLibTest is DSTestPlus { assertEq(SafeCastLib.safeCastTo224(2.5e36), 2.5e36); assertEq(SafeCastLib.safeCastTo224(2.5e27), 2.5e27); } - + function testSafeCastTo216() public { assertEq(SafeCastLib.safeCastTo216(2.5e36), 2.5e36); assertEq(SafeCastLib.safeCastTo216(2.5e27), 2.5e27); @@ -396,7 +396,6 @@ contract SafeCastLibTest is DSTestPlus { assertEq(SafeCastLib.safeCastTo88(x), x); } - function testSafeCastTo80(uint256 x) public { x = bound(x, 0, type(uint80).max); diff --git a/src/test/SignedWadMath.t.sol b/src/test/SignedWadMath.t.sol index fc4f31b9..48494480 100644 --- a/src/test/SignedWadMath.t.sol +++ b/src/test/SignedWadMath.t.sol @@ -6,7 +6,12 @@ import {DSTestPlus} from "./utils/DSTestPlus.sol"; import {wadMul, wadDiv} from "../utils/SignedWadMath.sol"; contract SignedWadMathTest is DSTestPlus { - function testWadMul(uint256 x, uint256 y, bool negX, bool negY) public { + function testWadMul( + uint256 x, + uint256 y, + bool negX, + bool negY + ) public { x = bound(x, 0, 99999999999999e18); y = bound(x, 0, 99999999999999e18); @@ -39,7 +44,12 @@ contract SignedWadMathTest is DSTestPlus { wadMul(x, y); } - function testWadDiv(uint256 x, uint256 y, bool negX, bool negY) public { + function testWadDiv( + uint256 x, + uint256 y, + bool negX, + bool negY + ) public { x = bound(x, 0, 99999999e18); y = bound(x, 1, 99999999e18); diff --git a/src/utils/SafeCastLib.sol b/src/utils/SafeCastLib.sol index 5a7cc965..9e8a2af0 100644 --- a/src/utils/SafeCastLib.sol +++ b/src/utils/SafeCastLib.sol @@ -52,7 +52,7 @@ library SafeCastLib { y = uint192(x); } - + function safeCastTo184(uint256 x) internal pure returns (uint184 y) { require(x < 1 << 184); diff --git a/src/utils/SignedWadMath.sol b/src/utils/SignedWadMath.sol index f17b92c1..e7d30a43 100644 --- a/src/utils/SignedWadMath.sol +++ b/src/utils/SignedWadMath.sol @@ -58,18 +58,21 @@ function unsafeWadDiv(int256 x, int256 y) pure returns (int256 r) { function wadMul(int256 x, int256 y) pure returns (int256 r) { /// @solidity memory-safe-assembly assembly { - // Check for the specific edge case where x == -1 and y == type(int256).min - // For y == -1 and x == min int256, the second overflow check will catch this. - // See: https://secure-contracts.com/learn_evm/arithmetic-checks.html#arithmetic-checks-for-int256-multiplication - if and(eq(x, not(0)), eq(y, 0x8000000000000000000000000000000000000000000000000000000000000000)) { - revert(0, 0) - } - // Store x * y in r for now. r := mul(x, y) - // Equivalent to require(x == 0 || (x * y) / x == y) - if iszero(or(iszero(x), eq(sdiv(r, x), y))) { + // Combined overflow check (`x == 0 || (x * y) / x == y`) and edge case check + // where x == -1 and y == type(int256).min, for y == -1 and x == min int256, + // the second overflow check will catch this. + // See: https://secure-contracts.com/learn_evm/arithmetic-checks.html#arithmetic-checks-for-int256-multiplication + // Combining into 1 expression saves gas as resulting bytecode will only have 1 `JUMPI` + // rather than 2. + if iszero( + and( + or(iszero(x), eq(sdiv(r, x), y)), + or(lt(x, not(0)), sgt(y, 0x8000000000000000000000000000000000000000000000000000000000000000)) + ) + ) { revert(0, 0) }