From 10fdf5dbfd9b9363c4d07471207cd9ec7c669e95 Mon Sep 17 00:00:00 2001 From: Piotr Dyraga Date: Mon, 19 Sep 2022 12:26:21 +0200 Subject: [PATCH 1/4] Chaosnet support This is a beta staker program for stakers willing to go the extra mile with monitoring, share their logs with the dev team, and allow to more carefully monitor the bootstrapping network. As the network matures, the beta program will be ended. --- contracts/Chaosnet.sol | 78 ++++++++++++ contracts/SortitionPool.sol | 17 ++- test/sortitionPoolTest.js | 246 +++++++++++++++++++++++++++++++++--- 3 files changed, 324 insertions(+), 17 deletions(-) create mode 100644 contracts/Chaosnet.sol diff --git a/contracts/Chaosnet.sol b/contracts/Chaosnet.sol new file mode 100644 index 0000000..bdf5523 --- /dev/null +++ b/contracts/Chaosnet.sol @@ -0,0 +1,78 @@ +pragma solidity 0.8.9; + +/// @title Chaosnet +/// @notice This is a beta staker program for stakers willing to go the extra +/// mile with monitoring, share their logs with the dev team, and allow to more +/// carefully monitor the bootstrapping network. As the network matures, the +/// beta program will be ended. +contract Chaosnet { + /// @notice Indicates if the chaosnet is active. The chaosnet is active + /// after the contract deployment and can be ended with a call to + /// `deactivateChaosnet()`. Once deactivated chaosnet can not be activated + /// again. + bool public isChaosnetActive; + + /// @notice Indicates if the given operator is a beta operator for chaosnet. + mapping(address => bool) public isBetaOperator; + + /// @notice Address controlling chaosnet status and beta operator addresses. + address public chaosnetMaestro; + + event BetaOperatorsAdded(address[] operators); + + event ChaosnetMaestroRoleTransferred( + address oldChaosnetMaestro, + address newChaosnetMaestro + ); + + event ChaosnetDeactivated(); + + constructor() { + _transferChaosnetMaestro(msg.sender); + isChaosnetActive = true; + } + + modifier onlyChaosnetMaestro() { + require(msg.sender == chaosnetMaestro, "Not the chaosnet maestro"); + _; + } + + /// @notice Adds beta operator to chaosnet. Can be called only by the + /// chaosnet maestro. + function addBetaOperators(address[] calldata operators) + public + onlyChaosnetMaestro + { + for (uint256 i = 0; i < operators.length; i++) { + isBetaOperator[operators[i]] = true; + } + + emit BetaOperatorsAdded(operators); + } + + /// @notice Deactivates the chaosnet. Can be called only by the chaosnet + /// maestro. Once deactivated chaosnet can not be activated again. + function deactivateChaosnet() public onlyChaosnetMaestro { + require(isChaosnetActive, "Chaosnet is not active"); + isChaosnetActive = false; + emit ChaosnetDeactivated(); + } + + /// @notice Transfers the chaosnet maestro role to another non-zero address. + function transferChaosnetMaestroRole(address newChaosnetMaestro) + public + onlyChaosnetMaestro + { + require( + newChaosnetMaestro != address(0), + "New chaosnet maestro must not be zero address" + ); + _transferChaosnetMaestro(newChaosnetMaestro); + } + + function _transferChaosnetMaestro(address newChaosnetMaestro) internal { + address oldChaosnetMaestro = chaosnetMaestro; + chaosnetMaestro = newChaosnetMaestro; + emit ChaosnetMaestroRoleTransferred(oldChaosnetMaestro, newChaosnetMaestro); + } +} diff --git a/contracts/SortitionPool.sol b/contracts/SortitionPool.sol index e7139c5..1ffd107 100644 --- a/contracts/SortitionPool.sol +++ b/contracts/SortitionPool.sol @@ -8,12 +8,19 @@ import "@openzeppelin/contracts/access/Ownable.sol"; import "./RNG.sol"; import "./SortitionTree.sol"; import "./Rewards.sol"; +import "./Chaosnet.sol"; /// @title Sortition Pool /// @notice A logarithmic data structure used to store the pool of eligible /// operators weighted by their stakes. It allows to select a group of operators /// based on the provided pseudo-random seed. -contract SortitionPool is SortitionTree, Rewards, Ownable, IReceiveApproval { +contract SortitionPool is + SortitionTree, + Rewards, + Ownable, + Chaosnet, + IReceiveApproval +{ using Branch for uint256; using Leaf for uint256; using Position for uint256; @@ -98,7 +105,9 @@ contract SortitionPool is SortitionTree, Rewards, Ownable, IReceiveApproval { } /// @notice Inserts an operator to the pool. Reverts if the operator is - /// already present. + /// already present. Reverts if the operator is not eligible because of their + /// authorized stake. Reverts if the chaosnet is active and the operator is + /// not a beta operator. /// @dev Can be called only by the contract owner. /// @param operator Address of the inserted operator. /// @param authorizedStake Inserted operator's authorized stake for the application. @@ -110,6 +119,10 @@ contract SortitionPool is SortitionTree, Rewards, Ownable, IReceiveApproval { uint256 weight = getWeight(authorizedStake); require(weight > 0, "Operator not eligible"); + if (isChaosnetActive) { + require(isBetaOperator[operator], "Not beta operator for chaosnet"); + } + _insertOperator(operator, weight); uint32 id = getOperatorID(operator); Rewards.updateOperatorRewards(id, uint32(weight)); diff --git a/test/sortitionPoolTest.js b/test/sortitionPoolTest.js index c6ce250..177a9f2 100644 --- a/test/sortitionPoolTest.js +++ b/test/sortitionPoolTest.js @@ -2,6 +2,8 @@ const chai = require("chai") const expect = chai.expect const { ethers, helpers } = require("hardhat") +const { ZERO_ADDRESS } = require("@openzeppelin/test-helpers/src/constants") + describe("SortitionPool", () => { const seed = "0xff39d6cca87853892d2854566e883008bc000000000000000000000000000000" @@ -19,8 +21,17 @@ describe("SortitionPool", () => { let bobBeneficiary beforeEach(async () => { - ;[deployer, owner, alice, bob, carol, aliceBeneficiary, bobBeneficiary] = - await ethers.getSigners() + ;[ + deployer, + owner, + chaosnetMaestro, + alice, + bob, + carol, + aliceBeneficiary, + bobBeneficiary, + thirdParty, + ] = await ethers.getSigners() const TokenStub = await ethers.getContractFactory("TokenStub") token = await TokenStub.deploy() @@ -31,6 +42,15 @@ describe("SortitionPool", () => { await pool.deployed() await pool.connect(deployer).transferOwnership(owner.address) + await pool + .connect(deployer) + .transferChaosnetMaestroRole(chaosnetMaestro.address) + }) + + describe("constructor", () => { + it("should activate chaosnet", async () => { + expect(await pool.isChaosnetActive()).to.be.true + }) }) describe("lock", () => { @@ -82,25 +102,88 @@ describe("SortitionPool", () => { describe("insertOperator", () => { context("when sortition pool is unlocked", () => { context("when called by the owner", () => { - context("when operator is eligible", () => { - beforeEach(async () => { - await pool - .connect(owner) - .insertOperator(alice.address, poolWeightDivisor) + context("when chaosnet is active", () => { + context("when operator is eligible", () => { + context("when operator is not beta operator", () => { + it("should revert", async () => { + await expect( + pool + .connect(owner) + .insertOperator(alice.address, poolWeightDivisor), + ).to.be.revertedWith("Not beta operator for chaosnet") + }) + }) + + context("when operator is beta operator", () => { + beforeEach(async () => { + await pool + .connect(chaosnetMaestro) + .addBetaOperators([alice.address]) + await pool + .connect(owner) + .insertOperator(alice.address, poolWeightDivisor) + }) + + it("should insert the operator to the pool", async () => { + expect(await pool.isOperatorInPool(alice.address)).to.be.true + }) + }) }) - it("should insert the operator to the pool", async () => { - expect(await pool.isOperatorInPool(alice.address)).to.be.true + context("when operator is not eligible", () => { + context("when operator is not beta operator", () => { + it("should revert", async () => { + await expect( + pool + .connect(owner) + .insertOperator(alice.address, poolWeightDivisor - 1), + ).to.be.revertedWith("Operator not eligible") + }) + }) + + context("when operator is beta operator", () => { + beforeEach(async () => { + await pool + .connect(chaosnetMaestro) + .addBetaOperators([alice.address]) + }) + + it("should revert", async () => { + await expect( + pool + .connect(owner) + .insertOperator(alice.address, poolWeightDivisor - 1), + ).to.be.revertedWith("Operator not eligible") + }) + }) }) }) - context("when operator is not eligible", () => { - it("should revert", async () => { - await expect( - pool + context("when chaosnet is not active", () => { + beforeEach(async () => { + await pool.connect(chaosnetMaestro).deactivateChaosnet() + }) + + context("when operator is eligible", () => { + beforeEach(async () => { + await pool .connect(owner) - .insertOperator(alice.address, poolWeightDivisor - 1), - ).to.be.revertedWith("Operator not eligible") + .insertOperator(alice.address, poolWeightDivisor) + }) + + it("should insert the operator to the pool", async () => { + expect(await pool.isOperatorInPool(alice.address)).to.be.true + }) + }) + + context("when operator is not eligible", () => { + it("should revert", async () => { + await expect( + pool + .connect(owner) + .insertOperator(alice.address, poolWeightDivisor - 1), + ).to.be.revertedWith("Operator not eligible") + }) }) }) }) @@ -131,6 +214,7 @@ describe("SortitionPool", () => { describe("updateOperatorStatus", () => { beforeEach(async () => { + await pool.connect(chaosnetMaestro).deactivateChaosnet() await pool.connect(owner).insertOperator(alice.address, 2000) }) @@ -185,6 +269,10 @@ describe("SortitionPool", () => { }) describe("selectGroup", async () => { + beforeEach(async () => { + await pool.connect(chaosnetMaestro).deactivateChaosnet() + }) + context("when sortition pool is locked", () => { beforeEach(async () => {}) @@ -259,6 +347,10 @@ describe("SortitionPool", () => { }) describe("pool rewards", async () => { + beforeEach(async () => { + await pool.connect(chaosnetMaestro).deactivateChaosnet() + }) + async function withdrawRewards( pool, owner, @@ -436,4 +528,128 @@ describe("SortitionPool", () => { expect(group.length).to.equal(100) }) }) + + describe("addBetaOperators", () => { + context("when called by third party", () => { + it("should revert", async () => { + await expect( + pool.connect(thirdParty).addBetaOperators([alice.address]), + ).to.be.revertedWith("Not the chaosnet maestro") + }) + }) + + context("when called by the operator", () => { + it("should revert", async () => { + await expect( + pool.connect(alice).addBetaOperators([alice.address]), + ).to.be.revertedWith("Not the chaosnet maestro") + }) + }) + + context("when called by the chaosnet maestro", () => { + let tx + + beforeEach(async () => { + tx = await pool + .connect(chaosnetMaestro) + .addBetaOperators([alice.address, bob.address]) + }) + + it("should set selected operators as beta operators", async () => { + expect(await pool.isBetaOperator(alice.address)).to.be.true + expect(await pool.isBetaOperator(bob.address)).to.be.true + expect(await pool.isBetaOperator(carol.address)).to.be.false + }) + + it("should emit BetaOperatorsAdded event", async () => { + await expect(tx) + .to.emit(pool, "BetaOperatorsAdded") + .withArgs([alice.address, bob.address]) + }) + }) + }) + + describe("transferChaosnetMaestroRole", () => { + context("when called by third party", () => { + it("should revert", async () => { + await expect( + pool + .connect(thirdParty) + .transferChaosnetMaestroRole(thirdParty.address), + ).to.be.revertedWith("Not the chaosnet maestro") + }) + }) + + context("when called by the current chaosnet maestro", () => { + context("when called with the new address set to zero", () => { + it("should revert", async () => { + await expect( + pool + .connect(chaosnetMaestro) + .transferChaosnetMaestroRole(ZERO_ADDRESS), + ).to.be.revertedWith("New chaosnet maestro must not be zero address") + }) + }) + + context("when called with non-zero new address", () => { + let tx + + beforeEach(async () => { + tx = await pool + .connect(chaosnetMaestro) + .transferChaosnetMaestroRole(alice.address) + }) + + it("should transfer the role", async () => { + expect(await pool.chaosnetMaestro()).to.equal(alice.address) + }) + + it("should emit ChaosnetMaestroRoleTransferred event", async () => { + await expect(tx) + .to.emit(pool, "ChaosnetMaestroRoleTransferred") + .withArgs(chaosnetMaestro.address, alice.address) + }) + }) + }) + }) + + describe("deactivate chaosnet", () => { + context("when called by third party", () => { + it("should revert", async () => { + await expect( + pool.connect(thirdParty).deactivateChaosnet(), + ).to.be.revertedWith("Not the chaosnet maestro") + }) + }) + + context("when called by the chaosnet maestro", () => { + context("when chaosnet is not active", () => { + beforeEach(async () => { + await pool.connect(chaosnetMaestro).deactivateChaosnet() + }) + + it("should revert", async () => { + await expect( + pool.connect(chaosnetMaestro).deactivateChaosnet(), + ).to.be.revertedWith("Chaosnet is not active") + }) + }) + + context("when chaosnet is active", () => { + let tx + + beforeEach(async () => { + tx = await pool.connect(chaosnetMaestro).deactivateChaosnet() + }) + + it("should deactivate chaosnet", async () => { + expect(await pool.isChaosnetActive()).to.be.false + }) + + it("should emit ChaosnetDeactivated event", async () => { + await expect(tx).to.emit(pool, "ChaosnetDeactivated") + }) + }) + }) + }) }) From 1ad78dfa8babd0980ac9ef891d19b2e360bd0699 Mon Sep 17 00:00:00 2001 From: Piotr Dyraga Date: Mon, 19 Sep 2022 18:48:23 +0200 Subject: [PATCH 2/4] Renamed chaosnet maestro role to chaosnet owner --- contracts/Chaosnet.sol | 42 +++++++++++++-------------- test/sortitionPoolTest.js | 60 +++++++++++++++++++-------------------- 2 files changed, 51 insertions(+), 51 deletions(-) diff --git a/contracts/Chaosnet.sol b/contracts/Chaosnet.sol index bdf5523..74d8b53 100644 --- a/contracts/Chaosnet.sol +++ b/contracts/Chaosnet.sol @@ -16,32 +16,32 @@ contract Chaosnet { mapping(address => bool) public isBetaOperator; /// @notice Address controlling chaosnet status and beta operator addresses. - address public chaosnetMaestro; + address public chaosnetOwner; event BetaOperatorsAdded(address[] operators); - event ChaosnetMaestroRoleTransferred( - address oldChaosnetMaestro, - address newChaosnetMaestro + event ChaosnetOwnerRoleTransferred( + address oldChaosnetOwner, + address newChaosnetOwner ); event ChaosnetDeactivated(); constructor() { - _transferChaosnetMaestro(msg.sender); + _transferChaosnetOwner(msg.sender); isChaosnetActive = true; } - modifier onlyChaosnetMaestro() { - require(msg.sender == chaosnetMaestro, "Not the chaosnet maestro"); + modifier onlyChaosnetOwner() { + require(msg.sender == chaosnetOwner, "Not the chaosnet owner"); _; } /// @notice Adds beta operator to chaosnet. Can be called only by the - /// chaosnet maestro. + /// chaosnet owner. function addBetaOperators(address[] calldata operators) public - onlyChaosnetMaestro + onlyChaosnetOwner { for (uint256 i = 0; i < operators.length; i++) { isBetaOperator[operators[i]] = true; @@ -51,28 +51,28 @@ contract Chaosnet { } /// @notice Deactivates the chaosnet. Can be called only by the chaosnet - /// maestro. Once deactivated chaosnet can not be activated again. - function deactivateChaosnet() public onlyChaosnetMaestro { + /// owner. Once deactivated chaosnet can not be activated again. + function deactivateChaosnet() public onlyChaosnetOwner { require(isChaosnetActive, "Chaosnet is not active"); isChaosnetActive = false; emit ChaosnetDeactivated(); } - /// @notice Transfers the chaosnet maestro role to another non-zero address. - function transferChaosnetMaestroRole(address newChaosnetMaestro) + /// @notice Transfers the chaosnet owner role to another non-zero address. + function transferChaosnetOwnerRole(address newChaosnetOwner) public - onlyChaosnetMaestro + onlyChaosnetOwner { require( - newChaosnetMaestro != address(0), - "New chaosnet maestro must not be zero address" + newChaosnetOwner != address(0), + "New chaosnet owner must not be zero address" ); - _transferChaosnetMaestro(newChaosnetMaestro); + _transferChaosnetOwner(newChaosnetOwner); } - function _transferChaosnetMaestro(address newChaosnetMaestro) internal { - address oldChaosnetMaestro = chaosnetMaestro; - chaosnetMaestro = newChaosnetMaestro; - emit ChaosnetMaestroRoleTransferred(oldChaosnetMaestro, newChaosnetMaestro); + function _transferChaosnetOwner(address newChaosnetOwner) internal { + address oldChaosnetOwner = chaosnetOwner; + chaosnetOwner = newChaosnetOwner; + emit ChaosnetOwnerRoleTransferred(oldChaosnetOwner, newChaosnetOwner); } } diff --git a/test/sortitionPoolTest.js b/test/sortitionPoolTest.js index 177a9f2..3b7cace 100644 --- a/test/sortitionPoolTest.js +++ b/test/sortitionPoolTest.js @@ -24,7 +24,7 @@ describe("SortitionPool", () => { ;[ deployer, owner, - chaosnetMaestro, + chaosnetOwner, alice, bob, carol, @@ -44,7 +44,7 @@ describe("SortitionPool", () => { await pool.connect(deployer).transferOwnership(owner.address) await pool .connect(deployer) - .transferChaosnetMaestroRole(chaosnetMaestro.address) + .transferChaosnetOwnerRole(chaosnetOwner.address) }) describe("constructor", () => { @@ -117,7 +117,7 @@ describe("SortitionPool", () => { context("when operator is beta operator", () => { beforeEach(async () => { await pool - .connect(chaosnetMaestro) + .connect(chaosnetOwner) .addBetaOperators([alice.address]) await pool .connect(owner) @@ -144,7 +144,7 @@ describe("SortitionPool", () => { context("when operator is beta operator", () => { beforeEach(async () => { await pool - .connect(chaosnetMaestro) + .connect(chaosnetOwner) .addBetaOperators([alice.address]) }) @@ -161,7 +161,7 @@ describe("SortitionPool", () => { context("when chaosnet is not active", () => { beforeEach(async () => { - await pool.connect(chaosnetMaestro).deactivateChaosnet() + await pool.connect(chaosnetOwner).deactivateChaosnet() }) context("when operator is eligible", () => { @@ -214,7 +214,7 @@ describe("SortitionPool", () => { describe("updateOperatorStatus", () => { beforeEach(async () => { - await pool.connect(chaosnetMaestro).deactivateChaosnet() + await pool.connect(chaosnetOwner).deactivateChaosnet() await pool.connect(owner).insertOperator(alice.address, 2000) }) @@ -270,7 +270,7 @@ describe("SortitionPool", () => { describe("selectGroup", async () => { beforeEach(async () => { - await pool.connect(chaosnetMaestro).deactivateChaosnet() + await pool.connect(chaosnetOwner).deactivateChaosnet() }) context("when sortition pool is locked", () => { @@ -348,7 +348,7 @@ describe("SortitionPool", () => { describe("pool rewards", async () => { beforeEach(async () => { - await pool.connect(chaosnetMaestro).deactivateChaosnet() + await pool.connect(chaosnetOwner).deactivateChaosnet() }) async function withdrawRewards( @@ -534,7 +534,7 @@ describe("SortitionPool", () => { it("should revert", async () => { await expect( pool.connect(thirdParty).addBetaOperators([alice.address]), - ).to.be.revertedWith("Not the chaosnet maestro") + ).to.be.revertedWith("Not the chaosnet owner") }) }) @@ -542,16 +542,16 @@ describe("SortitionPool", () => { it("should revert", async () => { await expect( pool.connect(alice).addBetaOperators([alice.address]), - ).to.be.revertedWith("Not the chaosnet maestro") + ).to.be.revertedWith("Not the chaosnet owner") }) }) - context("when called by the chaosnet maestro", () => { + context("when called by the chaosnet owner", () => { let tx beforeEach(async () => { tx = await pool - .connect(chaosnetMaestro) + .connect(chaosnetOwner) .addBetaOperators([alice.address, bob.address]) }) @@ -569,25 +569,25 @@ describe("SortitionPool", () => { }) }) - describe("transferChaosnetMaestroRole", () => { + describe("transferChaosnetOwnerRole", () => { context("when called by third party", () => { it("should revert", async () => { await expect( pool .connect(thirdParty) - .transferChaosnetMaestroRole(thirdParty.address), - ).to.be.revertedWith("Not the chaosnet maestro") + .transferChaosnetOwnerRole(thirdParty.address), + ).to.be.revertedWith("Not the chaosnet owner") }) }) - context("when called by the current chaosnet maestro", () => { + context("when called by the current chaosnet owner", () => { context("when called with the new address set to zero", () => { it("should revert", async () => { await expect( pool - .connect(chaosnetMaestro) - .transferChaosnetMaestroRole(ZERO_ADDRESS), - ).to.be.revertedWith("New chaosnet maestro must not be zero address") + .connect(chaosnetOwner) + .transferChaosnetOwnerRole(ZERO_ADDRESS), + ).to.be.revertedWith("New chaosnet owner must not be zero address") }) }) @@ -596,18 +596,18 @@ describe("SortitionPool", () => { beforeEach(async () => { tx = await pool - .connect(chaosnetMaestro) - .transferChaosnetMaestroRole(alice.address) + .connect(chaosnetOwner) + .transferChaosnetOwnerRole(alice.address) }) it("should transfer the role", async () => { - expect(await pool.chaosnetMaestro()).to.equal(alice.address) + expect(await pool.chaosnetOwner()).to.equal(alice.address) }) - it("should emit ChaosnetMaestroRoleTransferred event", async () => { + it("should emit ChaosnetOwnerRoleTransferred event", async () => { await expect(tx) - .to.emit(pool, "ChaosnetMaestroRoleTransferred") - .withArgs(chaosnetMaestro.address, alice.address) + .to.emit(pool, "ChaosnetOwnerRoleTransferred") + .withArgs(chaosnetOwner.address, alice.address) }) }) }) @@ -618,19 +618,19 @@ describe("SortitionPool", () => { it("should revert", async () => { await expect( pool.connect(thirdParty).deactivateChaosnet(), - ).to.be.revertedWith("Not the chaosnet maestro") + ).to.be.revertedWith("Not the chaosnet owner") }) }) - context("when called by the chaosnet maestro", () => { + context("when called by the chaosnet owner", () => { context("when chaosnet is not active", () => { beforeEach(async () => { - await pool.connect(chaosnetMaestro).deactivateChaosnet() + await pool.connect(chaosnetOwner).deactivateChaosnet() }) it("should revert", async () => { await expect( - pool.connect(chaosnetMaestro).deactivateChaosnet(), + pool.connect(chaosnetOwner).deactivateChaosnet(), ).to.be.revertedWith("Chaosnet is not active") }) }) @@ -639,7 +639,7 @@ describe("SortitionPool", () => { let tx beforeEach(async () => { - tx = await pool.connect(chaosnetMaestro).deactivateChaosnet() + tx = await pool.connect(chaosnetOwner).deactivateChaosnet() }) it("should deactivate chaosnet", async () => { From 8784ec198c93f5e0dd1275edc35a0615433e959b Mon Sep 17 00:00:00 2001 From: Piotr Dyraga Date: Tue, 20 Sep 2022 09:17:57 +0200 Subject: [PATCH 3/4] Do not allow adding beta operators when chaosnet is not active It does not hurt to add beta operators after deactivating chaosnet but for the consistency of events, we should require adding beta operators is possible only on the active chaosnet. This way, we are sure all beta operators actually participated in the chaosnet. --- contracts/Chaosnet.sol | 11 ++++++--- test/sortitionPoolTest.js | 48 +++++++++++++++++++++++++-------------- 2 files changed, 39 insertions(+), 20 deletions(-) diff --git a/contracts/Chaosnet.sol b/contracts/Chaosnet.sol index 74d8b53..47e1880 100644 --- a/contracts/Chaosnet.sol +++ b/contracts/Chaosnet.sol @@ -37,10 +37,16 @@ contract Chaosnet { _; } + modifier onlyOnChaosnet() { + require(isChaosnetActive, "Chaosnet is not active"); + _; + } + /// @notice Adds beta operator to chaosnet. Can be called only by the - /// chaosnet owner. + /// chaosnet owner when the chaosnet is active. function addBetaOperators(address[] calldata operators) public + onlyOnChaosnet onlyChaosnetOwner { for (uint256 i = 0; i < operators.length; i++) { @@ -52,8 +58,7 @@ contract Chaosnet { /// @notice Deactivates the chaosnet. Can be called only by the chaosnet /// owner. Once deactivated chaosnet can not be activated again. - function deactivateChaosnet() public onlyChaosnetOwner { - require(isChaosnetActive, "Chaosnet is not active"); + function deactivateChaosnet() public onlyOnChaosnet onlyChaosnetOwner { isChaosnetActive = false; emit ChaosnetDeactivated(); } diff --git a/test/sortitionPoolTest.js b/test/sortitionPoolTest.js index 3b7cace..b28ddf5 100644 --- a/test/sortitionPoolTest.js +++ b/test/sortitionPoolTest.js @@ -547,24 +547,40 @@ describe("SortitionPool", () => { }) context("when called by the chaosnet owner", () => { - let tx + context("when chaosnet is not active", () => { + beforeEach(async () => { + await pool.connect(chaosnetOwner).deactivateChaosnet() + }) - beforeEach(async () => { - tx = await pool - .connect(chaosnetOwner) - .addBetaOperators([alice.address, bob.address]) + it("should revert", async () => { + await expect( + pool + .connect(chaosnetOwner) + .addBetaOperators([alice.address, bob.address]), + ).to.be.revertedWith("Chaosnet is not active") + }) }) - it("should set selected operators as beta operators", async () => { - expect(await pool.isBetaOperator(alice.address)).to.be.true - expect(await pool.isBetaOperator(bob.address)).to.be.true - expect(await pool.isBetaOperator(carol.address)).to.be.false - }) + context("when chaosnet is active", () => { + let tx + + beforeEach(async () => { + tx = await pool + .connect(chaosnetOwner) + .addBetaOperators([alice.address, bob.address]) + }) - it("should emit BetaOperatorsAdded event", async () => { - await expect(tx) - .to.emit(pool, "BetaOperatorsAdded") - .withArgs([alice.address, bob.address]) + it("should set selected operators as beta operators", async () => { + expect(await pool.isBetaOperator(alice.address)).to.be.true + expect(await pool.isBetaOperator(bob.address)).to.be.true + expect(await pool.isBetaOperator(carol.address)).to.be.false + }) + + it("should emit BetaOperatorsAdded event", async () => { + await expect(tx) + .to.emit(pool, "BetaOperatorsAdded") + .withArgs([alice.address, bob.address]) + }) }) }) }) @@ -584,9 +600,7 @@ describe("SortitionPool", () => { context("when called with the new address set to zero", () => { it("should revert", async () => { await expect( - pool - .connect(chaosnetOwner) - .transferChaosnetOwnerRole(ZERO_ADDRESS), + pool.connect(chaosnetOwner).transferChaosnetOwnerRole(ZERO_ADDRESS), ).to.be.revertedWith("New chaosnet owner must not be zero address") }) }) From 82d14dfbc04155f3dc01dcfdbd5f638fda27fc39 Mon Sep 17 00:00:00 2001 From: Piotr Dyraga Date: Thu, 22 Sep 2022 12:09:39 +0200 Subject: [PATCH 4/4] Improved documentation of Chaosnet.addBetaOperators Made it clear that operator once added can not be removed. --- contracts/Chaosnet.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contracts/Chaosnet.sol b/contracts/Chaosnet.sol index 47e1880..90d1b0d 100644 --- a/contracts/Chaosnet.sol +++ b/contracts/Chaosnet.sol @@ -43,7 +43,8 @@ contract Chaosnet { } /// @notice Adds beta operator to chaosnet. Can be called only by the - /// chaosnet owner when the chaosnet is active. + /// chaosnet owner when the chaosnet is active. Once the operator is added + /// as a beta operator, it can not be removed. function addBetaOperators(address[] calldata operators) public onlyOnChaosnet