diff --git a/contracts/interfaces/handlers/IBosonSequentialCommitHandler.sol b/contracts/interfaces/handlers/IBosonSequentialCommitHandler.sol index 1acc6f980..cd36403a5 100644 --- 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 new file mode 100644 index 000000000..6cb352112 --- /dev/null +++ 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 index 2b3f50356..f7b70d480 100644 --- 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 index 6b4b6d047..6336bfcfe 100644 --- 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 index 086a61bcd..6975f4035 100644 --- 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 index 3b52bba78..8a670c8b1 100644 --- 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); + }); + }); }); });