Skip to content

Commit

Permalink
fix: uint256 withdraw all tokens feature
Browse files Browse the repository at this point in the history
  • Loading branch information
@0xmikko
0xmikko committed Oct 31, 2023
1 parent 8d2392b commit 8251461
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 156 deletions.
51 changes: 12 additions & 39 deletions contracts/credit/CreditFacadeV3.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {
} from "../interfaces/ICreditManagerV3.sol";
import {AllowanceAction} from "../interfaces/ICreditConfiguratorV3.sol";
import {ETH_ADDRESS, IWithdrawalManagerV3} from "../interfaces/IWithdrawalManagerV3.sol";
import {IPriceOracleBase} from "@gearbox-protocol/core-v2/contracts/interfaces/IPriceOracleBase.sol";
import {IPriceOracleV3} from "../interfaces/IPriceOracleV3.sol";
import {IUpdatablePriceFeed} from "@gearbox-protocol/core-v2/contracts/interfaces/IPriceFeed.sol";

import {IPoolV3} from "../interfaces/IPoolV3.sol";
Expand Down Expand Up @@ -440,10 +440,6 @@ contract CreditFacadeV3 is ICreditFacadeV3, ACLNonReentrantTrait {
revert NotApprovedBotException(); // U:[FA-19]
}

if (!hasSpecialPermissions) {
botPermissions = botPermissions.enable(PAY_BOT_CAN_BE_CALLED);
}

_multicallFullCollateralCheck(creditAccount, calls, botPermissions); // U:[FA-19, 20]
}

Expand Down Expand Up @@ -594,20 +590,15 @@ contract CreditFacadeV3 is ICreditFacadeV3, ACLNonReentrantTrait {
enabledTokensMask = enabledTokensMask.enableDisable(tokensToEnable, tokensToDisable); // U:[FA-34]
}
// withdraw
else if (
method == ICreditFacadeV3Multicall.withdraw.selector
|| method == ICreditFacadeV3Multicall.withdrawAll.selector
) {
else if (method == ICreditFacadeV3Multicall.withdraw.selector) {
_revertIfNoPermission(flags, WITHDRAW_PERMISSION); // U:[FA-21]

flags = flags.enable(REVERT_ON_FORBIDDEN_TOKENS_AFTER_CALLS); // U:[FA-30]

// It enabled additinal check that all collateral tokens pricefeeds work correctly
fullCheckParams.reservePriceFeedCheck = true;

uint256 tokensToDisable = method == ICreditFacadeV3Multicall.withdraw.selector
? _withdrawAmount(creditAccount, mcall.callData[4:])
: _withdrawAll(creditAccount, mcall.callData[4:]); // U:[FA-34]
uint256 tokensToDisable = _withdraw(creditAccount, mcall.callData[4:]); // U:[FA-34]

quotedTokensMaskInverted = _getInvertedQuotedTokensMask(quotedTokensMaskInverted);

Expand Down Expand Up @@ -636,12 +627,6 @@ contract CreditFacadeV3 is ICreditFacadeV3, ACLNonReentrantTrait {
); // U:[FA-31]
enabledTokensMask = enabledTokensMask.disable(tokensToDisable); // U:[FA-31]
}
// payBot
else if (method == ICreditFacadeV3Multicall.payBot.selector) {
_revertIfNoPermission(flags, PAY_BOT_CAN_BE_CALLED); // U:[FA-21]
flags = flags.disable(PAY_BOT_CAN_BE_CALLED); // U:[FA-37]
_payBot(creditAccount, mcall.callData[4:]); // U:[FA-37]
}
// setFullCheckParams
else if (method == ICreditFacadeV3Multicall.setFullCheckParams.selector) {
(fullCheckParams.collateralHints, fullCheckParams.minHealthFactor) =
Expand Down Expand Up @@ -746,10 +731,12 @@ contract CreditFacadeV3 is ICreditFacadeV3, ACLNonReentrantTrait {
mcall.target == address(this)
&& bytes4(mcall.callData) == ICreditFacadeV3Multicall.onDemandPriceUpdate.selector
) {
(address token, bytes memory data) = abi.decode(mcall.callData[4:], (address, bytes)); // U:[FA-25]
(address token, bool reserve, bytes memory data) =
abi.decode(mcall.callData[4:], (address, bool, bytes)); // U:[FA-25]

priceOracle = _getPriceOracle(priceOracle); // U:[FA-25]
address priceFeed = IPriceOracleBase(priceOracle).priceFeeds(token); // U:[FA-25]
address priceFeed = IPriceOracleV3(priceOracle).priceFeedsRaw(token, reserve); // U:[FA-25]

if (priceFeed == address(0)) {
revert PriceFeedDoesNotExistException(); // U:[FA-25]
}
Expand Down Expand Up @@ -879,30 +866,16 @@ contract CreditFacadeV3 is ICreditFacadeV3, ACLNonReentrantTrait {
}); // U:[FA-34]
}

function _withdrawAmount(address creditAccount, bytes calldata callData)
internal
returns (uint256 tokensToDisable)
{
function _withdraw(address creditAccount, bytes calldata callData) internal returns (uint256 tokensToDisable) {
(address token, uint256 amount, address to) = abi.decode(callData, (address, uint256, address)); // U:[FA-35]
tokensToDisable = _withdraw(creditAccount, token, amount, to);
}

function _withdrawAll(address creditAccount, bytes calldata callData) internal returns (uint256 tokensToDisable) {
(address token, address to) = abi.decode(callData, (address, address)); // U:[FA-35]

uint256 amount = IERC20(token).balanceOf(creditAccount);
if (amount > 1) {
if (amount == type(uint256).max) {
uint256 amount = IERC20(token).balanceOf(creditAccount);
if (amount <= 1) return 0;
unchecked {
tokensToDisable = _withdraw(creditAccount, token, amount - 1, to);
--amount;
}
}
}

/// @dev `ICreditFacadeV3Multicall.withdraw` implementation
function _withdraw(address creditAccount, address token, uint256 amount, address to)
internal
returns (uint256 tokensToDisable)
{
tokensToDisable = ICreditManagerV3(creditManager).withdraw(creditAccount, token, amount, to); // U:[FA-35]
}

Expand Down
19 changes: 2 additions & 17 deletions contracts/interfaces/ICreditFacadeV3Multicall.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,20 +42,17 @@ uint256 constant REVERT_ON_FORBIDDEN_TOKENS_AFTER_CALLS = 1 << 193;
/// set to true on the first call to the adapter
uint256 constant EXTERNAL_CONTRACT_WAS_CALLED = 1 << 194;

/// @dev Indicates that `payBot` can be called during multicall to fund the bot, set to true when
/// multicall is initiated in `botMulticall` and reset after the first `payBot` call
uint256 constant PAY_BOT_CAN_BE_CALLED = 1 << 195;

/// @title Credit facade V3 multicall interface
/// @dev Unless specified otherwise, all these methods are only available in `openCreditAccount`,
/// `closeCreditAccount`, `multicall`, and, with account owner's permission, `botMulticall`
interface ICreditFacadeV3Multicall {
/// @notice Updates the price for a token with on-demand updatable price feed
/// @param token Token to push the price update for
/// @param reserve Whether to update reserve price feed or main price feed
/// @param data Data to call `updatePrice` with
/// @dev Calls of this type must be placed before all other calls in the multicall not to revert
/// @dev This method is available in all kinds of multicalls
function onDemandPriceUpdate(address token, bytes calldata data) external;
function onDemandPriceUpdate(address token, bool reserve, bytes calldata data) external;

/// @notice Ensures that token balances increase at least by specified deltas after the following calls
/// @param balanceDeltas Array of (token, minBalanceDelta) pairs, deltas are allowed to be negative
Expand Down Expand Up @@ -115,18 +112,6 @@ interface ICreditFacadeV3Multicall {
/// @dev Withdrawals are prohibited when opening or closing an account
function withdraw(address token, uint256 amount, address to) external;

/// @notice Withdraw tokens from account
/// @param token Token to withdraw
/// @param amount Amount to withdraw
/// @dev Withdrawals are prohibited if there are forbidden tokens enabled as collateral on the account
/// @dev Withdrawals are prohibited when opening or closing an account
function withdrawAll(address token, address to) external;

/// @notice Requests bot list to make a payment to the caller
/// @param paymentAmount Paymenet amount in WETH
/// @dev This method is only available in `botMulticall` and can only be called once
function payBot(uint72 paymentAmount) external;

/// @notice Sets advanced collateral check parameters
/// @param collateralHints Optional array of token masks to check first to reduce the amount of computation
/// when known subset of account's collateral tokens covers all the debt
Expand Down
103 changes: 3 additions & 100 deletions contracts/test/unit/credit/CreditFacadeV3.unit.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1025,53 +1025,6 @@ contract CreditFacadeV3UnitTest is TestHelper, BalanceHelper, ICreditFacadeV3Eve
creditFacade.botMulticall(creditAccount, calls);
}

/// @dev U:[FA-20]: only botMulticall doesn't revert for pay bot call
function test_U_FA_20_only_botMulticall_doesnt_revert_for_pay_bot_call() public notExpirableCase {
address creditAccount = DUMB_ADDRESS;

creditManagerMock.setFlagFor(creditAccount, BOT_PERMISSIONS_SET_FLAG, true);

vm.prank(CONFIGURATOR);
creditFacade.setDebtLimits(1, 100, 1);

creditManagerMock.setBorrower(USER);
botListMock.setBotStatusReturns(ALL_PERMISSIONS, false, false);

MultiCall[] memory calls = MultiCallBuilder.build(
MultiCall({target: address(creditFacade), callData: abi.encodeCall(ICreditFacadeV3Multicall.payBot, (1))})
);

vm.expectRevert(abi.encodeWithSelector(NoPermissionException.selector, PAY_BOT_CAN_BE_CALLED));
creditFacade.openCreditAccount({onBehalfOf: USER, calls: calls, referralCode: 0});

vm.prank(USER);
vm.expectRevert(abi.encodeWithSelector(NoPermissionException.selector, PAY_BOT_CAN_BE_CALLED));
creditFacade.multicall(creditAccount, calls);

/// Case: it works for bot multicall
creditFacade.botMulticall(creditAccount, calls);
}

/// @dev U:[FA-20A]: botMulticall reverts for payBot calls for bots with special permissions
function test_U_FA_20A_payBot_reverts_for_special_bots() public notExpirableCase {
address creditAccount = DUMB_ADDRESS;

creditManagerMock.setFlagFor(creditAccount, BOT_PERMISSIONS_SET_FLAG, true);

vm.prank(CONFIGURATOR);
creditFacade.setDebtLimits(1, 100, 1);

creditManagerMock.setBorrower(USER);
botListMock.setBotStatusReturns(ALL_PERMISSIONS, false, true);

MultiCall[] memory calls = MultiCallBuilder.build(
MultiCall({target: address(creditFacade), callData: abi.encodeCall(ICreditFacadeV3Multicall.payBot, (1))})
);

vm.expectRevert(abi.encodeWithSelector(NoPermissionException.selector, PAY_BOT_CAN_BE_CALLED));
creditFacade.botMulticall(creditAccount, calls);
}

struct MultiCallPermissionTestCase {
bytes callData;
uint256 permissionRquired;
Expand All @@ -1097,7 +1050,7 @@ contract CreditFacadeV3UnitTest is TestHelper, BalanceHelper, ICreditFacadeV3Eve

creditManagerMock.setBorrower(USER);

MultiCallPermissionTestCase[10] memory cases = [
MultiCallPermissionTestCase[9] memory cases = [
MultiCallPermissionTestCase({
callData: abi.encodeCall(ICreditFacadeV3Multicall.enableToken, (token)),
permissionRquired: ENABLE_TOKEN_PERMISSION
Expand Down Expand Up @@ -1135,10 +1088,6 @@ contract CreditFacadeV3UnitTest is TestHelper, BalanceHelper, ICreditFacadeV3Eve
MultiCallPermissionTestCase({
callData: abi.encodeCall(ICreditFacadeV3Multicall.revokeAdapterAllowances, (new RevocationPair[](0))),
permissionRquired: REVOKE_ALLOWANCES_PERMISSION
}),
MultiCallPermissionTestCase({
callData: abi.encodeCall(ICreditFacadeV3Multicall.payBot, (0)),
permissionRquired: PAY_BOT_CAN_BE_CALLED
})
];

Expand Down Expand Up @@ -1285,7 +1234,7 @@ contract CreditFacadeV3UnitTest is TestHelper, BalanceHelper, ICreditFacadeV3Eve
MultiCall[] memory calls = MultiCallBuilder.build(
MultiCall({
target: address(creditFacade),
callData: abi.encodeCall(ICreditFacadeV3Multicall.onDemandPriceUpdate, (token, cd))
callData: abi.encodeCall(ICreditFacadeV3Multicall.onDemandPriceUpdate, (token, false, cd))
})
);

Expand All @@ -1297,7 +1246,7 @@ contract CreditFacadeV3UnitTest is TestHelper, BalanceHelper, ICreditFacadeV3Eve
calls = MultiCallBuilder.build(
MultiCall({
target: address(creditFacade),
callData: abi.encodeCall(ICreditFacadeV3Multicall.onDemandPriceUpdate, (DUMB_ADDRESS, cd))
callData: abi.encodeCall(ICreditFacadeV3Multicall.onDemandPriceUpdate, (DUMB_ADDRESS, false, cd))
})
);

Expand Down Expand Up @@ -1860,52 +1809,6 @@ contract CreditFacadeV3UnitTest is TestHelper, BalanceHelper, ICreditFacadeV3Eve
});
}

/// @dev U:[FA-37]: multicall payBot works properly
function test_U_FA_37_multicall_payBot_works_properly() public notExpirableCase {
address creditAccount = DUMB_ADDRESS;

creditManagerMock.setBorrower(USER);

uint72 paymentAmount = 100_000;

address bot = makeAddr("BOT");
vm.expectCall(
address(botListMock),
abi.encodeCall(IBotListV3.payBot, (USER, address(creditManagerMock), creditAccount, bot, paymentAmount))
);

vm.prank(bot);
creditFacade.multicallInt({
creditAccount: creditAccount,
calls: MultiCallBuilder.build(
MultiCall({
target: address(creditFacade),
callData: abi.encodeCall(ICreditFacadeV3Multicall.payBot, (paymentAmount))
})
),
enabledTokensMask: 0,
flags: PAY_BOT_CAN_BE_CALLED
});

vm.expectRevert(abi.encodeWithSelector(NoPermissionException.selector, PAY_BOT_CAN_BE_CALLED));
vm.prank(bot);
creditFacade.multicallInt({
creditAccount: creditAccount,
calls: MultiCallBuilder.build(
MultiCall({
target: address(creditFacade),
callData: abi.encodeCall(ICreditFacadeV3Multicall.payBot, (paymentAmount))
}),
MultiCall({
target: address(creditFacade),
callData: abi.encodeCall(ICreditFacadeV3Multicall.payBot, (paymentAmount))
})
),
enabledTokensMask: 0,
flags: PAY_BOT_CAN_BE_CALLED
});
}

struct ExternalCallTestCase {
string name;
uint256 quotedTokensMask;
Expand Down

0 comments on commit 8251461

Please sign in to comment.