diff --git a/src/base/Notifier.sol b/src/base/Notifier.sol index 2b1f6bfe..b162aab1 100644 --- a/src/base/Notifier.sol +++ b/src/base/Notifier.sol @@ -73,6 +73,12 @@ abstract contract Notifier is INotifier { if (address(_subscriber).code.length > 0) { uint256 subscriberGasLimit = block.gaslimit.calculatePortion(BLOCK_LIMIT_BPS); + + // require that the remaining gas is sufficient to notify the subscriber + // otherwise, users can select a gas limit where .notifyUnsubscribe hits OutOfGas yet the transaction/unsubscription + // can still succeed + // to account for EIP-150, condition could be 64 * gasleft() / 63 <= subscriberGasLimit + if ((64 * gasleft() / 63) < subscriberGasLimit) GasLimitTooLow.selector.revertWith(); try _subscriber.notifyUnsubscribe{gas: subscriberGasLimit}(tokenId, config, data) {} catch {} } diff --git a/src/interfaces/INotifier.sol b/src/interfaces/INotifier.sol index f70d5b8b..00257c06 100644 --- a/src/interfaces/INotifier.sol +++ b/src/interfaces/INotifier.sol @@ -10,6 +10,8 @@ interface INotifier { error NotSubscribed(); /// @notice Thrown when a subscriber does not have code error NoCodeSubscriber(); + /// @notice Thrown when a user specifies a gas limit too low to avoid valid unsubscribe notifications + error GasLimitTooLow(); /// @notice Wraps the revert message of the subscriber contract on a reverting subscription error Wrap__SubsciptionReverted(address subscriber, bytes reason); /// @notice Wraps the revert message of the subscriber contract on a reverting modify liquidity notification diff --git a/test/position-managers/PositionManager.notifier.t.sol b/test/position-managers/PositionManager.notifier.t.sol index 96624121..d617b24c 100644 --- a/test/position-managers/PositionManager.notifier.t.sol +++ b/test/position-managers/PositionManager.notifier.t.sol @@ -19,11 +19,13 @@ import {Actions} from "../../src/libraries/Actions.sol"; import {INotifier} from "../../src/interfaces/INotifier.sol"; import {MockReturnDataSubscriber, MockRevertSubscriber} from "../mocks/MockBadSubscribers.sol"; import {BalanceDelta, toBalanceDelta} from "@uniswap/v4-core/src/types/BalanceDelta.sol"; +import {BipsLibrary} from "../../src/libraries/BipsLibrary.sol"; contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { using PoolIdLibrary for PoolKey; using StateLibrary for IPoolManager; using Planner for Plan; + using BipsLibrary for uint256; MockSubscriber sub; MockReturnDataSubscriber badSubscriber; @@ -575,4 +577,33 @@ contract PositionManagerNotifierTest is Test, PosmTestSetup, GasSnapshot { ); lpm.safeTransferFrom(alice, bob, tokenId, ""); } + + /// @notice Test that users cannot forcibly avoid unsubscribe logic via gas limits + function test_fuzz_unsubscribe_with_gas_limit(uint64 gasLimit) public { + // enforce a minimum amount of gas to avoid OutOfGas reverts + gasLimit = uint64(bound(gasLimit, 150_000, block.gaslimit)); + + uint256 tokenId = lpm.nextTokenId(); + mint(config, 100e18, alice, ZERO_BYTES); + + // approve this contract to operate on alices liq + vm.startPrank(alice); + lpm.approve(address(this), tokenId); + vm.stopPrank(); + + lpm.subscribe(tokenId, config, address(sub), ZERO_BYTES); + uint256 beforeUnsubCount = sub.notifyUnsubscribeCount(); + + uint256 subscriberGasLimit = block.gaslimit.calculatePortion(100); + + if (gasLimit < subscriberGasLimit) { + // gas too low to call a valid unsubscribe + vm.expectRevert(INotifier.GasLimitTooLow.selector); + lpm.unsubscribe{gas: gasLimit}(tokenId, config, ZERO_BYTES); + } else { + // increasing gas limit succeeds and unsubscribe was called + lpm.unsubscribe{gas: gasLimit}(tokenId, config, ZERO_BYTES); + assertEq(sub.notifyUnsubscribeCount(), beforeUnsubCount + 1); + } + } }