diff --git a/contracts/core/AliasedLossPolicyV3.sol b/contracts/core/AliasedLossPolicyV3.sol index ae266785..0be8420c 100644 --- a/contracts/core/AliasedLossPolicyV3.sol +++ b/contracts/core/AliasedLossPolicyV3.sol @@ -66,10 +66,14 @@ contract AliasedLossPolicyV3 is ACLTrait, PriceFeedValidationTrait, IAliasedLoss } /// @notice Contract version - uint256 public constant override version = 3_10; + function version() external view virtual override returns (uint256) { + return 3_11; + } /// @notice Contract type - bytes32 public constant override contractType = "LOSS_POLICY::ALIASED"; + function contractType() external view virtual override returns (bytes32) { + return "LOSS_POLICY::ALIASED"; + } /// @notice Pool for which the loss policy is applied address public immutable override pool; @@ -108,7 +112,7 @@ contract AliasedLossPolicyV3 is ACLTrait, PriceFeedValidationTrait, IAliasedLoss /// @notice Serializes the loss policy state /// @custom:tests U:[ALP-1], U:[ALP-2], U:[ALP-3] - function serialize() external view override returns (bytes memory) { + function serialize() external view virtual override returns (bytes memory) { address[] memory tokens = _tokensWithAliasSet.values(); uint256 numTokens = tokens.length; PriceFeedParams[] memory priceFeedParams = new PriceFeedParams[](numTokens); @@ -121,7 +125,8 @@ contract AliasedLossPolicyV3 is ACLTrait, PriceFeedValidationTrait, IAliasedLoss /// @notice Returns whether `creditAccount` can be liquidated with loss by `caller` /// @custom:tests U:[ALP-4], U:[ALP-5] function isLiquidatableWithLoss(address creditAccount, address caller, Params calldata params) - external + public + virtual override returns (bool) { diff --git a/contracts/core/DefaultAccountFactoryV3.sol b/contracts/core/DefaultAccountFactoryV3.sol index ef336799..15dd54a9 100644 --- a/contracts/core/DefaultAccountFactoryV3.sol +++ b/contracts/core/DefaultAccountFactoryV3.sol @@ -12,52 +12,26 @@ import {CreditManagerV3} from "../credit/CreditManagerV3.sol"; import {IDefaultAccountFactoryV3} from "../interfaces/IDefaultAccountFactoryV3.sol"; import { CallerNotCreditManagerException, - CreditAccountIsInUseException, - MasterCreditAccountAlreadyDeployedException + MasterCreditAccountAlreadyDeployedException, + CreditAccountIsInUseException } from "../interfaces/IExceptions.sol"; import {IAddressProvider} from "../interfaces/base/IAddressProvider.sol"; import {AP_INSTANCE_MANAGER_PROXY, NO_VERSION_CONTROL} from "../libraries/Constants.sol"; -/// @dev Struct holding factory and queue params for a credit manager -/// @param masterCreditAccount Address of the contract to clone to create new accounts for the credit manager -/// @param head Index of the next credit account to be taken from the queue in case it's already reusable -/// @param tail Index of the last credit account returned to the queue -struct FactoryParams { - address masterCreditAccount; - uint40 head; - uint40 tail; -} - -/// @dev Struct holding queued credit account address and timestamp after which it becomes reusable -struct QueuedAccount { - address creditAccount; - uint40 reusableAfter; -} - /// @title Default account factory V3 -/// @notice Reusable credit accounts factory. +/// @notice Credit accounts factory. /// - Account deployment is cheap thanks to the clones proxy pattern -/// - Accounts are reusable: new accounts are only deployed when the queue of reusable accounts is empty -/// (a separate queue is maintained for each credit manager) -/// - When account is returned to the factory, it is only added to the queue after a certain delay, which -/// allows DAO to rescue funds that might have been accidentally left upon account closure, and serves -/// as protection against potential attacks involving reopening an account right after closing it -contract DefaultAccountFactoryV3 is IDefaultAccountFactoryV3, Ownable { +/// - Each take deploys a new credit account +contract DefaultAccountFactoryV3 is Ownable, IDefaultAccountFactoryV3 { /// @notice Contract version - uint256 public constant override version = 3_10; + uint256 public constant override version = 3_11; /// @notice Contract type bytes32 public constant override contractType = "ACCOUNT_FACTORY::DEFAULT"; - /// @notice Delay after which returned credit accounts can be reused - uint40 public constant override delay = 3 days; - - /// @dev Mapping credit manager => factory params - mapping(address => FactoryParams) internal _factoryParams; - - /// @dev Mapping (credit manager, index) => queued account - mapping(address => mapping(uint256 => QueuedAccount)) internal _queuedAccounts; + /// @dev Mapping credit manager => master credit account used for cloning + mapping(address => address) internal _masterCreditAccounts; /// @notice Constructor /// @param addressProvider_ Address provider contract address @@ -70,52 +44,23 @@ contract DefaultAccountFactoryV3 is IDefaultAccountFactoryV3, Ownable { /// @notice Empty state serialization function serialize() external view override returns (bytes memory) {} - /// @notice Provides a reusable credit account from the queue to the credit manager. - /// If there are no accounts that can be reused in the queue, deploys a new one. + /// @notice Deploys a new credit account for the calling credit manager /// @return creditAccount Address of the provided credit account /// @dev Parameters are ignored and only kept for backward compatibility /// @custom:expects Credit manager sets account's borrower to non-zero address after calling this function function takeCreditAccount(uint256, uint256) external override returns (address creditAccount) { - FactoryParams storage fp = _factoryParams[msg.sender]; - - address masterCreditAccount = fp.masterCreditAccount; + address masterCreditAccount = _masterCreditAccounts[msg.sender]; if (masterCreditAccount == address(0)) { revert CallerNotCreditManagerException(); // U:[AF-1] } - uint256 head = fp.head; - if (head == fp.tail || block.timestamp < _queuedAccounts[msg.sender][head].reusableAfter) { - creditAccount = Clones.clone(masterCreditAccount); // U:[AF-2A] - emit DeployCreditAccount({creditAccount: creditAccount, creditManager: msg.sender}); // U:[AF-2A] - } else { - creditAccount = _queuedAccounts[msg.sender][head].creditAccount; // U:[AF-2B] - delete _queuedAccounts[msg.sender][head]; // U:[AF-2B] - unchecked { - ++fp.head; // U:[AF-2B] - } - } - - emit TakeCreditAccount({creditAccount: creditAccount, creditManager: msg.sender}); // U:[AF-2A,2B] + creditAccount = Clones.clone(masterCreditAccount); // U:[AF-2] + emit DeployCreditAccount({creditAccount: creditAccount, creditManager: msg.sender}); // U:[AF-2] + emit TakeCreditAccount({creditAccount: creditAccount, creditManager: msg.sender}); // U:[AF-2] } - /// @notice Returns a used credit account to the queue - /// @param creditAccount Address of the returned credit account - /// @custom:expects Credit account is connected to the calling credit manager - /// @custom:expects Credit manager sets account's borrower to zero-address before calling this function - function returnCreditAccount(address creditAccount) external override { - FactoryParams storage fp = _factoryParams[msg.sender]; - - if (fp.masterCreditAccount == address(0)) { - revert CallerNotCreditManagerException(); // U:[AF-1] - } - - _queuedAccounts[msg.sender][fp.tail] = - QueuedAccount({creditAccount: creditAccount, reusableAfter: uint40(block.timestamp) + delay}); // U:[AF-3] - unchecked { - ++fp.tail; // U:[AF-3] - } - emit ReturnCreditAccount({creditAccount: creditAccount, creditManager: msg.sender}); // U:[AF-3] - } + /// @dev Account reuse is no longer supported, so this function is a no-op. + function returnCreditAccount(address) external pure virtual override {} // ------------- // // CONFIGURATION // @@ -124,11 +69,11 @@ contract DefaultAccountFactoryV3 is IDefaultAccountFactoryV3, Ownable { /// @notice Adds a credit manager to the factory and deploys the master credit account for it /// @param creditManager Credit manager address function addCreditManager(address creditManager) external override { - if (_factoryParams[creditManager].masterCreditAccount != address(0)) { + if (_masterCreditAccounts[creditManager] != address(0)) { revert MasterCreditAccountAlreadyDeployedException(); // U:[AF-4A] } address masterCreditAccount = address(new CreditAccountV3(creditManager)); // U:[AF-4B] - _factoryParams[creditManager].masterCreditAccount = masterCreditAccount; // U:[AF-4B] + _masterCreditAccounts[creditManager] = masterCreditAccount; // U:[AF-4B] emit AddCreditManager(creditManager, masterCreditAccount); // U:[AF-4B] } @@ -142,6 +87,7 @@ contract DefaultAccountFactoryV3 is IDefaultAccountFactoryV3, Ownable { external override onlyOwner // U:[AF-1] + { address creditManager = CreditAccountV3(creditAccount).creditManager(); diff --git a/contracts/interfaces/IDefaultAccountFactoryV3.sol b/contracts/interfaces/IDefaultAccountFactoryV3.sol index 3bbbb570..2c1aa1b0 100644 --- a/contracts/interfaces/IDefaultAccountFactoryV3.sol +++ b/contracts/interfaces/IDefaultAccountFactoryV3.sol @@ -12,9 +12,6 @@ interface IDefaultAccountFactoryV3Events { /// @notice Emitted when credit account is taken by the credit manager event TakeCreditAccount(address indexed creditAccount, address indexed creditManager); - /// @notice Emitted when used credit account is returned to the queue - event ReturnCreditAccount(address indexed creditAccount, address indexed creditManager); - /// @notice Emitted when new credit manager is added to the factory event AddCreditManager(address indexed creditManager, address masterCreditAccount); @@ -24,7 +21,5 @@ interface IDefaultAccountFactoryV3Events { /// @title Default account factory V3 interface interface IDefaultAccountFactoryV3 is IAccountFactory, IDefaultAccountFactoryV3Events { - function delay() external view returns (uint40); - function rescue(address creditAccount, address target, bytes calldata data) external; } diff --git a/contracts/test/integration/credit/CreditConfigurator.int.t.sol b/contracts/test/integration/credit/CreditConfigurator.int.t.sol index 139ff8fa..62432f86 100644 --- a/contracts/test/integration/credit/CreditConfigurator.int.t.sol +++ b/contracts/test/integration/credit/CreditConfigurator.int.t.sol @@ -1120,7 +1120,7 @@ contract CreditConfiguratorIntegrationTest is IntegrationTestHelper, ICreditConf makeTokenQuoted(address(token), 1, uint96(type(int96).max)); vm.prank(CONFIGURATOR); - creditConfigurator.addCollateralToken{gas: 100_000}(address(token), 8800); + creditConfigurator.addCollateralToken{gas: 180_000}(address(token), 8800); assertTrue(creditManager.getTokenMaskOrRevert(address(token)) > 0, "Token wasn't added"); diff --git a/contracts/test/unit/core/DefaultAccountFactoryV3.unit.t.sol b/contracts/test/unit/core/DefaultAccountFactoryV3.unit.t.sol index b459c707..259b5312 100644 --- a/contracts/test/unit/core/DefaultAccountFactoryV3.unit.t.sol +++ b/contracts/test/unit/core/DefaultAccountFactoryV3.unit.t.sol @@ -6,18 +6,18 @@ pragma solidity ^0.8.17; import {AddressProviderV3ACLMock} from "../../mocks/core/AddressProviderV3ACLMock.sol"; import {CreditAccountV3} from "../../../credit/CreditAccountV3.sol"; -import {CreditAccountInfo, CreditManagerV3} from "../../../credit/CreditManagerV3.sol"; +import {CreditManagerV3} from "../../../credit/CreditManagerV3.sol"; import {IDefaultAccountFactoryV3Events} from "../../../interfaces/IDefaultAccountFactoryV3.sol"; +import {CreditAccountInfo} from "../../../interfaces/ICreditManagerV3.sol"; import { CallerNotCreditManagerException, CreditAccountIsInUseException, - MasterCreditAccountAlreadyDeployedException, - RegisteredCreditManagerOnlyException + MasterCreditAccountAlreadyDeployedException } from "../../../interfaces/IExceptions.sol"; import {TestHelper} from "../../lib/helper.sol"; -import {DefaultAccountFactoryV3Harness, FactoryParams, QueuedAccount} from "./DefaultAccountFactoryV3Harness.sol"; +import {DefaultAccountFactoryV3Harness} from "./DefaultAccountFactoryV3Harness.sol"; /// @title Default account factory V3 unit test /// @notice U:[AF]: Unit tests for account factory @@ -33,8 +33,6 @@ contract DefaultAccountFactoryV3UnitTest is TestHelper, IDefaultAccountFactoryV3 owner = accountFactory.owner(); creditManager = makeAddr("CREDIT_MANAGER"); - - vm.prank(owner); accountFactory.addCreditManager(creditManager); } @@ -45,10 +43,6 @@ contract DefaultAccountFactoryV3UnitTest is TestHelper, IDefaultAccountFactoryV3 vm.expectRevert(CallerNotCreditManagerException.selector); accountFactory.takeCreditAccount(0, 0); } - if (caller != creditManager) { - vm.expectRevert(CallerNotCreditManagerException.selector); - accountFactory.returnCreditAccount(address(0)); - } if (caller != owner) { vm.expectRevert("Ownable: caller is not the owner"); accountFactory.rescue(address(0), address(0), bytes("")); @@ -56,19 +50,8 @@ contract DefaultAccountFactoryV3UnitTest is TestHelper, IDefaultAccountFactoryV3 vm.stopPrank(); } - /// @notice U:[AF-2A]: `takeCreditAccount` works correctly when queue has no reusable accounts - function test_U_AF_02A_takeCreditAccount_works_correctly_when_queue_has_no_reusable_accounts( - uint40 head, - uint40 tail - ) public { - tail = uint40(bound(tail, 0, 512)); - head = uint40(bound(head, 0, tail)); - FactoryParams memory fp = accountFactory.factoryParams(creditManager); - accountFactory.setFactoryParams(creditManager, fp.masterCreditAccount, head, tail); - if (head < tail) { - accountFactory.setQueuedAccount(creditManager, head, address(0), uint40(block.timestamp + 1)); - } - + /// @notice U:[AF-2]: `takeCreditAccount` always deploys a new credit account + function test_U_AF_02_takeCreditAccount_deploys_new_credit_account() public { vm.expectEmit(false, true, false, false); emit DeployCreditAccount(address(0), creditManager); @@ -81,48 +64,14 @@ contract DefaultAccountFactoryV3UnitTest is TestHelper, IDefaultAccountFactoryV3 assertNotEq(creditAccount, address(0), "Incorrect clone account"); assertEq(CreditAccountV3(creditAccount).factory(), address(accountFactory), "Incorrect clone account's factory"); assertEq( - CreditAccountV3(creditAccount).creditManager(), creditManager, "Incorrect cline deployed's creditManager" + CreditAccountV3(creditAccount).creditManager(), creditManager, "Incorrect clone account's creditManager" ); } - /// @notice U:[AF-2B]: `takeCreditAccount` works correctly when queue has reusable accounts - function test_U_AF_02B_takeCreditAccount_works_correctly_when_queue_has_reusable_accounts( - address creditAccount, - uint40 head, - uint40 tail - ) public { - tail = uint40(bound(tail, 1, 512)); - head = uint40(bound(head, 0, tail - 1)); - - FactoryParams memory fp = accountFactory.factoryParams(creditManager); - accountFactory.setFactoryParams(creditManager, fp.masterCreditAccount, head, tail); - accountFactory.setQueuedAccount(creditManager, head, creditAccount, uint40(block.timestamp - 1)); - - vm.expectEmit(true, true, false, false); - emit TakeCreditAccount(creditAccount, creditManager); - - vm.prank(creditManager); - address result = accountFactory.takeCreditAccount(0, 0); - - assertEq(result, creditAccount, "Incorrect creditAccount"); - assertEq(accountFactory.factoryParams(creditManager).head, uint40(head) + 1, "Incorrect head"); - } - - /// @notice U:[AF-3]: `returnCreditAccount` works correctly - function test_U_AF_03_returnCreditAccount_works_correctly(address creditAccount, uint8 tail) public { - FactoryParams memory fp = accountFactory.factoryParams(creditManager); - accountFactory.setFactoryParams(creditManager, fp.masterCreditAccount, fp.head, tail); - - vm.expectEmit(true, true, false, false); - emit ReturnCreditAccount(creditAccount, creditManager); - - vm.prank(creditManager); + /// @notice U:[AF-3]: `returnCreditAccount` is a no-op + function test_U_AF_03_returnCreditAccount_is_noop(address caller, address creditAccount) public { + vm.prank(caller); accountFactory.returnCreditAccount(creditAccount); - - QueuedAccount memory qa = accountFactory.queuedAccounts(creditManager, tail); - assertEq(qa.creditAccount, creditAccount, "Incorrect creditAccount"); - assertEq(qa.reusableAfter, uint40(block.timestamp + 3 days), "Incorrect reusableAfter"); - assertEq(accountFactory.factoryParams(creditManager).tail, uint40(tail) + 1, "Incorrect tail"); } /// @notice U:[AF-4A]: `addCreditManager` reverts on already added credit manager @@ -132,10 +81,9 @@ contract DefaultAccountFactoryV3UnitTest is TestHelper, IDefaultAccountFactoryV3 ) public { vm.assume(manager != creditManager && creditAccount != address(0)); - accountFactory.setFactoryParams(manager, creditAccount, 0, 0); + accountFactory.setMasterCreditAccount(manager, creditAccount); vm.expectRevert(MasterCreditAccountAlreadyDeployedException.selector); - vm.prank(owner); accountFactory.addCreditManager(manager); } @@ -146,10 +94,9 @@ contract DefaultAccountFactoryV3UnitTest is TestHelper, IDefaultAccountFactoryV3 vm.expectEmit(true, false, false, false); emit AddCreditManager(manager, address(0)); - vm.prank(owner); accountFactory.addCreditManager(manager); - address account = accountFactory.factoryParams(manager).masterCreditAccount; + address account = accountFactory.masterCreditAccount(manager); assertNotEq(account, address(0), "Incorrect master account"); assertEq(CreditAccountV3(account).factory(), address(accountFactory), "Incorrect master account's factory"); assertEq(CreditAccountV3(account).creditManager(), manager, "Incorrect master account's creditManager"); diff --git a/contracts/test/unit/core/DefaultAccountFactoryV3Harness.sol b/contracts/test/unit/core/DefaultAccountFactoryV3Harness.sol index 7f91d17a..d17dc24b 100644 --- a/contracts/test/unit/core/DefaultAccountFactoryV3Harness.sol +++ b/contracts/test/unit/core/DefaultAccountFactoryV3Harness.sol @@ -3,26 +3,16 @@ // (c) Gearbox Foundation, 2023. pragma solidity ^0.8.17; -import {DefaultAccountFactoryV3, FactoryParams, QueuedAccount} from "../../../core/DefaultAccountFactoryV3.sol"; +import {DefaultAccountFactoryV3} from "../../../core/DefaultAccountFactoryV3.sol"; contract DefaultAccountFactoryV3Harness is DefaultAccountFactoryV3 { constructor(address addressProvider) DefaultAccountFactoryV3(addressProvider) {} - function queuedAccounts(address creditManager, uint256 index) external view returns (QueuedAccount memory) { - return _queuedAccounts[creditManager][index]; + function masterCreditAccount(address creditManager) external view returns (address) { + return _masterCreditAccounts[creditManager]; } - function setQueuedAccount(address creditManager, uint256 index, address creditAccount, uint40 reusableAfter) - external - { - _queuedAccounts[creditManager][index] = QueuedAccount(creditAccount, reusableAfter); - } - - function factoryParams(address creditManager) external view returns (FactoryParams memory) { - return _factoryParams[creditManager]; - } - - function setFactoryParams(address creditManager, address masterCreditAccount, uint40 head, uint40 tail) external { - _factoryParams[creditManager] = FactoryParams(masterCreditAccount, head, tail); + function setMasterCreditAccount(address creditManager, address masterCreditAccount_) external { + _masterCreditAccounts[creditManager] = masterCreditAccount_; } } diff --git a/lib/forge-std b/lib/forge-std index 620536fa..74302c55 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit 620536fa5277db4e3fd46772d5cbc1ea0696fb43 +Subproject commit 74302c55493479410b143d9dd08a60690c726ffe