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

Code cleanup #17

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion .husky/pre-commit
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env sh
. "$(dirname -- "$0")/_/husky.sh"

npm run format && npm run lint:check && npm run test
npm run format && npm run lint:check && forge test --fork-url "https://lb.drpc.org/ogrpc?network=arbitrum&dkey=AndBdVMW9kqLhpV95Kl_zsdLWrYu3PsR7qO5GtyyLTIM"
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,21 @@ The following outlines principles for **core** protocol funcitonality.

1. Immutable.
2. No Governance on the core protocol.
3. No Admin Keys.
3. No Admin Keys on the core protocol.

## To-Do

Logic:

- [ ] Add gas optimizations where possible.
- None at the moment🙂

Tests:

- None at the moment🙂

Considerations:

- None at the moment🙂
- [ ] Consider moving the protocol fee rate to the fee collector contract.

Cleanup:

Expand Down
83 changes: 31 additions & 52 deletions src/FeeCollector.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,35 +4,45 @@ pragma solidity ^0.8.21;
// Local Imports
import { Ownable } from "src/dependencies/access/Ownable.sol";
import { SafeTransferLib, ERC20 } from "solmate/utils/SafeTransferLib.sol";
import { IFeeCollector } from "src/interfaces/IFeeCollector.sol";
import { IERC20 } from "src/interfaces/token/IERC20.sol";

/// @title FeeCollector
/// @title The fee collector contract
/// @author Chain Rule, LLC
/// @notice Collects protocol fees
contract FeeCollector is Ownable {
/// @notice Collects all protocol fees
contract FeeCollector is Ownable, IFeeCollector {
// Constants: no SLOAD to save gas
address private constant CONTRACT_DEPLOYER = 0x0a5B347509621337cDDf44CBCf6B6E7C9C908CD2;

// Storage
/// @inheritdoc IFeeCollector
uint256 public clientRate;

/// @inheritdoc IFeeCollector
mapping(address => uint256) public clientTakeRates;

/// @inheritdoc IFeeCollector
mapping(address => uint256) public totalClientBalances;

/// @inheritdoc IFeeCollector
mapping(address => mapping(address => uint256)) public balances;

/// @inheritdoc IFeeCollector
uint256 public feeRate;

// Errors
error Unauthorized();
error OutOfRange();

constructor(address _owner) Ownable(_owner) {
/// @notice This function is called when the FeeCollector is deployed.
/// @param _owner The account address of the FeeCollector contract's owner.
/// @param _feeRate The protocol fee rate.
constructor(address _owner, uint256 _feeRate) Ownable(_owner) {
if (msg.sender != CONTRACT_DEPLOYER) revert Unauthorized();
feeRate = _feeRate;
}

/**
* @notice Collects fees from Position contracts when collateral is added.
* @param _client The address where a client operator will receive protocols fees.
* @param _token The token to collect fees in (the collateral token of the calling Position contract).
* @param _amt The total amount of fees to collect.
*/
/// @inheritdoc IFeeCollector
function collectFees(address _client, address _token, uint256 _amt, uint256 _clientFee) external payable {
// 1. Transfer tokens to this contract
SafeTransferLib.safeTransferFrom(ERC20(_token), msg.sender, address(this), _amt);
Expand All @@ -47,10 +57,7 @@ contract FeeCollector is Ownable {
}
}

/**
* @notice Withdraw collected fees from this contract.
* @param _token The token address to withdraw.
*/
/// @inheritdoc IFeeCollector
function clientWithdraw(address _token) public payable {
uint256 withdrawAmt = balances[msg.sender][_token];

Expand All @@ -62,28 +69,13 @@ contract FeeCollector is Ownable {
SafeTransferLib.safeTransfer(ERC20(_token), msg.sender, withdrawAmt);
}

/**
* @notice Allows clients to set the percentage of the clientRate they will receive each revenue-generating tx.
* Amounts less than 100 will give the calling client's users a protocol fee discount:
* clientPercentOfProtocolFee = clientRate * _clientTakeRate
* userPercentOfProtocolFee = clientRate * (1 - _clientTakeRate)
* clientFee = protocolFee * clientPercentOfProtocolFee
* userSavings = protocolFee * userPercentOfProtocolFee
* @param _clientTakeRate The percentage of the clientRate the client will receive each revenue-generating tx (100 = 100%).
*/
/// @inheritdoc IFeeCollector
function setClientTakeRate(uint256 _clientTakeRate) public payable {
if (_clientTakeRate > 100) revert OutOfRange();
clientTakeRates[msg.sender] = _clientTakeRate;
}

/**
* @notice Returns the amount discounted from the protocol fee for using the provided client,
* and the amount of fees the client will receive.
* @param _client The address where a client operator will receive protocols fees.
* @param _maxFee The maximum amount of fees the protocol will collect.
* @return userSavings The amount of fees discounted from the protocol fee.
* @return clientFee The amount of fees the client will receive.
*/
/// @inheritdoc IFeeCollector
function getClientAllocations(address _client, uint256 _maxFee)
public
view
Expand All @@ -105,40 +97,27 @@ contract FeeCollector is Ownable {
**
******************************************************************************/

/**
* @notice Allows owner to set client rate.
* @param _clientRate The percentage of total transaction-specific protocol fee, allocated to the utilized client.
*/
function setFeeRate(uint256 _feeRate) public payable onlyOwner {
if (_feeRate > 1000) revert OutOfRange();
feeRate = _feeRate;
}

/// @inheritdoc IFeeCollector
function setClientRate(uint256 _clientRate) public payable onlyOwner {
if (_clientRate < 30 || _clientRate > 100) revert OutOfRange();

clientRate = _clientRate;
}

/**
* @notice Allows OWNER to withdraw all of this contract's native token balance.
*/
/// @inheritdoc IFeeCollector
function extractNative() public payable onlyOwner {
payable(msg.sender).transfer(address(this).balance);
}

/**
* @notice Allows owner to withdraw protocol fees from this contract.
* @param _token The address of token to remove.
*/
/// @inheritdoc IFeeCollector
function extractERC20(address _token) public payable onlyOwner {
uint256 withdrawAmt = IERC20(_token).balanceOf(address(this)) - totalClientBalances[_token];

SafeTransferLib.safeTransfer(ERC20(_token), msg.sender, withdrawAmt);
}

/**
* @notice Executes when native is sent to this contract through a non-existent function.
*/
fallback() external payable { } // solhint-disable-line no-empty-blocks
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. We can probably remove receive() everywhere too, right? I can't ever see a reason we want the contract to be able to accept ETH.

  2. If so, we can remove the unit tests for both receive() and fallback() everywhere in the tests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  1. There is no reason for the contract to receive ETH as I see it, so receive() could also go. I think the only time ETH would be sent would be some kind of user error, sending to the wrong address or erroneously thinking they need to send ETH to interact with the protocol.

  2. Agreed. I can go through and remove those as well if that is what we want to do with removing both.

Copy link
Contributor

@cucupac cucupac Mar 15, 2024

Choose a reason for hiding this comment

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

For now, let's just remove the unit tests for fallback.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fallback() removed, alongside the associated fuzz test for each.

Tranquil-Flow marked this conversation as resolved.
Show resolved Hide resolved

/**
* @notice Executes when native is sent to this contract with a plain transaction.
*/
receive() external payable { } // solhint-disable-line no-empty-blocks
}
81 changes: 35 additions & 46 deletions src/Position.sol
Original file line number Diff line number Diff line change
@@ -1,43 +1,57 @@
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.21;
pragma solidity 0.8.21;

// Local Imports
import { DebtService } from "src/services/DebtService.sol";
import { SwapService } from "src/services/SwapService.sol";
import { SafeTransferLib, ERC20 } from "solmate/utils/SafeTransferLib.sol";
import { FeeLib } from "src/libraries/FeeLib.sol";
import { IPosition } from "src/interfaces/IPosition.sol";
import { IERC20 } from "src/interfaces/token/IERC20.sol";
import { IERC20Permit } from "src/interfaces/token/IERC20Permit.sol";
import { IERC20Metadata } from "src/interfaces/token/IERC20Metadata.sol";

/// @title Position
/// @title The position contract
/// @author Chain Rule, LLC
/// @notice Manages the owner's individual position
contract Position is DebtService, SwapService {
/// @notice Allows an owner account to manage its individual position
contract Position is DebtService, SwapService, IPosition {
// Immutables: no SLOAD to save gas

/// @inheritdoc IPosition
uint8 public immutable B_DECIMALS;

/// @inheritdoc IPosition
address public immutable B_TOKEN;

// Events
/// @notice An event emitted when a position is created or added to.
/// @param cAmt The amount of collateral token supplied (units: C_DECIMALS).
/// @param dAmt The amount of debt token borrowed (units: D_DECIMALS).
/// @param bAmt The amount of base token received and subsequently supplied as collateral (units: B_DECIMALS).
event Add(uint256 cAmt, uint256 dAmt, uint256 bAmt);

/// @notice An event emitted when leverage is added to a position.
/// @param dAmt The amount of debt token borrowed (units: D_DECIMALS).
/// @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.
/// @param gains The amount of base token gained from the position (units: B_DECIMALS).
event Close(uint256 gains);

/// @notice This function is called when a Position contract is deployed.
/// @param _owner The account address of the Position contract's owner.
/// @param _cToken The address of the token to be used as collateral.
/// @param _dToken The address of the token to be borrowed.
/// @param _bToken The address of the token to swap _dToken for.
constructor(address _owner, address _cToken, address _dToken, address _bToken)
DebtService(_owner, _cToken, _dToken)
{
B_TOKEN = _bToken;
B_DECIMALS = IERC20Metadata(_bToken).decimals();
}

/**
* @notice Adds to this contract's position.
* @param _cAmt The amount of collateral token to be supplied for this transaction-specific loan (units: C_DECIMALS).
* @param _ltv The desired loan-to-value ratio for this transaction-specific loan (ex: 75 is 75%).
* @param _swapAmtOutMin The minimum amount of output tokens from swap for the tx to go through.
* @param _poolFee The fee of the Uniswap pool.
* @param _client The address of the client operator. Use address(0) if not using a client.
*/
/// @inheritdoc IPosition
function add(uint256 _cAmt, uint256 _ltv, uint256 _swapAmtOutMin, uint24 _poolFee, address _client)
public
payable
Expand All @@ -61,19 +75,7 @@ contract Position is DebtService, SwapService {
emit Add(cAmtNet, dAmt, bAmt);
}

/**
* @notice Adds to this contract's position with permit, obviating the need for a separate approve tx.
* This function can only be used for ERC-2612-compliant tokens.
* @param _cAmt The amount of collateral token to be supplied for this transaction-specific loan (units: C_DECIMALS).
* @param _ltv The desired loan-to-value ratio for this transaction-specific loan (ex: 75 is 75%).
* @param _swapAmtOutMin The minimum amount of output tokens from swap for the tx to go through.
* @param _poolFee The fee of the Uniswap pool.
* @param _client The address of the client operator. Use address(0) if not using a client.
* @param _deadline The expiration timestamp of the permit.
* @param _v The V parameter of ERC712 signature for the permit.
* @param _r The R parameter of ERC712 signature for the permit.
* @param _s The S parameter of ERC712 signature for the permit.
*/
/// @inheritdoc IPosition
function addWithPermit(
uint256 _cAmt,
uint256 _ltv,
Expand All @@ -85,21 +87,11 @@ contract Position is DebtService, SwapService {
bytes32 _r,
bytes32 _s
) public payable onlyOwner {
// 1. Approve with permit
IERC20Permit(C_TOKEN).permit(msg.sender, address(this), _cAmt, _deadline, _v, _r, _s);

// 2. Short
add(_cAmt, _ltv, _swapAmtOutMin, _poolFee, _client);
}

/**
* @notice Adds leverage to this contract's position. This function can only be used for positions where the
* collateral token is the same as the base token.
* @param _dAmt The amount of D_TOKEN to borrow; use position LTV to identify max amount.
* @param _swapAmtOutMin The minimum amount of output tokens from swap for the tx to go through.
* @param _poolFee The fee of the Uniswap pool.
* @param _client The address of the client operator. Use address(0) if not using a client.
*/
/// @inheritdoc IPosition
function addLeverage(uint256 _dAmt, uint256 _swapAmtOutMin, uint24 _poolFee, address _client)
public
payable
Expand All @@ -120,14 +112,7 @@ contract Position is DebtService, SwapService {
emit AddLeverage(dAmtNet, bAmt);
}

/**
* @notice Fully closes the position.
* @param _poolFee The fee of the Uniswap pool.
* @param _exactOutput Whether to swap exact output or exact input (true for exact output, false for exact input).
* @param _swapAmtOutMin The minimum amount of output tokens from swap for the tx to go through (only used if _exactOutput is false, supply 0 if 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).
*/
/// @inheritdoc IPosition
function close(
uint24 _poolFee,
bool _exactOutput,
Expand Down Expand Up @@ -155,8 +140,12 @@ contract Position is DebtService, SwapService {
withdraw(C_TOKEN, _withdrawCAmt, OWNER);
}

// 5. pay gains if any: NOTE: can probably be unchecked as bAmtIn will never be greater than _withdrawBAmt
uint256 gains = _withdrawBAmt - bAmtIn;
// 5. pay gains if any
uint256 gains;
unchecked {
gains = _withdrawBAmt - bAmtIn; // unchecked because bAmtIn will never be greater than _withdrawBAmt
}

if (gains != 0) {
SafeTransferLib.safeTransfer(ERC20(B_TOKEN), OWNER, gains);
}
Expand Down
Loading
Loading