diff --git a/src/base/Permit2Forwarder.sol b/src/base/Permit2Forwarder.sol index 41525c6e3..b04de73b4 100644 --- a/src/base/Permit2Forwarder.sol +++ b/src/base/Permit2Forwarder.sol @@ -8,6 +8,8 @@ import {IAllowanceTransfer} from "permit2/src/interfaces/IAllowanceTransfer.sol" contract Permit2Forwarder { IAllowanceTransfer public immutable permit2; + error Wrap__Permit2Reverted(address _permit2, bytes reason); + constructor(IAllowanceTransfer _permit2) { permit2 = _permit2; } @@ -17,8 +19,13 @@ contract Permit2Forwarder { function permit(address owner, IAllowanceTransfer.PermitSingle calldata permitSingle, bytes calldata signature) external payable + returns (bytes memory err) { - permit2.permit(owner, permitSingle, signature); + // use try/catch in case an actor front-runs the permit, which would DOS multicalls + try permit2.permit(owner, permitSingle, signature) {} + catch (bytes memory reason) { + err = abi.encodeWithSelector(Wrap__Permit2Reverted.selector, address(permit2), reason); + } } /// @notice allows forwarding batch permits to permit2 @@ -26,7 +33,12 @@ contract Permit2Forwarder { function permitBatch(address owner, IAllowanceTransfer.PermitBatch calldata _permitBatch, bytes calldata signature) external payable + returns (bytes memory err) { - permit2.permit(owner, _permitBatch, signature); + // use try/catch in case an actor front-runs the permit, which would DOS multicalls + try permit2.permit(owner, _permitBatch, signature) {} + catch (bytes memory reason) { + err = abi.encodeWithSelector(Wrap__Permit2Reverted.selector, address(permit2), reason); + } } } diff --git a/test/position-managers/PositionManager.multicall.t.sol b/test/position-managers/PositionManager.multicall.t.sol index 4f0ef8fa9..d83a62885 100644 --- a/test/position-managers/PositionManager.multicall.t.sol +++ b/test/position-managers/PositionManager.multicall.t.sol @@ -46,6 +46,8 @@ contract PositionManagerMulticallTest is Test, Permit2SignatureHelpers, PosmTest address bob; // bob used for permit2 signature tests uint256 bobPK; + address charlie; // charlie will NOT approve posm in setup() + uint256 charliePK; Permit2Forwarder permit2Forwarder; @@ -54,6 +56,9 @@ contract PositionManagerMulticallTest is Test, Permit2SignatureHelpers, PosmTest uint48 permitExpiration = uint48(block.timestamp + 10e18); uint48 permitNonce = 0; + // redefine error from permit2/src/PermitErrors.sol since its hard-pinned to a solidity version + error InvalidNonce(); + bytes32 PERMIT2_DOMAIN_SEPARATOR; PositionConfig config; @@ -61,6 +66,7 @@ contract PositionManagerMulticallTest is Test, Permit2SignatureHelpers, PosmTest function setUp() public { (alice, alicePK) = makeAddrAndKey("ALICE"); (bob, bobPK) = makeAddrAndKey("BOB"); + (charlie, charliePK) = makeAddrAndKey("CHARLIE"); deployFreshManagerAndRouters(); deployMintAndApprove2Currencies(); @@ -78,6 +84,13 @@ contract PositionManagerMulticallTest is Test, Permit2SignatureHelpers, PosmTest seedBalance(bob); approvePosmFor(bob); + + // do not approve posm for charlie, but approve permit2 for allowance transfer + seedBalance(charlie); + vm.startPrank(charlie); + IERC20(Currency.unwrap(currency0)).approve(address(permit2), type(uint256).max); + IERC20(Currency.unwrap(currency1)).approve(address(permit2), type(uint256).max); + vm.stopPrank(); } function test_multicall_initializePool_mint() public { @@ -135,7 +148,6 @@ contract PositionManagerMulticallTest is Test, Permit2SignatureHelpers, PosmTest bytes[] memory calls = new bytes[](1); calls[0] = abi.encodeWithSelector(IPositionManager.modifyLiquidities.selector, actions, _deadline); - address charlie = makeAddr("CHARLIE"); vm.startPrank(charlie); vm.expectRevert(abi.encodeWithSelector(IPositionManager.NotApproved.selector, charlie)); lpm.multicall(calls); @@ -321,4 +333,136 @@ contract PositionManagerMulticallTest is Test, Permit2SignatureHelpers, PosmTest assertEq(liquidity, 10e18); assertEq(lpm.ownerOf(tokenId), bob); } + + /// @notice test that a front-ran permit does not fail a multicall with permit + function test_multicall_permit_frontrun_suceeds() public { + config = PositionConfig({ + poolKey: key, + tickLower: TickMath.minUsableTick(key.tickSpacing), + tickUpper: TickMath.maxUsableTick(key.tickSpacing) + }); + + // Charlie signs permit for the two tokens + IAllowanceTransfer.PermitSingle memory permit0 = + defaultERC20PermitAllowance(Currency.unwrap(currency0), permitAmount, permitExpiration, permitNonce); + permit0.spender = address(lpm); + bytes memory sig0 = getPermitSignature(permit0, charliePK, PERMIT2_DOMAIN_SEPARATOR); + + IAllowanceTransfer.PermitSingle memory permit1 = + defaultERC20PermitAllowance(Currency.unwrap(currency1), permitAmount, permitExpiration, permitNonce); + permit1.spender = address(lpm); + bytes memory sig1 = getPermitSignature(permit1, charliePK, PERMIT2_DOMAIN_SEPARATOR); + + // bob front-runs the permits + vm.startPrank(bob); + lpm.permit(charlie, permit0, sig0); + lpm.permit(charlie, permit1, sig1); + vm.stopPrank(); + + // bob's front-run was successful + (uint160 _amount, uint48 _expiration, uint48 _nonce) = + permit2.allowance(charlie, Currency.unwrap(currency0), address(lpm)); + assertEq(_amount, permitAmount); + assertEq(_expiration, permitExpiration); + assertEq(_nonce, permitNonce + 1); + (uint160 _amount1, uint48 _expiration1, uint48 _nonce1) = + permit2.allowance(charlie, Currency.unwrap(currency1), address(lpm)); + assertEq(_amount1, permitAmount); + assertEq(_expiration1, permitExpiration); + assertEq(_nonce1, permitNonce + 1); + + // charlie tries to mint an LP token with multicall(permit, permit, mint) + bytes[] memory calls = new bytes[](3); + calls[0] = abi.encodeWithSelector(Permit2Forwarder(lpm).permit.selector, charlie, permit0, sig0); + calls[1] = abi.encodeWithSelector(Permit2Forwarder(lpm).permit.selector, charlie, permit1, sig1); + bytes memory mintCall = getMintEncoded(config, 10e18, charlie, ZERO_BYTES); + calls[2] = abi.encodeWithSelector(IPositionManager.modifyLiquidities.selector, mintCall, _deadline); + + uint256 tokenId = lpm.nextTokenId(); + vm.expectRevert(); + lpm.ownerOf(tokenId); // token does not exist + + bytes[] memory results = lpm.multicall(calls); + assertEq( + results[0], + abi.encode( + abi.encodeWithSelector( + Permit2Forwarder.Wrap__Permit2Reverted.selector, + address(permit2), + abi.encodeWithSelector(InvalidNonce.selector) + ) + ) + ); + assertEq( + results[1], + abi.encode( + abi.encodeWithSelector( + Permit2Forwarder.Wrap__Permit2Reverted.selector, + address(permit2), + abi.encodeWithSelector(InvalidNonce.selector) + ) + ) + ); + + assertEq(lpm.ownerOf(tokenId), charlie); + } + + /// @notice test that a front-ran permitBatch does not fail a multicall with permitBatch + function test_multicall_permitBatch_frontrun_suceeds() public { + config = PositionConfig({ + poolKey: key, + tickLower: TickMath.minUsableTick(key.tickSpacing), + tickUpper: TickMath.maxUsableTick(key.tickSpacing) + }); + + // Charlie signs permitBatch for the two tokens + address[] memory tokens = new address[](2); + tokens[0] = Currency.unwrap(currency0); + tokens[1] = Currency.unwrap(currency1); + + IAllowanceTransfer.PermitBatch memory permit = + defaultERC20PermitBatchAllowance(tokens, permitAmount, permitExpiration, permitNonce); + permit.spender = address(lpm); + bytes memory sig = getPermitBatchSignature(permit, charliePK, PERMIT2_DOMAIN_SEPARATOR); + + // bob front-runs the permits + vm.prank(bob); + lpm.permitBatch(charlie, permit, sig); + + // bob's front-run was successful + (uint160 _amount, uint48 _expiration, uint48 _nonce) = + permit2.allowance(charlie, Currency.unwrap(currency0), address(lpm)); + assertEq(_amount, permitAmount); + assertEq(_expiration, permitExpiration); + assertEq(_nonce, permitNonce + 1); + (uint160 _amount1, uint48 _expiration1, uint48 _nonce1) = + permit2.allowance(charlie, Currency.unwrap(currency1), address(lpm)); + assertEq(_amount1, permitAmount); + assertEq(_expiration1, permitExpiration); + assertEq(_nonce1, permitNonce + 1); + + // charlie tries to mint an LP token with multicall(permitBatch, mint) + bytes[] memory calls = new bytes[](2); + calls[0] = abi.encodeWithSelector(Permit2Forwarder(lpm).permitBatch.selector, charlie, permit, sig); + bytes memory mintCall = getMintEncoded(config, 10e18, charlie, ZERO_BYTES); + calls[1] = abi.encodeWithSelector(IPositionManager.modifyLiquidities.selector, mintCall, _deadline); + + uint256 tokenId = lpm.nextTokenId(); + vm.expectRevert(); + lpm.ownerOf(tokenId); // token does not exist + + bytes[] memory results = lpm.multicall(calls); + assertEq( + results[0], + abi.encode( + abi.encodeWithSelector( + Permit2Forwarder.Wrap__Permit2Reverted.selector, + address(permit2), + abi.encodeWithSelector(InvalidNonce.selector) + ) + ) + ); + + assertEq(lpm.ownerOf(tokenId), charlie); + } }