Skip to content

Commit

Permalink
Merge branch 'main' into oz-L06-permit2forwarder-dos
Browse files Browse the repository at this point in the history
  • Loading branch information
saucepoint authored Aug 29, 2024
2 parents b03a53b + e2c3d0b commit 7efca6b
Show file tree
Hide file tree
Showing 18 changed files with 84 additions and 43 deletions.
Original file line number Diff line number Diff line change
@@ -1 +1 @@
421127
421104
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_permit.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
79484
79506
Original file line number Diff line number Diff line change
@@ -1 +1 @@
62372
62394
2 changes: 1 addition & 1 deletion .forge-snapshots/PositionManager_permit_twice.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
45260
45282
7 changes: 1 addition & 6 deletions src/PositionManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@ pragma solidity ^0.8.24;
import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol";
import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol";
import {PoolIdLibrary} from "@uniswap/v4-core/src/types/PoolId.sol";
import {Currency, CurrencyLibrary} from "@uniswap/v4-core/src/types/Currency.sol";
import {Currency} from "@uniswap/v4-core/src/types/Currency.sol";
import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol";
import {SafeCast} from "@uniswap/v4-core/src/libraries/SafeCast.sol";
import {Position} from "@uniswap/v4-core/src/libraries/Position.sol";
import {StateLibrary} from "@uniswap/v4-core/src/libraries/StateLibrary.sol";
import {TransientStateLibrary} from "@uniswap/v4-core/src/libraries/TransientStateLibrary.sol";
import {SafeTransferLib} from "solmate/src/utils/SafeTransferLib.sol";
import {ERC20} from "solmate/src/tokens/ERC20.sol";
import {IAllowanceTransfer} from "permit2/src/interfaces/IAllowanceTransfer.sol";

import {ERC721Permit_v4} from "./base/ERC721Permit_v4.sol";
Expand All @@ -25,7 +23,6 @@ import {BaseActionsRouter} from "./base/BaseActionsRouter.sol";
import {Actions} from "./libraries/Actions.sol";
import {Notifier} from "./base/Notifier.sol";
import {CalldataDecoder} from "./libraries/CalldataDecoder.sol";
import {INotifier} from "./interfaces/INotifier.sol";
import {Permit2Forwarder} from "./base/Permit2Forwarder.sol";
import {SlippageCheckLibrary} from "./libraries/SlippageCheck.sol";
import {PositionConfigId, PositionConfigIdLibrary} from "./libraries/PositionConfigId.sol";
Expand Down Expand Up @@ -107,8 +104,6 @@ contract PositionManager is
Notifier,
Permit2Forwarder
{
using SafeTransferLib for *;
using CurrencyLibrary for Currency;
using PoolIdLibrary for PoolKey;
using PositionConfigLibrary for PositionConfig;
using StateLibrary for IPoolManager;
Expand Down
2 changes: 1 addition & 1 deletion src/V4Router.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pragma solidity ^0.8.19;
import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol";
import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol";
import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol";
import {Currency, CurrencyLibrary} from "@uniswap/v4-core/src/types/Currency.sol";
import {Currency} from "@uniswap/v4-core/src/types/Currency.sol";
import {TickMath} from "@uniswap/v4-core/src/libraries/TickMath.sol";
import {SafeCast} from "@uniswap/v4-core/src/libraries/SafeCast.sol";

Expand Down
1 change: 0 additions & 1 deletion src/base/BaseActionsRouter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ pragma solidity ^0.8.24;
import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol";
import {SafeCallback} from "./SafeCallback.sol";
import {CalldataDecoder} from "../libraries/CalldataDecoder.sol";
import {Actions} from "../libraries/Actions.sol";
import {ActionConstants} from "../libraries/ActionConstants.sol";

/// @notice Abstract contract for performing a combination of actions on Uniswap v4.
Expand Down
2 changes: 0 additions & 2 deletions src/base/DeltaResolver.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,12 @@ import {Currency} from "@uniswap/v4-core/src/types/Currency.sol";
import {TransientStateLibrary} from "@uniswap/v4-core/src/libraries/TransientStateLibrary.sol";
import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol";
import {ImmutableState} from "./ImmutableState.sol";
import {SafeCast} from "@uniswap/v4-core/src/libraries/SafeCast.sol";
import {ActionConstants} from "../libraries/ActionConstants.sol";

/// @notice Abstract contract used to sync, send, and settle funds to the pool manager
/// @dev Note that sync() is called before any erc-20 transfer in `settle`.
abstract contract DeltaResolver is ImmutableState {
using TransientStateLibrary for IPoolManager;
using SafeCast for *;

/// @notice Emitted trying to settle a positive delta.
error DeltaNotPositive(Currency currency);
Expand Down
15 changes: 5 additions & 10 deletions src/base/Notifier.sol
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,8 @@ abstract contract Notifier is INotifier {
if (_subscriber != NO_SUBSCRIBER) revert AlreadySubscribed(address(_subscriber));
subscriber[tokenId] = ISubscriber(newSubscriber);

bool success = _call(
address(newSubscriber), abi.encodeWithSelector(ISubscriber.notifySubscribe.selector, tokenId, config, data)
);
bool success =
_call(address(newSubscriber), abi.encodeCall(ISubscriber.notifySubscribe, (tokenId, config, data)));

if (!success) {
Wrap__SubsciptionReverted.selector.bubbleUpAndRevertWith(address(newSubscriber));
Expand Down Expand Up @@ -87,9 +86,7 @@ abstract contract Notifier is INotifier {

bool success = _call(
address(_subscriber),
abi.encodeWithSelector(
ISubscriber.notifyModifyLiquidity.selector, tokenId, config, liquidityChange, feesAccrued
)
abi.encodeCall(ISubscriber.notifyModifyLiquidity, (tokenId, config, liquidityChange, feesAccrued))
);

if (!success) {
Expand All @@ -100,10 +97,8 @@ abstract contract Notifier is INotifier {
function _notifyTransfer(uint256 tokenId, address previousOwner, address newOwner) internal {
ISubscriber _subscriber = subscriber[tokenId];

bool success = _call(
address(_subscriber),
abi.encodeWithSelector(ISubscriber.notifyTransfer.selector, tokenId, previousOwner, newOwner)
);
bool success =
_call(address(_subscriber), abi.encodeCall(ISubscriber.notifyTransfer, (tokenId, previousOwner, newOwner)));

if (!success) {
Wrap__TransferNotificationReverted.selector.bubbleUpAndRevertWith(address(_subscriber));
Expand Down
7 changes: 7 additions & 0 deletions src/base/UnorderedNonce.sol
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,11 @@ contract UnorderedNonce {
uint256 flipped = nonces[owner][wordPos] ^= bit;
if (flipped & bit == 0) revert NonceAlreadyUsed();
}

/// @notice Revoke a nonce by spending it, preventing it from being used again
/// @dev Used in cases where a valid nonce has not been broadcasted onchain, and the owner wants to revoke the validity of the nonce
/// @dev payable so it can be multicalled with native-token related actions
function revokeNonce(uint256 nonce) external payable {
_useUnorderedNonce(msg.sender, nonce);
}
}
2 changes: 0 additions & 2 deletions src/interfaces/IPositionManager.sol
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.24;

import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol";
import {Currency} from "@uniswap/v4-core/src/types/Currency.sol";
import {PositionConfig} from "../libraries/PositionConfig.sol";

import {INotifier} from "./INotifier.sol";
Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/IV4Router.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
pragma solidity ^0.8.19;

import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol";
import {Currency, CurrencyLibrary} from "@uniswap/v4-core/src/types/Currency.sol";
import {Currency} from "@uniswap/v4-core/src/types/Currency.sol";
import {PathKey} from "../libraries/PathKey.sol";

/// @title IV4Router
Expand Down
23 changes: 10 additions & 13 deletions src/lens/Quoter.sol
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-or-later
pragma solidity ^0.8.20;

import {Hooks} from "@uniswap/v4-core/src/libraries/Hooks.sol";
import {TickMath} from "@uniswap/v4-core/src/libraries/TickMath.sol";
import {IHooks} from "@uniswap/v4-core/src/interfaces/IHooks.sol";
import {IUnlockCallback} from "@uniswap/v4-core/src/interfaces/callback/IUnlockCallback.sol";
import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol";
import {BalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol";
import {Currency} from "@uniswap/v4-core/src/types/Currency.sol";
Expand All @@ -17,17 +14,17 @@ import {StateLibrary} from "@uniswap/v4-core/src/libraries/StateLibrary.sol";
import {SafeCallback} from "../base/SafeCallback.sol";

contract Quoter is IQuoter, SafeCallback {
using Hooks for IHooks;
using PoolIdLibrary for PoolKey;
using PathKeyLib for PathKey;
using StateLibrary for IPoolManager;

/// @dev cache used to check a safety condition in exact output swaps.
uint128 private amountOutCached;

/// @dev min valid reason is 3-words long
/// @dev int128[2] + sqrtPriceX96After padded to 32bytes + intializeTicksLoaded padded to 32bytes
uint256 internal constant MINIMUM_VALID_RESPONSE_LENGTH = 96;
/// @dev min valid reason is 6-words long (192 bytes)
/// @dev int128[2] includes 32 bytes for offset, 32 bytes for length, and 32 bytes for each element
/// @dev Plus sqrtPriceX96After padded to 32 bytes and initializedTicksLoaded padded to 32 bytes
uint256 internal constant MINIMUM_VALID_RESPONSE_LENGTH = 192;

struct QuoteResult {
int128[] deltaAmounts;
Expand Down Expand Up @@ -60,7 +57,7 @@ contract Quoter is IQuoter, SafeCallback {
override
returns (int128[] memory deltaAmounts, uint160 sqrtPriceX96After, uint32 initializedTicksLoaded)
{
try poolManager.unlock(abi.encodeWithSelector(this._quoteExactInputSingle.selector, params)) {}
try poolManager.unlock(abi.encodeCall(this._quoteExactInputSingle, (params))) {}
catch (bytes memory reason) {
return _handleRevertSingle(reason);
}
Expand All @@ -75,7 +72,7 @@ contract Quoter is IQuoter, SafeCallback {
uint32[] memory initializedTicksLoadedList
)
{
try poolManager.unlock(abi.encodeWithSelector(this._quoteExactInput.selector, params)) {}
try poolManager.unlock(abi.encodeCall(this._quoteExactInput, (params))) {}
catch (bytes memory reason) {
return _handleRevert(reason);
}
Expand All @@ -87,7 +84,7 @@ contract Quoter is IQuoter, SafeCallback {
override
returns (int128[] memory deltaAmounts, uint160 sqrtPriceX96After, uint32 initializedTicksLoaded)
{
try poolManager.unlock(abi.encodeWithSelector(this._quoteExactOutputSingle.selector, params)) {}
try poolManager.unlock(abi.encodeCall(this._quoteExactOutputSingle, (params))) {}
catch (bytes memory reason) {
if (params.sqrtPriceLimitX96 == 0) delete amountOutCached;
return _handleRevertSingle(reason);
Expand All @@ -104,7 +101,7 @@ contract Quoter is IQuoter, SafeCallback {
uint32[] memory initializedTicksLoadedList
)
{
try poolManager.unlock(abi.encodeWithSelector(this._quoteExactOutput.selector, params)) {}
try poolManager.unlock(abi.encodeCall(this._quoteExactOutput, (params))) {}
catch (bytes memory reason) {
return _handleRevert(reason);
}
Expand Down Expand Up @@ -270,7 +267,7 @@ contract Quoter is IQuoter, SafeCallback {

/// @dev quote an ExactOutput swap on a pool, then revert with the result
function _quoteExactOutputSingle(QuoteExactSingleParams calldata params) public selfOnly returns (bytes memory) {
// if no price limit has been specified, cache the output amount for comparison in the swap callback
// if no price limit has been specified, cache the output amount for comparison inside the _swap function
if (params.sqrtPriceLimitX96 == 0) amountOutCached = params.exactAmount;

(, int24 tickBefore,,) = poolManager.getSlot0(params.poolKey.toId());
Expand All @@ -297,7 +294,7 @@ contract Quoter is IQuoter, SafeCallback {
}

/// @dev Execute a swap and return the amounts delta, as well as relevant pool state
/// @notice if amountSpecified > 0, the swap is exactInput, otherwise exactOutput
/// @notice if amountSpecified < 0, the swap is exactInput, otherwise exactOutput
function _swap(
PoolKey memory poolKey,
bool zeroForOne,
Expand Down
4 changes: 2 additions & 2 deletions src/libraries/CalldataDecoder.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ library CalldataDecoder {
/// @notice equivalent to SliceOutOfBounds.selector
bytes4 constant SLICE_ERROR_SELECTOR = 0x3b99b53d;

/// @dev equivalent to: abi.decode(params, (uint256[], bytes[])) in calldata
/// @dev equivalent to: abi.decode(params, (bytes, bytes[])) in calldata
function decodeActionsRouterParams(bytes calldata _bytes)
internal
pure
Expand Down Expand Up @@ -147,7 +147,7 @@ library CalldataDecoder {
}
}

/// @dev equivalent to: abi.decode(params, (IV4Router.ExactInputSingleParams))
/// @dev equivalent to: abi.decode(params, (IV4Router.ExactOutputSingleParams))
function decodeSwapExactOutSingleParams(bytes calldata params)
internal
pure
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/PoolTicksCounter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ pragma solidity >=0.8.20;

import {IPoolManager} from "@uniswap/v4-core/src/interfaces/IPoolManager.sol";
import {PoolKey} from "@uniswap/v4-core/src/types/PoolKey.sol";
import {PoolId, PoolIdLibrary} from "@uniswap/v4-core/src/types/PoolId.sol";
import {PoolIdLibrary} from "@uniswap/v4-core/src/types/PoolId.sol";
import {StateLibrary} from "@uniswap/v4-core/src/libraries/StateLibrary.sol";

library PoolTicksCounter {
Expand Down
12 changes: 12 additions & 0 deletions test/UnorderedNonce.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,16 @@ contract UnorderedNonceTest is Test {
unorderedNonce.spendNonce(address(this), second);
}
}

function test_fuzz_revokeNonce(uint256 nonce) public {
unorderedNonce.revokeNonce(nonce);
vm.expectRevert(UnorderedNonce.NonceAlreadyUsed.selector);
unorderedNonce.revokeNonce(nonce);
}

function test_fuzz_revokeNonce_twoNonces(uint256 first, uint256 second) public {
unorderedNonce.revokeNonce(first);
if (first == second) vm.expectRevert(UnorderedNonce.NonceAlreadyUsed.selector);
unorderedNonce.revokeNonce(second);
}
}
19 changes: 19 additions & 0 deletions test/erc721Permit/ERC721Permit.permitForAll.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,25 @@ contract ERC721PermitForAllTest is Test {
vm.stopPrank();
}

/// @notice revoking a nonce prevents it from being used in permitForAll()
function test_fuzz_erc721PermitForAll_revokedNonceUsed(uint256 nonce) public {
// alice revokes the nonce
vm.prank(alice);
erc721Permit.revokeNonce(nonce);

uint256 deadline = block.timestamp;
bytes32 digest = _getPermitForAllDigest(bob, true, nonce, deadline);
// alice signs a permit for bob
(uint8 v, bytes32 r, bytes32 s) = vm.sign(alicePK, digest);
bytes memory signature = abi.encodePacked(r, s, v);

// Nonce does not work with permitForAll
vm.startPrank(bob);
vm.expectRevert(UnorderedNonce.NonceAlreadyUsed.selector);
erc721Permit.permitForAll(alice, bob, true, deadline, nonce, signature);
vm.stopPrank();
}

// Helpers related to permitForAll
function _permitForAll(uint256 privateKey, address owner, address operator, bool approved, uint256 nonce)
internal
Expand Down
21 changes: 21 additions & 0 deletions test/position-managers/Permit.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,27 @@ contract PermitTest is Test, PosmTestSetup {
vm.stopPrank();
}

/// @notice revoking a nonce prevents it from being used in permit()
function test_fuzz_noPermit_revokeRevert(uint256 nonce) public {
uint256 liquidityAlice = 1e18;
vm.prank(alice);
mint(config, liquidityAlice, alice, ZERO_BYTES);
uint256 tokenIdAlice = lpm.nextTokenId() - 1;

// alice revokes the nonce
vm.prank(alice);
lpm.revokeNonce(nonce);

// alice gives bob spender permissions
bytes32 digest = getDigest(bob, tokenIdAlice, nonce, block.timestamp + 1);

(uint8 v, bytes32 r, bytes32 s) = vm.sign(alicePK, digest);
bytes memory signature = abi.encodePacked(r, s, v);

vm.expectRevert(UnorderedNonce.NonceAlreadyUsed.selector);
lpm.permit(bob, tokenIdAlice, block.timestamp + 1, nonce, signature);
}

// Bob can use alice's signature to permit & decrease liquidity
function test_permit_operatorSelfPermit() public {
uint256 liquidityAlice = 1e18;
Expand Down

0 comments on commit 7efca6b

Please sign in to comment.