Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/main' into rm-unnecessary-unlo…
Browse files Browse the repository at this point in the history
…cked-check
  • Loading branch information
wjmelements committed Oct 10, 2024
2 parents e4bf6f7 + fc5b4cf commit 662fb61
Show file tree
Hide file tree
Showing 47 changed files with 68 additions and 400 deletions.
Original file line number Diff line number Diff line change
@@ -1 +1 @@
144401
144425
2 changes: 1 addition & 1 deletion .forge-snapshots/addLiquidity CA fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
170697
170721
2 changes: 1 addition & 1 deletion .forge-snapshots/addLiquidity with empty hook.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
274002
274026
2 changes: 1 addition & 1 deletion .forge-snapshots/addLiquidity with native token.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
135010
135022
Original file line number Diff line number Diff line change
@@ -1 +1 @@
292593
292617
2 changes: 1 addition & 1 deletion .forge-snapshots/donate gas with 1 token.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
106202
106226
2 changes: 1 addition & 1 deletion .forge-snapshots/donate gas with 2 tokens.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
145504
145528
2 changes: 1 addition & 1 deletion .forge-snapshots/erc20 collect protocol fees.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
57326
57338
2 changes: 1 addition & 1 deletion .forge-snapshots/initialize.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
59536
51536
2 changes: 1 addition & 1 deletion .forge-snapshots/poolManager bytecode size.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
23884
23504
2 changes: 1 addition & 1 deletion .forge-snapshots/removeLiquidity CA fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
141195
141219
2 changes: 1 addition & 1 deletion .forge-snapshots/removeLiquidity with empty hook.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
130597
130621
2 changes: 1 addition & 1 deletion .forge-snapshots/removeLiquidity with native token.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
112525
112535
Original file line number Diff line number Diff line change
@@ -1 +1 @@
98719
98743
2 changes: 1 addition & 1 deletion .forge-snapshots/simple addLiquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
161264
161288
Original file line number Diff line number Diff line change
@@ -1 +1 @@
92971
92995
2 changes: 1 addition & 1 deletion .forge-snapshots/simple removeLiquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
85084
85108
2 changes: 1 addition & 1 deletion .forge-snapshots/simple swap with native.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
108503
108515
2 changes: 1 addition & 1 deletion .forge-snapshots/simple swap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
123204
123228
2 changes: 1 addition & 1 deletion .forge-snapshots/swap CA custom curve + swap noop.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
124391
124415
2 changes: 1 addition & 1 deletion .forge-snapshots/swap CA fee on unspecified.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
154630
154654
Original file line number Diff line number Diff line change
@@ -1 +1 @@
105625
105637
2 changes: 1 addition & 1 deletion .forge-snapshots/swap against liquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
116574
116598
2 changes: 1 addition & 1 deletion .forge-snapshots/swap burn 6909 for input.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
129257
129281
2 changes: 1 addition & 1 deletion .forge-snapshots/swap burn native 6909 for input.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
118713
118725
2 changes: 1 addition & 1 deletion .forge-snapshots/swap mint native output as 6909.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
139616
139628
2 changes: 1 addition & 1 deletion .forge-snapshots/swap mint output as 6909.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
155045
155069
Original file line number Diff line number Diff line change
@@ -1 +1 @@
206177
206165
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with hooks.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
132202
132226
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with return dynamic fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
145554
145542
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* @uniswap/protocols
13 changes: 9 additions & 4 deletions .github/workflows/coverage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -32,35 +32,41 @@ jobs:
run: |
{
echo 'COVERAGE<<EOF'
forge coverage --ir-minimum | grep '^|' | grep -v 'test/' | tr -d \`
forge coverage --ir-minimum | grep '^|' | grep -v 'test/' | tr -d \` | tr -d $ | tr -d { | tr -d }
echo EOF
} >> "$GITHUB_OUTPUT"
env:
FOUNDRY_RPC_URL: "${{ secrets.RPC_URL }}"

- name: Check coverage is updated
uses: actions/github-script@v5
env:
OUTPUT: ${{ steps.coverage.outputs.COVERAGE }}
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
script: |
const { OUTPUT } = process.env;
const fs = require('fs');
const file = "coverage.txt"
if(!fs.existsSync(file)) {
console.log("Nothing to check");
return
}
const currentCoverage = fs.readFileSync(file, "utf8").trim();
const newCoverage = (`${{ steps.coverage.outputs.COVERAGE }}`).trim();
const newCoverage = (`${ OUTPUT }`).trim();
if (newCoverage != currentCoverage) {
core.setFailed(`Code coverage not updated. Run : forge coverage | grep '^|' | grep -v 'test/' > coverage.txt`);
}
- name: Comment on PR
id: comment
uses: actions/github-script@v5
env:
OUTPUT: ${{ steps.coverage.outputs.COVERAGE }}
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
script: |
const { OUTPUT } = process.env;
const {data: comments} = await github.rest.issues.listComments({
owner: context.repo.owner,
repo: context.repo.repo,
Expand All @@ -69,8 +75,7 @@ jobs:
const botComment = comments.find(comment => comment.user.id === 41898282)
const output = `${{ steps.coverage.outputs.COVERAGE }}`;
const commentBody = `Forge code coverage:\n${output}\n`;
const commentBody = `Forge code coverage:\n${ OUTPUT }\n`;
if (botComment) {
github.rest.issues.updateComment({
Expand Down
4 changes: 3 additions & 1 deletion .github/workflows/tests-merge.yml
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
name: Tests

on:
workflow_dispatch:
push:
branches:
- main

jobs:
forge-tests:
name: Forge Tests
runs-on: ubuntu-latest
runs-on:
group: forge-testing

steps:
- uses: actions/checkout@v3
Expand Down
7 changes: 3 additions & 4 deletions src/PoolManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,8 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim
key.hooks.beforeInitialize(key, sqrtPriceX96);

PoolId id = key.toId();
uint24 protocolFee = _fetchProtocolFee(key);

tick = _pools[id].initialize(sqrtPriceX96, protocolFee, lpFee);
tick = _pools[id].initialize(sqrtPriceX96, lpFee);

key.hooks.afterInitialize(key, sqrtPriceX96, tick);

Expand Down Expand Up @@ -174,7 +173,7 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim
BalanceDelta hookDelta;
(callerDelta, hookDelta) = key.hooks.afterModifyLiquidity(key, params, callerDelta, feesAccrued, hookData);

// if the hook doesnt have the flag to be able to return deltas, hookDelta will always be 0
// if the hook doesn't have the flag to be able to return deltas, hookDelta will always be 0
if (hookDelta != BalanceDeltaLibrary.ZERO_DELTA) _accountPoolBalanceDelta(key, hookDelta, address(key.hooks));

_accountPoolBalanceDelta(key, callerDelta, msg.sender);
Expand Down Expand Up @@ -217,7 +216,7 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim
BalanceDelta hookDelta;
(swapDelta, hookDelta) = key.hooks.afterSwap(key, params, swapDelta, hookData, beforeSwapDelta);

// if the hook doesnt have the flag to be able to return deltas, hookDelta will always be 0
// if the hook doesn't have the flag to be able to return deltas, hookDelta will always be 0
if (hookDelta != BalanceDeltaLibrary.ZERO_DELTA) _accountPoolBalanceDelta(key, hookDelta, address(key.hooks));

_accountPoolBalanceDelta(key, swapDelta, msg.sender);
Expand Down
42 changes: 0 additions & 42 deletions src/ProtocolFees.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {IProtocolFeeController} from "./interfaces/IProtocolFeeController.sol";
import {IProtocolFees} from "./interfaces/IProtocolFees.sol";
import {PoolKey} from "./types/PoolKey.sol";
import {ProtocolFeeLibrary} from "./libraries/ProtocolFeeLibrary.sol";
import {BipsLibrary} from "./libraries/BipsLibrary.sol";
import {Owned} from "solmate/src/auth/Owned.sol";
import {PoolId} from "./types/PoolId.sol";
import {Pool} from "./libraries/Pool.sol";
Expand All @@ -17,18 +16,13 @@ abstract contract ProtocolFees is IProtocolFees, Owned {
using ProtocolFeeLibrary for uint24;
using Pool for Pool.State;
using CustomRevert for bytes4;
using BipsLibrary for uint256;

/// @inheritdoc IProtocolFees
mapping(Currency currency => uint256 amount) public protocolFeesAccrued;

/// @inheritdoc IProtocolFees
IProtocolFeeController public protocolFeeController;

// a percentage of the block.gaslimit denoted in basis points, used as the gas limit for fee controller calls
// 100 bps is 1%, at 30M gas, the limit is 300K
uint256 private constant BLOCK_LIMIT_BPS = 100;

constructor() Owned(msg.sender) {}

/// @inheritdoc IProtocolFees
Expand Down Expand Up @@ -65,42 +59,6 @@ abstract contract ProtocolFees is IProtocolFees, Owned {
/// @dev this is overridden in PoolManager.sol to give access to the _pools mapping
function _getPool(PoolId id) internal virtual returns (Pool.State storage);

/// @notice Fetch the protocol fees for a given pool
/// @dev the success of this function is false if the call fails or the returned fees are invalid
/// @dev to prevent an invalid protocol fee controller from blocking pools from being initialized
/// the success of this function is NOT checked on initialize and if the call fails, the protocol fees are set to 0.
function _fetchProtocolFee(PoolKey memory key) internal returns (uint24 protocolFee) {
if (address(protocolFeeController) != address(0)) {
uint256 controllerGasLimit = block.gaslimit.calculatePortion(BLOCK_LIMIT_BPS);

// note that EIP-150 mandates that calls requesting more than 63/64ths of remaining gas
// will be allotted no more than this amount, so controllerGasLimit must be set with this
// in mind.
if (gasleft() < controllerGasLimit) ProtocolFeeCannotBeFetched.selector.revertWith();

address toAddress = address(protocolFeeController);

bytes memory data = abi.encodeCall(IProtocolFeeController.protocolFeeForPool, (key));

bool success;
uint256 returnData;
assembly ("memory-safe") {
// only load the first 32 bytes of the return data to prevent gas griefing
success := call(controllerGasLimit, toAddress, 0, add(data, 0x20), mload(data), 0, 32)
// if success is false this wont actually be returned, instead 0 will be returned
returnData := mload(0)

// success if return data size is 32 bytes
success := and(success, eq(returndatasize(), 32))
}

// Ensure return data does not overflow a uint24 and that the underlying fees are within bounds.
protocolFee = success && (returnData == uint24(returnData)) && uint24(returnData).isValidProtocolFee()
? uint24(returnData)
: 0;
}
}

function _updateProtocolFees(Currency currency, uint256 amount) internal {
unchecked {
protocolFeesAccrued[currency] += amount;
Expand Down
2 changes: 0 additions & 2 deletions src/interfaces/IProtocolFees.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import {PoolKey} from "../types/PoolKey.sol";

/// @notice Interface for all protocol-fee related functions in the pool manager
interface IProtocolFees {
/// @notice Thrown when not enough gas is provided to look up the protocol fee
error ProtocolFeeCannotBeFetched();
/// @notice Thrown when protocol fee is set too high
error ProtocolFeeTooLarge(uint24 fee);

Expand Down
17 changes: 0 additions & 17 deletions src/libraries/BipsLibrary.sol

This file was deleted.

4 changes: 2 additions & 2 deletions src/libraries/Hooks.sol
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ library Hooks {
: (uint160(address(self)) & ALL_HOOK_MASK > 0 || fee.isDynamicFee());
}

/// @notice performs a hook call using the given calldata on the given hook that doesnt return a delta
/// @notice performs a hook call using the given calldata on the given hook that doesn't return a delta
/// @return result The complete data returned by the hook
function callHook(IHooks self, bytes memory data) internal returns (bytes memory result) {
bool success;
Expand Down Expand Up @@ -268,7 +268,7 @@ library Hooks {
// any return in unspecified is passed to the afterSwap hook for handling
int128 hookDeltaSpecified = hookReturn.getSpecifiedDelta();

// Update the swap amount according to the hook's return, and check that the swap type doesnt change (exact input/output)
// Update the swap amount according to the hook's return, and check that the swap type doesn't change (exact input/output)
if (hookDeltaSpecified != 0) {
bool exactInput = amountToSwap < 0;
amountToSwap += hookDeltaSpecified;
Expand Down
11 changes: 4 additions & 7 deletions src/libraries/Pool.sol
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,13 @@ library Pool {
if (tickUpper > TickMath.MAX_TICK) TickUpperOutOfBounds.selector.revertWith(tickUpper);
}

function initialize(State storage self, uint160 sqrtPriceX96, uint24 protocolFee, uint24 lpFee)
internal
returns (int24 tick)
{
function initialize(State storage self, uint160 sqrtPriceX96, uint24 lpFee) internal returns (int24 tick) {
if (self.slot0.sqrtPriceX96() != 0) PoolAlreadyInitialized.selector.revertWith();

tick = TickMath.getTickAtSqrtPrice(sqrtPriceX96);

self.slot0 = Slot0.wrap(bytes32(0)).setSqrtPriceX96(sqrtPriceX96).setTick(tick).setProtocolFee(protocolFee)
.setLpFee(lpFee);
// the initial protocolFee is 0 so doesn't need to be set
self.slot0 = Slot0.wrap(bytes32(0)).setSqrtPriceX96(sqrtPriceX96).setTick(tick).setLpFee(lpFee);
}

function setProtocolFee(State storage self, uint24 protocolFee) internal {
Expand Down Expand Up @@ -404,7 +401,7 @@ library Pool {
}

// Shift tick if we reached the next price, and preemptively decrement for zeroForOne swaps to tickNext - 1.
// If the swap doesnt continue (if amountRemaining == 0 or sqrtPriceLimit is met), slot0.tick will be 1 less
// If the swap doesn't continue (if amountRemaining == 0 or sqrtPriceLimit is met), slot0.tick will be 1 less
// than getTickAtSqrtPrice(slot0.sqrtPrice). This doesn't affect swaps, but donation calls should verify both
// price and tick to reward the correct LPs.
if (result.sqrtPriceX96 == step.sqrtPriceNextX96) {
Expand Down
Loading

0 comments on commit 662fb61

Please sign in to comment.