Skip to content

Commit

Permalink
🔒️ Fix possible underflow during accounting
Browse files Browse the repository at this point in the history
  • Loading branch information
@KONFeature
KONFeature committed Sep 4, 2023
1 parent 47a2786 commit 617e9bf
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 53 deletions.
72 changes: 36 additions & 36 deletions .gas-snapshot
View file
Original file line number Diff line number Diff line change
@@ -1,53 +1,53 @@
MonoPoolExecuteTest:test_constructor_ko() (gas: 178531)
MonoPoolExecuteTest:test_constructor_ko() (gas: 178609)
MonoPoolExecuteTest:test_execute_ko_InvalidOp() (gas: 11041)
MonoPoolLiquidityTest:test_addLiquidityFromZero_ko_InsuficientAllowance() (gas: 252629)
MonoPoolLiquidityTest:test_addLiquidityFromZero_ko_LeftOverDelta() (gas: 251066)
MonoPoolLiquidityTest:test_addLiquidityFromZero_ko_InsuficientAllowance() (gas: 252633)
MonoPoolLiquidityTest:test_addLiquidityFromZero_ko_LeftOverDelta() (gas: 251070)
MonoPoolLiquidityTest:test_addLiquidityFromZero_ko_NoAllowance() (gas: 193721)
MonoPoolLiquidityTest:test_addLiquidityFromZero_ok() (gas: 267999)
MonoPoolLiquidityTest:test_addLiquidity_ok() (gas: 95747)
MonoPoolLiquidityTest:test_addLiquidityFromZero_ok() (gas: 268005)
MonoPoolLiquidityTest:test_addLiquidity_ok() (gas: 95753)
MonoPoolLiquidityTest:test_claimFees_ko_NotFeeReceiver() (gas: 15821)
MonoPoolLiquidityTest:test_claimFees_ok() (gas: 74172)
MonoPoolLiquidityTest:test_claimFees_ok() (gas: 74550)
MonoPoolLiquidityTest:test_claimNoFees_ok() (gas: 19500)
MonoPoolLiquidityTest:test_fuzz_addLiquidityFromZero_ok(uint128,uint128) (runs: 1000, μ: 272620, ~: 272389)
MonoPoolLiquidityTest:test_fuzz_addLiquidity_ok(uint128,uint128) (runs: 1000, μ: 100508, ~: 100576)
MonoPoolLiquidityTest:test_removeHalfLiquidity_ok() (gas: 315902)
MonoPoolLiquidityTest:test_removeLiquidityAndSwap_ok() (gas: 318764)
MonoPoolLiquidityTest:test_removeLiquidityPostStates_ok() (gas: 33720)
MonoPoolLiquidityTest:test_fuzz_addLiquidityFromZero_ok(uint128,uint128) (runs: 1000, μ: 272621, ~: 272396)
MonoPoolLiquidityTest:test_fuzz_addLiquidity_ok(uint128,uint128) (runs: 1000, μ: 100517, ~: 100583)
MonoPoolLiquidityTest:test_removeHalfLiquidity_ok() (gas: 316211)
MonoPoolLiquidityTest:test_removeLiquidityAndSwap_ok() (gas: 319072)
MonoPoolLiquidityTest:test_removeLiquidityPostStates_ok() (gas: 34022)
MonoPoolLiquidityTest:test_removeLiquidity_ko_InsuficientLiquidity() (gas: 13004)
MonoPoolLiquidityTest:test_removeLiquidity_ok() (gas: 315894)
MonoPoolLiquidityTest:test_swapWithFees_ok() (gas: 78964)
MonoPoolLiquidityTest:test_removeLiquidity_ok() (gas: 316203)
MonoPoolLiquidityTest:test_swapWithFees_ok() (gas: 79119)
MonoPoolLiquidityTest:test_updateAddressAndFees_ko_CantHaveFeeWith0Address() (gas: 12997)
MonoPoolLiquidityTest:test_updateAddressAndFees_ko_NotTheFeeReceiver() (gas: 12658)
MonoPoolLiquidityTest:test_updateAddressAndFees_ko_TooLargeFee() (gas: 13118)
MonoPoolLiquidityTest:test_updateAddressAndFees_ok() (gas: 22019)
MonoPoolSwapNativeTest:test_fuzz_swap0to1_ok(uint128) (runs: 1000, μ: 279038, ~: 279003)
MonoPoolSwapNativeTest:test_fuzz_swap1to0_ok(uint128) (runs: 1000, μ: 253976, ~: 253959)
MonoPoolSwapNativeTest:test_removeNativeLiq_ok() (gas: 251532)
MonoPoolSwapNativeTest:test_swap0to1_ok() (gas: 275598)
MonoPoolSwapNativeTest:test_swap1to0_ko_InvalidTransferAmount_TooHigh() (gas: 223552)
MonoPoolSwapNativeTest:test_swap1to0_ko_InvalidTransferAmount_TooLow() (gas: 223739)
MonoPoolSwapNativeTest:test_swap1to0_ko_NotEnoughValue() (gas: 254743)
MonoPoolSwapNativeTest:test_swap1to0_ok() (gas: 249624)
MonoPoolSwapNativeTest:test_swap1to0_ok_ForceFeed() (gas: 292609)
MonoPoolSwapTest:test_fuzz_multiSwap_ok(uint128) (runs: 1000, μ: 174173, ~: 174265)
MonoPoolSwapTest:test_fuzz_swap0To1_ok(uint128) (runs: 1000, μ: 116482, ~: 116700)
MonoPoolSwapTest:test_swap0to1_ForceFeed_ok() (gas: 116060)
MonoPoolSwapTest:test_swap0to1_ReceiveAll_SendAllLimits_ok() (gas: 108427)
MonoPoolSwapTest:test_swap0to1_ReceiveAll_SendAll_ok() (gas: 113112)
MonoPoolSwapTest:test_swap0to1_ReceiveDirect_SendAll_ok() (gas: 113002)
MonoPoolSwapTest:test_swap0to1_ReceiveDirect_SendDirect_ok() (gas: 114832)
MonoPoolSwapTest:test_swap0to1_SwapDeadline_ok() (gas: 113124)
MonoPoolSwapNativeTest:test_fuzz_swap0to1_ok(uint128) (runs: 1000, μ: 279194, ~: 279164)
MonoPoolSwapNativeTest:test_fuzz_swap1to0_ok(uint128) (runs: 1000, μ: 254181, ~: 254160)
MonoPoolSwapNativeTest:test_removeNativeLiq_ok() (gas: 251841)
MonoPoolSwapNativeTest:test_swap0to1_ok() (gas: 275759)
MonoPoolSwapNativeTest:test_swap1to0_ko_InvalidTransferAmount_TooHigh() (gas: 223560)
MonoPoolSwapNativeTest:test_swap1to0_ko_InvalidTransferAmount_TooLow() (gas: 223747)
MonoPoolSwapNativeTest:test_swap1to0_ko_NotEnoughValue() (gas: 254944)
MonoPoolSwapNativeTest:test_swap1to0_ok() (gas: 249825)
MonoPoolSwapNativeTest:test_swap1to0_ok_ForceFeed() (gas: 293163)
MonoPoolSwapTest:test_fuzz_multiSwap_ok(uint128) (runs: 1000, μ: 174487, ~: 174576)
MonoPoolSwapTest:test_fuzz_swap0To1_ok(uint128) (runs: 1000, μ: 116633, ~: 116854)
MonoPoolSwapTest:test_swap0to1_ForceFeed_ok() (gas: 116497)
MonoPoolSwapTest:test_swap0to1_ReceiveAll_SendAllLimits_ok() (gas: 108581)
MonoPoolSwapTest:test_swap0to1_ReceiveAll_SendAll_ok() (gas: 113266)
MonoPoolSwapTest:test_swap0to1_ReceiveDirect_SendAll_ok() (gas: 113156)
MonoPoolSwapTest:test_swap0to1_ReceiveDirect_SendDirect_ok() (gas: 114986)
MonoPoolSwapTest:test_swap0to1_SwapDeadline_ok() (gas: 113279)
MonoPoolSwapTest:test_swap0to1_ko_0SwapInput() (gas: 83108)
MonoPoolSwapTest:test_swap0to1_ko_0SwapOutput() (gas: 85897)
MonoPoolSwapTest:test_swap0to1_ko_SwapDeadlineExpired() (gas: 77481)
MonoPoolSwapTest:test_swap0to1_ko_TooLargeSwap() (gas: 81867)
MonoPoolSwapTest:test_swapReceiveLimits_ko_AmountOutsideBounds_High() (gas: 70957)
MonoPoolSwapTest:test_swapReceiveLimits_ko_AmountOutsideBounds_Low() (gas: 70946)
MonoPoolSwapTest:test_swapReceiveLimits_ko_NegativeReceive() (gas: 70472)
MonoPoolSwapTest:test_swapReceiveLimits_ok() (gas: 79854)
MonoPoolSwapTest:test_swapSendLimitsWithStates_ok() (gas: 63752)
MonoPoolSwapTest:test_swapSendLimits_ko_AmountOutsideBounds_High() (gas: 59129)
MonoPoolSwapTest:test_swapSendLimits_ko_AmountOutsideBounds_Low() (gas: 59136)
MonoPoolSwapTest:test_swapReceiveLimits_ok() (gas: 80008)
MonoPoolSwapTest:test_swapSendLimitsWithStates_ok() (gas: 63907)
MonoPoolSwapTest:test_swapSendLimits_ko_AmountOutsideBounds_High() (gas: 59132)
MonoPoolSwapTest:test_swapSendLimits_ko_AmountOutsideBounds_Low() (gas: 59140)
MonoPoolSwapTest:test_swapSendLimits_ko_NegativeReceive() (gas: 72788)
MonoPoolSwapTest:test_swapSendLimits_ok() (gas: 82553)
MonoPoolSwapViaPermitTest:test_swap0to1_ok() (gas: 104960)
MonoPoolSwapTest:test_swapSendLimits_ok() (gas: 82708)
MonoPoolSwapViaPermitTest:test_swap0to1_ok() (gas: 105114)
24 changes: 7 additions & 17 deletions src/MonoPool.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,12 +322,8 @@ contract MonoPool is ReentrancyGuard {
(ptr, amount) = ptr.readUint128();

// Register the account changes
// We can perform all of this stuff in an uncheck block since the value came from a uint128 (readUint128()), and
// used in uint256 computation
unchecked {
accounter.accountChange(isToken0, amount.toInt256());
tokenState.totalReserves -= amount.toUint128();
}
accounter.accountChange(isToken0, amount.toInt256());
tokenState.totalReserves = tokenState.totalReserves - amount.toUint128();

// Simply transfer the tokens
token.transfer(to, amount);
Expand Down Expand Up @@ -366,10 +362,7 @@ contract MonoPool is ReentrancyGuard {
(ptr, to) = ptr.readAddress();

// Decrease the total reserve
// Can be done in an unchecked block since the amount will be checked on transfer below
unchecked {
tokenState.totalReserves -= amount.toUint128();
}
tokenState.totalReserves = tokenState.totalReserves - amount.toUint128();

// Simply transfer the tokens
token.transfer(to, amount);
Expand Down Expand Up @@ -418,17 +411,14 @@ contract MonoPool is ReentrancyGuard {
)
internal
{
uint256 reserves = tokenState.totalReserves;
uint256 directBalance = token.selfBalance();
uint256 totalReceived = directBalance - reserves;
uint256 totalReceived = directBalance - tokenState.totalReserves;

// If we got more than the target amount, add the overflow as protocol fees
if (totalReceived > targetAmount) {
unchecked {
uint256 overflow = totalReceived - targetAmount;
tokenState.protocolFees += overflow.toUint128();
totalReceived = targetAmount;
}
uint256 overflow = totalReceived - targetAmount;
tokenState.protocolFees += overflow.toUint128();
totalReceived = targetAmount;
}

// Register the change for either token 0 or 1 (based on the equality check)
Expand Down

0 comments on commit 617e9bf

Please sign in to comment.