Skip to content

Commit

Permalink
feat: remove credit manager whitelist from BotListV3
Browse files Browse the repository at this point in the history
Required for the new governance framework.
Malicious triplets of CM-CF-CA can't mess with permissions of normal CAs.
  • Loading branch information
lekhovitsky committed Jan 15, 2025
1 parent d50b662 commit 229d072
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 32 deletions.
26 changes: 8 additions & 18 deletions contracts/core/BotListV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ contract BotListV3 is IBotListV3, SanityCheckTrait, Ownable {
/// @notice Contract type
bytes32 public constant override contractType = "BOT_LIST";

/// @notice Credit manager's approved status
mapping(address => bool) public override approvedCreditManager;

/// @dev Mapping bot => info
mapping(address => BotInfo) internal _botInfo;

Expand Down Expand Up @@ -77,7 +74,7 @@ contract BotListV3 is IBotListV3, SanityCheckTrait, Ownable {

/// @notice Sets `bot`'s permissions for `creditAccount` in its credit manager to `permissions`
/// @return activeBotsRemaining Number of bots with non-zero permissions remaining after the update
/// @dev Reverts if `creditAccount`'s credit manager is not approved or caller is not a facade connected to it
/// @dev Reverts if `creditAccount` is not opened in its credit manager or caller is not a facade connected to it
/// @dev Reverts if trying to set non-zero permissions that don't meet bot's requirements
/// @dev Reverts if trying to set non-zero permissions for a forbidden bot
/// @custom:tests U:[BL-1]
Expand All @@ -88,7 +85,7 @@ contract BotListV3 is IBotListV3, SanityCheckTrait, Ownable {
returns (uint256 activeBotsRemaining)
{
address creditManager = ICreditAccountV3(creditAccount).creditManager();
_revertIfCallerNotValidCreditFacade(creditManager);
_validateCreditAccountAndCaller(creditManager, creditAccount);

BotInfo storage info = _botInfo[bot];
EnumerableSet.AddressSet storage accountBots = _activeBots[creditManager][creditAccount];
Expand All @@ -108,11 +105,11 @@ contract BotListV3 is IBotListV3, SanityCheckTrait, Ownable {
}

/// @notice Removes all bots' permissions for `creditAccount` in its credit manager
/// @dev Reverts if `creditAccount`'s credit manager is not approved or caller is not a facade connected to it
/// @dev Reverts if `creditAccount` is not opened in its credit manager or caller is not a facade connected to it
/// @custom:tests U:[BL-2]
function eraseAllBotPermissions(address creditAccount) external override {
address creditManager = ICreditAccountV3(creditAccount).creditManager();
_revertIfCallerNotValidCreditFacade(creditManager);
_validateCreditAccountAndCaller(creditManager, creditAccount);

EnumerableSet.AddressSet storage accountBots = _activeBots[creditManager][creditAccount];
unchecked {
Expand Down Expand Up @@ -143,21 +140,14 @@ contract BotListV3 is IBotListV3, SanityCheckTrait, Ownable {
}
}

/// @notice Approves `creditManager`
function approveCreditManager(address creditManager) external override onlyOwner {
if (!approvedCreditManager[creditManager]) {
approvedCreditManager[creditManager] = true;
emit ApproveCreditManager(creditManager);
}
}

// --------- //
// INTERNALS //
// --------- //

/// @dev Reverts if `creditManager` is not approved or caller is not a facade connected to it
function _revertIfCallerNotValidCreditFacade(address creditManager) internal view {
if (!approvedCreditManager[creditManager] || ICreditManagerV3(creditManager).creditFacade() != msg.sender) {
/// @dev Reverts if `creditAccount` is not opened in `creditManager` or caller is not a facade connected to it
function _validateCreditAccountAndCaller(address creditManager, address creditAccount) internal view {
ICreditManagerV3(creditManager).getBorrowerOrRevert(creditAccount);
if (ICreditManagerV3(creditManager).creditFacade() != msg.sender) {
revert CallerNotCreditFacadeException();
}
}
Expand Down
7 changes: 0 additions & 7 deletions contracts/interfaces/IBotListV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ interface IBotListV3Events {

/// @notice Emitted when `bot` is forbidden
event ForbidBot(address indexed bot);

/// @notice Emitted when `creditManager` is approved
event ApproveCreditManager(address indexed creditManager);
}

/// @title Bot list V3 interface
Expand Down Expand Up @@ -61,9 +58,5 @@ interface IBotListV3 is IBotListV3Events, IVersion {

function botForbiddenStatus(address bot) external view returns (bool);

function approvedCreditManager(address creditManager) external view returns (bool);

function forbidBot(address bot) external;

function approveCreditManager(address creditManager) external;
}
3 changes: 0 additions & 3 deletions contracts/test/helpers/IntegrationTestHelper.sol
Original file line number Diff line number Diff line change
Expand Up @@ -463,9 +463,6 @@ contract IntegrationTestHelper is TestHelper, BalanceHelper, ConfigManager {
vm.prank(CONFIGURATOR);
pool.setCreditManagerDebtLimit(address(creditManager), cmParams.poolLimit);

vm.prank(CONFIGURATOR);
botList.approveCreditManager(address(creditManager));

vm.label(address(creditFacade), "CreditFacadeV3");
vm.label(address(creditManager), "CreditManagerV3");
vm.label(address(creditConfigurator), "CreditConfiguratorV3");
Expand Down
26 changes: 22 additions & 4 deletions contracts/test/unit/core/BotListV3.unit.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ contract BotListV3UnitTest is Test, IBotListV3Events {
address creditFacade;
address creditAccount;
address invalidFacade;
address invalidAccount;

function setUp() public {
bot = address(new BotMock());
Expand All @@ -34,19 +35,32 @@ contract BotListV3UnitTest is Test, IBotListV3Events {
creditFacade = makeAddr("CREDIT_FACADE");
creditAccount = makeAddr("CREDIT_ACCOUNT");
invalidFacade = makeAddr("INVALID_FACADE");
invalidAccount = makeAddr("INVALID_ACCOUNT");

vm.mockCall(creditManager, abi.encodeWithSignature("creditFacade()"), abi.encode(creditFacade));
vm.mockCall(creditFacade, abi.encodeWithSignature("creditManager()"), abi.encode(creditManager));
vm.mockCall(invalidFacade, abi.encodeWithSignature("creditManager()"), abi.encode(creditManager));
vm.mockCall(creditAccount, abi.encodeWithSignature("creditManager()"), abi.encode(creditManager));
vm.mockCall(
creditManager,
abi.encodeWithSignature("getBorrowerOrRevert(address)", creditAccount),
abi.encode(makeAddr("borrower"))
);

vm.mockCall(invalidAccount, abi.encodeWithSignature("creditManager()"), abi.encode(creditManager));
vm.mockCallRevert(
creditManager,
abi.encodeWithSignature("getBorrowerOrRevert(address)", invalidAccount),
abi.encodeWithSignature("CreditAccountDoesNotExistException()")
);

botList = new BotListV3(CONFIGURATOR);
vm.prank(CONFIGURATOR);
botList.approveCreditManager(creditManager);
}

/// @notice U:[BL-1]: `setBotPermissions` works correctly
function test_U_BL_01_setBotPermissions_works_correctly() public {
vm.expectRevert(CreditAccountDoesNotExistException.selector);
vm.prank(creditFacade);
botList.setBotPermissions({bot: bot, creditAccount: invalidAccount, permissions: type(uint192).max});

vm.expectRevert(CallerNotCreditFacadeException.selector);
vm.prank(invalidFacade);
botList.setBotPermissions({bot: bot, creditAccount: creditAccount, permissions: type(uint192).max});
Expand Down Expand Up @@ -120,6 +134,10 @@ contract BotListV3UnitTest is Test, IBotListV3Events {

assertEq(activeBotsRemaining, 2, "Incorrect number of active bots");

vm.expectRevert(CreditAccountDoesNotExistException.selector);
vm.prank(creditFacade);
botList.eraseAllBotPermissions(invalidAccount);

vm.expectRevert(CallerNotCreditFacadeException.selector);
vm.prank(invalidFacade);
botList.eraseAllBotPermissions(creditAccount);
Expand Down

0 comments on commit 229d072

Please sign in to comment.