Skip to content

Commit

Permalink
fix: activeCreditAccount naming
Browse files Browse the repository at this point in the history
  • Loading branch information
0xmikko committed May 18, 2023
1 parent 655dd50 commit 4eef05b
Show file tree
Hide file tree
Showing 12 changed files with 60 additions and 97 deletions.
2 changes: 1 addition & 1 deletion contracts/adapters/AbstractAdapter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ abstract contract AbstractAdapter is IAdapter, ACLTrait {

/// @dev Ensures that external call credit account is set and returns its address
function _creditAccount() internal view returns (address) {
return ICreditManagerV3(creditManager).getExternalCallCreditAccountOrRevert(); // U:[AA-3]
return ICreditManagerV3(creditManager).getActiveCreditAccountOrRevert(); // U:[AA-3]
}

/// @dev Ensures that token is registered as collateral in the credit manager and returns its mask
Expand Down
19 changes: 7 additions & 12 deletions contracts/credit/CreditFacadeV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ contract CreditFacadeV3 is ICreditFacade, ACLNonReentrantTrait {

if (flags & EXTERNAL_CONTRACT_WAS_CALLED == 0) {
flags = flags.enable(EXTERNAL_CONTRACT_WAS_CALLED);
_setCaForExterallCall(creditAccount);
_setActiveCreditAccount(creditAccount);
}

// Makes a call
Expand Down Expand Up @@ -733,7 +733,7 @@ contract CreditFacadeV3 is ICreditFacade, ACLNonReentrantTrait {
}

if (flags & EXTERNAL_CONTRACT_WAS_CALLED != 0) {
_returnCaForExterallCall();
_unsetActiveCreditAccount();
}

// Emits event for multicall end - used in analytics to track actions within multicalls
Expand All @@ -743,18 +743,13 @@ contract CreditFacadeV3 is ICreditFacade, ACLNonReentrantTrait {
fullCheckParams.enabledTokensMaskAfter = enabledTokensMask;
}

function _setCaForExterallCall(address creditAccount) internal {
// Takes ownership of the Credit Account
_setCreditAccountForExternalCall(creditAccount); // F:[FA-26]
function _setActiveCreditAccount(address creditAccount) internal {
ICreditManagerV3(creditManager).setActiveCreditAccount(creditAccount); // F:[FA-26]
}

function _returnCaForExterallCall() internal {
// Takes ownership of the Credit Account
_setCreditAccountForExternalCall(address(1)); // F:[FA-26]
}

function _setCreditAccountForExternalCall(address creditAccount) internal {
ICreditManagerV3(creditManager).setCreditAccountForExternalCall(creditAccount); // F:[FA-26]
/// @dev returns activeCreditAccount to address(1) which means that multicall is not active
function _unsetActiveCreditAccount() internal {
_setActiveCreditAccount(address(1)); // F:[FA-26]
}

function _revertIfNoPermission(uint256 flags, uint256 permission) internal pure {
Expand Down
20 changes: 10 additions & 10 deletions contracts/credit/CreditManagerV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ contract CreditManagerV3 is ICreditManagerV3, SanityCheckTrait, ReentrancyGuardT

/// @dev Points to creditAccount during multicall, otherwise keeps address(1) for gas savings
/// CreditFacade is trusted source, so primarly it sends creditAccount as parameter
/// _externalCallCreditAccount is used for adapters interation when adapter calls approve / execute methods
address internal _externalCallCreditAccount;
/// _activeCreditAccount is used for adapters interation when adapter calls approve / execute methods
address internal _activeCreditAccount;

/// @dev Total number of known collateral tokens.
uint8 public collateralTokensCount;
Expand Down Expand Up @@ -221,7 +221,7 @@ contract CreditManagerV3 is ICreditManagerV3, SanityCheckTrait, ReentrancyGuardT

creditConfigurator = msg.sender; // U:[CM-1]

_externalCallCreditAccount = address(1);
_activeCreditAccount = address(1);
}

//
Expand Down Expand Up @@ -520,7 +520,7 @@ contract CreditManagerV3 is ICreditManagerV3, SanityCheckTrait, ReentrancyGuardT
{
address targetContract = _getTargetContractOrRevert(); // U:[CM-3]
_approveSpender({
creditAccount: getExternalCallCreditAccountOrRevert(),
creditAccount: getActiveCreditAccountOrRevert(),
token: token,
spender: targetContract,
amount: amount
Expand Down Expand Up @@ -552,7 +552,7 @@ contract CreditManagerV3 is ICreditManagerV3, SanityCheckTrait, ReentrancyGuardT
// Returned data is provided as-is to the caller;
// It is expected that is is parsed and returned as a correct type
// by the adapter itself.
address creditAccount = getExternalCallCreditAccountOrRevert(); // U:[CM-16]
address creditAccount = getActiveCreditAccountOrRevert(); // U:[CM-16]
return ICreditAccount(creditAccount).execute(targetContract, data); // U:[CM-16]
}

Expand Down Expand Up @@ -1204,18 +1204,18 @@ contract CreditManagerV3 is ICreditManagerV3, SanityCheckTrait, ReentrancyGuardT
}

///
function setCreditAccountForExternalCall(address creditAccount)
function setActiveCreditAccount(address creditAccount)
external
override
nonReentrant // U:[CM-5]
creditFacadeOnly // U:[CM-2]
{
_externalCallCreditAccount = creditAccount;
_activeCreditAccount = creditAccount;
}

function getExternalCallCreditAccountOrRevert() public view override returns (address creditAccount) {
creditAccount = _externalCallCreditAccount;
if (creditAccount == address(1)) revert ExternalCallCreditAccountNotSetException();
function getActiveCreditAccountOrRevert() public view override returns (address creditAccount) {
creditAccount = _activeCreditAccount;
if (creditAccount == address(1)) revert ActiveCreditAccountNotSetException();
}

function enabledTokensMaskOf(address creditAccount) public view override returns (uint256) {
Expand Down
4 changes: 2 additions & 2 deletions contracts/interfaces/ICreditManagerV3.sol
Original file line number Diff line number Diff line change
Expand Up @@ -323,9 +323,9 @@ interface ICreditManagerV3 is ICreditManagerV3Events, IVersion {
/// @param revocations Spender/token pairs to revoke allowances for
function revokeAdapterAllowances(address creditAccount, RevocationPair[] calldata revocations) external;

function setCreditAccountForExternalCall(address creditAccount) external;
function setActiveCreditAccount(address creditAccount) external;

function getExternalCallCreditAccountOrRevert() external view returns (address creditAccount);
function getActiveCreditAccountOrRevert() external view returns (address creditAccount);

function getTokenByMask(uint256 tokenMask) external view returns (address token);

Expand Down
2 changes: 1 addition & 1 deletion contracts/interfaces/IExceptions.sol
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ error NoFreeWithdrawalSlotsException();

error NoPermissionException(uint256 permission);

error ExternalCallCreditAccountNotSetException();
error ActiveCreditAccountNotSetException();

/// @dev Thrown when the caller is not a Credit Facade associated with required Credit Account
error CallerNotCreditAccountFacadeException();
Expand Down
36 changes: 10 additions & 26 deletions contracts/test/integration/credit/CreditFacade.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -699,9 +699,7 @@ contract CreditFacadeIntegrationTest is
MultiCall({target: address(adapterMock), callData: abi.encodeCall(AdapterMock.dumbCall, (0, 0))})
);

vm.expectCall(
address(creditManager), abi.encodeCall(ICreditManagerV3.setCreditAccountForExternalCall, (creditAccount))
);
vm.expectCall(address(creditManager), abi.encodeCall(ICreditManagerV3.setActiveCreditAccount, (creditAccount)));

vm.expectEmit(true, false, false, false);
emit StartMultiCall(creditAccount);
Expand All @@ -720,9 +718,7 @@ contract CreditFacadeIntegrationTest is

vm.expectCall(address(botList), abi.encodeCall(BotList.eraseAllBotPermissions, (creditAccount)));

vm.expectCall(
address(creditManager), abi.encodeCall(ICreditManagerV3.setCreditAccountForExternalCall, (address(1)))
);
vm.expectCall(address(creditManager), abi.encodeCall(ICreditManagerV3.setActiveCreditAccount, (address(1))));

// vm.expectCall(
// address(creditManager),
Expand Down Expand Up @@ -791,9 +787,7 @@ contract CreditFacadeIntegrationTest is

// EXPECTED STACK TRACE & EVENTS

vm.expectCall(
address(creditManager), abi.encodeCall(ICreditManagerV3.setCreditAccountForExternalCall, (creditAccount))
);
vm.expectCall(address(creditManager), abi.encodeCall(ICreditManagerV3.setActiveCreditAccount, (creditAccount)));

vm.expectEmit(true, false, false, false);
emit StartMultiCall(creditAccount);
Expand All @@ -812,9 +806,7 @@ contract CreditFacadeIntegrationTest is

vm.expectCall(address(botList), abi.encodeCall(BotList.eraseAllBotPermissions, (creditAccount)));

vm.expectCall(
address(creditManager), abi.encodeCall(ICreditManagerV3.setCreditAccountForExternalCall, (address(1)))
);
vm.expectCall(address(creditManager), abi.encodeCall(ICreditManagerV3.setActiveCreditAccount, (address(1))));

// Total value = 2 * DAI_ACCOUNT_AMOUNT, cause we have x2 leverage
uint256 totalValue = 2 * DAI_ACCOUNT_AMOUNT;
Expand Down Expand Up @@ -1384,9 +1376,7 @@ contract CreditFacadeIntegrationTest is
MultiCall({target: address(adapterMock), callData: abi.encodeCall(AdapterMock.dumbCall, (0, 0))})
);

vm.expectCall(
address(creditManager), abi.encodeCall(ICreditManagerV3.setCreditAccountForExternalCall, (creditAccount))
);
vm.expectCall(address(creditManager), abi.encodeCall(ICreditManagerV3.setActiveCreditAccount, (creditAccount)));

vm.expectEmit(true, true, false, true);
emit StartMultiCall(creditAccount);
Expand All @@ -1403,9 +1393,7 @@ contract CreditFacadeIntegrationTest is
vm.expectEmit(false, false, false, true);
emit FinishMultiCall();

vm.expectCall(
address(creditManager), abi.encodeCall(ICreditManagerV3.setCreditAccountForExternalCall, (address(1)))
);
vm.expectCall(address(creditManager), abi.encodeCall(ICreditManagerV3.setActiveCreditAccount, (address(1))));

vm.expectCall(
address(creditManager),
Expand Down Expand Up @@ -1890,7 +1878,7 @@ contract CreditFacadeIntegrationTest is

// // EXPECTED STACK TRACE & EVENTS

// vm.expectCall(address(creditManager), abi.encodeCall(ICreditManagerV3.setCreditAccountForExternalCall, (creditAccount)));
// vm.expectCall(address(creditManager), abi.encodeCall(ICreditManagerV3.setActiveCreditAccount, (creditAccount)));

// vm.expectEmit(true, false, false, false);
// emit StartMultiCall(creditAccount);
Expand All @@ -1907,7 +1895,7 @@ contract CreditFacadeIntegrationTest is
// vm.expectEmit(false, false, false, false);
// emit FinishMultiCall();

// vm.expectCall(address(creditManager), abi.encodeCall(ICreditManagerV3.setCreditAccountForExternalCall, (address(1))));
// vm.expectCall(address(creditManager), abi.encodeCall(ICreditManagerV3.setActiveCreditAccount, (address(1))));
// // Total value = 2 * DAI_ACCOUNT_AMOUNT, cause we have x2 leverage
// uint256 totalValue = balance;

Expand Down Expand Up @@ -2093,9 +2081,7 @@ contract CreditFacadeIntegrationTest is
MultiCall({target: address(adapterMock), callData: abi.encodeCall(AdapterMock.dumbCall, (0, 0))})
);

vm.expectCall(
address(creditManager), abi.encodeCall(ICreditManagerV3.setCreditAccountForExternalCall, (creditAccount))
);
vm.expectCall(address(creditManager), abi.encodeCall(ICreditManagerV3.setActiveCreditAccount, (creditAccount)));

vm.expectEmit(true, true, false, true);
emit StartMultiCall(creditAccount);
Expand All @@ -2112,9 +2098,7 @@ contract CreditFacadeIntegrationTest is
vm.expectEmit(false, false, false, true);
emit FinishMultiCall();

vm.expectCall(
address(creditManager), abi.encodeCall(ICreditManagerV3.setCreditAccountForExternalCall, (address(1)))
);
vm.expectCall(address(creditManager), abi.encodeCall(ICreditManagerV3.setActiveCreditAccount, (address(1))));

vm.expectCall(
address(creditManager),
Expand Down
10 changes: 5 additions & 5 deletions contracts/test/integration/credit/CreditManager.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -1034,7 +1034,7 @@ contract CreditManagerTest is Test, ICreditManagerV3Events, BalanceHelper {
/// @dev [CM-25A]: approveCreditAccount reverts if the token is not added
function test_CM_25A_approveCreditAccount_reverts_if_the_token_is_not_added() public {
(,,, address creditAccount) = _openCreditAccount();
creditManager.setCreditAccountForExternalCall(creditAccount);
creditManager.setActiveCreditAccount(creditAccount);

vm.prank(CONFIGURATOR);
creditManager.setContractAllowance(ADAPTER, DUMB_ADDRESS);
Expand All @@ -1048,7 +1048,7 @@ contract CreditManagerTest is Test, ICreditManagerV3Events, BalanceHelper {
/// @dev [CM-26]: approveCreditAccount approves with desired allowance
function test_CM_26_approveCreditAccount_approves_with_desired_allowance() public {
(,,, address creditAccount) = _openCreditAccount();
creditManager.setCreditAccountForExternalCall(creditAccount);
creditManager.setActiveCreditAccount(creditAccount);

vm.prank(CONFIGURATOR);
creditManager.setContractAllowance(ADAPTER, DUMB_ADDRESS);
Expand All @@ -1067,7 +1067,7 @@ contract CreditManagerTest is Test, ICreditManagerV3Events, BalanceHelper {
/// @dev [CM-27A]: approveCreditAccount works for ERC20 that revert if allowance > 0 before approve
function test_CM_27A_approveCreditAccount_works_for_ERC20_with_approve_restrictions() public {
(,,, address creditAccount) = _openCreditAccount();
creditManager.setCreditAccountForExternalCall(creditAccount);
creditManager.setActiveCreditAccount(creditAccount);

vm.prank(CONFIGURATOR);
creditManager.setContractAllowance(ADAPTER, DUMB_ADDRESS);
Expand All @@ -1089,7 +1089,7 @@ contract CreditManagerTest is Test, ICreditManagerV3Events, BalanceHelper {
// /// @dev [CM-27B]: approveCreditAccount works for ERC20 that returns false if allowance > 0 before approve
function test_CM_27B_approveCreditAccount_works_for_ERC20_with_approve_restrictions() public {
(,,, address creditAccount) = _openCreditAccount();
creditManager.setCreditAccountForExternalCall(creditAccount);
creditManager.setActiveCreditAccount(creditAccount);

address approveFalseToken = address(new ERC20ApproveRestrictedFalse());

Expand All @@ -1115,7 +1115,7 @@ contract CreditManagerTest is Test, ICreditManagerV3Events, BalanceHelper {
/// @dev [CM-29]: executeOrder calls credit account method and emit event
function test_CM_29_executeOrder_calls_credit_account_method_and_emit_event() public {
(,,, address creditAccount) = _openCreditAccount();
creditManager.setCreditAccountForExternalCall(creditAccount);
creditManager.setActiveCreditAccount(creditAccount);

TargetContractMock targetMock = new TargetContractMock();

Expand Down
4 changes: 2 additions & 2 deletions contracts/test/unit/adapters/AbstractAdapter.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ contract AbstractAdapterUnitTest is TestHelper, CreditManagerMockEvents {

/// @notice U:[AA-3]: `_creditAccount` works correctly
function test_U_AA_03_creditAccount_works_correctly(address creditAccount) public {
creditManager.setExternalCallCreditAccount(creditAccount);
creditManager.setActiveCreditAccount(creditAccount);

vm.expectCall(address(creditManager), abi.encodeCall(creditManager.getExternalCallCreditAccountOrRevert, ()));
vm.expectCall(address(creditManager), abi.encodeCall(creditManager.getActiveCreditAccountOrRevert, ()));
assertEq(abstractAdapter.creditAccount(), creditAccount, "Incorrect external call credit account");
}

Expand Down
6 changes: 3 additions & 3 deletions contracts/test/unit/adapters/CreditManagerMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ contract CreditManagerMock is CreditManagerMockEvents {
address public addressProvider;
address public creditFacade;

address public getExternalCallCreditAccountOrRevert;
address public getActiveCreditAccountOrRevert;
mapping(address => uint256) public getTokenMaskOrRevert;

bytes _result;
Expand All @@ -31,8 +31,8 @@ contract CreditManagerMock is CreditManagerMockEvents {
return _result;
}

function setExternalCallCreditAccount(address creditAccount) external {
getExternalCallCreditAccountOrRevert = creditAccount;
function setActiveCreditAccount(address creditAccount) external {
getActiveCreditAccountOrRevert = creditAccount;
}

function setMask(address token, uint256 mask) external {
Expand Down
Loading

0 comments on commit 4eef05b

Please sign in to comment.