review comments

.gas-snapshot

@@ -4,7 +4,7 @@ AddressArrayLib_fill:test_library() (gas: 13081)
 AddressArrayLib_fill:test_reference() (gas: 13691)
 BridgedPolygonNORI_name:test() (gas: 17205)
 BridgedPolygonNORI_permit:test() (gas: 92382)
-Certificate**msgSenderERC721A:test() (gas: 422)
+Certificate__msgSenderERC721A:test() (gas: 422)
 Certificate_approve:test() (gas: 17906)
 Certificate_burn:test() (gas: 188509)
 Certificate_burn:test_reverts_when_paused() (gas: 54783)
@@ -20,75 +20,75 @@ Certificate_supportsInterface:test() (gas: 5159)
 Certificate_transferFrom:test() (gas: 48618)
 Certificate_transferFrom:test_reverts_when_paused() (gas: 38088)
 Certificate_transferFrom_reverts_ForbiddenTransferAfterMinting:test() (gas: 18960)
-Checkout_buyingFromOneRemoval:test() (gas: 470314)
-Checkout_buyingFromOneRemoval_byApproval:test() (gas: 408705)
-Checkout_buyingFromTenRemovals:test() (gas: 1549359)
-Checkout_buyingFromTenRemovals_singleSupplier:test() (gas: 1309225)
-Checkout_buyingFromTenRemovals_singleSupplier_withoutFee:test() (gas: 1304425)
-Checkout_buyingFromTenRemovals_withoutFee:test() (gas: 1317176)
-Checkout_buyingFromTenSuppliers:test() (gas: 1818158)
-Checkout_buyingWithAlternateERC20:test() (gas: 540964)
-Checkout_buyingWithAlternateERC20_floatingPointPriceMultiple:test() (gas: 506930)
-Checkout_swapRevertsWhenBuyerIsMissingSANCTION_ALLOWLIST_ROLE:test() (gas: 168693)
-Checkout_swapRevertsWithDifferentPermitSignerAndMsgSender:test() (gas: 168705)
-Checkout_swapWithoutFeeSpecialOrder:test() (gas: 380597)
-Checkout_swapWithoutFeeSpecialOrder_specificSupplier:test() (gas: 375252)
+Checkout_buyingFromOneRemoval:test() (gas: 470402)
+Checkout_buyingFromOneRemoval_byApproval:test() (gas: 408749)
+Checkout_buyingFromTenRemovals:test() (gas: 1549843)
+Checkout_buyingFromTenRemovals_singleSupplier:test() (gas: 1309445)
+Checkout_buyingFromTenRemovals_singleSupplier_withoutFee:test() (gas: 1304645)
+Checkout_buyingFromTenRemovals_withoutFee:test() (gas: 1317396)
+Checkout_buyingFromTenSuppliers:test() (gas: 1818545)
+Checkout_buyingWithAlternateERC20:test() (gas: 541052)
+Checkout_buyingWithAlternateERC20_floatingPointPriceMultiple:test() (gas: 507018)
+Checkout_swapRevertsWhenBuyerIsMissingSANCTION_ALLOWLIST_ROLE:test() (gas: 168737)
+Checkout_swapRevertsWithDifferentPermitSignerAndMsgSender:test() (gas: 168749)
+Checkout_swapWithoutFeeSpecialOrder:test() (gas: 380619)
+Checkout_swapWithoutFeeSpecialOrder_specificSupplier:test() (gas: 375274)
 Checkout_swapWithoutFeeSpecialOrder_specificSupplier:test_revertsWhenSupplierDoesNotExistInMarket() (gas: 57753)
-Checkout_swapWithoutFeeSpecialOrder_specificVintages:test_basicFulfillment() (gas: 642423)
+Checkout_swapWithoutFeeSpecialOrder_specificVintages:test_basicFulfillment() (gas: 642489)
 Checkout_swapWithoutFeeSpecialOrder_specificVintages:test_revertsWhenNoRemovalsFromSpecifiedVintages() (gas: 91044)
-Checkout_swapWithoutFeeSpecialOrder_specificVintagesSpecificSupplier:test_basicFulfillment() (gas: 491863)
-GasBenchmark_buyingFromManyRemovals_singleSupplier:test() (gas: 29847653)
-GasBenchmark_buyingFromOneRemoval_singleSupplier:test() (gas: 322867)
+Checkout_swapWithoutFeeSpecialOrder_specificVintagesSpecificSupplier:test_basicFulfillment() (gas: 491903)
+GasBenchmark_buyingFromManyRemovals_singleSupplier:test() (gas: 29853285)
+GasBenchmark_buyingFromOneRemoval_singleSupplier:test() (gas: 322889)
 LockedNORILib_availableAmount:test() (gas: 12371)
 LockedNORITest:testBatchGrantCreation() (gas: 705179)
 LockedNORITest:testNormalWithdrawal() (gas: 1102743)
 LockedNORITest:testReentryTokensReceived() (gas: 1102887)
 LockedNORITest:testReentryTokensToSend() (gas: 1104432)
 LockedNORITest:testTokensReceivedReverts() (gas: 69026)
-MarketInvariantTest:invariant_callSummary() (runs: 400, calls: 6000, reverts: 4285)
-MarketInvariantTest:invariant_sumOfPurchaseAmounts() (runs: 400, calls: 6000, reverts: 4268)
-MarketSupplierSelectionNotUsingUpSuppliersLastRemoval:test() (gas: 644612)
+MarketInvariantTest:invariant_callSummary() (runs: 400, calls: 6000, reverts: 4265)
+MarketInvariantTest:invariant_sumOfPurchaseAmounts() (runs: 400, calls: 6000, reverts: 4261)
+MarketSupplierSelectionNotUsingUpSuppliersLastRemoval:test() (gas: 644744)
 Market_ALLOWLIST_ROLE:test() (gas: 12799)
 Market_SANCTION_ALLOWLIST_ROLE:test() (gas: 12897)
-Market_USDC_swap_respects_decimal_mismatch:test() (gas: 786916)
-Market**addActiveRemoval:test() (gas: 183344)
-Market**addActiveRemoval:test**lis2VintagesFor1SupplierFor2SubIdentifiers() (gas: 242879)
-Market**addActiveRemoval:test**list1VintageFor1Supplier() (gas: 188309)
-Market**addActiveRemoval:test**list1VintageFor2Suppliers() (gas: 360670)
-Market**addActiveRemoval:test**list2VintagesFor1SupplierFor1SubIdentifier() (gas: 262709)
-Market**isAuthorizedWithdrawal_false:test_returnsFalseWhenAllConditionsAreFalse() (gas: 7499)
-Market**isAuthorizedWithdrawal_true:test_returnsTrueWhenMsgSenderEqualsOwner() (gas: 16989)
-Market**isAuthorizedWithdrawal_true:test_returnsTrueWhenMsgSenderHasDefaultAdminRole() (gas: 113146)
-Market**isAuthorizedWithdrawal_true:test_returnsTrueWhenMsgSenderIsApprovedForAll() (gas: 23148)
-Market**multicall_empty_bytes_reverts:test() (gas: 20935)
-Market**multicall_initialize_reverts:test() (gas: 33859)
-Market**setPriceMultiple:test() (gas: 29132)
-Market**setPriceMultiple:test_canBeZero() (gas: 9259)
-Market**setPriceMultiple:test_revertsWhenSetBelow100() (gas: 3657)
-Market**setPurchasingToken:test() (gas: 1021494)
-Market**validatePrioritySupply:test_supplyAfterPurchaseIsLessThanPriorityRestrictedThreshold() (gas: 2499)
-Market**validatePrioritySupply:test_supplyAfterPurchaseIsZero() (gas: 2544)
-Market**validatePrioritySupply_buyerIsAllowlistedAndAmountExceedsPriorityRestrictedThreshold:test() (gas: 4848)
-Market**validatePrioritySupply_reverts_LowSupplyAllowlistRequired:test() (gas: 7692)
-Market**validateSupply:test() (gas: 292)
-Market**validateSupply:test_reverts_OutOfSupply() (gas: 3172)
+Market_USDC_swap_respects_decimal_mismatch:test() (gas: 787070)
+Market__addActiveRemoval:test() (gas: 183344)
+Market__addActiveRemoval:test__lis2VintagesFor1SupplierFor2SubIdentifiers() (gas: 242879)
+Market__addActiveRemoval:test__list1VintageFor1Supplier() (gas: 188309)
+Market__addActiveRemoval:test__list1VintageFor2Suppliers() (gas: 360670)
+Market__addActiveRemoval:test__list2VintagesFor1SupplierFor1SubIdentifier() (gas: 262709)
+Market__isAuthorizedWithdrawal_false:test_returnsFalseWhenAllConditionsAreFalse() (gas: 25767)
+Market__isAuthorizedWithdrawal_true:test_returnsTrueWhenMsgSenderEqualsOwner() (gas: 17086)
+Market__isAuthorizedWithdrawal_true:test_returnsTrueWhenMsgSenderHasDefaultAdminRole() (gas: 113243)
+Market__isAuthorizedWithdrawal_true:test_returnsTrueWhenMsgSenderIsApprovedForAll() (gas: 23245)
+Market__multicall_empty_bytes_reverts:test() (gas: 20935)
+Market__multicall_initialize_reverts:test() (gas: 33859)
+Market__setPriceMultiple:test() (gas: 29132)
+Market__setPriceMultiple:test_canBeZero() (gas: 9259)
+Market__setPriceMultiple:test_revertsWhenSetBelow100() (gas: 3657)
+Market__setPurchasingToken:test() (gas: 1021494)
+Market__validatePrioritySupply:test_supplyAfterPurchaseIsLessThanPriorityRestrictedThreshold() (gas: 2499)
+Market__validatePrioritySupply:test_supplyAfterPurchaseIsZero() (gas: 2544)
+Market__validatePrioritySupply_buyerIsAllowlistedAndAmountExceedsPriorityRestrictedThreshold:test() (gas: 4848)
+Market__validatePrioritySupply_reverts_LowSupplyAllowlistRequired:test() (gas: 7692)
+Market__validateSupply:test() (gas: 292)
+Market__validateSupply:test_reverts_OutOfSupply() (gas: 3172)
 Market_calculates_prices_using_decimal:test() (gas: 66624)
 Market_convertPurchasingTokenDecimalsToRemovalDecimals:test() (gas: 26029)
-Market_convertRemovalDecimalsToPurchasingTokenDecimals:test() (gas: 29773)
+Market_convertRemovalDecimalsToPurchasingTokenDecimals:test() (gas: 29817)
 Market_getActiveSuppliers:test_1_supplier() (gas: 447828)
 Market_getActiveSuppliers:test_3_suppliers() (gas: 1088942)
 Market_getActiveSuppliers:test_no_suppliers() (gas: 20859)
 Market_getPriceMultiple:test() (gas: 14851)
-Market_getRemovalIdsForSupplier:test_1_removal() (gas: 448231)
-Market_getRemovalIdsForSupplier:test_3_removals() (gas: 831646)
-Market_getRemovalIdsForSupplier:test_3_removals_different_vintages() (gas: 877755)
-Market_getRemovalIdsForSupplier:test_no_removals() (gas: 26044)
+Market_getRemovalIdsForSupplier:test_1_removal() (gas: 448142)
+Market_getRemovalIdsForSupplier:test_3_removals() (gas: 831557)
+Market_getRemovalIdsForSupplier:test_3_removals_different_vintages() (gas: 877666)
+Market_getRemovalIdsForSupplier:test_no_removals() (gas: 25955)
 Market_onERC1155BatchReceived:test() (gas: 208124)
 Market_onERC1155BatchReceived_reverts_SenderNotRemovalContract:test() (gas: 331570)
 Market_onERC1155Received:test() (gas: 206036)
 Market_onERC1155Received_reverts_SenderNotRemovalContract:test() (gas: 159044)
-Market_purchasingTokenAddress:test() (gas: 17080)
-Market_replace:test() (gas: 277991)
+Market_purchasingTokenAddress:test() (gas: 17102)
+Market_replace:test() (gas: 278013)
 Market_replace_reverts_CertificateNotYetMinted:test() (gas: 49559)
 Market_replace_reverts_ReplacementAmountExceedsNrtDeficit:test() (gas: 52590)
 Market_replace_reverts_ReplacementAmountMismatch:test() (gas: 86397)
@@ -97,49 +97,50 @@ Market_setPriorityRestrictedThreshold:test() (gas: 157448)
 Market_setPriorityRestrictedThreshold:test_zeroAvailable() (gas: 152423)
 Market_setPurchasingTokenAndPriceMultiple:test() (gas: 1026641)
 Market_setPurchasingTokenAndPriceMultiple_revertsIfNotAdmin:test() (gas: 50813)
-Market_supplierSelectionUsingUpSuppliersLastRemoval:test() (gas: 641321)
-Market_swap_revertsWhenUnsafeERC20TransferFails:test() (gas: 189648)
+Market_supplierSelectionUsingUpSuppliersLastRemoval:test() (gas: 641453)
+Market_swap_revertsWhenUnsafeERC20TransferFails:test() (gas: 189736)
 Market_validates_certificate_amount:test() (gas: 596800)
-Market_withdraw_1x3_center:test() (gas: 340814)
-Market_withdraw_2x1_back:test() (gas: 345496)
-Market_withdraw_2x1_front:test() (gas: 333831)
+Market_withdraw_1x3_center:test() (gas: 346739)
+Market_withdraw_2x1_back:test() (gas: 351412)
+Market_withdraw_2x1_front:test() (gas: 339756)
 Market_withdraw_2x1_front_relist:test() (gas: 381788)
-Market_withdraw_as_DEFAULT_ADMIN_ROLE:test() (gas: 276524)
-Market_withdraw_as_operator:test() (gas: 285669)
-Market_withdraw_as_supplier:test() (gas: 274665)
-Market_withdraw_reverts:test() (gas: 138709)
+Market_withdraw_as_DEFAULT_ADMIN_ROLE:test() (gas: 282449)
+Market_withdraw_as_operator:test() (gas: 291594)
+Market_withdraw_as_supplier:test() (gas: 280590)
+Market_withdraw_reverts:test() (gas: 144618)
+Market_withdraw_to_CONSIGNOR_ROLE:test() (gas: 284393)
 NORI_name:test() (gas: 17205)
 NORI_permit:test() (gas: 92382)
 NoriUSDC_permit:test() (gas: 122061)
 RemovalQueue_getTotalBalanceFromRemovalQueue:test() (gas: 23921)
-RemovalQueue_getTotalBalanceFromRemovalQueue:test_100xRemovalsOfTheDifferentVintages() (gas: 895808)
+RemovalQueue_getTotalBalanceFromRemovalQueue:test_100xRemovalsOfTheDifferentVintages() (gas: 895830)
 RemovalQueue_getTotalBalanceFromRemovalQueue:test_100xRemovalsOfTheSameVintage() (gas: 620321)
 RemovalQueue_insertRemovalByVintage:test_insertRemovalOnce() (gas: 119613)
 RemovalQueue_insertRemovalByVintage:test_insertRemovalTwice() (gas: 121103)
-Removal**beforeTokenTransfer:test() (gas: 18010)
-Removal**beforeTokenTransfer:test_paused_reverts_Paused() (gas: 29432)
-Removal**createRemovalData:test() (gas: 22593)
-Removal**createRemovalData:test_reverts_InvalidData() (gas: 25711)
-Removal**createRemovalDataBatch:test() (gas: 29594)
-Removal**createRemovalDataBatch:test_reverts_InvalidData2() (gas: 36802)
-Removal**isValidTransferAmount:testFuzz_ReturnFalse_NonMultiplesOf1e14(uint256) (runs: 256, μ: 13935, ~: 13891)
-Removal**isValidTransferAmount:testFuzz_ReturnTrue_MultiplesOf1e14(uint256) (runs: 256, μ: 14315, ~: 14432)
-Removal**isValidTransferAmount:testFuzz_ReturnTrue_SmallestGranularity() (gas: 6876)
-Removal**isValidTransferAmount:test_ReturnFalse_AmountIsTooGranular() (gas: 6854)
-Removal**isValidTransferAmount:test_ReturnFalse_AmountIsTooGranularAndToIsTheCertificate() (gas: 4789)
-Removal**isValidTransferAmount:test_ReturnFalse_AmountIsTooGranularAndToIsTheMarket() (gas: 2652)
-Removal**isValidTransferAmount:test_ReturnFalse_AmountIsZeroAndToIsTheCertificate() (gas: 4705)
-Removal**isValidTransferAmount:test_ReturnFalse_AmountIsZeroAndToIsTheMarket() (gas: 2565)
-Removal**isValidTransferAmount:test_ReturnTrue_AmountIsZeroAndToIsNeitherTheMarketNorCertificate() (gas: 6852)
-Removal**validateRemoval:test() (gas: 2491)
-Removal\_\_validateRemoval:test_reverts_InvalidData() (gas: 5373)
+Removal__beforeTokenTransfer:test() (gas: 18010)
+Removal__beforeTokenTransfer:test_paused_reverts_Paused() (gas: 29432)
+Removal__createRemovalData:test() (gas: 22593)
+Removal__createRemovalData:test_reverts_InvalidData() (gas: 25711)
+Removal__createRemovalDataBatch:test() (gas: 29594)
+Removal__createRemovalDataBatch:test_reverts_InvalidData2() (gas: 36802)
+Removal__isValidTransferAmount:testFuzz_ReturnFalse_NonMultiplesOf1e14(uint256) (runs: 256, μ: 13935, ~: 13891)
+Removal__isValidTransferAmount:testFuzz_ReturnTrue_MultiplesOf1e14(uint256) (runs: 256, μ: 14307, ~: 14432)
+Removal__isValidTransferAmount:testFuzz_ReturnTrue_SmallestGranularity() (gas: 6876)
+Removal__isValidTransferAmount:test_ReturnFalse_AmountIsTooGranular() (gas: 6854)
+Removal__isValidTransferAmount:test_ReturnFalse_AmountIsTooGranularAndToIsTheCertificate() (gas: 4789)
+Removal__isValidTransferAmount:test_ReturnFalse_AmountIsTooGranularAndToIsTheMarket() (gas: 2652)
+Removal__isValidTransferAmount:test_ReturnFalse_AmountIsZeroAndToIsTheCertificate() (gas: 4705)
+Removal__isValidTransferAmount:test_ReturnFalse_AmountIsZeroAndToIsTheMarket() (gas: 2565)
+Removal__isValidTransferAmount:test_ReturnTrue_AmountIsZeroAndToIsNeitherTheMarketNorCertificate() (gas: 6852)
+Removal__validateRemoval:test() (gas: 2491)
+Removal__validateRemoval:test_reverts_InvalidData() (gas: 5373)
 Removal_addBalance:test() (gas: 60280)
 Removal_addBalance_reverts_RemovalNotYetMinted:test() (gas: 31115)
-Removal_consign_revertsForSoldRemovals:test() (gas: 896622)
+Removal_consign_revertsForSoldRemovals:test() (gas: 896728)
 Removal_consignorBatchTransfer:test() (gas: 297325)
 Removal_consignorBatchTransfer:test_reverts_whenReceiverIsNotConsignor() (gas: 128791)
 Removal_consignorBatchTransfer:test_reverts_whenSenderIsNotConsignor() (gas: 65646)
-Removal_getMarketBalance:test() (gas: 907046)
+Removal_getMarketBalance:test() (gas: 910292)
 Removal_getOwnedTokenIds:test_multiple_tokens_with_transfer() (gas: 916416)
 Removal_getOwnedTokenIds:test_no_tokens() (gas: 18683)
 Removal_getProjectId:test() (gas: 19307)
@@ -157,7 +158,7 @@ Removal_mintBatch_zero_amount_removal:test() (gas: 139898)
 Removal_mintBatch_zero_amount_removal_to_market_reverts:test() (gas: 61595)
 Removal_multicall:test_balanceOfBatch() (gas: 320743)
 Removal_release_listed:test() (gas: 356643)
-Removal_release_listed_isRemovedFromMarket:test() (gas: 356996)
+Removal_release_listed_isRemovedFromMarket:test() (gas: 356854)
 Removal_release_partial_listed:test() (gas: 79593)
 Removal_release_retired:test() (gas: 92447)
 Removal_release_retired_2x:test() (gas: 98511)
@@ -193,4 +194,4 @@ UInt256ArrayLib_sum:test() (gas: 91155)
 UInt256ArrayLib_sum:test_gas() (gas: 91317)
 UInt256ArrayLib_sum:test_library() (gas: 40528)
 UInt256ArrayLib_sum:test_library_overflow() (gas: 9518)
-UInt256ArrayLib_sum:test_reference() (gas: 55513)
+UInt256ArrayLib_sum:test_reference() (gas: 55513)

contracts/Market.sol

@@ -1356,22 +1356,22 @@ contract Market is
    * does not have the role `MARKET_ADMIN_ROLE`, or if the recipient of the removal is either not the supplier
    * of the removal or does not have the `Removal.CONSIGNOR_ROLE`.
    * @param supplier The supplier of the removal.
+   * @param to The recipient of the removal.
    * @return Returns true if the caller is the supplier, an approved spender, or has the role `MARKET_ADMIN_ROLE`,
    * and the recipient is the removal supplier or has the `Removal.CONSIGNOR_ROLE`, false otherwise.
    */
   function _isAuthorizedWithdrawal(
     address supplier,
     address to
   ) internal view returns (bool) {
-    return
-      (_msgSender() == supplier ||
-        hasRole({role: MARKET_ADMIN_ROLE, account: _msgSender()}) ||
-        _removal.isApprovedForAll({
-          account: supplier,
-          operator: _msgSender()
-        })) &&
-      (_removal.hasRole({role: _removal.CONSIGNOR_ROLE(), account: to}) ||
-        supplier == to);
+    bool isApprovedCaller = _msgSender() == supplier ||
+      hasRole({role: MARKET_ADMIN_ROLE, account: _msgSender()}) ||
+      _removal.isApprovedForAll({account: supplier, operator: _msgSender()});
+    bool isApprovedRecipient = _removal.hasRole({
+      role: _removal.CONSIGNOR_ROLE(),
+      account: to
+    }) || supplier == to;
+    return isApprovedCaller && isApprovedRecipient;
   }
 
   /**

docs/Market.md

@@ -1179,7 +1179,7 @@ of the removal or does not have the `Removal.CONSIGNOR_ROLE`.</i>
 | Name | Type | Description |
 | ---- | ---- | ----------- |
 | supplier | address | The supplier of the removal. |
-| to | address |  |
+| to | address | The recipient of the removal. |
 
 | Name | Type | Description |
 | ---- | ---- | ----------- |

test/Market.t.sol

@@ -708,16 +708,24 @@ contract Market__isAuthorizedWithdrawal_true is
   }
 }
 
-contract Market__isAuthorizedWithdrawal_false is NonUpgradeableMarket {
+contract Market__isAuthorizedWithdrawal_false is
+  NonUpgradeableMarket,
+  UpgradeableRemoval
+{
+  Removal private _mockRemoval;
+
   function setUp() external {
+    _mockRemoval = _deployRemoval();
+
     vm.store(
       address(this),
-      bytes32(uint256(301)), // sets the _removal storage slot to the market contract to enable mock calls
-      bytes32(uint256(uint160(address(this))))
+      bytes32(uint256(301)), // sets the _removal storage slot in the market contract to enable mock calls
+      bytes32(uint256(uint160(address(_mockRemoval))))
     );
+
     vm.mockCall(
       address(this),
-      abi.encodeWithSelector(IERC1155Upgradeable.isApprovedForAll.selector),
+      abi.encodeWithSelector(_mockRemoval.isApprovedForAll.selector),
       abi.encode(false)
     );
   }