Skip to content

Commit

Permalink
Merge pull request #216 from OriginTrail/fix/sharding-table-v2
Browse files Browse the repository at this point in the history
ShardingTableV2 fix
  • Loading branch information
0xbraindevd authored Jan 29, 2024
2 parents add3d33 + 5bf3a15 commit 86642fa
Show file tree
Hide file tree
Showing 4 changed files with 266 additions and 10 deletions.
52 changes: 46 additions & 6 deletions contracts/v2/ShardingTable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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));
Expand All @@ -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;
Expand All @@ -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,
Expand All @@ -140,15 +152,21 @@ 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,
nextIdentityId,
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,
Expand All @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions contracts/v2/storage/ShardingTableStorage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
5 changes: 3 additions & 2 deletions test/v1/unit/ShardingTableStorage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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;
Expand Down
214 changes: 214 additions & 0 deletions test/v2/unit/ShardingTableV2.test.ts
Original file line number Diff line number Diff line change
@@ -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<ShardingTableFixture> {
await hre.deployments.fixture(['ShardingTableV2', 'IdentityStorageV2', 'StakingV2', 'Profile'], {
keepExistingDeployments: false,
});
accounts = await hre.ethers.getSigners();
const HubController = await hre.ethers.getContract<HubController>('HubController');
Profile = await hre.ethers.getContract<Profile>('Profile');
ShardingTableStorage = await hre.ethers.getContract<ShardingTableStorageV2>('ShardingTableStorage');
ShardingTable = await hre.ethers.getContract<ShardingTableV2>('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]);
});
});

0 comments on commit 86642fa

Please sign in to comment.