From c9de4f760b5b37074c3ff1361a129fbc77e031cb Mon Sep 17 00:00:00 2001 From: 0xAustrian Date: Mon, 26 Aug 2024 11:43:27 -0300 Subject: [PATCH 01/20] feat: add AllocationExtension.sol --- .../contracts/AllocationExtension.sol | 136 ++++++++++++++++++ .../interfaces/IAllocationExtension.sol | 18 +++ test/foundry/strategies/QVImpactStream.t.sol | 6 +- 3 files changed, 157 insertions(+), 3 deletions(-) create mode 100644 contracts/extensions/contracts/AllocationExtension.sol create mode 100644 contracts/extensions/interfaces/IAllocationExtension.sol diff --git a/contracts/extensions/contracts/AllocationExtension.sol b/contracts/extensions/contracts/AllocationExtension.sol new file mode 100644 index 000000000..bc345b6a7 --- /dev/null +++ b/contracts/extensions/contracts/AllocationExtension.sol @@ -0,0 +1,136 @@ +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity 0.8.19; + +import {IAllocationExtension} from "contracts/extensions/interfaces/IAllocationExtension.sol"; +import {CoreBaseStrategy} from "contracts/strategies/CoreBaseStrategy.sol"; + +abstract contract AllocationExtension is CoreBaseStrategy, IAllocationExtension { + /// ================================ + /// ========== Storage ============= + /// ================================ + + /// @notice The start and end times for allocations + uint64 public allocationStartTime; + uint64 public allocationEndTime; + + /// @notice Defines if the strategy is sending Metadata struct in the data parameter + bool public isUsingAllocationMetadata; + + /// @notice token -> isAllowed + mapping(address => bool) public allowedTokens; + + /// =============================== + /// ========= Initialize ========== + /// =============================== + + /// @notice This initializes the Alocation Extension + /// @dev This function MUST be called by the 'initialize' function in the strategy. + /// @param _allowedTokens The allowed tokens + /// @param _allocationStartTime The start time for the allocation period + /// @param _allocationEndTime The end time for the allocation period + function __AllocationExtension_init( + address[] memory _allowedTokens, + uint64 _allocationStartTime, + uint64 _allocationEndTime + ) internal { + if (_allowedTokens.length == 0) { + // all tokens + allowedTokens[address(0)] = true; + } else { + for (uint256 i; i < _allowedTokens.length; i++) { + allowedTokens[_allowedTokens[i]] = true; + } + } + + _updateAllocationTimestamps(_allocationStartTime, _allocationEndTime); + } + + /// ==================================== + /// =========== Modifiers ============== + /// ==================================== + + /// @notice Modifier to check if allocation has ended + /// @dev Reverts if allocation has not ended + modifier onlyAfterAllocation() { + _checkOnlyAfterAllocation(); + _; + } + + /// @notice Modifier to check if allocation is active + /// @dev Reverts if allocation is not active + modifier onlyActiveAllocation() { + _checkOnlyActiveAllocation(); + _; + } + + /// @notice Modifier to check if allocation has started + /// @dev Reverts if allocation has started + modifier onlyBeforeAllocation() { + _checkBeforeAllocation(); + _; + } + + /// ==================================== + /// ============ Internal ============== + /// ==================================== + + /// @notice Checks if the allocator is valid + /// @param _allocator The allocator address + /// @return 'true' if the allocator is valid, otherwise 'false' + function _isValidAllocator(address _allocator) internal view virtual returns (bool); + + /// @notice Returns TRUE if the token is allowed + /// @param _token The token to check + function _isAllowedToken(address _token) internal view virtual returns (bool) { + // all tokens allowed + if (allowedTokens[address(0)]) return true; + + if (allowedTokens[_token]) return true; + + return false; + } + + /// @notice Sets the start and end dates for allocation. + /// @dev The 'msg.sender' must be a pool manager. + /// @param _allocationStartTime The start time for the allocation + /// @param _allocationEndTime The end time for the allocation + function _updateAllocationTimestamps(uint64 _allocationStartTime, uint64 _allocationEndTime) internal virtual { + if (_allocationStartTime > _allocationEndTime) revert INVALID_ALLOCATION_TIMESTAMPS(); + + allocationStartTime = _allocationStartTime; + allocationEndTime = _allocationEndTime; + + emit AllocationTimestampsUpdated(allocationStartTime, allocationEndTime, msg.sender); + } + + /// @dev Ensure the function is called before allocation start time + function _checkBeforeAllocation() internal virtual { + if (block.timestamp > allocationStartTime) revert ALLOCATION_HAS_STARTED(); + } + + /// @dev Ensure the function is called during allocation times + function _checkOnlyActiveAllocation() internal virtual { + if (block.timestamp < allocationStartTime) revert ALLOCATION_NOT_ACTIVE(); + if (block.timestamp > allocationEndTime) revert ALLOCATION_NOT_ACTIVE(); + } + + /// @dev Ensure the function is called after allocation start time + function _checkOnlyAfterAllocation() internal virtual { + if (block.timestamp <= allocationEndTime) revert ALLOCATION_NOT_ENDED(); + } + + // ==================================== + // ==== External/Public Functions ===== + // ==================================== + + /// @notice Sets the start and end dates for allocation. + /// @dev The 'msg.sender' must be a pool manager. + /// @param _allocationStartTime The start time for the allocation + /// @param _allocationEndTime The end time for the allocation + function updateAllocationTimestamps(uint64 _allocationStartTime, uint64 _allocationEndTime) + external + onlyPoolManager(msg.sender) + { + _updateAllocationTimestamps(_allocationStartTime, _allocationEndTime); + } +} diff --git a/contracts/extensions/interfaces/IAllocationExtension.sol b/contracts/extensions/interfaces/IAllocationExtension.sol new file mode 100644 index 000000000..daad910c6 --- /dev/null +++ b/contracts/extensions/interfaces/IAllocationExtension.sol @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity 0.8.19; + +interface IAllocationExtension { + error INVALID_ALLOCATION_TIMESTAMPS(); + + error ALLOCATION_HAS_STARTED(); + + error ALLOCATION_NOT_ACTIVE(); + + error ALLOCATION_NOT_ENDED(); + + /// @notice Emitted when the allocation timestamps are updated + /// @param allocationStartTime The start time for the allocation period + /// @param allocationEndTime The end time for the allocation period + /// @param sender The sender of the transaction + event AllocationTimestampsUpdated(uint64 allocationStartTime, uint64 allocationEndTime, address sender); +} diff --git a/test/foundry/strategies/QVImpactStream.t.sol b/test/foundry/strategies/QVImpactStream.t.sol index 718cdd9c6..291483952 100644 --- a/test/foundry/strategies/QVImpactStream.t.sol +++ b/test/foundry/strategies/QVImpactStream.t.sol @@ -5,7 +5,7 @@ import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import {StdStorage, Test, stdStorage} from "forge-std/Test.sol"; import {IAllo} from "../../../contracts/core/interfaces/IAllo.sol"; -import {IStrategy} from "../../../contracts/core/interfaces/IStrategy.sol"; +import {IBaseStrategy} from "../../../contracts/core/interfaces/IBaseStrategy.sol"; import {IRecipientsExtension} from "../../../contracts/extensions/interfaces/IRecipientsExtension.sol"; import {QVSimple} from "../../../contracts/strategies/QVSimple.sol"; import {QVImpactStream} from "../../../contracts/strategies/QVImpactStream.sol"; @@ -149,7 +149,7 @@ contract QVImpactStreamTest is Test { function test__distributeRevertWhen_PayoutAmountForRecipientIsZero() external callWithPoolManager { IAllo.Pool memory poolData = IAllo.Pool({ profileId: keccak256(abi.encodePacked(recipient1)), - strategy: IStrategy(address(qvImpactStream)), + strategy: IBaseStrategy(address(qvImpactStream)), token: address(0), metadata: Metadata({protocol: 0, pointer: ""}), managerRole: keccak256("MANAGER_ROLE"), @@ -175,7 +175,7 @@ contract QVImpactStreamTest is Test { IAllo.Pool memory poolData = IAllo.Pool({ profileId: keccak256(abi.encodePacked(recipient1)), - strategy: IStrategy(address(qvImpactStream)), + strategy: IBaseStrategy(address(qvImpactStream)), token: address(0), metadata: Metadata({protocol: 0, pointer: ""}), managerRole: keccak256("MANAGER_ROLE"), From 86e080483e097a8262ba5bc86db90b7b74480c2d Mon Sep 17 00:00:00 2001 From: 0xAustrian Date: Mon, 26 Aug 2024 15:02:19 -0300 Subject: [PATCH 02/20] feat: complete interface, add natspec --- .../contracts/AllocationExtension.sol | 6 +++++- .../interfaces/IAllocationExtension.sol | 21 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/contracts/extensions/contracts/AllocationExtension.sol b/contracts/extensions/contracts/AllocationExtension.sol index bc345b6a7..ce40dfc7f 100644 --- a/contracts/extensions/contracts/AllocationExtension.sol +++ b/contracts/extensions/contracts/AllocationExtension.sol @@ -28,10 +28,12 @@ abstract contract AllocationExtension is CoreBaseStrategy, IAllocationExtension /// @param _allowedTokens The allowed tokens /// @param _allocationStartTime The start time for the allocation period /// @param _allocationEndTime The end time for the allocation period + /// @param _isUsingAllocationMetadata Defines if the strategy is sending Metadata struct in the data parameter function __AllocationExtension_init( address[] memory _allowedTokens, uint64 _allocationStartTime, - uint64 _allocationEndTime + uint64 _allocationEndTime, + bool _isUsingAllocationMetadata ) internal { if (_allowedTokens.length == 0) { // all tokens @@ -42,6 +44,8 @@ abstract contract AllocationExtension is CoreBaseStrategy, IAllocationExtension } } + isUsingAllocationMetadata = _isUsingAllocationMetadata; + _updateAllocationTimestamps(_allocationStartTime, _allocationEndTime); } diff --git a/contracts/extensions/interfaces/IAllocationExtension.sol b/contracts/extensions/interfaces/IAllocationExtension.sol index daad910c6..03dea6f01 100644 --- a/contracts/extensions/interfaces/IAllocationExtension.sol +++ b/contracts/extensions/interfaces/IAllocationExtension.sol @@ -2,12 +2,16 @@ pragma solidity 0.8.19; interface IAllocationExtension { + /// @dev Error thrown when the allocation timestamps are invalid error INVALID_ALLOCATION_TIMESTAMPS(); + /// @dev Error thrown when trying to call the function when the allocation has started error ALLOCATION_HAS_STARTED(); + /// @dev Error thrown when trying to call the function when the allocation is not active error ALLOCATION_NOT_ACTIVE(); + /// @dev Error thrown when trying to call the function when the allocation has ended error ALLOCATION_NOT_ENDED(); /// @notice Emitted when the allocation timestamps are updated @@ -15,4 +19,21 @@ interface IAllocationExtension { /// @param allocationEndTime The end time for the allocation period /// @param sender The sender of the transaction event AllocationTimestampsUpdated(uint64 allocationStartTime, uint64 allocationEndTime, address sender); + + /// @notice The start time for the allocation period + function allocationStartTime() external view returns (uint64); + + /// @notice The end time for the allocation period + function allocationEndTime() external view returns (uint64); + + /// @notice Defines if the strategy is sending Metadata struct in the data parameter + function isUsingAllocationMetadata() external view returns (bool); + + /// @notice Returns TRUE if the token is allowed, FALSE otherwise + function allowedTokens(address _token) external view returns (bool); + + /// @notice Sets the start and end dates for allocation. + /// @param _allocationStartTime The start time for the allocation + /// @param _allocationEndTime The end time for the allocation + function updateAllocationTimestamps(uint64 _allocationStartTime, uint64 _allocationEndTime) external; } From b31c7da42c045e9fe8bda51f4d34c1fa62fe386f Mon Sep 17 00:00:00 2001 From: 0xAustrian Date: Mon, 26 Aug 2024 16:14:50 -0300 Subject: [PATCH 03/20] test: add .tree and scaffolding --- .../contracts/AllocationExtension.sol | 3 +- .../extensions/AllocationExtension.t.sol | 77 +++++++++++++++++++ .../extensions/AllocationExtension.tree | 40 ++++++++++ test/utils/MockAllocationExtension.sol | 28 +++++++ 4 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 test/foundry/extensions/AllocationExtension.t.sol create mode 100644 test/foundry/extensions/AllocationExtension.tree create mode 100644 test/utils/MockAllocationExtension.sol diff --git a/contracts/extensions/contracts/AllocationExtension.sol b/contracts/extensions/contracts/AllocationExtension.sol index ce40dfc7f..036d3800c 100644 --- a/contracts/extensions/contracts/AllocationExtension.sol +++ b/contracts/extensions/contracts/AllocationExtension.sol @@ -34,7 +34,7 @@ abstract contract AllocationExtension is CoreBaseStrategy, IAllocationExtension uint64 _allocationStartTime, uint64 _allocationEndTime, bool _isUsingAllocationMetadata - ) internal { + ) internal virtual { if (_allowedTokens.length == 0) { // all tokens allowedTokens[address(0)] = true; @@ -133,6 +133,7 @@ abstract contract AllocationExtension is CoreBaseStrategy, IAllocationExtension /// @param _allocationEndTime The end time for the allocation function updateAllocationTimestamps(uint64 _allocationStartTime, uint64 _allocationEndTime) external + virtual onlyPoolManager(msg.sender) { _updateAllocationTimestamps(_allocationStartTime, _allocationEndTime); diff --git a/test/foundry/extensions/AllocationExtension.t.sol b/test/foundry/extensions/AllocationExtension.t.sol new file mode 100644 index 000000000..271894aa1 --- /dev/null +++ b/test/foundry/extensions/AllocationExtension.t.sol @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity 0.8.19; + +import {Test} from "forge-std/Test.sol"; + +contract AllocationExtension is Test { + function test___AllocationExtension_initWhenAllowedTokensArrayIsEmpty() external { + // It should mark address zero as true + vm.skip(true); + } + + function test___AllocationExtension_initWhenAllowedTokensArrayIsNotEmpty() external { + // It should mark the tokens in the array as true + vm.skip(true); + } + + function test___AllocationExtension_initShouldSetIsUsingAllocationMetadata() external { + // It should set isUsingAllocationMetadata + vm.skip(true); + } + + function test___AllocationExtension_initShouldCall_updateAllocationTimestamps() external { + // It should call _updateAllocationTimestamps + vm.skip(true); + } + + function test__isAllowedTokenWhenAllTokensAllowed() external { + // It should always return true + vm.skip(true); + } + + function test__isAllowedTokenWhenTheTokenSentIsAllowed() external { + // It should return true + vm.skip(true); + } + + function test__isAllowedTokenWhenTheTokenSentIsNotAllowed() external { + // It should return false + vm.skip(true); + } + + function test__updateAllocationTimestampsRevertWhen_StartTimeIsBiggerThanEndTime() external { + // It should revert + vm.skip(true); + } + + function test__updateAllocationTimestampsWhenTimesAreCorrect() external { + // It should update start and end time + // It should emit event + vm.skip(true); + } + + function test__checkBeforeAllocationRevertWhen_TimestampIsBiggerThanStartTime() external { + // It should revert + vm.skip(true); + } + + function test__checkOnlyActiveAllocationRevertWhen_TimestampIsSmallerThanStartTime() external { + // It should revert + vm.skip(true); + } + + function test__checkOnlyActiveAllocationRevertWhen_TimestampIsBiggerThanEndTime() external { + // It should revert + vm.skip(true); + } + + function test__checkOnlyAfterAllocationRevertWhen_TimestampIsSmallerOrEqualThanEndTime() external { + // It should revert + vm.skip(true); + } + + function test_UpdateAllocationTimestampsGivenSenderIsPoolManager() external { + // It should call _updateAllocationTimestamps + vm.skip(true); + } +} diff --git a/test/foundry/extensions/AllocationExtension.tree b/test/foundry/extensions/AllocationExtension.tree new file mode 100644 index 000000000..54f4c187f --- /dev/null +++ b/test/foundry/extensions/AllocationExtension.tree @@ -0,0 +1,40 @@ +AllocationExtension::__AllocationExtension_init +├── When allowedTokens array is empty +│ └── It should mark address zero as true +├── When allowedTokens array is not empty +│ └── It should mark the tokens in the array as true +├── It should set isUsingAllocationMetadata +└── It should call _updateAllocationTimestamps + +AllocationExtension::_isAllowedToken +├── When all tokens allowed +│ └── It should always return true +├── When the token sent is allowed +│ └── It should return true +└── When the token sent is not allowed + └── It should return false + +AllocationExtension::_updateAllocationTimestamps +├── When start time is bigger than end time +│ └── It should revert +└── When times are correct + ├── It should update start and end time + └── It should emit event + +AllocationExtension::_checkBeforeAllocation +└── When timestamp is bigger than start time + └── It should revert + +AllocationExtension::_checkOnlyActiveAllocation +├── When timestamp is smaller than start time +│ └── It should revert +└── When timestamp is bigger than end time + └── It should revert + +AllocationExtension::_checkOnlyAfterAllocation +└── When timestamp is smaller or equal than end time + └── It should revert + +AllocationExtension::updateAllocationTimestamps +└── Given sender is pool manager + └── It should call _updateAllocationTimestamps \ No newline at end of file diff --git a/test/utils/MockAllocationExtension.sol b/test/utils/MockAllocationExtension.sol new file mode 100644 index 000000000..bd0dd97ee --- /dev/null +++ b/test/utils/MockAllocationExtension.sol @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity 0.8.19; + +import {AllocationExtension} from "contracts/extensions/contracts/AllocationExtension.sol"; +import {CoreBaseStrategy} from "contracts/strategies/CoreBaseStrategy.sol"; + +contract MockAllocationExtension is CoreBaseStrategy, AllocationExtension { + constructor(address _allo) CoreBaseStrategy(_allo) {} + + function initialize(uint256 _poolId, bytes memory _data) external override { + __BaseStrategy_init(_poolId); + ( + address[] memory _allowedTokens, + uint64 _allocationStartTime, + uint64 _allocationEndTime, + bool _isUsingAllocationMetadata + ) = abi.decode(_data, (address[], uint64, uint64, bool)); + __AllocationExtension_init(_allowedTokens, _allocationStartTime, _allocationEndTime, _isUsingAllocationMetadata); + } + + function _allocate(address[] memory, uint256[] memory, bytes memory, address) internal override {} + + function _distribute(address[] memory, bytes memory, address) internal override {} + + function _isValidAllocator(address) internal view override returns (bool) {} + + function _register(address[] memory, bytes memory, address) internal override returns (address[] memory) {} +} From de598aa56bb492ed6450ef4baa09fee799038f56 Mon Sep 17 00:00:00 2001 From: 0xAustrian Date: Mon, 26 Aug 2024 16:30:27 -0300 Subject: [PATCH 04/20] test: create mock to integrate with smock --- package.json | 2 +- .../{ => mocks}/MockAllocationExtension.sol | 30 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) rename test/utils/{ => mocks}/MockAllocationExtension.sol (51%) diff --git a/package.json b/package.json index d717cadf2..c0686b98f 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,7 @@ "/////// deploy-test ///////": "echo 'deploy test scripts'", "create-profile": "npx hardhat run scripts/test/createProfile.ts --network", "create-pool": "npx hardhat run scripts/test/createPool.ts --network", - "smock": "smock-foundry --contracts contracts/core" + "smock": "smock-foundry --contracts contracts/core --contracts test/utils/mocks/" }, "devDependencies": { "@matterlabs/hardhat-zksync-deploy": "^1.2.1", diff --git a/test/utils/MockAllocationExtension.sol b/test/utils/mocks/MockAllocationExtension.sol similarity index 51% rename from test/utils/MockAllocationExtension.sol rename to test/utils/mocks/MockAllocationExtension.sol index bd0dd97ee..e88be2206 100644 --- a/test/utils/MockAllocationExtension.sol +++ b/test/utils/mocks/MockAllocationExtension.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.19; import {AllocationExtension} from "contracts/extensions/contracts/AllocationExtension.sol"; import {CoreBaseStrategy} from "contracts/strategies/CoreBaseStrategy.sol"; +/// @dev This mock allows smock to override the functions of AllocationExtension abstract contract contract MockAllocationExtension is CoreBaseStrategy, AllocationExtension { constructor(address _allo) CoreBaseStrategy(_allo) {} @@ -18,6 +19,35 @@ contract MockAllocationExtension is CoreBaseStrategy, AllocationExtension { __AllocationExtension_init(_allowedTokens, _allocationStartTime, _allocationEndTime, _isUsingAllocationMetadata); } + function __AllocationExtension_init( + address[] memory _allowedTokens, + uint64 _allocationStartTime, + uint64 _allocationEndTime, + bool _isUsingAllocationMetadata + ) internal override virtual { + super.__AllocationExtension_init(_allowedTokens, _allocationStartTime, _allocationEndTime, _isUsingAllocationMetadata); + } + + function _isAllowedToken(address _token) internal view override virtual returns (bool) { + return super._isAllowedToken(_token); + } + + function _updateAllocationTimestamps(uint64 _allocationStartTime, uint64 _allocationEndTime) internal override virtual { + super._updateAllocationTimestamps(_allocationStartTime, _allocationEndTime); + } + + function _checkBeforeAllocation() internal override virtual { + super._checkBeforeAllocation(); + } + + function _checkOnlyActiveAllocation() internal override virtual { + super._checkOnlyActiveAllocation(); + } + + function _checkOnlyAfterAllocation() internal override virtual { + super._checkOnlyAfterAllocation(); + } + function _allocate(address[] memory, uint256[] memory, bytes memory, address) internal override {} function _distribute(address[] memory, bytes memory, address) internal override {} From a7284da7c40ea4f28ffb0ac54414acf32fdb9a85 Mon Sep 17 00:00:00 2001 From: 0xAustrian Date: Tue, 27 Aug 2024 07:47:51 -0300 Subject: [PATCH 05/20] test: add units tests for the extension --- contracts/strategies/CoreBaseStrategy.sol | 2 +- .../extensions/AllocationExtension.t.sol | 173 +++++++++++++++--- test/utils/mocks/MockAllocationExtension.sol | 4 + 3 files changed, 150 insertions(+), 29 deletions(-) diff --git a/contracts/strategies/CoreBaseStrategy.sol b/contracts/strategies/CoreBaseStrategy.sol index 089e0c70b..6cbe7a00a 100644 --- a/contracts/strategies/CoreBaseStrategy.sol +++ b/contracts/strategies/CoreBaseStrategy.sol @@ -178,7 +178,7 @@ abstract contract CoreBaseStrategy is IBaseStrategy, Transfer { /// @notice Checks if the '_sender' is a pool manager. /// @dev Reverts if the '_sender' is not a pool manager. /// @param _sender The address to check if they are a pool manager - function _checkOnlyPoolManager(address _sender) internal view { + function _checkOnlyPoolManager(address _sender) internal view virtual { if (!allo.isPoolManager(poolId, _sender)) revert BaseStrategy_UNAUTHORIZED(); } diff --git a/test/foundry/extensions/AllocationExtension.t.sol b/test/foundry/extensions/AllocationExtension.t.sol index 271894aa1..0b2e7da19 100644 --- a/test/foundry/extensions/AllocationExtension.t.sol +++ b/test/foundry/extensions/AllocationExtension.t.sol @@ -2,76 +2,193 @@ pragma solidity 0.8.19; import {Test} from "forge-std/Test.sol"; +import {MockMockAllocationExtension} from "test/smock/MockMockAllocationExtension.sol"; +import {IAllocationExtension} from "contracts/extensions/interfaces/IAllocationExtension.sol"; contract AllocationExtension is Test { + MockMockAllocationExtension extension; + + event AllocationTimestampsUpdated(uint64 allocationStartTime, uint64 allocationEndTime, address sender); + + function setUp() public { + extension = new MockMockAllocationExtension(address(0)); + } + function test___AllocationExtension_initWhenAllowedTokensArrayIsEmpty() external { + extension.call___AllocationExtension_init(new address[](0), 0, 0, false); + // It should mark address zero as true - vm.skip(true); + assertTrue(extension.allowedTokens(address(0))); } - function test___AllocationExtension_initWhenAllowedTokensArrayIsNotEmpty() external { + function test___AllocationExtension_initWhenAllowedTokensArrayIsNotEmpty(address[] memory _tokens) external { + for (uint256 i; i < _tokens.length; i++) { + vm.assume(_tokens[i] != address(0)); + } + + extension.call___AllocationExtension_init(_tokens, 0, 0, false); + // It should mark the tokens in the array as true - vm.skip(true); + for (uint256 i; i < _tokens.length; i++) { + assertTrue(extension.allowedTokens(_tokens[i])); + } } - function test___AllocationExtension_initShouldSetIsUsingAllocationMetadata() external { + function test___AllocationExtension_initShouldSetIsUsingAllocationMetadata(bool _isUsingAllocationMetadata) + external + { + extension.call___AllocationExtension_init(new address[](0), 0, 0, _isUsingAllocationMetadata); + // It should set isUsingAllocationMetadata - vm.skip(true); + assertEq(extension.isUsingAllocationMetadata(), _isUsingAllocationMetadata); } - function test___AllocationExtension_initShouldCall_updateAllocationTimestamps() external { + function test___AllocationExtension_initShouldCall_updateAllocationTimestamps( + uint64 _allocationStartTime, + uint64 _allocationEndTime + ) external { + extension.mock_call__updateAllocationTimestamps(_allocationStartTime, _allocationEndTime); + // It should call _updateAllocationTimestamps - vm.skip(true); + extension.expectCall__updateAllocationTimestamps(_allocationStartTime, _allocationEndTime); + + extension.call___AllocationExtension_init(new address[](0), _allocationStartTime, _allocationEndTime, false); } - function test__isAllowedTokenWhenAllTokensAllowed() external { + function test__isAllowedTokenWhenAllTokensAllowed(address _tokenToCheck) external { + // Send empty array to allow all tokens + extension.call___AllocationExtension_init(new address[](0), 0, 0, false); + // It should always return true - vm.skip(true); + assertTrue(extension.call__isAllowedToken(_tokenToCheck)); } - function test__isAllowedTokenWhenTheTokenSentIsAllowed() external { + function test__isAllowedTokenWhenTheTokenSentIsAllowed(address _tokenToCheck) external { + vm.assume(_tokenToCheck != address(0)); + + // Send array with only that token + address[] memory tokens = new address[](1); + tokens[0] = _tokenToCheck; + + extension.call___AllocationExtension_init(tokens, 0, 0, false); + // It should return true - vm.skip(true); + assertTrue(extension.call__isAllowedToken(_tokenToCheck)); } - function test__isAllowedTokenWhenTheTokenSentIsNotAllowed() external { + function test__isAllowedTokenWhenTheTokenSentIsNotAllowed(address _tokenToCheck, address[] memory _allowedTokens) + external + { + vm.assume(_allowedTokens.length > 0); + for (uint256 i; i < _allowedTokens.length; i++) { + vm.assume(_allowedTokens[i] != address(0)); + vm.assume(_allowedTokens[i] != _tokenToCheck); + } + + extension.call___AllocationExtension_init(_allowedTokens, 0, 0, false); + // It should return false - vm.skip(true); + assertFalse(extension.call__isAllowedToken(_tokenToCheck)); } - function test__updateAllocationTimestampsRevertWhen_StartTimeIsBiggerThanEndTime() external { + function test__updateAllocationTimestampsRevertWhen_StartTimeIsBiggerThanEndTime( + uint64 _allocationStartTime, + uint64 _allocationEndTime + ) external { + vm.assume(_allocationStartTime > _allocationEndTime); + // It should revert - vm.skip(true); + vm.expectRevert(IAllocationExtension.INVALID_ALLOCATION_TIMESTAMPS.selector); + + extension.call__updateAllocationTimestamps(_allocationStartTime, _allocationEndTime); } - function test__updateAllocationTimestampsWhenTimesAreCorrect() external { - // It should update start and end time + function test__updateAllocationTimestampsWhenTimesAreCorrect(uint64 _allocationStartTime, uint64 _allocationEndTime) + external + { + vm.assume(_allocationStartTime < _allocationEndTime); + // It should emit event - vm.skip(true); + vm.expectEmit(); + emit AllocationTimestampsUpdated(_allocationStartTime, _allocationEndTime, address(this)); + + extension.call__updateAllocationTimestamps(_allocationStartTime, _allocationEndTime); + + // It should update start and end time + assertEq(extension.allocationStartTime(), _allocationStartTime); + assertEq(extension.allocationEndTime(), _allocationEndTime); } - function test__checkBeforeAllocationRevertWhen_TimestampIsBiggerThanStartTime() external { + function test__checkBeforeAllocationRevertWhen_TimestampIsBiggerThanStartTime( + uint64 _timestamp, + uint64 _allocationStartTime + ) external { + vm.assume(_timestamp > _allocationStartTime); + + extension.call___AllocationExtension_init(new address[](0), _allocationStartTime, _allocationStartTime, false); + vm.warp(_timestamp); + // It should revert - vm.skip(true); + vm.expectRevert(IAllocationExtension.ALLOCATION_HAS_STARTED.selector); + + extension.call__checkBeforeAllocation(); } - function test__checkOnlyActiveAllocationRevertWhen_TimestampIsSmallerThanStartTime() external { + function test__checkOnlyActiveAllocationRevertWhen_TimestampIsSmallerThanStartTime( + uint64 _timestamp, + uint64 _allocationStartTime + ) external { + vm.assume(_timestamp < _allocationStartTime); + + extension.call___AllocationExtension_init(new address[](0), _allocationStartTime, _allocationStartTime, false); + vm.warp(_timestamp); + // It should revert - vm.skip(true); + vm.expectRevert(IAllocationExtension.ALLOCATION_NOT_ACTIVE.selector); + + extension.call__checkOnlyActiveAllocation(); } - function test__checkOnlyActiveAllocationRevertWhen_TimestampIsBiggerThanEndTime() external { + function test__checkOnlyActiveAllocationRevertWhen_TimestampIsBiggerThanEndTime( + uint64 _timestamp, + uint64 _allocationEndTime + ) external { + vm.assume(_timestamp > _allocationEndTime); + + extension.call___AllocationExtension_init(new address[](0), _allocationEndTime, _allocationEndTime, false); + vm.warp(_timestamp); + // It should revert - vm.skip(true); + vm.expectRevert(IAllocationExtension.ALLOCATION_NOT_ACTIVE.selector); + + extension.call__checkOnlyActiveAllocation(); } - function test__checkOnlyAfterAllocationRevertWhen_TimestampIsSmallerOrEqualThanEndTime() external { + function test__checkOnlyAfterAllocationRevertWhen_TimestampIsSmallerOrEqualThanEndTime( + uint64 _timestamp, + uint64 _allocationEndTime + ) external { + vm.assume(_timestamp <= _allocationEndTime); + + extension.call___AllocationExtension_init(new address[](0), _allocationEndTime, _allocationEndTime, false); + vm.warp(_timestamp); + // It should revert - vm.skip(true); + vm.expectRevert(IAllocationExtension.ALLOCATION_NOT_ENDED.selector); + + extension.call__checkOnlyAfterAllocation(); } - function test_UpdateAllocationTimestampsGivenSenderIsPoolManager() external { + function test_UpdateAllocationTimestampsGivenSenderIsPoolManager( + uint64 _allocationStartTime, + uint64 _allocationEndTime + ) external { + extension.mock_call__checkOnlyPoolManager(address(this)); + extension.mock_call__updateAllocationTimestamps(_allocationStartTime, _allocationEndTime); + // It should call _updateAllocationTimestamps - vm.skip(true); + extension.expectCall__updateAllocationTimestamps(_allocationStartTime, _allocationEndTime); + + extension.updateAllocationTimestamps(_allocationStartTime, _allocationEndTime); } } diff --git a/test/utils/mocks/MockAllocationExtension.sol b/test/utils/mocks/MockAllocationExtension.sol index e88be2206..b0264077e 100644 --- a/test/utils/mocks/MockAllocationExtension.sol +++ b/test/utils/mocks/MockAllocationExtension.sol @@ -48,6 +48,10 @@ contract MockAllocationExtension is CoreBaseStrategy, AllocationExtension { super._checkOnlyAfterAllocation(); } + function _checkOnlyPoolManager(address _sender) internal view override virtual { + super._checkOnlyPoolManager(_sender); + } + function _allocate(address[] memory, uint256[] memory, bytes memory, address) internal override {} function _distribute(address[] memory, bytes memory, address) internal override {} From c01bbe78465711c9ad2961edfb16aef0c4688865 Mon Sep 17 00:00:00 2001 From: 0xAustrian Date: Tue, 27 Aug 2024 07:51:47 -0300 Subject: [PATCH 06/20] chore: add smock to CI --- .github/workflows/forge-test.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/forge-test.yml b/.github/workflows/forge-test.yml index e26237c24..53c066cec 100644 --- a/.github/workflows/forge-test.yml +++ b/.github/workflows/forge-test.yml @@ -27,6 +27,11 @@ jobs: uses: foundry-rs/foundry-toolchain@v1.0.9 with: version: nightly + + - name: Run Smock + run: | + bun smock + id: smock - name: Run Forge Format run: | From a8f4b666e809be30f38c45981aa2a90d5beb16a6 Mon Sep 17 00:00:00 2001 From: 0xAustrian Date: Tue, 27 Aug 2024 09:22:14 -0300 Subject: [PATCH 07/20] chore: rename unit file --- .../{AllocationExtension.t.sol => AllocationExtensionUnit.t.sol} | 0 .../{AllocationExtension.tree => AllocationExtensionUnit.tree} | 0 2 files changed, 0 insertions(+), 0 deletions(-) rename test/foundry/extensions/{AllocationExtension.t.sol => AllocationExtensionUnit.t.sol} (100%) rename test/foundry/extensions/{AllocationExtension.tree => AllocationExtensionUnit.tree} (100%) diff --git a/test/foundry/extensions/AllocationExtension.t.sol b/test/foundry/extensions/AllocationExtensionUnit.t.sol similarity index 100% rename from test/foundry/extensions/AllocationExtension.t.sol rename to test/foundry/extensions/AllocationExtensionUnit.t.sol diff --git a/test/foundry/extensions/AllocationExtension.tree b/test/foundry/extensions/AllocationExtensionUnit.tree similarity index 100% rename from test/foundry/extensions/AllocationExtension.tree rename to test/foundry/extensions/AllocationExtensionUnit.tree From fee01d44a8e148292ac368e76354cf4f4d8c63d0 Mon Sep 17 00:00:00 2001 From: 0xAustrian Date: Tue, 27 Aug 2024 14:13:59 -0300 Subject: [PATCH 08/20] fix: minor bug in _checkBeforeAllocation --- contracts/extensions/contracts/AllocationExtension.sol | 2 +- test/foundry/extensions/AllocationExtensionUnit.t.sol | 4 ++-- test/foundry/extensions/AllocationExtensionUnit.tree | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/extensions/contracts/AllocationExtension.sol b/contracts/extensions/contracts/AllocationExtension.sol index 036d3800c..375f845ad 100644 --- a/contracts/extensions/contracts/AllocationExtension.sol +++ b/contracts/extensions/contracts/AllocationExtension.sol @@ -109,7 +109,7 @@ abstract contract AllocationExtension is CoreBaseStrategy, IAllocationExtension /// @dev Ensure the function is called before allocation start time function _checkBeforeAllocation() internal virtual { - if (block.timestamp > allocationStartTime) revert ALLOCATION_HAS_STARTED(); + if (block.timestamp >= allocationStartTime) revert ALLOCATION_HAS_STARTED(); } /// @dev Ensure the function is called during allocation times diff --git a/test/foundry/extensions/AllocationExtensionUnit.t.sol b/test/foundry/extensions/AllocationExtensionUnit.t.sol index 0b2e7da19..69c89fb20 100644 --- a/test/foundry/extensions/AllocationExtensionUnit.t.sol +++ b/test/foundry/extensions/AllocationExtensionUnit.t.sol @@ -119,11 +119,11 @@ contract AllocationExtension is Test { assertEq(extension.allocationEndTime(), _allocationEndTime); } - function test__checkBeforeAllocationRevertWhen_TimestampIsBiggerThanStartTime( + function test__checkBeforeAllocationRevertWhen_TimestampIsBiggerOrEqualThanStartTime( uint64 _timestamp, uint64 _allocationStartTime ) external { - vm.assume(_timestamp > _allocationStartTime); + vm.assume(_timestamp >= _allocationStartTime); extension.call___AllocationExtension_init(new address[](0), _allocationStartTime, _allocationStartTime, false); vm.warp(_timestamp); diff --git a/test/foundry/extensions/AllocationExtensionUnit.tree b/test/foundry/extensions/AllocationExtensionUnit.tree index 54f4c187f..c240f2ea2 100644 --- a/test/foundry/extensions/AllocationExtensionUnit.tree +++ b/test/foundry/extensions/AllocationExtensionUnit.tree @@ -22,7 +22,7 @@ AllocationExtension::_updateAllocationTimestamps └── It should emit event AllocationExtension::_checkBeforeAllocation -└── When timestamp is bigger than start time +└── When timestamp is bigger or equal than start time └── It should revert AllocationExtension::_checkOnlyActiveAllocation From 3fa774762893a833b5aeff74b8ef1da6fbeab5ac Mon Sep 17 00:00:00 2001 From: 0xAustrian Date: Tue, 27 Aug 2024 17:50:41 -0300 Subject: [PATCH 09/20] feat: add AllocatorsAllowlistExtension.sol --- .../AllocatorsAllowlistExtension.sol | 71 +++++++++++++++++++ .../IAllocatorsAllowlistExtension.sol | 19 +++++ 2 files changed, 90 insertions(+) create mode 100644 contracts/extensions/contracts/AllocatorsAllowlistExtension.sol create mode 100644 contracts/extensions/interfaces/IAllocatorsAllowlistExtension.sol diff --git a/contracts/extensions/contracts/AllocatorsAllowlistExtension.sol b/contracts/extensions/contracts/AllocatorsAllowlistExtension.sol new file mode 100644 index 000000000..141822f37 --- /dev/null +++ b/contracts/extensions/contracts/AllocatorsAllowlistExtension.sol @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity 0.8.19; + +import {AllocationExtension} from "contracts/extensions/contracts/AllocationExtension.sol"; +import {IAllocatorsAllowlistExtension} from "contracts/extensions/interfaces/IAllocatorsAllowlistExtension.sol"; + +abstract contract AllocatorsAllowlistExtension is AllocationExtension, IAllocatorsAllowlistExtension { + /// ================================ + /// ========== Storage ============= + /// ================================ + + /// @dev allocator => isAllowed + mapping(address => bool) public allowedAllocators; + + /// ==================================== + /// ============ Internal ============== + /// ==================================== + + /// @notice Checks if the allocator is valid + /// @param _allocator The allocator address + /// @return true if the allocator is valid + function _isValidAllocator(address _allocator) internal view virtual override returns (bool) { + return allowedAllocators[_allocator]; + } + + /// @dev Mark an address as valid allocator + function _addAllocator(address _allocator) internal virtual { + allowedAllocators[_allocator] = true; + } + + /// @dev Remove an address from the valid allocators + function _removeAllocator(address _allocator) internal virtual { + allowedAllocators[_allocator] = false; + } + + // ==================================== + // ==== External/Public Functions ===== + // ==================================== + + /// @notice Add allocator + /// @dev Only the pool manager(s) can call this function and emits an `AllocatorAdded` event + /// @param _allocators The allocator addresses + function addAllocators(address[] memory _allocators) external onlyPoolManager(msg.sender) { + uint256 length = _allocators.length; + for (uint256 i = 0; i < length;) { + _addAllocator(_allocators[i]); + + unchecked { + ++i; + } + } + + emit AllocatorsAdded(_allocators, msg.sender); + } + + /// @notice Remove allocators + /// @dev Only the pool manager(s) can call this function and emits an `AllocatorRemoved` event + /// @param _allocators The allocator addresses + function removeAllocators(address[] memory _allocators) external onlyPoolManager(msg.sender) { + uint256 length = _allocators.length; + for (uint256 i = 0; i < length;) { + _removeAllocator(_allocators[i]); + + unchecked { + ++i; + } + } + + emit AllocatorsRemoved(_allocators, msg.sender); + } +} diff --git a/contracts/extensions/interfaces/IAllocatorsAllowlistExtension.sol b/contracts/extensions/interfaces/IAllocatorsAllowlistExtension.sol new file mode 100644 index 000000000..4366097cc --- /dev/null +++ b/contracts/extensions/interfaces/IAllocatorsAllowlistExtension.sol @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity 0.8.19; + +interface IAllocatorsAllowlistExtension { + /// @notice Emitted when an allocator is added + /// @param allocators The allocator addresses + /// @param sender The sender of the transaction + event AllocatorsAdded(address[] allocators, address sender); + + /// @notice Emitted when an allocator is removed + /// @param allocators The allocator addresses + /// @param sender The sender of the transaction + event AllocatorsRemoved(address[] allocators, address sender); + + /// @notice Returns TRUE if the allocator is allowed, FALSE otherwise + /// @param _allocator The allocator address to check + /// @return TRUE if the allocator is allowed, FALSE otherwise + function allowedAllocators(address _allocator) external view returns (bool); +} From ac85ce483f1518316ebd362360537a3476855c36 Mon Sep 17 00:00:00 2001 From: 0xAustrian Date: Wed, 28 Aug 2024 08:48:32 -0300 Subject: [PATCH 10/20] test: add tests for the extension --- .../AllocatorsAllowlistExtensionUnit.t.sol | 73 +++++++++++++++++++ .../AllocatorsAllowlistExtensionUnit.tree | 18 +++++ .../MockAllocatorsAllowlistExtension.sol | 43 +++++++++++ 3 files changed, 134 insertions(+) create mode 100644 test/foundry/extensions/AllocatorsAllowlistExtensionUnit.t.sol create mode 100644 test/foundry/extensions/AllocatorsAllowlistExtensionUnit.tree create mode 100644 test/utils/mocks/MockAllocatorsAllowlistExtension.sol diff --git a/test/foundry/extensions/AllocatorsAllowlistExtensionUnit.t.sol b/test/foundry/extensions/AllocatorsAllowlistExtensionUnit.t.sol new file mode 100644 index 000000000..b7be76846 --- /dev/null +++ b/test/foundry/extensions/AllocatorsAllowlistExtensionUnit.t.sol @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity 0.8.19; + +import {Test} from "forge-std/Test.sol"; +import {MockMockAllocatorsAllowlistExtension} from "test/smock/MockMockAllocatorsAllowlistExtension.sol"; + +contract AllocatorsAllowlistExtension is Test { + MockMockAllocatorsAllowlistExtension extension; + + event AllocatorsAdded(address[] allocators, address sender); + event AllocatorsRemoved(address[] allocators, address sender); + + function setUp() public { + extension = new MockMockAllocatorsAllowlistExtension(address(0)); + } + + function test__isValidAllocatorShouldReturnTRUEOrFALSEGivenTheStatusOfTheAllocator( + bool _isAllowed, + address _allocator + ) external { + extension.call__addAllocator(_allocator); + + if (!_isAllowed) extension.call__removeAllocator(_allocator); + + // It should return TRUE or FALSE given the status of the allocator + assertEq(extension.call__isValidAllocator(_allocator), _isAllowed); + } + + function test__addAllocatorShouldSetToTrueTheStatusOfTheAllocator(address _allocator) external { + extension.call__addAllocator(_allocator); + + // It should set to true the status of the allocator + assertTrue(extension.allowedAllocators(_allocator)); + } + + function test__removeAllocatorShouldSetToFalseTheStatusOfTheAllocator(address _allocator) external { + extension.call__addAllocator(_allocator); + extension.call__removeAllocator(_allocator); + + // It should set to false the status of the allocator + assertFalse(extension.allowedAllocators(_allocator)); + } + + function test_AddAllocatorsGivenSenderIsPoolManager(address[] memory _allocators) external { + extension.mock_call__checkOnlyPoolManager(address(this)); + + // It should call _addAllocator for each allocator in the list + for (uint256 i; i < _allocators.length; i++) { + extension.expectCall__addAllocator(_allocators[i]); + } + + // It should emit event + vm.expectEmit(); + emit AllocatorsAdded(_allocators, address(this)); + + extension.addAllocators(_allocators); + } + + function test_RemoveAllocatorsGivenSenderIsPoolManager(address[] memory _allocators) external { + extension.mock_call__checkOnlyPoolManager(address(this)); + + // It should call _removeAllocator for each allocator in the list + for (uint256 i; i < _allocators.length; i++) { + extension.expectCall__removeAllocator(_allocators[i]); + } + + // It should emit event + vm.expectEmit(); + emit AllocatorsRemoved(_allocators, address(this)); + + extension.removeAllocators(_allocators); + } +} diff --git a/test/foundry/extensions/AllocatorsAllowlistExtensionUnit.tree b/test/foundry/extensions/AllocatorsAllowlistExtensionUnit.tree new file mode 100644 index 000000000..860cdc5bc --- /dev/null +++ b/test/foundry/extensions/AllocatorsAllowlistExtensionUnit.tree @@ -0,0 +1,18 @@ +AllocatorsAllowlistExtension::_isValidAllocator +└── It should return TRUE or FALSE given the status of the allocator + +AllocatorsAllowlistExtension::_addAllocator +└── It should set to true the status of the allocator + +AllocatorsAllowlistExtension::_removeAllocator +└── It should set to false the status of the allocator + +AllocatorsAllowlistExtension::addAllocators +└── Given sender is pool manager + ├── It should call _addAllocator for each allocator in the list + └── It should emit event + +AllocatorsAllowlistExtension::removeAllocators +└── Given sender is pool manager + ├── It should call _removeAllocator for each allocator in the list + └── It should emit event \ No newline at end of file diff --git a/test/utils/mocks/MockAllocatorsAllowlistExtension.sol b/test/utils/mocks/MockAllocatorsAllowlistExtension.sol new file mode 100644 index 000000000..558b58d93 --- /dev/null +++ b/test/utils/mocks/MockAllocatorsAllowlistExtension.sol @@ -0,0 +1,43 @@ +// SPDX-License-Identifier: AGPL-3.0-only +pragma solidity 0.8.19; + +import {AllocatorsAllowlistExtension} from "contracts/extensions/contracts/AllocatorsAllowlistExtension.sol"; +import {CoreBaseStrategy} from "contracts/strategies/CoreBaseStrategy.sol"; + +contract MockAllocatorsAllowlistExtension is CoreBaseStrategy, AllocatorsAllowlistExtension { + + constructor(address _allo) CoreBaseStrategy(_allo) {} + + function initialize(uint256 _poolId, bytes memory _data) external override { + __BaseStrategy_init(_poolId); + ( + address[] memory _allowedTokens, + uint64 _allocationStartTime, + uint64 _allocationEndTime, + bool _isUsingAllocationMetadata + ) = abi.decode(_data, (address[], uint64, uint64, bool)); + __AllocationExtension_init(_allowedTokens, _allocationStartTime, _allocationEndTime, _isUsingAllocationMetadata); + } + + function _isValidAllocator(address _allocator) internal view virtual override returns (bool) { + return super._isValidAllocator(_allocator); + } + + function _addAllocator(address _allocator) internal virtual override { + super._addAllocator(_allocator); + } + + function _removeAllocator(address _allocator) internal virtual override { + super._removeAllocator(_allocator); + } + + function _checkOnlyPoolManager(address _sender) internal view override virtual { + super._checkOnlyPoolManager(_sender); + } + + function _allocate(address[] memory, uint256[] memory, bytes memory, address) internal override {} + + function _distribute(address[] memory, bytes memory, address) internal override {} + + function _register(address[] memory, bytes memory, address) internal override returns (address[] memory) {} +} \ No newline at end of file From 3b6bba3ea8c87e03fb7bf6c535e215f070220a03 Mon Sep 17 00:00:00 2001 From: 0xAustrian Date: Wed, 28 Aug 2024 10:19:04 -0300 Subject: [PATCH 11/20] feat: bump sol version --- contracts/extensions/contracts/AllocationExtension.sol | 2 +- contracts/extensions/interfaces/IAllocationExtension.sol | 2 +- test/foundry/extensions/AllocationExtensionUnit.t.sol | 2 +- test/utils/mocks/MockAllocationExtension.sol | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/extensions/contracts/AllocationExtension.sol b/contracts/extensions/contracts/AllocationExtension.sol index 375f845ad..2520db0a2 100644 --- a/contracts/extensions/contracts/AllocationExtension.sol +++ b/contracts/extensions/contracts/AllocationExtension.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: AGPL-3.0-only -pragma solidity 0.8.19; +pragma solidity ^0.8.19; import {IAllocationExtension} from "contracts/extensions/interfaces/IAllocationExtension.sol"; import {CoreBaseStrategy} from "contracts/strategies/CoreBaseStrategy.sol"; diff --git a/contracts/extensions/interfaces/IAllocationExtension.sol b/contracts/extensions/interfaces/IAllocationExtension.sol index 03dea6f01..88f4d34f5 100644 --- a/contracts/extensions/interfaces/IAllocationExtension.sol +++ b/contracts/extensions/interfaces/IAllocationExtension.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: AGPL-3.0-only -pragma solidity 0.8.19; +pragma solidity ^0.8.19; interface IAllocationExtension { /// @dev Error thrown when the allocation timestamps are invalid diff --git a/test/foundry/extensions/AllocationExtensionUnit.t.sol b/test/foundry/extensions/AllocationExtensionUnit.t.sol index 69c89fb20..42f8976fb 100644 --- a/test/foundry/extensions/AllocationExtensionUnit.t.sol +++ b/test/foundry/extensions/AllocationExtensionUnit.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: UNLICENSED -pragma solidity 0.8.19; +pragma solidity ^0.8.19; import {Test} from "forge-std/Test.sol"; import {MockMockAllocationExtension} from "test/smock/MockMockAllocationExtension.sol"; diff --git a/test/utils/mocks/MockAllocationExtension.sol b/test/utils/mocks/MockAllocationExtension.sol index b0264077e..a14f9f588 100644 --- a/test/utils/mocks/MockAllocationExtension.sol +++ b/test/utils/mocks/MockAllocationExtension.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: AGPL-3.0-only -pragma solidity 0.8.19; +pragma solidity ^0.8.19; import {AllocationExtension} from "contracts/extensions/contracts/AllocationExtension.sol"; import {CoreBaseStrategy} from "contracts/strategies/CoreBaseStrategy.sol"; From 714fb326de4ef70ba902384b4ea9244f0d00ad02 Mon Sep 17 00:00:00 2001 From: 0xAustrian Date: Wed, 28 Aug 2024 10:21:56 -0300 Subject: [PATCH 12/20] feat: bump sol version --- contracts/extensions/contracts/AllocatorsAllowlistExtension.sol | 2 +- contracts/extensions/interfaces/IAllocationExtension.sol | 2 +- test/foundry/extensions/AllocatorsAllowlistExtensionUnit.t.sol | 2 +- test/utils/mocks/MockAllocatorsAllowlistExtension.sol | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/contracts/extensions/contracts/AllocatorsAllowlistExtension.sol b/contracts/extensions/contracts/AllocatorsAllowlistExtension.sol index 141822f37..f1b464227 100644 --- a/contracts/extensions/contracts/AllocatorsAllowlistExtension.sol +++ b/contracts/extensions/contracts/AllocatorsAllowlistExtension.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: AGPL-3.0-only -pragma solidity 0.8.19; +pragma solidity ^0.8.19; import {AllocationExtension} from "contracts/extensions/contracts/AllocationExtension.sol"; import {IAllocatorsAllowlistExtension} from "contracts/extensions/interfaces/IAllocatorsAllowlistExtension.sol"; diff --git a/contracts/extensions/interfaces/IAllocationExtension.sol b/contracts/extensions/interfaces/IAllocationExtension.sol index 03dea6f01..88f4d34f5 100644 --- a/contracts/extensions/interfaces/IAllocationExtension.sol +++ b/contracts/extensions/interfaces/IAllocationExtension.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: AGPL-3.0-only -pragma solidity 0.8.19; +pragma solidity ^0.8.19; interface IAllocationExtension { /// @dev Error thrown when the allocation timestamps are invalid diff --git a/test/foundry/extensions/AllocatorsAllowlistExtensionUnit.t.sol b/test/foundry/extensions/AllocatorsAllowlistExtensionUnit.t.sol index b7be76846..a33f1a06c 100644 --- a/test/foundry/extensions/AllocatorsAllowlistExtensionUnit.t.sol +++ b/test/foundry/extensions/AllocatorsAllowlistExtensionUnit.t.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: UNLICENSED -pragma solidity 0.8.19; +pragma solidity ^0.8.19; import {Test} from "forge-std/Test.sol"; import {MockMockAllocatorsAllowlistExtension} from "test/smock/MockMockAllocatorsAllowlistExtension.sol"; diff --git a/test/utils/mocks/MockAllocatorsAllowlistExtension.sol b/test/utils/mocks/MockAllocatorsAllowlistExtension.sol index 558b58d93..d9f58b2da 100644 --- a/test/utils/mocks/MockAllocatorsAllowlistExtension.sol +++ b/test/utils/mocks/MockAllocatorsAllowlistExtension.sol @@ -1,5 +1,5 @@ // SPDX-License-Identifier: AGPL-3.0-only -pragma solidity 0.8.19; +pragma solidity ^0.8.19; import {AllocatorsAllowlistExtension} from "contracts/extensions/contracts/AllocatorsAllowlistExtension.sol"; import {CoreBaseStrategy} from "contracts/strategies/CoreBaseStrategy.sol"; From 8ec13831d7ed1384c09a875b5a6a5ed4284cd499 Mon Sep 17 00:00:00 2001 From: 0xAustrian Date: Wed, 28 Aug 2024 18:50:51 -0300 Subject: [PATCH 13/20] feat: small improvement when emitting event --- contracts/extensions/contracts/AllocationExtension.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/extensions/contracts/AllocationExtension.sol b/contracts/extensions/contracts/AllocationExtension.sol index 2520db0a2..ba2647e8b 100644 --- a/contracts/extensions/contracts/AllocationExtension.sol +++ b/contracts/extensions/contracts/AllocationExtension.sol @@ -104,7 +104,7 @@ abstract contract AllocationExtension is CoreBaseStrategy, IAllocationExtension allocationStartTime = _allocationStartTime; allocationEndTime = _allocationEndTime; - emit AllocationTimestampsUpdated(allocationStartTime, allocationEndTime, msg.sender); + emit AllocationTimestampsUpdated(_allocationStartTime, _allocationEndTime, msg.sender); } /// @dev Ensure the function is called before allocation start time From 0d4e82bd2bfa68ff7c55cb82fa7279833b819bc9 Mon Sep 17 00:00:00 2001 From: 0xAustrian Date: Wed, 28 Aug 2024 18:54:47 -0300 Subject: [PATCH 14/20] feat: remove unchecked block from for loops --- .../contracts/AllocatorsAllowlistExtension.sol | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/contracts/extensions/contracts/AllocatorsAllowlistExtension.sol b/contracts/extensions/contracts/AllocatorsAllowlistExtension.sol index f1b464227..2fdf3acde 100644 --- a/contracts/extensions/contracts/AllocatorsAllowlistExtension.sol +++ b/contracts/extensions/contracts/AllocatorsAllowlistExtension.sol @@ -42,12 +42,8 @@ abstract contract AllocatorsAllowlistExtension is AllocationExtension, IAllocato /// @param _allocators The allocator addresses function addAllocators(address[] memory _allocators) external onlyPoolManager(msg.sender) { uint256 length = _allocators.length; - for (uint256 i = 0; i < length;) { + for (uint256 i = 0; i < length; i++) { _addAllocator(_allocators[i]); - - unchecked { - ++i; - } } emit AllocatorsAdded(_allocators, msg.sender); @@ -58,12 +54,8 @@ abstract contract AllocatorsAllowlistExtension is AllocationExtension, IAllocato /// @param _allocators The allocator addresses function removeAllocators(address[] memory _allocators) external onlyPoolManager(msg.sender) { uint256 length = _allocators.length; - for (uint256 i = 0; i < length;) { + for (uint256 i = 0; i < length; i++) { _removeAllocator(_allocators[i]); - - unchecked { - ++i; - } } emit AllocatorsRemoved(_allocators, msg.sender); From 0b8e4882ef24b674d6be48c634ab77b7092ab42d Mon Sep 17 00:00:00 2001 From: 0xAustrian Date: Thu, 29 Aug 2024 08:50:00 -0300 Subject: [PATCH 15/20] fix: fmt --- foundry.toml | 4 ++-- test/utils/mocks/MockAllocationExtension.sol | 22 +++++++++++++------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/foundry.toml b/foundry.toml index c231e728f..75d40cfea 100644 --- a/foundry.toml +++ b/foundry.toml @@ -39,8 +39,8 @@ allow_paths = ['node_modules'] [fmt] ignore = [ - 'contracts/strategies/_poc/qv-hackathon/SchemaResolver.sol', - 'contracts/strategies/_poc/direct-grants-simple/DirectGrantsSimpleStrategy.sol' + 'contracts/strategies/deprecated/_poc/qv-hackathon/SchemaResolver.sol', + 'contracts/strategies/deprecated/_poc/direct-grants-simple/DirectGrantsSimpleStrategy.sol' ] [rpc_endpoints] diff --git a/test/utils/mocks/MockAllocationExtension.sol b/test/utils/mocks/MockAllocationExtension.sol index 54b408731..e03218fc4 100644 --- a/test/utils/mocks/MockAllocationExtension.sol +++ b/test/utils/mocks/MockAllocationExtension.sol @@ -24,31 +24,37 @@ contract MockAllocationExtension is BaseStrategy, AllocationExtension { uint64 _allocationStartTime, uint64 _allocationEndTime, bool _isUsingAllocationMetadata - ) internal override virtual { - super.__AllocationExtension_init(_allowedTokens, _allocationStartTime, _allocationEndTime, _isUsingAllocationMetadata); + ) internal virtual override { + super.__AllocationExtension_init( + _allowedTokens, _allocationStartTime, _allocationEndTime, _isUsingAllocationMetadata + ); } - function _isAllowedToken(address _token) internal view override virtual returns (bool) { + function _isAllowedToken(address _token) internal view virtual override returns (bool) { return super._isAllowedToken(_token); } - function _updateAllocationTimestamps(uint64 _allocationStartTime, uint64 _allocationEndTime) internal override virtual { + function _updateAllocationTimestamps(uint64 _allocationStartTime, uint64 _allocationEndTime) + internal + virtual + override + { super._updateAllocationTimestamps(_allocationStartTime, _allocationEndTime); } - function _checkBeforeAllocation() internal override virtual { + function _checkBeforeAllocation() internal virtual override { super._checkBeforeAllocation(); } - function _checkOnlyActiveAllocation() internal override virtual { + function _checkOnlyActiveAllocation() internal virtual override { super._checkOnlyActiveAllocation(); } - function _checkOnlyAfterAllocation() internal override virtual { + function _checkOnlyAfterAllocation() internal virtual override { super._checkOnlyAfterAllocation(); } - function _checkOnlyPoolManager(address _sender) internal view override virtual { + function _checkOnlyPoolManager(address _sender) internal view virtual override { super._checkOnlyPoolManager(_sender); } From bdbacac26ab76c09ac5ce2f132c54d9bb32a9ea5 Mon Sep 17 00:00:00 2001 From: 0xAustrian Date: Wed, 4 Sep 2024 13:24:35 -0300 Subject: [PATCH 16/20] feat: implement Allocation extension on DonationVotingOffchain strat --- .../DonationVotingOffchain.sol | 98 +++---------------- .../allocate/AllocationExtension.sol | 8 +- .../allocate/IAllocationExtension.sol | 6 +- .../DonationVotingMerkleDistribution.t.sol | 42 -------- test/integration/DonationVotingOffchain.t.sol | 42 -------- .../extensions/AllocationExtensionUnit.t.sol | 8 +- 6 files changed, 23 insertions(+), 181 deletions(-) diff --git a/contracts/strategies/examples/donation-voting/DonationVotingOffchain.sol b/contracts/strategies/examples/donation-voting/DonationVotingOffchain.sol index d071464b7..9bcc5d2e3 100644 --- a/contracts/strategies/examples/donation-voting/DonationVotingOffchain.sol +++ b/contracts/strategies/examples/donation-voting/DonationVotingOffchain.sol @@ -6,6 +6,7 @@ import {IAllo} from "contracts/core/interfaces/IAllo.sol"; // Core Contracts import {BaseStrategy} from "strategies/BaseStrategy.sol"; import {RecipientsExtension} from "strategies/extensions/register/RecipientsExtension.sol"; +import {AllocationExtension} from "strategies/extensions/allocate/AllocationExtension.sol"; // Internal Libraries import {Transfer} from "contracts/core/libraries/Transfer.sol"; import {Native} from "contracts/core/libraries/Native.sol"; @@ -28,7 +29,7 @@ import {Native} from "contracts/core/libraries/Native.sol"; /// @title Donation Voting Strategy with off-chain setup /// @notice Strategy that allows allocations in multiple tokens to accepted recipient. The actual payouts are set /// by the pool manager. -contract DonationVotingOffchain is BaseStrategy, RecipientsExtension, Native { +contract DonationVotingOffchain is BaseStrategy, RecipientsExtension, AllocationExtension, Native { using Transfer for address; /// =============================== @@ -46,12 +47,6 @@ contract DonationVotingOffchain is BaseStrategy, RecipientsExtension, Native { /// @param amount The amount of pool tokens set event PayoutSet(address indexed recipientId, uint256 amount); - /// @notice Emitted when the allocation timestamps are updated - /// @param allocationStartTime The start time for the allocation period - /// @param allocationEndTime The end time for the allocation period - /// @param sender The sender of the transaction - event AllocationTimestampsUpdated(uint64 allocationStartTime, uint64 allocationEndTime, address sender); - /// ================================ /// ========== Errors ============== /// ================================ @@ -59,9 +54,6 @@ contract DonationVotingOffchain is BaseStrategy, RecipientsExtension, Native { /// @notice Thrown when there is nothing to distribute for the given recipient. error NOTHING_TO_DISTRIBUTE(address recipientId); - /// @notice Thrown when the timestamps being set or updated don't meet the contracts requirements. - error INVALID_TIMESTAMPS(); - /// @notice Thrown when a the payout for a recipient is attempted to be overwritten. error PAYOUT_ALREADY_SET(address recipientId); @@ -94,40 +86,16 @@ contract DonationVotingOffchain is BaseStrategy, RecipientsExtension, Native { /// @notice If true, allocations are directly sent to recipients. Otherwise, they they must be claimed later. bool public immutable DIRECT_TRANSFER; - /// @notice The start and end times for allocations - uint64 public allocationStartTime; - uint64 public allocationEndTime; /// @notice Cooldown time from allocationEndTime after which the pool manager is allowed to withdraw tokens. uint64 public withdrawalCooldown; /// @notice amount to be distributed. `totalPayoutAmount` get reduced with each distribution. uint256 public totalPayoutAmount; - /// @notice token -> bool - mapping(address => bool) public allowedTokens; /// @notice recipientId -> PayoutSummary mapping(address => PayoutSummary) public payoutSummaries; /// @notice recipientId -> token -> amount mapping(address => mapping(address => uint256)) public amountAllocated; - /// ================================ - /// ========== Modifier ============ - /// ================================ - - /// @notice Modifier to check if allocation is active - /// @dev Reverts if allocation is not active - modifier onlyActiveAllocation() { - if (block.timestamp < allocationStartTime) revert ALLOCATION_NOT_ACTIVE(); - if (block.timestamp > allocationEndTime) revert ALLOCATION_NOT_ACTIVE(); - _; - } - - /// @notice Modifier to check if allocation has ended - /// @dev Reverts if allocation has not ended - modifier onlyAfterAllocation() { - if (block.timestamp <= allocationEndTime) revert ALLOCATION_NOT_ENDED(); - _; - } - /// =============================== /// ======== Constructor ========== /// =============================== @@ -151,7 +119,8 @@ contract DonationVotingOffchain is BaseStrategy, RecipientsExtension, Native { /// uint64 _allocationStartTime, /// uint64 _allocationEndTime, /// uint64 _withdrawalCooldown, - /// address[] _allowedTokens + /// address[] _allowedTokens, + /// bool _isUsingAllocationMetadata /// ) function initialize(uint256 _poolId, bytes memory _data) external virtual override { ( @@ -159,26 +128,15 @@ contract DonationVotingOffchain is BaseStrategy, RecipientsExtension, Native { uint64 _allocationStartTime, uint64 _allocationEndTime, uint64 _withdrawalCooldown, - address[] memory _allowedTokens - ) = abi.decode(_data, (RecipientInitializeData, uint64, uint64, uint64, address[])); - - allocationStartTime = _allocationStartTime; - allocationEndTime = _allocationEndTime; - emit AllocationTimestampsUpdated(_allocationStartTime, _allocationEndTime, msg.sender); + address[] memory _allowedTokens, + bool _isUsingAllocationMetadata + ) = abi.decode(_data, (RecipientInitializeData, uint64, uint64, uint64, address[], bool)); withdrawalCooldown = _withdrawalCooldown; - if (_allowedTokens.length == 0) { - // all tokens - allowedTokens[address(0)] = true; - } else { - for (uint256 i; i < _allowedTokens.length; i++) { - allowedTokens[_allowedTokens[i]] = true; - } - } - __BaseStrategy_init(_poolId); __RecipientsExtension_init(_recipientExtensionInitializeData); + __AllocationExtension_init(_allowedTokens, _allocationStartTime, _allocationEndTime, _isUsingAllocationMetadata); emit Initialized(_poolId, _data); } @@ -187,26 +145,6 @@ contract DonationVotingOffchain is BaseStrategy, RecipientsExtension, Native { /// ======= External/Custom ======= /// =============================== - /// @notice Sets the start and end dates. - /// @dev The 'msg.sender' must be a pool manager. - /// @param _registrationStartTime The start time for the registration - /// @param _registrationEndTime The end time for the registration - /// @param _allocationStartTime The start time for the allocation - /// @param _allocationEndTime The end time for the allocation - function updatePoolTimestamps( - uint64 _registrationStartTime, - uint64 _registrationEndTime, - uint64 _allocationStartTime, - uint64 _allocationEndTime - ) external onlyPoolManager(msg.sender) { - if (_allocationStartTime > _allocationEndTime) revert INVALID_TIMESTAMPS(); - allocationStartTime = _allocationStartTime; - allocationEndTime = _allocationEndTime; - emit AllocationTimestampsUpdated(allocationStartTime, allocationEndTime, msg.sender); - - _updatePoolTimestamps(_registrationStartTime, _registrationEndTime); - } - /// @notice Transfers the allocated tokens to recipients. /// @dev This function is ignored if DIRECT_TRANSFER is enabled, in which case allocated tokens are not stored /// in the contract for later claim but directly sent to recipients in `_allocate()`. @@ -341,26 +279,14 @@ contract DonationVotingOffchain is BaseStrategy, RecipientsExtension, Native { if (block.timestamp > allocationEndTime) revert POOL_INACTIVE(); } - /// @notice Checks if the timestamps are valid. - /// @param _registrationStartTime The start time for the registration - /// @param _registrationEndTime The end time for the registration - function _isPoolTimestampValid(uint64 _registrationStartTime, uint64 _registrationEndTime) - internal - view - virtual - override - { - if (_registrationStartTime > _registrationEndTime) revert INVALID_TIMESTAMPS(); - if (block.timestamp > _registrationStartTime) revert INVALID_TIMESTAMPS(); - // Check consistency with allocation timestamps - if (_registrationStartTime > allocationStartTime) revert INVALID_TIMESTAMPS(); - if (_registrationEndTime > allocationEndTime) revert INVALID_TIMESTAMPS(); - } - /// @notice Returns if the recipient is accepted /// @param _recipientId The recipient id /// @return If the recipient is accepted function _isAcceptedRecipient(address _recipientId) internal view virtual returns (bool) { return _getRecipientStatus(_recipientId) == Status.Accepted; } + + function _isValidAllocator(address) internal view override returns (bool) { + return true; + } } diff --git a/contracts/strategies/extensions/allocate/AllocationExtension.sol b/contracts/strategies/extensions/allocate/AllocationExtension.sol index 685a4f9bc..3cb4220bd 100644 --- a/contracts/strategies/extensions/allocate/AllocationExtension.sol +++ b/contracts/strategies/extensions/allocate/AllocationExtension.sol @@ -109,18 +109,18 @@ abstract contract AllocationExtension is BaseStrategy, IAllocationExtension { /// @dev Ensure the function is called before allocation start time function _checkBeforeAllocation() internal virtual { - if (block.timestamp >= allocationStartTime) revert ALLOCATION_HAS_STARTED(); + if (block.timestamp >= allocationStartTime) revert ALLOCATION_HAS_ALREADY_STARTED(); } /// @dev Ensure the function is called during allocation times function _checkOnlyActiveAllocation() internal virtual { - if (block.timestamp < allocationStartTime) revert ALLOCATION_NOT_ACTIVE(); - if (block.timestamp > allocationEndTime) revert ALLOCATION_NOT_ACTIVE(); + if (block.timestamp < allocationStartTime) revert ALLOCATION_IS_NOT_ACTIVE(); + if (block.timestamp > allocationEndTime) revert ALLOCATION_IS_NOT_ACTIVE(); } /// @dev Ensure the function is called after allocation start time function _checkOnlyAfterAllocation() internal virtual { - if (block.timestamp <= allocationEndTime) revert ALLOCATION_NOT_ENDED(); + if (block.timestamp <= allocationEndTime) revert ALLOCATION_IS_NOT_ENDED(); } // ==================================== diff --git a/contracts/strategies/extensions/allocate/IAllocationExtension.sol b/contracts/strategies/extensions/allocate/IAllocationExtension.sol index 88f4d34f5..0839d914b 100644 --- a/contracts/strategies/extensions/allocate/IAllocationExtension.sol +++ b/contracts/strategies/extensions/allocate/IAllocationExtension.sol @@ -6,13 +6,13 @@ interface IAllocationExtension { error INVALID_ALLOCATION_TIMESTAMPS(); /// @dev Error thrown when trying to call the function when the allocation has started - error ALLOCATION_HAS_STARTED(); + error ALLOCATION_HAS_ALREADY_STARTED(); /// @dev Error thrown when trying to call the function when the allocation is not active - error ALLOCATION_NOT_ACTIVE(); + error ALLOCATION_IS_NOT_ACTIVE(); /// @dev Error thrown when trying to call the function when the allocation has ended - error ALLOCATION_NOT_ENDED(); + error ALLOCATION_IS_NOT_ENDED(); /// @notice Emitted when the allocation timestamps are updated /// @param allocationStartTime The start time for the allocation period diff --git a/test/integration/DonationVotingMerkleDistribution.t.sol b/test/integration/DonationVotingMerkleDistribution.t.sol index 7eeef6225..ec59821ae 100644 --- a/test/integration/DonationVotingMerkleDistribution.t.sol +++ b/test/integration/DonationVotingMerkleDistribution.t.sol @@ -165,48 +165,6 @@ contract IntegrationDonationVotingMerkleDistributionReviewRecipients is } } -contract IntegrationDonationVotingMerkleDistributionTimestamps is IntegrationDonationVotingMerkleDistributionBase { - function test_updateTimestamps() public { - vm.warp(registrationStartTime - 1 days); - - // Review recipients - vm.startPrank(userAddr); - - vm.expectRevert(DonationVotingOffchain.INVALID_TIMESTAMPS.selector); - // allocationStartTime > allocationEndTime - strategy.updatePoolTimestamps( - registrationStartTime, registrationEndTime, allocationEndTime, allocationStartTime - ); - - vm.expectRevert(DonationVotingOffchain.INVALID_TIMESTAMPS.selector); - // _registrationStartTime > _registrationEndTime - strategy.updatePoolTimestamps( - registrationEndTime, registrationStartTime, allocationStartTime, allocationEndTime - ); - - vm.expectRevert(DonationVotingOffchain.INVALID_TIMESTAMPS.selector); - // _registrationStartTime > allocationStartTime - strategy.updatePoolTimestamps( - allocationStartTime + 1, allocationEndTime, allocationStartTime, allocationEndTime - ); - - vm.expectRevert(DonationVotingOffchain.INVALID_TIMESTAMPS.selector); - // _registrationEndTime > allocationEndTime - strategy.updatePoolTimestamps( - registrationStartTime, allocationEndTime + 1, allocationStartTime, allocationEndTime - ); - - vm.warp(registrationStartTime + 1); - vm.expectRevert(DonationVotingOffchain.INVALID_TIMESTAMPS.selector); - // block.timestamp > _registrationStartTime - strategy.updatePoolTimestamps( - registrationStartTime, registrationEndTime, allocationStartTime, allocationEndTime - ); - - vm.stopPrank(); - } -} - contract IntegrationDonationVotingMerkleDistributionAllocateERC20 is IntegrationDonationVotingMerkleDistributionBase { function setUp() public override { super.setUp(); diff --git a/test/integration/DonationVotingOffchain.t.sol b/test/integration/DonationVotingOffchain.t.sol index dee44be5f..c2455d6c7 100644 --- a/test/integration/DonationVotingOffchain.t.sol +++ b/test/integration/DonationVotingOffchain.t.sol @@ -160,48 +160,6 @@ contract IntegrationDonationVotingOffchainReviewRecipients is IntegrationDonatio } } -contract IntegrationDonationVotingOffchainTimestamps is IntegrationDonationVotingOffchainBase { - function test_updateTimestamps() public { - vm.warp(registrationStartTime - 1 days); - - // Review recipients - vm.startPrank(userAddr); - - vm.expectRevert(DonationVotingOffchain.INVALID_TIMESTAMPS.selector); - // allocationStartTime > allocationEndTime - strategy.updatePoolTimestamps( - registrationStartTime, registrationEndTime, allocationEndTime, allocationStartTime - ); - - vm.expectRevert(DonationVotingOffchain.INVALID_TIMESTAMPS.selector); - // _registrationStartTime > _registrationEndTime - strategy.updatePoolTimestamps( - registrationEndTime, registrationStartTime, allocationStartTime, allocationEndTime - ); - - vm.expectRevert(DonationVotingOffchain.INVALID_TIMESTAMPS.selector); - // _registrationStartTime > allocationStartTime - strategy.updatePoolTimestamps( - allocationStartTime + 1, allocationEndTime, allocationStartTime, allocationEndTime - ); - - vm.expectRevert(DonationVotingOffchain.INVALID_TIMESTAMPS.selector); - // _registrationEndTime > allocationEndTime - strategy.updatePoolTimestamps( - registrationStartTime, allocationEndTime + 1, allocationStartTime, allocationEndTime - ); - - vm.warp(registrationStartTime + 1); - vm.expectRevert(DonationVotingOffchain.INVALID_TIMESTAMPS.selector); - // block.timestamp > _registrationStartTime - strategy.updatePoolTimestamps( - registrationStartTime, registrationEndTime, allocationStartTime, allocationEndTime - ); - - vm.stopPrank(); - } -} - contract IntegrationDonationVotingOffchainAllocateERC20 is IntegrationDonationVotingOffchainBase { function setUp() public override { super.setUp(); diff --git a/test/unit/strategies/extensions/AllocationExtensionUnit.t.sol b/test/unit/strategies/extensions/AllocationExtensionUnit.t.sol index 78273365f..4d77a7903 100644 --- a/test/unit/strategies/extensions/AllocationExtensionUnit.t.sol +++ b/test/unit/strategies/extensions/AllocationExtensionUnit.t.sol @@ -129,7 +129,7 @@ contract AllocationExtension is Test { vm.warp(_timestamp); // It should revert - vm.expectRevert(IAllocationExtension.ALLOCATION_HAS_STARTED.selector); + vm.expectRevert(IAllocationExtension.ALLOCATION_HAS_ALREADY_STARTED.selector); extension.call__checkBeforeAllocation(); } @@ -144,7 +144,7 @@ contract AllocationExtension is Test { vm.warp(_timestamp); // It should revert - vm.expectRevert(IAllocationExtension.ALLOCATION_NOT_ACTIVE.selector); + vm.expectRevert(IAllocationExtension.ALLOCATION_IS_NOT_ACTIVE.selector); extension.call__checkOnlyActiveAllocation(); } @@ -159,7 +159,7 @@ contract AllocationExtension is Test { vm.warp(_timestamp); // It should revert - vm.expectRevert(IAllocationExtension.ALLOCATION_NOT_ACTIVE.selector); + vm.expectRevert(IAllocationExtension.ALLOCATION_IS_NOT_ACTIVE.selector); extension.call__checkOnlyActiveAllocation(); } @@ -174,7 +174,7 @@ contract AllocationExtension is Test { vm.warp(_timestamp); // It should revert - vm.expectRevert(IAllocationExtension.ALLOCATION_NOT_ENDED.selector); + vm.expectRevert(IAllocationExtension.ALLOCATION_IS_NOT_ENDED.selector); extension.call__checkOnlyAfterAllocation(); } From 0edbe1cb4c10df615b0b315e2b0df0cb2c2ec04a Mon Sep 17 00:00:00 2001 From: 0xAustrian Date: Wed, 4 Sep 2024 14:41:10 -0300 Subject: [PATCH 17/20] feat: implement allocation extension in DonationVotingOnchain --- .../donation-voting/DonationVotingOnchain.sol | 113 +++++------------- test/integration/DonationVotingOffchain.t.sol | 3 +- test/integration/DonationVotingOnchain.t.sol | 65 ++-------- 3 files changed, 41 insertions(+), 140 deletions(-) diff --git a/contracts/strategies/examples/donation-voting/DonationVotingOnchain.sol b/contracts/strategies/examples/donation-voting/DonationVotingOnchain.sol index 2865cf3ca..014b19ac4 100644 --- a/contracts/strategies/examples/donation-voting/DonationVotingOnchain.sol +++ b/contracts/strategies/examples/donation-voting/DonationVotingOnchain.sol @@ -6,6 +6,7 @@ import {IAllo} from "contracts/core/interfaces/IAllo.sol"; // Core Contracts import {BaseStrategy} from "strategies/BaseStrategy.sol"; import {RecipientsExtension} from "strategies/extensions/register/RecipientsExtension.sol"; +import {AllocationExtension} from "strategies/extensions/allocate/AllocationExtension.sol"; // Internal Libraries import {QFHelper} from "strategies/libraries/QFHelper.sol"; import {Native} from "contracts/core/libraries/Native.sol"; @@ -29,20 +30,10 @@ import {Transfer} from "contracts/core/libraries/Transfer.sol"; /// @title Donation Voting Strategy with qudratic funding tracked on-chain /// @notice Strategy that allows allocations in a specified token to accepted recipient. Payouts are calculated from /// allocations based on the quadratic funding formula. -contract DonationVotingOnchain is BaseStrategy, RecipientsExtension, Native { +contract DonationVotingOnchain is BaseStrategy, RecipientsExtension, AllocationExtension, Native { using QFHelper for QFHelper.State; using Transfer for address; - /// =============================== - /// ========== Events ============= - /// =============================== - - /// @notice Emitted when the allocation timestamps are updated - /// @param allocationStartTime The start time for the allocation period - /// @param allocationEndTime The end time for the allocation period - /// @param sender The sender of the transaction - event AllocationTimestampsUpdated(uint64 allocationStartTime, uint64 allocationEndTime, address sender); - /// =============================== /// ========== Errors ============= /// =============================== @@ -53,45 +44,24 @@ contract DonationVotingOnchain is BaseStrategy, RecipientsExtension, Native { /// @notice Thrown when the timestamps being set or updated don't meet the contracts requirements. error INVALID_TIMESTAMPS(); + /// @notice Thrown when the allocation token is not allowed. + error TOKEN_NOT_ALLOWED(); + /// ================================ /// ========== Storage ============= /// ================================ - /// @notice The start and end times for allocations - uint64 public allocationStartTime; - uint64 public allocationEndTime; /// @notice Cooldown time from allocationEndTime after which the pool manager is allowed to withdraw tokens. uint64 public withdrawalCooldown; /// @notice amount to be distributed. It is set during the first distribute() call and stays fixed. uint256 public totalPayoutAmount; - /// @notice token -> bool - address public allocationToken; /// @notice recipientId -> amount mapping(address => uint256) public amountAllocated; /// @notice QFHelper.State public QFState; - /// ================================ - /// ========== Modifier ============ - /// ================================ - - /// @notice Modifier to check if allocation is active - /// @dev Reverts if allocation is not active - modifier onlyActiveAllocation() { - if (block.timestamp < allocationStartTime) revert ALLOCATION_NOT_ACTIVE(); - if (block.timestamp > allocationEndTime) revert ALLOCATION_NOT_ACTIVE(); - _; - } - - /// @notice Modifier to check if allocation has ended - /// @dev Reverts if allocation has not ended - modifier onlyAfterAllocation() { - if (block.timestamp <= allocationEndTime) revert ALLOCATION_NOT_ENDED(); - _; - } - /// =============================== /// ======== Constructor ========== /// =============================== @@ -112,7 +82,8 @@ contract DonationVotingOnchain is BaseStrategy, RecipientsExtension, Native { /// uint64 _allocationStartTime, /// uint64 _allocationEndTime, /// uint64 _withdrawalCooldown, - /// address _allocationToken + /// address _allocationToken, + /// bool _isUsingAllocationMetadata /// ) function initialize(uint256 _poolId, bytes memory _data) external virtual override { ( @@ -120,46 +91,21 @@ contract DonationVotingOnchain is BaseStrategy, RecipientsExtension, Native { uint64 _allocationStartTime, uint64 _allocationEndTime, uint64 _withdrawalCooldown, - address _allocationToken - ) = abi.decode(_data, (RecipientInitializeData, uint64, uint64, uint64, address)); - - allocationStartTime = _allocationStartTime; - allocationEndTime = _allocationEndTime; - emit AllocationTimestampsUpdated(_allocationStartTime, _allocationEndTime, msg.sender); + address _allocationToken, + bool _isUsingAllocationMetadata + ) = abi.decode(_data, (RecipientInitializeData, uint64, uint64, uint64, address, bool)); withdrawalCooldown = _withdrawalCooldown; - allocationToken = _allocationToken; __BaseStrategy_init(_poolId); __RecipientsExtension_init(_recipientExtensionInitializeData); + address[] memory _allowedTokens = new address[](1); + _allowedTokens[0] = _allocationToken; + __AllocationExtension_init(_allowedTokens, _allocationStartTime, _allocationEndTime, _isUsingAllocationMetadata); emit Initialized(_poolId, _data); } - /// =============================== - /// ======= External/Custom ======= - /// =============================== - - /// @notice Sets the start and end dates. - /// @dev The 'msg.sender' must be a pool manager. - /// @param _registrationStartTime The start time for the registration - /// @param _registrationEndTime The end time for the registration - /// @param _allocationStartTime The start time for the allocation - /// @param _allocationEndTime The end time for the allocation - function updatePoolTimestamps( - uint64 _registrationStartTime, - uint64 _registrationEndTime, - uint64 _allocationStartTime, - uint64 _allocationEndTime - ) external onlyPoolManager(msg.sender) { - if (_allocationStartTime > _allocationEndTime) revert INVALID_TIMESTAMPS(); - allocationStartTime = _allocationStartTime; - allocationEndTime = _allocationEndTime; - emit AllocationTimestampsUpdated(allocationStartTime, allocationEndTime, msg.sender); - - _updatePoolTimestamps(_registrationStartTime, _registrationEndTime); - } - /// ==================================== /// ============ Internal ============== /// ==================================== @@ -167,7 +113,7 @@ contract DonationVotingOnchain is BaseStrategy, RecipientsExtension, Native { /// @notice This will allocate to recipients. /// @param _recipients The addresses of the recipients to allocate to /// @param _amounts The amounts to allocate to the recipients - /// @param _data The data containing permit data for the sum of '_amounts' if needed (ignored if empty) + /// @param _data The data containing the allocationToken and permit data for the sum of '_amounts' if needed (ignored if empty) /// @param _sender The address of the sender function _allocate(address[] memory _recipients, uint256[] memory _amounts, bytes memory _data, address _sender) internal @@ -175,6 +121,9 @@ contract DonationVotingOnchain is BaseStrategy, RecipientsExtension, Native { override onlyActiveAllocation { + (address allocationToken, bytes memory permitData) = abi.decode(_data, (address, bytes)); + if (!_isAllowedToken(allocationToken)) revert TOKEN_NOT_ALLOWED(); + uint256 totalAmount; for (uint256 i = 0; i < _recipients.length; i++) { if (!_isAcceptedRecipient(_recipients[i])) revert RECIPIENT_NOT_ACCEPTED(); @@ -189,7 +138,7 @@ contract DonationVotingOnchain is BaseStrategy, RecipientsExtension, Native { if (allocationToken == NATIVE) { if (msg.value != totalAmount) revert ETH_MISMATCH(); } else { - allocationToken.usePermit(_sender, address(this), totalAmount, _data); + allocationToken.usePermit(_sender, address(this), totalAmount, permitData); allocationToken.transferAmountFrom(_sender, address(this), totalAmount); } @@ -198,8 +147,9 @@ contract DonationVotingOnchain is BaseStrategy, RecipientsExtension, Native { /// @notice Distributes funds (tokens) to recipients. /// @param _recipientIds The IDs of the recipients + /// @param _data The data containing the allocation token /// @param _sender The address of the sender - function _distribute(address[] memory _recipientIds, bytes memory, address _sender) + function _distribute(address[] memory _recipientIds, bytes memory _data, address _sender) internal virtual override @@ -207,6 +157,9 @@ contract DonationVotingOnchain is BaseStrategy, RecipientsExtension, Native { { if (totalPayoutAmount == 0) totalPayoutAmount = poolAmount; + (address allocationToken) = abi.decode(_data, (address)); + if (!_isAllowedToken(allocationToken)) revert TOKEN_NOT_ALLOWED(); + for (uint256 i; i < _recipientIds.length; i++) { address recipientId = _recipientIds[i]; address recipientAddress = _recipients[recipientId].recipientAddress; @@ -240,26 +193,14 @@ contract DonationVotingOnchain is BaseStrategy, RecipientsExtension, Native { if (block.timestamp > allocationEndTime) revert POOL_INACTIVE(); } - /// @notice Checks if the timestamps are valid. - /// @param _registrationStartTime The start time for the registration - /// @param _registrationEndTime The end time for the registration - function _isPoolTimestampValid(uint64 _registrationStartTime, uint64 _registrationEndTime) - internal - view - virtual - override - { - if (_registrationStartTime > _registrationEndTime) revert INVALID_TIMESTAMPS(); - if (block.timestamp > _registrationStartTime) revert INVALID_TIMESTAMPS(); - // Check consistency with allocation timestamps - if (_registrationStartTime > allocationStartTime) revert INVALID_TIMESTAMPS(); - if (_registrationEndTime > allocationEndTime) revert INVALID_TIMESTAMPS(); - } - /// @notice Returns if the recipient is accepted /// @param _recipientId The recipient id /// @return If the recipient is accepted function _isAcceptedRecipient(address _recipientId) internal view virtual returns (bool) { return _getRecipientStatus(_recipientId) == Status.Accepted; } + + function _isValidAllocator(address) internal view override returns (bool) { + return true; + } } diff --git a/test/integration/DonationVotingOffchain.t.sol b/test/integration/DonationVotingOffchain.t.sol index c2455d6c7..185756a35 100644 --- a/test/integration/DonationVotingOffchain.t.sol +++ b/test/integration/DonationVotingOffchain.t.sol @@ -69,7 +69,8 @@ contract IntegrationDonationVotingOffchainBase is IntegrationBase { allocationStartTime, allocationEndTime, withdrawalCooldown, - allowedTokens + allowedTokens, + true ), DAI, POOL_AMOUNT, diff --git a/test/integration/DonationVotingOnchain.t.sol b/test/integration/DonationVotingOnchain.t.sol index 0b473b9b6..87a287b9b 100644 --- a/test/integration/DonationVotingOnchain.t.sol +++ b/test/integration/DonationVotingOnchain.t.sol @@ -65,7 +65,8 @@ contract IntegrationDonationVotingOnchainBase is IntegrationBase { allocationStartTime, allocationEndTime, withdrawalCooldown, - allocationToken + allocationToken, + true ), DAI, POOL_AMOUNT, @@ -132,48 +133,6 @@ contract IntegrationDonationVotingOnchainReviewRecipients is IntegrationDonation } } -contract IntegrationDonationVotingOnchainTimestamps is IntegrationDonationVotingOnchainBase { - function test_updateTimestamps() public { - vm.warp(registrationStartTime - 1 days); - - // Review recipients - vm.startPrank(userAddr); - - vm.expectRevert(DonationVotingOnchain.INVALID_TIMESTAMPS.selector); - // allocationStartTime > allocationEndTime - strategy.updatePoolTimestamps( - registrationStartTime, registrationEndTime, allocationEndTime, allocationStartTime - ); - - vm.expectRevert(DonationVotingOnchain.INVALID_TIMESTAMPS.selector); - // _registrationStartTime > _registrationEndTime - strategy.updatePoolTimestamps( - registrationEndTime, registrationStartTime, allocationStartTime, allocationEndTime - ); - - vm.expectRevert(DonationVotingOnchain.INVALID_TIMESTAMPS.selector); - // _registrationStartTime > allocationStartTime - strategy.updatePoolTimestamps( - allocationStartTime + 1, allocationEndTime, allocationStartTime, allocationEndTime - ); - - vm.expectRevert(DonationVotingOnchain.INVALID_TIMESTAMPS.selector); - // _registrationEndTime > allocationEndTime - strategy.updatePoolTimestamps( - registrationStartTime, allocationEndTime + 1, allocationStartTime, allocationEndTime - ); - - vm.warp(registrationStartTime + 1); - vm.expectRevert(DonationVotingOnchain.INVALID_TIMESTAMPS.selector); - // block.timestamp > _registrationStartTime - strategy.updatePoolTimestamps( - registrationStartTime, registrationEndTime, allocationStartTime, allocationEndTime - ); - - vm.stopPrank(); - } -} - contract IntegrationDonationVotingOnchainAllocateERC20 is IntegrationDonationVotingOnchainBase { function setUp() public override { super.setUp(); @@ -214,14 +173,14 @@ contract IntegrationDonationVotingOnchainAllocateERC20 is IntegrationDonationVot vm.startPrank(address(allo)); - strategy.allocate(recipients, amounts, "", allocator0); + strategy.allocate(recipients, amounts, abi.encode(allocationToken, bytes("")), allocator0); assertEq(IERC20(allocationToken).balanceOf(allocator0), 0); assertEq(IERC20(allocationToken).balanceOf(address(strategy)), 4 + 25); recipients[0] = recipient2Addr; vm.expectRevert(Errors.RECIPIENT_NOT_ACCEPTED.selector); - strategy.allocate(recipients, amounts, "", allocator0); + strategy.allocate(recipients, amounts, abi.encode(allocationToken, bytes("")), allocator0); vm.stopPrank(); } @@ -266,14 +225,14 @@ contract IntegrationDonationVotingOnchainAllocateETH is IntegrationDonationVotin vm.startPrank(address(allo)); - strategy.allocate{value: 4 + 25}(recipients, amounts, "", allocator0); + strategy.allocate{value: 4 + 25}(recipients, amounts, abi.encode(allocationToken, bytes("")), allocator0); assertEq(allocator0.balance, 0); assertEq(address(strategy).balance, 4 + 25); recipients[0] = recipient2Addr; vm.expectRevert(Errors.RECIPIENT_NOT_ACCEPTED.selector); - strategy.allocate(recipients, amounts, "", allocator0); + strategy.allocate(recipients, amounts, abi.encode(allocationToken, bytes("")), allocator0); vm.stopPrank(); } @@ -316,7 +275,7 @@ contract IntegrationDonationVotingOnchainDistributeERC20 is IntegrationDonationV amounts[1] = 25; vm.startPrank(address(allo)); - strategy.allocate(recipients, amounts, "", allocator0); + strategy.allocate(recipients, amounts, abi.encode(allocationToken, bytes("")), allocator0); vm.stopPrank(); } @@ -329,7 +288,7 @@ contract IntegrationDonationVotingOnchainDistributeERC20 is IntegrationDonationV address[] memory recipients = new address[](2); recipients[0] = recipient0Addr; recipients[1] = recipient1Addr; - strategy.distribute(recipients, "", recipient0Addr); + strategy.distribute(recipients, abi.encode(allocationToken), recipient0Addr); uint256 recipient0Matching = POOL_AMOUNT * 4 / 29; uint256 recipient1Matching = POOL_AMOUNT * 25 / 29; @@ -342,7 +301,7 @@ contract IntegrationDonationVotingOnchainDistributeERC20 is IntegrationDonationV assertEq(IERC20(allocationToken).balanceOf(address(strategy)), 0); vm.expectRevert(abi.encodeWithSelector(DonationVotingOnchain.NOTHING_TO_DISTRIBUTE.selector, recipient0Addr)); - strategy.distribute(recipients, "", recipient0Addr); + strategy.distribute(recipients, abi.encode(allocationToken), recipient0Addr); vm.stopPrank(); } @@ -384,7 +343,7 @@ contract IntegrationDonationVotingOnchainDistributeETH is IntegrationDonationVot amounts[1] = 25; vm.prank(address(allo)); - strategy.allocate{value: 4 + 25}(recipients, amounts, "", allocator0); + strategy.allocate{value: 4 + 25}(recipients, amounts, abi.encode(allocationToken, bytes("")), allocator0); } function test_distribute() public { @@ -396,7 +355,7 @@ contract IntegrationDonationVotingOnchainDistributeETH is IntegrationDonationVot address[] memory recipients = new address[](2); recipients[0] = recipient0Addr; recipients[1] = recipient1Addr; - strategy.distribute(recipients, "", recipient0Addr); + strategy.distribute(recipients, abi.encode(allocationToken), recipient0Addr); uint256 recipient0Matching = POOL_AMOUNT * 4 / 29; uint256 recipient1Matching = POOL_AMOUNT * 25 / 29; @@ -409,7 +368,7 @@ contract IntegrationDonationVotingOnchainDistributeETH is IntegrationDonationVot assertEq(address(strategy).balance, 0); vm.expectRevert(abi.encodeWithSelector(DonationVotingOnchain.NOTHING_TO_DISTRIBUTE.selector, recipient0Addr)); - strategy.distribute(recipients, "", recipient0Addr); + strategy.distribute(recipients, abi.encode(allocationToken), recipient0Addr); vm.stopPrank(); } From d74fb487b3eaee75eb19ea54b151d41c99864f3d Mon Sep 17 00:00:00 2001 From: 0xAustrian Date: Wed, 4 Sep 2024 14:58:04 -0300 Subject: [PATCH 18/20] feat: implement allocation extensions for QVSimple and QVImpactStream --- .../examples/impact-stream/QVImpactStream.sol | 20 ----- .../examples/quadratic-voting/QVSimple.sol | 90 +------------------ test/integration/QVImpactStream.t.sol | 6 +- test/integration/QVSimple.t.sol | 8 +- test/unit/strategies/QVImpactStream.t.sol | 30 ------- test/unit/strategies/QVImpactStream.tree | 9 -- 6 files changed, 11 insertions(+), 152 deletions(-) diff --git a/contracts/strategies/examples/impact-stream/QVImpactStream.sol b/contracts/strategies/examples/impact-stream/QVImpactStream.sol index 978097d64..6c3a0acc5 100644 --- a/contracts/strategies/examples/impact-stream/QVImpactStream.sol +++ b/contracts/strategies/examples/impact-stream/QVImpactStream.sol @@ -81,26 +81,6 @@ contract QVImpactStream is QVSimple { /// ==== External/Public Functions ===== /// ==================================== - /// @notice Add allocator array - /// @dev Only the pool manager(s) can call this function and emits an `AllocatorAdded` event - /// @param _allocators The allocator address array - function batchAddAllocator(address[] memory _allocators) external onlyPoolManager(msg.sender) { - uint256 length = _allocators.length; - for (uint256 i = 0; i < length; ++i) { - _addAllocator(_allocators[i]); - } - } - - /// @notice Remove allocator array - /// @dev Only the pool manager(s) can call this function and emits an `AllocatorRemoved` event - /// @param _allocators The allocators address array - function batchRemoveAllocator(address[] memory _allocators) external onlyPoolManager(msg.sender) { - uint256 length = _allocators.length; - for (uint256 i = 0; i < length; ++i) { - _removeAllocator(_allocators[i]); - } - } - /// @notice Set the payouts to distribute /// @dev Only the pool manager(s) can call this function /// @param _payouts The payouts to distribute diff --git a/contracts/strategies/examples/quadratic-voting/QVSimple.sol b/contracts/strategies/examples/quadratic-voting/QVSimple.sol index e2f41ed1e..e24a52442 100644 --- a/contracts/strategies/examples/quadratic-voting/QVSimple.sol +++ b/contracts/strategies/examples/quadratic-voting/QVSimple.sol @@ -7,6 +7,7 @@ import {IRecipientsExtension} from "strategies/extensions/register/IRecipientsEx // Contracts import {BaseStrategy} from "contracts/strategies/BaseStrategy.sol"; import {RecipientsExtension} from "strategies/extensions/register/RecipientsExtension.sol"; +import {AllocatorsAllowlistExtension} from "strategies/extensions/allocate/AllocatorsAllowlistExtension.sol"; // Internal Libraries import {Transfer} from "contracts/core/libraries/Transfer.sol"; import {QVHelper} from "strategies/libraries/QVHelper.sol"; @@ -25,7 +26,7 @@ import {QVHelper} from "strategies/libraries/QVHelper.sol"; // ⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠈⠛⣿⣿⣿⣿⣿⣿⣿⣿⣿⣿⣿⣿⣿⣿⡄⠀⠀⠀⠀⠀⠀⠀⠀⠀⢰⣿⣿⣿⣿⠃⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠘⣿⣿⣿⣿⣧⠀⠀⢸⣿⣿⣿⣗⠀⠀⠀⢸⣿⣿⣿⡯⠀⠀⠀⠀⠹⢿⣿⣿⣿⣿⣾⣾⣷⣿⣿⣿⣿⡿⠋⠀⠀⠀⠀ // ⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠈⠙⠙⠋⠛⠙⠋⠛⠙⠋⠛⠙⠋⠃⠀⠀⠀⠀⠀⠀⠀⠀⠠⠿⠻⠟⠿⠃⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠀⠸⠟⠿⠟⠿⠆⠀⠸⠿⠿⠟⠯⠀⠀⠀⠸⠿⠿⠿⠏⠀⠀⠀⠀⠀⠈⠉⠻⠻⡿⣿⢿⡿⡿⠿⠛⠁⠀⠀⠀⠀⠀⠀ // allo.gitcoin.co -contract QVSimple is BaseStrategy, RecipientsExtension { +contract QVSimple is BaseStrategy, RecipientsExtension, AllocatorsAllowlistExtension { using QVHelper for QVHelper.VotingState; using Transfer for address; @@ -39,18 +40,9 @@ contract QVSimple is BaseStrategy, RecipientsExtension { /// @notice The maximum voice credits per allocator uint256 public maxVoiceCreditsPerAllocator; - /// @notice The start and end times for allocations - /// @dev The values will be in milliseconds since the epoch - uint64 public allocationStartTime; - uint64 public allocationEndTime; - /// @notice Whether the distribution started or not bool public distributionStarted; - /// @notice The details of the allowed allocator - /// @dev allocator => bool - mapping(address => bool) public allowedAllocators; - /// @notice The voice credits allocated for each allocator mapping(address => uint256) public voiceCreditsAllocated; @@ -88,20 +80,6 @@ contract QVSimple is BaseStrategy, RecipientsExtension { emit Initialized(_poolId, _data); } - /// ====================== - /// ======= Events ======= - /// ====================== - - /// @notice Emitted when an allocator is added - /// @param allocator The allocator address - /// @param sender The sender of the transaction - event AllocatorAdded(address indexed allocator, address sender); - - /// @notice Emitted when an allocator is removed - /// @param allocator The allocator address - /// @param sender The sender of the transaction - event AllocatorRemoved(address indexed allocator, address sender); - /// ====================== /// ======= Struct ======= /// ====================== @@ -113,45 +91,6 @@ contract QVSimple is BaseStrategy, RecipientsExtension { uint256 maxVoiceCreditsPerAllocator; } - /// ================================ - /// ========== Modifier ============ - /// ================================ - - /// @notice Modifier to check if the allocation has ended - /// @dev Reverts if the allocation has not ended - modifier onlyAfterAllocation() { - _checkOnlyAfterAllocation(); - _; - } - - /// ==================================== - /// ==== External/Public Functions ===== - /// ==================================== - - /// @notice Add allocator - /// @dev Only the pool manager(s) can call this function and emits an `AllocatorAdded` event - /// @param _allocator The allocator address - function addAllocator(address _allocator) external onlyPoolManager(msg.sender) { - _addAllocator(_allocator); - } - - /// @notice Remove allocator - /// @dev Only the pool manager(s) can call this function and emits an `AllocatorRemoved` event - /// @param _allocator The allocator address - function removeAllocator(address _allocator) external onlyPoolManager(msg.sender) { - _removeAllocator(_allocator); - } - - /// ==================================== - /// ============ Internal ============== - /// ==================================== - - /// @notice Check if the allocation has ended - /// @dev Reverts if the allocation has not ended - function _checkOnlyAfterAllocation() internal view virtual { - if (block.timestamp <= allocationEndTime) revert ALLOCATION_NOT_ENDED(); - } - /// @notice Distribute the tokens to the recipients /// @dev The '_sender' must be a pool manager and the allocation must have ended /// @param _recipientIds The recipient ids @@ -224,24 +163,6 @@ contract QVSimple is BaseStrategy, RecipientsExtension { voiceCreditsAllocated[_sender] += voiceCreditsToAllocate; } - /// @notice Add allocator - /// @dev Only the pool manager(s) can call this function and emits an `AllocatorAdded` event - /// @param _allocator The allocator address - function _addAllocator(address _allocator) internal virtual { - allowedAllocators[_allocator] = true; - - emit AllocatorAdded(_allocator, msg.sender); - } - - /// @notice Remove allocator - /// @dev Only the pool manager(s) can call this function and emits an `AllocatorRemoved` event - /// @param _allocator The allocator address - function _removeAllocator(address _allocator) internal virtual { - allowedAllocators[_allocator] = false; - - emit AllocatorRemoved(_allocator, msg.sender); - } - /// @notice Returns if the recipient is accepted /// @param _recipientId The recipient id /// @return true if the recipient is accepted @@ -249,13 +170,6 @@ contract QVSimple is BaseStrategy, RecipientsExtension { return _getRecipientStatus(_recipientId) == Status.Accepted; } - /// @notice Checks if the allocator is valid - /// @param _allocator The allocator address - /// @return true if the allocator is valid - function _isValidAllocator(address _allocator) internal view returns (bool) { - return allowedAllocators[_allocator]; - } - /// @notice Checks if the allocator has voice credits left /// @param _voiceCreditsToAllocate The voice credits to allocate /// @param _allocatedVoiceCredits The allocated voice credits diff --git a/test/integration/QVImpactStream.t.sol b/test/integration/QVImpactStream.t.sol index 0341bdfc8..ad0a97f1b 100644 --- a/test/integration/QVImpactStream.t.sol +++ b/test/integration/QVImpactStream.t.sol @@ -88,8 +88,10 @@ contract IntegrationQVImpactStream is Test { // Adding allocators vm.startPrank(profileOwner); - strategy.addAllocator(allocator0); - strategy.addAllocator(allocator1); + address[] memory allocators = new address[](2); + allocators[0] = allocator0; + allocators[1] = allocator1; + strategy.addAllocators(allocators); vm.stopPrank(); // Adding recipients diff --git a/test/integration/QVSimple.t.sol b/test/integration/QVSimple.t.sol index 9f461726f..8ca6261cb 100644 --- a/test/integration/QVSimple.t.sol +++ b/test/integration/QVSimple.t.sol @@ -59,9 +59,11 @@ contract IntegrationQVSimple is IntegrationBase { ); // Adding allocators - vm.startPrank(userAddr); - strategy.addAllocator(allocator0); - strategy.addAllocator(allocator1); + address[] memory allocators = new address[](2); + allocators[0] = allocator0; + allocators[1] = allocator1; + vm.prank(userAddr); + strategy.addAllocators(allocators); // Adding recipients vm.startPrank(address(allo)); diff --git a/test/unit/strategies/QVImpactStream.t.sol b/test/unit/strategies/QVImpactStream.t.sol index 924c47156..4fd926d96 100644 --- a/test/unit/strategies/QVImpactStream.t.sol +++ b/test/unit/strategies/QVImpactStream.t.sol @@ -71,36 +71,6 @@ contract QVImpactStreamTest is Test { _; } - function test_BatchAddAllocatorWhenCalledByPoolManager() external callWithPoolManager { - // if AllocatorAdded event emits with the correct parameters then _addAllocator was also called - // with the correct parameters - vm.expectEmit(true, true, true, true); - emit AllocatorAdded(recipient1, poolManager); - - address[] memory _recipients = new address[](1); - _recipients[0] = recipient1; - - vm.prank(poolManager); - qvImpactStream.batchAddAllocator(_recipients); - - assertTrue(qvImpactStream.allowedAllocators(recipient1)); - } - - function test_BatchRemoveAllocatorWhenCalledByPoolManager() external callWithPoolManager { - // if AllocatorRemoved event emits with the correct parameters then _removeAllocator was also called - // with the correct parameters - vm.expectEmit(true, true, true, true); - emit AllocatorRemoved(recipient1, poolManager); - - address[] memory _recipients = new address[](1); - _recipients[0] = recipient1; - - vm.prank(poolManager); - qvImpactStream.batchRemoveAllocator(_recipients); - - assertFalse(qvImpactStream.allowedAllocators(recipient1)); - } - function test_SetPayoutsRevertWhen_PayoutSetIsTrue() external callWithPoolManager { stdstore.target(address(qvImpactStream)).sig("payoutSet()").checked_write(true); vm.expectRevert(QVImpactStream.PAYOUT_ALREADY_SET.selector); diff --git a/test/unit/strategies/QVImpactStream.tree b/test/unit/strategies/QVImpactStream.tree index cfa32cc02..81a0dce38 100644 --- a/test/unit/strategies/QVImpactStream.tree +++ b/test/unit/strategies/QVImpactStream.tree @@ -1,12 +1,3 @@ -QVImpactStream::batchAddAllocator -└── when called by PoolManager - └── it should call _addAllocator - -QVImpactStream::batchRemoveAllocator -└── when called by PoolManager - └── it should call _removeAllocator - - QVImpactStream::setPayouts ├── when payoutSet is true │ └── it should revert From 8e8956a2f86b8b81458eb163b546a371b3c00d89 Mon Sep 17 00:00:00 2001 From: 0xAustrian Date: Wed, 4 Sep 2024 18:46:04 -0300 Subject: [PATCH 19/20] fix: improvements and feedback --- .../examples/donation-voting/DonationVotingOffchain.sol | 1 + .../examples/donation-voting/DonationVotingOnchain.sol | 1 + .../strategies/examples/quadratic-voting/QVSimple.sol | 9 +++++++-- test/integration/QVImpactStream.t.sol | 3 ++- test/integration/QVSimple.t.sol | 3 ++- test/unit/strategies/QVImpactStream.t.sol | 3 ++- 6 files changed, 15 insertions(+), 5 deletions(-) diff --git a/contracts/strategies/examples/donation-voting/DonationVotingOffchain.sol b/contracts/strategies/examples/donation-voting/DonationVotingOffchain.sol index 9bcc5d2e3..4116a9d51 100644 --- a/contracts/strategies/examples/donation-voting/DonationVotingOffchain.sol +++ b/contracts/strategies/examples/donation-voting/DonationVotingOffchain.sol @@ -286,6 +286,7 @@ contract DonationVotingOffchain is BaseStrategy, RecipientsExtension, Allocation return _getRecipientStatus(_recipientId) == Status.Accepted; } + /// @notice Returns always true as all addresses are valid allocators function _isValidAllocator(address) internal view override returns (bool) { return true; } diff --git a/contracts/strategies/examples/donation-voting/DonationVotingOnchain.sol b/contracts/strategies/examples/donation-voting/DonationVotingOnchain.sol index 014b19ac4..00f8ca8f2 100644 --- a/contracts/strategies/examples/donation-voting/DonationVotingOnchain.sol +++ b/contracts/strategies/examples/donation-voting/DonationVotingOnchain.sol @@ -200,6 +200,7 @@ contract DonationVotingOnchain is BaseStrategy, RecipientsExtension, AllocationE return _getRecipientStatus(_recipientId) == Status.Accepted; } + /// @notice Returns always true as all addresses are valid allocators function _isValidAllocator(address) internal view override returns (bool) { return true; } diff --git a/contracts/strategies/examples/quadratic-voting/QVSimple.sol b/contracts/strategies/examples/quadratic-voting/QVSimple.sol index e24a52442..932a4f20b 100644 --- a/contracts/strategies/examples/quadratic-voting/QVSimple.sol +++ b/contracts/strategies/examples/quadratic-voting/QVSimple.sol @@ -72,10 +72,14 @@ contract QVSimple is BaseStrategy, RecipientsExtension, AllocatorsAllowlistExten ) = abi.decode(_data, (IRecipientsExtension.RecipientInitializeData, QVSimpleInitializeData)); __RecipientsExtension_init(recipientInitializeData); + __AllocationExtension_init( + new address[](0), + qvSimpleInitializeData.allocationStartTime, + qvSimpleInitializeData.allocationEndTime, + qvSimpleInitializeData.isUsingAllocationMetadata + ); maxVoiceCreditsPerAllocator = qvSimpleInitializeData.maxVoiceCreditsPerAllocator; - allocationStartTime = qvSimpleInitializeData.allocationStartTime; - allocationEndTime = qvSimpleInitializeData.allocationEndTime; emit Initialized(_poolId, _data); } @@ -89,6 +93,7 @@ contract QVSimple is BaseStrategy, RecipientsExtension, AllocatorsAllowlistExten uint64 allocationStartTime; uint64 allocationEndTime; uint256 maxVoiceCreditsPerAllocator; + bool isUsingAllocationMetadata; } /// @notice Distribute the tokens to the recipients diff --git a/test/integration/QVImpactStream.t.sol b/test/integration/QVImpactStream.t.sol index ad0a97f1b..42fde97b3 100644 --- a/test/integration/QVImpactStream.t.sol +++ b/test/integration/QVImpactStream.t.sol @@ -77,7 +77,8 @@ contract IntegrationQVImpactStream is Test { QVSimple.QVSimpleInitializeData({ allocationStartTime: uint64(block.timestamp), allocationEndTime: uint64(block.timestamp + 7 days), - maxVoiceCreditsPerAllocator: 100 + maxVoiceCreditsPerAllocator: 100, + isUsingAllocationMetadata: false }) ), dai, diff --git a/test/integration/QVSimple.t.sol b/test/integration/QVSimple.t.sol index 8ca6261cb..f0f884429 100644 --- a/test/integration/QVSimple.t.sol +++ b/test/integration/QVSimple.t.sol @@ -49,7 +49,8 @@ contract IntegrationQVSimple is IntegrationBase { QVSimple.QVSimpleInitializeData({ allocationStartTime: uint64(block.timestamp), allocationEndTime: uint64(block.timestamp + 7 days), - maxVoiceCreditsPerAllocator: 100 + maxVoiceCreditsPerAllocator: 100, + isUsingAllocationMetadata: false }) ), DAI, diff --git a/test/unit/strategies/QVImpactStream.t.sol b/test/unit/strategies/QVImpactStream.t.sol index 4fd926d96..36d2aee06 100644 --- a/test/unit/strategies/QVImpactStream.t.sol +++ b/test/unit/strategies/QVImpactStream.t.sol @@ -53,7 +53,8 @@ contract QVImpactStreamTest is Test { QVSimple.QVSimpleInitializeData memory qvInitData = QVSimple.QVSimpleInitializeData({ allocationStartTime: uint64(block.timestamp), allocationEndTime: uint64(block.timestamp + allocationWindow), - maxVoiceCreditsPerAllocator: 100 + maxVoiceCreditsPerAllocator: 100, + isUsingAllocationMetadata: false }); /// initialize vm.prank(mockAlloAddress); From ef18eaa588aac0c3a674d2e49accd1e02d257927 Mon Sep 17 00:00:00 2001 From: 0xAustrian Date: Mon, 9 Sep 2024 09:05:25 -0300 Subject: [PATCH 20/20] feat: remove unused error --- .../examples/donation-voting/DonationVotingOnchain.sol | 3 --- 1 file changed, 3 deletions(-) diff --git a/contracts/strategies/examples/donation-voting/DonationVotingOnchain.sol b/contracts/strategies/examples/donation-voting/DonationVotingOnchain.sol index 1ccc1c5bc..0a90437d9 100644 --- a/contracts/strategies/examples/donation-voting/DonationVotingOnchain.sol +++ b/contracts/strategies/examples/donation-voting/DonationVotingOnchain.sol @@ -42,9 +42,6 @@ contract DonationVotingOnchain is BaseStrategy, RecipientsExtension, AllocationE /// @param recipientId The recipientId to which distribution was attempted. error NOTHING_TO_DISTRIBUTE(address recipientId); - /// @notice Thrown when the timestamps being set or updated don't meet the contracts requirements. - error INVALID_TIMESTAMPS(); - /// @notice Thrown when the allocation token is not allowed. error TOKEN_NOT_ALLOWED();