Skip to content

Commit

Permalink
feat: IACL now provides hasRole
Browse files Browse the repository at this point in the history
- set of emergency liquidators is now stored in ACL instead of credit facades
- credit facade's constructor accepts ACL as argument since deployed pools have legacy acl
- other contracts don't use roles at all so they can just inherit ACL even from deployed pools
  • Loading branch information
lekhovitsky committed Dec 9, 2024
1 parent 50cdb4a commit 725c7f3
Show file tree
Hide file tree
Showing 13 changed files with 61 additions and 223 deletions.
48 changes: 0 additions & 48 deletions contracts/credit/CreditConfiguratorV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -515,8 +515,6 @@ contract CreditConfiguratorV3 is ICreditConfiguratorV3, ACLTrait, SanityCheckTra
address lossLiquidator = prevCreditFacade.lossLiquidator();
if (lossLiquidator != address(0)) _setLossLiquidator(lossLiquidator); // I:[CC-22]

_migrateEmergencyLiquidators(prevCreditFacade); // I:[CC-22C]

_migrateForbiddenTokens(prevCreditFacade.forbiddenTokenMask()); // I:[CC-22C]

if (prevCreditFacade.expirable() && CreditFacadeV3(newCreditFacade).expirable()) {
Expand All @@ -527,17 +525,6 @@ contract CreditConfiguratorV3 is ICreditConfiguratorV3, ACLTrait, SanityCheckTra
emit SetCreditFacade(newCreditFacade); // I:[CC-22]
}

/// @dev Migrate emergency liquidators to the new credit facade
function _migrateEmergencyLiquidators(CreditFacadeV3 prevCreditFacade) internal {
address[] memory emergencyLiquidators = prevCreditFacade.emergencyLiquidators();
uint256 len = emergencyLiquidators.length;
unchecked {
for (uint256 i; i < len; ++i) {
_addEmergencyLiquidator(emergencyLiquidators[i]);
}
}
}

/// @dev Migrates forbidden tokens to the new credit facade
function _migrateForbiddenTokens(uint256 forbiddenTokensMask) internal {
unchecked {
Expand Down Expand Up @@ -690,41 +677,6 @@ contract CreditConfiguratorV3 is ICreditConfiguratorV3, ACLTrait, SanityCheckTra
emit SetExpirationDate(newExpirationDate); // I:[CC-25]
}

/// @notice Adds an address to the list of emergency liquidators
/// @param liquidator Address to add to the list
function addEmergencyLiquidator(address liquidator)
external
override
configuratorOnly // I:[CC-2]
{
_addEmergencyLiquidator(liquidator); // I:[CC-27]
}

/// @dev `addEmergencyLiquidator` implementation
function _addEmergencyLiquidator(address liquidator) internal {
CreditFacadeV3 cf = CreditFacadeV3(creditFacade());

if (cf.isEmergencyLiquidator(liquidator)) return;

cf.setEmergencyLiquidator(liquidator, AllowanceAction.ALLOW); // I:[CC-27]
emit AddEmergencyLiquidator(liquidator); // I:[CC-27]
}

/// @notice Removes an address from the list of emergency liquidators
/// @param liquidator Address to remove from the list
function removeEmergencyLiquidator(address liquidator)
external
override
configuratorOnly // I:[CC-2]
{
CreditFacadeV3 cf = CreditFacadeV3(creditFacade());

if (cf.isEmergencyLiquidator(liquidator)) {
cf.setEmergencyLiquidator(liquidator, AllowanceAction.FORBID); // I:[CC-28]
emit RemoveEmergencyLiquidator(liquidator); // I:[CC-28]
}
}

// --------- //
// INTERNALS //
// --------- //
Expand Down
44 changes: 10 additions & 34 deletions contracts/credit/CreditFacadeV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,6 @@ contract CreditFacadeV3 is ICreditFacadeV3, Pausable, ACLTrait, ReentrancyGuardT
/// @notice Contract that enforces a policy on how liquidations with loss are performed
address public override lossLiquidator;

/// @dev Set of emergency liquidators
EnumerableSet.AddressSet internal _emergencyLiquidatorsSet;

/// @dev Ensures that function caller is credit configurator
modifier creditConfiguratorOnly() {
_checkCreditConfigurator();
Expand All @@ -138,7 +135,7 @@ contract CreditFacadeV3 is ICreditFacadeV3, Pausable, ACLTrait, ReentrancyGuardT
/// caller is an approved emergency liquidator or the loss liquidator
modifier whenNotPausedOrEmergency() {
require(
!paused() || _emergencyLiquidatorsSet.contains(msg.sender) || msg.sender == lossLiquidator,
!paused() || _hasRole("EMERGENCY_LIQUIDATOR", msg.sender) || msg.sender == lossLiquidator,
"Pausable: paused"
);
_;
Expand All @@ -157,16 +154,21 @@ contract CreditFacadeV3 is ICreditFacadeV3, Pausable, ACLTrait, ReentrancyGuardT
}

/// @notice Constructor
/// @param _acl ACL contract address
/// @param _creditManager Credit manager to connect this facade to
/// @param _botList Bot list address
/// @param _weth WETH token address
/// @param _degenNFT Degen NFT address or `address(0)`
/// @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)
ACLTrait(ACLTrait(ICreditManagerV3(_creditManager).pool()).acl())
nonZeroAddress(_botList)
{
constructor(
address _acl,
address _creditManager,
address _botList,
address _weth,
address _degenNFT,
bool _expirable
) ACLTrait(_acl) nonZeroAddress(_botList) {
creditManager = _creditManager; // U:[FA-1]
botList = _botList; // U:[FA-1]
weth = _weth; // U:[FA-1]
Expand All @@ -177,16 +179,6 @@ contract CreditFacadeV3 is ICreditFacadeV3, Pausable, ACLTrait, ReentrancyGuardT
treasury = IPoolV3(ICreditManagerV3(_creditManager).pool()).treasury(); // U:[FA-1]
}

/// @notice Whether `addr` is an approved emergency liquidator
function isEmergencyLiquidator(address addr) public view override returns (bool) {
return _emergencyLiquidatorsSet.contains(addr);
}

/// @notice Returns emergency liquidators
function emergencyLiquidators() external view override returns (address[] memory) {
return _emergencyLiquidatorsSet.values();
}

// ------------------ //
// ACCOUNT MANAGEMENT //
// ------------------ //
Expand Down Expand Up @@ -886,22 +878,6 @@ contract CreditFacadeV3 is ICreditFacadeV3, Pausable, ACLTrait, ReentrancyGuardT
: forbiddenTokenMask.enable(tokenMask); // U:[FA-52]
}

/// @notice Changes account's status as emergency liquidator
/// @param liquidator Account to change the status for
/// @param allowance Status to set
/// @dev Reverts if caller is not credit configurator
function setEmergencyLiquidator(address liquidator, AllowanceAction allowance)
external
override
creditConfiguratorOnly // U:[FA-6]
{
if (allowance == AllowanceAction.ALLOW) {
_emergencyLiquidatorsSet.add(liquidator);
} else {
_emergencyLiquidatorsSet.remove(liquidator);
} // U:[FA-53]
}

/// @notice Pauses contract, can only be called by an account with pausable admin role
/// @dev Pause blocks all user entrypoints to the contract.
/// Liquidations remain open only to emergency and loss liquidators.
Expand Down
10 changes: 0 additions & 10 deletions contracts/interfaces/ICreditConfiguratorV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,6 @@ interface ICreditConfiguratorV3Events {

/// @notice Emitted when a new expiration timestamp is set in the credit facade
event SetExpirationDate(uint40 expirationDate);

/// @notice Emitted when an address is added to the list of emergency liquidators
event AddEmergencyLiquidator(address indexed liquidator);

/// @notice Emitted when an address is removed from the list of emergency liquidators
event RemoveEmergencyLiquidator(address indexed liquidator);
}

/// @title Credit configurator V3 interface
Expand Down Expand Up @@ -163,8 +157,4 @@ interface ICreditConfiguratorV3 is IVersion, IACLTrait, ICreditConfiguratorV3Eve
function setLossLiquidator(address newLossLiquidator) external;

function setExpirationDate(uint40 newExpirationDate) external;

function addEmergencyLiquidator(address liquidator) external;

function removeEmergencyLiquidator(address liquidator) external;
}
6 changes: 0 additions & 6 deletions contracts/interfaces/ICreditFacadeV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,6 @@ interface ICreditFacadeV3 is IVersion, IACLTrait, ICreditFacadeV3Events {

function forbiddenTokenMask() external view returns (uint256);

function emergencyLiquidators() external view returns (address[] memory);

function isEmergencyLiquidator(address) external view returns (bool);

// ------------------ //
// ACCOUNT MANAGEMENT //
// ------------------ //
Expand Down Expand Up @@ -149,8 +145,6 @@ interface ICreditFacadeV3 is IVersion, IACLTrait, ICreditFacadeV3Events {

function setTokenAllowance(address token, AllowanceAction allowance) external;

function setEmergencyLiquidator(address liquidator, AllowanceAction allowance) external;

function pause() external;

function unpause() external;
Expand Down
3 changes: 1 addition & 2 deletions contracts/interfaces/base/IACL.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,5 @@ pragma solidity ^0.8.17;

interface IACL {
function isConfigurator(address account) external view returns (bool);
function isPausableAdmin(address addr) external view returns (bool);
function isUnpausableAdmin(address addr) external view returns (bool);
function hasRole(bytes32 role, address account) external view returns (bool);
}
83 changes: 11 additions & 72 deletions contracts/test/integration/credit/CreditConfigurator.int.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -237,12 +237,6 @@ contract CreditConfiguratorIntegrationTest is IntegrationTestHelper, ICreditConf
vm.expectRevert(CallerNotConfiguratorException.selector);
creditConfigurator.setLossLiquidator(address(0));

vm.expectRevert(CallerNotConfiguratorException.selector);
creditConfigurator.addEmergencyLiquidator(address(0));

vm.expectRevert(CallerNotConfiguratorException.selector);
creditConfigurator.removeEmergencyLiquidator(address(0));

vm.expectRevert(CallerNotConfiguratorException.selector);
creditConfigurator.setExpirationDate(0);

Expand Down Expand Up @@ -805,8 +799,9 @@ contract CreditConfiguratorIntegrationTest is IntegrationTestHelper, ICreditConf
bool migrateSettings = ms != 0;

if (expirable) {
CreditFacadeV3 initialCf =
new CreditFacadeV3(address(creditManager), address(botList), address(0), address(0), true);
CreditFacadeV3 initialCf = new CreditFacadeV3(
address(acl), address(creditManager), address(botList), address(0), address(0), true
);

vm.prank(CONFIGURATOR);
creditConfigurator.setCreditFacade(address(initialCf), migrateSettings);
Expand All @@ -825,8 +820,9 @@ contract CreditConfiguratorIntegrationTest is IntegrationTestHelper, ICreditConf
vm.prank(CONFIGURATOR);
creditConfigurator.setMaxDebtPerBlockMultiplier(DEFAULT_LIMIT_PER_BLOCK_MULTIPLIER + 1);

CreditFacadeV3 cf =
new CreditFacadeV3(address(creditManager), address(botList), address(0), address(0), expirable);
CreditFacadeV3 cf = new CreditFacadeV3(
address(acl), address(creditManager), address(botList), address(0), address(0), expirable
);

uint8 maxDebtPerBlockMultiplier = creditFacade.maxDebtPerBlockMultiplier();

Expand Down Expand Up @@ -870,7 +866,8 @@ contract CreditConfiguratorIntegrationTest is IntegrationTestHelper, ICreditConf
function test_I_CC_22B_setCreditFacade_reverts_if_new_facade_is_adapter() public creditTest {
vm.startPrank(CONFIGURATOR);

CreditFacadeV3 cf = new CreditFacadeV3(address(creditManager), address(botList), address(0), address(0), false);
CreditFacadeV3 cf =
new CreditFacadeV3(address(acl), address(creditManager), address(botList), address(0), address(0), false);
AdapterMock adapter = new AdapterMock(address(creditManager), address(cf));
TargetContractMock target = new TargetContractMock();

Expand All @@ -897,10 +894,6 @@ contract CreditConfiguratorIntegrationTest is IntegrationTestHelper, ICreditConf
bool migrateSettings = ms != 0;

vm.startPrank(CONFIGURATOR);

creditConfigurator.addEmergencyLiquidator(DUMB_ADDRESS);
creditConfigurator.addEmergencyLiquidator(DUMB_ADDRESS2);

address crvToken = tokenTestSuite.addressOf(TOKEN_CRV);
uint256 crvMask = creditManager.getTokenMaskOrRevert(crvToken);
address cvxToken = tokenTestSuite.addressOf(TOKEN_CVX);
Expand All @@ -911,8 +904,9 @@ contract CreditConfiguratorIntegrationTest is IntegrationTestHelper, ICreditConf

vm.stopPrank();

CreditFacadeV3 cf =
new CreditFacadeV3(address(creditManager), address(botList), address(0), address(0), false);
CreditFacadeV3 cf = new CreditFacadeV3(
address(acl), address(creditManager), address(botList), address(0), address(0), false
);

vm.prank(CONFIGURATOR);
creditConfigurator.setCreditFacade(address(cf), migrateSettings);
Expand All @@ -921,23 +915,7 @@ contract CreditConfiguratorIntegrationTest is IntegrationTestHelper, ICreditConf
cf.forbiddenTokenMask(), migrateSettings ? crvMask | cvxMask : 0, "Incorrect forbidden mask migration"
);

assertEq(
cf.isEmergencyLiquidator(DUMB_ADDRESS),
migrateSettings,
"Emergency liquidator 1 was not migrated correctly"
);

assertEq(
cf.isEmergencyLiquidator(DUMB_ADDRESS2),
migrateSettings,
"Emergency liquidator 2 was not migrated correctly"
);

if (!migrateSettings) {
address[] memory el = cf.emergencyLiquidators();

assertEq(el.length, 0, "Emergency liquidator array was not deleted");

assertEq(cf.forbiddenTokenMask(), 0, "Incorrect forbidden token mask");
}

Expand Down Expand Up @@ -1043,45 +1021,6 @@ contract CreditConfiguratorIntegrationTest is IntegrationTestHelper, ICreditConf
assertEq(creditFacade.lossLiquidator(), liquidator, "Loss liquidator not set");
}

/// @dev I:[CC-27]: addEmergencyLiquidator works correctly and emits event
function test_I_CC_27_addEmergencyLiquidator_works_correctly() public creditTest {
vm.expectEmit(false, false, false, true);
emit AddEmergencyLiquidator(DUMB_ADDRESS);

vm.prank(CONFIGURATOR);
creditConfigurator.addEmergencyLiquidator(DUMB_ADDRESS);

assertTrue(
creditFacade.isEmergencyLiquidator(DUMB_ADDRESS), "Credit manager emergency liquidator status incorrect"
);

address[] memory el = creditFacade.emergencyLiquidators();

assertEq(el.length, 1, "Emergency liquidator was not added to array");

assertEq(el[0], DUMB_ADDRESS, "Emergency liquidator address is incorrect");
}

/// @dev I:[CC-28]: removeEmergencyLiquidator works correctly and emits event
function test_I_CC_28_removeEmergencyLiquidator_works_correctly() public creditTest {
vm.prank(CONFIGURATOR);
creditConfigurator.addEmergencyLiquidator(DUMB_ADDRESS);

vm.expectEmit(false, false, false, true);
emit RemoveEmergencyLiquidator(DUMB_ADDRESS);

vm.prank(CONFIGURATOR);
creditConfigurator.removeEmergencyLiquidator(DUMB_ADDRESS);

assertTrue(
!creditFacade.isEmergencyLiquidator(DUMB_ADDRESS), "Credit manager emergency liquidator status incorrect"
);

address[] memory el = creditFacade.emergencyLiquidators();

assertEq(el.length, 0, "Emergency liquidator was not removed from array");
}

/// @dev I:[CC-29]: Array-based parameters are migrated correctly to new CC
function test_I_CC_29_arrays_are_migrated_correctly_for_new_CC() public withAdapterMock creditTest {
vm.prank(CONFIGURATOR);
Expand Down
11 changes: 5 additions & 6 deletions contracts/test/mocks/core/AddressProviderV3ACLMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ contract AddressProviderV3ACLMock is Test, IAddressProviderV3, Ownable {
mapping(address => bool) public isPool;
mapping(address => bool) public isCreditManager;

mapping(address => bool) public isPausableAdmin;
mapping(address => bool) public isUnpausableAdmin;
mapping(bytes32 => mapping(address => bool)) public hasRole;

constructor() {
_setAddress(AP_ACL, address(this), NO_VERSION_CONTROL);
Expand Down Expand Up @@ -82,11 +81,11 @@ contract AddressProviderV3ACLMock is Test, IAddressProviderV3, Ownable {
isCreditManager[creditManager] = true;
}

function addPausableAdmin(address newAdmin) external {
isPausableAdmin[newAdmin] = true;
function grantRole(bytes32 role, address account) external {
hasRole[role][account] = true;
}

function addUnpausableAdmin(address newAdmin) external {
isUnpausableAdmin[newAdmin] = true;
function revokeRole(bytes32 role, address account) external {
hasRole[role][account] = false;
}
}
4 changes: 3 additions & 1 deletion contracts/test/suites/CreditManagerFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ pragma solidity ^0.8.17;
import "@openzeppelin/contracts/utils/Create2.sol";

import "../interfaces/IAddressProviderV3.sol";
import {IPoolV3} from "../../interfaces/IPoolV3.sol";

import {CreditManagerV3} from "../../credit/CreditManagerV3.sol";
import {CreditFacadeV3} from "../../credit/CreditFacadeV3.sol";
Expand Down Expand Up @@ -35,7 +36,8 @@ contract CreditManagerFactory {
) {
creditManager = new CreditManagerV3(pool, accountFactory, priceOracle, maxEnabledTokens, feeInterest, name);

creditFacade = new CreditFacadeV3(address(creditManager), botList, weth, degenNFT, expirable);
creditFacade =
new CreditFacadeV3(IPoolV3(pool).acl(), address(creditManager), botList, weth, degenNFT, expirable);
creditManager.setCreditFacade(address(creditFacade));

creditConfigurator = new CreditConfiguratorV3(address(creditManager));
Expand Down
4 changes: 2 additions & 2 deletions contracts/test/suites/GenesisFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ contract GenesisFactory is Ownable {

gearStaking = new GearStakingV3(msg.sender, address(gearToken), 1);

acl.addPausableAdmin(msg.sender);
acl.addUnpausableAdmin(msg.sender);
acl.grantRole("PAUSABLE_ADMIN", msg.sender);
acl.grantRole("UNPAUSABLE_ADMIN", msg.sender);
acl.transferOwnership(msg.sender);
}
}
Loading

0 comments on commit 725c7f3

Please sign in to comment.