diff --git a/contracts/v2/ShardingTable.sol b/contracts/v2/ShardingTable.sol index 0a391a8e..bbef6660 100644 --- a/contracts/v2/ShardingTable.sol +++ b/contracts/v2/ShardingTable.sol @@ -70,8 +70,13 @@ contract ShardingTableV2 is Named, Versioned, ContractStatus, Initializable { ShardingTableStructsV2.Node memory nodeToRemove = sts.getNode(identityId); + // If removing Head => set new Head (can also be 0 if there is only 1 node in the list) + if (nodeToRemove.prevIdentityId == NULL) sts.setHead(nodeToRemove.nextIdentityId); + + // Link left and right nodes (both can be NULL, there is a check in link function) sts.link(nodeToRemove.prevIdentityId, nodeToRemove.nextIdentityId); + // Decrement indexes of all nodes after removed one + add pointers to identityId for changed indexes uint72 index = nodeToRemove.index; uint72 nextIdentityId = nodeToRemove.nextIdentityId; while (nextIdentityId != NULL) { @@ -84,7 +89,9 @@ contract ShardingTableV2 is Named, Versioned, ContractStatus, Initializable { nextIdentityId = sts.getNode(nextIdentityId).nextIdentityId; } + // Delete node object + set last index pointer to be NULL + decrement total nodes count sts.deleteNodeObject(identityId); + sts.setIdentityId(index, 0); sts.decrementNodesCount(); emit NodeRemoved(identityId, profileStorage.getNodeId(identityId)); @@ -104,12 +111,15 @@ contract ShardingTableV2 is Named, Versioned, ContractStatus, Initializable { ShardingTableStructsV2.Node memory currentNode = sts.getNodeByIndex(uint72(mid)); if (currentNode.hashRingPosition < hashRingPosition) { + // Node is in the right half of the range, move left pointers prevIdentityId = currentNode.identityId; left = mid + 1; } else if (currentNode.hashRingPosition > hashRingPosition) { + // Node is in the left half of the range, move right pointers nextIdentityId = currentNode.identityId; right = mid - 1; } else { + // Exact match found prevIdentityId = currentNode.identityId; nextIdentityId = currentNode.nextIdentityId; break; @@ -130,7 +140,9 @@ contract ShardingTableV2 is Named, Versioned, ContractStatus, Initializable { ShardingTableStructsV2.Node memory prevNode = sts.getNode(prevIdentityId); - if (prevNode.hashRingPosition > newNodeHashRingPosition) + // Check that the new Node is indeed on the right from the prevNode + // Also allows new Head insertion as prevNode.hashRingPosition will be 0 in such case + if (newNodeHashRingPosition < prevNode.hashRingPosition) revert ShardingTableErrors.InvalidPreviousIdentityId( identityId, newNodeHashRingPosition, @@ -140,7 +152,9 @@ contract ShardingTableV2 is Named, Versioned, ContractStatus, Initializable { ShardingTableStructsV2.Node memory nextNode = sts.getNode(nextIdentityId); - if (nextNode.identityId != NULL && nextNode.hashRingPosition < newNodeHashRingPosition) + // Check that the new Node is indeed on the left from the nextNode + // Check is skipped when inserting new Tail + if (nextNode.identityId != NULL && newNodeHashRingPosition > nextNode.hashRingPosition) revert ShardingTableErrors.InvalidNextIdentityId( identityId, newNodeHashRingPosition, @@ -148,7 +162,11 @@ contract ShardingTableV2 is Named, Versioned, ContractStatus, Initializable { nextNode.hashRingPosition ); - if (prevNode.nextIdentityId != nextNode.prevIdentityId) + // Verify that prevNode and nextNode are direct neighbors before inserting a new node between them + if ( + (prevIdentityId != NULL && nextIdentityId != prevNode.nextIdentityId) || + (nextIdentityId != NULL && prevIdentityId != nextNode.prevIdentityId) + ) revert ShardingTableErrors.InvalidPreviousOrNextIdentityId( identityId, prevIdentityId, @@ -157,16 +175,38 @@ contract ShardingTableV2 is Named, Versioned, ContractStatus, Initializable { prevNode.nextIdentityId ); - sts.createNodeObject(newNodeHashRingPosition, identityId, prevIdentityId, nextIdentityId, nextNode.index); - sts.setIdentityId(nextNode.index, identityId); + uint72 index; + if (nextIdentityId == NULL) { + // Inserting a new Tail + if (prevIdentityId != NULL) { + // The list is not empty, calculate the new Tail's index + index = prevNode.index + 1; + } else { + // The list is empty, start with index 0 + index = 0; + } + } else { + // Inserting a node before the nextNode + index = nextNode.index; + } + + // Create node object + set index pointer to new identityId + increment total nodes count + sts.createNodeObject(newNodeHashRingPosition, identityId, prevIdentityId, nextIdentityId, index); + sts.setIdentityId(index, identityId); sts.incrementNodesCount(); + // If Head => add Head pointer + // If not Head => add the link between prevNode and new Node if (prevIdentityId == NULL) sts.setHead(identityId); else sts.link(prevIdentityId, identityId); + // If not Tail => add the link between new Node and nextNode if (nextIdentityId != NULL) sts.link(identityId, nextIdentityId); - uint72 index = nextNode.index + 1; + // Increment indexes of all nodes after inserted one + add pointers to identityId for changed indexes + unchecked { + index += 1; + } while (nextIdentityId != NULL) { sts.incrementNodeIndex(nextIdentityId); sts.setIdentityId(index, nextIdentityId); diff --git a/contracts/v2/storage/ShardingTableStorage.sol b/contracts/v2/storage/ShardingTableStorage.sol index 6291cb38..78b2b414 100644 --- a/contracts/v2/storage/ShardingTableStorage.sol +++ b/contracts/v2/storage/ShardingTableStorage.sol @@ -119,7 +119,8 @@ contract ShardingTableStorageV2 is Named, Versioned, HubDependent { } function link(uint72 leftNodeIdentityId, uint72 rightNodeIdentityId) external onlyContracts { - nodes[leftNodeIdentityId].nextIdentityId = rightNodeIdentityId; - nodes[rightNodeIdentityId].prevIdentityId = leftNodeIdentityId; + if (leftNodeIdentityId != NULL) nodes[leftNodeIdentityId].nextIdentityId = rightNodeIdentityId; + + if (rightNodeIdentityId != NULL) nodes[rightNodeIdentityId].prevIdentityId = leftNodeIdentityId; } } diff --git a/test/v1/unit/ShardingTableStorage.test.ts b/test/v1/unit/ShardingTableStorage.test.ts index c6fe2acc..1ab45165 100644 --- a/test/v1/unit/ShardingTableStorage.test.ts +++ b/test/v1/unit/ShardingTableStorage.test.ts @@ -44,8 +44,8 @@ describe('@v1 @unit ShardingTableStorage Contract', function () { } async function createMultipleProfiles() { - const adminWallet1 = await Profile.connect(accounts[1]); - const adminWallet2 = await Profile.connect(accounts[2]); + const adminWallet1 = Profile.connect(accounts[1]); + const adminWallet2 = Profile.connect(accounts[2]); const profile1 = await Profile.createProfile(accounts[3].address, nodeId1, 'Token', 'TKN'); const profile2 = await adminWallet1.createProfile(accounts[4].address, nodeId2, 'Token1', 'TKN1'); const profile3 = await adminWallet2.createProfile(accounts[5].address, nodeId3, 'Token2', 'TKN2'); @@ -56,6 +56,7 @@ describe('@v1 @unit ShardingTableStorage Contract', function () { const receipt = await singleIdentityId.wait(); identityId = receipt.events?.[3].args?.identityId.toNumber(); + console.log(identityId); idsArray.push(identityId); } return idsArray; diff --git a/test/v2/unit/ShardingTableV2.test.ts b/test/v2/unit/ShardingTableV2.test.ts new file mode 100644 index 00000000..82f03688 --- /dev/null +++ b/test/v2/unit/ShardingTableV2.test.ts @@ -0,0 +1,214 @@ +import { loadFixture } from '@nomicfoundation/hardhat-network-helpers'; +import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers'; +import { expect } from 'chai'; +import hre from 'hardhat'; + +import { HubController, Profile, ShardingTableStorageV2, ShardingTableV2 } from '../../../typechain'; + +type ShardingTableFixture = { + accounts: SignerWithAddress[]; + Profile: Profile; + ShardingTableStorage: ShardingTableStorageV2; + ShardingTable: ShardingTableV2; +}; + +describe('@v2 @unit ShardingTableV2 contract', function () { + let accounts: SignerWithAddress[]; + let Profile: Profile; + let ShardingTableStorage: ShardingTableStorageV2; + let ShardingTable: ShardingTableV2; + let identityId: number; + + const nodeId1 = '0x01'; + const nodeId2 = '0x02'; + const nodeId3 = '0x03'; + const nodeId4 = '0x04'; + const nodeId5 = '0x05'; + + // 3 1 2 4 5 + + // console.log(hre.ethers.BigNumber.from(hre.ethers.utils.sha256(nodeId1)).toString()); + // console.log(hre.ethers.BigNumber.from(hre.ethers.utils.sha256(nodeId2)).toString()); + // console.log(hre.ethers.BigNumber.from(hre.ethers.utils.sha256(nodeId3)).toString()); + // console.log(hre.ethers.BigNumber.from(hre.ethers.utils.sha256(nodeId4)).toString()); + // console.log(hre.ethers.BigNumber.from(hre.ethers.utils.sha256(nodeId5)).toString()); + + async function deployShardingTableFixture(): Promise { + await hre.deployments.fixture(['ShardingTableV2', 'IdentityStorageV2', 'StakingV2', 'Profile'], { + keepExistingDeployments: false, + }); + accounts = await hre.ethers.getSigners(); + const HubController = await hre.ethers.getContract('HubController'); + Profile = await hre.ethers.getContract('Profile'); + ShardingTableStorage = await hre.ethers.getContract('ShardingTableStorage'); + ShardingTable = await hre.ethers.getContract('ShardingTable'); + await HubController.setContractAddress('HubOwner', accounts[0].address); + + return { accounts, Profile, ShardingTableStorage, ShardingTable }; + } + + async function createMultipleProfiles() { + const opWallet1 = Profile.connect(accounts[1]); + const opWallet2 = Profile.connect(accounts[2]); + const opWallet3 = Profile.connect(accounts[3]); + const opWallet4 = Profile.connect(accounts[4]); + const profile1 = await Profile.createProfile(accounts[6].address, nodeId1, 'Token', 'TKN'); + const profile2 = await opWallet1.createProfile(accounts[7].address, nodeId2, 'Token1', 'TKN1'); + const profile3 = await opWallet2.createProfile(accounts[8].address, nodeId3, 'Token2', 'TKN2'); + const profile4 = await opWallet3.createProfile(accounts[9].address, nodeId4, 'Token3', 'TKN3'); + const profile5 = await opWallet4.createProfile(accounts[10].address, nodeId5, 'Token4', 'TKN4'); + const idsArray = []; + + const profileArray = [profile1, profile2, profile3, profile4, profile5]; + for (const singleIdentityId of profileArray) { + const receipt = await singleIdentityId.wait(); + + identityId = receipt.events?.[3].args?.identityId.toNumber(); + idsArray.push(identityId); + } + return idsArray; + } + + async function validateShardingTableResult(identityIds: number[]) { + const nodesCount = (await ShardingTableStorage.nodesCount()).toNumber(); + + expect(identityIds.length, 'Invalid number of nodes').to.equal(nodesCount); + + for (let i = 0; i < identityIds.length; i++) { + const node = await ShardingTableStorage.getNode(identityIds[i]); + const nodeByIndex = await ShardingTableStorage.getNodeByIndex(i); + + expect(node).to.be.eql(nodeByIndex); + + expect(node.identityId.toNumber(), 'Invalid node on this position').to.equal(identityIds[i]); + + expect(node.index.toNumber(), 'Invalid node index').to.equal(i); + if (i === 0) { + expect(node.prevIdentityId.toNumber(), 'Invalid prevIdentityId').to.equal(0); + } else { + expect(node.prevIdentityId.toNumber(), 'Invalid prevIdentityId').to.equal(identityIds[i - 1]); + } + + if (i === nodesCount - 1) { + expect(node.nextIdentityId.toNumber(), 'Invalid nextIdentityId').to.equal(0); + } else { + expect(node.nextIdentityId.toNumber(), 'Invalid nextIdentityId').to.equal(identityIds[i + 1]); + } + } + } + + beforeEach(async () => { + hre.helpers.resetDeploymentsJson(); + ({ accounts, Profile, ShardingTableStorage, ShardingTable } = await loadFixture(deployShardingTableFixture)); + }); + + it('Should initialize contract with correct values', async () => { + const name = await ShardingTable.name(); + const version = await ShardingTable.version(); + + expect(name).to.equal('ShardingTable'); + expect(version).to.equal('2.0.0'); + }); + + it('Insert 5 nodes, nodes are sorted expect to pass', async () => { + const profiles = await createMultipleProfiles(); + + // 2 + await ShardingTable['insertNode(uint72,uint72,uint72)'](profiles[1], 0, 0); + await validateShardingTableResult([2]); + + // 3 2 + await ShardingTable['insertNode(uint72,uint72,uint72)'](profiles[2], 0, profiles[1]); + await validateShardingTableResult([3, 2]); + + // 3 2 5 + await ShardingTable['insertNode(uint72,uint72,uint72)'](profiles[4], profiles[1], 0); + await validateShardingTableResult([3, 2, 5]); + + // 3 2 4 5 + await ShardingTable['insertNode(uint72,uint72,uint72)'](profiles[3], profiles[1], profiles[4]); + await validateShardingTableResult([3, 2, 4, 5]); + + // 3 1 2 4 5 + await ShardingTable['insertNode(uint72,uint72,uint72)'](profiles[0], profiles[2], profiles[1]); + await validateShardingTableResult([3, 1, 2, 4, 5]); + }); + + it('Insert 5 nodes, without sorting, expect to be pass and be sorted on insert', async () => { + const profiles = await createMultipleProfiles(); + + // 2 + await ShardingTable['insertNode(uint72)'](profiles[1]); + await validateShardingTableResult([2]); + + // 3 2 + await ShardingTable['insertNode(uint72)'](profiles[2]); + await validateShardingTableResult([3, 2]); + + // 3 2 5 + await ShardingTable['insertNode(uint72)'](profiles[4]); + await validateShardingTableResult([3, 2, 5]); + + // 3 2 4 5 + await ShardingTable['insertNode(uint72)'](profiles[3]); + await validateShardingTableResult([3, 2, 4, 5]); + + // 3 1 2 4 5 + await ShardingTable['insertNode(uint72)'](profiles[0]); + await validateShardingTableResult([3, 1, 2, 4, 5]); + }); + + it('Insert node with invalid prevIdentityId expect to fail', async () => { + const profiles = await createMultipleProfiles(); + + await ShardingTable['insertNode(uint72,uint72,uint72)'](profiles[1], 0, 0); + + await expect( + ShardingTable['insertNode(uint72,uint72,uint72)'](profiles[0], profiles[1], 0), + ).to.be.revertedWithCustomError(ShardingTable, 'InvalidPreviousIdentityId'); + }); + + it('Insert node with invalid nextIdentityId expect to fail', async () => { + const profiles = await createMultipleProfiles(); + + await ShardingTable['insertNode(uint72,uint72,uint72)'](profiles[1], 0, 0); + + await expect( + ShardingTable['insertNode(uint72,uint72,uint72)'](profiles[3], 0, profiles[1]), + ).to.be.revertedWithCustomError(ShardingTable, 'InvalidNextIdentityId'); + }); + + it('Insert node with invalid prevIdentityId and nextIdentityId expect to fail', async () => { + const profiles = await createMultipleProfiles(); + + await ShardingTable['insertNode(uint72,uint72,uint72)'](profiles[2], 0, 0); + await ShardingTable['insertNode(uint72,uint72,uint72)'](profiles[1], profiles[2], 0); + await ShardingTable['insertNode(uint72,uint72,uint72)'](profiles[3], profiles[1], 0); + + await expect( + ShardingTable['insertNode(uint72,uint72,uint72)'](profiles[0], profiles[2], profiles[3]), + ).to.be.revertedWithCustomError(ShardingTable, 'InvalidPreviousOrNextIdentityId'); + }); + + it('Remove node from sharding table, expect to pass', async () => { + const profiles = await createMultipleProfiles(); + + await ShardingTable['insertNode(uint72,uint72,uint72)'](profiles[2], 0, 0); + await ShardingTable['insertNode(uint72,uint72,uint72)'](profiles[0], profiles[2], 0); + await ShardingTable['insertNode(uint72,uint72,uint72)'](profiles[1], profiles[0], 0); + await ShardingTable['insertNode(uint72,uint72,uint72)'](profiles[3], profiles[1], 0); + await ShardingTable['insertNode(uint72,uint72,uint72)'](profiles[4], profiles[3], 0); + + // remove from index 0 + await ShardingTable.removeNode(profiles[2]); + await validateShardingTableResult([1, 2, 4, 5]); + + // remove from last index + await ShardingTable.removeNode(profiles[4]); + await validateShardingTableResult([1, 2, 4]); + + // remove from middle + await ShardingTable.removeNode(profiles[1]); + await validateShardingTableResult([1, 4]); + }); +});