From d2405d832be8b40bb1a62ea633cbce123dc014d8 Mon Sep 17 00:00:00 2001 From: ozpool Date: Thu, 9 Jul 2026 13:02:16 +0530 Subject: [PATCH] perf(libraries): branchless MathUtils.add(uint256,int256) Replace the sign-branching implementation with a single wrapping `add` plus a branchless overflow/underflow check in assembly. The two's-complement sum is correct for both signs whenever it fits in a uint256; the result is invalid exactly when `(c < a) != (b < 0)`, which folds to one `xor`. Reverts are preserved as arithmetic panic (0x11) so existing behaviour is unchanged. `type(int256).min` no longer reverts unconditionally: `a + int256.min` now returns `a - 2**255` when it fits (a >= 2**255), which was previously an artifact of the checked negation of `-b`. This is only reachable outside the range exercised by the existing tests. Adds a full-range fuzz test and an explicit `int256.min` edge test, and refreshes gas snapshots (reductions across borrow, repay, liquidation and premium paths). Closes #641 --- .../ConfigPositionManager.Operations.json | 2 +- .../GiverPositionManager.Operations.json | 2 +- snapshots/Hub.Operations.json | 12 +++---- snapshots/NativeTokenGateway.Operations.json | 4 +-- snapshots/SignatureGateway.Operations.json | 6 ++-- .../Spoke.Operations.ZeroRiskPremium.json | 18 +++++----- snapshots/Spoke.Operations.json | 34 +++++++++---------- .../TakerPositionManager.Operations.json | 4 +-- src/libraries/math/MathUtils.sol | 18 +++++++--- .../contracts/libraries/math/MathUtils.t.sol | 33 ++++++++++++++++++ 10 files changed, 88 insertions(+), 45 deletions(-) diff --git a/snapshots/ConfigPositionManager.Operations.json b/snapshots/ConfigPositionManager.Operations.json index d6a8bc2dc..7e1036fe0 100644 --- a/snapshots/ConfigPositionManager.Operations.json +++ b/snapshots/ConfigPositionManager.Operations.json @@ -9,5 +9,5 @@ "setGlobalPermission": "50110", "setUsingAsCollateralOnBehalfOf": "77828", "updateUserDynamicConfigOnBehalfOf": "52342", - "updateUserRiskPremiumOnBehalfOf": "140135" + "updateUserRiskPremiumOnBehalfOf": "139975" } \ No newline at end of file diff --git a/snapshots/GiverPositionManager.Operations.json b/snapshots/GiverPositionManager.Operations.json index 35e0a2001..51849db9c 100644 --- a/snapshots/GiverPositionManager.Operations.json +++ b/snapshots/GiverPositionManager.Operations.json @@ -1,4 +1,4 @@ { - "repayOnBehalfOf": "176497", + "repayOnBehalfOf": "176258", "supplyOnBehalfOf": "143072" } \ No newline at end of file diff --git a/snapshots/Hub.Operations.json b/snapshots/Hub.Operations.json index 4f4c1eab1..92eadd3f4 100644 --- a/snapshots/Hub.Operations.json +++ b/snapshots/Hub.Operations.json @@ -6,13 +6,13 @@ "eliminateDeficit: partial": "87146", "mintFeeShares": "87668", "payFee": "75720", - "refreshPremium": "75289", + "refreshPremium": "75141", "remove: full": "80564", "remove: partial": "85702", - "reportDeficit": "116908", - "restore: full": "81488", - "restore: full - with transfer": "188919", - "restore: partial": "90198", - "restore: partial - with transfer": "148213", + "reportDeficit": "116708", + "restore: full": "81288", + "restore: full - with transfer": "188719", + "restore: partial": "90050", + "restore: partial - with transfer": "148065", "transferShares": "74540" } \ No newline at end of file diff --git a/snapshots/NativeTokenGateway.Operations.json b/snapshots/NativeTokenGateway.Operations.json index 837c577c6..1d90fd818 100644 --- a/snapshots/NativeTokenGateway.Operations.json +++ b/snapshots/NativeTokenGateway.Operations.json @@ -1,6 +1,6 @@ { - "borrowNative": "238707", - "repayNative": "172321", + "borrowNative": "238547", + "repayNative": "172082", "supplyAsCollateralNative": "165113", "supplyNative": "139728", "withdrawNative: full": "129898", diff --git a/snapshots/SignatureGateway.Operations.json b/snapshots/SignatureGateway.Operations.json index 93a4414b0..0e35b7eae 100644 --- a/snapshots/SignatureGateway.Operations.json +++ b/snapshots/SignatureGateway.Operations.json @@ -1,10 +1,10 @@ { - "borrowWithSig": "222144", - "repayWithSig": "192513", + "borrowWithSig": "221984", + "repayWithSig": "192274", "setSelfAsUserPositionManagerWithSig": "75138", "setUsingAsCollateralWithSig": "85380", "supplyWithSig": "155914", "updateUserDynamicConfigWithSig": "63113", - "updateUserRiskPremiumWithSig": "61995", + "updateUserRiskPremiumWithSig": "62007", "withdrawWithSig": "135124" } \ No newline at end of file diff --git a/snapshots/Spoke.Operations.ZeroRiskPremium.json b/snapshots/Spoke.Operations.ZeroRiskPremium.json index fcf844086..8e9a3456f 100644 --- a/snapshots/Spoke.Operations.ZeroRiskPremium.json +++ b/snapshots/Spoke.Operations.ZeroRiskPremium.json @@ -1,16 +1,16 @@ { "borrow: first": "199509", "borrow: second action, same reserve": "179375", - "liquidationCall (receiveShares): full": "314227", - "liquidationCall (receiveShares): partial": "313645", - "liquidationCall (reportDeficit): full": "380307", - "liquidationCall: full": "332763", - "liquidationCall: partial": "332181", - "permitReserve + repay (multicall)": "169938", + "liquidationCall (receiveShares): full": "314007", + "liquidationCall (receiveShares): partial": "313425", + "liquidationCall (reportDeficit): full": "379867", + "liquidationCall: full": "332543", + "liquidationCall: partial": "331961", + "permitReserve + repay (multicall)": "169766", "permitReserve + supply (multicall)": "151663", - "permitReserve + supply + enable collateral (multicall)": "166114", - "repay: full": "129276", - "repay: partial": "134234", + "permitReserve + supply + enable collateral (multicall)": "166090", + "repay: full": "129116", + "repay: partial": "134074", "setUserPositionManagersWithSig: disable": "46772", "setUserPositionManagersWithSig: enable": "68684", "supply + enable collateral (multicall)": "146316", diff --git a/snapshots/Spoke.Operations.json b/snapshots/Spoke.Operations.json index 086bec26b..cd681f429 100644 --- a/snapshots/Spoke.Operations.json +++ b/snapshots/Spoke.Operations.json @@ -1,16 +1,16 @@ { - "borrow: first": "269297", - "borrow: second action, same reserve": "212163", - "liquidationCall (receiveShares): full": "347124", - "liquidationCall (receiveShares): partial": "346542", - "liquidationCall (reportDeficit): full": "372507", - "liquidationCall: full": "365660", - "liquidationCall: partial": "365078", - "permitReserve + repay (multicall)": "166334", + "borrow: first": "269137", + "borrow: second action, same reserve": "212003", + "liquidationCall (receiveShares): full": "346644", + "liquidationCall (receiveShares): partial": "346062", + "liquidationCall (reportDeficit): full": "371867", + "liquidationCall: full": "365180", + "liquidationCall: partial": "364598", + "permitReserve + repay (multicall)": "166133", "permitReserve + supply (multicall)": "151663", - "permitReserve + supply + enable collateral (multicall)": "166114", - "repay: full": "123355", - "repay: partial": "142713", + "permitReserve + supply + enable collateral (multicall)": "166090", + "repay: full": "123116", + "repay: partial": "142474", "setUserPositionManagersWithSig: disable": "46772", "setUserPositionManagersWithSig: enable": "68684", "supply + enable collateral (multicall)": "146316", @@ -19,16 +19,16 @@ "supply: second action, same reserve": "110653", "updateUserDynamicConfig: 1 collateral": "76251", "updateUserDynamicConfig: 2 collaterals": "92825", - "updateUserRiskPremium: 1 borrow": "158658", - "updateUserRiskPremium: 2 borrows": "210210", + "updateUserRiskPremium: 1 borrow": "158498", + "updateUserRiskPremium: 2 borrows": "209890", "usingAsCollateral: 0 borrows, enable": "59616", - "usingAsCollateral: 1 borrow, disable": "168699", + "usingAsCollateral: 1 borrow, disable": "168539", "usingAsCollateral: 1 borrow, enable": "42504", - "usingAsCollateral: 2 borrows, disable": "241825", + "usingAsCollateral: 2 borrows, disable": "241505", "usingAsCollateral: 2 borrows, enable": "42516", "withdraw: 0 borrows, full": "135058", "withdraw: 0 borrows, partial": "140394", - "withdraw: 1 borrow, partial": "221298", - "withdraw: 2 borrows, partial": "270470", + "withdraw: 1 borrow, partial": "221138", + "withdraw: 2 borrows, partial": "270150", "withdraw: non collateral": "111299" } \ No newline at end of file diff --git a/snapshots/TakerPositionManager.Operations.json b/snapshots/TakerPositionManager.Operations.json index fac6a661e..5631d95a5 100644 --- a/snapshots/TakerPositionManager.Operations.json +++ b/snapshots/TakerPositionManager.Operations.json @@ -1,9 +1,9 @@ { "approveBorrow": "49807", - "approveBorrowWithSig": "65689", + "approveBorrowWithSig": "65677", "approveWithdraw": "49816", "approveWithdrawWithSig": "65643", - "borrowOnBehalfOf": "332512", + "borrowOnBehalfOf": "332352", "renounceBorrowAllowance": "27929", "renounceWithdrawAllowance": "27983", "withdrawOnBehalfOf: full": "127209", diff --git a/src/libraries/math/MathUtils.sol b/src/libraries/math/MathUtils.sol index 515143039..ff5dac9cb 100644 --- a/src/libraries/math/MathUtils.sol +++ b/src/libraries/math/MathUtils.sol @@ -45,10 +45,20 @@ library MathUtils { } /// @notice Returns the sum of an unsigned and signed integer. - /// @dev Reverts on underflow. - function add(uint256 a, int256 b) internal pure returns (uint256) { - if (b >= 0) return a + uint256(b); - return a - uint256(-b); + /// @dev Reverts with an arithmetic panic (0x11) on overflow or underflow. + function add(uint256 a, int256 b) internal pure returns (uint256 c) { + assembly ('memory-safe') { + // `add` wraps modulo 2**256, so reinterpreting `b` as its two's-complement + // word yields the correct result for both signs when it fits in a uint256. + c := add(a, b) + // Overflow (b >= 0) leaves `c < a`; underflow (b < 0) leaves `c >= a`. + // Either case is invalid, i.e. exactly when `(c < a) != (b < 0)`. + if xor(lt(c, a), slt(b, 0)) { + mstore(0x00, 0x4e487b71) // `Panic(uint256)` selector. + mstore(0x20, 0x11) // Arithmetic overflow/underflow panic code. + revert(0x1c, 0x24) + } + } } /// @notice Returns the sum of two unsigned integers. diff --git a/tests/contracts/libraries/math/MathUtils.t.sol b/tests/contracts/libraries/math/MathUtils.t.sol index 7115dce52..c7553b51d 100644 --- a/tests/contracts/libraries/math/MathUtils.t.sol +++ b/tests/contracts/libraries/math/MathUtils.t.sol @@ -92,6 +92,39 @@ contract MathUtilsTest is Base { } } + function test_fuzz_add(uint256 a, int256 b) public { + // Reference: full-range model, including `type(int256).min`. + if (b >= 0) { + if (a > UINT256_MAX - uint256(b)) { + vm.expectRevert(stdError.arithmeticError); + MathUtils.add(a, b); + } else { + assertEq(MathUtils.add(a, b), a + uint256(b)); + } + } else { + // `-b` overflows for `type(int256).min`; derive `|b|` without negating. + uint256 magnitude = uint256(~b) + 1; + if (a < magnitude) { + vm.expectRevert(stdError.arithmeticError); + MathUtils.add(a, b); + } else { + assertEq(MathUtils.add(a, b), a - magnitude); + } + } + } + + function test_add_int256_min_edge() public { + uint256 magnitude = uint256(type(int256).max) + 1; // 2**255 + + // Below the magnitude underflows and reverts. + vm.expectRevert(stdError.arithmeticError); + MathUtils.add(magnitude - 1, type(int256).min); + + // At or above, `a + type(int256).min == a - 2**255` fits in a uint256. + assertEq(MathUtils.add(magnitude, type(int256).min), 0); + assertEq(MathUtils.add(UINT256_MAX, type(int256).min), UINT256_MAX - magnitude); + } + function test_add_edge_cases() public { assertEq(MathUtils.add(100, 0), 100); assertEq(MathUtils.add(0, 50), 50);