From 97a88dbb7d435b275186d732e936f033a12fa090 Mon Sep 17 00:00:00 2001 From: Dima Lekhovitsky Date: Tue, 23 Apr 2024 17:44:46 +0300 Subject: [PATCH] feat: `BotListV3` improvements (#199) In this PR: * new `IBotV3` interface that defines bot's required permissions which are checked in the bot list * special permission bots are deprecated * `BotListV3` interface changes: * redundant `creditManager` parameter removed from most functions that operate on `creditAccount` * `EraseBot` event removed in favor of `SetBotPermissions(..., 0)` * minor implementation improvements that make code more consistent * increase `BotListV3` version to `3_10` --- contracts/core/BotListV3.sol | 128 ++++++----------- contracts/credit/CreditFacadeV3.sol | 33 ++--- contracts/interfaces/IBotListV3.sol | 31 ++-- contracts/interfaces/IBotV3.sol | 11 ++ contracts/interfaces/IExceptions.sol | 7 +- .../test/helpers/IntegrationTestHelper.sol | 2 +- .../test/integration/credit/Bots.int.sol | 22 +-- .../credit/CloseCreditAccount.int.sol | 10 +- .../credit/CreditConfigurator.int.t.sol | 2 +- .../credit/LiquidateCreditAccount.int.t.sol | 17 +-- .../mocks/core/AddressProviderV3ACLMock.sol | 2 +- contracts/test/mocks/core/BotListMock.sol | 19 +-- contracts/test/mocks/core/BotMock.sol | 14 ++ contracts/test/unit/core/BotListV3.unit.t.sol | 136 +++++------------- .../unit/credit/CreditFacadeV3.unit.t.sol | 29 ++-- 15 files changed, 157 insertions(+), 306 deletions(-) create mode 100644 contracts/interfaces/IBotV3.sol create mode 100644 contracts/test/mocks/core/BotMock.sol diff --git a/contracts/core/BotListV3.sol b/contracts/core/BotListV3.sol index 4d252a7f..28d710b5 100644 --- a/contracts/core/BotListV3.sol +++ b/contracts/core/BotListV3.sol @@ -1,16 +1,18 @@ // SPDX-License-Identifier: BUSL-1.1 // Gearbox Protocol. Generalized leverage for DeFi protocols -// (c) Gearbox Foundation, 2023. +// (c) Gearbox Foundation, 2024. pragma solidity ^0.8.17; -import {Address} from "@openzeppelin/contracts/utils/Address.sol"; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; import {IBotListV3, BotInfo} from "../interfaces/IBotListV3.sol"; +import {IBotV3} from "../interfaces/IBotV3.sol"; +import {ICreditAccountBase} from "../interfaces/ICreditAccountV3.sol"; import {ICreditManagerV3} from "../interfaces/ICreditManagerV3.sol"; import { AddressIsNotContractException, CallerNotCreditFacadeException, + InsufficientBotPermissionsException, InvalidBotException } from "../interfaces/IExceptions.sol"; @@ -19,15 +21,11 @@ import {ContractsRegisterTrait} from "../traits/ContractsRegisterTrait.sol"; /// @title Bot list V3 /// @notice Stores bot permissions (bit masks dictating which actions can be performed with credit accounts in multicall). -/// Besides normal per-account permissions, there are special per-manager permissions that apply to all accounts -/// in a given credit manager and can be used to extend the core system or enforce additional safety measures -/// with special DAO-approved bots. contract BotListV3 is ACLNonReentrantTrait, ContractsRegisterTrait, IBotListV3 { - using Address for address; using EnumerableSet for EnumerableSet.AddressSet; /// @notice Contract version - uint256 public constant override version = 3_00; + uint256 public constant override version = 3_10; /// @notice Credit manager's approved status mapping(address => bool) public override approvedCreditManager; @@ -38,12 +36,6 @@ contract BotListV3 is ACLNonReentrantTrait, ContractsRegisterTrait, IBotListV3 { /// @dev Mapping credit manager => credit account => set of bots with non-zero permissions mapping(address => mapping(address => EnumerableSet.AddressSet)) internal _activeBots; - /// @dev Ensures that function can only be called by a facade connected to approved `creditManager` - modifier onlyValidCreditFacade(address creditManager) { - _revertIfCallerNotValidCreditFacade(creditManager); - _; - } - /// @notice Constructor /// @param addressProvider Address provider contract address constructor(address addressProvider) @@ -55,88 +47,78 @@ contract BotListV3 is ACLNonReentrantTrait, ContractsRegisterTrait, IBotListV3 { // PERMISSIONS // // ----------- // - /// @notice Returns `bot`'s permissions for `creditAccount` in `creditManager` - function botPermissions(address bot, address creditManager, address creditAccount) - external - view - override - returns (uint192) - { + /// @notice Returns `bot`'s permissions for `creditAccount` in its credit manager + function botPermissions(address bot, address creditAccount) external view override returns (uint192) { + address creditManager = ICreditAccountBase(creditAccount).creditManager(); return _botInfo[bot].permissions[creditManager][creditAccount]; } - /// @notice Returns all bots with non-zero permissions for `creditAccount` in `creditManager` - function activeBots(address creditManager, address creditAccount) - external - view - override - returns (address[] memory) - { + /// @notice Returns all bots with non-zero permissions for `creditAccount` in its credit manager + function activeBots(address creditAccount) external view override returns (address[] memory) { + address creditManager = ICreditAccountBase(creditAccount).creditManager(); return _activeBots[creditManager][creditAccount].values(); } - /// @notice Returns `bot`'s permissions for `creditAccount` in `creditManager`, including information - /// on whether bot is forbidden or has special permissions in the credit manager - function getBotStatus(address bot, address creditManager, address creditAccount) + /// @notice Returns `bot`'s permissions for `creditAccount` in its credit manager and whether it is forbidden + function getBotStatus(address bot, address creditAccount) external view override - returns (uint192 permissions, bool forbidden, bool hasSpecialPermissions) + returns (uint192 permissions, bool forbidden) { BotInfo storage info = _botInfo[bot]; - if (info.forbidden) return (0, true, false); + if (info.forbidden) return (0, true); - uint192 specialPermissions = info.specialPermissions[creditManager]; - if (specialPermissions != 0) return (specialPermissions, false, true); - - return (info.permissions[creditManager][creditAccount], false, false); + address creditManager = ICreditAccountBase(creditAccount).creditManager(); + return (info.permissions[creditManager][creditAccount], false); } - /// @notice Sets `bot`'s permissions for `creditAccount` in `creditManager` to `permissions` + /// @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 caller is not a facade connected to approved `creditManager` - /// @dev Reverts if `bot` is zero address or not a contract - /// @dev Reverts if trying to set non-zero permissions for a forbidden bot or for a bot with special permissions - function setBotPermissions(address bot, address creditManager, address creditAccount, uint192 permissions) + /// @dev Reverts if `creditAccount`'s credit manager is not approved 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] + function setBotPermissions(address bot, address creditAccount, uint192 permissions) external override nonZeroAddress(bot) - onlyValidCreditFacade(creditManager) returns (uint256 activeBotsRemaining) { - if (!bot.isContract()) revert AddressIsNotContractException(bot); + address creditManager = ICreditAccountBase(creditAccount).creditManager(); + _revertIfCallerNotValidCreditFacade(creditManager); + BotInfo storage info = _botInfo[bot]; EnumerableSet.AddressSet storage accountBots = _activeBots[creditManager][creditAccount]; - if (permissions != 0) { - BotInfo storage info = _botInfo[bot]; - if (info.forbidden || info.specialPermissions[creditManager] != 0) { - revert InvalidBotException(); - } - + if (IBotV3(bot).requiredPermissions() & ~permissions != 0) revert InsufficientBotPermissionsException(); + if (info.forbidden) revert InvalidBotException(); accountBots.add(bot); - info.permissions[creditManager][creditAccount] = permissions; - emit SetBotPermissions(bot, creditManager, creditAccount, permissions); } else { - _eraseBot(bot, creditManager, creditAccount); accountBots.remove(bot); } - activeBotsRemaining = accountBots.length(); + + if (info.permissions[creditManager][creditAccount] != permissions) { + info.permissions[creditManager][creditAccount] = permissions; + emit SetBotPermissions(bot, creditManager, creditAccount, permissions); + } } - /// @notice Removes all bots' permissions for `creditAccount` in `creditManager` - function eraseAllBotPermissions(address creditManager, address creditAccount) - external - override - onlyValidCreditFacade(creditManager) - { + /// @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 + /// @custom:tests U:[BL-2] + function eraseAllBotPermissions(address creditAccount) external override { + address creditManager = ICreditAccountBase(creditAccount).creditManager(); + _revertIfCallerNotValidCreditFacade(creditManager); + EnumerableSet.AddressSet storage accountBots = _activeBots[creditManager][creditAccount]; unchecked { for (uint256 i = accountBots.length(); i != 0; --i) { address bot = accountBots.at(i - 1); - _eraseBot(bot, creditManager, creditAccount); accountBots.remove(bot); + _botInfo[bot].permissions[creditManager][creditAccount] = 0; + emit SetBotPermissions(bot, creditManager, creditAccount, 0); } } } @@ -150,11 +132,6 @@ contract BotListV3 is ACLNonReentrantTrait, ContractsRegisterTrait, IBotListV3 { return _botInfo[bot].forbidden; } - /// @notice Returns `bot`'s special permissions in `creditManager` - function botSpecialPermissions(address bot, address creditManager) external view override returns (uint192) { - return _botInfo[bot].specialPermissions[creditManager]; - } - /// @notice Sets `bot`'s status to `forbidden` function setBotForbiddenStatus(address bot, bool forbidden) external override configuratorOnly { BotInfo storage info = _botInfo[bot]; @@ -164,19 +141,6 @@ contract BotListV3 is ACLNonReentrantTrait, ContractsRegisterTrait, IBotListV3 { } } - /// @notice Sets `bot`'s special permissions in `creditManager` to `permissions` - function setBotSpecialPermissions(address bot, address creditManager, uint192 permissions) - external - override - configuratorOnly - { - BotInfo storage info = _botInfo[bot]; - if (info.specialPermissions[creditManager] != permissions) { - info.specialPermissions[creditManager] = permissions; - emit SetBotSpecialPermissions(bot, creditManager, permissions); - } - } - /// @notice Sets `creditManager`'s status to `approved` function setCreditManagerApprovedStatus(address creditManager, bool approved) external @@ -194,16 +158,10 @@ contract BotListV3 is ACLNonReentrantTrait, ContractsRegisterTrait, IBotListV3 { // INTERNALS // // --------- // - /// @dev Reverts if `creditManager` is not approved or caller is not a facade connected to `creditManager` + /// @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) { revert CallerNotCreditFacadeException(); } } - - /// @dev Removes `bot`'s permissions for `creditAccount` in `creditManager` - function _eraseBot(address bot, address creditManager, address creditAccount) internal { - delete _botInfo[bot].permissions[creditManager][creditAccount]; - emit EraseBot(bot, creditManager, creditAccount); - } } diff --git a/contracts/credit/CreditFacadeV3.sol b/contracts/credit/CreditFacadeV3.sol index f9514fd2..c4eafd1c 100644 --- a/contracts/credit/CreditFacadeV3.sol +++ b/contracts/credit/CreditFacadeV3.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: BUSL-1.1 // Gearbox Protocol. Generalized leverage for DeFi protocols -// (c) Gearbox Foundation, 2023. +// (c) Gearbox Foundation, 2024. pragma solidity ^0.8.17; // THIRD-PARTY @@ -72,7 +72,7 @@ contract CreditFacadeV3 is ICreditFacadeV3, ACLNonReentrantTrait { using SafeERC20 for IERC20; /// @notice Contract version - uint256 public constant override version = 3_01; + uint256 public constant override version = 3_10; /// @notice Maximum quota size, as a multiple of `maxDebt` uint256 public constant override maxQuotaMultiplier = 2; @@ -157,7 +157,7 @@ contract CreditFacadeV3 is ICreditFacadeV3, ACLNonReentrantTrait { address addressProvider = ICreditManagerV3(_creditManager).addressProvider(); weth = IAddressProviderV3(addressProvider).getAddressOrRevert(AP_WETH_TOKEN, NO_VERSION_CONTROL); // U:[FA-1] - botList = IAddressProviderV3(addressProvider).getAddressOrRevert(AP_BOT_LIST, 3_00); // U:[FA-1] + botList = IAddressProviderV3(addressProvider).getAddressOrRevert(AP_BOT_LIST, 3_10); // U:[FA-1] degenNFT = _degenNFT; // U:[FA-1] @@ -255,7 +255,7 @@ contract CreditFacadeV3 is ICreditFacadeV3, ACLNonReentrantTrait { if (enabledTokensMask != 0) revert CloseAccountWithEnabledTokensException(); // U:[FA-11] if (_flagsOf(creditAccount) & BOT_PERMISSIONS_SET_FLAG != 0) { - IBotListV3(botList).eraseAllBotPermissions(creditManager, creditAccount); // U:[FA-11] + IBotListV3(botList).eraseAllBotPermissions(creditAccount); // U:[FA-11] } ICreditManagerV3(creditManager).closeCreditAccount(creditAccount); // U:[FA-11] @@ -370,8 +370,7 @@ contract CreditFacadeV3 is ICreditFacadeV3, ACLNonReentrantTrait { } /// @notice Executes a batch of calls allowing bot to manage a credit account - /// - Performs a multicall (allowed calls are determined by permissions given by account's owner - /// or by DAO in case bot has special permissions in the credit manager) + /// - Performs a multicall (allowed calls are determined by permissions given by account's owner) /// - Runs the collateral check /// @param creditAccount Account to perform the calls on /// @param calls List of calls to perform @@ -387,16 +386,10 @@ contract CreditFacadeV3 is ICreditFacadeV3, ACLNonReentrantTrait { { _getBorrowerOrRevert(creditAccount); // U:[FA-5] - (uint256 botPermissions, bool forbidden, bool hasSpecialPermissions) = IBotListV3(botList).getBotStatus({ - bot: msg.sender, - creditManager: creditManager, - creditAccount: creditAccount - }); + (uint256 botPermissions, bool forbidden) = + IBotListV3(botList).getBotStatus({bot: msg.sender, creditAccount: creditAccount}); - if ( - botPermissions == 0 || forbidden - || (!hasSpecialPermissions && (_flagsOf(creditAccount) & BOT_PERMISSIONS_SET_FLAG == 0)) - ) { + if (forbidden || botPermissions == 0 || _flagsOf(creditAccount) & BOT_PERMISSIONS_SET_FLAG == 0) { revert NotApprovedBotException(); // U:[FA-19] } @@ -408,7 +401,7 @@ contract CreditFacadeV3 is ICreditFacadeV3, ACLNonReentrantTrait { /// @param bot Bot to set permissions for /// @param permissions A bit mask encoding bot permissions /// @dev Reverts if `creditAccount` is not opened in connected credit manager by caller - /// @dev Reverts if `permissions` has unexpected bits enabled + /// @dev Reverts if `permissions` has unexpected bits enabled or some bits required by `bot` disabled /// @dev Reverts if account has more active bots than allowed after changing permissions /// @dev Changes account's `BOT_PERMISSIONS_SET_FLAG` in the credit manager if needed function setBotPermissions(address creditAccount, address bot, uint192 permissions) @@ -419,12 +412,8 @@ contract CreditFacadeV3 is ICreditFacadeV3, ACLNonReentrantTrait { { if (permissions & ~ALL_PERMISSIONS != 0) revert UnexpectedPermissionsException(); // U:[FA-41] - uint256 remainingBots = IBotListV3(botList).setBotPermissions({ - bot: bot, - creditManager: creditManager, - creditAccount: creditAccount, - permissions: permissions - }); // U:[FA-41] + uint256 remainingBots = + IBotListV3(botList).setBotPermissions({bot: bot, creditAccount: creditAccount, permissions: permissions}); // U:[FA-41] if (remainingBots == 0) { _setFlagFor({creditAccount: creditAccount, flag: BOT_PERMISSIONS_SET_FLAG, value: false}); // U:[FA-41] diff --git a/contracts/interfaces/IBotListV3.sol b/contracts/interfaces/IBotListV3.sol index 0a13b9ae..50d66591 100644 --- a/contracts/interfaces/IBotListV3.sol +++ b/contracts/interfaces/IBotListV3.sol @@ -1,17 +1,15 @@ // SPDX-License-Identifier: MIT // Gearbox Protocol. Generalized leverage for DeFi protocols -// (c) Gearbox Foundation, 2023. +// (c) Gearbox Foundation, 2024. pragma solidity ^0.8.17; import {IVersion} from "@gearbox-protocol/core-v2/contracts/interfaces/IVersion.sol"; /// @notice Bot info /// @param forbidden Whether bot is forbidden -/// @param specialPermissions Mapping credit manager => bot's special permissions /// @param permissions Mapping credit manager => credit account => bot's permissions struct BotInfo { bool forbidden; - mapping(address => uint192) specialPermissions; mapping(address => mapping(address => uint192)) permissions; } @@ -20,14 +18,11 @@ interface IBotListV3Events { // PERMISSIONS // // ----------- // - /// @notice Emitted when new `bot`'s permissions and funding params are set for `creditAccount` in `creditManager` + /// @notice Emitted when new `bot`'s permissions are set for `creditAccount` in `creditManager` event SetBotPermissions( address indexed bot, address indexed creditManager, address indexed creditAccount, uint192 permissions ); - /// @notice Emitted when `bot`'s permissions and funding params are removed for `creditAccount` in `creditManager` - event EraseBot(address indexed bot, address indexed creditManager, address indexed creditAccount); - // ------------- // // CONFIGURATION // // ------------- // @@ -35,9 +30,6 @@ interface IBotListV3Events { /// @notice Emitted when `bot`'s forbidden status is set event SetBotForbiddenStatus(address indexed bot, bool forbidden); - /// @notice Emitted when `bot`'s special permissions in `creditManager` are set - event SetBotSpecialPermissions(address indexed bot, address indexed creditManager, uint192 permissions); - /// @notice Emitted when `creditManager`'s approved status is set event SetCreditManagerApprovedStatus(address indexed creditManager, bool approved); } @@ -48,23 +40,20 @@ interface IBotListV3 is IBotListV3Events, IVersion { // PERMISSIONS // // ----------- // - function botPermissions(address bot, address creditManager, address creditAccount) - external - view - returns (uint192); + function botPermissions(address bot, address creditAccount) external view returns (uint192); - function activeBots(address creditManager, address creditAccount) external view returns (address[] memory); + function activeBots(address creditAccount) external view returns (address[] memory); - function getBotStatus(address bot, address creditManager, address creditAccount) + function getBotStatus(address bot, address creditAccount) external view - returns (uint192 permissions, bool forbidden, bool hasSpecialPermissions); + returns (uint192 permissions, bool forbidden); - function setBotPermissions(address bot, address creditManager, address creditAccount, uint192 permissions) + function setBotPermissions(address bot, address creditAccount, uint192 permissions) external returns (uint256 activeBotsRemaining); - function eraseAllBotPermissions(address creditManager, address creditAccount) external; + function eraseAllBotPermissions(address creditAccount) external; // ------------- // // CONFIGURATION // @@ -72,13 +61,9 @@ interface IBotListV3 is IBotListV3Events, IVersion { function botForbiddenStatus(address bot) external view returns (bool); - function botSpecialPermissions(address bot, address creditManager) external view returns (uint192); - function approvedCreditManager(address creditManager) external view returns (bool); function setBotForbiddenStatus(address bot, bool forbidden) external; - function setBotSpecialPermissions(address bot, address creditManager, uint192 permissions) external; - function setCreditManagerApprovedStatus(address creditManager, bool approved) external; } diff --git a/contracts/interfaces/IBotV3.sol b/contracts/interfaces/IBotV3.sol new file mode 100644 index 00000000..5a65101c --- /dev/null +++ b/contracts/interfaces/IBotV3.sol @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: MIT +// Gearbox Protocol. Generalized leverage for DeFi protocols +// (c) Gearbox Foundation, 2024. +pragma solidity ^0.8.17; + +/// @title Bot V3 interface +/// @notice Minimal interface contracts must conform to in order to be used as bots in Gearbox V3 +interface IBotV3 { + /// @notice Mask of permissions required for bot operation, see `ICreditFacadeV3Multicall` + function requiredPermissions() external view returns (uint192); +} diff --git a/contracts/interfaces/IExceptions.sol b/contracts/interfaces/IExceptions.sol index 7b9a25c4..aa364102 100644 --- a/contracts/interfaces/IExceptions.sol +++ b/contracts/interfaces/IExceptions.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT // Gearbox Protocol. Generalized leverage for DeFi protocols -// (c) Gearbox Foundation, 2023. +// (c) Gearbox Foundation, 2024. pragma solidity ^0.8.17; // ------- // @@ -286,9 +286,12 @@ error ParameterChangedAfterQueuedTxException(); // BOT LIST // // -------- // -/// @notice Thrown when attempting to set non-zero permissions for a forbidden or special bot +/// @notice Thrown when attempting to set non-zero permissions for a forbidden bot error InvalidBotException(); +/// @notice Thrown when attempting to set permissions for a bot that don't meet its requirements +error InsufficientBotPermissionsException(); + // --------------- // // ACCOUNT FACTORY // // --------------- // diff --git a/contracts/test/helpers/IntegrationTestHelper.sol b/contracts/test/helpers/IntegrationTestHelper.sol index a7c34be0..7ff9c2f2 100644 --- a/contracts/test/helpers/IntegrationTestHelper.sol +++ b/contracts/test/helpers/IntegrationTestHelper.sol @@ -294,7 +294,7 @@ contract IntegrationTestHelper is TestHelper, BalanceHelper, ConfigManager { cr = ContractsRegister(addressProvider.getAddressOrRevert(AP_CONTRACTS_REGISTER, NO_VERSION_CONTROL)); accountFactory = AccountFactory(addressProvider.getAddressOrRevert(AP_ACCOUNT_FACTORY, NO_VERSION_CONTROL)); priceOracle = IPriceOracleV3(addressProvider.getAddressOrRevert(AP_PRICE_ORACLE, 3_00)); - botList = BotListV3(payable(addressProvider.getAddressOrRevert(AP_BOT_LIST, 3_00))); + botList = BotListV3(payable(addressProvider.getAddressOrRevert(AP_BOT_LIST, 3_10))); } function _attachPool(address _pool) internal returns (bool isCompartible) { diff --git a/contracts/test/integration/credit/Bots.int.sol b/contracts/test/integration/credit/Bots.int.sol index de12e9a9..19ce5c08 100644 --- a/contracts/test/integration/credit/Bots.int.sol +++ b/contracts/test/integration/credit/Bots.int.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: UNLICENSED // Gearbox Protocol. Generalized leverage for DeFi protocols -// (c) Gearbox Foundation, 2023. +// (c) Gearbox Foundation, 2024. pragma solidity ^0.8.17; import {CreditManagerV3} from "../../../credit/CreditManagerV3.sol"; @@ -34,9 +34,8 @@ import "../../lib/constants.sol"; import "../../../interfaces/IExceptions.sol"; // MOCKS -import {AdapterMock} from "../../mocks//core/AdapterMock.sol"; - -import {GeneralMock} from "../../mocks//GeneralMock.sol"; +import {AdapterMock} from "../../mocks/core/AdapterMock.sol"; +import {BotMock} from "../../mocks/core/BotMock.sol"; // SUITES @@ -51,12 +50,12 @@ contract BotsIntegrationTest is IntegrationTestHelper, ICreditFacadeV3Events { function test_I_BOT_01_botMulticall_works_correctly() public withAdapterMock creditTest { (address creditAccount,) = _openTestCreditAccount(); - address bot = address(new GeneralMock()); + address bot = address(new BotMock()); bytes memory DUMB_CALLDATA = adapterMock.dumbCallData(); vm.prank(address(creditFacade)); - botList.setBotPermissions(bot, address(creditManager), creditAccount, uint192(ALL_PERMISSIONS)); + botList.setBotPermissions(bot, creditAccount, uint192(ALL_PERMISSIONS)); vm.expectRevert(NotApprovedBotException.selector); creditFacade.botMulticall( @@ -67,17 +66,10 @@ contract BotsIntegrationTest is IntegrationTestHelper, ICreditFacadeV3Events { MultiCall({target: address(adapterMock), callData: abi.encodeCall(AdapterMock.dumbCall, (0, 0))}) ); - vm.prank(CONFIGURATOR); - botList.setBotSpecialPermissions(address(bot), address(creditManager), type(uint192).max); - vm.prank(bot); - creditFacade.botMulticall(creditAccount, calls); - vm.prank(CONFIGURATOR); - botList.setBotSpecialPermissions(address(bot), address(creditManager), 0); - vm.prank(USER); creditFacade.setBotPermissions(creditAccount, bot, uint192(ALL_PERMISSIONS)); - botList.getBotStatus({creditManager: address(creditManager), creditAccount: creditAccount, bot: bot}); + botList.getBotStatus({creditAccount: creditAccount, bot: bot}); vm.expectEmit(true, true, false, true); emit StartMultiCall({creditAccount: creditAccount, caller: bot}); @@ -120,7 +112,7 @@ contract BotsIntegrationTest is IntegrationTestHelper, ICreditFacadeV3Events { function test_I_BOT_02_setBotPermissions_works_correctly() public creditTest { (address creditAccount,) = _openTestCreditAccount(); - address bot = address(new GeneralMock()); + address bot = address(new BotMock()); vm.expectRevert(CallerNotCreditAccountOwnerException.selector); vm.prank(FRIEND); diff --git a/contracts/test/integration/credit/CloseCreditAccount.int.sol b/contracts/test/integration/credit/CloseCreditAccount.int.sol index e236f5f5..a7b957ac 100644 --- a/contracts/test/integration/credit/CloseCreditAccount.int.sol +++ b/contracts/test/integration/credit/CloseCreditAccount.int.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: UNLICENSED // Gearbox Protocol. Generalized leverage for DeFi protocols -// (c) Gearbox Foundation, 2023. +// (c) Gearbox Foundation, 2024. pragma solidity ^0.8.17; import "../../../interfaces/IAddressProviderV3.sol"; @@ -39,7 +39,7 @@ import "../../../interfaces/IExceptions.sol"; // MOCKS import {AdapterMock} from "../../mocks/core/AdapterMock.sol"; import {PriceFeedMock} from "../../mocks/oracles/PriceFeedMock.sol"; -import {GeneralMock} from "../../mocks/GeneralMock.sol"; +import {BotMock} from "../../mocks/core/BotMock.sol"; // SUITES @@ -163,7 +163,7 @@ contract CloseCreditAccountIntegrationTest is IntegrationTestHelper, ICreditFaca MultiCall({target: address(adapterMock), callData: abi.encodeCall(AdapterMock.dumbCall, (0, 0))}) ); - address bot = address(new GeneralMock()); + address bot = address(new BotMock()); vm.prank(USER); creditFacade.setBotPermissions({ @@ -193,9 +193,7 @@ contract CloseCreditAccountIntegrationTest is IntegrationTestHelper, ICreditFaca vm.expectEmit(false, false, false, true); emit FinishMultiCall(); - vm.expectCall( - address(botList), abi.encodeCall(BotListV3.eraseAllBotPermissions, (address(creditManager), creditAccount)) - ); + vm.expectCall(address(botList), abi.encodeCall(BotListV3.eraseAllBotPermissions, (creditAccount))); vm.expectEmit(true, true, false, false); emit CloseCreditAccount(creditAccount, USER); diff --git a/contracts/test/integration/credit/CreditConfigurator.int.t.sol b/contracts/test/integration/credit/CreditConfigurator.int.t.sol index 6907128a..80d4c6ab 100644 --- a/contracts/test/integration/credit/CreditConfigurator.int.t.sol +++ b/contracts/test/integration/credit/CreditConfigurator.int.t.sol @@ -969,7 +969,7 @@ contract CreditConfiguratorIntegrationTest is IntegrationTestHelper, ICreditConf assertEq( botList2, - migrateSettings ? botList : addressProvider.getAddressOrRevert(AP_BOT_LIST, 300), + migrateSettings ? botList : addressProvider.getAddressOrRevert(AP_BOT_LIST, 3_10), "Bot list was not transferred" ); diff --git a/contracts/test/integration/credit/LiquidateCreditAccount.int.t.sol b/contracts/test/integration/credit/LiquidateCreditAccount.int.t.sol index a0a5b3d2..49486b6e 100644 --- a/contracts/test/integration/credit/LiquidateCreditAccount.int.t.sol +++ b/contracts/test/integration/credit/LiquidateCreditAccount.int.t.sol @@ -1,16 +1,10 @@ // SPDX-License-Identifier: UNLICENSED // Gearbox Protocol. Generalized leverage for DeFi protocols -// (c) Gearbox Foundation, 2023. +// (c) Gearbox Foundation, 2024. pragma solidity ^0.8.17; -import {BotListV3} from "../../../core/BotListV3.sol"; import {ICreditAccountBase} from "../../../interfaces/ICreditAccountV3.sol"; -import { - ICreditManagerV3, - ICreditManagerV3Events, - ManageDebtAction, - BOT_PERMISSIONS_SET_FLAG -} from "../../../interfaces/ICreditManagerV3.sol"; +import {ICreditManagerV3, ICreditManagerV3Events, ManageDebtAction} from "../../../interfaces/ICreditManagerV3.sol"; import "../../../interfaces/ICreditFacadeV3.sol"; import {MultiCallBuilder} from "../../lib/MultiCallBuilder.sol"; @@ -53,13 +47,6 @@ contract LiquidateCreditAccountIntegrationTest is IntegrationTestHelper, ICredit bytes memory DUMB_CALLDATA = adapterMock.dumbCallData(); - vm.prank(USER); - creditFacade.setBotPermissions({ - creditAccount: creditAccount, - bot: address(adapterMock), - permissions: uint192(ADD_COLLATERAL_PERMISSION) - }); - MultiCall[] memory calls = MultiCallBuilder.build( MultiCall({target: address(adapterMock), callData: abi.encodeCall(AdapterMock.dumbCall, (0, 0))}) ); diff --git a/contracts/test/mocks/core/AddressProviderV3ACLMock.sol b/contracts/test/mocks/core/AddressProviderV3ACLMock.sol index 634163c1..434639ec 100644 --- a/contracts/test/mocks/core/AddressProviderV3ACLMock.sol +++ b/contracts/test/mocks/core/AddressProviderV3ACLMock.sol @@ -34,7 +34,7 @@ contract AddressProviderV3ACLMock is Test, AddressProviderV3 { _setAddress(AP_ACCOUNT_FACTORY, address(accountFactoryMock), NO_VERSION_CONTROL); BotListMock botListMock = new BotListMock(); - _setAddress(AP_BOT_LIST, address(botListMock), 3_00); + _setAddress(AP_BOT_LIST, address(botListMock), 3_10); _setAddress(AP_CONTRACTS_REGISTER, address(this), 0); diff --git a/contracts/test/mocks/core/BotListMock.sol b/contracts/test/mocks/core/BotListMock.sol index 75a22cf0..01813c00 100644 --- a/contracts/test/mocks/core/BotListMock.sol +++ b/contracts/test/mocks/core/BotListMock.sol @@ -8,27 +8,20 @@ contract BotListMock { uint256 return_botPermissions; bool return_forbidden; - bool return_hasSpecialPermissions; uint256 return_activeBotsRemaining; - function setBotStatusReturns(uint256 botPermissions, bool forbidden, bool hasSpecialPermissions) external { + function setBotStatusReturns(uint256 botPermissions, bool forbidden) external { return_botPermissions = botPermissions; return_forbidden = forbidden; - return_hasSpecialPermissions = hasSpecialPermissions; } - function getBotStatus(address, address, address) - external - view - returns (uint256 botPermissions, bool forbidden, bool hasSpecialPermissions) - { + function getBotStatus(address, address) external view returns (uint256 botPermissions, bool forbidden) { botPermissions = return_botPermissions; forbidden = return_forbidden; - hasSpecialPermissions = return_hasSpecialPermissions; } - function eraseAllBotPermissions(address, address) external view { + function eraseAllBotPermissions(address) external view { if (revertOnErase) { revert("Unexpected call to eraseAllBotPermissions"); } @@ -42,11 +35,7 @@ contract BotListMock { return_activeBotsRemaining = activeBotsRemaining; } - function setBotPermissions(address, address, address, uint192) - external - view - returns (uint256 activeBotsRemaining) - { + function setBotPermissions(address, address, uint192) external view returns (uint256 activeBotsRemaining) { activeBotsRemaining = return_activeBotsRemaining; } } diff --git a/contracts/test/mocks/core/BotMock.sol b/contracts/test/mocks/core/BotMock.sol new file mode 100644 index 00000000..891cec27 --- /dev/null +++ b/contracts/test/mocks/core/BotMock.sol @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: UNLICENSED +// Gearbox Protocol. Generalized leverage for DeFi protocols +// (c) Gearbox Foundation, 2024. +pragma solidity ^0.8.17; + +import {IBotV3} from "../../../interfaces/IBotV3.sol"; + +contract BotMock is IBotV3 { + uint192 public override requiredPermissions; + + function setRequiredPermissions(uint192 permissions) external { + requiredPermissions = permissions; + } +} diff --git a/contracts/test/unit/core/BotListV3.unit.t.sol b/contracts/test/unit/core/BotListV3.unit.t.sol index a68298ef..dda8085e 100644 --- a/contracts/test/unit/core/BotListV3.unit.t.sol +++ b/contracts/test/unit/core/BotListV3.unit.t.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: UNLICENSED // Gearbox Protocol. Generalized leverage for DeFi protocols -// (c) Gearbox Foundation, 2023. +// (c) Gearbox Foundation, 2024. pragma solidity ^0.8.17; import {BotListV3} from "../../../core/BotListV3.sol"; @@ -11,6 +11,7 @@ import "../../lib/constants.sol"; // MOCKS import {AddressProviderV3ACLMock} from "../../mocks/core/AddressProviderV3ACLMock.sol"; +import {BotMock} from "../../mocks/core/BotMock.sol"; // EXCEPTIONS import "../../../interfaces/IExceptions.sol"; @@ -30,15 +31,13 @@ contract BotListV3UnitTest is Test, IBotListV3Events { address invalidFacade; function setUp() public { - bot = makeAddr("BOT"); - otherBot = makeAddr("OTHER_BOT"); + bot = address(new BotMock()); + otherBot = address(new BotMock()); creditManager = makeAddr("CREDIT_MANAGER"); creditFacade = makeAddr("CREDIT_FACADE"); creditAccount = makeAddr("CREDIT_ACCOUNT"); invalidFacade = makeAddr("INVALID_FACADE"); - vm.etch(bot, "RANDOM CODE"); - vm.etch(otherBot, "RANDOM CODE"); 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)); @@ -56,155 +55,94 @@ contract BotListV3UnitTest is Test, IBotListV3Events { function test_U_BL_01_setBotPermissions_works_correctly() public { vm.expectRevert(CallerNotCreditFacadeException.selector); vm.prank(invalidFacade); - botList.setBotPermissions({ - bot: bot, - creditManager: creditManager, - creditAccount: creditAccount, - permissions: type(uint192).max - }); - - vm.expectRevert(abi.encodeWithSelector(AddressIsNotContractException.selector, DUMB_ADDRESS)); - vm.prank(creditFacade); - botList.setBotPermissions({ - bot: DUMB_ADDRESS, - creditManager: creditManager, - creditAccount: creditAccount, - permissions: type(uint192).max - }); + botList.setBotPermissions({bot: bot, creditAccount: creditAccount, permissions: type(uint192).max}); - vm.prank(CONFIGURATOR); - botList.setBotForbiddenStatus(bot, true); + BotMock(bot).setRequiredPermissions(1); + BotMock(bot).requiredPermissions(); - vm.expectRevert(InvalidBotException.selector); + vm.expectRevert(InsufficientBotPermissionsException.selector); vm.prank(creditFacade); - botList.setBotPermissions({ - bot: bot, - creditManager: creditManager, - creditAccount: creditAccount, - permissions: type(uint192).max - }); - - vm.prank(CONFIGURATOR); - botList.setBotForbiddenStatus(bot, false); + botList.setBotPermissions({bot: bot, creditAccount: creditAccount, permissions: 2}); + BotMock(bot).setRequiredPermissions(0); vm.prank(CONFIGURATOR); - botList.setBotSpecialPermissions(bot, creditManager, 1); + botList.setBotForbiddenStatus(bot, true); vm.expectRevert(InvalidBotException.selector); vm.prank(creditFacade); - botList.setBotPermissions({ - bot: bot, - creditManager: creditManager, - creditAccount: creditAccount, - permissions: type(uint192).max - }); + botList.setBotPermissions({bot: bot, creditAccount: creditAccount, permissions: type(uint192).max}); vm.prank(CONFIGURATOR); - botList.setBotSpecialPermissions(bot, creditManager, 0); + botList.setBotForbiddenStatus(bot, false); vm.expectEmit(true, true, true, true); emit SetBotPermissions(bot, creditManager, creditAccount, 1); vm.prank(creditFacade); - uint256 activeBotsRemaining = botList.setBotPermissions({ - bot: bot, - creditManager: creditManager, - creditAccount: creditAccount, - permissions: 1 - }); + uint256 activeBotsRemaining = + botList.setBotPermissions({bot: bot, creditAccount: creditAccount, permissions: 1}); assertEq(activeBotsRemaining, 1, "Incorrect number of bots returned"); - assertEq(botList.botPermissions(bot, creditManager, creditAccount), 1, "Bot permissions were not set"); + assertEq(botList.botPermissions(bot, creditAccount), 1, "Bot permissions were not set"); - address[] memory bots = botList.activeBots(creditManager, creditAccount); + address[] memory bots = botList.activeBots(creditAccount); assertEq(bots.length, 1, "Incorrect active bots array length"); assertEq(bots[0], bot, "Incorrect address added to active bots list"); vm.prank(creditFacade); - activeBotsRemaining = botList.setBotPermissions({ - bot: bot, - creditManager: creditManager, - creditAccount: creditAccount, - permissions: 2 - }); + activeBotsRemaining = botList.setBotPermissions({bot: bot, creditAccount: creditAccount, permissions: 2}); assertEq(activeBotsRemaining, 1, "Incorrect number of bots returned"); - assertEq(botList.botPermissions(bot, creditManager, creditAccount), 2, "Bot permissions were not set"); + assertEq(botList.botPermissions(bot, creditAccount), 2, "Bot permissions were not set"); - bots = botList.activeBots(creditManager, creditAccount); + bots = botList.activeBots(creditAccount); assertEq(bots.length, 1, "Incorrect active bots array length"); assertEq(bots[0], bot, "Incorrect address added to active bots list"); vm.prank(CONFIGURATOR); botList.setBotForbiddenStatus(bot, true); - vm.expectEmit(true, true, true, false); - emit EraseBot(bot, creditManager, creditAccount); + vm.expectEmit(true, true, true, true); + emit SetBotPermissions(bot, creditManager, creditAccount, 0); vm.prank(creditFacade); - activeBotsRemaining = botList.setBotPermissions({ - bot: bot, - creditManager: creditManager, - creditAccount: creditAccount, - permissions: 0 - }); + activeBotsRemaining = botList.setBotPermissions({bot: bot, creditAccount: creditAccount, permissions: 0}); assertEq(activeBotsRemaining, 0, "Incorrect number of bots returned"); - assertEq(botList.botPermissions(bot, creditManager, creditAccount), 0, "Bot permissions were not set"); + assertEq(botList.botPermissions(bot, creditAccount), 0, "Bot permissions were not set"); - bots = botList.activeBots(creditManager, creditAccount); + bots = botList.activeBots(creditAccount); assertEq(bots.length, 0, "Incorrect active bots array length"); } /// @dev U:[BL-2]: `eraseAllBotPermissions` works correctly function test_U_BL_02_eraseAllBotPermissions_works_correctly() public { vm.prank(creditFacade); - botList.setBotPermissions({bot: bot, creditManager: creditManager, creditAccount: creditAccount, permissions: 1}); + botList.setBotPermissions({bot: bot, creditAccount: creditAccount, permissions: 1}); vm.prank(creditFacade); - uint256 activeBotsRemaining = botList.setBotPermissions({ - bot: otherBot, - creditManager: creditManager, - creditAccount: creditAccount, - permissions: 2 - }); + uint256 activeBotsRemaining = + botList.setBotPermissions({bot: otherBot, creditAccount: creditAccount, permissions: 2}); assertEq(activeBotsRemaining, 2, "Incorrect number of active bots"); vm.expectRevert(CallerNotCreditFacadeException.selector); vm.prank(invalidFacade); - botList.eraseAllBotPermissions(creditManager, creditAccount); + botList.eraseAllBotPermissions(creditAccount); - vm.expectEmit(true, true, true, false); - emit EraseBot(otherBot, creditManager, creditAccount); + vm.expectEmit(true, true, true, true); + emit SetBotPermissions(otherBot, creditManager, creditAccount, 0); - vm.expectEmit(true, true, true, false); - emit EraseBot(bot, creditManager, creditAccount); + vm.expectEmit(true, true, true, true); + emit SetBotPermissions(bot, creditManager, creditAccount, 0); vm.prank(creditFacade); - botList.eraseAllBotPermissions(creditManager, creditAccount); + botList.eraseAllBotPermissions(creditAccount); - assertEq(botList.botPermissions(bot, creditManager, creditAccount), 0, "Permissions not erased for bot 1"); - assertEq(botList.botPermissions(otherBot, creditManager, creditAccount), 0, "Permissions not erased for bot 2"); + assertEq(botList.botPermissions(bot, creditAccount), 0, "Permissions not erased for bot 1"); + assertEq(botList.botPermissions(otherBot, creditAccount), 0, "Permissions not erased for bot 2"); - address[] memory activeBots = botList.activeBots(creditManager, creditAccount); + address[] memory activeBots = botList.activeBots(creditAccount); assertEq(activeBots.length, 0, "Not all active bots were disabled"); } - - /// @dev U:[BL-3]: `setBotSpecialPermissions` works correctly - function test_U_BL_03_setBotSpecialPermissions_works_correctly() public { - vm.expectRevert(CallerNotConfiguratorException.selector); - botList.setBotSpecialPermissions(bot, creditManager, 2); - - vm.expectEmit(true, true, false, true); - emit SetBotSpecialPermissions(bot, creditManager, 2); - - vm.prank(CONFIGURATOR); - botList.setBotSpecialPermissions(bot, creditManager, 2); - - (uint192 permissions,, bool hasSpecialPermissions) = botList.getBotStatus(bot, creditManager, creditAccount); - - assertEq(permissions, 2, "Special permissions are incorrect"); - assertTrue(hasSpecialPermissions, "Special permissions status returned incorrectly"); - } } diff --git a/contracts/test/unit/credit/CreditFacadeV3.unit.t.sol b/contracts/test/unit/credit/CreditFacadeV3.unit.t.sol index ab25bdb1..742acee4 100644 --- a/contracts/test/unit/credit/CreditFacadeV3.unit.t.sol +++ b/contracts/test/unit/credit/CreditFacadeV3.unit.t.sol @@ -129,7 +129,7 @@ contract CreditFacadeV3UnitTest is TestHelper, BalanceHelper, ICreditFacadeV3Eve addressProvider.setAddress(AP_WETH_TOKEN, tokenTestSuite.addressOf(Tokens.WETH), false); - botListMock = BotListMock(addressProvider.getAddressOrRevert(AP_BOT_LIST, 3_00)); + botListMock = BotListMock(addressProvider.getAddressOrRevert(AP_BOT_LIST, 3_10)); priceOracleMock = PriceOracleMock(addressProvider.getAddressOrRevert(AP_PRICE_ORACLE, 3_00)); @@ -438,10 +438,7 @@ contract CreditFacadeV3UnitTest is TestHelper, BalanceHelper, ICreditFacadeV3Eve } if (hasBotPermissions) { - vm.expectCall( - address(botListMock), - abi.encodeCall(IBotListV3.eraseAllBotPermissions, (address(creditManagerMock), creditAccount)) - ); + vm.expectCall(address(botListMock), abi.encodeCall(IBotListV3.eraseAllBotPermissions, (creditAccount))); } vm.expectEmit(true, true, true, true); @@ -713,7 +710,7 @@ contract CreditFacadeV3UnitTest is TestHelper, BalanceHelper, ICreditFacadeV3Eve uint256 enabledTokensMaskBefore = 123123123; - botListMock.setBotStatusReturns(ALL_PERMISSIONS, false, false); + botListMock.setBotStatusReturns(ALL_PERMISSIONS, false); creditManagerMock.setEnabledTokensMask(enabledTokensMaskBefore); creditManagerMock.setBorrower(USER); @@ -755,26 +752,22 @@ contract CreditFacadeV3UnitTest is TestHelper, BalanceHelper, ICreditFacadeV3Eve creditManagerMock.setFlagFor(creditAccount, BOT_PERMISSIONS_SET_FLAG, true); - botListMock.setBotStatusReturns(ALL_PERMISSIONS, true, false); + botListMock.setBotStatusReturns(ALL_PERMISSIONS, true); vm.expectRevert(NotApprovedBotException.selector); creditFacade.botMulticall(creditAccount, calls); - botListMock.setBotStatusReturns(0, false, false); + botListMock.setBotStatusReturns(0, false); vm.expectRevert(NotApprovedBotException.selector); creditFacade.botMulticall(creditAccount, calls); creditManagerMock.setFlagFor(creditAccount, BOT_PERMISSIONS_SET_FLAG, false); - botListMock.setBotStatusReturns(ALL_PERMISSIONS, false, false); + botListMock.setBotStatusReturns(ALL_PERMISSIONS, false); vm.expectRevert(NotApprovedBotException.selector); creditFacade.botMulticall(creditAccount, calls); - - botListMock.setBotStatusReturns(ALL_PERMISSIONS, false, true); - - creditFacade.botMulticall(creditAccount, calls); } struct MultiCallPermissionTestCase { @@ -1769,20 +1762,14 @@ contract CreditFacadeV3UnitTest is TestHelper, BalanceHelper, ICreditFacadeV3Eve address(creditManagerMock), abi.encodeCall(ICreditManagerV3.setFlagFor, (creditAccount, BOT_PERMISSIONS_SET_FLAG, true)) ); - vm.expectCall( - address(botListMock), - abi.encodeCall(IBotListV3.setBotPermissions, (bot, address(creditManagerMock), creditAccount, 1)) - ); + vm.expectCall(address(botListMock), abi.encodeCall(IBotListV3.setBotPermissions, (bot, creditAccount, 1))); vm.prank(USER); creditFacade.setBotPermissions({creditAccount: creditAccount, bot: bot, permissions: 1}); /// It removes flag if no bots left botListMock.setBotPermissionsReturn(0); - vm.expectCall( - address(botListMock), - abi.encodeCall(IBotListV3.setBotPermissions, (bot, address(creditManagerMock), creditAccount, 1)) - ); + vm.expectCall(address(botListMock), abi.encodeCall(IBotListV3.setBotPermissions, (bot, creditAccount, 1))); vm.expectCall( address(creditManagerMock),