Skip to content

Commit

Permalink
Feat/safe cast external fee amt (#110)
Browse files Browse the repository at this point in the history
* feat: use safe128 for getExternalFeeAmt

* feat: include updated bytecode size
  • Loading branch information
ChefMist committed Jul 2, 2024
1 parent 55f48d3 commit a7612e1
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 17 deletions.
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerBytecodeSize.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
24980
24982
4 changes: 2 additions & 2 deletions src/libraries/ProtocolFeeLibrary.sol
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ library ProtocolFeeLibrary {
}

// The protocol fee is taken from the input amount first and then the LP fee is taken from the remaining
// The swap fee is capped at 100%
// equivalent to protocolFee + lpFee(1_000_000 - protocolFee) / 1_000_000
// The swap fee is capped at 1_000_000 (100%) for cl pool and 100_000 (10%) for bin pool
// Equivalent to protocolFee + lpFee(1_000_000 - protocolFee) / 1_000_000
function calculateSwapFee(uint24 self, uint24 lpFee) internal pure returns (uint24 swapFee) {
assembly ("memory-safe") {
let numerator := mul(self, lpFee)
Expand Down
2 changes: 1 addition & 1 deletion src/pool-bin/libraries/BinPool.sol
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ library BinPool {

if (amountsInWithFees > 0) {
/// @dev calc protocol fee for current bin, totalFee * protocolFee / (protocolFee + lpFee)
bytes32 pFee = totalFee.getExternalFeeAmt(slot0Cache.protocolFee, swapState.swapFee);
bytes32 pFee = totalFee.getProtocolFeeAmt(slot0Cache.protocolFee, swapState.swapFee);
if (pFee != 0) {
swapState.feeForProtocol = swapState.feeForProtocol.add(pFee);
amountsInWithFees = amountsInWithFees.sub(pFee);
Expand Down
9 changes: 5 additions & 4 deletions src/pool-bin/libraries/math/PackedUint128Math.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ pragma solidity ^0.8.24;

import {Constants} from "../Constants.sol";
import {ProtocolFeeLibrary} from "../../../libraries/ProtocolFeeLibrary.sol";
import {SafeCast} from "./SafeCast.sol";

/// @notice This library contains functions to encode and decode two uint128 into a single bytes32
/// and interact with the encoded bytes32.
library PackedUint128Math {
using ProtocolFeeLibrary for uint24;
using SafeCast for uint256;

error PackedUint128Math__AddOverflow();
error PackedUint128Math__SubUnderflow();
Expand Down Expand Up @@ -214,7 +216,7 @@ library PackedUint128Math {
/// @param amount encoded bytes with (x, y)
/// @param protocolFee Protocol fee from the swap, also denominated in hundredths of a bip
/// @param swapFee The fee collected upon every swap in the pool (including protocol fee and LP fee), denominated in hundredths of a bip
function getExternalFeeAmt(bytes32 amount, uint24 protocolFee, uint24 swapFee) internal pure returns (bytes32 z) {
function getProtocolFeeAmt(bytes32 amount, uint24 protocolFee, uint24 swapFee) internal pure returns (bytes32 z) {
if (protocolFee == 0 || swapFee == 0) return 0;

(uint128 amountX, uint128 amountY) = decode(amount);
Expand All @@ -223,10 +225,9 @@ library PackedUint128Math {

uint128 feeForX;
uint128 feeForY;
// todo: double check on this unchecked condition
unchecked {
feeForX = fee0 == 0 ? 0 : uint128(uint256(amountX) * fee0 / swapFee);
feeForY = fee1 == 0 ? 0 : uint128(uint256(amountY) * fee1 / swapFee);
feeForX = fee0 == 0 ? 0 : (uint256(amountX) * fee0 / swapFee).safe128();
feeForY = fee1 == 0 ? 0 : (uint256(amountY) * fee1 / swapFee).safe128();
}

return encode(feeForX, feeForY);
Expand Down
31 changes: 22 additions & 9 deletions test/pool-bin/libraries/math/PackedUint128Math.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pragma solidity ^0.8.24;
import "forge-std/Test.sol";
import {Constants} from "../../../../src/pool-bin/libraries/Constants.sol";
import {PackedUint128Math} from "../../../../src/pool-bin/libraries/math/PackedUint128Math.sol";
import {SafeCast} from "../../../../src/pool-bin/libraries/math/SafeCast.sol";
import {ProtocolFeeLibrary} from "../../../../src/libraries/ProtocolFeeLibrary.sol";

contract PackedUint128MathTest is Test {
Expand Down Expand Up @@ -148,30 +149,42 @@ contract PackedUint128MathTest is Test {
assertEq(x.gt(y), x1 > y1 || x2 > y2, "testFuzz_GreaterThan::1");
}

function testFuzz_getExternalFeeAmt(bytes32 x, uint24 protocolFee, uint24 swapFee) external pure {
protocolFee = uint24(bound(protocolFee, 0, 1000_000));
swapFee = uint24(bound(swapFee, protocolFee, 1000_000));
function testFuzz_getProtocolFeeAmt(bytes32 x, uint24 protocolFee, uint24 swapFee) external pure {
protocolFee = uint24(bound(protocolFee, 0, 1_000_000));
swapFee = uint24(bound(swapFee, protocolFee, 1_000_000));

(uint128 x1, uint128 x2) = x.decode();

if (protocolFee == 0 || swapFee == 0) {
assertEq(x.getExternalFeeAmt(protocolFee, swapFee), 0);
assertEq(x.getProtocolFeeAmt(protocolFee, swapFee), 0);
} else {
uint24 fee0 = protocolFee % 4096;
uint24 fee1 = protocolFee >> 12;

uint128 x1Fee = fee0 > 0 ? uint128(uint256(x1) * fee0 / swapFee) : 0;
uint128 x2Fee = fee1 > 0 ? uint128(uint256(x2) * fee1 / swapFee) : 0;
assertEq(x.getExternalFeeAmt(protocolFee, swapFee), uint128(x1Fee).encode(uint128(x2Fee)));
assertEq(x.getProtocolFeeAmt(protocolFee, swapFee), uint128(x1Fee).encode(uint128(x2Fee)));
}
}

function test_getExternalFeeAmt() external pure {
function test_getProtocolFeeAmt_Overflow() external {
bytes32 amounts = uint128(type(uint128).max).encode(uint128(type(uint128).max));

/// @dev This shouldn't happen as swapFee passed in will be inclusive of protocolFee
/// However, adding safeCast protects against future extension of v4 in the case the fee is not inclusive
uint24 protocolFee = 100;
uint24 swapFee = 10;

vm.expectRevert(SafeCast.SafeCastOverflow.selector);
amounts.getProtocolFeeAmt(protocolFee, swapFee);
}

function test_getProtocolFeeAmt() external pure {
{
// 0% fee
bytes32 x = uint128(100).encode(uint128(100));
uint24 fee = (0 << 12) + 0; // amt / 0 = 0%
assertEq(x.getExternalFeeAmt(fee, 0), 0);
assertEq(x.getProtocolFeeAmt(fee, 0), 0);
}

{
Expand All @@ -180,7 +193,7 @@ contract PackedUint128MathTest is Test {
uint24 fee = (100 << 12) + 100;

// lpFee 0%
assertEq(x.getExternalFeeAmt(fee, 100), uint128(10000).encode(uint128(10000)));
assertEq(x.getProtocolFeeAmt(fee, 100), uint128(10000).encode(uint128(10000)));
}

{
Expand All @@ -190,7 +203,7 @@ contract PackedUint128MathTest is Test {

// 0.3% lp fee => swap fee 0.3997%
uint24 swapFee = ProtocolFeeLibrary.calculateSwapFee(1000, 3000);
assertEq(x.getExternalFeeAmt(fee, swapFee), uint128(2501).encode(uint128(2501)));
assertEq(x.getProtocolFeeAmt(fee, swapFee), uint128(2501).encode(uint128(2501)));
}
}
}

0 comments on commit a7612e1

Please sign in to comment.