[SCH-01M] Incorrect Specification of Error & [SCH-04M] Duplicate Emis…

…sion of Event (#879) * Safe Transfer voucher to buyer during ask order * Remove redundant comment (#865) --------- Co-authored-by: Ludovic Levalleux <[email protected]>
bosonprotocol · Jan 9, 2024 · 06209b1 · 06209b1
1 parent e965d76
commit 06209b1
Show file tree

Hide file tree

Showing 6 changed files with 95 additions and 4 deletions.
diff --git a/contracts/interfaces/handlers/IBosonSequentialCommitHandler.sol b/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

diff --git a/contracts/mock/BuyerContract.sol b/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;
+    }
+}
diff --git a/contracts/protocol/bases/PriceDiscoveryBase.sol b/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;

diff --git a/contracts/protocol/facets/SequentialCommitHandlerFacet.sol b/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);
                 }
             }
         }

diff --git a/scripts/config/revert-reasons.js b/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",

diff --git a/test/protocol/SequentialCommitHandlerTest.js b/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);
+              });
+            });
           });
         });