From 13b8c95511496d581ce0f3788d2d744b891118ac Mon Sep 17 00:00:00 2001 From: Van0k Date: Wed, 31 May 2023 08:21:10 +0400 Subject: [PATCH 1/4] fix: botList sends weth to bots --- contracts/support/BotListV3.sol | 23 ++++++++++++++----- .../mocks/core/AddressProviderV3ACLMock.sol | 3 +++ contracts/test/unit/support/BotList.t.sol | 17 +++++++------- 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/contracts/support/BotListV3.sol b/contracts/support/BotListV3.sol index 1be76a31..f4f26ba7 100644 --- a/contracts/support/BotListV3.sol +++ b/contracts/support/BotListV3.sol @@ -9,17 +9,18 @@ import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol"; import {ACLNonReentrantTrait} from "../traits/ACLNonReentrantTrait.sol"; import {IBotListV3, BotFunding} from "../interfaces/IBotListV3.sol"; -import {IAddressProvider} from "@gearbox-protocol/core-v2/contracts/interfaces/IAddressProvider.sol"; +import {IAddressProviderV3, AP_TREASURY, AP_WETH_TOKEN, NO_VERSION_CONTROL} from "../core/AddressProviderV3.sol"; import {ICreditManagerV3} from "../interfaces/ICreditManagerV3.sol"; import {ICreditFacadeV3} from "../interfaces/ICreditFacadeV3.sol"; import {ICreditAccountBase} from "../interfaces/ICreditAccountV3.sol"; +import {IWETH} from "@gearbox-protocol/core-v2/contracts/interfaces/external/IWETH.sol"; import "../interfaces/IExceptions.sol"; import {PERCENTAGE_FACTOR} from "@gearbox-protocol/core-v2/contracts/libraries/Constants.sol"; /// @title BotList /// @notice Used to store a mapping of borrowers => bots. A separate contract is used for transferability when -/// changing Credit Facades +/// changing Credit Facades. contract BotListV3 is ACLNonReentrantTrait, IBotListV3 { using SafeCast for uint256; using Address for address; @@ -51,11 +52,15 @@ contract BotListV3 is ACLNonReentrantTrait, IBotListV3 { /// @notice Address of the DAO treasury address public immutable treasury; + /// @notice Address of the WETH token + address public immutable weth; + /// @notice Contract version uint256 public constant override version = 3_00; constructor(address _addressProvider) ACLNonReentrantTrait(_addressProvider) { - treasury = IAddressProvider(_addressProvider).getTreasuryContract(); + treasury = IAddressProviderV3(_addressProvider).getAddressOrRevert(AP_TREASURY, NO_VERSION_CONTROL); + weth = IAddressProviderV3(_addressProvider).getAddressOrRevert(AP_WETH_TOKEN, NO_VERSION_CONTROL); } /// @notice Limits access to a function only to Credit Facades connected to approved CMs @@ -128,7 +133,6 @@ contract BotListV3 is ACLNonReentrantTrait, IBotListV3 { botFunding[creditAccount][bot].remainingFunds = 0; // F: [BL-06] botFunding[creditAccount][bot].maxWeeklyAllowance = 0; // F: [BL-06] botFunding[creditAccount][bot].remainingWeeklyAllowance = 0; // F: [BL-06] - botFunding[creditAccount][bot].allowanceLU = uint40(block.timestamp); // F: [BL-06] activeBots[creditAccount].remove(bot); // F: [BL-06] unchecked { ++i; @@ -165,9 +169,9 @@ contract BotListV3 is ACLNonReentrantTrait, IBotListV3 { bf.remainingWeeklyAllowance -= paymentAmount + feeAmount; // F: [BL-05] bf.remainingFunds -= paymentAmount + feeAmount; // F: [BL-05] - fundingBalances[payer] -= uint256(paymentAmount + feeAmount); // F: [BL-05] + fundingBalances[payer] -= paymentAmount + feeAmount; // F: [BL-05] - payable(bot).sendValue(paymentAmount); // F: [BL-05] + _convertAndSendWETH(bot, paymentAmount); if (feeAmount > 0) payable(treasury).sendValue(feeAmount); // F: [BL-05] emit PayBot(payer, creditAccount, bot, paymentAmount, feeAmount); // F: [BL-05] @@ -210,6 +214,13 @@ contract BotListV3 is ACLNonReentrantTrait, IBotListV3 { return (botPermissions[creditAccount][bot], forbiddenBot[bot]); } + /// @dev Converts ETH to WETH and sends to recipient + /// @dev Used to avoid giving flow control to bots + function _convertAndSendWETH(address to, uint256 amount) internal { + IWETH(weth).deposit{value: amount}(); + IWETH(weth).transfer(to, amount); + } + // // CONFIGURATION // diff --git a/contracts/test/mocks/core/AddressProviderV3ACLMock.sol b/contracts/test/mocks/core/AddressProviderV3ACLMock.sol index bfc8728b..a68952e1 100644 --- a/contracts/test/mocks/core/AddressProviderV3ACLMock.sol +++ b/contracts/test/mocks/core/AddressProviderV3ACLMock.sol @@ -9,6 +9,7 @@ import {PriceOracleMock} from "../oracles/PriceOracleMock.sol"; import {WETHGatewayMock} from "../support/WETHGatewayMock.sol"; import {WithdrawalManagerMock} from "../support/WithdrawalManagerMock.sol"; import {BotListMock} from "../support/BotListMock.sol"; +import {WETHMock} from "../token/WETHMock.sol"; import {Test} from "forge-std/Test.sol"; @@ -47,6 +48,8 @@ contract AddressProviderV3ACLMock is Test, AddressProviderV3 { _setAddress(AP_TREASURY, makeAddr("TREASURY"), 0); + _setAddress(AP_WETH_TOKEN, address(new WETHMock()), 0); + isConfigurator[msg.sender] = true; owner = msg.sender; } diff --git a/contracts/test/unit/support/BotList.t.sol b/contracts/test/unit/support/BotList.t.sol index 83a8c55e..709110dc 100644 --- a/contracts/test/unit/support/BotList.t.sol +++ b/contracts/test/unit/support/BotList.t.sol @@ -13,8 +13,8 @@ import {ICreditFacadeV3} from "../../../interfaces/ICreditFacadeV3.sol"; import "../../lib/constants.sol"; // MOCKS -import {AddressProviderV3ACLMock} from "../../mocks/core/AddressProviderV3ACLMock.sol"; - +import {AddressProviderV3ACLMock, AP_WETH_TOKEN, AP_TREASURY} from "../../mocks/core/AddressProviderV3ACLMock.sol"; +import {WETHMock} from "../../mocks/token/WETHMock.sol"; import {GeneralMock} from "../../mocks/GeneralMock.sol"; // SUITES @@ -35,6 +35,7 @@ contract InvalidCFMock { /// @notice Designed for unit test purposes only contract BotListTest is Test, IBotListV3Events { AddressProviderV3ACLMock public addressProvider; + WETHMock public weth; BotListV3 botList; @@ -50,6 +51,7 @@ contract BotListTest is Test, IBotListV3Events { function setUp() public { vm.prank(CONFIGURATOR); addressProvider = new AddressProviderV3ACLMock(); + weth = WETHMock(payable(addressProvider.getAddressOrRevert(AP_WETH_TOKEN, 0))); tokenTestSuite = new TokensTestSuite(); @@ -92,7 +94,8 @@ contract BotListTest is Test, IBotListV3Events { /// @dev [BL-1]: constructor sets correct values function test_BL_01_constructor_sets_correct_values() public { - assertEq(botList.treasury(), addressProvider.getTreasuryContract(), "Treasury contract incorrect"); + assertEq(botList.treasury(), addressProvider.getAddressOrRevert(AP_TREASURY, 0), "Treasury contract incorrect"); + assertEq(botList.weth(), address(weth), "WETH incorrect"); assertEq(botList.daoFee(), 0, "Initial DAO fee incorrect"); } @@ -359,7 +362,7 @@ contract BotListTest is Test, IBotListV3Events { assertEq(allowanceLU, block.timestamp - 1 days, "Allowance update timestamp incorrect"); - assertEq(address(bot).balance, 1 ether / 20, "Bot was sent incorrect ETH amount"); + assertEq(weth.balanceOf(address(bot)), 1 ether / 20, "Bot was sent incorrect WETH amount"); assertEq(addressProvider.getTreasuryContract().balance, 1 ether / 40, "Treasury was sent incorrect amount"); @@ -392,7 +395,7 @@ contract BotListTest is Test, IBotListV3Events { "User remaining funding balance incorrect" ); - assertEq(address(bot).balance, 2 ether / 20, "Bot was sent incorrect ETH amount"); + assertEq(weth.balanceOf(address(bot)), 2 ether / 20, "Bot was sent incorrect WETH amount"); assertEq(addressProvider.getTreasuryContract().balance, 2 ether / 40, "Treasury was sent incorrect amount"); } @@ -448,8 +451,6 @@ contract BotListTest is Test, IBotListV3Events { assertEq(remainingWeeklyAllowance, 0, "Remaining funds were not zeroed"); - assertEq(allowanceLU, block.timestamp, "Allowance update timestamp incorrect"); - (remainingFunds, maxWeeklyAllowance, remainingWeeklyAllowance, allowanceLU) = botList.botFunding(address(creditAccount), address(bot2)); @@ -459,8 +460,6 @@ contract BotListTest is Test, IBotListV3Events { assertEq(remainingWeeklyAllowance, 0, "Remaining funds were not zeroed"); - assertEq(allowanceLU, block.timestamp, "Allowance update timestamp incorrect"); - address[] memory activeBots = botList.getActiveBots(address(creditAccount)); assertEq(activeBots.length, 0, "Not all active bots were disabled"); From 4eebdb249105f98f60f44e45d85a270fec3b70f0 Mon Sep 17 00:00:00 2001 From: Van0k Date: Wed, 31 May 2023 11:02:14 +0400 Subject: [PATCH 2/4] test: balanceLogic tests + fix --- contracts/libraries/BalancesLogic.sol | 2 +- .../test/unit/libraries/BalancesLogic.t.sol | 210 ++++++++++++++++++ .../unit/libraries/BalancesLogicCaller.sol | 24 ++ 3 files changed, 235 insertions(+), 1 deletion(-) create mode 100644 contracts/test/unit/libraries/BalancesLogic.t.sol create mode 100644 contracts/test/unit/libraries/BalancesLogicCaller.sol diff --git a/contracts/libraries/BalancesLogic.sol b/contracts/libraries/BalancesLogic.sol index 7b8e0907..eb554ecb 100644 --- a/contracts/libraries/BalancesLogic.sol +++ b/contracts/libraries/BalancesLogic.sol @@ -73,7 +73,7 @@ library BalancesLogic { forbiddenBalances = new BalanceWithMask[](forbiddenTokensOnAccount.calcEnabledTokens()); unchecked { uint256 i; - for (uint256 tokenMask = 1; tokenMask < forbiddenTokensOnAccount; tokenMask <<= 1) { + for (uint256 tokenMask = 1; tokenMask <= forbiddenTokensOnAccount; tokenMask <<= 1) { if (forbiddenTokensOnAccount & tokenMask != 0) { address token = getTokenByMaskFn(tokenMask); forbiddenBalances[i].token = token; diff --git a/contracts/test/unit/libraries/BalancesLogic.t.sol b/contracts/test/unit/libraries/BalancesLogic.t.sol new file mode 100644 index 00000000..308f08c7 --- /dev/null +++ b/contracts/test/unit/libraries/BalancesLogic.t.sol @@ -0,0 +1,210 @@ +// SPDX-License-Identifier: UNLICENSED +// Gearbox Protocol. Generalized leverage for DeFi protocols +// (c) Gearbox Holdings, 2023 +pragma solidity ^0.8.17; + +import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; + +import {Balance} from "@gearbox-protocol/core-v2/contracts/libraries/Balances.sol"; +import {BalancesLogic, BalanceWithMask} from "../../../libraries/BalancesLogic.sol"; +import {BitMask} from "../../../libraries/BitMask.sol"; +import {TestHelper} from "../../lib/helper.sol"; +import {BalancesLogicCaller} from "./BalancesLogicCaller.sol"; + +import {BalanceLessThanMinimumDesiredException, ForbiddenTokensException} from "../../../interfaces/IExceptions.sol"; + +/// @title BalancesLogic test +/// @notice [BM]: Unit tests for BalancesLogic +contract BalancesLogicTest is TestHelper { + address creditAccount; + address[16] tokens; + mapping(uint256 => uint256) maskToIndex; + + function _setupTokenBalances(uint128[16] calldata balances, uint256 length) internal { + creditAccount = makeAddr("CREDIT_ACCOUNT"); + + for (uint256 i = 0; i < length; ++i) { + tokens[i] = makeAddr(string(abi.encodePacked("TOKEN", i))); + maskToIndex[1 << i] = i; + vm.mockCall(tokens[i], abi.encodeCall(IERC20.balanceOf, (creditAccount)), abi.encode(balances[i])); + } + } + + /// @notice U:[BLL-1]: storeBalances works correctly + function test_BLL_01_storeBalances_works_correctly( + uint128[16] calldata balances, + uint128[16] calldata deltas, + uint256 length + ) public { + vm.assume(length <= 16); + + _setupTokenBalances(balances, length); + + Balance[] memory deltaArray = new Balance[](length); + + for (uint256 i = 0; i < length; ++i) { + deltaArray[i] = Balance({token: tokens[i], balance: deltas[i]}); + } + + Balance[] memory expectedBalances = BalancesLogic.storeBalances(creditAccount, deltaArray); + + assertEq(expectedBalances.length, deltaArray.length, "Wrong length array was returned"); + + for (uint256 i = 0; i < length; ++i) { + assertEq(expectedBalances[i].token, tokens[i]); + + assertEq( + expectedBalances[i].balance, uint256(balances[i]) + uint256(deltas[i]), "Incorrect expected balance" + ); + } + } + + /// @notice U:[BLL-2]: compareBalances works correctly + function test_BLL_02_compareBalances_works_correctly( + uint128[16] calldata balances, + uint128[16] calldata expectedBalances, + uint256 length + ) public { + vm.assume(length <= 16); + + BalancesLogicCaller caller = new BalancesLogicCaller(); + + _setupTokenBalances(balances, length); + + bool expectRevert; + address exceptionToken; + + for (uint256 i = 0; i < length; ++i) { + if (expectedBalances[i] > balances[i]) { + expectRevert = true; + exceptionToken = tokens[i]; + break; + } + } + + Balance[] memory expectedArray = new Balance[](length); + + for (uint256 i = 0; i < length; ++i) { + expectedArray[i] = Balance({token: tokens[i], balance: expectedBalances[i]}); + } + + if (expectRevert) { + vm.expectRevert(abi.encodeWithSelector(BalanceLessThanMinimumDesiredException.selector, exceptionToken)); + } + + caller.compareBalances(creditAccount, expectedArray); + } + + function _getTokenByMask(uint256 mask) internal view returns (address) { + return tokens[maskToIndex[mask]]; + } + + /// @notice U:[BLL-3]: storeForbiddenBalances works correctly + function test_BLL_03_storeForbiddenBalances_works_correctly( + uint128[16] calldata balances, + uint256 enabledTokensMask, + uint256 forbiddenTokensMask + ) public { + enabledTokensMask %= (2 ** 16); + forbiddenTokensMask %= (2 ** 16); + + _setupTokenBalances(balances, 16); + + BalanceWithMask[] memory forbiddenBalances = + BalancesLogic.storeForbiddenBalances(creditAccount, enabledTokensMask, forbiddenTokensMask, _getTokenByMask); + + uint256 j; + + for (uint256 i = 0; i < 16; ++i) { + uint256 tokenMask = 1 << i; + if (tokenMask & enabledTokensMask & forbiddenTokensMask > 0) { + assertEq(forbiddenBalances[j].balance, balances[i], "Incorrect forbidden token balance"); + + assertEq(forbiddenBalances[j].token, tokens[i], "Incorrect forbidden token address"); + + assertEq(forbiddenBalances[j].tokenMask, tokenMask, "Incorrect forbidden token mask"); + ++j; + } + } + } + + // /// @dev Checks that no new forbidden tokens were enabled and that balances of existing forbidden tokens + // /// were not increased + // /// @param creditAccount Credit Account to check + // /// @param enabledTokensMaskBefore Mask of enabled tokens on the account before operations + // /// @param enabledTokensMaskAfter Mask of enabled tokens on the account after operations + // /// @param forbiddenBalances Array of balances of forbidden tokens (received from `storeForbiddenBalances`) + // /// @param forbiddenTokenMask Mask of forbidden tokens + // function checkForbiddenBalances( + // address creditAccount, + // uint256 enabledTokensMaskBefore, + // uint256 enabledTokensMaskAfter, + // BalanceWithMask[] memory forbiddenBalances, + // uint256 forbiddenTokenMask + // ) internal view { + // uint256 forbiddenTokensOnAccount = enabledTokensMaskAfter & forbiddenTokenMask; + // if (forbiddenTokensOnAccount == 0) return; + + // /// A diff between the forbidden tokens before and after is computed + // /// If there are forbidden tokens enabled during operations, the function would revert + // uint256 forbiddenTokensOnAccountBefore = enabledTokensMaskBefore & forbiddenTokenMask; + // if (forbiddenTokensOnAccount & ~forbiddenTokensOnAccountBefore != 0) revert ForbiddenTokensException(); + + // /// Then, the function checks that any remaining forbidden tokens didn't have their balances increased + // unchecked { + // uint256 len = forbiddenBalances.length; + // for (uint256 i = 0; i < len; ++i) { + // if (forbiddenTokensOnAccount & forbiddenBalances[i].tokenMask != 0) { + // uint256 currentBalance = IERC20Helper.balanceOf(forbiddenBalances[i].token, creditAccount); + // if (currentBalance > forbiddenBalances[i].balance) { + // revert ForbiddenTokensException(); + // } + // } + // } + // } + // } + + /// @notice U:[BLL-4]: checkForbiddenBalances works correctly + function test_BLL_04_storeForbiddenBalances_works_correctly( + uint128[16] calldata balancesBefore, + uint128[16] calldata balancesAfter, + uint256 enabledTokensMaskBefore, + uint256 enabledTokensMaskAfter, + uint256 forbiddenTokensMask + ) public { + enabledTokensMaskBefore %= (2 ** 16); + enabledTokensMaskAfter %= (2 ** 16); + forbiddenTokensMask %= (2 ** 16); + + BalancesLogicCaller caller = new BalancesLogicCaller(); + + _setupTokenBalances(balancesBefore, 16); + + BalanceWithMask[] memory forbiddenBalances = BalancesLogic.storeForbiddenBalances( + creditAccount, enabledTokensMaskBefore, forbiddenTokensMask, _getTokenByMask + ); + + _setupTokenBalances(balancesAfter, 16); + + bool shouldRevert; + + if ((enabledTokensMaskAfter & ~enabledTokensMaskBefore) & forbiddenTokensMask > 0) shouldRevert = true; + + for (uint256 i = 0; i < 16; ++i) { + uint256 tokenMask = 1 << i; + if ((enabledTokensMaskAfter & forbiddenTokensMask & tokenMask > 0) && balancesAfter[i] > balancesBefore[i]) + { + shouldRevert = true; + break; + } + } + + if (shouldRevert) { + vm.expectRevert(ForbiddenTokensException.selector); + } + + caller.checkForbiddenBalances( + creditAccount, enabledTokensMaskBefore, enabledTokensMaskAfter, forbiddenBalances, forbiddenTokensMask + ); + } +} diff --git a/contracts/test/unit/libraries/BalancesLogicCaller.sol b/contracts/test/unit/libraries/BalancesLogicCaller.sol new file mode 100644 index 00000000..a40cb1b7 --- /dev/null +++ b/contracts/test/unit/libraries/BalancesLogicCaller.sol @@ -0,0 +1,24 @@ +// SPDX-License-Identifier: UNLICENSED +// Gearbox Protocol. Generalized leverage for DeFi protocols +// (c) Gearbox Holdings, 2023 +pragma solidity ^0.8.17; + +import {BalancesLogic, Balance, BalanceWithMask} from "../../../libraries/BalancesLogic.sol"; + +contract BalancesLogicCaller { + function compareBalances(address creditAccount, Balance[] memory expected) external view { + BalancesLogic.compareBalances(creditAccount, expected); + } + + function checkForbiddenBalances( + address creditAccount, + uint256 enabledTokensMaskBefore, + uint256 enabledTokensMaskAfter, + BalanceWithMask[] memory forbiddenBalances, + uint256 forbiddenTokenMask + ) external view { + BalancesLogic.checkForbiddenBalances( + creditAccount, enabledTokensMaskBefore, enabledTokensMaskAfter, forbiddenBalances, forbiddenTokenMask + ); + } +} From a4ef3c4e802a2926957d1b6efce3a6abfda40e0c Mon Sep 17 00:00:00 2001 From: Mikael <26343374+0xmikko@users.noreply.github.com> Date: Wed, 31 May 2023 12:25:39 +0200 Subject: [PATCH 3/4] fix: bot list update --- contracts/interfaces/IBotListV3.sol | 18 ++- contracts/support/BotListV3.sol | 153 ++++++++++++---------- contracts/test/suites/PoolDeployer.sol | 2 +- contracts/test/unit/support/BotList.t.sol | 37 +++--- 4 files changed, 121 insertions(+), 89 deletions(-) diff --git a/contracts/interfaces/IBotListV3.sol b/contracts/interfaces/IBotListV3.sol index 6b3f4186..64f46033 100644 --- a/contracts/interfaces/IBotListV3.sol +++ b/contracts/interfaces/IBotListV3.sol @@ -26,7 +26,10 @@ interface IBotListV3Events { event BotForbiddenStatusChanged(address indexed bot, bool status); /// @dev Emits when the user changes the amount of funds in his bot wallet - event ChangeFunding(address indexed payer, uint256 newRemainingFunds); + event Deposit(address indexed payer, uint256 amount); + + /// @dev Emits when the user changes the amount of funds in his bot wallet + event Withdraw(address indexed payer, uint256 amount); /// @dev Emits when the allowed weekly amount of bot's spending is changed by the user event ChangeBotWeeklyAllowance(address indexed payer, address indexed bot, uint72 newWeeklyAllowance); @@ -44,7 +47,7 @@ interface IBotListV3Events { event SetBotDAOFee(uint16 newFee); /// @dev Emits when all bot permissions for a Credit Account are erased - event EraseBots(address creditAccount); + event EraseBot(address indexed creditAccount, address indexed bot); /// @dev Emits when a new Credit Manager is approved in BotList event CreditManagerAdded(address indexed creditManager); @@ -69,10 +72,10 @@ interface IBotListV3 is IBotListV3Events, IVersion { function eraseAllBotPermissions(address creditAccount) external; /// @dev Adds funds to the borrower's bot payment wallet - function addFunding() external payable; + function deposit() external payable; /// @dev Removes funds from the borrower's bot payment wallet - function removeFunding(uint256 amount) external; + function withdraw(uint256 amount) external; /// @dev Takes payment for performed services from the user's balance and sends to the bot /// @param payer Address to charge @@ -81,6 +84,10 @@ interface IBotListV3 is IBotListV3Events, IVersion { /// @param paymentAmount Amount to pay function payBot(address payer, address creditAccount, address bot, uint72 paymentAmount) external; + // + // GETTERS + // + /// @dev Returns all active bots currently on the account function getActiveBots(address creditAccount) external view returns (address[] memory); @@ -95,4 +102,7 @@ interface IBotListV3 is IBotListV3Events, IVersion { external view returns (uint192 permissions, bool forbidden); + + /// @dev Returns user funcding balance in ETH + function balanceOf(address payer) external view returns (uint256); } diff --git a/contracts/support/BotListV3.sol b/contracts/support/BotListV3.sol index f4f26ba7..46342482 100644 --- a/contracts/support/BotListV3.sol +++ b/contracts/support/BotListV3.sol @@ -6,26 +6,45 @@ pragma solidity ^0.8.17; import {EnumerableSet} from "@openzeppelin/contracts/utils/structs/EnumerableSet.sol"; import {Address} from "@openzeppelin/contracts/utils/Address.sol"; import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol"; +import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; +import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; +import {IWETH} from "@gearbox-protocol/core-v2/contracts/interfaces/external/IWETH.sol"; +import {PERCENTAGE_FACTOR} from "@gearbox-protocol/core-v2/contracts/libraries/Constants.sol"; + +import "../interfaces/IAddressProviderV3.sol"; import {ACLNonReentrantTrait} from "../traits/ACLNonReentrantTrait.sol"; import {IBotListV3, BotFunding} from "../interfaces/IBotListV3.sol"; -import {IAddressProviderV3, AP_TREASURY, AP_WETH_TOKEN, NO_VERSION_CONTROL} from "../core/AddressProviderV3.sol"; import {ICreditManagerV3} from "../interfaces/ICreditManagerV3.sol"; import {ICreditFacadeV3} from "../interfaces/ICreditFacadeV3.sol"; import {ICreditAccountBase} from "../interfaces/ICreditAccountV3.sol"; -import {IWETH} from "@gearbox-protocol/core-v2/contracts/interfaces/external/IWETH.sol"; import "../interfaces/IExceptions.sol"; -import {PERCENTAGE_FACTOR} from "@gearbox-protocol/core-v2/contracts/libraries/Constants.sol"; /// @title BotList /// @notice Used to store a mapping of borrowers => bots. A separate contract is used for transferability when -/// changing Credit Facades. +/// changing Credit Facades contract BotListV3 is ACLNonReentrantTrait, IBotListV3 { using SafeCast for uint256; using Address for address; using Address for address payable; using EnumerableSet for EnumerableSet.AddressSet; + using SafeERC20 for IERC20; + + /// @notice Contract version + uint256 public constant override version = 3_00; + + /// @notice Address of the DAO treasury + address public immutable treasury; + + /// @notice Address of the DAO treasury + address public immutable weth; + + /// @notice ERC20 compatibility to be able to add to wallet to manager user's bot funding + string public constant symbol = "gETH"; + + /// @notice ERC20 compatibility to be able to add to wallet to manager user's bot funding + string public constant name = "Gearbox bot funding"; /// @notice Mapping from Credit Manager address to their status as an approved Credit Manager /// Only Credit Facades connected to approved Credit Managers can alter bot permissions @@ -41,7 +60,7 @@ contract BotListV3 is ACLNonReentrantTrait, IBotListV3 { mapping(address => bool) public forbiddenBot; /// @notice Mapping from borrower to their bot funding balance - mapping(address => uint256) public fundingBalances; + mapping(address => uint256) public override balanceOf; /// @notice Mapping of (creditAccount, bot) to bot funding parameters mapping(address => mapping(address => BotFunding)) public botFunding; @@ -49,18 +68,9 @@ contract BotListV3 is ACLNonReentrantTrait, IBotListV3 { /// @notice A fee (in PERCENTAGE_FACTOR format) charged by the DAO on bot payments uint16 public daoFee = 0; - /// @notice Address of the DAO treasury - address public immutable treasury; - - /// @notice Address of the WETH token - address public immutable weth; - - /// @notice Contract version - uint256 public constant override version = 3_00; - - constructor(address _addressProvider) ACLNonReentrantTrait(_addressProvider) { - treasury = IAddressProviderV3(_addressProvider).getAddressOrRevert(AP_TREASURY, NO_VERSION_CONTROL); - weth = IAddressProviderV3(_addressProvider).getAddressOrRevert(AP_WETH_TOKEN, NO_VERSION_CONTROL); + constructor(address addressProvider) ACLNonReentrantTrait(addressProvider) { + treasury = IAddressProviderV3(addressProvider).getAddressOrRevert(AP_TREASURY, NO_VERSION_CONTROL); + weth = IAddressProviderV3(addressProvider).getAddressOrRevert(AP_WETH_TOKEN, NO_VERSION_CONTROL); } /// @notice Limits access to a function only to Credit Facades connected to approved CMs @@ -100,23 +110,28 @@ contract BotListV3 is ACLNonReentrantTrait, IBotListV3 { if (permissions != 0) { activeBots[creditAccount].add(bot); // F: [BL-03] - } else if (permissions == 0) { - if (fundingAmount != 0 || weeklyFundingAllowance != 0) { - revert PositiveFundingForInactiveBotException(); // F: [BL-03] - } - activeBots[creditAccount].remove(bot); // F: [BL-03] - } + botPermissions[creditAccount][bot] = permissions; // F: [BL-03] - botPermissions[creditAccount][bot] = permissions; // F: [BL-03] - botFunding[creditAccount][bot].remainingFunds = fundingAmount; // F: [BL-03] - botFunding[creditAccount][bot].maxWeeklyAllowance = weeklyFundingAllowance; // F: [BL-03] - botFunding[creditAccount][bot].remainingWeeklyAllowance = weeklyFundingAllowance; // F: [BL-03] - botFunding[creditAccount][bot].allowanceLU = uint40(block.timestamp); // F: [BL-03] + BotFunding storage bf = botFunding[creditAccount][bot]; - activeBotsRemaining = activeBots[creditAccount].length(); // F: [BL-03] + bf.remainingFunds = fundingAmount; // F: [BL-03] + bf.maxWeeklyAllowance = weeklyFundingAllowance; // F: [BL-03] + bf.remainingWeeklyAllowance = weeklyFundingAllowance; // F: [BL-03] + bf.allowanceLU = uint40(block.timestamp); // F: [BL-03] - emit SetBotPermissions(creditAccount, bot, permissions, fundingAmount, weeklyFundingAllowance); // F: [BL-03] + emit SetBotPermissions({ + creditAccount: creditAccount, + bot: bot, + permissions: permissions, + fundingAmount: fundingAmount, + weeklyFundingAllowance: weeklyFundingAllowance + }); // F: [BL-03] + } else { + _ereaseBot(creditAccount, bot); // F: [BL-03] + } + + activeBotsRemaining = activeBots[creditAccount].length(); // F: [BL-03] } /// @notice Removes permissions and funding for all bots with non-zero permissions for a credit account @@ -127,21 +142,20 @@ contract BotListV3 is ACLNonReentrantTrait, IBotListV3 { { uint256 len = activeBots[creditAccount].length(); - for (uint256 i = 0; i < len;) { - address bot = activeBots[creditAccount].at(0); // F: [BL-06] - botPermissions[creditAccount][bot] = 0; // F: [BL-06] - botFunding[creditAccount][bot].remainingFunds = 0; // F: [BL-06] - botFunding[creditAccount][bot].maxWeeklyAllowance = 0; // F: [BL-06] - botFunding[creditAccount][bot].remainingWeeklyAllowance = 0; // F: [BL-06] - activeBots[creditAccount].remove(bot); // F: [BL-06] - unchecked { - ++i; + unchecked { + for (uint256 i = 0; i < len; ++i) { + address bot = activeBots[creditAccount].at(len - i - 1); // F: [BL-06] + _ereaseBot(creditAccount, bot); } } + } - if (len > 0) { - emit EraseBots(creditAccount); // F: [BL-06] - } + function _ereaseBot(address creditAccount, address bot) internal { + delete botPermissions[creditAccount][bot]; // F: [BL-06] + delete botFunding[creditAccount][bot]; // F: [BL-06] + + activeBots[creditAccount].remove(bot); // F: [BL-06] + emit EraseBot(creditAccount, bot); // F: [BL-06] } /// @notice Takes payment for performed services from the user's balance and sends to the bot @@ -153,9 +167,7 @@ contract BotListV3 is ACLNonReentrantTrait, IBotListV3 { external onlyValidCreditFacade { - if (paymentAmount == 0) { - revert AmountCantBeZeroException(); // F: [BL-05] - } + if (paymentAmount == 0) return; BotFunding storage bf = botFunding[creditAccount][bot]; // F: [BL-05] @@ -164,40 +176,45 @@ contract BotListV3 is ACLNonReentrantTrait, IBotListV3 { bf.remainingWeeklyAllowance = bf.maxWeeklyAllowance; // F: [BL-05] } - uint72 feeAmount = daoFee * paymentAmount / PERCENTAGE_FACTOR; // F: [BL-05] + /// feeAmount is always < paymentAmount, however uint256 conversation adds more space for computations + uint72 feeAmount = uint72(uint256(daoFee) * paymentAmount / PERCENTAGE_FACTOR); // F: [BL-05] + + uint72 totalAmount = paymentAmount + feeAmount; + + bf.remainingWeeklyAllowance -= totalAmount; // F: [BL-05] + bf.remainingFunds -= totalAmount; // F: [BL-05] - bf.remainingWeeklyAllowance -= paymentAmount + feeAmount; // F: [BL-05] - bf.remainingFunds -= paymentAmount + feeAmount; // F: [BL-05] + balanceOf[payer] -= totalAmount; // F: [BL-05] - fundingBalances[payer] -= paymentAmount + feeAmount; // F: [BL-05] + IERC20(weth).safeTransfer(bot, paymentAmount); // F: [BL-05] - _convertAndSendWETH(bot, paymentAmount); - if (feeAmount > 0) payable(treasury).sendValue(feeAmount); // F: [BL-05] + if (feeAmount != 0) { + IERC20(weth).safeTransfer(treasury, feeAmount); // F: [BL-05] + } emit PayBot(payer, creditAccount, bot, paymentAmount, feeAmount); // F: [BL-05] } /// @notice Adds funds to the borrower's bot payment wallet - function addFunding() external payable nonReentrant { + function deposit() public payable nonReentrant { if (msg.value == 0) { revert AmountCantBeZeroException(); // F: [BL-04] } - uint256 newFunds = fundingBalances[msg.sender] + msg.value; // F: [BL-04] - - fundingBalances[msg.sender] = newFunds; // F: [BL-04] + IWETH(weth).deposit{value: msg.value}(); + balanceOf[msg.sender] += msg.value; - emit ChangeFunding(msg.sender, newFunds); // F: [BL-04] + emit Deposit(msg.sender, msg.value); // F: [BL-04] } /// @notice Removes funds from the borrower's bot payment wallet - function removeFunding(uint256 amount) external nonReentrant { - uint256 newFunds = fundingBalances[msg.sender] - amount; // F: [BL-04] + function withdraw(uint256 amount) external nonReentrant { + balanceOf[msg.sender] -= amount; // F: [BL-04] - fundingBalances[msg.sender] = newFunds; // F: [BL-04] + IWETH(weth).withdraw(amount); payable(msg.sender).sendValue(amount); // F: [BL-04] - emit ChangeFunding(msg.sender, newFunds); // F: [BL-04] + emit Withdraw(msg.sender, amount); // F: [BL-04] } /// @notice Returns all active bots currently on the account @@ -214,13 +231,6 @@ contract BotListV3 is ACLNonReentrantTrait, IBotListV3 { return (botPermissions[creditAccount][bot], forbiddenBot[bot]); } - /// @dev Converts ETH to WETH and sends to recipient - /// @dev Used to avoid giving flow control to bots - function _convertAndSendWETH(address to, uint256 amount) internal { - IWETH(weth).deposit{value: amount}(); - IWETH(weth).transfer(to, amount); - } - // // CONFIGURATION // @@ -234,6 +244,10 @@ contract BotListV3 is ACLNonReentrantTrait, IBotListV3 { /// @notice Sets the DAO fee on bot payments /// @param newFee The new fee value function setDAOFee(uint16 newFee) external configuratorOnly { + if (daoFee > PERCENTAGE_FACTOR) { + revert IncorrectParameterException(); + } + daoFee = newFee; // F: [BL-02] emit SetBotDAOFee(newFee); // F: [BL-02] @@ -251,4 +265,9 @@ contract BotListV3 is ACLNonReentrantTrait, IBotListV3 { emit CreditManagerRemoved(creditManager); } } + + /// @notice Allows this contract to unwrap WETH and deposit if address is not WETH + receive() external payable { + if (msg.sender != address(weth)) deposit(); + } } diff --git a/contracts/test/suites/PoolDeployer.sol b/contracts/test/suites/PoolDeployer.sol index d016a434..68fbd657 100644 --- a/contracts/test/suites/PoolDeployer.sol +++ b/contracts/test/suites/PoolDeployer.sol @@ -90,7 +90,7 @@ contract PoolDeployer is Test { withdrawalManager = WithdrawalManagerV3(addressProvider.getAddressOrRevert(AP_WITHDRAWAL_MANAGER, 3_00)); - botList = BotListV3(addressProvider.getAddressOrRevert(AP_BOT_LIST, 3_00)); + botList = BotListV3(payable(addressProvider.getAddressOrRevert(AP_BOT_LIST, 3_00))); underlying = _underlying; diff --git a/contracts/test/unit/support/BotList.t.sol b/contracts/test/unit/support/BotList.t.sol index 709110dc..85f06a95 100644 --- a/contracts/test/unit/support/BotList.t.sol +++ b/contracts/test/unit/support/BotList.t.sol @@ -262,36 +262,36 @@ contract BotListTest is Test, IBotListV3Events { assertEq(bots.length, 0, "Incorrect active bots array length"); } - /// @dev [BL-4]: addFunding and removeFunding work correctly - function test_BL_04_addFunding_removeFunding_work_correctly() public { + /// @dev [BL-4]: deposit and withdraw work correctly + function test_BL_04_deposit_withdraw_work_correctly() public { vm.deal(USER, 10 ether); vm.expectRevert(AmountCantBeZeroException.selector); - botList.addFunding(); + botList.deposit(); vm.expectEmit(true, false, false, true); - emit ChangeFunding(USER, 1 ether); + emit Deposit(USER, 1 ether); vm.prank(USER); - botList.addFunding{value: 1 ether}(); + botList.deposit{value: 1 ether}(); - assertEq(botList.fundingBalances(USER), 1 ether, "User's bot funding wallet has incorrect balance"); + assertEq(botList.balanceOf(USER), 1 ether, "User's bot funding wallet has incorrect balance"); vm.expectEmit(true, false, false, true); - emit ChangeFunding(USER, 2 ether); + emit Deposit(USER, 1 ether); vm.prank(USER); - botList.addFunding{value: 1 ether}(); + botList.deposit{value: 1 ether}(); - assertEq(botList.fundingBalances(USER), 2 ether, "User's bot funding wallet has incorrect balance"); + assertEq(botList.balanceOf(USER), 2 ether, "User's bot funding wallet has incorrect balance"); vm.expectEmit(true, false, false, true); - emit ChangeFunding(USER, 3 ether / 2); + emit Withdraw(USER, 1 ether / 2); vm.prank(USER); - botList.removeFunding(1 ether / 2); + botList.withdraw(1 ether / 2); - assertEq(botList.fundingBalances(USER), 3 ether / 2, "User's bot funding wallet has incorrect balance"); + assertEq(botList.balanceOf(USER), 3 ether / 2, "User's bot funding wallet has incorrect balance"); assertEq(USER.balance, 85 ether / 10, "User's balance is incorrect"); } @@ -310,7 +310,7 @@ contract BotListTest is Test, IBotListV3Events { vm.deal(USER, 10 ether); vm.prank(USER); - botList.addFunding{value: 2 ether}(); + botList.deposit{value: 2 ether}(); vm.prank(address(creditFacade)); botList.setBotPermissions({ @@ -355,7 +355,7 @@ contract BotListTest is Test, IBotListV3Events { ); assertEq( - botList.fundingBalances(USER), + botList.balanceOf(USER), 2 ether - (1 ether / 20) - (1 ether / 40), "User remaining funding balance incorrect" ); @@ -390,7 +390,7 @@ contract BotListTest is Test, IBotListV3Events { assertEq(allowanceLU, block.timestamp, "Allowance update timestamp incorrect"); assertEq( - botList.fundingBalances(USER), + botList.balanceOf(USER), 2 ether - (2 ether / 20) - (2 ether / 40), "User remaining funding balance incorrect" ); @@ -428,8 +428,11 @@ contract BotListTest is Test, IBotListV3Events { vm.prank(invalidCF); botList.eraseAllBotPermissions(address(creditAccount)); - vm.expectEmit(true, false, false, false); - emit EraseBots(address(creditAccount)); + vm.expectEmit(true, true, false, false); + emit EraseBot(address(creditAccount), address(bot)); + + vm.expectEmit(true, true, false, false); + emit EraseBot(address(creditAccount), address(bot2)); vm.prank(address(creditFacade)); botList.eraseAllBotPermissions(address(creditAccount)); From 43dbea6d60cd6534b6187f791cf18b511d2cdf29 Mon Sep 17 00:00:00 2001 From: 0xmikko <26343374+0xmikko@users.noreply.github.com> Date: Wed, 31 May 2023 13:53:56 +0200 Subject: [PATCH 4/4] fix: small typos fixed --- contracts/support/BotListV3.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/support/BotListV3.sol b/contracts/support/BotListV3.sol index 46342482..b2ea3569 100644 --- a/contracts/support/BotListV3.sol +++ b/contracts/support/BotListV3.sol @@ -128,7 +128,7 @@ contract BotListV3 is ACLNonReentrantTrait, IBotListV3 { weeklyFundingAllowance: weeklyFundingAllowance }); // F: [BL-03] } else { - _ereaseBot(creditAccount, bot); // F: [BL-03] + _eraseBot(creditAccount, bot); // F: [BL-03] } activeBotsRemaining = activeBots[creditAccount].length(); // F: [BL-03] @@ -145,12 +145,12 @@ contract BotListV3 is ACLNonReentrantTrait, IBotListV3 { unchecked { for (uint256 i = 0; i < len; ++i) { address bot = activeBots[creditAccount].at(len - i - 1); // F: [BL-06] - _ereaseBot(creditAccount, bot); + _eraseBot(creditAccount, bot); } } } - function _ereaseBot(address creditAccount, address bot) internal { + function _eraseBot(address creditAccount, address bot) internal { delete botPermissions[creditAccount][bot]; // F: [BL-06] delete botFunding[creditAccount][bot]; // F: [BL-06] @@ -268,6 +268,6 @@ contract BotListV3 is ACLNonReentrantTrait, IBotListV3 { /// @notice Allows this contract to unwrap WETH and deposit if address is not WETH receive() external payable { - if (msg.sender != address(weth)) deposit(); + if (msg.sender != weth) deposit(); } }