diff --git a/contracts/contract/util/AddressLinkedQueueStorage.sol b/contracts/contract/util/LinkedListStorage.sol similarity index 69% rename from contracts/contract/util/AddressLinkedQueueStorage.sol rename to contracts/contract/util/LinkedListStorage.sol index ae229281..1e16026a 100644 --- a/contracts/contract/util/AddressLinkedQueueStorage.sol +++ b/contracts/contract/util/LinkedListStorage.sol @@ -4,17 +4,17 @@ pragma abicoder v2; // SPDX-License-Identifier: GPL-3.0-only import "../RocketBase.sol"; -import "../../interface/util/AddressLinkedQueueStorageInterface.sol"; +import "../../interface/util/LinkedListStorageInterface.sol"; -/// @notice Address linked queue storage helper for RocketStorage data (linked list implementation) -contract AddressLinkedQueueStorage is RocketBase, AddressLinkedQueueStorageInterface { +/// @notice A linked list storage helper for the deposit requests queue data +contract LinkedListStorage is RocketBase, LinkedListStorageInterface { // Constants for packing queue metadata into a single uint256 uint256 constant internal startOffset = 256 - 64; uint256 constant internal endOffset = 256 - 128; uint256 constant internal lengthOffset = 256 - 192; - // Constants for packing a deposit queue struct into a single uint256 + // Constants for packing a deposit item (struct) into a single uint256 uint256 constant internal receiverOffset = 256 - 160; uint256 constant internal indexOffset = 256 - 160 - 32; uint256 constant internal suppliedOffset = 256 - 160 - 32 - 32; @@ -29,27 +29,22 @@ contract AddressLinkedQueueStorage is RocketBase, AddressLinkedQueueStorageInter /// @notice The number of items in the queue /// @param _namespace defines the queue to be used function getLength(bytes32 _namespace) override public view returns (uint256) { - return uint256(uint64(getUint(keccak256(abi.encodePacked(_namespace, ".data"))) >> lengthOffset)); + return uint64(getUint(keccak256(abi.encodePacked(_namespace, ".data"))) >> lengthOffset); } /// @notice The item in a queue by index /// @param _namespace defines the queue to be used /// @param _index the item index function getItem(bytes32 _namespace, uint256 _index) override external view returns (DepositQueueValue memory) { - uint256 index = getUint(keccak256(abi.encodePacked(_namespace, ".data"))) >> startOffset + _index; - uint256 packedValue = getUint(keccak256(abi.encodePacked(_namespace, ".item", index))); - return unpackDepositQueueValue(packedValue); + uint256 packedValue = getUint(keccak256(abi.encodePacked(_namespace, ".item", _index))); + return _unpackItem(packedValue); } - /// @notice The index of an item in a queue. Returns -1 if the value is not found + /// @notice The index of an item in a queue. Returns 0 if the value is not found /// @param _namespace defines the queue to be used /// @param _value the deposit queue value - function getIndexOf(bytes32 _namespace, DepositQueueValue memory _value) override external view returns (int) { - uint256 index = getUint(keccak256(abi.encodePacked(_namespace, ".index", _value.receiver, _value.validatorId))); - if (index > 0) { - return int256(index); - } - return -1; + function getIndexOf(bytes32 _namespace, DepositQueueValue memory _value) override external view returns (uint256) { + return getUint(keccak256(abi.encodePacked(_namespace, ".index", _value.receiver, _value.validatorId))); } /// @notice Finds an item index in a queue and returns the previous item @@ -59,7 +54,7 @@ contract AddressLinkedQueueStorage is RocketBase, AddressLinkedQueueStorageInter uint256 index = getUint(keccak256(abi.encodePacked(_namespace, ".index", _value.receiver, _value.validatorId))); if (index > 0) { uint256 previousIndex = getUint(keccak256(abi.encodePacked(_namespace, ".prev", index))); - previousItem = unpackDepositQueueValue(getUint(keccak256(abi.encodePacked(_namespace, ".item", previousIndex)))); + previousItem = _unpackItem(getUint(keccak256(abi.encodePacked(_namespace, ".item", previousIndex)))); } } @@ -70,16 +65,22 @@ contract AddressLinkedQueueStorage is RocketBase, AddressLinkedQueueStorageInter uint256 index = getUint(keccak256(abi.encodePacked(_namespace, ".index", _value.receiver, _value.validatorId))); if (index > 0) { uint256 nextIndex = getUint(keccak256(abi.encodePacked(_namespace, ".next", index))); - nextItem = unpackDepositQueueValue(getUint(keccak256(abi.encodePacked(_namespace, ".item", nextIndex)))); + nextItem = _unpackItem(getUint(keccak256(abi.encodePacked(_namespace, ".item", nextIndex)))); } } /// @notice Add an item to the end of the list. Requires that the item does not exist in the list /// @param _namespace defines the queue to be used - /// @param _value the deposit queue value - function enqueueItem(bytes32 _namespace, DepositQueueValue memory _value) virtual override external { - // onlyLatestContract("addressLinkedListStorage", address(this)) onlyLatestNetworkContract { - require(getUint(keccak256(abi.encodePacked(_namespace, ".index", _value.receiver, _value.validatorId))) == 0, "Item already exists in queue"); + /// @param _item the deposit queue item to be added + function enqueueItem(bytes32 _namespace, DepositQueueValue memory _item) virtual override external onlyLatestContract("addressLinkedListStorage", address(this)) onlyLatestNetworkContract { + _enqueueItem(_namespace, _item); + } + + /// @notice Internal function created to allow testing enqueueItem + /// @param _namespace defines the queue to be used + /// @param _item the deposit queue value + function _enqueueItem(bytes32 _namespace, DepositQueueValue memory _item) internal { + require(getUint(keccak256(abi.encodePacked(_namespace, ".index", _item.receiver, _item.validatorId))) == 0, "Item already exists in queue"); uint256 data = getUint(keccak256(abi.encodePacked(_namespace, ".data"))); uint256 endIndex = uint64(data >> endOffset); uint256 newIndex = endIndex + 1; @@ -93,8 +94,8 @@ contract AddressLinkedQueueStorage is RocketBase, AddressLinkedQueueStorageInter data |= newIndex << startOffset; } - setUint(keccak256(abi.encodePacked(_namespace, ".item", newIndex)), packDepositQueueValue(_value)); - setUint(keccak256(abi.encodePacked(_namespace, ".index", _value.receiver, _value.validatorId)), newIndex); + setUint(keccak256(abi.encodePacked(_namespace, ".item", newIndex)), _packItem(_item)); + setUint(keccak256(abi.encodePacked(_namespace, ".index", _item.receiver, _item.validatorId)), newIndex); // clear the 64 bits used to stored the 'end' pointer data &= ~(uint256(ones64Bits) << endOffset); data |= newIndex << endOffset; @@ -110,11 +111,18 @@ contract AddressLinkedQueueStorage is RocketBase, AddressLinkedQueueStorageInter /// @notice Remove an item from the start of a queue and return it. Requires that the queue is not empty /// @param _namespace defines the queue to be used function dequeueItem(bytes32 _namespace) public virtual override onlyLatestContract("addressLinkedListStorage", address(this)) onlyLatestNetworkContract returns (DepositQueueValue memory item) { - require(getLength(_namespace) > 0, "Queue is empty"); + return _dequeueItem(_namespace); + } + + /// @notice Remove an item from the start of a queue and return it. Requires that the queue is not empty + /// @param _namespace defines the queue to be used + function _dequeueItem(bytes32 _namespace) internal returns (DepositQueueValue memory item) { uint256 data = getUint(keccak256(abi.encodePacked(_namespace, ".data"))); + uint256 length = uint64(data >> lengthOffset); + require(length > 0, "Queue can't be empty"); uint256 start = uint64(data >> startOffset); uint256 packedItem = getUint(keccak256(abi.encodePacked(_namespace, ".item", start))); - item = unpackDepositQueueValue(packedItem); + item = _unpackItem(packedItem); uint256 nextItem = getUint(keccak256(abi.encodePacked(_namespace, ".next", start))); // clear the 64 bits used to stored the 'start' pointer @@ -130,10 +138,9 @@ contract AddressLinkedQueueStorage is RocketBase, AddressLinkedQueueStorageInter } // Update the length of the queue - uint256 currentLength = uint64(data >> lengthOffset); // clear the 64 bits used to stored the 'length' information data &= ~(uint256(ones64Bits) << lengthOffset); - data |= (currentLength -1) << lengthOffset; + data |= (length - 1) << lengthOffset; setUint(keccak256(abi.encodePacked(_namespace, ".data")), data); return item; @@ -141,8 +148,16 @@ contract AddressLinkedQueueStorage is RocketBase, AddressLinkedQueueStorageInter /// @notice Removes an item from a queue. Requires that the item exists in the queue /// @param _namespace defines the queue to be used - function removeItem(bytes32 _namespace, DepositQueueValue memory _value) public virtual override onlyLatestContract("addressLinkedListStorage", address(this)) onlyLatestNetworkContract { - uint256 index = getUint(keccak256(abi.encodePacked(_namespace, ".index", _value.receiver, _value.validatorId))); + /// @param _item to be removed from the queue + function removeItem(bytes32 _namespace, DepositQueueValue memory _item) public virtual override onlyLatestContract("addressLinkedListStorage", address(this)) onlyLatestNetworkContract { + _removeItem(_namespace, _item); + } + + /// @notice Internal funciton to remove an item from a queue. Requires that the item exists in the queue + /// @param _namespace defines the queue to be used + /// @param _item to be removed from the queue + function _removeItem(bytes32 _namespace, DepositQueueValue memory _item) internal { + uint256 index = getUint(keccak256(abi.encodePacked(_namespace, ".index", _item.receiver, _item.validatorId))); uint256 data = getUint(keccak256(abi.encodePacked(_namespace, ".data"))); require(index > 0, "Item does not exist in queue"); @@ -169,7 +184,7 @@ contract AddressLinkedQueueStorage is RocketBase, AddressLinkedQueueStorageInter data |= prevIndex << endOffset; } - setUint(keccak256(abi.encodePacked(_namespace, ".index", _value.receiver, _value.validatorId)), 0); + setUint(keccak256(abi.encodePacked(_namespace, ".index", _item.receiver, _item.validatorId)), 0); setUint(keccak256(abi.encodePacked(_namespace, ".next", index)), 0); setUint(keccak256(abi.encodePacked(_namespace, ".prev", index)), 0); @@ -182,21 +197,21 @@ contract AddressLinkedQueueStorage is RocketBase, AddressLinkedQueueStorageInter } /// @notice packs a deposit queue value into a single uint256 - /// @param _struct the deposit queue value to be packed - function packDepositQueueValue(DepositQueueValue memory _struct) internal pure returns (uint256 packed) { - packed |= uint256(uint160(_struct.receiver)) << receiverOffset; - packed |= uint256(_struct.validatorId) << indexOffset; - packed |= uint256(_struct.suppliedValue) << suppliedOffset; - packed |= uint256(_struct.requestedValue); + /// @param _item the deposit queue item to be packed + function _packItem(DepositQueueValue memory _item) internal pure returns (uint256 packed) { + packed |= uint256(uint160(_item.receiver)) << receiverOffset; + packed |= uint256(_item.validatorId) << indexOffset; + packed |= uint256(_item.suppliedValue) << suppliedOffset; + packed |= uint256(_item.requestedValue); } /// @notice unpacks an uint256 value into a deposit queue struct /// @param _packedValue the packed deposit queue value - function unpackDepositQueueValue(uint256 _packedValue) internal pure returns (DepositQueueValue memory value) { - value.receiver = address(uint160(_packedValue >> receiverOffset)); - value.validatorId = uint32(_packedValue >> indexOffset); - value.suppliedValue = uint32(_packedValue >> suppliedOffset); - value.requestedValue = uint32(_packedValue); + function _unpackItem(uint256 _packedValue) internal pure returns (DepositQueueValue memory item) { + item.receiver = address(uint160(_packedValue >> receiverOffset)); + item.validatorId = uint32(_packedValue >> indexOffset); + item.suppliedValue = uint32(_packedValue >> suppliedOffset); + item.requestedValue = uint32(_packedValue); } } \ No newline at end of file diff --git a/contracts/contract/util/LinkedListStorageHelper.sol b/contracts/contract/util/LinkedListStorageHelper.sol new file mode 100644 index 00000000..200677b5 --- /dev/null +++ b/contracts/contract/util/LinkedListStorageHelper.sol @@ -0,0 +1,43 @@ +pragma solidity 0.8.18; +pragma abicoder v2; + +// SPDX-License-Identifier: GPL-3.0-only + +import "./LinkedListStorage.sol"; + +/// @notice A linked list storage helper to test internal functions +contract LinkedListStorageHelper is LinkedListStorage { + + // Construct + constructor(RocketStorageInterface _rocketStorageAddress) LinkedListStorage(_rocketStorageAddress) { + version = 1; + } + + /// @notice Add an item to the end of the list. Requires that the item does not exist in the list + /// @param _namespace defines the queue to be used + /// @param _item the deposit queue item + function enqueueItem(bytes32 _namespace, DepositQueueValue memory _item) public override { + _enqueueItem(_namespace, _item); + } + + /// @notice Remove an item from the start of a queue and return it. Requires that the queue is not empty + /// @param _namespace defines the queue to be used + function dequeueItem(bytes32 _namespace) public virtual override returns (DepositQueueValue memory item) { + return _dequeueItem(_namespace); + } + + /// @notice Removes an item from a queue. Requires that the item exists in the queue + /// @param _namespace to be used + /// @param _item to be removed from the queue + function removeItem(bytes32 _namespace, DepositQueueValue memory _item) public virtual override { + return _removeItem(_namespace, _item); + } + + function packItem(DepositQueueValue memory _item) public pure returns (uint256 packed) { + return _packItem(_item); + } + + function unpackItem(uint256 _item) public pure returns (DepositQueueValue memory item) { + return _unpackItem(_item); + } +} \ No newline at end of file diff --git a/contracts/interface/util/AddressLinkedQueueStorageInterface.sol b/contracts/interface/util/LinkedListStorageInterface.sol similarity index 81% rename from contracts/interface/util/AddressLinkedQueueStorageInterface.sol rename to contracts/interface/util/LinkedListStorageInterface.sol index 62cd9b5b..5e0605de 100644 --- a/contracts/interface/util/AddressLinkedQueueStorageInterface.sol +++ b/contracts/interface/util/LinkedListStorageInterface.sol @@ -4,16 +4,16 @@ pragma abicoder v2; // SPDX-License-Identifier: GPL-3.0-only struct DepositQueueValue { - address receiver; // the megapool address + address receiver; // the address that will receive the requested value uint32 validatorId; // internal validator id uint32 suppliedValue; // in milliether uint32 requestedValue; // in milliether } -interface AddressLinkedQueueStorageInterface { - function getLength(bytes32 _namespace) external view returns (uint); +interface LinkedListStorageInterface { + function getLength(bytes32 _namespace) external view returns (uint256); function getItem(bytes32 _namespace, uint _index) external view returns (DepositQueueValue memory); - function getIndexOf(bytes32 _namespace, DepositQueueValue memory _value) external view returns (int); + function getIndexOf(bytes32 _namespace, DepositQueueValue memory _value) external view returns (uint256); function enqueueItem(bytes32 _namespace, DepositQueueValue memory _value) external; function dequeueItem(bytes32 _namespace) external returns (DepositQueueValue memory); function removeItem(bytes32 _namespace, DepositQueueValue memory _value) external; diff --git a/test/_helpers/deployment.js b/test/_helpers/deployment.js index a962eda7..e7da0b5f 100644 --- a/test/_helpers/deployment.js +++ b/test/_helpers/deployment.js @@ -1,6 +1,5 @@ /*** Dependencies ********************/ -import { RocketDAOProtocolSettingsNode, RocketStorage } from '../_utils/artifacts'; -import { setDAOProtocolBootstrapSetting } from '../dao/scenario-dao-protocol-bootstrap'; +import { LinkedListStorage, RocketStorage } from '../_utils/artifacts'; const hre = require('hardhat'); const pako = require('pako'); @@ -101,6 +100,7 @@ const contracts = { rocketDAOProtocolProposal: artifacts.require('RocketDAOProtocolProposal.sol'), // Utils addressQueueStorage: artifacts.require('AddressQueueStorage.sol'), + LinkedListStorage: artifacts.require('LinkedListStorageHelper.sol'), addressSetStorage: artifacts.require('AddressSetStorage.sol'), }; diff --git a/test/_utils/artifacts.js b/test/_utils/artifacts.js index 7a9e1828..c97e1ade 100644 --- a/test/_utils/artifacts.js +++ b/test/_utils/artifacts.js @@ -1,3 +1,5 @@ +import { artifacts } from "hardhat"; + export const RocketAuctionManager = artifacts.require('RocketAuctionManager.sol'); export const RocketClaimDAO = artifacts.require('RocketClaimDAO.sol'); export const RocketDAONodeTrusted = artifacts.require('RocketDAONodeTrusted.sol'); @@ -50,6 +52,7 @@ export const RocketMinipoolQueue = artifacts.require('RocketMinipoolQueue.sol'); export const RocketNodeDeposit = artifacts.require('RocketNodeDeposit.sol'); export const RocketMinipoolDelegate = artifacts.require('RocketMinipoolDelegate.sol'); export const RocketDAOProtocolSettingsMinipool = artifacts.require('RocketDAOProtocolSettingsMinipool.sol'); +export const LinkedListStorage = artifacts.require('LinkedListStorageHelper.sol'); export const RocketDepositPool = artifacts.require('RocketDepositPool.sol'); export const RocketMinipoolBondReducer = artifacts.require('RocketMinipoolBondReducer.sol'); export const RocketNetworkSnapshots = artifacts.require('RocketNetworkSnapshots.sol'); diff --git a/test/deposit/deposit-pool-tests.js b/test/deposit/deposit-pool-tests.js index 59bdf81a..e995e929 100644 --- a/test/deposit/deposit-pool-tests.js +++ b/test/deposit/deposit-pool-tests.js @@ -9,7 +9,7 @@ import { getDepositSetting } from '../_helpers/settings'; import { deposit } from './scenario-deposit'; import { RocketDAONodeTrustedSettingsMembers, - RocketDAOProtocolSettingsDeposit, RocketDepositPool, + RocketDAOProtocolSettingsDeposit, RocketDepositPool, AddressLinkedQueueStorage } from '../_utils/artifacts'; import { setDAOProtocolBootstrapSetting } from '../dao/scenario-dao-protocol-bootstrap'; import { setDAONodeTrustedBootstrapSetting } from '../dao/scenario-dao-node-trusted-bootstrap' diff --git a/test/rocket-pool-tests.js b/test/rocket-pool-tests.js index d374a104..adab5d59 100644 --- a/test/rocket-pool-tests.js +++ b/test/rocket-pool-tests.js @@ -30,6 +30,7 @@ import { injectBNHelpers } from './_helpers/bn'; import { checkInvariants } from './_helpers/invariants'; import networkSnapshotsTests from './network/network-snapshots-tests'; import networkVotingTests from './network/network-voting-tests'; +import utilTests from './util/util-tests'; // Header console.log('\n'); @@ -85,3 +86,4 @@ nodeDistributorTests(); rethTests(); rplTests(); rewardsPoolTests(); +utilTests(); diff --git a/test/util/util-tests.js b/test/util/util-tests.js new file mode 100644 index 00000000..ebd424ab --- /dev/null +++ b/test/util/util-tests.js @@ -0,0 +1,136 @@ +import { printTitle } from '../_utils/formatting'; +import { shouldRevert } from '../_utils/testing'; +import { + LinkedListStorage +} from '../_utils/artifacts'; +import { assert } from 'hardhat'; + +export default function() { + contract('LinkedListStorage', async (accounts) => { + + + // Accounts + const [ + random, + ] = accounts; + + const regularQueue = web3.utils.soliditySha3('regular') + const expressQueue = web3.utils.soliditySha3('express') + + // Setup + before(async () => { + + }); + + it.only(printTitle('random', 'pack/unpack shouldnt change values'), async () => { + const linkedListStorage = await LinkedListStorage.deployed(); + let item = { + receiver: random, + validatorId: 1, + suppliedValue: 8000, + requestedValue: 32000, + } + let packedItem = await linkedListStorage.packItem(item) + let unpackedItem = await linkedListStorage.unpackItem(packedItem) + assert.equal(item.receiver, unpackedItem.receiver) + assert.equal(item.validatorId, unpackedItem.validatorId) + assert.equal(item.suppliedValue, unpackedItem.suppliedValue) + assert.equal(item.requestedValue, unpackedItem.requestedValue) + }); + + it(printTitle('random', 'can enqueue/dequeue items'), async () => { + const linkedListStorage = await LinkedListStorage.deployed(); + let itemIn = { + receiver: random, + validatorId: 1, + suppliedValue: 8000, + requestedValue: 32000, + } + // enqueue 3 items, check for the correct indexOf and length + await linkedListStorage.enqueueItem(regularQueue, itemIn); + let indexOfFirst = await linkedListStorage.getIndexOf(regularQueue, itemIn) + assert.equal(indexOfFirst, 1) + let listLength = await linkedListStorage.getLength(regularQueue); + assert.equal(listLength, 1) + + itemIn.validatorId = 2 + await linkedListStorage.enqueueItem(regularQueue, itemIn) + listLength = await linkedListStorage.getLength(regularQueue); + assert.equal(listLength, 2) + + itemIn.validatorId = 3 + await linkedListStorage.enqueueItem(regularQueue, itemIn) + listLength = await linkedListStorage.getLength(regularQueue); + assert.equal(listLength, 3) + + itemIn.validatorId = 2 + // remove the second item + await linkedListStorage.removeItem(regularQueue, itemIn) + + let first = await linkedListStorage.getItem.call(regularQueue, 1); + assert.equal(first.validatorId, 1) + let last = await linkedListStorage.getItem.call(regularQueue, 3); + assert.equal(last.validatorId, 3) + await linkedListStorage.dequeueItem(regularQueue) + listLength = await linkedListStorage.getLength.call(regularQueue); + assert.equal(listLength, 1) + await linkedListStorage.dequeueItem(regularQueue) + listLength = await linkedListStorage.getLength.call(regularQueue); + assert.equal(listLength, 0) + }); + + it.only(printTitle('random', 'can remove the only queue item'), async () => { + const linkedListStorage = await LinkedListStorage.deployed(); + let itemIn = { + receiver: random, + validatorId: 1, + suppliedValue: 8000, + requestedValue: 32000, + } + await linkedListStorage.enqueueItem(regularQueue, itemIn) + await linkedListStorage.dequeueItem(regularQueue) + let listLength = await linkedListStorage.getLength(regularQueue); + assert.equal(listLength, 0) + }); + + it(printTitle('random', 'cannot add the same item twice'), async () => { + const linkedListStorage = await LinkedListStorage.deployed(); + let itemIn = { + receiver: random, + validatorId: 1, + suppliedValue: 8000, + requestedValue: 32000, + } + await linkedListStorage.enqueueItem(regularQueue, itemIn) + let listLength = await linkedListStorage.getLength(regularQueue); + assert.equal(listLength, 1) + await shouldRevert(linkedListStorage.enqueueItem(regularQueue, itemIn)) + }); + + it(printTitle('random', 'indexOf for non existing item returns 0'), async () => { + const linkedListStorage = await LinkedListStorage.deployed(); + let itemIn = { + receiver: random, + validatorId: 1, + suppliedValue: 8000, + requestedValue: 32000, + } + let indexOf = await linkedListStorage.getIndexOf(regularQueue, itemIn); + assert.equal(indexOf, 0) + }); + + it(printTitle('random', 'reverts when trying to remove non existent item'), async () => { + const linkedListStorage = await LinkedListStorage.deployed(); + let itemIn = { + receiver: random, + validatorId: 1, + suppliedValue: 8000, + requestedValue: 32000, + } + await linkedListStorage.enqueueItem(regularQueue, itemIn) + itemIn.validatorId = 2 + await shouldRevert(linkedListStorage.removeItem(regularQueue, itemIn)); + }); + + }); +}