Skip to content

Commit

Permalink
fix: pause
Browse files Browse the repository at this point in the history
- rename backoffice role to pauser role
- add whenNotPaused to cancel and matchOrderFrom
  • Loading branch information
adjisb committed Oct 3, 2023
1 parent 3beceaf commit 2057238
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 16 deletions.
10 changes: 5 additions & 5 deletions packages/marketplace/contracts/exchange/Exchange.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -144,7 +144,7 @@ contract Exchange is
}

// @notice Triggers stopped state.
function pause() external onlyRole(BACKOFFICE_ROLE) {
function pause() external onlyRole(PAUSER_ROLE) {
_pause();
}

Expand Down
3 changes: 3 additions & 0 deletions packages/marketplace/test/exchange/Exchange.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {});
});
20 changes: 11 additions & 9 deletions packages/marketplace/test/exchange/ExchangeSettings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions packages/marketplace/test/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

1 comment on commit 2057238

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverage for this commit

81.47%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
packages/marketplace/contracts/exchange
   AssetMatcher.sol100%100%100%100%
   Exchange.sol80.28%71.88%87.50%86.96%101, 176, 66, 82, 92, 92, 92, 92–93, 93, 93–94
   ExchangeCore.sol86.17%65.63%100%96.08%116, 140–141, 141, 141, 144, 205, 207, 211, 232–233, 250, 76
   OrderValidator.sol71.70%58.33%100%79.17%38, 67–68, 68, 68, 72, 83–84, 89, 91, 91, 91, 91–92, 94
   WhiteList.sol97.73%93.75%100%100%57
packages/marketplace/contracts/exchange/libraries
   LibFill.sol60.87%33.33%75%69.23%32–33, 58, 68–69, 69, 69–70
   LibMath.sol27.50%18.75%50%30%100–103, 17–18, 33–34, 50, 50, 50–51, 72, 72, 72–73, 75, 88, 88, 88–89, 93, 93, 93, 93, 93, 97
packages/marketplace/contracts/interfaces
   IAssetMatcher.sol100%100%100%100%
   IOrderValidator.sol100%100%100%100%
   IRoyaltiesProvider.sol100%100%100%100%
   IWhiteList.sol100%100%100%100%
packages/marketplace/contracts/lib-asset
   LibAsset.sol100%100%100%100%
packages/marketplace/contracts/lib-bp
   BpLibrary.sol100%100%100%100%
packages/marketplace/contracts/lib-order
   LibOrder.sol73.33%50%100%100%64, 64, 66, 66
packages/marketplace/contracts/lib-part
   LibPart.sol0%100%0%0%21
packages/marketplace/contracts/royalties
   IERC2981.sol100%100%100%100%
   LibRoyalties2981.sol78.57%50%100%88.89%19–20, 23
packages/marketplace/contracts/royalties-registry
   IMultiRoyaltyRecipients.sol100%100%100%100%
   RoyaltiesRegistry.sol79.20%79.41%100%75%166–167, 170–171, 212, 216, 232–233, 236–244, 247, 247, 247–248, 250, 256, 259, 276, 60
packages/marketplace/contracts/transfer-manager
   TransferExecutor.sol75.76%64.29%100%81.25%46–48, 55–56, 60, 63–64
   TransferManager.sol82.58%71.43%100%87.36%110, 114–115, 195, 202–203, 203, 203–204, 208, 248, 265, 269–271, 271, 271–273, 278–279, 302, 306–307, 62, 82–83
packages/marketplace/contracts/transfer-manager/interfaces
   IRoyaltyUGC.sol100%100%100%100%
   ITransferExecutor.sol100%100%100%100%
   ITransferManager.sol100%100%100%100%
packages/marketplace/contracts/transfer-manager/lib
   LibDeal.sol100%100%100%100%
   LibFeeSide.sol44.44%37.50%100%44.44%21, 24, 24, 24–25, 27, 27, 27–28, 30

Please sign in to comment.