Skip to content

Commit

Permalink
fix: better consistency
Browse files Browse the repository at this point in the history
  • Loading branch information
0xmikko committed Apr 1, 2023
1 parent 0a29e3f commit e7e3d73
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 29 deletions.
18 changes: 9 additions & 9 deletions contracts/core/DataCompressor.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {PERCENTAGE_FACTOR} from "@gearbox-protocol/core-v2/contracts/libraries/PercentageMath.sol";

import {IDataCompressor} from "@gearbox-protocol/core-v2/contracts/interfaces/IDataCompressor.sol";
import {ICreditManager} from "@gearbox-protocol/core-v2/contracts/interfaces/V1/ICreditManager.sol";
import {ICreditManager as ICreditManagerV1} from "@gearbox-protocol/core-v2/contracts/interfaces/V1/ICreditManager.sol";
import {ICreditManagerV2} from "../interfaces/ICreditManagerV2.sol";
import {ICreditFacade} from "../interfaces/ICreditFacade.sol";
import {ICreditFilter} from "@gearbox-protocol/core-v2/contracts/interfaces/V1/ICreditFilter.sol";
Expand Down Expand Up @@ -115,7 +115,7 @@ contract DataCompressor is IDataCompressor, ContractsRegisterTrait {
{
(
uint8 ver,
ICreditManager creditManager,
ICreditManagerV1 creditManager,
ICreditFilter creditFilter,
ICreditManagerV2 creditManagerV2,
ICreditFacade creditFacade,
Expand All @@ -136,15 +136,15 @@ contract DataCompressor is IDataCompressor, ContractsRegisterTrait {
result.totalValue = creditFilter.calcTotalValue(creditAccount);
result.healthFactor = creditFilter.calcCreditAccountHealthFactor(creditAccount);

try ICreditManager(creditManager).calcRepayAmount(borrower, false) returns (uint256 value) {
try ICreditManagerV1(creditManager).calcRepayAmount(borrower, false) returns (uint256 value) {
result.repayAmount = value;
} catch {}

try ICreditManager(creditManager).calcRepayAmount(borrower, true) returns (uint256 value) {
try ICreditManagerV1(creditManager).calcRepayAmount(borrower, true) returns (uint256 value) {
result.liquidationAmount = value;
} catch {}

try ICreditManager(creditManager)._calcClosePayments(creditAccount, result.totalValue, false) returns (
try ICreditManagerV1(creditManager)._calcClosePayments(creditAccount, result.totalValue, false) returns (
uint256, uint256, uint256 remainingFunds, uint256, uint256
) {
result.canBeClosed = remainingFunds > 0;
Expand Down Expand Up @@ -217,7 +217,7 @@ contract DataCompressor is IDataCompressor, ContractsRegisterTrait {
function getCreditManagerData(address _creditManager) public view returns (CreditManagerData memory result) {
(
uint8 ver,
ICreditManager creditManager,
ICreditManagerV1 creditManager,
ICreditFilter creditFilter,
ICreditManagerV2 creditManagerV2,
ICreditFacade creditFacade,
Expand Down Expand Up @@ -297,7 +297,7 @@ contract DataCompressor is IDataCompressor, ContractsRegisterTrait {

if (ver == 1) {
// VERSION 1 SPECIFIC FIELDS
result.maxLeverageFactor = ICreditManager(creditManager).maxLeverageFactor();
result.maxLeverageFactor = ICreditManagerV1(creditManager).maxLeverageFactor();
result.maxEnabledTokensLength = 255;
result.feeInterest = uint16(creditManager.feeInterest());
result.feeLiquidation = uint16(creditManager.feeLiquidation());
Expand Down Expand Up @@ -399,7 +399,7 @@ contract DataCompressor is IDataCompressor, ContractsRegisterTrait {
registeredCreditManagerOnly(_creditManager)
returns (
uint8 ver,
ICreditManager creditManager,
ICreditManagerV1 creditManager,
ICreditFilter creditFilter,
ICreditManagerV2 creditManagerV2,
ICreditFacade creditFacade,
Expand All @@ -408,7 +408,7 @@ contract DataCompressor is IDataCompressor, ContractsRegisterTrait {
{
ver = uint8(IVersion(_creditManager).version());
if (ver == 1) {
creditManager = ICreditManager(_creditManager);
creditManager = ICreditManagerV1(_creditManager);
creditFilter = ICreditFilter(creditManager.creditFilter());
} else {
creditManagerV2 = ICreditManagerV2(_creditManager);
Expand Down
6 changes: 4 additions & 2 deletions contracts/credit/CreditManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ contract CreditManager is ICreditManagerV2, ACLNonReentrantTrait {
whenNotPaused // F:[CM-5]
nonReentrant
creditFacadeOnly // F:[CM-2]
nonZeroAddress(onBehalfOf) // TODO: Add test
returns (address)
{
// Takes a Credit Account from the factory and sets initial parameters
Expand Down Expand Up @@ -606,6 +607,7 @@ contract CreditManager is ICreditManagerV2, ACLNonReentrantTrait {
whenNotPausedOrEmergency // F:[CM-5]
nonReentrant
creditFacadeOnly // F:[CM-2]
nonZeroAddress(to) // TODO: Add test
{
address creditAccount = getCreditAccountOrRevert(from); // F:[CM-6]
delete creditAccounts[from]; // F:[CM-24]
Expand Down Expand Up @@ -1144,8 +1146,8 @@ contract CreditManager is ICreditManagerV2, ACLNonReentrantTrait {
/// @param borrower The new owner of the Credit Account
/// @param creditAccount The Credit Account address
function _safeCreditAccountSet(address borrower, address creditAccount) internal {
if (borrower == address(0) || creditAccounts[borrower] != address(0)) {
revert ZeroAddressOrUserAlreadyHasAccountException();
if (creditAccounts[borrower] != address(0)) {
revert UserAlreadyHasAccountException();
} // F:[CM-7]
creditAccounts[borrower] = creditAccount; // F:[CM-7]
}
Expand Down
3 changes: 2 additions & 1 deletion contracts/interfaces/IExceptions.sol
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ error CallerNotUnPausableAdminException();
/// @dev Thrown when a gauge-only function is called by non-gauge
error CallerNotGaugeException();

/// @dev Thrown when a poolQuotaKeeper function is called by non-pqk
error CallerNotPoolQuotaKeeperException();

/// @dev Thrown when `vote` or `unvote` are called from non-voter address
Expand Down Expand Up @@ -176,7 +177,7 @@ error AdaptersOrCreditFacadeOnlyException();

/// @dev Thrown on attempting to open a Credit Account for or transfer a Credit Account
/// to the zero address or an address that already owns a Credit Account
error ZeroAddressOrUserAlreadyHasAccountException();
error UserAlreadyHasAccountException();

/// @dev Thrown on attempting to execute an order to an address that is not an allowed
/// target contract
Expand Down
6 changes: 1 addition & 5 deletions contracts/interfaces/IPool4626.sol
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,7 @@ interface IPool4626Events {
event NewWithdrawFee(uint256 fee);
}

/// @title Pool Service Interface
/// @notice Implements business logic:
/// - Adding/removing pool liquidity
/// - Managing diesel tokens & diesel rates
/// - Taking/repaying Credit Manager debt
/// @title Pool 4626
/// More: https://dev.gearbox.fi/developers/pool/abstractpoolservice
interface IPool4626 is IPool4626Events, IERC4626, IVersion {
function depositReferral(uint256 assets, address receiver, uint16 referralCode) external returns (uint256 shares);
Expand Down
8 changes: 4 additions & 4 deletions contracts/test/credit/CreditManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -566,11 +566,11 @@ contract CreditManagerTest is DSTest, ICreditManagerV2Events, BalanceHelper {
/// @dev [CM-7]: openCreditAccount reverts if zero address or address exists
function test_CM_07_openCreditAccount_reverts_if_zero_address_or_address_exists() public {
// Zero address case
evm.expectRevert(ZeroAddressOrUserAlreadyHasAccountException.selector);
evm.expectRevert(ZeroAddressException.selector);
creditManager.openCreditAccount(1, address(0));
// Existing address case
creditManager.openCreditAccount(1, USER);
evm.expectRevert(ZeroAddressOrUserAlreadyHasAccountException.selector);
evm.expectRevert(UserAlreadyHasAccountException.selector);
creditManager.openCreditAccount(1, USER);
}

Expand Down Expand Up @@ -1209,10 +1209,10 @@ contract CreditManagerTest is DSTest, ICreditManagerV2Events, BalanceHelper {

creditManager.openCreditAccount(1, FRIEND);
// address(0) case
evm.expectRevert(ZeroAddressOrUserAlreadyHasAccountException.selector);
evm.expectRevert(ZeroAddressException.selector);
creditManager.transferAccountOwnership(USER, address(0));
// Existing account case
evm.expectRevert(ZeroAddressOrUserAlreadyHasAccountException.selector);
evm.expectRevert(UserAlreadyHasAccountException.selector);
creditManager.transferAccountOwnership(FRIEND, USER);
}

Expand Down
12 changes: 7 additions & 5 deletions contracts/traits/ACLNonReentrantTrait.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ import {
CallerNotControllerException
} from "../interfaces/IExceptions.sol";

import {SanityCheckTrait} from "./SanityCheckTrait.sol";

/// @title ACL Trait
/// @notice Utility class for ACL consumers
abstract contract ACLNonReentrantTrait is Pausable {
abstract contract ACLNonReentrantTrait is Pausable, SanityCheckTrait {
uint8 private constant _NOT_ENTERED = 1;
uint8 private constant _ENTERED = 2;

Expand Down Expand Up @@ -63,10 +65,10 @@ abstract contract ACLNonReentrantTrait is Pausable {

event NewController(address indexed newController);

modifier nonZeroAddress(address addr) {
if (addr == address(0)) revert ZeroAddressException(); // F:[P4-2]
_;
}
// modifier nonZeroAddress(address addr) {
// if (addr == address(0)) revert ZeroAddressException(); // F:[P4-2]
// _;
// }

/// @dev constructor
/// @param addressProvider Address of address repository
Expand Down
7 changes: 4 additions & 3 deletions contracts/traits/ContractsRegisterTrait.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@ import {
RegisteredPoolOnlyException
} from "../interfaces/IExceptions.sol";

import {SanityCheckTrait} from "./SanityCheckTrait.sol";

/// @title ContractsRegister Trait
/// @notice Trait enables checks for registered pools & creditManagers
abstract contract ContractsRegisterTrait {
abstract contract ContractsRegisterTrait is SanityCheckTrait {
// ACL contract to check rights
ContractsRegister immutable _cr;

constructor(address addressProvider) {
if (addressProvider == address(0)) revert ZeroAddressException(); // F:[P4-2]
constructor(address addressProvider) nonZeroAddress(addressProvider) {
_cr = ContractsRegister(AddressProvider(addressProvider).getContractsRegister());
}

Expand Down
15 changes: 15 additions & 0 deletions contracts/traits/SanityCheckTrait.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// SPDX-License-Identifier: BUSL-1.1
// Gearbox Protocol. Generalized leverage for DeFi protocols
// (c) Gearbox Holdings, 2022
pragma solidity ^0.8.17;

import {ZeroAddressException} from "../interfaces/IExceptions.sol";

/// @title ACL Trait
/// @notice Utility class for ACL consumers
abstract contract SanityCheckTrait {
modifier nonZeroAddress(address addr) {
if (addr == address(0)) revert ZeroAddressException(); // F:[P4-2]
_;
}
}

0 comments on commit e7e3d73

Please sign in to comment.