From 2057238d378b8e2613beaa5c718233dff9e4b327 Mon Sep 17 00:00:00 2001 From: Andres Adjimann Date: Tue, 3 Oct 2023 10:54:58 -0300 Subject: [PATCH] fix: pause - rename backoffice role to pauser role - add whenNotPaused to cancel and matchOrderFrom --- .../contracts/exchange/Exchange.sol | 10 +++++----- .../test/exchange/Exchange.test.ts | 3 +++ .../test/exchange/ExchangeSettings.test.ts | 20 ++++++++++--------- packages/marketplace/test/fixtures.ts | 4 ++-- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/packages/marketplace/contracts/exchange/Exchange.sol b/packages/marketplace/contracts/exchange/Exchange.sol index 2625f3e66a..a2d59b8dfa 100644 --- a/packages/marketplace/contracts/exchange/Exchange.sol +++ b/packages/marketplace/contracts/exchange/Exchange.sol @@ -35,8 +35,8 @@ contract Exchange is bytes32 public constant EXCHANGE_ADMIN_ROLE = keccak256("EXCHANGE_ADMIN_ROLE"); /// @notice role business addresses that can react on an emergency, pause/unpause - /// @return hash for BACKOFFICE_ROLE - bytes32 public constant BACKOFFICE_ROLE = keccak256("BACKOFFICE_ROLE"); + /// @return hash for PAUSER_ROLE + bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE"); /// @dev this protects the implementation contract from being initialized. /// @custom:oz-upgrades-unsafe-allow constructor @@ -89,7 +89,7 @@ contract Exchange is function matchOrdersFrom( address sender, ExchangeMatch[] calldata matchedOrders - ) external onlyRole(ERC1776_OPERATOR_ROLE) { + ) external onlyRole(ERC1776_OPERATOR_ROLE) whenNotPaused { require(sender != address(0), "invalid sender"); _matchOrders(sender, matchedOrders); } @@ -98,7 +98,7 @@ contract Exchange is /// @param order to be canceled /// @param orderKeyHash used as a checksum to avoid mistakes in the values of order /// @dev require msg sender to be order maker and salt different from 0 - function cancel(LibOrder.Order calldata order, bytes32 orderKeyHash) external { + function cancel(LibOrder.Order calldata order, bytes32 orderKeyHash) external whenNotPaused { require(_msgSender() == order.maker, "ExchangeCore: not maker"); _cancel(order, orderKeyHash); } @@ -144,7 +144,7 @@ contract Exchange is } // @notice Triggers stopped state. - function pause() external onlyRole(BACKOFFICE_ROLE) { + function pause() external onlyRole(PAUSER_ROLE) { _pause(); } diff --git a/packages/marketplace/test/exchange/Exchange.test.ts b/packages/marketplace/test/exchange/Exchange.test.ts index 786707f64e..a28ebf5b69 100644 --- a/packages/marketplace/test/exchange/Exchange.test.ts +++ b/packages/marketplace/test/exchange/Exchange.test.ts @@ -1627,4 +1627,7 @@ describe('Exchange.sol', function () { expect(await ERC721Contract.ownerOf(345)).to.be.equal(taker.address); }); }); + // TODO + // describe("test match from", function () {}); + // describe("test on pause", function () {}); }); diff --git a/packages/marketplace/test/exchange/ExchangeSettings.test.ts b/packages/marketplace/test/exchange/ExchangeSettings.test.ts index fa024b0cc3..4a127b5c65 100644 --- a/packages/marketplace/test/exchange/ExchangeSettings.test.ts +++ b/packages/marketplace/test/exchange/ExchangeSettings.test.ts @@ -66,18 +66,20 @@ describe('Exchange.sol settings', function () { ); }); }); - describe('backoffice admin', function () { + describe('pauser role', function () { it('should not pause if caller is not in the role', async function () { - const {ExchangeContractAsUser, BACKOFFICE_ROLE, user} = - await loadFixture(deployFixtures); + const {ExchangeContractAsUser, PAUSER_ROLE, user} = await loadFixture( + deployFixtures + ); await expect(ExchangeContractAsUser.pause()).to.be.revertedWith( - `AccessControl: account ${user.address.toLowerCase()} is missing role ${BACKOFFICE_ROLE}` + `AccessControl: account ${user.address.toLowerCase()} is missing role ${PAUSER_ROLE}` ); }); it('should be able to pause the contract', async function () { - const {ExchangeContractAsAdmin, BACKOFFICE_ROLE, user2} = - await loadFixture(deployFixtures); - await ExchangeContractAsAdmin.grantRole(BACKOFFICE_ROLE, user2); + const {ExchangeContractAsAdmin, PAUSER_ROLE, user2} = await loadFixture( + deployFixtures + ); + await ExchangeContractAsAdmin.grantRole(PAUSER_ROLE, user2); expect(await ExchangeContractAsAdmin.paused()).to.be.false; await ExchangeContractAsAdmin.connect(user2).pause(); expect(await ExchangeContractAsAdmin.paused()).to.be.true; @@ -128,12 +130,12 @@ describe('Exchange.sol settings', function () { const { ExchangeContractAsAdmin, ExchangeContractAsUser, - BACKOFFICE_ROLE, + PAUSER_ROLE, EXCHANGE_ADMIN_ROLE, user, user2, } = await loadFixture(deployFixtures); - await ExchangeContractAsAdmin.grantRole(BACKOFFICE_ROLE, user2); + await ExchangeContractAsAdmin.grantRole(PAUSER_ROLE, user2); await ExchangeContractAsAdmin.grantRole(EXCHANGE_ADMIN_ROLE, user); await ExchangeContractAsAdmin.connect(user2).pause(); diff --git a/packages/marketplace/test/fixtures.ts b/packages/marketplace/test/fixtures.ts index 027ac41d89..983c4647d1 100644 --- a/packages/marketplace/test/fixtures.ts +++ b/packages/marketplace/test/fixtures.ts @@ -100,13 +100,13 @@ async function deploy() { const EXCHANGE_ADMIN_ROLE = await ExchangeContractAsAdmin.EXCHANGE_ADMIN_ROLE(); const DEFAULT_ADMIN_ROLE = await ExchangeContractAsAdmin.DEFAULT_ADMIN_ROLE(); - const BACKOFFICE_ROLE = await ExchangeContractAsAdmin.BACKOFFICE_ROLE(); + const PAUSER_ROLE = await ExchangeContractAsAdmin.PAUSER_ROLE(); return { protocolFeePrimary, protocolFeeSecondary, EXCHANGE_ADMIN_ROLE, DEFAULT_ADMIN_ROLE, - BACKOFFICE_ROLE, + PAUSER_ROLE, assetMatcherAsDeployer, assetMatcherAsUser, ExchangeContractAsDeployer,