From c6e58a24414e7ea48259dfb7f92a8e059d774e07 Mon Sep 17 00:00:00 2001 From: Artem Payvin Date: Fri, 6 Mar 2020 18:27:19 +0200 Subject: [PATCH 1/4] Fix delete monitor bug --- contracts/MonitorsData.sol | 4 ++++ contracts/MonitorsFunctionality.sol | 8 ++++++- test/MonitorsFunctionality.spec.ts | 36 +++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/contracts/MonitorsData.sol b/contracts/MonitorsData.sol index a4c8126fb..d8188f761 100644 --- a/contracts/MonitorsData.sol +++ b/contracts/MonitorsData.sol @@ -81,6 +81,10 @@ contract MonitorsData is GroupsData { return checkedNodes[monitorIndex]; } + function getCheckedArrayLength(bytes32 monitorIndex) external view returns (uint) { + return checkedNodes[monitorIndex].length; + } + function getLengthOfMetrics(bytes32 monitorIndex) external view returns (uint) { return verdicts[monitorIndex].length; } diff --git a/contracts/MonitorsFunctionality.sol b/contracts/MonitorsFunctionality.sol index 923733084..6491154ee 100644 --- a/contracts/MonitorsFunctionality.sol +++ b/contracts/MonitorsFunctionality.sol @@ -22,7 +22,9 @@ pragma solidity ^0.5.3; import "./GroupsFunctionality.sol"; import "./interfaces/IConstants.sol"; import "./interfaces/INodesData.sol"; +import "./NodesData.sol"; import "./MonitorsData.sol"; +import "@nomiclabs/buidler/console.sol"; contract MonitorsFunctionality is GroupsFunctionality { @@ -118,6 +120,7 @@ contract MonitorsFunctionality is GroupsFunctionality { function deleteMonitor(uint nodeIndex) external allow(executorName) { bytes32 groupIndex = keccak256(abi.encodePacked(nodeIndex)); MonitorsData data = MonitorsData(contractManager.getContract("MonitorsData")); + NodesData nodesData = NodesData(contractManager.getContract("NodesData")); data.removeAllVerdicts(groupIndex); data.removeAllCheckedNodes(groupIndex); uint[] memory nodesInGroup = data.getNodesInGroup(groupIndex); @@ -126,7 +129,9 @@ contract MonitorsFunctionality is GroupsFunctionality { for (uint i = 0; i < nodesInGroup.length; i++) { monitorIndex = keccak256(abi.encodePacked(nodesInGroup[i])); (index, ) = find(monitorIndex, nodeIndex); - data.removeCheckedNode(monitorIndex, index); + if (index < data.getCheckedArrayLength(monitorIndex)) { + data.removeCheckedNode(monitorIndex, index); + } } deleteGroup(groupIndex); } @@ -266,6 +271,7 @@ contract MonitorsFunctionality is GroupsFunctionality { bytes32[] memory checkedNodes = data.getCheckedArray(monitorIndex); uint possibleIndex; uint32 possibleTime; + index = checkedNodes.length; for (uint i = 0; i < checkedNodes.length; i++) { (possibleIndex, possibleTime) = getDataFromBytes(checkedNodes[i]); if (possibleIndex == nodeIndex && (time == 0 || possibleTime < time)) { diff --git a/test/MonitorsFunctionality.spec.ts b/test/MonitorsFunctionality.spec.ts index 3fa5ce561..ebd655678 100644 --- a/test/MonitorsFunctionality.spec.ts +++ b/test/MonitorsFunctionality.spec.ts @@ -241,6 +241,42 @@ contract("MonitorsFunctionality", ([owner, validator]) => { await monitorsData.getCheckedArray(node4Hash).should.be.eventually.empty; }); + it("should delete nodes from checked list", async () => { + await monitorsFunctionality.addMonitor(0); + await monitorsFunctionality.addMonitor(1); + + const node0Hash = web3.utils.soliditySha3(0); + const node1Hash = web3.utils.soliditySha3(1); + const node2Hash = web3.utils.soliditySha3(2); + const node3Hash = web3.utils.soliditySha3(3); + const node4Hash = web3.utils.soliditySha3(4); + + (await monitorsData.getCheckedArray(node0Hash)).length.should.be.equal(1); + (await monitorsData.getCheckedArray(node1Hash)).length.should.be.equal(1); + (await monitorsData.getCheckedArray(node2Hash)).length.should.be.equal(2); + (await monitorsData.getCheckedArray(node3Hash)).length.should.be.equal(2); + (await monitorsData.getCheckedArray(node4Hash)).length.should.be.equal(2); + + await monitorsFunctionality.deleteMonitor(0); + console.log("Finished delete 0 monitor"); + + await monitorsData.getCheckedArray(node0Hash).should.be.eventually.empty; + await monitorsData.getCheckedArray(node1Hash).should.be.eventually.empty; + (await monitorsData.getCheckedArray(node2Hash)).length.should.be.equal(1); + (await monitorsData.getCheckedArray(node3Hash)).length.should.be.equal(1); + (await monitorsData.getCheckedArray(node4Hash)).length.should.be.equal(1); + + console.log("Started delete 1 monitor"); + await monitorsFunctionality.deleteMonitor(1); + console.log("Finished delete 1 monitor"); + + // await monitorsData.getCheckedArray(node0Hash).should.be.eventually.empty; + // await monitorsData.getCheckedArray(node1Hash).should.be.eventually.empty; + // await monitorsData.getCheckedArray(node2Hash).should.be.eventually.empty; + // await monitorsData.getCheckedArray(node3Hash).should.be.eventually.empty; + // await monitorsData.getCheckedArray(node4Hash).should.be.eventually.empty; + }); + const nodesCount = 50; const activeNodesCount = 30; describe("when " + nodesCount + " nodes in network", async () => { From 069a99f1d86f283212197a0ad203369980257c2b Mon Sep 17 00:00:00 2001 From: Artem Payvin Date: Sat, 7 Mar 2020 15:49:51 +0200 Subject: [PATCH 2/4] Fixed linting --- contracts/MonitorsFunctionality.sol | 1 - test/MonitorsFunctionality.spec.ts | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/contracts/MonitorsFunctionality.sol b/contracts/MonitorsFunctionality.sol index 6491154ee..eadd723ec 100644 --- a/contracts/MonitorsFunctionality.sol +++ b/contracts/MonitorsFunctionality.sol @@ -120,7 +120,6 @@ contract MonitorsFunctionality is GroupsFunctionality { function deleteMonitor(uint nodeIndex) external allow(executorName) { bytes32 groupIndex = keccak256(abi.encodePacked(nodeIndex)); MonitorsData data = MonitorsData(contractManager.getContract("MonitorsData")); - NodesData nodesData = NodesData(contractManager.getContract("NodesData")); data.removeAllVerdicts(groupIndex); data.removeAllCheckedNodes(groupIndex); uint[] memory nodesInGroup = data.getNodesInGroup(groupIndex); diff --git a/test/MonitorsFunctionality.spec.ts b/test/MonitorsFunctionality.spec.ts index ebd655678..aadba49b8 100644 --- a/test/MonitorsFunctionality.spec.ts +++ b/test/MonitorsFunctionality.spec.ts @@ -259,7 +259,7 @@ contract("MonitorsFunctionality", ([owner, validator]) => { await monitorsFunctionality.deleteMonitor(0); console.log("Finished delete 0 monitor"); - + await monitorsData.getCheckedArray(node0Hash).should.be.eventually.empty; await monitorsData.getCheckedArray(node1Hash).should.be.eventually.empty; (await monitorsData.getCheckedArray(node2Hash)).length.should.be.equal(1); From 2cfcaef6361e6935f123ac43f1d3018096c95401 Mon Sep 17 00:00:00 2001 From: Artem Payvin Date: Tue, 10 Mar 2020 20:06:09 +0200 Subject: [PATCH 3/4] Remove console --- contracts/MonitorsFunctionality.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/MonitorsFunctionality.sol b/contracts/MonitorsFunctionality.sol index eadd723ec..d501156f0 100644 --- a/contracts/MonitorsFunctionality.sol +++ b/contracts/MonitorsFunctionality.sol @@ -24,7 +24,6 @@ import "./interfaces/IConstants.sol"; import "./interfaces/INodesData.sol"; import "./NodesData.sol"; import "./MonitorsData.sol"; -import "@nomiclabs/buidler/console.sol"; contract MonitorsFunctionality is GroupsFunctionality { From 9e59dc50627898acaacc9ab221bd0917cfa02057 Mon Sep 17 00:00:00 2001 From: Artem Payvin Date: Tue, 10 Mar 2020 20:10:50 +0200 Subject: [PATCH 4/4] Removed unused import --- contracts/MonitorsFunctionality.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/MonitorsFunctionality.sol b/contracts/MonitorsFunctionality.sol index d501156f0..3c481c7e4 100644 --- a/contracts/MonitorsFunctionality.sol +++ b/contracts/MonitorsFunctionality.sol @@ -22,7 +22,6 @@ pragma solidity ^0.5.3; import "./GroupsFunctionality.sol"; import "./interfaces/IConstants.sol"; import "./interfaces/INodesData.sol"; -import "./NodesData.sol"; import "./MonitorsData.sol";