Skip to content

Commit

Permalink
refactor: rename close to reduce
Browse files Browse the repository at this point in the history
  • Loading branch information
@0xernesto
0xernesto committed Apr 9, 2024
1 parent 55d35e7 commit fdb3229
Show file tree
Hide file tree
Showing 10 changed files with 1,286 additions and 57 deletions.
4 changes: 0 additions & 4 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,7 @@ Tests:

Considerations:

- [ ] Consider moving the protocol fee rate to the fee collector contract.

Cleanup:

- [ ] Change `close()` to `reduce()` in `Position.sol`
- [ ] Change all relevant test names related from `close` to `reduce`
- [ ] Ensure terminology and variable references are consistent across all comments
- [ ] Ensure full NatSpec comliance
8 changes: 4 additions & 4 deletions src/Position.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ contract Position is DebtService, SwapService, IPosition {
/// @param bAmt The amount of base token received and subsequently supplied as collateral (units: B_DECIMALS).
event AddLeverage(uint256 dAmt, uint256 bAmt);

/// @notice An event emitted when a position is closed.
/// @notice An event emitted when a position is reduced.
/// @param gains The amount of base token gained from the position (units: B_DECIMALS).
event Close(uint256 gains);
event Reduce(uint256 gains);

/// @notice This function is called when a Position contract is deployed.
/// @param _owner The account address of the Position contract's owner.
Expand Down Expand Up @@ -113,7 +113,7 @@ contract Position is DebtService, SwapService, IPosition {
}

/// @inheritdoc IPosition
function close(
function reduce(
uint24 _poolFee,
bool _exactOutput,
uint256 _swapAmtOutMin,
Expand Down Expand Up @@ -150,6 +150,6 @@ contract Position is DebtService, SwapService, IPosition {
SafeTransferLib.safeTransfer(ERC20(B_TOKEN), OWNER, gains);
}

emit Close(gains);
emit Reduce(gains);

Check warning on line 153 in src/Position.sol

View check run for this annotation

Codecov / codecov/patch

src/Position.sol#L153 

Added line #L153 was not covered by tests
}
}
2 changes: 1 addition & 1 deletion src/interfaces/IPosition.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ interface IPosition is IDebtService {
/// @param _swapAmtOutMin The min amount of output tokens from swap (supply 0 if _exactOutput = true).
/// @param _withdrawCAmt The amount of C_TOKEN to withdraw (units: C_DECIMALS).
/// @param _withdrawBAmt The amount of B_TOKEN to withdraw (units: B_DECIMALS).
function close(
function reduce(
uint24 _poolFee,
bool _exactOutput,
uint256 _swapAmtOutMin,
Expand Down
4 changes: 2 additions & 2 deletions test/Position.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ contract PositionTest is Test, TokenUtils, DebtUtils {

/// @dev
// - It should revert with Unauthorized() error when called by an unauthorized sender.
function testFuzz_CannotClose(address _sender) public {
function testFuzz_CannotReduce(address _sender) public {
// Assumptions
vm.assume(_sender != owner);

Expand All @@ -224,7 +224,7 @@ contract PositionTest is Test, TokenUtils, DebtUtils {
// Act
vm.prank(_sender);
vm.expectRevert(AdminService.Unauthorized.selector);
IPosition(addr).close(TEST_POOL_FEE, false, 0, cAmt, dAmt);
IPosition(addr).reduce(TEST_POOL_FEE, false, 0, cAmt, dAmt);

// Revert to snapshot
vm.revertTo(id);
Expand Down
40 changes: 20 additions & 20 deletions test/integration/Position.close.gains.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import { IAaveOracle } from "src/interfaces/aave/IAaveOracle.sol";
import { IPosition } from "src/interfaces/IPosition.sol";
import { IERC20 } from "src/interfaces/token/IERC20.sol";

contract PositionCloseGainsTest is Test, TokenUtils, DebtUtils {
contract PositionReduceGainsTest is Test, TokenUtils, DebtUtils {
/* solhint-disable func-name-mixedcase */

struct TestPosition {
Expand Down Expand Up @@ -120,7 +120,7 @@ contract PositionCloseGainsTest is Test, TokenUtils, DebtUtils {
// - Owner's cToken balance should increase by the amount of collateral withdrawn.
// - Owner's bToken balance should increase by the position's gains amount.
// - The above should be true for all supported tokens.
function test_CloseExactOutputDiffCAndB() public {
function test_ReduceExactOutputDiffCAndB() public {
// Setup
ContractBalances memory contractBalances;
OwnerBalances memory ownerBalances;
Expand Down Expand Up @@ -165,7 +165,7 @@ contract PositionCloseGainsTest is Test, TokenUtils, DebtUtils {
/// @dev start event recorder
vm.recordLogs();
/// @dev since profitable, withdrawCAmt is max int and withdrawBAmt is its (base) AToken balance
IPosition(p.addr).close(TEST_POOL_FEE, true, 0, type(uint256).max, contractBalances.preBAToken);
IPosition(p.addr).reduce(TEST_POOL_FEE, true, 0, type(uint256).max, contractBalances.preBAToken);
VmSafe.Log[] memory entries = vm.getRecordedLogs();

// Get post-act balances
Expand All @@ -176,11 +176,11 @@ contract PositionCloseGainsTest is Test, TokenUtils, DebtUtils {
ownerBalances.postBToken = IERC20(p.bToken).balanceOf(owner);
ownerBalances.postCToken = IERC20(p.cToken).balanceOf(owner);

bytes memory closeEvent = entries[entries.length - 1].data;
bytes memory reduceEvent = entries[entries.length - 1].data;
uint256 gains;

assembly {
gains := mload(add(closeEvent, 0x20))
gains := mload(add(reduceEvent, 0x20))
}

// Assertions:
Expand Down Expand Up @@ -209,7 +209,7 @@ contract PositionCloseGainsTest is Test, TokenUtils, DebtUtils {
// - Owner's cToken balance should increase by (collateral withdrawn + position's gains).
// - Owner's bToken balance should equal its cToken balance.
// - The above should be true for all supported tokens.
function test_CloseExactOutputSameCAndB() public {
function test_ReduceExactOutputSameCAndB() public {
// Setup
ContractBalances memory contractBalances;
OwnerBalances memory ownerBalances;
Expand Down Expand Up @@ -262,7 +262,7 @@ contract PositionCloseGainsTest is Test, TokenUtils, DebtUtils {

// Act
/// @dev since profitable, withdrawCAmt is max int and withdrawBAmt is its (base) AToken balance
IPosition(p.addr).close(TEST_POOL_FEE, true, 0, type(uint256).max, suppliedBAmt);
IPosition(p.addr).reduce(TEST_POOL_FEE, true, 0, type(uint256).max, suppliedBAmt);
entries = vm.getRecordedLogs();

// Get post-act balances
Expand All @@ -273,11 +273,11 @@ contract PositionCloseGainsTest is Test, TokenUtils, DebtUtils {
ownerBalances.postBToken = IERC20(p.bToken).balanceOf(owner);
ownerBalances.postCToken = IERC20(p.cToken).balanceOf(owner);

bytes memory closeEvent = entries[entries.length - 1].data;
bytes memory reduceEvent = entries[entries.length - 1].data;
uint256 gains;

assembly {
gains := mload(add(closeEvent, 0x20))
gains := mload(add(reduceEvent, 0x20))
}

// Assertions:
Expand All @@ -303,7 +303,7 @@ contract PositionCloseGainsTest is Test, TokenUtils, DebtUtils {
}
}

/// @dev: Simulates close where all B_TOKEN is withdrawn and swapped for D_TOKEN,
/// @dev: Simulates reduction where all B_TOKEN is withdrawn and swapped for D_TOKEN,
// where D_TOKEN amount is greater than total debt.
// - Position contract's (bToken) AToken balance should go to 0 (full withdraw).
// - Position contract's (cToken) AToken balance should go to 0 (full withdraw).
Expand All @@ -314,7 +314,7 @@ contract PositionCloseGainsTest is Test, TokenUtils, DebtUtils {
// - Owner's cToken balance should increase by the amount of collateral withdrawn.
// - Owner's bToken balance should stay the same, as gains will be in debt token if exactInput is called.
// - The above should be true for all supported tokens.
function testFuzz_CloseFullExactInputDiffCAndB(uint256 _dAmtRemainder) public {
function testFuzz_ReduceFullExactInputDiffCAndB(uint256 _dAmtRemainder) public {
// Setup
ContractBalances memory contractBalances;
OwnerBalances memory ownerBalances;
Expand Down Expand Up @@ -366,7 +366,7 @@ contract PositionCloseGainsTest is Test, TokenUtils, DebtUtils {
// Act
/// @dev start event recorder
vm.recordLogs();
IPosition(p.addr).close(TEST_POOL_FEE, false, 0, type(uint256).max, contractBalances.preBAToken);
IPosition(p.addr).reduce(TEST_POOL_FEE, false, 0, type(uint256).max, contractBalances.preBAToken);
VmSafe.Log[] memory entries = vm.getRecordedLogs();

// Get post-act balances
Expand All @@ -378,11 +378,11 @@ contract PositionCloseGainsTest is Test, TokenUtils, DebtUtils {
ownerBalances.postBToken = IERC20(p.bToken).balanceOf(owner);
ownerBalances.postCToken = IERC20(p.cToken).balanceOf(owner);

bytes memory closeEvent = entries[entries.length - 1].data;
bytes memory reduceEvent = entries[entries.length - 1].data;
uint256 gains;

assembly {
gains := mload(add(closeEvent, 0x20))
gains := mload(add(reduceEvent, 0x20))
}

// Assertions:
Expand All @@ -401,11 +401,11 @@ contract PositionCloseGainsTest is Test, TokenUtils, DebtUtils {
}
}

/// @dev Tests that close function works when the position has gains and collateral token and base token are the same.
/// @dev Tests that reduce function works when the position has gains and collateral token and base token are the same.
/// @notice Test strategy:
// - 1. Open a position. In doing so, extract the amount of base token added to Aave.
// - 2. Mock Uniswap to ensure position gains.
// - 3. Close the position, such that all B_TOKEN is withdrawn and all C_TOKEN is withdrawn.
// - 3. Reduce the position, such that all B_TOKEN is withdrawn and all C_TOKEN is withdrawn.

/// @notice Assertions:
// - Position contract's (bToken) AToken balance should go to 0 (full withdraw).
Expand All @@ -416,7 +416,7 @@ contract PositionCloseGainsTest is Test, TokenUtils, DebtUtils {
// - Owner's cToken balance should increase by the amount of collateral withdrawn.
// - Owner's bToken balance should increase by the amount of collateral withdrawn.
// - The above should be true for all supported tokens.
function testFuzz_CloseExactInputSameCAndB(uint256 _dAmtRemainder) public {
function testFuzz_ReduceExactInputSameCAndB(uint256 _dAmtRemainder) public {
// Setup
ContractBalances memory contractBalances;
OwnerBalances memory ownerBalances;
Expand Down Expand Up @@ -474,7 +474,7 @@ contract PositionCloseGainsTest is Test, TokenUtils, DebtUtils {
vm.etch(SWAP_ROUTER, code);

// Act
IPosition(p.addr).close(TEST_POOL_FEE, false, 0, type(uint256).max, suppliedBAmt);
IPosition(p.addr).reduce(TEST_POOL_FEE, false, 0, type(uint256).max, suppliedBAmt);
entries = vm.getRecordedLogs();

// Get post-act balances
Expand All @@ -486,11 +486,11 @@ contract PositionCloseGainsTest is Test, TokenUtils, DebtUtils {
ownerBalances.postBToken = IERC20(p.bToken).balanceOf(owner);
ownerBalances.postCToken = IERC20(p.cToken).balanceOf(owner);

bytes memory closeEvent = entries[entries.length - 1].data;
bytes memory reduceEvent = entries[entries.length - 1].data;
uint256 gains;

assembly {
gains := mload(add(closeEvent, 0x20))
gains := mload(add(reduceEvent, 0x20))
}

// Assertions:
Expand Down
30 changes: 15 additions & 15 deletions test/integration/Position.close.losses.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import { IAaveOracle } from "src/interfaces/aave/IAaveOracle.sol";
import { IPosition } from "src/interfaces/IPosition.sol";
import { IERC20 } from "src/interfaces/token/IERC20.sol";

contract PositionCloseLossesTest is Test, TokenUtils, DebtUtils {
contract PositionReduceLossesTest is Test, TokenUtils, DebtUtils {
/* solhint-disable func-name-mixedcase */

struct TestPosition {
Expand Down Expand Up @@ -116,24 +116,24 @@ contract PositionCloseLossesTest is Test, TokenUtils, DebtUtils {
}
}

/// @dev Tests that close function works when the position has losses and collateral token and base token are different.
/// @dev Tests that reduce function works when the position has losses and collateral token and base token are different.
/// @notice Test strategy:
// - 1. Open a position.
// - 2. Mock Uniswap to ensure position losses.
// - 3. Using screenshot, obtain max withdrawable collateral amount after withdrawing all B_TOKEN and partially repaying debt.
// - 4. Close the position.
// - 4. Reduce the position.

/// @notice assertions.
// - Position contract's (bToken) AToken balance should go to 0 (full withdraw).
// - Position contract's (cToken) AToken balance should decrease by the amount withdrawn.
// - Position contract's dToken balance should be 0; no gains, so all was used for swap.
// - Position contract's debt on Aave should decrease by repayment (amount received from the swap).
// - Position contract's debt should be greater than 0 after close.
// - Position contract's debt should be greater than 0 after reduction.
// - Owner's cToken balance should increase by the amount of collateral withdrawn.
// - Owner's bToken balance should not increase; no gains.
// - Position contract's gains should be 0.
// - The above should be true for all supported tokens.
function test_CloseExactInputDiffCAndB(uint256 withdrawCAmt) public {
function test_ReduceExactInputDiffCAndB(uint256 withdrawCAmt) public {
// Setup
ContractBalances memory contractBalances;
OwnerBalances memory ownerBalances;
Expand Down Expand Up @@ -193,7 +193,7 @@ contract PositionCloseLossesTest is Test, TokenUtils, DebtUtils {
// Act
/// @dev start event recorder
vm.recordLogs();
IPosition(p.addr).close(TEST_POOL_FEE, false, 0, withdrawCAmt, contractBalances.preBAToken);
IPosition(p.addr).reduce(TEST_POOL_FEE, false, 0, withdrawCAmt, contractBalances.preBAToken);
VmSafe.Log[] memory entries = vm.getRecordedLogs();

// Get post-act balances
Expand All @@ -205,11 +205,11 @@ contract PositionCloseLossesTest is Test, TokenUtils, DebtUtils {
ownerBalances.postBToken = IERC20(p.bToken).balanceOf(owner);
ownerBalances.postCToken = IERC20(p.cToken).balanceOf(owner);

bytes memory closeEvent = entries[entries.length - 1].data;
bytes memory reduceEvent = entries[entries.length - 1].data;
uint256 gains;

assembly {
gains := mload(add(closeEvent, 0x20))
gains := mload(add(reduceEvent, 0x20))
}

// Assertions
Expand All @@ -229,24 +229,24 @@ contract PositionCloseLossesTest is Test, TokenUtils, DebtUtils {
}
}

/// @dev Tests that close function works when the position has losses and collateral token and base token are the same.
/// @dev Tests that reduce function works when the position has losses and collateral token and base token are the same.
/// @notice Test strategy:
// - 1. Open a position.
// - 2. Mock Uniswap to ensure position losses.
// - 3. Using screenshot, obtain max withdrawable collateral amount after withdrawing all B_TOKEN and partially repaying debt.
// - 4. Close the position.
// - 4. Reduce the position.

/// @notice Assertions:
// - Position contract's (bToken) AToken balance should decrease by the amount withdrawn.
// - Position contract's (cToken) AToken balance should decrease by the amount withdrawn.
// - Position contract's dToken balance should be 0; no gains, so all was used for swap.
// - Position contract's debt on Aave should decrease by repayment (amount received from the swap).
// - Position contract's debt should be greater than 0 after close.
// - Position contract's debt should be greater than 0 after reduction.
// - Position contract's gains should be 0.
// - Owner's cToken balance should increase by the amount of collateral withdrawn.
// - Owner's bToken balance should increase by the amount of collateral withdrawn.
// - The above should be true for all supported tokens.
function test_CloseExactInputSameCAndB(uint256 withdrawCAmt) public {
function test_ReduceExactInputSameCAndB(uint256 withdrawCAmt) public {
// Setup
ContractBalances memory contractBalances;
OwnerBalances memory ownerBalances;
Expand Down Expand Up @@ -315,7 +315,7 @@ contract PositionCloseLossesTest is Test, TokenUtils, DebtUtils {
withdrawCAmt = bound(withdrawCAmt, 1, maxCTokenWithdrawal);

// Act
IPosition(p.addr).close(TEST_POOL_FEE, false, 0, withdrawCAmt, suppliedBAmt);
IPosition(p.addr).reduce(TEST_POOL_FEE, false, 0, withdrawCAmt, suppliedBAmt);
entries = vm.getRecordedLogs();

// Get post-act balances
Expand All @@ -327,11 +327,11 @@ contract PositionCloseLossesTest is Test, TokenUtils, DebtUtils {
ownerBalances.postBToken = IERC20(p.bToken).balanceOf(owner);
ownerBalances.postCToken = IERC20(p.cToken).balanceOf(owner);

bytes memory closeEvent = entries[entries.length - 1].data;
bytes memory reduceEvent = entries[entries.length - 1].data;
uint256 gains;

assembly {
gains := mload(add(closeEvent, 0x20))
gains := mload(add(reduceEvent, 0x20))
}

// Assertions
Expand Down
Loading

0 comments on commit fdb3229

Please sign in to comment.