separate endpoint and tests

nori-dot-eco · Mar 6, 2024 · f866408 · f866408
1 parent 4cbf69f
commit f866408
Show file tree

Hide file tree

Showing 2 changed files with 81 additions and 15 deletions.
diff --git a/contracts/Removal.sol b/contracts/Removal.sol
@@ -619,7 +619,7 @@ contract Removal is
    *
    * ##### Requirements:
    *
-   * - Can only be called by the Market contract or an account with the `CONSIGNOR_ROLE`.
+   * - Can only be called by the Market contract.
    * - `ids` and `amounts` must have the same length.
    * - If `to` refers to a smart contract, it must implement {IERC1155Receiver-onERC1155BatchReceived} and return the
    * acceptance magic value.
@@ -636,9 +636,7 @@ contract Removal is
     uint256[] memory amounts,
     bytes memory data
   ) public override {
-    if (
-      _msgSender() != address(_market) && !hasRole(CONSIGNOR_ROLE, _msgSender())
-    ) {
+    if (_msgSender() != address(_market)) {
       revert ForbiddenTransfer();
     }
     super.safeBatchTransferFrom({
@@ -650,6 +648,36 @@ contract Removal is
     });
   }
 
+  /**
+   * @notice Permissioned version of `safeTransferFrom`.
+   * @dev Emits a `TransferBatch` event.
+   *
+   * ##### Requirements:
+   *
+   * - Can only be called by an address with the `CONSIGNOR_ROLE`.
+   * - `ids` and `amounts` must have the same length.
+   * - If `to` refers to a smart contract, it must implement {IERC1155Receiver-onERC1155BatchReceived} and return the
+   * acceptance magic value.
+   * @param from The address to transfer from.
+   * @param to The address to transfer to.
+   * @param ids The removal IDs to transfer.
+   * @param amounts The amounts of removals to transfer.
+   */
+  function consignorBatchTransfer(
+    address from,
+    address to,
+    uint256[] memory ids,
+    uint256[] memory amounts
+  ) public onlyRole(CONSIGNOR_ROLE) {
+    super._safeBatchTransferFrom({
+      from: from,
+      to: to,
+      ids: ids,
+      amounts: amounts,
+      data: ""
+    });
+  }
+
   /**
    * @notice Grants or revokes permission to `operator` to transfer the caller's tokens, according to `approved`.
    * @dev Emits an `ApprovalForAll` event.

diff --git a/test/Removal.t.sol b/test/Removal.t.sol
@@ -1352,31 +1352,69 @@ contract Removal_safeBatchTransferFrom is UpgradeableMarket {
       data: ""
     });
   }
+}
 
-  function test_isCallableByConsignor() external {
-    vm.expectRevert(ForbiddenTransfer.selector);
+contract Removal_consignorBatchTransfer is UpgradeableMarket {
+  uint256[] private _removalIds;
+
+  function setUp() external {
+    _removalIds = _seedRemovals({
+      to: _namedAccounts.supplier,
+      count: 2,
+      list: false
+    });
+  }
+
+  function test() external {
+    _removal.grantRole({
+      role: _removal.CONSIGNOR_ROLE(),
+      account: _namedAccounts.admin
+    });
+    assert(_removal.hasRole(_removal.CONSIGNOR_ROLE(), _namedAccounts.admin));
     vm.prank(_namedAccounts.admin);
-    _removal.safeBatchTransferFrom({
+    _removal.consignorBatchTransfer({
       from: _namedAccounts.supplier,
       to: _namedAccounts.admin,
       ids: _removalIds,
-      amounts: new uint256[](2).fill(1 ether),
-      data: ""
+      amounts: new uint256[](2).fill(1 ether)
     });
+    assertEq(_removal.balanceOf(_namedAccounts.admin, _removalIds[0]), 1 ether);
+    assertEq(_removal.balanceOf(_namedAccounts.admin, _removalIds[1]), 1 ether);
+  }
+
+  function test_reverts_whenSenderIsNotConsignor() external {
+    assert(!_removal.hasRole(_removal.CONSIGNOR_ROLE(), _namedAccounts.admin));
+    vm.expectRevert(
+      bytes(
+        string.concat(
+          "AccessControl: account 0x05127efcd2fc6a781bfed49188da1081670b22d8 is missing role ",
+          "0xa269776b75ac4c5fa422bb11bec3ed3cee626848d07687372583174b209261fb"
+        )
+      )
+    );
+    vm.prank(_namedAccounts.admin);
+    _removal.consignorBatchTransfer({
+      from: _namedAccounts.supplier,
+      to: _namedAccounts.admin,
+      ids: _removalIds,
+      amounts: new uint256[](2).fill(1 ether)
+    });
+  }
+
+  function test_reverts_whenReceiverIsNotConsignor() external {
     _removal.grantRole({
       role: _removal.CONSIGNOR_ROLE(),
       account: _namedAccounts.admin
     });
+    assert(_removal.hasRole(_removal.CONSIGNOR_ROLE(), _namedAccounts.admin));
+    vm.expectRevert(ForbiddenTransfer.selector);
     vm.prank(_namedAccounts.admin);
-    _removal.safeBatchTransferFrom({
+    _removal.consignorBatchTransfer({
       from: _namedAccounts.supplier,
-      to: _namedAccounts.admin,
+      to: _namedAccounts.supplier2,
       ids: _removalIds,
-      amounts: new uint256[](2).fill(1 ether),
-      data: ""
+      amounts: new uint256[](2).fill(1 ether)
     });
-    assertEq(_removal.balanceOf(_namedAccounts.admin, _removalIds[0]), 1 ether);
-    assertEq(_removal.balanceOf(_namedAccounts.admin, _removalIds[1]), 1 ether);
   }
 }