From 241e2f035375002bc0c021f5d30a77fb91d15837 Mon Sep 17 00:00:00 2001 From: Thomas Nguy Date: Thu, 12 Jan 2023 11:45:57 +0900 Subject: [PATCH] add more flexibility for control --- solidity/contracts/Gravity.sol | 33 +++++++++++-------- solidity/test/pause.ts | 21 +++++++----- ...erAdmin.ts.bak => transferRelayerAdmin.ts} | 18 +++++----- 3 files changed, 41 insertions(+), 31 deletions(-) rename solidity/test/{transferRelayerAdmin.ts.bak => transferRelayerAdmin.ts} (67%) diff --git a/solidity/contracts/Gravity.sol b/solidity/contracts/Gravity.sol index 0d4e0d97a..0d117eed7 100644 --- a/solidity/contracts/Gravity.sol +++ b/solidity/contracts/Gravity.sol @@ -85,7 +85,8 @@ contract Gravity is ReentrancyGuard, AccessControl, Pausable, Ownable { using SafeERC20 for IERC20; bytes32 public constant RELAYER = keccak256("RELAYER"); - bytes32 public constant RELAYER_ADMIN = keccak256("RELAYER_ADMIN"); + bytes32 public constant CONTROL = keccak256("CONTROL"); + bytes32 public constant ADMIN = keccak256("ADMIN"); // These are updated often bytes32 public state_lastValsetCheckpoint; @@ -128,6 +129,11 @@ contract Gravity is ReentrancyGuard, AccessControl, Pausable, Ownable { _; } + modifier checkControl() { + _checkRole(CONTROL, msg.sender); + _; + } + // TransactionBatchExecutedEvent and SendToCosmosEvent both include the field _eventNonce. // This is incremented every time one of these events is emitted. It is checked by the // Cosmos module to ensure that all events are received in order, and that none are lost. @@ -686,7 +692,7 @@ contract Gravity is ReentrancyGuard, AccessControl, Pausable, Ownable { string calldata _name, string calldata _symbol, uint8 _decimals - ) external { + ) external whenNotPaused checkWhiteList { // Deploy an ERC20 with entire supply granted to Gravity.sol CosmosERC20 erc20 = new CosmosERC20(address(this), _name, _symbol, _decimals); @@ -709,7 +715,7 @@ contract Gravity is ReentrancyGuard, AccessControl, Pausable, Ownable { * Only owner * pause will deactivate contract functionalities */ - function pause() public onlyOwner { + function pause() public checkControl { _pause(); } @@ -717,7 +723,7 @@ contract Gravity is ReentrancyGuard, AccessControl, Pausable, Ownable { * Only owner * unpause will re activate contract functionalities */ - function unpause() public onlyOwner { + function unpause() public checkControl { _unpause(); } @@ -765,16 +771,16 @@ contract Gravity is ReentrancyGuard, AccessControl, Pausable, Ownable { function setAnyoneCanRelay ( bool _anyoneCanRelay - ) public onlyRole(RELAYER_ADMIN) { + ) public onlyRole(ADMIN) { anyoneCanRelay = _anyoneCanRelay; emit AnyoneCanRelay(anyoneCanRelay); } - function transferRelayerAdmin ( + function transferAdmin ( address _newAdmin - ) public onlyRole(RELAYER_ADMIN) { - grantRole(RELAYER_ADMIN, _newAdmin); - revokeRole(RELAYER_ADMIN, msg.sender); + ) public onlyRole(ADMIN) { + grantRole(ADMIN, _newAdmin); + revokeRole(ADMIN, msg.sender); } constructor( @@ -785,7 +791,7 @@ contract Gravity is ReentrancyGuard, AccessControl, Pausable, Ownable { // The validator set address[] memory _validators, uint256[] memory _powers, - address relayerAdmin + address _admin ) { // CHECKS @@ -824,9 +830,10 @@ contract Gravity is ReentrancyGuard, AccessControl, Pausable, Ownable { // ACL - _setupRole(RELAYER_ADMIN, relayerAdmin); - _setRoleAdmin(RELAYER, RELAYER_ADMIN); - _setRoleAdmin(RELAYER_ADMIN, RELAYER_ADMIN); + _setupRole(ADMIN, _admin); + _setRoleAdmin(RELAYER, ADMIN); + _setRoleAdmin(CONTROL, ADMIN); + _setRoleAdmin(ADMIN, ADMIN); // LOGS diff --git a/solidity/test/pause.ts b/solidity/test/pause.ts index ef05efb07..0c86ce410 100644 --- a/solidity/test/pause.ts +++ b/solidity/test/pause.ts @@ -17,7 +17,7 @@ const { expect } = chai; async function runTest(opts: { isRelayer?: boolean; - isOwner?: boolean; + isControl?: boolean; pause?: boolean; unpause?: boolean; }) { @@ -37,8 +37,11 @@ async function runTest(opts: { checkpoint: deployCheckpoint } = await deployContracts(gravityId, validators, powers, powerThreshold); - if (opts.isOwner) { - await gravity.transferOwnership(signers[1].address); + if (opts.isControl) { + await gravity.grantRole( + await gravity.CONTROL(), + signers[1].address, + ); } if (opts.pause) { @@ -51,21 +54,21 @@ async function runTest(opts: { } describe("pause tests", function () { - it("non-owner cannot call pause()", async function () { + it("non control admin cannot call pause()", async function () { await expect(runTest({ pause: true - })).to.be.revertedWith("Ownable: caller is not the owner"); + })).to.be.revertedWith("AccessControl: account 0xead9c93b79ae7c1591b1fb5323bd777e86e150d4 is missing role 0xbdded29a54e6a5d6169bedea55373b06f57e35d7b0f67ac187565b435e2cc943"); }); - it("non-owner cannot call unpause()", async function () { + it("non control admin call unpause()", async function () { await expect(runTest({ unpause: true - })).to.be.revertedWith("Ownable: caller is not the owner"); + })).to.be.revertedWith("AccessControl: account 0xead9c93b79ae7c1591b1fb5323bd777e86e150d4 is missing role 0xbdded29a54e6a5d6169bedea55373b06f57e35d7b0f67ac187565b435e2cc943"); }); - it("owner can call pause() and unpause()", async function () { + it("control admin can call pause() and unpause()", async function () { await runTest({ - isOwner: true, + isControl: true, pause: true, unpause: true, }); diff --git a/solidity/test/transferRelayerAdmin.ts.bak b/solidity/test/transferRelayerAdmin.ts similarity index 67% rename from solidity/test/transferRelayerAdmin.ts.bak rename to solidity/test/transferRelayerAdmin.ts index df469dced..6260315ae 100644 --- a/solidity/test/transferRelayerAdmin.ts.bak +++ b/solidity/test/transferRelayerAdmin.ts @@ -16,7 +16,7 @@ const { expect } = chai; async function runTest(opts: { - isRelayerAdmin?: boolean; + isAdmin?: boolean; }) { @@ -35,22 +35,22 @@ async function runTest(opts: { } = await deployContracts(gravityId, validators, powers, powerThreshold); - if (!opts.isRelayerAdmin) { - await gravity.connect(signers[1]).transferRelayerAdmin(signers[1].address); + if (!opts.isAdmin) { + await gravity.connect(signers[1]).transferAdmin(signers[1].address); } } describe("transferRelayerAdmin tests", function () { - it("Only relayerAdmin can call transferRelayerAdmin", async function () { + it("non admin cannot call transferAdmin", async function () { await expect(runTest({ - isRelayerAdmin: false - })).to.be.revertedWith("Ownable: caller is not the owner"); + isAdmin: false + })).to.be.revertedWith("AccessControl: account 0xead9c93b79ae7c1591b1fb5323bd777e86e150d4 is missing role 0xdf8b4c520ffe197c5343c6f5aec59570151ef9a492f2c624fd45ddde6135ec42"); }); - it("old relayAdmin have no role after transferred", async function () { + it("admin can call transferAdmin", async function () { await expect(runTest({ - isRelayerAdmin: true - })).to.be.revertedWith("Ownable: caller is not the owner"); + isAdmin: true + })) }); }); \ No newline at end of file