From 4c8211970826dd6d7be00c9056b52fd95dc9f14b Mon Sep 17 00:00:00 2001 From: MerlinEgalite Date: Fri, 12 Aug 2022 14:12:36 +0200 Subject: [PATCH 01/10] =?UTF-8?q?=E2=9A=A1=EF=B8=8F=20Optimize=20`mulDivDo?= =?UTF-8?q?wn`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/utils/FixedPointMathLib.sol | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/utils/FixedPointMathLib.sol b/src/utils/FixedPointMathLib.sol index 95d80de4..71d833fe 100644 --- a/src/utils/FixedPointMathLib.sol +++ b/src/utils/FixedPointMathLib.sol @@ -9,6 +9,7 @@ library FixedPointMathLib { SIMPLIFIED FIXED POINT OPERATIONS //////////////////////////////////////////////////////////////*/ + uint256 internal constant MAX_UINT256 = 2**256 - 1; uint256 internal constant WAD = 1e18; // The scalar of ETH and most ERC20s. function mulWadDown(uint256 x, uint256 y) internal pure returns (uint256) { @@ -37,16 +38,13 @@ library FixedPointMathLib { uint256 denominator ) internal pure returns (uint256 z) { assembly { - // Store x * y in z for now. - z := mul(x, y) - - // Equivalent to require(denominator != 0 && (x == 0 || (x * y) / x == y)) - if iszero(and(iszero(iszero(denominator)), or(iszero(x), eq(div(z, x), y)))) { + // Revert if x * y > type(uint256).max + // <=> y > 0 and x > type(uint256).max / y + if or(iszero(denominator), mul(y, gt(x, div(MAX_UINT256, y)))) { revert(0, 0) } - // Divide z by the denominator. - z := div(z, denominator) + z := div(mul(x, y), denominator) } } From 1e2c9748eefa814ea74a80a30db87a3f0138d8f1 Mon Sep 17 00:00:00 2001 From: MerlinEgalite Date: Fri, 12 Aug 2022 14:16:14 +0200 Subject: [PATCH 02/10] =?UTF-8?q?=E2=9A=A1=EF=B8=8F=20Reduce=20gas=20for?= =?UTF-8?q?=20`mulDivDown`=20a=20bit=20further?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/utils/FixedPointMathLib.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/FixedPointMathLib.sol b/src/utils/FixedPointMathLib.sol index 71d833fe..76488ef8 100644 --- a/src/utils/FixedPointMathLib.sol +++ b/src/utils/FixedPointMathLib.sol @@ -40,7 +40,7 @@ library FixedPointMathLib { assembly { // Revert if x * y > type(uint256).max // <=> y > 0 and x > type(uint256).max / y - if or(iszero(denominator), mul(y, gt(x, div(MAX_UINT256, y)))) { + if iszero(mul(denominator, iszero(mul(y, gt(x, div(MAX_UINT256, y)))))) { revert(0, 0) } From 460a035b3ba331ddcf0a58b2cc09bb1e0cca18bb Mon Sep 17 00:00:00 2001 From: MerlinEgalite Date: Fri, 12 Aug 2022 14:24:29 +0200 Subject: [PATCH 03/10] =?UTF-8?q?=E2=9A=A1=EF=B8=8F=20Optimize=20`mulDivUp?= =?UTF-8?q?`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/utils/FixedPointMathLib.sol | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/utils/FixedPointMathLib.sol b/src/utils/FixedPointMathLib.sol index 76488ef8..3d96973f 100644 --- a/src/utils/FixedPointMathLib.sol +++ b/src/utils/FixedPointMathLib.sol @@ -54,14 +54,15 @@ library FixedPointMathLib { uint256 denominator ) internal pure returns (uint256 z) { assembly { - // Store x * y in z for now. - z := mul(x, y) - - // Equivalent to require(denominator != 0 && (x == 0 || (x * y) / x == y)) - if iszero(and(iszero(iszero(denominator)), or(iszero(x), eq(div(z, x), y)))) { + // Revert if x * y > type(uint256).max + // <=> y > 0 and x > type(uint256).max / y + if iszero(mul(denominator, iszero(mul(y, gt(x, div(MAX_UINT256, y)))))) { revert(0, 0) } + // Store x * y in z for now. + z := mul(x, y) + // First, divide z - 1 by the denominator and add 1. // We allow z - 1 to underflow if z is 0, because we multiply the // end result by 0 if z is zero, ensuring we return 0 if z is zero. From d1d443013164fda083bbfc07526341c9a3dbc1f3 Mon Sep 17 00:00:00 2001 From: MerlinEgalite Date: Fri, 12 Aug 2022 14:28:37 +0200 Subject: [PATCH 04/10] =?UTF-8?q?=E2=9A=A1=EF=B8=8F=20Use=20modulo=20and?= =?UTF-8?q?=20add?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/utils/FixedPointMathLib.sol | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/utils/FixedPointMathLib.sol b/src/utils/FixedPointMathLib.sol index 3d96973f..ee0cd799 100644 --- a/src/utils/FixedPointMathLib.sol +++ b/src/utils/FixedPointMathLib.sol @@ -63,10 +63,7 @@ library FixedPointMathLib { // Store x * y in z for now. z := mul(x, y) - // First, divide z - 1 by the denominator and add 1. - // We allow z - 1 to underflow if z is 0, because we multiply the - // end result by 0 if z is zero, ensuring we return 0 if z is zero. - z := mul(iszero(iszero(z)), add(div(sub(z, 1), denominator), 1)) + z := add(gt(mod(z, denominator), 0), div(z, denominator)) } } From 6ea30dfd9cf5f96d23ad3d6ef0734372d492c235 Mon Sep 17 00:00:00 2001 From: MerlinEgalite Date: Fri, 12 Aug 2022 14:34:49 +0200 Subject: [PATCH 05/10] =?UTF-8?q?=F0=9F=93=B8=20Update=20snapshot?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gas-snapshot | 84 +++++++++++++++++++++++++-------------------------- 1 file changed, 42 insertions(+), 42 deletions(-) diff --git a/.gas-snapshot b/.gas-snapshot index eb62dce4..b8e332a8 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -146,22 +146,22 @@ ERC20Test:testTransfer() (gas: 60272) ERC20Test:testTransfer(address,uint256) (runs: 256, μ: 58773, ~: 60484) ERC20Test:testTransferFrom() (gas: 83777) ERC20Test:testTransferFrom(address,uint256,uint256) (runs: 256, μ: 86464, ~: 92841) -ERC4626Test:invariantMetadata() (runs: 256, calls: 3840, reverts: 2881) +ERC4626Test:invariantMetadata() (runs: 256, calls: 3840, reverts: 2885) ERC4626Test:testFailDepositWithNoApproval() (gas: 13357) ERC4626Test:testFailDepositWithNotEnoughApproval() (gas: 86993) ERC4626Test:testFailDepositZero() (gas: 7780) ERC4626Test:testFailMintWithNoApproval() (gas: 13296) ERC4626Test:testFailRedeemWithNoShareAmount() (gas: 32342) -ERC4626Test:testFailRedeemWithNotEnoughShareAmount() (gas: 203638) +ERC4626Test:testFailRedeemWithNotEnoughShareAmount() (gas: 203631) ERC4626Test:testFailRedeemZero() (gas: 7967) ERC4626Test:testFailWithdrawWithNoUnderlyingAmount() (gas: 32289) -ERC4626Test:testFailWithdrawWithNotEnoughUnderlyingAmount() (gas: 203615) -ERC4626Test:testMetadata(string,string) (runs: 256, μ: 1479572, ~: 1471277) +ERC4626Test:testFailWithdrawWithNotEnoughUnderlyingAmount() (gas: 203595) +ERC4626Test:testMetadata(string,string) (runs: 256, μ: 1489261, ~: 1481501) ERC4626Test:testMintZero() (gas: 54595) -ERC4626Test:testMultipleMintDepositRedeemWithdraw() (gas: 411940) -ERC4626Test:testSingleDepositWithdraw(uint128) (runs: 256, μ: 201569, ~: 201579) -ERC4626Test:testSingleMintRedeem(uint128) (runs: 256, μ: 201484, ~: 201494) -ERC4626Test:testVaultInteractionsForSomeoneElse() (gas: 286247) +ERC4626Test:testMultipleMintDepositRedeemWithdraw() (gas: 411747) +ERC4626Test:testSingleDepositWithdraw(uint128) (runs: 256, μ: 201526, ~: 201536) +ERC4626Test:testSingleMintRedeem(uint128) (runs: 256, μ: 201451, ~: 201461) +ERC4626Test:testVaultInteractionsForSomeoneElse() (gas: 286209) ERC4626Test:testWithdrawZero() (gas: 52462) ERC721Test:invariantMetadata() (runs: 256, calls: 3840, reverts: 2170) ERC721Test:testApprove() (gas: 78427) @@ -240,44 +240,44 @@ ERC721Test:testTransferFromApproveAll() (gas: 92898) ERC721Test:testTransferFromApproveAll(uint256,address) (runs: 256, μ: 93182, ~: 93182) ERC721Test:testTransferFromSelf() (gas: 64776) ERC721Test:testTransferFromSelf(uint256,address) (runs: 256, μ: 65061, ~: 65061) -FixedPointMathLibTest:testDifferentiallyFuzzSqrt(uint256) (runs: 256, μ: 13827, ~: 4181) -FixedPointMathLibTest:testDivWadDown() (gas: 841) -FixedPointMathLibTest:testDivWadDown(uint256,uint256) (runs: 256, μ: 718, ~: 820) -FixedPointMathLibTest:testDivWadDownEdgeCases() (gas: 446) -FixedPointMathLibTest:testDivWadUp() (gas: 1003) -FixedPointMathLibTest:testDivWadUp(uint256,uint256) (runs: 256, μ: 809, ~: 972) -FixedPointMathLibTest:testDivWadUpEdgeCases() (gas: 462) -FixedPointMathLibTest:testFailDivWadDownOverflow(uint256,uint256) (runs: 256, μ: 443, ~: 419) -FixedPointMathLibTest:testFailDivWadDownZeroDenominator() (gas: 342) -FixedPointMathLibTest:testFailDivWadDownZeroDenominator(uint256) (runs: 256, μ: 397, ~: 397) -FixedPointMathLibTest:testFailDivWadUpOverflow(uint256,uint256) (runs: 256, μ: 398, ~: 374) -FixedPointMathLibTest:testFailDivWadUpZeroDenominator() (gas: 342) -FixedPointMathLibTest:testFailDivWadUpZeroDenominator(uint256) (runs: 256, μ: 396, ~: 396) -FixedPointMathLibTest:testFailMulDivDownOverflow(uint256,uint256,uint256) (runs: 256, μ: 437, ~: 414) -FixedPointMathLibTest:testFailMulDivDownZeroDenominator() (gas: 338) -FixedPointMathLibTest:testFailMulDivDownZeroDenominator(uint256,uint256) (runs: 256, μ: 395, ~: 395) -FixedPointMathLibTest:testFailMulDivUpOverflow(uint256,uint256,uint256) (runs: 256, μ: 460, ~: 437) -FixedPointMathLibTest:testFailMulDivUpZeroDenominator() (gas: 339) -FixedPointMathLibTest:testFailMulDivUpZeroDenominator(uint256,uint256) (runs: 256, μ: 438, ~: 438) -FixedPointMathLibTest:testFailMulWadDownOverflow(uint256,uint256) (runs: 256, μ: 424, ~: 387) -FixedPointMathLibTest:testFailMulWadUpOverflow(uint256,uint256) (runs: 256, μ: 401, ~: 364) -FixedPointMathLibTest:testMulDivDown() (gas: 1883) -FixedPointMathLibTest:testMulDivDown(uint256,uint256,uint256) (runs: 256, μ: 691, ~: 793) -FixedPointMathLibTest:testMulDivDownEdgeCases() (gas: 707) -FixedPointMathLibTest:testMulDivUp() (gas: 2295) -FixedPointMathLibTest:testMulDivUp(uint256,uint256,uint256) (runs: 256, μ: 835, ~: 1054) -FixedPointMathLibTest:testMulDivUpEdgeCases() (gas: 845) -FixedPointMathLibTest:testMulWadDown() (gas: 844) -FixedPointMathLibTest:testMulWadDown(uint256,uint256) (runs: 256, μ: 686, ~: 810) -FixedPointMathLibTest:testMulWadDownEdgeCases() (gas: 843) -FixedPointMathLibTest:testMulWadUp() (gas: 981) -FixedPointMathLibTest:testMulWadUp(uint256,uint256) (runs: 256, μ: 835, ~: 1073) -FixedPointMathLibTest:testMulWadUpEdgeCases() (gas: 959) +FixedPointMathLibTest:testDifferentiallyFuzzSqrt(uint256) (runs: 256, μ: 13830, ~: 4181) +FixedPointMathLibTest:testDivWadDown() (gas: 820) +FixedPointMathLibTest:testDivWadDown(uint256,uint256) (runs: 256, μ: 713, ~: 813) +FixedPointMathLibTest:testDivWadDownEdgeCases() (gas: 439) +FixedPointMathLibTest:testDivWadUp() (gas: 943) +FixedPointMathLibTest:testDivWadUp(uint256,uint256) (runs: 256, μ: 795, ~: 952) +FixedPointMathLibTest:testDivWadUpEdgeCases() (gas: 442) +FixedPointMathLibTest:testFailDivWadDownOverflow(uint256,uint256) (runs: 256, μ: 441, ~: 419) +FixedPointMathLibTest:testFailDivWadDownZeroDenominator() (gas: 332) +FixedPointMathLibTest:testFailDivWadDownZeroDenominator(uint256) (runs: 256, μ: 387, ~: 387) +FixedPointMathLibTest:testFailDivWadUpOverflow(uint256,uint256) (runs: 256, μ: 396, ~: 374) +FixedPointMathLibTest:testFailDivWadUpZeroDenominator() (gas: 332) +FixedPointMathLibTest:testFailDivWadUpZeroDenominator(uint256) (runs: 256, μ: 386, ~: 386) +FixedPointMathLibTest:testFailMulDivDownOverflow(uint256,uint256,uint256) (runs: 256, μ: 434, ~: 414) +FixedPointMathLibTest:testFailMulDivDownZeroDenominator() (gas: 328) +FixedPointMathLibTest:testFailMulDivDownZeroDenominator(uint256,uint256) (runs: 256, μ: 385, ~: 385) +FixedPointMathLibTest:testFailMulDivUpOverflow(uint256,uint256,uint256) (runs: 256, μ: 457, ~: 437) +FixedPointMathLibTest:testFailMulDivUpZeroDenominator() (gas: 329) +FixedPointMathLibTest:testFailMulDivUpZeroDenominator(uint256,uint256) (runs: 256, μ: 428, ~: 428) +FixedPointMathLibTest:testFailMulWadDownOverflow(uint256,uint256) (runs: 256, μ: 421, ~: 387) +FixedPointMathLibTest:testFailMulWadUpOverflow(uint256,uint256) (runs: 256, μ: 398, ~: 364) +FixedPointMathLibTest:testMulDivDown() (gas: 1813) +FixedPointMathLibTest:testMulDivDown(uint256,uint256,uint256) (runs: 256, μ: 686, ~: 786) +FixedPointMathLibTest:testMulDivDownEdgeCases() (gas: 686) +FixedPointMathLibTest:testMulDivUp() (gas: 2095) +FixedPointMathLibTest:testMulDivUp(uint256,uint256,uint256) (runs: 256, μ: 821, ~: 1034) +FixedPointMathLibTest:testMulDivUpEdgeCases() (gas: 785) +FixedPointMathLibTest:testMulWadDown() (gas: 823) +FixedPointMathLibTest:testMulWadDown(uint256,uint256) (runs: 256, μ: 681, ~: 803) +FixedPointMathLibTest:testMulWadDownEdgeCases() (gas: 822) +FixedPointMathLibTest:testMulWadUp() (gas: 921) +FixedPointMathLibTest:testMulWadUp(uint256,uint256) (runs: 256, μ: 821, ~: 1053) +FixedPointMathLibTest:testMulWadUpEdgeCases() (gas: 899) FixedPointMathLibTest:testRPow() (gas: 2164) FixedPointMathLibTest:testSqrt() (gas: 2580) FixedPointMathLibTest:testSqrt(uint256) (runs: 256, μ: 997, ~: 1013) FixedPointMathLibTest:testSqrtBack(uint256) (runs: 256, μ: 15210, ~: 340) -FixedPointMathLibTest:testSqrtBackHashed(uint256) (runs: 256, μ: 59040, ~: 59500) +FixedPointMathLibTest:testSqrtBackHashed(uint256) (runs: 256, μ: 59034, ~: 59500) FixedPointMathLibTest:testSqrtBackHashedSingle() (gas: 58937) LibStringTest:testDifferentiallyFuzzToString(uint256,bytes) (runs: 256, μ: 20749, ~: 8925) LibStringTest:testToString() (gas: 10047) From 8f64001fc8eb00d7a65aea22f0bbfec65f0deeee Mon Sep 17 00:00:00 2001 From: MerlinEgalite Date: Tue, 16 Aug 2022 11:45:08 +0200 Subject: [PATCH 06/10] =?UTF-8?q?=F0=9F=93=9D=20Come=20back=20to=20previou?= =?UTF-8?q?s=20comment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/utils/FixedPointMathLib.sol | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/utils/FixedPointMathLib.sol b/src/utils/FixedPointMathLib.sol index ee0cd799..19284c1c 100644 --- a/src/utils/FixedPointMathLib.sol +++ b/src/utils/FixedPointMathLib.sol @@ -38,8 +38,7 @@ library FixedPointMathLib { uint256 denominator ) internal pure returns (uint256 z) { assembly { - // Revert if x * y > type(uint256).max - // <=> y > 0 and x > type(uint256).max / y + // Equivalent to require(denominator != 0 && (x == 0 || (x * y) / x == y)) if iszero(mul(denominator, iszero(mul(y, gt(x, div(MAX_UINT256, y)))))) { revert(0, 0) } @@ -54,8 +53,7 @@ library FixedPointMathLib { uint256 denominator ) internal pure returns (uint256 z) { assembly { - // Revert if x * y > type(uint256).max - // <=> y > 0 and x > type(uint256).max / y + // Equivalent to require(denominator != 0 && (x == 0 || (x * y) / x == y)) if iszero(mul(denominator, iszero(mul(y, gt(x, div(MAX_UINT256, y)))))) { revert(0, 0) } From d6fb0a83128e0fb2744a43f2276c96a5351c2688 Mon Sep 17 00:00:00 2001 From: MerlinEgalite Date: Tue, 16 Aug 2022 11:47:39 +0200 Subject: [PATCH 07/10] =?UTF-8?q?=F0=9F=94=A5=20Remove=20assignment=20(doe?= =?UTF-8?q?s=20not=20change=20gas=20cost)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/utils/FixedPointMathLib.sol | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/utils/FixedPointMathLib.sol b/src/utils/FixedPointMathLib.sol index 19284c1c..05975c9d 100644 --- a/src/utils/FixedPointMathLib.sol +++ b/src/utils/FixedPointMathLib.sol @@ -58,10 +58,7 @@ library FixedPointMathLib { revert(0, 0) } - // Store x * y in z for now. - z := mul(x, y) - - z := add(gt(mod(z, denominator), 0), div(z, denominator)) + z := add(gt(mod(mul(x, y), denominator), 0), div(mul(x, y), denominator)) } } From 880ad0f851049ba547588f6f5d78d7ec0fe32942 Mon Sep 17 00:00:00 2001 From: MerlinEgalite Date: Tue, 16 Aug 2022 17:58:56 +0200 Subject: [PATCH 08/10] =?UTF-8?q?=F0=9F=93=9D=20Better=20comments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/utils/FixedPointMathLib.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/utils/FixedPointMathLib.sol b/src/utils/FixedPointMathLib.sol index 05975c9d..d45ddd1c 100644 --- a/src/utils/FixedPointMathLib.sol +++ b/src/utils/FixedPointMathLib.sol @@ -38,7 +38,7 @@ library FixedPointMathLib { uint256 denominator ) internal pure returns (uint256 z) { assembly { - // Equivalent to require(denominator != 0 && (x == 0 || (x * y) / x == y)) + // Equivalent to require(denominator != 0 && (y == 0 || x <= type(uint256).max / y)) if iszero(mul(denominator, iszero(mul(y, gt(x, div(MAX_UINT256, y)))))) { revert(0, 0) } @@ -53,7 +53,7 @@ library FixedPointMathLib { uint256 denominator ) internal pure returns (uint256 z) { assembly { - // Equivalent to require(denominator != 0 && (x == 0 || (x * y) / x == y)) + // Equivalent to require(denominator != 0 && (y == 0 || x <= type(uint256).max / y)) if iszero(mul(denominator, iszero(mul(y, gt(x, div(MAX_UINT256, y)))))) { revert(0, 0) } From cc81c0235360e60ff8b0636f45034d595dc1a6b3 Mon Sep 17 00:00:00 2001 From: MerlinEgalite Date: Sun, 16 Oct 2022 16:49:22 +0200 Subject: [PATCH 09/10] =?UTF-8?q?=F0=9F=92=A1=20Add=20comments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/utils/FixedPointMathLib.sol | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/utils/FixedPointMathLib.sol b/src/utils/FixedPointMathLib.sol index d45ddd1c..57334bcb 100644 --- a/src/utils/FixedPointMathLib.sol +++ b/src/utils/FixedPointMathLib.sol @@ -43,6 +43,7 @@ library FixedPointMathLib { revert(0, 0) } + // Divide x * y by the denominator. z := div(mul(x, y), denominator) } } @@ -58,6 +59,8 @@ library FixedPointMathLib { revert(0, 0) } + // If x * y modulo the denominator is strictly greater than 0, + // 1 is added to round up the division of x * y by the denominator. z := add(gt(mod(mul(x, y), denominator), 0), div(mul(x, y), denominator)) } } From 49fa6269485e3ead10f8e57ed393cbd521954c5f Mon Sep 17 00:00:00 2001 From: t11s Date: Mon, 17 Oct 2022 08:36:54 -0700 Subject: [PATCH 10/10] Update FixedPointMathLib.sol --- src/utils/FixedPointMathLib.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/src/utils/FixedPointMathLib.sol b/src/utils/FixedPointMathLib.sol index 57334bcb..364268c2 100644 --- a/src/utils/FixedPointMathLib.sol +++ b/src/utils/FixedPointMathLib.sol @@ -10,6 +10,7 @@ library FixedPointMathLib { //////////////////////////////////////////////////////////////*/ uint256 internal constant MAX_UINT256 = 2**256 - 1; + uint256 internal constant WAD = 1e18; // The scalar of ETH and most ERC20s. function mulWadDown(uint256 x, uint256 y) internal pure returns (uint256) {