Skip to content

Commit

Permalink
refactor: make FEE_COLLECTOR address a constant
Browse files Browse the repository at this point in the history
  • Loading branch information
cucupac committed Jan 28, 2024
1 parent e3ef758 commit 1da24a5
Show file tree
Hide file tree
Showing 7 changed files with 176 additions and 114 deletions.
5 changes: 2 additions & 3 deletions src/Position.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,18 @@ import { IFeeCollector } from "src/interfaces/IFeeCollector.sol";
contract Position is DebtService, SwapService {
// Constants: no SLOAD to save gas
uint256 public constant PROTOCOL_FEE = 3;
address public constant FEE_COLLECTOR = 0x7A7AbDb9E12F3a9845E2433958Eef8FB9C8489Ee;

// Immutables: no SLOAD to save gas
address public immutable FEE_COLLECTOR;
address public immutable B_TOKEN;

// Events
event Short(uint256 cAmt, uint256 dAmt, uint256 bAmt);
event Close(uint256 gains);

constructor(address _owner, address _feeCollector, address _cToken, address _dToken, address _bToken)
constructor(address _owner, address _cToken, address _dToken, address _bToken)
DebtService(_owner, _cToken, _dToken)
{
FEE_COLLECTOR = _feeCollector;
B_TOKEN = _bToken;
}

Expand Down
8 changes: 2 additions & 6 deletions src/PositionFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ contract PositionFactory is Ownable {
// Constants: no SLOAD to save gas
address private constant CONTRACT_DEPLOYER = 0x0a5B347509621337cDDf44CBCf6B6E7C9C908CD2;

// Immutables: no SLOAD to save gas
address public immutable FEE_COLLECTOR;

// Factory Storage
/// @dev Mapping from owner to cToken to dToken to bToken to position
mapping(address => mapping(address => mapping(address => mapping(address => address)))) public positions;
Expand All @@ -26,9 +23,8 @@ contract PositionFactory is Ownable {
error Unauthorized();
error PositionExists();

constructor(address _owner, address _feeCollector) Ownable(_owner) {
constructor(address _owner) Ownable(_owner) {
if (msg.sender != CONTRACT_DEPLOYER) revert Unauthorized();
FEE_COLLECTOR = _feeCollector;
}

/**
Expand All @@ -44,7 +40,7 @@ contract PositionFactory is Ownable {
{
if (positions[msg.sender][_cToken][_dToken][_bToken] != address(0)) revert PositionExists();

position = address(new Position(msg.sender, FEE_COLLECTOR, _cToken, _dToken, _bToken));
position = address(new Position(msg.sender, _cToken, _dToken, _bToken));

positionsLookup[msg.sender].push(position);
positions[msg.sender][_cToken][_dToken][_bToken] = position;
Expand Down
248 changes: 164 additions & 84 deletions test/FeeCollector.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
Assets,
AAVE_ORACLE,
CONTRACT_DEPLOYER,
FEE_COLLECTOR,
TEST_CLIENT,
PROTOCOL_FEE,
CLIENT_RATE,
Expand All @@ -19,6 +20,7 @@ import {
WBTC
} from "test/common/Constants.t.sol";
import { TokenUtils } from "test/common/utils/TokenUtils.t.sol";
import { IFeeCollector } from "src/interfaces/IFeeCollector.sol";
import { IAaveOracle } from "src/interfaces/aave/IAaveOracle.sol";
import { IPosition } from "src/interfaces/IPosition.sol";
import { IERC20 } from "src/interfaces/token/IERC20.sol";
Expand All @@ -44,8 +46,8 @@ contract FeeCollectorTest is Test, TokenUtils {

// Test Storage
uint256 public mainnetFork;
address public feeCollectorAddr;
address public positionOwner = address(this);
address public feeCollectorAddr;

// Events
event Short(uint256 cAmt, uint256 dAmt, uint256 bAmt);
Expand All @@ -66,7 +68,7 @@ contract FeeCollectorTest is Test, TokenUtils {

// Deploy PositionFactory
vm.prank(CONTRACT_DEPLOYER);
positionFactory = new PositionFactory(CONTRACT_DEPLOYER, feeCollectorAddr);
positionFactory = new PositionFactory(CONTRACT_DEPLOYER);

// Set client rate
vm.prank(CONTRACT_DEPLOYER);
Expand Down Expand Up @@ -179,88 +181,6 @@ contract FeeCollectorTest is Test, TokenUtils {
}
}

/// @dev
// - The FeeCollector's cToken balance should increase by protocolFee
// - The cToken totalClientBalances should increase by clientFee
// - The client's cToken balance on the FeeCollector contract should increase by clientFee
function testFuzz_CollectFeesWithClientIntegrated(uint256 _cAmt) external payable {
for (uint256 i; i < positions.length; i++) {
// Test Variables
address positionAddr = positions[i].addr;
address cToken = positions[i].cToken;

// Bound fuzzed variables
_cAmt = bound(_cAmt, assets.minCAmts(cToken), assets.maxCAmts(cToken));

// Expectations
uint256 protocolFee = (_cAmt * PROTOCOL_FEE) / 1000;
uint256 clientFee = (protocolFee * CLIENT_RATE) / 100;

// Fund positionOwner with _cAmt of cToken
_fund(positionOwner, cToken, _cAmt);

// Approve Position contract to spend collateral
IERC20(cToken).approve(positionAddr, _cAmt);

// Pre-act balances
uint256 preContractBalance = IERC20(cToken).balanceOf(feeCollectorAddr);
uint256 preTotalClientBalances = feeCollector.totalClientBalances(cToken);
uint256 preClientFeeBalance = feeCollector.balances(TEST_CLIENT, cToken);

// Act: increase short position
IPosition(positionAddr).short(_cAmt, 50, 0, 3000, TEST_CLIENT);

// Post-act balances
uint256 postContractBalance = IERC20(cToken).balanceOf(feeCollectorAddr);
uint256 postTotalClientBalances = feeCollector.totalClientBalances(cToken);
uint256 postClientFeeBalance = feeCollector.balances(TEST_CLIENT, cToken);

// Assertions
assertEq(postContractBalance, preContractBalance + protocolFee);
assertEq(postTotalClientBalances, preTotalClientBalances + clientFee);
assertEq(postClientFeeBalance, preClientFeeBalance + clientFee);
}
}

/// @dev
// - The FeeCollector's cToken balance should increase by protocolFee
// - The cToken totalClientBalances should not change
// - The above should be true when _client is sent as address(0)
function testFuzz_CollectFeesNoClientIntegrated(uint256 _cAmt) external payable {
for (uint256 i; i < positions.length; i++) {
// Test Variables
address positionAddr = positions[i].addr;
address cToken = positions[i].cToken;

// Bound fuzzed variables
_cAmt = bound(_cAmt, assets.minCAmts(cToken), assets.maxCAmts(cToken));

// Expectations
uint256 protocolFee = (_cAmt * PROTOCOL_FEE) / 1000;

// Fund positionOwner with _cAmt of cToken
_fund(positionOwner, cToken, _cAmt);

// Approve Position contract to spend collateral
IERC20(cToken).approve(positionAddr, _cAmt);

// Pre-act balances
uint256 preContractBalance = IERC20(cToken).balanceOf(feeCollectorAddr);
uint256 preTotalClientBalances = feeCollector.totalClientBalances(cToken);

// Act: increase short position
IPosition(positionAddr).short(_cAmt, 50, 0, 3000, address(0));

// Post-act balances
uint256 postContractBalance = IERC20(cToken).balanceOf(feeCollectorAddr);
uint256 postTotalClientBalances = feeCollector.totalClientBalances(cToken);

// Assertions
assertEq(postContractBalance, preContractBalance + protocolFee);
assertEq(postTotalClientBalances, preTotalClientBalances);
}
}

/// @dev
// - The FeeCollector's feeToken balance should decrease by amount withdrawn
// - The feeToken totalClientBalances should decrease by amount withdrawn
Expand Down Expand Up @@ -514,3 +434,163 @@ contract FeeCollectorTest is Test, TokenUtils {
assertEq(postContractBalance, preContractBalance + _amount);
}
}

contract FeeCollectorIntegrationTest is Test, TokenUtils {
/* solhint-disable func-name-mixedcase */

struct TestPosition {
address addr;
address cToken;
address dToken;
address bToken;
}

// Errors
error OwnableUnauthorizedAccount(address account);

// Test contracts
PositionFactory public positionFactory;
Assets public assets;
TestPosition[] public positions;

// Test Storage
uint256 public mainnetFork;
address public positionOwner = address(this);

// Events
event Short(uint256 cAmt, uint256 dAmt, uint256 bAmt);

function setUp() public {
// Setup: use mainnet fork
mainnetFork = vm.createFork(vm.envString("RPC_URL"));
vm.selectFork(mainnetFork);

// Deploy assets
assets = new Assets();
address[4] memory supportedAssets = assets.getSupported();

// Deploy FeeCollector
vm.prank(CONTRACT_DEPLOYER);
deployCodeTo("FeeCollector.sol", abi.encode(CONTRACT_DEPLOYER), FEE_COLLECTOR);

// Deploy PositionFactory
vm.prank(CONTRACT_DEPLOYER);
positionFactory = new PositionFactory(CONTRACT_DEPLOYER);

// Set client rate
vm.prank(CONTRACT_DEPLOYER);
IFeeCollector(FEE_COLLECTOR).setClientRate(CLIENT_RATE);

// Deploy and store four position contracts - one for each supported asset as collateral
address positionAddr;
TestPosition memory newPosition;
for (uint256 i; i < supportedAssets.length; i++) {
if (supportedAssets[i] != WETH) {
positionAddr = positionFactory.createPosition(supportedAssets[i], WETH, WBTC);
newPosition =
TestPosition({ addr: positionAddr, cToken: supportedAssets[i], dToken: WETH, bToken: WBTC });
positions.push(newPosition);
}
}
positionAddr = positionFactory.createPosition(WETH, USDC, WETH);
newPosition = TestPosition({ addr: positionAddr, cToken: WETH, dToken: USDC, bToken: WETH });
positions.push(newPosition);

// Mock AaveOracle
for (uint256 i; i < supportedAssets.length; i++) {
vm.mockCall(
AAVE_ORACLE,
abi.encodeWithSelector(IAaveOracle(AAVE_ORACLE).getAssetPrice.selector, supportedAssets[i]),
abi.encode(assets.prices(supportedAssets[i]))
);
}
}

/// @dev
// - The active fork should be the forked network created in the setup
function test_ActiveFork() public {
assertEq(vm.activeFork(), mainnetFork, "vm.activeFork() != mainnetFork");
}

/// @dev
// - The FeeCollector's cToken balance should increase by protocolFee
// - The cToken totalClientBalances should increase by clientFee
// - The client's cToken balance on the FeeCollector contract should increase by clientFee
function testFuzz_CollectFeesWithClientIntegrated(uint256 _cAmt) external payable {
for (uint256 i; i < positions.length; i++) {
// Test Variables
address positionAddr = positions[i].addr;
address cToken = positions[i].cToken;

// Bound fuzzed variables
_cAmt = bound(_cAmt, assets.minCAmts(cToken), assets.maxCAmts(cToken));

// Expectations
uint256 protocolFee = (_cAmt * PROTOCOL_FEE) / 1000;
uint256 clientFee = (protocolFee * CLIENT_RATE) / 100;

// Fund positionOwner with _cAmt of cToken
_fund(positionOwner, cToken, _cAmt);

// Approve Position contract to spend collateral
IERC20(cToken).approve(positionAddr, _cAmt);

// Pre-act balances
uint256 preContractBalance = IERC20(cToken).balanceOf(FEE_COLLECTOR);
uint256 preTotalClientBalances = IFeeCollector(FEE_COLLECTOR).totalClientBalances(cToken);
uint256 preClientFeeBalance = IFeeCollector(FEE_COLLECTOR).balances(TEST_CLIENT, cToken);

// Act: increase short position
IPosition(positionAddr).short(_cAmt, 50, 0, 3000, TEST_CLIENT);

// Post-act balances
uint256 postContractBalance = IERC20(cToken).balanceOf(FEE_COLLECTOR);
uint256 postTotalClientBalances = IFeeCollector(FEE_COLLECTOR).totalClientBalances(cToken);
uint256 postClientFeeBalance = IFeeCollector(FEE_COLLECTOR).balances(TEST_CLIENT, cToken);

// Assertions
assertEq(postContractBalance, preContractBalance + protocolFee);
assertEq(postTotalClientBalances, preTotalClientBalances + clientFee);
assertEq(postClientFeeBalance, preClientFeeBalance + clientFee);
}
}

/// @dev
// - The FeeCollector's cToken balance should increase by protocolFee
// - The cToken totalClientBalances should not change
// - The above should be true when _client is sent as address(0)
function testFuzz_CollectFeesNoClientIntegrated(uint256 _cAmt) external payable {
for (uint256 i; i < positions.length; i++) {
// Test Variables
address positionAddr = positions[i].addr;
address cToken = positions[i].cToken;

// Bound fuzzed variables
_cAmt = bound(_cAmt, assets.minCAmts(cToken), assets.maxCAmts(cToken));

// Expectations
uint256 protocolFee = (_cAmt * PROTOCOL_FEE) / 1000;

// Fund positionOwner with _cAmt of cToken
_fund(positionOwner, cToken, _cAmt);

// Approve Position contract to spend collateral
IERC20(cToken).approve(positionAddr, _cAmt);

// Pre-act balances
uint256 preContractBalance = IERC20(cToken).balanceOf(FEE_COLLECTOR);
uint256 preTotalClientBalances = IFeeCollector(FEE_COLLECTOR).totalClientBalances(cToken);

// Act: increase short position
IPosition(positionAddr).short(_cAmt, 50, 0, 3000, address(0));

// Post-act balances
uint256 postContractBalance = IERC20(cToken).balanceOf(FEE_COLLECTOR);
uint256 postTotalClientBalances = IFeeCollector(FEE_COLLECTOR).totalClientBalances(cToken);

// Assertions
assertEq(postContractBalance, preContractBalance + protocolFee);
assertEq(postTotalClientBalances, preTotalClientBalances);
}
}
}
12 changes: 5 additions & 7 deletions test/Position.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ import { Test } from "forge-std/Test.sol";
import { VmSafe } from "forge-std/Vm.sol";

// Local Imports
import { FeeCollector } from "src/FeeCollector.sol";
import { PositionFactory } from "src/PositionFactory.sol";
import { PositionAdmin } from "src/PositionAdmin.sol";
import {
Assets,
AAVE_ORACLE,
CONTRACT_DEPLOYER,
DAI,
FEE_COLLECTOR,
PROFIT_PERCENT,
REPAY_PERCENT,
SWAP_ROUTER,
Expand Down Expand Up @@ -58,7 +58,6 @@ contract PositionTest is Test, TokenUtils, DebtUtils {

// Test contracts
PositionFactory public positionFactory;
FeeCollector public feeCollector;
Assets public assets;
TestPosition[] public positions;

Expand All @@ -81,11 +80,11 @@ contract PositionTest is Test, TokenUtils, DebtUtils {

// Deploy FeeCollector
vm.prank(CONTRACT_DEPLOYER);
feeCollector = new FeeCollector(CONTRACT_DEPLOYER);
deployCodeTo("FeeCollector.sol", abi.encode(CONTRACT_DEPLOYER), FEE_COLLECTOR);

// Deploy PositionFactory
vm.prank(CONTRACT_DEPLOYER);
positionFactory = new PositionFactory(CONTRACT_DEPLOYER, address(feeCollector));
positionFactory = new PositionFactory(CONTRACT_DEPLOYER);

// Deploy and store all possible positions
for (uint256 i; i < supportedAssets.length; i++) {
Expand Down Expand Up @@ -560,7 +559,6 @@ contract PositionPermitTest is Test, TokenUtils, DebtUtils {

// Test contracts
PositionFactory public positionFactory;
FeeCollector public feeCollector;
Assets public assets;
TestPosition[] public positions;

Expand All @@ -584,11 +582,11 @@ contract PositionPermitTest is Test, TokenUtils, DebtUtils {

// Deploy FeeCollector
vm.prank(CONTRACT_DEPLOYER);
feeCollector = new FeeCollector(CONTRACT_DEPLOYER);
deployCodeTo("FeeCollector.sol", abi.encode(CONTRACT_DEPLOYER), FEE_COLLECTOR);

// Deploy PositionFactory
vm.prank(CONTRACT_DEPLOYER);
positionFactory = new PositionFactory(CONTRACT_DEPLOYER, address(feeCollector));
positionFactory = new PositionFactory(CONTRACT_DEPLOYER);

// Set contract owner
wallet = vm.createWallet(uint256(keccak256(abi.encodePacked(uint256(1)))));
Expand Down
Loading

0 comments on commit 1da24a5

Please sign in to comment.