Skip to content

Commit

Permalink
feat: reorganize traits to remove redundant functions from contracts (#…
Browse files Browse the repository at this point in the history
…283)

Previously, `ACLNonReentrantTrait` was a Swiss knife that combined all traits we commonly use, which had its benefits, but resulted in contracts having functions that they never use (like `setController` in `CreditFacadeV3` or `pause` in `PoolQuotaKeeperV3`), which is not really good considering that contract size limit, well, exists. This PR solves the original issue and makes separation of all functionality much cleaner.
  • Loading branch information
lekhovitsky authored Sep 1, 2024
1 parent 18dff5e commit dcf879b
Show file tree
Hide file tree
Showing 15 changed files with 172 additions and 150 deletions.
7 changes: 4 additions & 3 deletions contracts/core/PriceOracleV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ import {
import {IPriceOracleV3, PriceFeedParams, PriceUpdate} from "../interfaces/IPriceOracleV3.sol";
import {IUpdatablePriceFeed} from "../interfaces/base/IPriceFeed.sol";

import {ACLNonReentrantTrait} from "../traits/ACLNonReentrantTrait.sol";
import {ControlledTrait} from "../traits/ControlledTrait.sol";
import {PriceFeedValidationTrait} from "../traits/PriceFeedValidationTrait.sol";
import {SanityCheckTrait} from "../traits/SanityCheckTrait.sol";

/// @title Price oracle V3
/// @notice Acts as router that dispatches calls to corresponding price feeds.
Expand All @@ -33,7 +34,7 @@ import {PriceFeedValidationTrait} from "../traits/PriceFeedValidationTrait.sol";
/// not be used for general collateral evaluation, including decisions on whether accounts are liquidatable.
/// - Finally, this contract serves as register for updatable price feeds and can be used to apply batched
/// on-demand price updates while ensuring that those are not calls to arbitrary contracts.
contract PriceOracleV3 is ACLNonReentrantTrait, PriceFeedValidationTrait, IPriceOracleV3 {
contract PriceOracleV3 is ControlledTrait, PriceFeedValidationTrait, SanityCheckTrait, IPriceOracleV3 {
using EnumerableSet for EnumerableSet.AddressSet;

/// @notice Contract version
Expand All @@ -50,7 +51,7 @@ contract PriceOracleV3 is ACLNonReentrantTrait, PriceFeedValidationTrait, IPrice

/// @notice Constructor
/// @param _acl ACL contract address
constructor(address _acl) ACLNonReentrantTrait(_acl) {}
constructor(address _acl) ControlledTrait(_acl) {}

/// @notice Returns all tokens that have price feeds
function getTokens() external view override returns (address[] memory) {
Expand Down
9 changes: 6 additions & 3 deletions contracts/credit/CreditConfiguratorV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {CreditLogic} from "../libraries/CreditLogic.sol";
import {PERCENTAGE_FACTOR, UNDERLYING_TOKEN_MASK, WAD} from "../libraries/Constants.sol";

// CONTRACTS
import {ACLNonReentrantTrait} from "../traits/ACLNonReentrantTrait.sol";
import {CreditFacadeV3} from "./CreditFacadeV3.sol";
import {CreditManagerV3} from "./CreditManagerV3.sol";

Expand All @@ -24,13 +23,17 @@ import {IPoolQuotaKeeperV3} from "../interfaces/IPoolQuotaKeeperV3.sol";
import {IPriceOracleV3} from "../interfaces/IPriceOracleV3.sol";
import {IAdapter} from "../interfaces/base/IAdapter.sol";

// TRAITS
import {ControlledTrait} from "../traits/ControlledTrait.sol";
import {SanityCheckTrait} from "../traits/SanityCheckTrait.sol";

// EXCEPTIONS
import "../interfaces/IExceptions.sol";

/// @title Credit configurator V3
/// @notice Provides funcionality to configure various aspects of credit manager and facade's behavior
/// @dev Most of the functions can only be accessed by configurator or timelock controller
contract CreditConfiguratorV3 is ICreditConfiguratorV3, ACLNonReentrantTrait {
contract CreditConfiguratorV3 is ICreditConfiguratorV3, ControlledTrait, SanityCheckTrait {
using EnumerableSet for EnumerableSet.AddressSet;
using Address for address;
using BitMask for uint256;
Expand All @@ -57,7 +60,7 @@ contract CreditConfiguratorV3 is ICreditConfiguratorV3, ACLNonReentrantTrait {
/// @param _creditManager Credit manager to connect to
/// @dev Copies allowed adaprters from the currently connected configurator
constructor(address _creditManager)
ACLNonReentrantTrait(ACLNonReentrantTrait(CreditManagerV3(_creditManager).pool()).acl())
ControlledTrait(ControlledTrait(CreditManagerV3(_creditManager).pool()).acl())
{
creditManager = _creditManager; // I:[CC-1]
underlying = CreditManagerV3(_creditManager).underlying(); // I:[CC-1]
Expand Down
21 changes: 18 additions & 3 deletions contracts/credit/CreditFacadeV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {IERC20Permit} from "@openzeppelin/contracts/token/ERC20/extensions/IERC2
import {Address} from "@openzeppelin/contracts/utils/Address.sol";
import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";
import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
import {Pausable} from "@openzeppelin/contracts/security/Pausable.sol";

// INTERFACES
import {IBotListV3} from "../interfaces/IBotListV3.sol";
Expand Down Expand Up @@ -46,7 +47,9 @@ import {
} from "../libraries/Constants.sol";

// TRAITS
import {ACLNonReentrantTrait} from "../traits/ACLNonReentrantTrait.sol";
import {ACLTrait} from "../traits/ACLTrait.sol";
import {ReentrancyGuardTrait} from "../traits/ReentrancyGuardTrait.sol";
import {SanityCheckTrait} from "../traits/SanityCheckTrait.sol";

/// @title Credit facade V3
/// @notice Provides a user interface to open, close and liquidate leveraged positions in the credit manager,
Expand All @@ -62,7 +65,7 @@ import {ACLNonReentrantTrait} from "../traits/ACLNonReentrantTrait.sol";
/// quota size validation, pausing on large protocol losses, Degen NFT whitelist mode, and forbidden tokens
/// (they count towards account value, but having them enabled as collateral restricts available actions and
/// activates a safer version of collateral check).
contract CreditFacadeV3 is ICreditFacadeV3, ACLNonReentrantTrait {
contract CreditFacadeV3 is ICreditFacadeV3, Pausable, ACLTrait, ReentrancyGuardTrait, SanityCheckTrait {
using Address for address;
using BitMask for uint256;
using SafeERC20 for IERC20;
Expand Down Expand Up @@ -157,7 +160,7 @@ contract CreditFacadeV3 is ICreditFacadeV3, ACLNonReentrantTrait {
/// @param _expirable Whether this facade should be expirable. If `true`, the expiration date remains unset,
/// and facade never expires, until the date is set via `setExpirationDate` in the configurator.
constructor(address _creditManager, address _botList, address _weth, address _degenNFT, bool _expirable)
ACLNonReentrantTrait(ACLNonReentrantTrait(ICreditManagerV3(_creditManager).pool()).acl())
ACLTrait(ACLTrait(ICreditManagerV3(_creditManager).pool()).acl())
nonZeroAddress(_botList)
{
creditManager = _creditManager; // U:[FA-1]
Expand Down Expand Up @@ -845,6 +848,18 @@ contract CreditFacadeV3 is ICreditFacadeV3, ACLNonReentrantTrait {
} // U:[FA-53]
}

/// @notice Pauses contract, can only be called by an account with pausable admin role
/// @dev Reverts if contract is already paused
function pause() external override pausableAdminsOnly {
_pause();
}

/// @notice Unpauses contract, can only be called by an account with unpausable admin role
/// @dev Reverts if contract is already unpaused
function unpause() external override unpausableAdminsOnly {
_unpause();
}

// --------- //
// INTERNALS //
// --------- //
Expand Down
4 changes: 4 additions & 0 deletions contracts/interfaces/ICreditFacadeV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -152,4 +152,8 @@ interface ICreditFacadeV3 is IVersion, ICreditFacadeV3Events {
function setTokenAllowance(address token, AllowanceAction allowance) external;

function setEmergencyLiquidator(address liquidator, AllowanceAction allowance) external;

function pause() external;

function unpause() external;
}
4 changes: 4 additions & 0 deletions contracts/interfaces/IPoolV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -129,4 +129,8 @@ interface IPoolV3 is IVersion, IPoolV3Events, IERC4626, IERC20Permit {
function setCreditManagerDebtLimit(address creditManager, uint256 newLimit) external;

function setWithdrawFee(uint256 newWithdrawFee) external;

function pause() external;

function unpause() external;
}
7 changes: 4 additions & 3 deletions contracts/pool/GaugeV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import {IPoolQuotaKeeperV3} from "../interfaces/IPoolQuotaKeeperV3.sol";
import {IPoolV3} from "../interfaces/IPoolV3.sol";

// TRAITS
import {ACLNonReentrantTrait} from "../traits/ACLNonReentrantTrait.sol";
import {ControlledTrait} from "../traits/ControlledTrait.sol";
import {SanityCheckTrait} from "../traits/SanityCheckTrait.sol";

// EXCEPTIONS
import {
Expand All @@ -29,7 +30,7 @@ import {
/// determined by the Gearbox DAO. GEAR holders then vote either for CA side, which moves the rate towards min,
/// or for LP side, which moves it towards max.
/// Rates are only updated once per epoch (1 week), to avoid manipulation and make strategies more predictable.
contract GaugeV3 is IGaugeV3, ACLNonReentrantTrait {
contract GaugeV3 is IGaugeV3, ControlledTrait, SanityCheckTrait {
/// @notice Contract version
uint256 public constant override version = 3_10;

Expand Down Expand Up @@ -61,7 +62,7 @@ contract GaugeV3 is IGaugeV3, ACLNonReentrantTrait {
/// @param _pool Address of the lending pool
/// @param _gearStaking Address of the GEAR staking contract
constructor(address _pool, address _gearStaking)
ACLNonReentrantTrait(ACLNonReentrantTrait(_pool).acl())
ControlledTrait(ControlledTrait(_pool).acl())
nonZeroAddress(_gearStaking) // U:[GA-1]
{
pool = _pool; // U:[GA-1]
Expand Down
9 changes: 5 additions & 4 deletions contracts/pool/PoolQuotaKeeperV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ pragma abicoder v1;

import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";

/// LIBS & TRAITS
import {ACLNonReentrantTrait} from "../traits/ACLNonReentrantTrait.sol";
// LIBS & TRAITS
import {ControlledTrait} from "../traits/ControlledTrait.sol";
import {ContractsRegisterTrait} from "../traits/ContractsRegisterTrait.sol";
import {SanityCheckTrait} from "../traits/SanityCheckTrait.sol";
import {QuotasLogic} from "../libraries/QuotasLogic.sol";

import {IPoolV3} from "../interfaces/IPoolV3.sol";
Expand All @@ -29,7 +30,7 @@ import "../interfaces/IExceptions.sol";
/// * increase fee that is charged when additional quota is purchased (more suited to leveraged trading).
/// Quota keeper stores information about quotas of accounts in all credit managers connected to the pool, and
/// performs calculations that help to keep pool's expected liquidity and credit managers' debt consistent.
contract PoolQuotaKeeperV3 is IPoolQuotaKeeperV3, ACLNonReentrantTrait, ContractsRegisterTrait {
contract PoolQuotaKeeperV3 is IPoolQuotaKeeperV3, ControlledTrait, ContractsRegisterTrait, SanityCheckTrait {
using EnumerableSet for EnumerableSet.AddressSet;
using QuotasLogic for TokenQuotaParams;

Expand Down Expand Up @@ -77,7 +78,7 @@ contract PoolQuotaKeeperV3 is IPoolQuotaKeeperV3, ACLNonReentrantTrait, Contract
/// @notice Constructor
/// @param _pool Pool address
constructor(address _pool)
ACLNonReentrantTrait(ACLNonReentrantTrait(_pool).acl())
ControlledTrait(ControlledTrait(_pool).acl())
ContractsRegisterTrait(ContractsRegisterTrait(_pool).contractsRegister())
{
pool = _pool; // U:[PQK-1]
Expand Down
30 changes: 27 additions & 3 deletions contracts/pool/PoolV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {ERC20Permit} from "@openzeppelin/contracts/token/ERC20/extensions/ERC20P
import {Math} from "@openzeppelin/contracts/utils/math/Math.sol";
import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol";
import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
import {Pausable} from "@openzeppelin/contracts/security/Pausable.sol";

// INTERFACES
import {ICreditManagerV3} from "../interfaces/ICreditManagerV3.sol";
Expand All @@ -25,8 +26,10 @@ import {IInterestRateModel} from "../interfaces/base/IInterestRateModel.sol";

// LIBS & TRAITS
import {CreditLogic} from "../libraries/CreditLogic.sol";
import {ACLNonReentrantTrait} from "../traits/ACLNonReentrantTrait.sol";
import {ControlledTrait} from "../traits/ControlledTrait.sol";
import {ContractsRegisterTrait} from "../traits/ContractsRegisterTrait.sol";
import {ReentrancyGuardTrait} from "../traits/ReentrancyGuardTrait.sol";
import {SanityCheckTrait} from "../traits/SanityCheckTrait.sol";

// CONSTANTS
import {RAY, MAX_WITHDRAW_FEE, PERCENTAGE_FACTOR} from "../libraries/Constants.sol";
Expand All @@ -49,7 +52,16 @@ struct DebtParams {
/// (nearly all tokens with decimals between 6 and 18 work)
/// @dev To prevent the first depositor front-running attack, small amount of shares must be minted to some
/// dead address before allowing borrowing
contract PoolV3 is ERC4626, ERC20Permit, ACLNonReentrantTrait, ContractsRegisterTrait, IPoolV3 {
contract PoolV3 is
ERC4626,
ERC20Permit,
Pausable,
SanityCheckTrait,
ControlledTrait,
ContractsRegisterTrait,
ReentrancyGuardTrait,
IPoolV3
{
using Math for uint256;
using SafeCast for int256;
using SafeCast for uint256;
Expand Down Expand Up @@ -119,7 +131,7 @@ contract PoolV3 is ERC4626, ERC20Permit, ACLNonReentrantTrait, ContractsRegister
string memory name_,
string memory symbol_
)
ACLNonReentrantTrait(acl_) // U:[LP-1A]
ControlledTrait(acl_) // U:[LP-1A]
ContractsRegisterTrait(contractsRegister_)
ERC4626(IERC20(underlyingToken_)) // U:[LP-1B]
ERC20(name_, symbol_) // U:[LP-1B]
Expand Down Expand Up @@ -742,6 +754,18 @@ contract PoolV3 is ERC4626, ERC20Permit, ACLNonReentrantTrait, ContractsRegister
emit SetWithdrawFee(newWithdrawFee); // U:[LP-26B]
}

/// @notice Pauses contract, can only be called by an account with pausable admin role
/// @dev Reverts if contract is already paused
function pause() external override pausableAdminsOnly {
_pause();
}

/// @notice Unpauses contract, can only be called by an account with unpausable admin role
/// @dev Reverts if contract is already unpaused
function unpause() external override unpausableAdminsOnly {
_unpause();
}

/// @dev Sets new total debt limit
function _setTotalDebtLimit(uint256 limit) internal {
uint128 newLimit = _convertToU128(limit);
Expand Down
7 changes: 4 additions & 3 deletions contracts/pool/TumblerV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@ import {
import {IPoolQuotaKeeperV3} from "../interfaces/IPoolQuotaKeeperV3.sol";
import {IPoolV3} from "../interfaces/IPoolV3.sol";
import {ITumblerV3} from "../interfaces/ITumblerV3.sol";
import {ACLNonReentrantTrait} from "../traits/ACLNonReentrantTrait.sol";
import {ControlledTrait} from "../traits/ControlledTrait.sol";
import {SanityCheckTrait} from "../traits/SanityCheckTrait.sol";

/// @title Tumbler V3
/// @notice Extremely simplified version of `GaugeV3` contract for quota rates management, which,
/// instead of voting, allows controller to set rates directly with custom epoch length
contract TumblerV3 is ITumblerV3, ACLNonReentrantTrait {
contract TumblerV3 is ITumblerV3, ControlledTrait, SanityCheckTrait {
using EnumerableSet for EnumerableSet.AddressSet;

/// @notice Contract version
Expand Down Expand Up @@ -49,7 +50,7 @@ contract TumblerV3 is ITumblerV3, ACLNonReentrantTrait {
/// @param pool_ Pool whose quota rates to set by this contract
/// @param epochLength_ Epoch length in seconds
/// @custom:tests U:[TU-1]
constructor(address pool_, uint256 epochLength_) ACLNonReentrantTrait(ACLNonReentrantTrait(pool_).acl()) {
constructor(address pool_, uint256 epochLength_) ControlledTrait(ControlledTrait(pool_).acl()) {
pool = pool_;
underlying = IPoolV3(pool_).underlyingToken();
poolQuotaKeeper = IPoolV3(pool_).poolQuotaKeeper();
Expand Down
20 changes: 0 additions & 20 deletions contracts/test/mocks/core/ACLTraitTest.sol

This file was deleted.

7 changes: 4 additions & 3 deletions contracts/test/mocks/governance/GaugeMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";

import {SafeERC20} from "@1inch/solidity-utils/contracts/libraries/SafeERC20.sol";
import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol";
import {ACLNonReentrantTrait} from "../../../traits/ACLNonReentrantTrait.sol";
import {ACLTrait} from "../../../traits/ACLTrait.sol";
import {SanityCheckTrait} from "../../../traits/SanityCheckTrait.sol";

// interfaces

Expand All @@ -16,7 +17,7 @@ import {IPoolQuotaKeeperV3} from "../../../interfaces/IPoolQuotaKeeperV3.sol";
import {PoolV3} from "../../../pool/PoolV3.sol";

/// @title Gauge fore new 4626 pools
contract GaugeMock is ACLNonReentrantTrait {
contract GaugeMock is ACLTrait, SanityCheckTrait {
using EnumerableSet for EnumerableSet.AddressSet;
using SafeERC20 for IERC20;

Expand All @@ -32,7 +33,7 @@ contract GaugeMock is ACLNonReentrantTrait {

/// @dev Constructor

constructor(address acl, address _pool) ACLNonReentrantTrait(acl) nonZeroAddress(_pool) {
constructor(address acl, address _pool) ACLTrait(acl) nonZeroAddress(_pool) {
pool = PoolV3(payable(_pool)); // F:[P4-01]
}

Expand Down
Loading

0 comments on commit dcf879b

Please sign in to comment.