From 46b1a4671064d0ae9c1b5b48de9447887316a8bf Mon Sep 17 00:00:00 2001 From: Valdorff Date: Sat, 23 Jul 2022 07:18:46 -0500 Subject: [PATCH 1/3] Allows deposit size to scale with minipool queue space in addition to deposit pool space - Also allows assignment to scale with value of deposit --- .../contract/deposit/RocketDepositPool.sol | 63 +++++++++++++++---- 1 file changed, 50 insertions(+), 13 deletions(-) diff --git a/contracts/contract/deposit/RocketDepositPool.sol b/contracts/contract/deposit/RocketDepositPool.sol index 014c0607c..22a57b165 100644 --- a/contracts/contract/deposit/RocketDepositPool.sol +++ b/contracts/contract/deposit/RocketDepositPool.sol @@ -76,7 +76,20 @@ contract RocketDepositPool is RocketBase, RocketDepositPoolInterface, RocketVaul require(rocketDAOProtocolSettingsDeposit.getDepositEnabled(), "Deposits into Rocket Pool are currently disabled"); require(msg.value >= rocketDAOProtocolSettingsDeposit.getMinimumDeposit(), "The deposited amount is less than the minimum deposit size"); RocketVaultInterface rocketVault = RocketVaultInterface(getContractAddress("rocketVault")); - require(rocketVault.balanceOf("rocketDepositPool").add(msg.value) <= rocketDAOProtocolSettingsDeposit.getMaximumDepositPoolSize(), "The deposit pool size after depositing exceeds the maximum size"); + uint256 capacityNeeded = rocketVault.balanceOf("rocketDepositPool").add(msg.value); + if (capacityNeeded > rocketDAOProtocolSettingsDeposit.getMaximumDepositPoolSize()) { + // Doing a conditional require() instead of a single one optimizes for the common + // case where capacityNeeded fits in the deposit pool without looking at the queue + if (rocketDAOProtocolSettingsDeposit.getAssignDepositsEnabled()) { + RocketMinipoolQueueInterface rocketMinipoolQueue = RocketMinipoolQueueInterface(getContractAddress("rocketMinipoolQueue")); + require(capacity <= rocketDAOProtocolSettingsDeposit.getMaximumDepositPoolSize() + rocketMinipoolQueue.getEffectiveCapacity(), + "The deposit pool size after depositing (and matching with minipools) exceeds the maximum size"); + } else { + require(capacity <= rocketDAOProtocolSettingsDeposit.getMaximumDepositPoolSize(), + "The deposit pool size after depositing exceeds the maximum size"); + } + } + // Record last deposit to time delay ability to withdraw setUint(keccak256(abi.encodePacked("user.deposit.block", msg.sender)), block.number); // Mint rETH to user account @@ -138,6 +151,7 @@ contract RocketDepositPool is RocketBase, RocketDepositPoolInterface, RocketVaul } // Assigns deposits to available minipools, returns false if assignment is currently disabled + // Can assign deposits up to the value of the deposit plus getMaximumDepositAssignments() function _assignDeposits(RocketVaultInterface rocketVault, RocketDAOProtocolSettingsDepositInterface rocketDAOProtocolSettingsDeposit) private returns (bool) { // Check if assigning deposits is enabled if (!rocketDAOProtocolSettingsDeposit.getAssignDepositsEnabled()) { @@ -150,28 +164,51 @@ contract RocketDepositPool is RocketBase, RocketDepositPoolInterface, RocketVaul uint256 balance = rocketVault.balanceOf("rocketDepositPool"); uint256 totalEther = 0; // Calculate minipool assignments - uint256 maxAssignments = rocketDAOProtocolSettingsDeposit.getMaximumDepositAssignments(); - MinipoolAssignment[] memory assignments = new MinipoolAssignment[](maxAssignments); - MinipoolDeposit depositType = MinipoolDeposit.None; + uint256 count = 0; uint256 minipoolCapacity = 0; - for (uint256 i = 0; i < maxAssignments; ++i) { - // Optimised for multiple of the same deposit type - if (count == 0) { - (depositType, count) = rocketMinipoolQueue.getNextDeposit(); - if (depositType == MinipoolDeposit.None) { break; } - minipoolCapacity = rocketDAOProtocolSettingsMinipool.getDepositUserAmount(depositType); + uint256 depositValueForAssignments = msg.value; + uint256 socializedAssignments = rocketDAOProtocolSettingsDeposit.getMaximumDepositAssignments(); + MinipoolAssignment[] memory assignments = new MinipoolAssignment[](maxAssignments); + + // Prepare half deposit assignments + count = rocketMinipoolQueue.getLength(MinipoolDeposit.Half); + minipoolCapacity = rocketDAOProtocolSettingsMinipool.getDepositUserAmount(MinipoolDeposit.Half); + for (uint256 i=0; i < count; ++i) { // (see note in full deposit loop) + if (depositValueForAssignments < minipoolCapacity) { + if (socializedAssignments == 0) { break; } + else {socializedAssignments--;} + } else { + depositValueForAssignments.sub(minipoolCapacity); } - count--; - if (minipoolCapacity == 0 || balance.sub(totalEther) < minipoolCapacity) { break; } + if (balance.sub(totalEther) < minipoolCapacity) { break; } // Dequeue the minipool - address minipoolAddress = rocketMinipoolQueue.dequeueMinipoolByDeposit(depositType); + address minipoolAddress = rocketMinipoolQueue.dequeueMinipoolByDeposit(MinipoolDeposit.Half); // Update running total totalEther = totalEther.add(minipoolCapacity); // Add assignment assignments[i].etherAssigned = minipoolCapacity; assignments[i].minipoolAddress = minipoolAddress; } + + // Prepare full deposit assignments + count = rocketMinipoolQueue.getLength(MinipoolDeposit.Full); + minipoolCapacity = rocketDAOProtocolSettingsMinipool.getDepositUserAmount(MinipoolDeposit.Full); + for (i; i < i + count; ++i) { // NOTE - this is a weird line - we continue the indexing from the half deposit loop + if (depositValueForAssignments < minipoolCapacity) { + if (socializedAssignments == 0) { break; } + else {socializedAssignments--;} + } + if (balance.sub(totalEther) < minipoolCapacity) { break; } + // Dequeue the minipool + address minipoolAddress = rocketMinipoolQueue.dequeueMinipoolByDeposit(MinipoolDeposit.Full); + // Update running total + totalEther = totalEther.add(minipoolCapacity); + // Add assignment + assignments[i].etherAssigned = minipoolCapacity; + assignments[i].minipoolAddress = minipoolAddress; + } + if (totalEther > 0) { // Withdraw ETH from vault rocketVault.withdrawEther(totalEther); From 3e3d594de672d73f4b6f9123ec0d5cc5e7d0bd38 Mon Sep 17 00:00:00 2001 From: Valdorff Date: Tue, 26 Jul 2022 19:20:59 -0500 Subject: [PATCH 2/3] Remove unused functions --- .../contract/minipool/RocketMinipoolQueue.sol | 36 ++----------------- .../minipool/RocketMinipoolQueueInterface.sol | 3 -- test/deposit/scenario-assign-deposits.js | 6 ++-- 3 files changed, 4 insertions(+), 41 deletions(-) diff --git a/contracts/contract/minipool/RocketMinipoolQueue.sol b/contracts/contract/minipool/RocketMinipoolQueue.sol index 9afcd055b..a558bd86c 100644 --- a/contracts/contract/minipool/RocketMinipoolQueue.sol +++ b/contracts/contract/minipool/RocketMinipoolQueue.sol @@ -57,18 +57,6 @@ contract RocketMinipoolQueue is RocketBase, RocketMinipoolQueueInterface { return addressQueueStorage.getLength(_key); } - // Get the total combined capacity of the queues - function getTotalCapacity() override external view returns (uint256) { - RocketDAOProtocolSettingsMinipoolInterface rocketDAOProtocolSettingsMinipool = RocketDAOProtocolSettingsMinipoolInterface(getContractAddress("rocketDAOProtocolSettingsMinipool")); - return ( - getLength(queueKeyFull).mul(rocketDAOProtocolSettingsMinipool.getFullDepositUserAmount()) - ).add( - getLength(queueKeyHalf).mul(rocketDAOProtocolSettingsMinipool.getHalfDepositUserAmount()) - ).add( - getLength(queueKeyEmpty).mul(rocketDAOProtocolSettingsMinipool.getEmptyDepositUserAmount()) - ); - } - // Get the total effective capacity of the queues (used in node demand calculation) function getEffectiveCapacity() override external view returns (uint256) { RocketDAOProtocolSettingsMinipoolInterface rocketDAOProtocolSettingsMinipool = RocketDAOProtocolSettingsMinipoolInterface(getContractAddress("rocketDAOProtocolSettingsMinipool")); @@ -79,28 +67,6 @@ contract RocketMinipoolQueue is RocketBase, RocketMinipoolQueueInterface { ); } - // Get the capacity of the next available minipool - // Returns 0 if no minipools are available - function getNextCapacity() override external view returns (uint256) { - RocketDAOProtocolSettingsMinipoolInterface rocketDAOProtocolSettingsMinipool = RocketDAOProtocolSettingsMinipoolInterface(getContractAddress("rocketDAOProtocolSettingsMinipool")); - if (getLength(queueKeyHalf) > 0) { return rocketDAOProtocolSettingsMinipool.getHalfDepositUserAmount(); } - if (getLength(queueKeyFull) > 0) { return rocketDAOProtocolSettingsMinipool.getFullDepositUserAmount(); } - if (getLength(queueKeyEmpty) > 0) { return rocketDAOProtocolSettingsMinipool.getEmptyDepositUserAmount(); } - return 0; - } - - // Get the deposit type of the next available minipool and the number of deposits in that queue - // Returns None if no minipools are available - function getNextDeposit() override external view returns (MinipoolDeposit, uint256) { - uint256 length = getLength(queueKeyHalf); - if (length > 0) { return (MinipoolDeposit.Half, length); } - length = getLength(queueKeyFull); - if (length > 0) { return (MinipoolDeposit.Full, length); } - length = getLength(queueKeyEmpty); - if (length > 0) { return (MinipoolDeposit.Empty, length); } - return (MinipoolDeposit.None, 0); - } - // Add a minipool to the end of the appropriate queue // Only accepts calls from the RocketMinipoolManager contract function enqueueMinipool(MinipoolDeposit _depositType, address _minipool) override external onlyLatestContract("rocketMinipoolQueue", address(this)) onlyLatestContract("rocketMinipoolManager", msg.sender) { @@ -143,6 +109,8 @@ contract RocketMinipoolQueue is RocketBase, RocketMinipoolQueueInterface { // Remove a minipool from a queue // Only accepts calls from registered minipools + // Note: this removal is made computationally efficient by swapping with the last item in the + // queue. This is acceptable because removing minipools should be rare. function removeMinipool(MinipoolDeposit _depositType) override external onlyLatestContract("rocketMinipoolQueue", address(this)) onlyRegisteredMinipool(msg.sender) { // Remove minipool from queue if (_depositType == MinipoolDeposit.Half) { return removeMinipool(queueKeyHalf, msg.sender); } diff --git a/contracts/interface/minipool/RocketMinipoolQueueInterface.sol b/contracts/interface/minipool/RocketMinipoolQueueInterface.sol index bbe9c272b..4ebb2e3ad 100644 --- a/contracts/interface/minipool/RocketMinipoolQueueInterface.sol +++ b/contracts/interface/minipool/RocketMinipoolQueueInterface.sol @@ -7,10 +7,7 @@ import "../../types/MinipoolDeposit.sol"; interface RocketMinipoolQueueInterface { function getTotalLength() external view returns (uint256); function getLength(MinipoolDeposit _depositType) external view returns (uint256); - function getTotalCapacity() external view returns (uint256); function getEffectiveCapacity() external view returns (uint256); - function getNextCapacity() external view returns (uint256); - function getNextDeposit() external view returns (MinipoolDeposit, uint256); function enqueueMinipool(MinipoolDeposit _depositType, address _minipool) external; function dequeueMinipool() external returns (address minipoolAddress); function dequeueMinipoolByDeposit(MinipoolDeposit _depositType) external returns (address minipoolAddress); diff --git a/test/deposit/scenario-assign-deposits.js b/test/deposit/scenario-assign-deposits.js index b518c88c3..17bc1e100 100644 --- a/test/deposit/scenario-assign-deposits.js +++ b/test/deposit/scenario-assign-deposits.js @@ -66,10 +66,9 @@ export async function assignDeposits(txOptions) { function getMinipoolQueueDetails() { return Promise.all([ rocketMinipoolQueue.getTotalLength.call(), - rocketMinipoolQueue.getTotalCapacity.call(), ]).then( - ([totalLength, totalCapacity]) => - ({totalLength, totalCapacity}) + ([totalLength]) => + ({totalLength}) ); } @@ -94,7 +93,6 @@ export async function assignDeposits(txOptions) { // Check minipool queues assert(queue2.totalLength.eq(queue1.totalLength.sub(web3.utils.toBN(expectedDepositAssignments))), 'Incorrect updated minipool queue length'); - assert(queue2.totalCapacity.eq(queue1.totalCapacity.sub(expectedEthAssigned)), 'Incorrect updated minipool queue capacity'); } From 27ee57afa364024fa834a503bca467ca648ae86c Mon Sep 17 00:00:00 2001 From: Valdorff Date: Sat, 13 Aug 2022 13:13:22 -0500 Subject: [PATCH 3/3] Address comments from Kane - 2 comments in PR - Need for a hard max on assignments to ensure there's enough gas in a block - Unrelated; did away with the weird counter-spanning-for-loops trick to be more explicit --- .../RocketDAOProtocolSettingsDeposit.sol | 7 ++- .../contract/deposit/RocketDepositPool.sol | 43 ++++++++++--------- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/contracts/contract/dao/protocol/settings/RocketDAOProtocolSettingsDeposit.sol b/contracts/contract/dao/protocol/settings/RocketDAOProtocolSettingsDeposit.sol index dd86537b6..c19146265 100644 --- a/contracts/contract/dao/protocol/settings/RocketDAOProtocolSettingsDeposit.sol +++ b/contracts/contract/dao/protocol/settings/RocketDAOProtocolSettingsDeposit.sol @@ -20,7 +20,8 @@ contract RocketDAOProtocolSettingsDeposit is RocketDAOProtocolSettings, RocketDA setSettingBool("deposit.assign.enabled", true); setSettingUint("deposit.minimum", 0.01 ether); setSettingUint("deposit.pool.maximum", 160 ether); - setSettingUint("deposit.assign.maximum", 2); + setSettingUint("deposit.assign.maximum", 90); + setSettingUint("deposit.assign.socializedmaximum", 2); // Settings initialised setBool(keccak256(abi.encodePacked(settingNameSpace, "deployed")), true); } @@ -51,4 +52,8 @@ contract RocketDAOProtocolSettingsDeposit is RocketDAOProtocolSettings, RocketDA return getSettingUint("deposit.assign.maximum"); } + // The maximum number of socialized (ie, not related to deposit size) assignments to perform + function getMaximumDepositSocializedAssignments() override external view returns (uint256) { + return getSettingUint("deposit.assign.socializedmaximum"); + } } diff --git a/contracts/contract/deposit/RocketDepositPool.sol b/contracts/contract/deposit/RocketDepositPool.sol index 22a57b165..1583a0341 100644 --- a/contracts/contract/deposit/RocketDepositPool.sol +++ b/contracts/contract/deposit/RocketDepositPool.sol @@ -82,11 +82,10 @@ contract RocketDepositPool is RocketBase, RocketDepositPoolInterface, RocketVaul // case where capacityNeeded fits in the deposit pool without looking at the queue if (rocketDAOProtocolSettingsDeposit.getAssignDepositsEnabled()) { RocketMinipoolQueueInterface rocketMinipoolQueue = RocketMinipoolQueueInterface(getContractAddress("rocketMinipoolQueue")); - require(capacity <= rocketDAOProtocolSettingsDeposit.getMaximumDepositPoolSize() + rocketMinipoolQueue.getEffectiveCapacity(), + require(capacityNeeded <= rocketDAOProtocolSettingsDeposit.getMaximumDepositPoolSize() + rocketMinipoolQueue.getEffectiveCapacity(), "The deposit pool size after depositing (and matching with minipools) exceeds the maximum size"); } else { - require(capacity <= rocketDAOProtocolSettingsDeposit.getMaximumDepositPoolSize(), - "The deposit pool size after depositing exceeds the maximum size"); + revert("The deposit pool size after depositing exceeds the maximum size"); } } @@ -163,21 +162,23 @@ contract RocketDepositPool is RocketBase, RocketDepositPoolInterface, RocketVaul // Setup initial variable values uint256 balance = rocketVault.balanceOf("rocketDepositPool"); uint256 totalEther = 0; - // Calculate minipool assignments - uint256 count = 0; + // Calculate minipool assignments + uint256 i; + uint256 assignmentIndex = 0; uint256 minipoolCapacity = 0; uint256 depositValueForAssignments = msg.value; - uint256 socializedAssignments = rocketDAOProtocolSettingsDeposit.getMaximumDepositAssignments(); + uint256 socializedAssignmentsLeft = rocketDAOProtocolSettingsDeposit.getMaximumDepositSocializedAssignments(); + uint256 maxAssignments = rocketDAOProtocolSettingsDeposit.getMaximumDepositAssignments(); MinipoolAssignment[] memory assignments = new MinipoolAssignment[](maxAssignments); // Prepare half deposit assignments - count = rocketMinipoolQueue.getLength(MinipoolDeposit.Half); minipoolCapacity = rocketDAOProtocolSettingsMinipool.getDepositUserAmount(MinipoolDeposit.Half); - for (uint256 i=0; i < count; ++i) { // (see note in full deposit loop) + for (i=0; i < rocketMinipoolQueue.getLength(MinipoolDeposit.Half); ++i) { + if (assignmentIndex == maxAssignments) { break; } if (depositValueForAssignments < minipoolCapacity) { - if (socializedAssignments == 0) { break; } - else {socializedAssignments--;} + if (socializedAssignmentsLeft == 0) { break; } + else {socializedAssignmentsLeft--;} } else { depositValueForAssignments.sub(minipoolCapacity); } @@ -186,27 +187,29 @@ contract RocketDepositPool is RocketBase, RocketDepositPoolInterface, RocketVaul address minipoolAddress = rocketMinipoolQueue.dequeueMinipoolByDeposit(MinipoolDeposit.Half); // Update running total totalEther = totalEther.add(minipoolCapacity); - // Add assignment - assignments[i].etherAssigned = minipoolCapacity; - assignments[i].minipoolAddress = minipoolAddress; + // Add assignment, increment index + assignments[assignmentIndex].etherAssigned = minipoolCapacity; + assignments[assignmentIndex].minipoolAddress = minipoolAddress; + assignmentIndex++; } // Prepare full deposit assignments - count = rocketMinipoolQueue.getLength(MinipoolDeposit.Full); minipoolCapacity = rocketDAOProtocolSettingsMinipool.getDepositUserAmount(MinipoolDeposit.Full); - for (i; i < i + count; ++i) { // NOTE - this is a weird line - we continue the indexing from the half deposit loop + for (i=0; i < rocketMinipoolQueue.getLength(MinipoolDeposit.Full); ++i) { + if (assignmentIndex == maxAssignments) { break; } if (depositValueForAssignments < minipoolCapacity) { - if (socializedAssignments == 0) { break; } - else {socializedAssignments--;} + if (socializedAssignmentsLeft == 0) { break; } + else {socializedAssignmentsLeft--;} } if (balance.sub(totalEther) < minipoolCapacity) { break; } // Dequeue the minipool address minipoolAddress = rocketMinipoolQueue.dequeueMinipoolByDeposit(MinipoolDeposit.Full); // Update running total totalEther = totalEther.add(minipoolCapacity); - // Add assignment - assignments[i].etherAssigned = minipoolCapacity; - assignments[i].minipoolAddress = minipoolAddress; + // Add assignment, increment index + assignments[assignmentIndex].etherAssigned = minipoolCapacity; + assignments[assignmentIndex].minipoolAddress = minipoolAddress; + assignmentIndex++; } if (totalEther > 0) {