Optimize `ProtocolFeeLibrary`

[shuhuiluo]

Description of changes

The _checkProtocolFee function within the PoolManager contract has been optimized using inline assembly and a hack to perform bounds check on fixed bounds. For this purpose, two additional internal constants were added to reduce the number of runtime opcodes.

For example, the inequality a < x < b is equivalent to x - a - 1 < b - a - 1 on unchecked math. Because when a >= x, x - a - 1 underflows and the lt is false.

The existing suite of Hardhat tests doesn't seem to reflect the changes on _checkProtocolFee. The results from running forge snapshot --diff, which show the change in gas costs, are as follows:

testSwapWithProtocolFeeAllAndHookFeeAllButOnlySwapFlag() (gas: -162 (-0.021%))
testCollectFees() (gas: -148 (-0.022%))
testProtocolSwapFeeAndHookSwapFeeSameDirection() (gas: -148 (-0.023%))
testInitializeWithSwapProtocolFeeAndHookFeeDifferentDirections() (gas: -148 (-0.024%))
testHookWithdrawFeeProtocolWithdrawFee(uint8,uint8) (gas: -162 (-0.027%))
testProtocolFeeOnWithdrawalRemainsZeroIfNoHookWithdrawalFeeSet(uint8,uint8) (gas: -162 (-0.029%))
testNoHookProtocolFee(uint8,uint8) (gas: -371 (-0.039%))
testInitializeAllFees(uint8,uint8,uint8,uint8) (gas: -204 (-0.113%))
testInitializeHookProtocolSwapFee(uint8,uint8) (gas: -202 (-0.154%))

[hensha256]

Would you mind also commenting a comparison of the gas of this PR and #276 pls 🙏 That PR seems to have brought down some of our hardhat snapshots too, which this PR hasnt.

[shuhuiluo]

Would you mind also commenting a comparison of the gas of this PR and #276 pls 🙏 That PR seems to have brought down some of our hardhat snapshots too, which this PR hasnt.

@hensha256 PR #276 also modified Pool.sol and TickBitmap.sol which changed the Hardhat snapshots. I didn't modify Pool.sol because issue #190 may change the number of fee bits in the future. Optimizations for TickBitmap are planned for a separate PR. The emphasis of this PR is on _checkProtocolFee, and it serves as a demonstration of an efficient bounds check technique, which was also employed in #273.

In terms of gas cost reductions, the full assembly version presents the following diffs in forge snapshots:

testCollectFees() (gas: -55 (-0.008%))
testSwapWithProtocolFeeAllAndHookFeeAllButOnlySwapFlag() (gas: -64 (-0.008%))
testProtocolSwapFeeAndHookSwapFeeSameDirection() (gas: -56 (-0.009%))
testInitializeWithSwapProtocolFeeAndHookFeeDifferentDirections() (gas: -55 (-0.009%))
testHookWithdrawFeeProtocolWithdrawFee(uint8,uint8) (gas: -64 (-0.011%))
testProtocolFeeOnWithdrawalRemainsZeroIfNoHookWithdrawalFeeSet(uint8,uint8) (gas: -64 (-0.011%))
testNoHookProtocolFee(uint8,uint8) (gas: -176 (-0.019%))
testInitializeHookProtocolSwapFee(uint8,uint8) (gas: -80 (-0.061%))
testPoolManagerFetchFeeWhenController(uint160) (gas: -70 (-0.062%))
testInitializeAllFees(uint8,uint8,uint8,uint8) (gas: -130 (-0.072%))
testInitializeSucceedsWithHook() (gas: -10838 (-0.160%))

If adopting https://github.com/Uniswap/v4-core/pull/283#discussion_r1240219244, here's the result:

testCollectFees() (gas: -37 (-0.005%))
testProtocolSwapFeeAndHookSwapFeeSameDirection() (gas: -38 (-0.006%))
testSwapWithProtocolFeeAllAndHookFeeAllButOnlySwapFlag() (gas: -47 (-0.006%))
testInitializeWithSwapProtocolFeeAndHookFeeDifferentDirections() (gas: -38 (-0.006%))
testHookWithdrawFeeProtocolWithdrawFee(uint8,uint8) (gas: -46 (-0.008%))
testProtocolFeeOnWithdrawalRemainsZeroIfNoHookWithdrawalFeeSet(uint8,uint8) (gas: -46 (-0.008%))
testNoHookProtocolFee(uint8,uint8) (gas: -159 (-0.017%))
testInitializeAllFees(uint8,uint8,uint8,uint8) (gas: -55 (-0.030%))
testPoolManagerFetchFeeWhenController(uint160) (gas: -46 (-0.041%))
testInitializeHookProtocolSwapFee(uint8,uint8) (gas: -58 (-0.044%))
testInitializeSucceedsWithHook() (gas: -8020 (-0.119%))

[hensha256]

Hey!!

We have now changed protocol fee logic in a few ways, and will be changing it further in #547. After 547 is merged we should be done making our updates to the protocol fee logic, and then it would be great if you could update this PR to be on the new logic with updated snapshots if thats ok 🙏 how does that sound?

[shuhuiluo]

Hey!!

We have now changed protocol fee logic in a few ways, and will be changing it further in #547. After 547 is merged we should be done making our updates to the protocol fee logic, and then it would be great if you could update this PR to be on the new logic with updated snapshots if thats ok 🙏 how does that sound?

Will take a look. Thanks.

[snreynolds]

Could you update to main?