Merge branch 'main' into audit_v2_4_0_flb_01c

contracts/interfaces/handlers/IBosonSequentialCommitHandler.sol

@@ -37,7 +37,7 @@ interface IBosonSequentialCommitHandler is BosonErrors, IBosonExchangeEvents, IB
      *   - Calling transferFrom on token fails for some reason (e.g. protocol is not approved to transfer)
      *   - Received ERC20 token amount differs from the expected value
      *   - Protocol does not receive the voucher
-     *   - Transfer of voucher to the buyer fails for some reasong (e.g. buyer is contract that doesn't accept voucher)
+     *   - Transfer of voucher to the buyer fails for some reason (e.g. buyer is contract that doesn't accept voucher)
      *   - Reseller did not approve protocol to transfer exchange token in escrow
      * - Call to price discovery contract fails
      * - Protocol fee and royalties combined exceed the secondary price

contracts/mock/BuyerContract.sol

@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+pragma solidity 0.8.22;
+
+import { IERC721Receiver } from "../interfaces/IERC721Receiver.sol";
+
+/**
+ * @title BuyerContract
+ *
+ * @notice Contract that acts as a buyer for testing purposes
+ */
+contract BuyerContract is IERC721Receiver {
+    enum FailType {
+        None,
+        Revert,
+        ReturnWrongSelector
+    }
+
+    FailType public failType;
+
+    /**
+     * @dev Set fail type
+     */
+    function setFailType(FailType _failType) external {
+        failType = _failType;
+    }
+
+    /**
+     * @dev Return wrong selector to test revert
+     */
+    function onERC721Received(
+        address operator,
+        address from,
+        uint256 tokenId,
+        bytes calldata data
+    ) external returns (bytes4) {
+        if (failType == FailType.Revert) revert("BuyerContract: revert");
+        if (failType == FailType.ReturnWrongSelector) return 0x12345678;
+        return IERC721Receiver.onERC721Received.selector;
+    }
+}

contracts/protocol/bases/PriceDiscoveryBase.sol

@@ -140,7 +140,7 @@ contract PriceDiscoveryBase is ProtocolBase {
             if (_bosonVoucher.ownerOf(_tokenId) != address(this)) revert VoucherNotReceived();
 
             // Transfer voucher to buyer
-            _bosonVoucher.transferFrom(address(this), _buyer, _tokenId);
+            _bosonVoucher.safeTransferFrom(address(this), _buyer, _tokenId);
         }
 
         uint256 overchargedAmount = _priceDiscovery.price - actualPrice;

contracts/protocol/facets/SellerHandlerFacet.sol

@@ -172,6 +172,8 @@ contract SellerHandlerFacet is SellerBase {
                             1;
                     }
                     royaltyRecipients.pop();
+
+                    delete royaltyRecipientIndexBySellerAndRecipient[_seller.treasury];
                 }
 
                 // Update treasury

contracts/protocol/facets/SequentialCommitHandlerFacet.sol

@@ -60,7 +60,7 @@ contract SequentialCommitHandlerFacet is IBosonSequentialCommitHandler, PriceDis
      *   - Calling transferFrom on token fails for some reason (e.g. protocol is not approved to transfer)
      *   - Received ERC20 token amount differs from the expected value
      *   - Protocol does not receive the voucher
-     *   - Transfer of voucher to the buyer fails for some reasong (e.g. buyer is contract that doesn't accept voucher)
+     *   - Transfer of voucher to the buyer fails for some reason (e.g. buyer is contract that doesn't accept voucher)
      *   - Reseller did not approve protocol to transfer exchange token in escrow
      * - Call to price discovery contract fails
      * - Protocol fee and royalties combined exceed the secondary price
@@ -174,7 +174,7 @@ contract SequentialCommitHandlerFacet is IBosonSequentialCommitHandler, PriceDis
                 }
 
                 if (payout > 0) {
-                    FundsLib.transferFundsFromProtocol(exchangeToken, payable(seller), payout); // also emits FundsWithdrawn
+                    FundsLib.transferFundsFromProtocol(exchangeToken, payable(seller), payout);
                 }
             }
         }

scripts/config/revert-reasons.js

@@ -175,6 +175,8 @@ exports.RevertReasons = {
   SAFE_ERC20_LOW_LEVEL_CALL: "SafeERC20: low-level call failed",
   SAFE_ERC20_OPERATION_FAILED: "SafeERC20: ERC20 operation did not succeed",
   INITIALIZABLE_ALREADY_INITIALIZED: "Initializable: contract is already initialized",
+  ERC721_NON_RECEIVER: "ERC721: transfer to non ERC721Receiver implementer",
+  BUYER_CONTRACT_REVERT: "BuyerContract: revert",
 
   // Meta-Transactions related
   NONCE_USED_ALREADY: "NonceUsedAlready",

test/protocol/SellerHandlerTest.js

@@ -3885,7 +3885,72 @@ describe("SellerHandler", function () {
           );
           expect(returnedRoyaltyRecipientList).to.deep.equal(
             expectedRoyaltyRecipientList,
-            "Default royalty recipient mismatch"
+            "Royalty recipient mismatch"
+          );
+        });
+
+        it("correctly handle treasury during the seller update", async function () {
+          // Add royalty recipients
+          await accountHandler.connect(admin).addRoyaltyRecipients(seller.id, royaltyRecipientList.toStruct());
+
+          // Update the seller, so one of the recipients is removed
+          seller.treasury = other1.address;
+          await accountHandler.connect(admin).updateSeller(seller, emptyAuthToken);
+
+          // other1 is not a recipient anymore
+          let returnedRoyaltyRecipientList = RoyaltyRecipientList.fromStruct(
+            await accountHandler.connect(rando).getRoyaltyRecipients(seller.id)
+          );
+
+          royaltyRecipientList = new RoyaltyRecipientList([
+            new RoyaltyRecipient(other3.address, "300", "other3"),
+            new RoyaltyRecipient(other2.address, "200", "other2"),
+          ]);
+
+          expectedRoyaltyRecipientList = new RoyaltyRecipientList([
+            new RoyaltyRecipient(ZeroAddress, voucherInitValues.royaltyPercentage, DEFAULT_ROYALTY_RECIPIENT),
+            ...royaltyRecipientList.royaltyRecipients,
+          ]);
+
+          expect(returnedRoyaltyRecipientList).to.deep.equal(
+            expectedRoyaltyRecipientList,
+            "Royalty recipient mismatch"
+          );
+
+          // other 1 now cannot be added as another recipient
+          royaltyRecipientList = new RoyaltyRecipientList([new RoyaltyRecipient(other1.address, "100", "other1")]);
+
+          // Adding other 1 should fail
+          await expect(
+            accountHandler.connect(admin).addRoyaltyRecipients(seller.id, royaltyRecipientList.toStruct())
+          ).to.revertedWithCustomError(bosonErrors, RevertReasons.RECIPIENT_NOT_UNIQUE);
+
+          // Update the seller again, so other 1 can later be added as a recipient
+          seller.treasury = rando.address;
+          await accountHandler.connect(admin).updateSeller(seller, emptyAuthToken);
+
+          // Now adding should succeed
+          await accountHandler.connect(admin).addRoyaltyRecipients(seller.id, royaltyRecipientList.toStruct());
+
+          // other1 is back on the list
+          returnedRoyaltyRecipientList = RoyaltyRecipientList.fromStruct(
+            await accountHandler.connect(rando).getRoyaltyRecipients(seller.id)
+          );
+
+          royaltyRecipientList = new RoyaltyRecipientList([
+            new RoyaltyRecipient(other3.address, "300", "other3"),
+            new RoyaltyRecipient(other2.address, "200", "other2"),
+            new RoyaltyRecipient(other1.address, "100", "other1"),
+          ]);
+
+          expectedRoyaltyRecipientList = new RoyaltyRecipientList([
+            new RoyaltyRecipient(ZeroAddress, voucherInitValues.royaltyPercentage, DEFAULT_ROYALTY_RECIPIENT),
+            ...royaltyRecipientList.royaltyRecipients,
+          ]);
+
+          expect(returnedRoyaltyRecipientList).to.deep.equal(
+            expectedRoyaltyRecipientList,
+            "Royalty recipient mismatch"
           );
         });
 

test/protocol/SequentialCommitHandlerTest.js

@@ -706,6 +706,55 @@ describe("IBosonSequentialCommitHandler", function () {
                   .sequentialCommitToOffer(buyer2.address, tokenId, priceDiscovery, { value: price2 })
               ).to.revertedWithCustomError(bosonErrors, RevertReasons.VOUCHER_NOT_RECEIVED);
             });
+
+            context("Buyer rejects the voucher", async function () {
+              it("Buyer contract does not implement the receiver", async function () {
+                const [buyerContract] = await deployMockTokens(["Foreign20"]);
+
+                // Sequential commit to offer, expect revert
+                await expect(
+                  sequentialCommitHandler
+                    .connect(rando)
+                    .sequentialCommitToOffer(await buyerContract.getAddress(), tokenId, priceDiscovery, {
+                      value: price2,
+                    })
+                ).to.revertedWith(RevertReasons.ERC721_NON_RECEIVER);
+              });
+
+              it("Buyer contract reverts with custom error", async function () {
+                const buyerContractFactory = await getContractFactory("BuyerContract");
+                const buyerContract = await buyerContractFactory.deploy();
+                await buyerContract.waitForDeployment();
+
+                await buyerContract.setFailType(1); // Type 1 = revert with own error
+
+                // Sequential commit to offer, expect revert
+                await expect(
+                  sequentialCommitHandler
+                    .connect(rando)
+                    .sequentialCommitToOffer(await buyerContract.getAddress(), tokenId, priceDiscovery, {
+                      value: price2,
+                    })
+                ).to.revertedWith(RevertReasons.BUYER_CONTRACT_REVERT);
+              });
+
+              it("Buyer contract returns the wrong selector", async function () {
+                const buyerContractFactory = await getContractFactory("BuyerContract");
+                const buyerContract = await buyerContractFactory.deploy();
+                await buyerContract.waitForDeployment();
+
+                await buyerContract.setFailType(2); // Type 2 = wrong selector
+
+                // Sequential commit to offer, expect revert
+                await expect(
+                  sequentialCommitHandler
+                    .connect(rando)
+                    .sequentialCommitToOffer(await buyerContract.getAddress(), tokenId, priceDiscovery, {
+                      value: price2,
+                    })
+                ).to.revertedWith(RevertReasons.ERC721_NON_RECEIVER);
+              });
+            });
           });
         });