Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Solve ToB-UNI4-3 without restricting sync #886

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
95d28ff
rm unnecessary unlocked check in sync
wjmelements Sep 28, 2024
784dbc8
fix test
wjmelements Sep 28, 2024
dbd6f56
solve TOB-UNI4-3 without breaking external sync
wjmelements Sep 28, 2024
777360b
add doc for the error
wjmelements Sep 28, 2024
100b161
forge fmt
wjmelements Oct 1, 2024
ae093d6
updated forge snapshots
wjmelements Oct 1, 2024
d54c779
Merge branch 'main' into solve-TOB-UNI4-3
wjmelements Oct 8, 2024
5907995
rerun with latest forge
wjmelements Oct 8, 2024
0c762cb
remove unlocked check from collectProtocolFees
wjmelements Oct 8, 2024
13afb31
fix tests
wjmelements Oct 8, 2024
6d1ec06
rm unused ContractUnlocked
wjmelements Oct 10, 2024
9d090bf
Merge remote-tracking branch 'upstream/main' into solve-TOB-UNI4-3
wjmelements Oct 10, 2024
64b578f
Merge remote-tracking branch 'upstream/main' into solve-TOB-UNI4-3
wjmelements Oct 18, 2024
7d6ef40
Merge remote-tracking branch 'upstream/main' into solve-TOB-UNI4-3
wjmelements Oct 22, 2024
475de19
Update test/Sync.t.sol
wjmelements Oct 23, 2024
fd74a08
document the reason for the check
wjmelements Oct 23, 2024
e710ad1
test_collectProtocolFees_unlocked_revertsWithProtocolFeeCurrencySynced
wjmelements Oct 23, 2024
6a86713
vm.prank
wjmelements Oct 23, 2024
4d56810
debug test failure
wjmelements Oct 23, 2024
e2af146
test_collectProtocolFees_unlocked_revertsWithProtocolFeeCurrencySynced
wjmelements Oct 23, 2024
58b48fb
Merge remote-tracking branch 'upstream/main' into solve-TOB-UNI4-3
wjmelements Oct 24, 2024
b692606
restore noIsolate, and test_collectProtocolFees_locked_revertsWithPro…
wjmelements Oct 24, 2024
82dda85
test_sync_multiple_unlocked
wjmelements Oct 24, 2024
2f12837
test_sync_locked_collectProtocolFees_unlocked_revertsWithProtocolFeeC…
wjmelements Oct 24, 2024
0fded0b
getReserves snapshot appears to have changed
wjmelements Oct 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1 +1 @@
144639
144401
2 changes: 1 addition & 1 deletion .forge-snapshots/addLiquidity CA fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
170935
170697
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 @@
274206
273968
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 @@
135129
135010
Original file line number Diff line number Diff line change
@@ -1 +1 @@
292831
292593
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 @@
106321
106202
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 @@
145742
145504
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 @@
57442
57607
2 changes: 1 addition & 1 deletion .forge-snapshots/native collect protocol fees.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
59726
59762
2 changes: 1 addition & 1 deletion .forge-snapshots/poolManager bytecode size.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
24073
24128
Original file line number Diff line number Diff line change
@@ -1 +1 @@
98838
98719
2 changes: 1 addition & 1 deletion .forge-snapshots/simple addLiquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
161383
161264
2 changes: 1 addition & 1 deletion .forge-snapshots/simple swap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
123323
123204
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 @@
124651
124413
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 @@
154771
154652
2 changes: 1 addition & 1 deletion .forge-snapshots/swap against liquidity.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
116693
116574
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 @@
139735
139616
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 @@
155164
155045
Original file line number Diff line number Diff line change
@@ -1 +1 @@
206437
206199
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with dynamic fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
139368
139249
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with hooks.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
132331
132212
2 changes: 1 addition & 1 deletion .forge-snapshots/swap with lp fee and protocol fee.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
169615
169496
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 @@
145695
145576
2 changes: 1 addition & 1 deletion .forge-snapshots/update dynamic fee in before swap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
147978
147859
2 changes: 1 addition & 1 deletion src/PoolManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ contract PoolManager is IPoolManager, ProtocolFees, NoDelegateCall, ERC6909Claim
}

/// @inheritdoc IPoolManager
function sync(Currency currency) external onlyWhenUnlocked {
function sync(Currency currency) external {
// address(0) is used for the native currency
if (currency.isAddressZero()) {
// The reserves balance is not used for native settling, so we only need to reset the currency.
Expand Down
4 changes: 4 additions & 0 deletions src/ProtocolFees.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
pragma solidity ^0.8.0;

import {Currency} from "./types/Currency.sol";
import {CurrencyReserves} from "./libraries/CurrencyReserves.sol";
import {IProtocolFeeController} from "./interfaces/IProtocolFeeController.sol";
import {IProtocolFees} from "./interfaces/IProtocolFees.sol";
import {PoolKey} from "./types/PoolKey.sol";
Expand Down Expand Up @@ -53,6 +54,9 @@ abstract contract ProtocolFees is IProtocolFees, Owned {
{
if (msg.sender != address(protocolFeeController)) InvalidCaller.selector.revertWith();
if (_isUnlocked()) ContractUnlocked.selector.revertWith();
wjmelements marked this conversation as resolved.
Show resolved Hide resolved
wjmelements marked this conversation as resolved.
Show resolved Hide resolved
if (!currency.isAddressZero() && CurrencyReserves.getSyncedCurrency() == currency) {
wjmelements marked this conversation as resolved.
Show resolved Hide resolved
ProtocolFeeCurrencySynced.selector.revertWith();
}

amountCollected = (amount == 0) ? protocolFeesAccrued[currency] : amount;
protocolFeesAccrued[currency] -= amountCollected;
Expand Down
3 changes: 3 additions & 0 deletions src/interfaces/IProtocolFees.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ interface IProtocolFees {
/// @notice Thrown when collectProtocolFees or setProtocolFee is not called by the controller.
error InvalidCaller();

/// @notice Thrown when collectProtocolFees is attempted on a token that is synced.
error ProtocolFeeCurrencySynced();

/// @notice Emitted when the protocol fee controller address is updated in setProtocolFeeController.
event ProtocolFeeControllerUpdated(address indexed protocolFeeController);

Expand Down
3 changes: 1 addition & 2 deletions test/Sync.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ contract SyncTest is Test, Deployers, GasSnapshot {
currency2 = deployMintAndApproveCurrency();
}

function test_settle_failsIfLocked() public {
vm.expectRevert(IPoolManager.ManagerLocked.selector);
function test_settle_worksInIsolation() public {
wjmelements marked this conversation as resolved.
Show resolved Hide resolved
manager.sync(currency0);
}

Expand Down
Loading