diff --git a/contracts/trading/seaport/zones/immutable-signed-zone/v2/ImmutableSignedZoneV2.sol b/contracts/trading/seaport/zones/immutable-signed-zone/v2/ImmutableSignedZoneV2.sol index a49906f0..b22dd795 100644 --- a/contracts/trading/seaport/zones/immutable-signed-zone/v2/ImmutableSignedZoneV2.sol +++ b/contracts/trading/seaport/zones/immutable-signed-zone/v2/ImmutableSignedZoneV2.sol @@ -4,13 +4,14 @@ // solhint-disable-next-line compiler-version pragma solidity ^0.8.20; -import {ZoneInterface} from "seaport/contracts/interfaces/ZoneInterface.sol"; -import {ZoneParameters, Schema, ReceivedItem} from "seaport-types/src/lib/ConsiderationStructs.sol"; import {AccessControlEnumerable} from "openzeppelin-contracts-5.0.2/access/extensions/AccessControlEnumerable.sol"; import {ECDSA} from "openzeppelin-contracts-5.0.2/utils/cryptography/ECDSA.sol"; import {MessageHashUtils} from "openzeppelin-contracts-5.0.2/utils/cryptography/MessageHashUtils.sol"; import {ERC165} from "openzeppelin-contracts-5.0.2/utils/introspection/ERC165.sol"; import {Math} from "openzeppelin-contracts-5.0.2/utils/math/Math.sol"; +import {ZoneInterface} from "seaport/contracts/interfaces/ZoneInterface.sol"; +import {ZoneParameters, Schema, ReceivedItem} from "seaport-types/src/lib/ConsiderationStructs.sol"; +import {ZoneAccessControl} from "./ZoneAccessControl.sol"; import {SIP5Interface} from "./interfaces/SIP5Interface.sol"; import {SIP6Interface} from "./interfaces/SIP6Interface.sol"; import {SIP7Interface} from "./interfaces/SIP7Interface.sol"; @@ -24,11 +25,11 @@ import {SIP7Interface} from "./interfaces/SIP7Interface.sol"; */ contract ImmutableSignedZoneV2 is ERC165, + ZoneAccessControl, ZoneInterface, SIP5Interface, SIP6Interface, - SIP7Interface, - AccessControlEnumerable + SIP7Interface { /// @dev The EIP-712 domain type hash. bytes32 private constant _EIP_712_DOMAIN_TYPEHASH = keccak256( @@ -82,7 +83,9 @@ contract ImmutableSignedZoneV2 is * @param owner The address of the owner of this contract. Specified in the * constructor to be CREATE2 / CREATE3 compatible. */ - constructor(string memory zoneName, string memory apiEndpoint, string memory documentationURI, address owner) { + constructor(string memory zoneName, string memory apiEndpoint, string memory documentationURI, address owner) + ZoneAccessControl(owner) + { // Set the zone name. _ZONE_NAME = zoneName; @@ -100,9 +103,6 @@ contract ImmutableSignedZoneV2 is // Emit an event to signal a SIP-5 contract has been deployed. emit SeaportCompatibleContractDeployed(); - - // Grant admin role to the specified owner. - _grantRole(DEFAULT_ADMIN_ROLE, owner); } /** @@ -110,7 +110,7 @@ contract ImmutableSignedZoneV2 is * * @param signer The new signer address to add. */ - function addSigner(address signer) external override onlyRole(DEFAULT_ADMIN_ROLE) { + function addSigner(address signer) external override onlyRole(ZONE_MANAGER_ROLE) { // Do not allow the zero address to be added as a signer. if (signer == address(0)) { revert SignerCannotBeZeroAddress(); @@ -140,7 +140,7 @@ contract ImmutableSignedZoneV2 is * * @param signer The signer address to remove. */ - function removeSigner(address signer) external override onlyRole(DEFAULT_ADMIN_ROLE) { + function removeSigner(address signer) external override onlyRole(ZONE_MANAGER_ROLE) { // Revert if the signer is not active. if (!_signers[signer].active) { revert SignerNotActive(signer); @@ -158,7 +158,7 @@ contract ImmutableSignedZoneV2 is * * @param newApiEndpoint The new API endpoint. */ - function updateAPIEndpoint(string calldata newApiEndpoint) external override onlyRole(DEFAULT_ADMIN_ROLE) { + function updateAPIEndpoint(string calldata newApiEndpoint) external override onlyRole(ZONE_MANAGER_ROLE) { _apiEndpoint = newApiEndpoint; } @@ -170,7 +170,7 @@ contract ImmutableSignedZoneV2 is function updateDocumentationURI(string calldata newDocumentationURI) external override - onlyRole(DEFAULT_ADMIN_ROLE) + onlyRole(ZONE_MANAGER_ROLE) { _documentationURI = newDocumentationURI; } diff --git a/contracts/trading/seaport/zones/immutable-signed-zone/v2/README.md b/contracts/trading/seaport/zones/immutable-signed-zone/v2/README.md index 303daab4..13863d78 100644 --- a/contracts/trading/seaport/zones/immutable-signed-zone/v2/README.md +++ b/contracts/trading/seaport/zones/immutable-signed-zone/v2/README.md @@ -33,12 +33,16 @@ flowchart LR seaport -- 3a. transferFrom --> erc20[IERC20.sol] seaport -- 3b. transferFrom --> erc721[IERC721.sol] seaport -- 3c. safeTransferFrom --> erc1155[IERC1155.sol] - seaport -- 4. validateOrder --> zone[ImmutableSignedZoneV2.sol] + seaport -- 4. validateOrder --> Zone + subgraph Zone + direction TB + zone[ImmutableSignedZoneV2.sol] --> AccessControlEnumerable.sol + end ``` The sequence of events is as follows: -1. The client makes a HTTP `POST .../fulfillment-data` request to the Immutable Orderbook, which will construct signs and sign an `extraData` payload to return to the client +1. The client makes a HTTP `POST .../fulfillment-data` request to the Immutable Orderbook, which will construct and sign an `extraData` payload to return to the client 2. The client calls `fulfillAdvancedOrder` or `fulfillAvailableAdavancedOrders` on `ImmutableSeaport.sol` to fulfill an order 3. `ImmutableSeaport.sol` executes the fufilment by transferring items between parties 4. `ImmutableSeaport.sol` calls `validateOrder` on `ImmutableSignedZoneV2.sol`, passing it the fulfilment execution details as well as the `extraData` parameter diff --git a/contracts/trading/seaport/zones/immutable-signed-zone/v2/ZoneAccessControl.sol b/contracts/trading/seaport/zones/immutable-signed-zone/v2/ZoneAccessControl.sol new file mode 100644 index 00000000..19a5c50b --- /dev/null +++ b/contracts/trading/seaport/zones/immutable-signed-zone/v2/ZoneAccessControl.sol @@ -0,0 +1,51 @@ +// Copyright (c) Immutable Pty Ltd 2018 - 2024 +// SPDX-License-Identifier: Apache-2 + +// solhint-disable-next-line compiler-version +pragma solidity ^0.8.20; + +import {AccessControl} from "openzeppelin-contracts-5.0.2/access/AccessControl.sol"; +import {IAccessControl} from "openzeppelin-contracts-5.0.2/access/IAccessControl.sol"; +import {AccessControlEnumerable} from "openzeppelin-contracts-5.0.2/access/extensions/AccessControlEnumerable.sol"; +import {ZoneAccessControlEventsAndErrors} from + "../../../../../../contracts/trading/seaport/zones/immutable-signed-zone/v2/interfaces/ZoneAccessControlEventsAndErrors.sol"; + +/** + * @notice ZoneAccessControl encapsulates access control functionality for the zone. + */ +abstract contract ZoneAccessControl is AccessControlEnumerable, ZoneAccessControlEventsAndErrors { + /// @dev Zone manager manages the zone. + bytes32 public constant ZONE_MANAGER_ROLE = bytes32("ZONE_MANAGER"); + + /** + * @notice Constructor to setup initial default admin. + * + * @param owner The address to assign the DEFAULT_ADMIN_ROLE. + */ + constructor(address owner) { + // Grant admin role to the specified owner. + _grantRole(DEFAULT_ADMIN_ROLE, owner); + } + + /** + * @inheritdoc AccessControl + */ + function revokeRole(bytes32 role, address account) public override(AccessControl, IAccessControl) onlyRole(getRoleAdmin(role)) { + super.revokeRole(role, account); + + if (role == DEFAULT_ADMIN_ROLE && super.getRoleMemberCount(DEFAULT_ADMIN_ROLE) == 0) { + revert LastDefaultAdminRole(account); + } + } + + /** + * @inheritdoc AccessControl + */ + function renounceRole(bytes32 role, address callerConfirmation) public override(AccessControl, IAccessControl) { + super.renounceRole(role, callerConfirmation); + + if (role == DEFAULT_ADMIN_ROLE && super.getRoleMemberCount(DEFAULT_ADMIN_ROLE) == 0) { + revert LastDefaultAdminRole(callerConfirmation); + } + } +} diff --git a/contracts/trading/seaport/zones/immutable-signed-zone/v2/interfaces/ZoneAccessControlEventsAndErrors.sol b/contracts/trading/seaport/zones/immutable-signed-zone/v2/interfaces/ZoneAccessControlEventsAndErrors.sol new file mode 100644 index 00000000..cf86ab3b --- /dev/null +++ b/contracts/trading/seaport/zones/immutable-signed-zone/v2/interfaces/ZoneAccessControlEventsAndErrors.sol @@ -0,0 +1,16 @@ +// Copyright (c) Immutable Pty Ltd 2018 - 2024 +// SPDX-License-Identifier: Apache-2 + +// solhint-disable compiler-version +pragma solidity ^0.8.17; + +/** + * @notice ZoneAccessControlEventsAndErrors contains errors and events + * related to zone access control. + */ +interface ZoneAccessControlEventsAndErrors { + /** + * @dev Revert with an error if revoking last DEFAULT_ADMIN_ROLE. + */ + error LastDefaultAdminRole(address account); +} diff --git a/test/trading/seaport/ImmutableSeaportSignedZoneV2Integration.t.sol b/test/trading/seaport/ImmutableSeaportSignedZoneV2Integration.t.sol index aa65560c..923560d9 100644 --- a/test/trading/seaport/ImmutableSeaportSignedZoneV2Integration.t.sol +++ b/test/trading/seaport/ImmutableSeaportSignedZoneV2Integration.t.sol @@ -41,6 +41,7 @@ contract ImmutableSeaportSignedZoneV2IntegrationTest is Test, SigningTestHelper "./foundry-out/ImmutableSignedZoneV2Harness.t.sol/ImmutableSignedZoneV2Harness.json"; address private immutable OWNER = makeAddr("owner"); + address private immutable ZONE_MANAGER = makeAddr("zone_manager"); address private immutable SIGNER; uint256 private immutable SIGNER_PRIVATE_KEY; address private immutable FULFILLER = makeAddr("fulfiller"); @@ -98,6 +99,10 @@ contract ImmutableSeaportSignedZoneV2IntegrationTest is Test, SigningTestHelper ) ); vm.prank(OWNER); + bytes32 managerRole = zone.ZONE_MANAGER_ROLE(); + vm.prank(OWNER); + zone.grantRole(managerRole, ZONE_MANAGER); + vm.prank(ZONE_MANAGER); zone.addSigner(SIGNER); // seaport diff --git a/test/trading/seaport/zones/immutable-signed-zone/v2/IImmutableSignedZoneV2Harness.t.sol b/test/trading/seaport/zones/immutable-signed-zone/v2/IImmutableSignedZoneV2Harness.t.sol index 8a2fca35..85b5e699 100644 --- a/test/trading/seaport/zones/immutable-signed-zone/v2/IImmutableSignedZoneV2Harness.t.sol +++ b/test/trading/seaport/zones/immutable-signed-zone/v2/IImmutableSignedZoneV2Harness.t.sol @@ -12,6 +12,12 @@ import {SIP7Interface} from // solhint-disable func-name-mixedcase interface IImmutableSignedZoneV2Harness is ZoneInterface, SIP7Interface { + function grantRole(bytes32 role, address account) external; + + function DEFAULT_ADMIN_ROLE() external view returns (bytes32); + + function ZONE_MANAGER_ROLE() external view returns (bytes32); + function exposed_domainSeparator() external view returns (bytes32); function exposed_deriveDomainSeparator() external view returns (bytes32 domainSeparator); diff --git a/test/trading/seaport/zones/immutable-signed-zone/v2/ImmutableSignedZoneV2.t.sol b/test/trading/seaport/zones/immutable-signed-zone/v2/ImmutableSignedZoneV2.t.sol index 0dd27a8d..2cb42bf2 100644 --- a/test/trading/seaport/zones/immutable-signed-zone/v2/ImmutableSignedZoneV2.t.sol +++ b/test/trading/seaport/zones/immutable-signed-zone/v2/ImmutableSignedZoneV2.t.sol @@ -17,6 +17,8 @@ import {SIP6EventsAndErrors} from "../../../../../../contracts/trading/seaport/zones/immutable-signed-zone/v2/interfaces/SIP6EventsAndErrors.sol"; import {SIP7EventsAndErrors} from "../../../../../../contracts/trading/seaport/zones/immutable-signed-zone/v2/interfaces/SIP7EventsAndErrors.sol"; +import {ZoneAccessControlEventsAndErrors} from + "../../../../../../contracts/trading/seaport/zones/immutable-signed-zone/v2/interfaces/ZoneAccessControlEventsAndErrors.sol"; import {SigningTestHelper} from "../../../utils/SigningTestHelper.t.sol"; import {ImmutableSignedZoneV2Harness} from "./ImmutableSignedZoneV2Harness.t.sol"; @@ -25,6 +27,7 @@ import {ImmutableSignedZoneV2Harness} from "./ImmutableSignedZoneV2Harness.t.sol contract ImmutableSignedZoneV2Test is Test, SigningTestHelper, + ZoneAccessControlEventsAndErrors, SIP5EventsAndErrors, SIP6EventsAndErrors, SIP7EventsAndErrors @@ -39,6 +42,7 @@ contract ImmutableSignedZoneV2Test is // OpenZeppelin v5 access/IAccessControl.sol error AccessControlUnauthorizedAccount(address account, bytes32 neededRole); + error AccessControlBadConfirmation(); constructor() { (SIGNER, SIGNER_PRIVATE_KEY) = makeAddrAndKey("signer"); @@ -69,24 +73,142 @@ contract ImmutableSignedZoneV2Test is ); } + /* grantRole */ + + function test_grantRole_revertsIfCalledByNonAdminRole() public { + ImmutableSignedZoneV2 zone = _newZone(OWNER); + address nonAdmin = makeAddr("non_admin"); + bytes32 managerRole = zone.ZONE_MANAGER_ROLE(); + vm.expectRevert( + abi.encodeWithSelector(AccessControlUnauthorizedAccount.selector, nonAdmin, zone.DEFAULT_ADMIN_ROLE()) + ); + vm.prank(nonAdmin); + zone.grantRole(managerRole, OWNER); + } + + function test_grantRole_grantsIfCalledByAdminRole() public { + ImmutableSignedZoneV2 zone = _newZone(OWNER); + address newManager = makeAddr("new_manager"); + bytes32 managerRole = zone.ZONE_MANAGER_ROLE(); + vm.prank(OWNER); + zone.grantRole(managerRole, newManager); + bool newManagerHasManagerRole = zone.hasRole(managerRole, newManager); + assertTrue(newManagerHasManagerRole); + } + + /* revokeRole */ + + function test_revokeRole_revertsIfCalledByNonAdminRole() public { + ImmutableSignedZoneV2 zone = _newZone(OWNER); + bytes32 managerRole = zone.ZONE_MANAGER_ROLE(); + address managerOne = makeAddr("manager_one"); + address managerTwo = makeAddr("manager_two"); + vm.prank(OWNER); + zone.grantRole(managerRole, managerOne); + vm.prank(OWNER); + zone.grantRole(managerRole, managerTwo); + vm.expectRevert( + abi.encodeWithSelector(AccessControlUnauthorizedAccount.selector, managerOne, zone.DEFAULT_ADMIN_ROLE()) + ); + vm.prank(managerOne); + zone.revokeRole(managerRole, managerTwo); + } + + function test_revokeRole_revertsIfRevokingLastDefaultAdminRole() public { + ImmutableSignedZoneV2 zone = _newZone(OWNER); + bytes32 adminRole = zone.DEFAULT_ADMIN_ROLE(); + vm.expectRevert(abi.encodeWithSelector(LastDefaultAdminRole.selector, OWNER)); + vm.prank(OWNER); + zone.revokeRole(adminRole, OWNER); + } + + function test_revokeRole_revokesIfRevokingNonLastDefaultAdminRole() public { + ImmutableSignedZoneV2 zone = _newZone(OWNER); + bytes32 adminRole = zone.DEFAULT_ADMIN_ROLE(); + address newAdmin = makeAddr("new_admin"); + vm.prank(OWNER); + zone.grantRole(adminRole, newAdmin); + vm.prank(OWNER); + zone.revokeRole(adminRole, OWNER); + bool ownerHasAdminRole = zone.hasRole(adminRole, OWNER); + assertFalse(ownerHasAdminRole); + } + + function test_revokeRole_revokesIfRevokingLastNonDefaultAdminRole() public { + ImmutableSignedZoneV2 zone = _newZone(OWNER); + bytes32 managerRole = zone.ZONE_MANAGER_ROLE(); + vm.prank(OWNER); + zone.grantRole(managerRole, OWNER); + vm.prank(OWNER); + zone.revokeRole(managerRole, OWNER); + bool ownerHasManagerRole = zone.hasRole(managerRole, OWNER); + uint256 managerCount = zone.getRoleMemberCount(managerRole); + assertFalse(ownerHasManagerRole); + assertEq(managerCount, 0); + } + + /* renounceRole */ + + function test_renounceRole_revertsIfCallerDoesNotMatchCallerConfirmationAddress() public { + ImmutableSignedZoneV2 zone = _newZone(OWNER); + bytes32 managerRole = zone.ZONE_MANAGER_ROLE(); + address newManager = makeAddr("new_manager"); + vm.prank(OWNER); + zone.grantRole(managerRole, newManager); + vm.expectRevert(abi.encodeWithSelector(AccessControlBadConfirmation.selector)); + vm.prank(newManager); + zone.renounceRole(managerRole, makeAddr("random")); + } + + function test_renounceRole_revertsIfRenouncingLastDefaultAdminRole() public { + ImmutableSignedZoneV2 zone = _newZone(OWNER); + bytes32 adminRole = zone.DEFAULT_ADMIN_ROLE(); + vm.expectRevert(abi.encodeWithSelector(LastDefaultAdminRole.selector, OWNER)); + vm.prank(OWNER); + zone.renounceRole(adminRole, OWNER); + } + + function test_renounceRole_revokesIfRenouncingNonLastDefaultAdminRole() public { + ImmutableSignedZoneV2 zone = _newZone(OWNER); + bytes32 adminRole = zone.DEFAULT_ADMIN_ROLE(); + address newAdmin = makeAddr("new_admin"); + vm.prank(OWNER); + zone.grantRole(adminRole, newAdmin); + vm.prank(OWNER); + zone.renounceRole(adminRole, OWNER); + bool ownerHasAdminRole = zone.hasRole(adminRole, OWNER); + assertFalse(ownerHasAdminRole); + } + + function test_renounceRole_revokesIfRenouncingLastNonDefaultAdminRole() public { + ImmutableSignedZoneV2 zone = _newZone(OWNER); + bytes32 managerRole = zone.ZONE_MANAGER_ROLE(); + vm.prank(OWNER); + zone.grantRole(managerRole, OWNER); + vm.prank(OWNER); + zone.renounceRole(managerRole, OWNER); + bool ownerHasManagerRole = zone.hasRole(managerRole, OWNER); + uint256 managerCount = zone.getRoleMemberCount(managerRole); + assertFalse(ownerHasManagerRole); + assertEq(managerCount, 0); + } + /* addSigner */ - function test_addSigner_revertsIfCalledByNonAdminRole() public { + function test_addSigner_revertsIfCalledByNonZoneManagerRole() public { ImmutableSignedZoneV2 zone = _newZone(OWNER); - address nonAdminAccount = makeAddr("non_admin"); vm.expectRevert( - abi.encodeWithSelector( - AccessControlUnauthorizedAccount.selector, - nonAdminAccount, - zone.DEFAULT_ADMIN_ROLE() - ) + abi.encodeWithSelector(AccessControlUnauthorizedAccount.selector, OWNER, zone.ZONE_MANAGER_ROLE()) ); - vm.prank(nonAdminAccount); + vm.prank(OWNER); zone.addSigner(makeAddr("signer_to_add")); } function test_addSigner_revertsIfSignerIsTheZeroAddress() public { ImmutableSignedZoneV2 zone = _newZone(OWNER); + bytes32 managerRole = zone.ZONE_MANAGER_ROLE(); + vm.prank(OWNER); + zone.grantRole(managerRole, OWNER); vm.expectRevert(abi.encodeWithSelector(SignerCannotBeZeroAddress.selector)); vm.prank(OWNER); zone.addSigner(address(0)); @@ -95,6 +217,9 @@ contract ImmutableSignedZoneV2Test is function test_addSigner_emitsSignerAddedEvent() public { address signerToAdd = makeAddr("signer_to_add"); ImmutableSignedZoneV2 zone = _newZone(OWNER); + bytes32 managerRole = zone.ZONE_MANAGER_ROLE(); + vm.prank(OWNER); + zone.grantRole(managerRole, OWNER); vm.expectEmit(address(zone)); emit SignerAdded(signerToAdd); vm.prank(OWNER); @@ -104,6 +229,9 @@ contract ImmutableSignedZoneV2Test is function test_addSigner_revertsIfSignerAlreadyActive() public { address signerToAdd = makeAddr("signer_to_add"); ImmutableSignedZoneV2 zone = _newZone(OWNER); + bytes32 managerRole = zone.ZONE_MANAGER_ROLE(); + vm.prank(OWNER); + zone.grantRole(managerRole, OWNER); vm.prank(OWNER); zone.addSigner(signerToAdd); vm.expectRevert(abi.encodeWithSelector(SignerAlreadyActive.selector, signerToAdd)); @@ -114,6 +242,9 @@ contract ImmutableSignedZoneV2Test is function test_addSigner_revertsIfSignerWasPreviouslyActive() public { address signerToAdd = makeAddr("signer_to_add"); ImmutableSignedZoneV2 zone = _newZone(OWNER); + bytes32 managerRole = zone.ZONE_MANAGER_ROLE(); + vm.prank(OWNER); + zone.grantRole(managerRole, OWNER); vm.prank(OWNER); zone.addSigner(signerToAdd); vm.prank(OWNER); @@ -125,23 +256,21 @@ contract ImmutableSignedZoneV2Test is /* removeSigner */ - function test_removeSigner_revertsIfCalledByNonAdminRole() public { + function test_removeSigner_revertsIfCalledByNonZoneManagerRole() public { ImmutableSignedZoneV2 zone = _newZone(OWNER); - address nonAdminAccount = makeAddr("non_admin"); vm.expectRevert( - abi.encodeWithSelector( - AccessControlUnauthorizedAccount.selector, - nonAdminAccount, - zone.DEFAULT_ADMIN_ROLE() - ) + abi.encodeWithSelector(AccessControlUnauthorizedAccount.selector, OWNER, zone.ZONE_MANAGER_ROLE()) ); - vm.prank(nonAdminAccount); + vm.prank(OWNER); zone.removeSigner(makeAddr("signer_to_remove")); } function test_removeSigner_revertsIfSignerNotActive() public { address signerToRemove = makeAddr("signer_to_remove"); ImmutableSignedZoneV2 zone = _newZone(OWNER); + bytes32 managerRole = zone.ZONE_MANAGER_ROLE(); + vm.prank(OWNER); + zone.grantRole(managerRole, OWNER); vm.expectRevert(abi.encodeWithSelector(SignerNotActive.selector, signerToRemove)); vm.prank(OWNER); zone.removeSigner(signerToRemove); @@ -150,6 +279,9 @@ contract ImmutableSignedZoneV2Test is function test_removeSigner_emitsSignerRemovedEvent() public { address signerToRemove = makeAddr("signer_to_remove"); ImmutableSignedZoneV2 zone = _newZone(OWNER); + bytes32 managerRole = zone.ZONE_MANAGER_ROLE(); + vm.prank(OWNER); + zone.grantRole(managerRole, OWNER); vm.prank(OWNER); zone.addSigner(signerToRemove); vm.expectEmit(address(zone)); @@ -160,22 +292,20 @@ contract ImmutableSignedZoneV2Test is /* updateAPIEndpoint */ - function test_updateAPIEndpoint_revertsIfCalledByNonAdminRole() public { + function test_updateAPIEndpoint_revertsIfCalledByNonZoneManagerRole() public { ImmutableSignedZoneV2 zone = _newZone(OWNER); - address nonAdminAccount = makeAddr("non_admin"); vm.expectRevert( - abi.encodeWithSelector( - AccessControlUnauthorizedAccount.selector, - nonAdminAccount, - zone.DEFAULT_ADMIN_ROLE() - ) + abi.encodeWithSelector(AccessControlUnauthorizedAccount.selector, OWNER, zone.ZONE_MANAGER_ROLE()) ); - vm.prank(nonAdminAccount); + vm.prank(OWNER); zone.updateAPIEndpoint("https://www.new-immutable.com"); } - function test_updateAPIEndpoint_updatesAPIEndpointIfCalledByAdminRole() public { + function test_updateAPIEndpoint_updatesAPIEndpointIfCalledByZoneManagerRole() public { ImmutableSignedZoneV2 zone = _newZone(OWNER); + bytes32 managerRole = zone.ZONE_MANAGER_ROLE(); + vm.prank(OWNER); + zone.grantRole(managerRole, OWNER); string memory expectedApiEndpoint = "https://www.new-immutable.com"; vm.prank(OWNER); zone.updateAPIEndpoint(expectedApiEndpoint); @@ -186,22 +316,20 @@ contract ImmutableSignedZoneV2Test is /* updateDocumentationURI */ - function test_updateDocumentationURI_revertsIfCalledByNonAdminRole() public { + function test_updateDocumentationURI_revertsIfCalledByNonZoneManagerRole() public { ImmutableSignedZoneV2 zone = _newZone(OWNER); - address nonAdminAccount = makeAddr("non_admin"); vm.expectRevert( - abi.encodeWithSelector( - AccessControlUnauthorizedAccount.selector, - nonAdminAccount, - zone.DEFAULT_ADMIN_ROLE() - ) + abi.encodeWithSelector(AccessControlUnauthorizedAccount.selector, OWNER, zone.ZONE_MANAGER_ROLE()) ); - vm.prank(nonAdminAccount); + vm.prank(OWNER); zone.updateDocumentationURI("https://www.new-immutable.com/docs"); } - function test_updateDocumentationURI_updatesDocumentationURIIfCalledByAdminRole() public { + function test_updateDocumentationURI_updatesDocumentationURIIfCalledByZoneManagerRole() public { ImmutableSignedZoneV2 zone = _newZone(OWNER); + bytes32 managerRole = zone.ZONE_MANAGER_ROLE(); + vm.prank(OWNER); + zone.grantRole(managerRole, OWNER); string memory expectedDocumentationURI = "https://www.new-immutable.com/docs"; vm.prank(OWNER); zone.updateDocumentationURI(expectedDocumentationURI); @@ -430,6 +558,9 @@ contract ImmutableSignedZoneV2Test is function test_validateOrder_returnsMagicValueOnSuccessfulValidation() public { ImmutableSignedZoneV2Harness zone = _newZoneHarness(OWNER); + bytes32 managerRole = zone.ZONE_MANAGER_ROLE(); + vm.prank(OWNER); + zone.grantRole(managerRole, OWNER); vm.prank(OWNER); zone.addSigner(SIGNER); diff --git a/test/trading/seaport/zones/immutable-signed-zone/v2/README.md b/test/trading/seaport/zones/immutable-signed-zone/v2/README.md index dee26fac..a5dc1cc8 100644 --- a/test/trading/seaport/zones/immutable-signed-zone/v2/README.md +++ b/test/trading/seaport/zones/immutable-signed-zone/v2/README.md @@ -4,86 +4,98 @@ Constructor tests: -| Test name | Description | Happy Case | Implemented | -| ----------------------------------------------------------- | ------------------------------------------------------------- | ---------- | ----------- | -| test_contructor_grantsAdminRoleToOwner | Check `DEFAULT_ADMIN_ROLE` is granted to the specified owner. | Yes | Yes | -| test_contructor_emitsSeaportCompatibleContractDeployedEvent | Emits `SeaportCompatibleContractDeployed` event. | Yes | Yes | +| Test name | Description | Happy Case | Implemented | +| ------------------------------------------------------------- | ------------------------------------------------------------- | ---------- | ----------- | +| `test_contructor_grantsAdminRoleToOwner` | Check `DEFAULT_ADMIN_ROLE` is granted to the specified owner. | Yes | Yes | +| `test_contructor_emitsSeaportCompatibleContractDeployedEvent` | Emits `SeaportCompatibleContractDeployed` event. | Yes | Yes | Control function tests: -| Test name | Description | Happy Case | Implemented | -| ------------------------------------------------------------ | ------------------------------------------ | ---------- | ----------- | -| test_addSigner_revertsIfCalledByNonAdminRole | Add signer without authorization. | No | Yes | -| test_addSigner_revertsIfSignerIsTheZeroAddress | Add zero address as signer. | No | Yes | -| test_addSigner_emitsSignerAddedEvent | Emits `SignerAdded` event. | Yes | Yes | -| test_addSigner_revertsIfSignerAlreadyActive | Add an already active signer. | No | Yes | -| test_addSigner_revertsIfSignerWasPreviouslyActive | Add a previously active signer. | No | Yes | -| test_removeSigner_revertsIfCalledByNonAdminRole | Remove signer without authorization. | Yes | Yes | -| test_removeSigner_revertsIfSignerNotActive | Remove a signer that is not active. | No | Yes | -| test_removeSigner_emitsSignerRemovedEvent | Emits `SignerRemoved` event. | Yes | Yes | -| test_updateAPIEndpoint_revertsIfCalledByNonAdminRole | Update API endpoint without authorization. | No | Yes | -| test_updateAPIEndpoint_updatesAPIEndpointIfCalledByAdminRole | Update API endpoint with authorization. | Yes | Yes | +| Test name | Description | Happy Case | Implemented | +| ------------------------------------------------------------------------------ | ----------------------------------------------- | ---------- | ----------- | +| `test_grantRole_revertsIfCalledByNonAdminRole` | Grant role without authorization | No | Yes | +| `test_grantRole_grantsIfCalledByAdminRole` | Grant role with authorization | Yes | Yes | +| `test_revokeRole_revertsIfCalledByNonAdminRole` | Revoke role without authorization | No | Yes | +| `test_revokeRole_revertsIfRevokingLastDefaultAdminRole` | Revoke last `DEFAULT_ADMIN_ROLE`. | No | Yes | +| `test_revokeRole_revokesIfRevokingNonLastDefaultAdminRole` | Revoke non-last `DEFAULT_ADMIN_ROLE`. | Yes | Yes | +| `test_revokeRole_revokesIfRevokingLastNonDefaultAdminRole` | Revoke last non-`DEFAULT_ADMIN_ROLE`. | Yes | Yes | +| `test_renounceRole_revertsIfCallerDoesNotMatchCallerConfirmationAddress` | Renounce role without authorization | No | Yes | +| `test_renounceRole_revertsIfRenouncingLastDefaultAdminRole` | Renounce last `DEFAULT_ADMIN_ROLE`. | No | Yes | +| `test_renounceRole_revokesIfRenouncingNonLastDefaultAdminRole` | Renounce non-last `DEFAULT_ADMIN_ROLE`. | Yes | Yes | +| `test_renounceRole_revokesIfRenouncingLastNonDefaultAdminRole` | Renounce last non-`DEFAULT_ADMIN_ROLE`. | Yes | Yes | +| `test_addSigner_revertsIfCalledByNonZoneManagerRole` | Add signer without authorization. | No | Yes | +| `test_addSigner_revertsIfSignerIsTheZeroAddress` | Add zero address as signer. | No | Yes | +| `test_addSigner_emitsSignerAddedEvent` | Emits `SignerAdded` event. | Yes | Yes | +| `test_addSigner_revertsIfSignerAlreadyActive` | Add an already active signer. | No | Yes | +| `test_addSigner_revertsIfSignerWasPreviouslyActive` | Add a previously active signer. | No | Yes | +| `test_removeSigner_revertsIfCalledByNonZoneManagerRole` | Remove signer without authorization. | Yes | Yes | +| `test_removeSigner_revertsIfSignerNotActive` | Remove a signer that is not active. | No | Yes | +| `test_removeSigner_emitsSignerRemovedEvent` | Emits `SignerRemoved` event. | Yes | Yes | +| `test_updateAPIEndpoint_revertsIfCalledByNonZoneManagerRole` | Update API endpoint without authorization. | No | Yes | +| `test_updateAPIEndpoint_updatesAPIEndpointIfCalledByZoneManagerRole` | Update API endpoint with authorization. | Yes | Yes | +| `test_updateDocumentationURI_revertsIfCalledByNonZoneManagerRole` | Update documentation URI without authorization. | No | Yes | +| `test_updateDocumentationURI_updatesDocumentationURIIfCalledByZoneManagerRole` | Update documentation URI with authorization. | Yes | Yes | Operational function tests: -| Test name | Description | Happy Case | Implemented | -| ------------------------------------------------------------------------ | -------------------------------------------------- | ---------- | ----------- | -| test_getSeaportMetadata | Retrieve metadata describing the Zone. | Yes | Yes | -| test_sip7Information | Retrieve SIP-7 specific information. | Yes | Yes | -| test_supportsInterface | ERC165 support. | Yes | Yes | -| test_validateOrder_revertsIfEmptyExtraData | Validate order with empty `extraData`. | No | Yes | -| test_validateOrder_revertsIfExtraDataLengthIsLessThan93 | Validate order with unexpected `extraData` length. | No | Yes | -| test_validateOrder_revertsIfExtraDataVersionIsNotSupported | Validate order with unexpected SIP-6 version byte. | No | Yes | -| test_validateOrder_revertsIfSignatureHasExpired | Validate order with an expired signature. | No | Yes | -| test_validateOrder_revertsIfActualFulfillerDoesNotMatchExpectedFulfiller | Validate order with unexpected fufiller. | No | Yes | -| test_validateOrder_revertsIfActualFulfillerDoesNotMatchExpectedFulfiller | Validate order with expected *any* fufiller. | Yes | No | -| test_validateOrder_revertsIfSignerIsNotActive | Validate order with inactive signer. | No | Yes | -| test_validateOrder_returnsMagicValueOnSuccessfulValidation | Validate order successfully. | Yes | Yes | +| Test name | Description | Happy Case | Implemented | +| -------------------------------------------------------------------------- | -------------------------------------------------- | ---------- | ----------- | +| `test_getSeaportMetadata` | Retrieve metadata describing the Zone. | Yes | Yes | +| `test_sip7Information` | Retrieve SIP-7 specific information. | Yes | Yes | +| `test_supportsInterface` | ERC165 support. | Yes | Yes | +| `test_validateOrder_revertsIfEmptyExtraData` | Validate order with empty `extraData`. | No | Yes | +| `test_validateOrder_revertsIfExtraDataLengthIsLessThan93` | Validate order with unexpected `extraData` length. | No | Yes | +| `test_validateOrder_revertsIfExtraDataVersionIsNotSupported` | Validate order with unexpected SIP-6 version byte. | No | Yes | +| `test_validateOrder_revertsIfSignatureHasExpired` | Validate order with an expired signature. | No | Yes | +| `test_validateOrder_revertsIfActualFulfillerDoesNotMatchExpectedFulfiller` | Validate order with unexpected fufiller. | No | Yes | +| `test_validateOrder_revertsIfActualFulfillerDoesNotMatchExpectedFulfiller` | Validate order with expected *any* fufiller. | Yes | No | +| `test_validateOrder_revertsIfSignerIsNotActive` | Validate order with inactive signer. | No | Yes | +| `test_validateOrder_returnsMagicValueOnSuccessfulValidation` | Validate order successfully. | Yes | Yes | Internal operational function tests: -| Test name | Description | Happy Case | Implemented | -| ---------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------- | ---------- | ----------- | -| test_domainSeparator_returnsCachedDomainSeparatorWhenChainIDMatchesValueSetOnDeployment | Domain separator basic test. | Yes | Yes | -| test_domainSeparator_returnsUpdatedDomainSeparatorIfChainIDIsDifferentFromValueSetOnDeployment | Domain separator changes when chain ID changes. | Yes | Yes | -| test_deriveDomainSeparator_returnsDomainSeparatorForChainID | Domain separator derivation. | Yes | Yes | -| test_getSupportedSubstandards | Retrieve Zone's supported substandards. | Yes | Yes | -| test_deriveSignedOrderHash_returnsHashOfSignedOrder | Signed order hash derivation. | Yes | Yes | -| test_validateSubstandards_emptyContext | Empty context without substandards. | Yes | Yes | -| test_validateSubstandards_substandard3 | Context with substandard 3. | Yes | Yes | -| test_validateSubstandards_substandard4 | Context with substandard 4. | Yes | Yes | -| test_validateSubstandards_substandard6 | Context with substandard 6. | Yes | Yes | -| test_validateSubstandards_multipleSubstandardsInCorrectOrder | Context with multiple substandards. | Yes | Yes | -| test_validateSubstandards_substandards3Then6 | Context with substandards 3 and 6, but not 4. | Yes | Yes | -| test_validateSubstandards_allSubstandards | Context with all substandards. | Yes | Yes | -| test_validateSubstandards_revertsOnMultipleSubstandardsInIncorrectOrder | Context with multiple substandards out of order. | No | Yes | -| test_validateSubstandard3_returnsZeroLengthIfNotSubstandard3 | Substandard 3 validation skips when version byte is not 3. | Yes | Yes | -| test_validateSubstandard3_revertsIfContextLengthIsInvalid | Substandard 3 validation with invalid data. | No | Yes | -| test_validateSubstandard3_revertsIfDerivedReceivedItemsHashNotEqualToHashInContext | Substandard 3 validation when derived hash doesn't match expected hash. | No | Yes | -| test_validateSubstandard3_returns33OnSuccess | Substandard 3 validation when derived hash matches expected hash. | Yes | Yes | -| test_validateSubstandard4_returnsZeroLengthIfNotSubstandard4 | Substandard 4 validation skips when version byte is not 4. | Yes | Yes | -| test_validateSubstandard4_revertsIfContextLengthIsInvalid | Substandard 4 validation with invalid data. | No | Yes | -| test_validateSubstandard4_revertsIfExpectedOrderHashesAreNotPresent | Substandard 4 validation when required order hashes are not present. | No | Yes | -| test_validateSubstandard4_returnsLengthOfSubstandardSegmentOnSuccess | Substandard 4 validation when required order hashes are present. | Yes | Yes | -| test_validateSubstandard6_returnsZeroLengthIfNotSubstandard6 | Substandard 6 validation skips when version byte is not 6. | Yes | Yes | -| test_validateSubstandard6_revertsIfContextLengthIsInvalid | Substandard 6 validation with invalid data. | No | Yes | -| test_validateSubstandard6_revertsIfDerivedReceivedItemsHashesIsNotEqualToHashesInContext | Substandard 6 validation when derived hash doesn't match expected hash. | No | Yes | -| test_validateSubstandard6_returnsLengthOfSubstandardSegmentOnSuccess | Substandard 6 validation when derived hash matches expected hash. | Yes | Yes | -| test_deriveReceivedItemsHash_returnsHashIfNoReceivedItems | Received items derivation with not items. | Yes | Yes | -| test_deriveReceivedItemsHash_returnsHashForValidReceivedItems | Received items derivation with some items. | Yes | Yes | -| test_deriveReceivedItemsHash_returnsHashForReceivedItemWithAVeryLargeAmount | Received items derivation with scaling factor forcing `> uint256` intermediate calcualtions. | Yes | Yes | -| test_bytes32ArrayIncludes_returnsFalseIfSourceArrayIsSmallerThanValuesArray | `byte32` array inclusion check when more values than in source. | Yes | Yes | -| test_bytes32ArrayIncludes_returnsFalseIfSourceArrayDoesNotIncludeValuesArray | `byte32` array inclusion check when values are not present in source. | Yes | Yes | -| test_bytes32ArrayIncludes_returnsTrueIfSourceArrayEqualsValuesArray | `byte32` array inclusion check when source and values are identical. | Yes | Yes | -| test_bytes32ArrayIncludes_returnsTrueIfValuesArrayIsASubsetOfSourceArray | `byte32` array inclusion check when values are present in source. | Yes | Yes | +| Test name | Description | Happy Case | Implemented | +| ------------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------------------- | ---------- | ----------- | +| `test_domainSeparator_returnsCachedDomainSeparatorWhenChainIDMatchesValueSetOnDeployment` | Domain separator basic test. | Yes | Yes | +| `test_domainSeparator_returnsUpdatedDomainSeparatorIfChainIDIsDifferentFromValueSetOnDeployment` | Domain separator changes when chain ID changes. | Yes | Yes | +| `test_deriveDomainSeparator_returnsDomainSeparatorForChainID` | Domain separator derivation. | Yes | Yes | +| `test_getSupportedSubstandards` | Retrieve Zone's supported substandards. | Yes | Yes | +| `test_deriveSignedOrderHash_returnsHashOfSignedOrder` | Signed order hash derivation. | Yes | Yes | +| `test_validateSubstandards_emptyContext` | Empty context without substandards. | Yes | Yes | +| `test_validateSubstandards_substandard3` | Context with substandard 3. | Yes | Yes | +| `test_validateSubstandards_substandard4` | Context with substandard 4. | Yes | Yes | +| `test_validateSubstandards_substandard6` | Context with substandard 6. | Yes | Yes | +| `test_validateSubstandards_multipleSubstandardsInCorrectOrder` | Context with multiple substandards. | Yes | Yes | +| `test_validateSubstandards_substandards3Then6` | Context with substandards 3 and 6, but not 4. | Yes | Yes | +| `test_validateSubstandards_allSubstandards` | Context with all substandards. | Yes | Yes | +| `test_validateSubstandards_revertsOnMultipleSubstandardsInIncorrectOrder` | Context with multiple substandards out of order. | No | Yes | +| `test_validateSubstandard3_returnsZeroLengthIfNotSubstandard3` | Substandard 3 validation skips when version byte is not 3. | Yes | Yes | +| `test_validateSubstandard3_revertsIfContextLengthIsInvalid` | Substandard 3 validation with invalid data. | No | Yes | +| `test_validateSubstandard3_revertsIfDerivedReceivedItemsHashNotEqualToHashInContext` | Substandard 3 validation when derived hash doesn't match expected hash. | No | Yes | +| `test_validateSubstandard3_returns33OnSuccess` | Substandard 3 validation when derived hash matches expected hash. | Yes | Yes | +| `test_validateSubstandard4_returnsZeroLengthIfNotSubstandard4` | Substandard 4 validation skips when version byte is not 4. | Yes | Yes | +| `test_validateSubstandard4_revertsIfContextLengthIsInvalid` | Substandard 4 validation with invalid data. | No | Yes | +| `test_validateSubstandard4_revertsIfExpectedOrderHashesAreNotPresent` | Substandard 4 validation when required order hashes are not present. | No | Yes | +| `test_validateSubstandard4_returnsLengthOfSubstandardSegmentOnSuccess` | Substandard 4 validation when required order hashes are present. | Yes | Yes | +| `test_validateSubstandard6_returnsZeroLengthIfNotSubstandard6` | Substandard 6 validation skips when version byte is not 6. | Yes | Yes | +| `test_validateSubstandard6_revertsIfContextLengthIsInvalid` | Substandard 6 validation with invalid data. | No | Yes | +| `test_validateSubstandard6_revertsIfDerivedReceivedItemsHashesIsNotEqualToHashesInContext` | Substandard 6 validation when derived hash doesn't match expected hash. | No | Yes | +| `test_validateSubstandard6_returnsLengthOfSubstandardSegmentOnSuccess` | Substandard 6 validation when derived hash matches expected hash. | Yes | Yes | +| `test_deriveReceivedItemsHash_returnsHashIfNoReceivedItems` | Received items derivation with not items. | Yes | Yes | +| `test_deriveReceivedItemsHash_returnsHashForValidReceivedItems` | Received items derivation with some items. | Yes | Yes | +| `test_deriveReceivedItemsHash_returnsHashForReceivedItemWithAVeryLargeAmount` | Received items derivation with scaling factor forcing `> uint256` intermediate calcualtions. | Yes | Yes | +| `test_bytes32ArrayIncludes_returnsFalseIfSourceArrayIsSmallerThanValuesArray` | `byte32` array inclusion check when more values than in source. | Yes | Yes | +| `test_bytes32ArrayIncludes_returnsFalseIfSourceArrayDoesNotIncludeValuesArray` | `byte32` array inclusion check when values are not present in source. | Yes | Yes | +| `test_bytes32ArrayIncludes_returnsTrueIfSourceArrayEqualsValuesArray` | `byte32` array inclusion check when source and values are identical. | Yes | Yes | +| `test_bytes32ArrayIncludes_returnsTrueIfValuesArrayIsASubsetOfSourceArray` | `byte32` array inclusion check when values are present in source. | Yes | Yes | Integration tests: All of these tests are in [test/trading/seaport/ImmutableSeaportSignedZoneV2Integration.t.sol](../../../ImmutableSeaportSignedZoneV2Integration.t.sol). -| Test name | Description | Happy Case | Implemented | -| -------------------------------------------------- | ------------------------------- | ---------- | ----------- | -| test_fulfillAdvancedOrder_withCompleteFulfilment | Full fulfilment. | Yes | Yes | -| test_fulfillAdvancedOrder_withPartialFill | Partial fulfilment. | Yes | Yes | -| test_fulfillAdvancedOrder_withMultiplePartialFills | Sequential partial fulfilments. | Yes | Yes | -| test_fulfillAdvancedOrder_withOverfilling | Over fulfilment. | Yes | Yes | \ No newline at end of file +| Test name | Description | Happy Case | Implemented | +| ---------------------------------------------------- | ------------------------------- | ---------- | ----------- | +| `test_fulfillAdvancedOrder_withCompleteFulfilment` | Full fulfilment. | Yes | Yes | +| `test_fulfillAdvancedOrder_withPartialFill` | Partial fulfilment. | Yes | Yes | +| `test_fulfillAdvancedOrder_withMultiplePartialFills` | Sequential partial fulfilments. | Yes | Yes | +| `test_fulfillAdvancedOrder_withOverfilling` | Over fulfilment. | Yes | Yes | \ No newline at end of file