feat: [internal-r8] require lock for sync and require unlock for coll…

.forge-snapshots/BinPoolManagerTest#testGasDonate.snap

@@ -1 +1 @@
-119431
+119687

.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-1.snap

@@ -1 +1 @@
-971609
+971865

.forge-snapshots/BinPoolManagerTest#testGasMintNneBins-2.snap

@@ -1 +1 @@
-330920
+331176

.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-1.snap

@@ -1 +1 @@
-338521
+338777

.forge-snapshots/BinPoolManagerTest#testGasMintOneBin-2.snap

@@ -1 +1 @@
-141071
+141327

.forge-snapshots/BinPoolManagerTest#testGasSwapMultipleBins.snap

@@ -1 +1 @@
-173973
+174101

.forge-snapshots/BinPoolManagerTest#testGasSwapOverBigBinIdGate.snap

@@ -1 +1 @@
-180002
+180130

.forge-snapshots/BinPoolManagerTest#testGasSwapSingleBin.snap

@@ -1 +1 @@
-134004
+134132

.forge-snapshots/BinPoolManagerTest#testMintNativeCurrency.snap

@@ -1 +1 @@
-305477
+305605

.forge-snapshots/CLPoolManagerTest#addLiquidity_fromEmpty.snap

@@ -1 +1 @@
-348312
+348568

.forge-snapshots/CLPoolManagerTest#addLiquidity_fromNonEmpty.snap

@@ -1 +1 @@
-163783
+164039

.forge-snapshots/CLPoolManagerTest#donateBothTokens.snap

@@ -1 +1 @@
-164024
+164280

.forge-snapshots/CLPoolManagerTest#gasDonateOneToken.snap

@@ -1 +1 @@
-108967
+109095

.forge-snapshots/CLPoolManagerTest#swap_againstLiquidity.snap

@@ -1 +1 @@
-131784
+131912

.forge-snapshots/CLPoolManagerTest#swap_leaveSurplusTokenInVault.snap

@@ -1 +1 @@
-164348
+164476

.forge-snapshots/CLPoolManagerTest#swap_runOutOfLiquidity.snap

@@ -1 +1 @@
-149801
+149929

.forge-snapshots/VaultBytecodeSize.snap

@@ -1 +1 @@
-7769
+7931

src/Vault.sol

@@ -121,7 +121,7 @@ contract Vault is IVault, VaultToken, Ownable {
         }
     }
 
-    function sync(Currency currency) public {
+    function sync(Currency currency) public override isLocked {
         VaultReserve.alreadySettledLastSync();
         if (currency.isNative()) return;
         uint256 balance = currency.balanceOfSelf();
@@ -157,6 +157,7 @@ contract Vault is IVault, VaultToken, Ownable {
 
     /// @inheritdoc IVault
     function collectFee(Currency currency, uint256 amount, address recipient) external onlyRegisteredApp {
+        if (SettlementGuard.getLocker() != address(0)) revert LockHeld();
         reservesOfApp[msg.sender][currency] -= amount;
         currency.transfer(recipient, amount);
     }

src/interfaces/IProtocolFees.sol

@@ -44,6 +44,7 @@ interface IProtocolFees {
     function setProtocolFeeController(IProtocolFeeController controller) external;
 
     /// @notice Collects the protocol fee accrued in the given currency, called by the owner or the protocol fee controller
+    /// @dev This will revert if vault is locked
     /// @param recipient The address to which the protocol fees should be sent
     /// @param currency The currency in which to collect the protocol fees
     /// @param amount The amount of protocol fees to collect

src/interfaces/IVault.sol

@@ -29,6 +29,9 @@ interface IVault is IVaultToken {
     /// @notice Thrown when there is no locker
     error NoLocker();
 
+    /// @notice Thrown when lock is held by someone
+    error LockHeld();
+
     function isAppRegistered(address app) external returns (bool);
 
     /// @notice Returns the reserves for a a given pool type and currency

test/vault/Vault.t.sol

@@ -461,6 +461,15 @@ contract VaultTest is Test, NoIsolate, GasSnapshot, TokenFixture {
 
     function test_CollectFee() public noIsolate {
         vault.lock(abi.encodeCall(VaultTest._test_CollectFee, ()));
+
+        // collectFee must be called when vault is unlocked
+        vm.prank(address(poolManager1));
+        vault.collectFee(currency0, 10 ether, address(poolManager1));
+
+        // after collectFee assert
+        assertEq(IERC20(Currency.unwrap(currency0)).balanceOf(address(vault)), 0 ether);
+        assertEq(vault.reservesOfApp(address(poolKey1.poolManager), currency0), 0 ether);
+        assertEq(IERC20(Currency.unwrap(currency0)).balanceOf(address(poolManager1)), 10 ether);
     }
 
     function _test_CollectFee() external {
@@ -472,15 +481,6 @@ contract VaultTest is Test, NoIsolate, GasSnapshot, TokenFixture {
         assertEq(IERC20(Currency.unwrap(currency0)).balanceOf(address(vault)), 10 ether);
         assertEq(vault.reservesOfApp(address(poolKey1.poolManager), currency0), 10 ether);
         assertEq(IERC20(Currency.unwrap(currency0)).balanceOf(address(poolManager1)), 0 ether);
-
-        // collectFee
-        vm.prank(address(poolManager1));
-        vault.collectFee(currency0, 10 ether, address(poolManager1));
-
-        // after collectFee assert
-        assertEq(IERC20(Currency.unwrap(currency0)).balanceOf(address(vault)), 0 ether);
-        assertEq(vault.reservesOfApp(address(poolKey1.poolManager), currency0), 0 ether);
-        assertEq(IERC20(Currency.unwrap(currency0)).balanceOf(address(poolManager1)), 10 ether);
     }
 
     function test_CollectFeeFromRandomUser() public {
@@ -491,6 +491,16 @@ contract VaultTest is Test, NoIsolate, GasSnapshot, TokenFixture {
         vault.collectFee(currency0, 10 ether, bob);
     }
 
+    function test_CollectFeeWhenVaultIsLocked() public noIsolate {
+        vm.expectRevert(IVault.LockHeld.selector);
+        vault.lock(abi.encodeCall(VaultTest._test_CollectFeeWhenVaultIsLocked, ()));
+    }
+
+    function _test_CollectFeeWhenVaultIsLocked() external {
+        vm.prank(address(poolManager1));
+        vault.collectFee(currency0, 10 ether, address(poolManager1));
+    }
+
     function testTake_failsWithNoLiquidity() public {
         vault.lock(abi.encodeCall(VaultTest._testTake_failsWithNoLiquidity, ()));
     }

test/vault/VaultSync.t.sol

@@ -37,7 +37,11 @@ contract VaultSyncTest is Test, TokenFixture, GasSnapshot, NoIsolate {
         });
     }
 
-    function test_sync_balanceIsZero() public noIsolate {
+    function test_sync_balanceIsZero() public {
+        vault.lock(abi.encodeCall(VaultSyncTest._test_sync_balanceIsZero, ()));
+    }
+
+    function _test_sync_balanceIsZero() external {
         assertEq(currency0.balanceOf(address(vault)), uint256(0));
         vault.sync(currency0);
 
@@ -46,7 +50,11 @@ contract VaultSyncTest is Test, TokenFixture, GasSnapshot, NoIsolate {
         assertEq(amount, 0);
     }
 
-    function test_sync_balanceIsNonZero() public noIsolate {
+    function test_sync_balanceIsNonZero() public {
+        vault.lock(abi.encodeCall(VaultSyncTest._test_sync_balanceIsNonZero, ()));
+    }
+
+    function _test_sync_balanceIsNonZero() external {
         // transfer without calling sync ahead cause token lost
         currency0.transfer(address(vault), 10 ether);
         uint256 currency0Balance = currency0.balanceOf(address(vault));
@@ -116,7 +124,16 @@ contract VaultSyncTest is Test, TokenFixture, GasSnapshot, NoIsolate {
         assertEq(amount, 10 ether);
     }
 
-    function test_sync_twiceWithoutSettle() public noIsolate {
+    function test_sync_NoLock() public {
+        vm.expectRevert(abi.encodeWithSelector(IVault.NoLocker.selector));
+        vault.sync(currency0);
+    }
+
+    function test_sync_twiceWithoutSettle() public {
+        vault.lock(abi.encodeCall(VaultSyncTest._test_sync_twiceWithoutSettle, ()));
+    }
+
+    function _test_sync_twiceWithoutSettle() external {
         vault.sync(currency0);
         vm.expectRevert(abi.encodeWithSelector(VaultReserve.LastSyncNotSettled.selector));
         vault.sync(currency0);