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

Add settleAndRefund #2

Merged
merged 5 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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 @@
92310
92245
Original file line number Diff line number Diff line change
@@ -1 +1 @@
67502
67437
Original file line number Diff line number Diff line change
@@ -1 +1 @@
151057
150992
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerTest#testGasBurnOneBin.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
68681
68616
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerTest#testGasDonate.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
53973
53952
Original file line number Diff line number Diff line change
@@ -1 +1 @@
968068
968047
Original file line number Diff line number Diff line change
@@ -1 +1 @@
121582
121561
Original file line number Diff line number Diff line change
@@ -1 +1 @@
337258
337237
Original file line number Diff line number Diff line change
@@ -1 +1 @@
56319
56298
Original file line number Diff line number Diff line change
@@ -1 +1 @@
93083
93040
Original file line number Diff line number Diff line change
@@ -1 +1 @@
95068
95025
Original file line number Diff line number Diff line change
@@ -1 +1 @@
74512
74469
Original file line number Diff line number Diff line change
@@ -1 +1 @@
318966
318945
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerTest#testNoOpGas_Burn.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
41941
41876
Original file line number Diff line number Diff line change
@@ -1 +1 @@
19558
19493
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerTest#testNoOpGas_Mint.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
37410
37345
2 changes: 1 addition & 1 deletion .forge-snapshots/BinPoolManagerTest#testNoOpGas_Swap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
22764
22699
Original file line number Diff line number Diff line change
@@ -1 +1 @@
348473
348452
Original file line number Diff line number Diff line change
@@ -1 +1 @@
61021
61000
Original file line number Diff line number Diff line change
@@ -1 +1 @@
241433
241390
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPoolManagerTest#donateBothTokens.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
84159
84138
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPoolManagerTest#gasDonateOneToken.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
53488
53445
Original file line number Diff line number Diff line change
@@ -1 +1 @@
43387
43322
Original file line number Diff line number Diff line change
@@ -1 +1 @@
56488
56445
Original file line number Diff line number Diff line change
@@ -1 +1 @@
104597
104554
Original file line number Diff line number Diff line change
@@ -1 +1 @@
25044312
25044269
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPoolManagerTest#swap_simple.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
36401
36336
Original file line number Diff line number Diff line change
@@ -1 +1 @@
103140
103030
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPoolManagerTest#swap_withHooks.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
42051
41986
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPoolManagerTest#swap_withNative.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
36404
36339
Original file line number Diff line number Diff line change
@@ -1 +1 @@
19513
19448
Original file line number Diff line number Diff line change
@@ -1 +1 @@
27680
27615
2 changes: 1 addition & 1 deletion .forge-snapshots/CLPoolManagerTest#testNoOp_gas_Swap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
21962
21897
2 changes: 1 addition & 1 deletion .forge-snapshots/VaultTest#Vault.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
6762
7016
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
122519
2 changes: 1 addition & 1 deletion .forge-snapshots/VaultTest#lockSettledWhenFlashloan.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
158832
158811
Original file line number Diff line number Diff line change
@@ -1 +1 @@
47017
46974
2 changes: 1 addition & 1 deletion .forge-snapshots/VaultTest#lockSettledWhenSwap.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
47016
46973
2 changes: 1 addition & 1 deletion .forge-snapshots/VaultTest#registerPoolManager.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
24484
24506
2 changes: 1 addition & 1 deletion .forge-snapshots/VaultTest#testLock_NoOp.snap
Original file line number Diff line number Diff line change
@@ -1 +1 @@
11502
11532
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
75609
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
33010
18 changes: 17 additions & 1 deletion src/Vault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,30 @@ contract Vault is IVault, VaultToken, Ownable {
}

/// @inheritdoc IVault
function settle(Currency currency) external payable override isLocked returns (uint256 paid) {
function settle(Currency currency) public payable override isLocked returns (uint256 paid) {
uint256 reservesBefore = reservesOfVault[currency];
reservesOfVault[currency] = currency.balanceOfSelf();
paid = reservesOfVault[currency] - reservesBefore;
// subtraction must be safe
SettlementGuard.accountDelta(msg.sender, currency, -(paid.toInt128()));
}

function settleAndRefund(Currency currency) external payable isLocked returns (uint256 paid, uint256 refund) {
ChefMist marked this conversation as resolved.
Show resolved Hide resolved
paid = settle(currency);
Copy link
Collaborator

@ChefSnoopy ChefSnoopy Mar 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we calculate the amount we need to refund in advance? then just need to execute SettlementGuard.accountDelta once if have excess token.

function settleAndRefund(Currency currency, address to)
        external
        payable
        override
        isLocked
        returns (uint256 paid, uint256 refund)
    {
        paid = currency.balanceOfSelf() - reservesOfVault[currency];
        // need to consider the case when the currencyDelta is negative ?.
        uint256 currentCurrencyDelta = (SettlementGuard.getCurrencyDelta(msg.sender, currency)).toUint256();
       
        if (paid > currentCurrencyDelta) {
            refund = paid - currentCurrencyDelta;
            paid = currentCurrencyDelta;
        }
        reservesOfVault[currency] += paid;
        SettlementGuard.accountDelta(msg.sender, currency, -(paid.toInt128()));
        if (refund > 0) {
            currency.transfer(to, refund);
        }
    }

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! adopted this idea since it'll save 1 sload for refund case. though did a small tweak to handle negative currency delta as settle() wont revert with SafeCastOverflow for negative balance delta


int256 afterCurrencyDelta = SettlementGuard.getCurrencyDelta(msg.sender, currency);
if (afterCurrencyDelta < 0) {
/// If user currencyDelta is negative (vault owes owner) after settle: either user have overpaid
/// or someone have transfer token to the vault.
refund = uint256(-afterCurrencyDelta);

/// thus refund msg.sender and update accountDelta
SettlementGuard.accountDelta(msg.sender, currency, refund.toInt128());
reservesOfVault[currency] -= refund;
currency.transfer(msg.sender, refund);
}
}

/// @inheritdoc IVault
function settleFor(Currency currency, address target, uint256 amount) external isLocked {
/// @notice settle all outstanding debt if amount is 0
Expand Down
3 changes: 3 additions & 0 deletions src/interfaces/IVault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ interface IVault is IVaultToken {
/// @notice Called by the user to pay what is owed
function settle(Currency token) external payable returns (uint256 paid);

/// @notice Called by the user to pay what is owed. If the payment is more than the debt, the surplus is refunded
function settleAndRefund(Currency token) external payable returns (uint256 paid, uint256 refund);

/// @notice move the delta from target to the msg.sender, only payment delta can be moved
/// @param currency The currency to settle
/// @param target The address whose delta will be updated
Expand Down
3 changes: 3 additions & 0 deletions test/vault/FakePoolManagerRouter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,9 @@ contract FakePoolManagerRouter {
// settle ETH
vault.settle{value: 5 ether}(CurrencyLibrary.NATIVE);
vault.take(CurrencyLibrary.NATIVE, address(this), 5 ether);
} else if (data[0] == 0x18) {
vault.settleAndRefund(poolKey.currency0);
vault.settleAndRefund(poolKey.currency1);
}

return "";
Expand Down
34 changes: 33 additions & 1 deletion test/vault/Vault.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,38 @@ contract VaultTest is Test, GasSnapshot {
vault.lock(hex"02");
}

function testSettleAndRefund_WithErc20Transfer() public {
// simulate someone transferred token to vault
currency0.transfer(address(vault), 10 ether);
assertEq(IERC20(Currency.unwrap(currency0)).balanceOf(address(fakePoolManagerRouter)), 0 ether);
assertEq(IERC20(Currency.unwrap(currency1)).balanceOf(address(fakePoolManagerRouter)), 0 ether);

// settle and refund
vm.prank(address(fakePoolManagerRouter));
snapStart("VaultTest#testSettleAndRefund_WithErc20Transfer");
vault.lock(hex"18");
snapEnd();

// verify
assertEq(IERC20(Currency.unwrap(currency0)).balanceOf(address(fakePoolManagerRouter)), 10 ether);
assertEq(IERC20(Currency.unwrap(currency1)).balanceOf(address(fakePoolManagerRouter)), 0 ether);
}

function testSettleAndRefund_WithoutErc20Transfer() public {
assertEq(IERC20(Currency.unwrap(currency0)).balanceOf(address(fakePoolManagerRouter)), 0 ether);
assertEq(IERC20(Currency.unwrap(currency1)).balanceOf(address(fakePoolManagerRouter)), 0 ether);

// settleAndRefund works even if there's no excess currency
vm.prank(address(fakePoolManagerRouter));
snapStart("VaultTest#testSettleAndRefund_WithoutErc20Transfer");
vault.lock(hex"18");
snapEnd();

// verify
assertEq(IERC20(Currency.unwrap(currency0)).balanceOf(address(fakePoolManagerRouter)), 0 ether);
assertEq(IERC20(Currency.unwrap(currency1)).balanceOf(address(fakePoolManagerRouter)), 0 ether);
}

function testNotCorrectPoolManager() public {
// router => vault.lock
// vault.lock => periphery.lockAcquired
Expand Down Expand Up @@ -208,7 +240,7 @@ contract VaultTest is Test, GasSnapshot {
vm.prank(address(fakePoolManagerRouter));
snapStart("VaultTest#lockSettledWhenAddLiquidity");
vault.lock(hex"02");
snapStart("end");
snapEnd();

assertEq(IERC20(Currency.unwrap(currency0)).balanceOf(address(vault)), 10 ether);
assertEq(IERC20(Currency.unwrap(currency1)).balanceOf(address(vault)), 10 ether);
Expand Down
28 changes: 27 additions & 1 deletion test/vault/VaultInvariant.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ contract VaultPoolManager is Test {
enum ActionType {
Take,
Settle,
SettleAndRefund,
SettleFor,
Mint,
Burn
Expand Down Expand Up @@ -84,6 +85,21 @@ contract VaultPoolManager is Test {
vault.lock(abi.encode(Action(ActionType.Settle, uint128(amt0), uint128(amt1))));
}

/// @dev In settleAndRefund case, assume user add liquidity and paying to the vault
/// but theres another folk who minted extra token to the vault
function settleAndRefund(uint256 amt0, uint256 amt1, bool sendToVault) public {
amt0 = bound(amt0, 0, MAX_TOKEN_BALANCE - 1 ether);
amt1 = bound(amt1, 0, MAX_TOKEN_BALANCE - 1 ether);

// someone send some token directly to vault
if (sendToVault) token0.mint(address(vault), 1 ether);

// mint token to VaultPoolManager, so VaultPoolManager can pay to the vault
token0.mint(address(this), amt0);
token1.mint(address(this), amt1);
vault.lock(abi.encode(Action(ActionType.SettleAndRefund, uint128(amt0), uint128(amt1))));
}

/// @dev In settleFor case, assume user is paying for hook
function settleFor(uint256 amt0, uint256 amt1) public {
amt0 = bound(amt0, 0, MAX_TOKEN_BALANCE);
Expand Down Expand Up @@ -165,6 +181,15 @@ contract VaultPoolManager is Test {

vault.settle(currency0);
vault.settle(currency1);
} else if (action.actionType == ActionType.SettleAndRefund) {
BalanceDelta delta = toBalanceDelta(int128(action.amt0), int128(action.amt1));
vault.accountPoolBalanceDelta(poolKey, delta, address(this));

token0.transfer(address(vault), action.amt0);
token1.transfer(address(vault), action.amt1);

vault.settleAndRefund(currency0);
vault.settleAndRefund(currency1);
} else if (action.actionType == ActionType.SettleFor) {
// hook cash out the fee ahead
BalanceDelta delta = toBalanceDelta(int128(action.amt0), int128(action.amt1));
Expand Down Expand Up @@ -212,13 +237,14 @@ contract VaultInvariant is Test, GasSnapshot {
// Only call vaultPoolManager, otherwise all other contracts deployed in setUp will be called
targetContract(address(vaultPoolManager));

bytes4[] memory selectors = new bytes4[](6);
bytes4[] memory selectors = new bytes4[](7);
selectors[0] = VaultPoolManager.take.selector;
selectors[1] = VaultPoolManager.mint.selector;
selectors[2] = VaultPoolManager.settle.selector;
selectors[3] = VaultPoolManager.burn.selector;
selectors[4] = VaultPoolManager.settleFor.selector;
selectors[5] = VaultPoolManager.collectFee.selector;
selectors[6] = VaultPoolManager.settleAndRefund.selector;
targetSelector(FuzzSelector({addr: address(vaultPoolManager), selectors: selectors}));
}

Expand Down
Loading