[PDB-01M] Arbitrary External Contract Calls (#894)

* Make price discovery client

* pricediscovery unit tests and fixes

* Unit tests pass

* support erc721 unsafe transfer

* Protocol can move the vouchers not the client

* PriceDiscoveryAddress part of protocol addresses

* ConfigHandler tests

* Initialize price discovery address during upgrade

* Price discovery client tests

* Prevent wrong tokenId transfer + natspec update

* Fix failing unit tests

* Remove unused variables and clean up code

* Fix failing unit tests

* Applied review suggestions

* Remove commented code

contracts/domain/BosonErrors.sol

@@ -340,8 +340,8 @@ interface BosonErrors {
     error InteractionNotAllowed();
 
     // Price discovery related
-    // Price discovery returned a price that is too low
-    error PriceTooLow();
+    // Price discovery returned a price that does not match the expected one
+    error PriceMismatch();
     // Token id is mandatory for bid orders and wrappers
     error TokenIdMandatory();
     // Incoming token id does not match the expected one
@@ -362,6 +362,8 @@ interface BosonErrors {
     error NegativePriceNotAllowed();
     // Price discovery did not send the voucher to the protocol
     error VoucherNotReceived();
+    // Price discovery did not send the voucher from the protocol
+    error VoucherNotTransferred();
     // Either token with wrong id received or wrong voucher contract made the transfer
     error UnexpectedERC721Received();
     // Royalty fee exceeds the price

contracts/example/Sudoswap/SudoswapWrapper.sol

@@ -60,6 +60,7 @@ contract SudoswapWrapper is BosonTypes, Ownable, ERC721 {
     address private poolAddress;
     address private immutable factoryAddress;
     address private immutable protocolAddress;
+    address private immutable unwrapperAddress;
     address private immutable wethAddress;
 
     // Mapping from token ID to price. If pendingTokenId == tokenId, this is not the final price.
@@ -80,12 +81,14 @@ contract SudoswapWrapper is BosonTypes, Ownable, ERC721 {
         address _voucherAddress,
         address _factoryAddress,
         address _protocolAddress,
-        address _wethAddress
+        address _wethAddress,
+        address _unwrapperAddress
     ) ERC721(getVoucherName(_voucherAddress), getVoucherSymbol(_voucherAddress)) {
         voucherAddress = _voucherAddress;
         factoryAddress = _factoryAddress;
         protocolAddress = _protocolAddress;
         wethAddress = _wethAddress;
+        unwrapperAddress = _unwrapperAddress;
 
         // Approve pool to transfer wrapped vouchers
         _setApprovalForAll(address(this), _factoryAddress, true);
@@ -137,7 +140,7 @@ contract SudoswapWrapper is BosonTypes, Ownable, ERC721 {
         // Either contract owner or protocol can unwrap
         // If contract owner is unwrapping, this is equivalent to removing the voucher from the pool
         require(
-            msg.sender == protocolAddress || wrappedVoucherOwner == msg.sender,
+            msg.sender == unwrapperAddress || wrappedVoucherOwner == msg.sender,
             "SudoswapWrapper: Only owner or protocol can unwrap"
         );
 
@@ -152,7 +155,7 @@ contract SudoswapWrapper is BosonTypes, Ownable, ERC721 {
         // Transfer token to protocol
         if (priceToPay > 0) {
             // This example only supports WETH
-            IERC20(cachedExchangeToken[_tokenId]).safeTransfer(protocolAddress, priceToPay);
+            IERC20(cachedExchangeToken[_tokenId]).safeTransfer(unwrapperAddress, priceToPay);
         }
 
         delete cachedExchangeToken[_tokenId]; // gas refund

contracts/example/ZoraWrapper/ZoraWrapper.sol

@@ -50,6 +50,7 @@ contract ZoraWrapper is BosonTypes, ERC721, IERC721Receiver {
     address private immutable voucherAddress;
     address private immutable zoraAuctionHouseAddress;
     address private immutable protocolAddress;
+    address private immutable unwrapperAddress;
     address private immutable wethAddress;
 
     // Token ID for which the price is not yet known
@@ -73,12 +74,14 @@ contract ZoraWrapper is BosonTypes, ERC721, IERC721Receiver {
         address _voucherAddress,
         address _zoraAuctionHouseAddress,
         address _protocolAddress,
-        address _wethAddress
+        address _wethAddress,
+        address _unwrapperAddress
     ) ERC721(getVoucherName(_voucherAddress), getVoucherSymbol(_voucherAddress)) {
         voucherAddress = _voucherAddress;
         zoraAuctionHouseAddress = _zoraAuctionHouseAddress;
         protocolAddress = _protocolAddress;
         wethAddress = _wethAddress;
+        unwrapperAddress = _unwrapperAddress;
 
         // Approve Zora Auction House to transfer wrapped vouchers
         _setApprovalForAll(address(this), _zoraAuctionHouseAddress, true);
@@ -132,7 +135,7 @@ contract ZoraWrapper is BosonTypes, ERC721, IERC721Receiver {
         // Either contract owner or protocol can unwrap
         // If contract owner is unwrapping, this is equivalent to canceled auction
         require(
-            msg.sender == protocolAddress || wrappedVoucherOwner == msg.sender,
+            msg.sender == unwrapperAddress || wrappedVoucherOwner == msg.sender,
             "ZoraWrapper: Only owner or protocol can unwrap"
         );
 
@@ -151,7 +154,7 @@ contract ZoraWrapper is BosonTypes, ERC721, IERC721Receiver {
         // Transfer token to protocol
         if (priceToPay > 0) {
             // No need to handle native separately, since Zora Auction House always sends WETH
-            IERC20(cachedExchangeToken[_tokenId]).safeTransfer(protocolAddress, priceToPay);
+            IERC20(cachedExchangeToken[_tokenId]).safeTransfer(unwrapperAddress, priceToPay);
         }
 
         delete cachedExchangeToken[_tokenId]; // gas refund

contracts/interfaces/clients/IBosonPriceDiscovery.sol

@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.22;
+
+import { IBosonVoucher } from "./IBosonVoucher.sol";
+import { IERC721Receiver } from "../IERC721Receiver.sol";
+import { BosonTypes } from "../../domain/BosonTypes.sol";
+
+/**
+ * @title BosonPriceDiscovery
+ *
+ * @notice This is the interface for the Boson Price Discovery contract.
+ *
+ * The ERC-165 identifier for this interface is: 0x8bcce417
+ */
+interface IBosonPriceDiscovery is IERC721Receiver {
+    /**
+     * @notice Fulfils an ask order on external contract.
+     *
+     * Reverts if:
+     * - Call to price discovery contract fails
+     * - The implied price is negative
+     * - Any external calls to erc20 contract fail
+     *
+     * @param _exchangeToken - the address of the exchange contract
+     * @param _priceDiscovery - the fully populated BosonTypes.PriceDiscovery struct
+     * @param _bosonVoucher - the boson voucher contract
+     * @param _msgSender - the address of the caller, as seen in boson protocol
+     * @return actualPrice - the actual price of the order
+     */
+    function fulfilAskOrder(
+        address _exchangeToken,
+        BosonTypes.PriceDiscovery calldata _priceDiscovery,
+        IBosonVoucher _bosonVoucher,
+        address payable _msgSender
+    ) external returns (uint256 actualPrice);
+
+    /**
+     * @notice Fulfils a bid order on external contract.
+     *
+     * Reverts if:
+     * - Call to price discovery contract fails
+     * - Protocol balance change after price discovery call is lower than the expected price
+     * - This contract is still owner of the voucher
+     * - Token id sent to buyer and token id set by the caller don't match
+     * - The implied price is negative
+     * - Any external calls to erc20 contract fail
+     *
+     * @param _tokenId - the id of the token
+     * @param _exchangeToken - the address of the exchange token
+     * @param _priceDiscovery - the fully populated BosonTypes.PriceDiscovery struct
+     * @param _seller - the seller's address
+     * @param _bosonVoucher - the boson voucher contract
+     * @return actualPrice - the actual price of the order
+     */
+    function fulfilBidOrder(
+        uint256 _tokenId,
+        address _exchangeToken,
+        BosonTypes.PriceDiscovery calldata _priceDiscovery,
+        address _seller,
+        IBosonVoucher _bosonVoucher
+    ) external payable returns (uint256 actualPrice);
+
+    /**
+     * @notice Call `unwrap` (or equivalent) function on the price discovery contract.
+     *
+     * Reverts if:
+     * - Protocol balance doesn't increase by the expected amount.
+     * - Token id sent to buyer and token id set by the caller don't match
+     * - The wrapper contract sends back the native currency
+     * - The implied price is negative
+     *
+     * @param _exchangeToken - the address of the exchange contract
+     * @param _priceDiscovery - the fully populated BosonTypes.PriceDiscovery struct
+     * @return actualPrice - the actual price of the order
+     */
+    function handleWrapper(
+        address _exchangeToken,
+        BosonTypes.PriceDiscovery calldata _priceDiscovery
+    ) external payable returns (uint256 actualPrice);
+}

contracts/interfaces/events/IBosonConfigEvents.sol

@@ -13,6 +13,7 @@ interface IBosonConfigEvents {
     event TreasuryAddressChanged(address indexed treasuryAddress, address indexed executedBy);
     event VoucherBeaconAddressChanged(address indexed voucherBeaconAddress, address indexed executedBy);
     event BeaconProxyAddressChanged(address indexed beaconProxyAddress, address indexed executedBy);
+    event PriceDiscoveryAddressChanged(address indexed priceDiscoveryAddress, address indexed executedBy);
     event ProtocolFeePercentageChanged(uint256 feePercentage, address indexed executedBy);
     event ProtocolFeeFlatBosonChanged(uint256 feeFlatBoson, address indexed executedBy);
     event MaxEscalationResponsePeriodChanged(uint256 maxEscalationResponsePeriod, address indexed executedBy);

contracts/interfaces/handlers/IBosonConfigHandler.sol

@@ -10,7 +10,7 @@ import { IBosonConfigEvents } from "../events/IBosonConfigEvents.sol";
  *
  * @notice Handles management of configuration within the protocol.
  *
- * The ERC-165 identifier for this interface is: 0x7899c7b9
+ * The ERC-165 identifier for this interface is: 0xe27f0773
  */
 interface IBosonConfigHandler is IBosonConfigEvents, BosonErrors {
     /**
@@ -93,6 +93,26 @@ interface IBosonConfigHandler is IBosonConfigEvents, BosonErrors {
      */
     function getBeaconProxyAddress() external view returns (address);
 
+    /**
+     * @notice Sets the Boson Price Discovery contract address.
+     *
+     * Emits a PriceDiscoveryAddressChanged event if successful.
+     *
+     * Reverts if _priceDiscovery is the zero address
+     *
+     * @dev Caller must have ADMIN role.
+     *
+     * @param _priceDiscovery - the Boson Price Discovery contract address
+     */
+    function setPriceDiscoveryAddress(address _priceDiscovery) external;
+
+    /**
+     * @notice Gets the Boson Price Discovery contract address.
+     *
+     * @return the Boson Price Discovery contract address
+     */
+    function getPriceDiscoveryAddress() external view returns (address);
+
     /**
      * @notice Sets the protocol fee percentage.
      *

contracts/mock/MockWrapper.sol

@@ -25,6 +25,7 @@ contract MockWrapper is BosonTypes, ERC721, IERC721Receiver {
     address private immutable voucherAddress;
     address private immutable mockAuctionAddress;
     address private immutable protocolAddress;
+    address private immutable unwrapperAddress;
     address private immutable wethAddress;
 
     // Token ID for which the price is not yet known
@@ -48,12 +49,14 @@ contract MockWrapper is BosonTypes, ERC721, IERC721Receiver {
         address _voucherAddress,
         address _mockAuctionAddress,
         address _protocolAddress,
-        address _wethAddress
+        address _wethAddress,
+        address _unwrapperAddress
     ) ERC721(getVoucherName(_voucherAddress), getVoucherSymbol(_voucherAddress)) {
         voucherAddress = _voucherAddress;
         mockAuctionAddress = _mockAuctionAddress;
         protocolAddress = _protocolAddress;
         wethAddress = _wethAddress;
+        unwrapperAddress = _unwrapperAddress;
 
         // Approve Mock Auction to transfer wrapped vouchers
         _setApprovalForAll(address(this), _mockAuctionAddress, true);
@@ -107,7 +110,7 @@ contract MockWrapper is BosonTypes, ERC721, IERC721Receiver {
         // Either contract owner or protocol can unwrap
         // If contract owner is unwrapping, this is equivalent to canceled auction
         require(
-            msg.sender == protocolAddress || wrappedVoucherOwner == msg.sender,
+            msg.sender == unwrapperAddress || wrappedVoucherOwner == msg.sender,
             "MockWrapper: Only owner or protocol can unwrap"
         );
 
@@ -125,7 +128,7 @@ contract MockWrapper is BosonTypes, ERC721, IERC721Receiver {
 
         // Transfer token to protocol
         if (priceToPay > 0) {
-            IERC20(cachedExchangeToken[_tokenId]).safeTransfer(protocolAddress, priceToPay);
+            IERC20(cachedExchangeToken[_tokenId]).safeTransfer(unwrapperAddress, priceToPay);
         }
 
         delete cachedExchangeToken[_tokenId]; // gas refund

contracts/mock/PriceDiscovery.sol

@@ -13,7 +13,7 @@ import { IERC721Receiver } from "../interfaces/IERC721Receiver.sol";
  * This contract simulates external price discovery mechanism.
  * When user commits to an offer, protocol talks to this contract to validate the exchange.
  */
-contract PriceDiscovery {
+contract PriceDiscoveryMock {
     struct Order {
         address seller;
         address buyer;
@@ -100,14 +100,65 @@ contract PriceDiscovery {
         // return half of the sent value back to the caller
         payable(msg.sender).transfer(msg.value / 2);
     }
+
+    event MockFulfilCalled();
+
+    function mockFulfil(uint256 _percentReturn) public payable virtual {
+        if (orderType == OrderType.Ask) {
+            // received value must be equal to the price (or greater for ETH)
+            if (order.exchangeToken == address(0)) {
+                require(msg.value >= order.price, "ETH value mismatch");
+                if (_percentReturn > 0) {
+                    payable(msg.sender).transfer((msg.value * _percentReturn) / 100);
+                }
+            } else {
+                IERC20(order.exchangeToken).transferFrom(msg.sender, address(this), order.price);
+                if (_percentReturn > 0) {
+                    IERC20(order.exchangeToken).transfer(msg.sender, (order.price * _percentReturn) / 100);
+                }
+            }
+        } else {
+            // handling bid and wrapper is the same for test purposes
+            if (order.exchangeToken == address(0)) {
+                if (_percentReturn > 0) {
+                    payable(msg.sender).transfer((order.price * _percentReturn) / 100);
+                }
+            } else {
+                if (_percentReturn > 0) {
+                    IERC20(order.exchangeToken).transfer(msg.sender, (order.price * _percentReturn) / 100);
+                }
+            }
+
+            if (orderType == OrderType.Bid) {
+                IERC721(order.voucherContract).transferFrom(msg.sender, order.buyer, order.tokenId);
+            }
+        }
+
+        emit MockFulfilCalled();
+    }
+
+    Order public order;
+    OrderType public orderType;
+    enum OrderType {
+        Ask,
+        Bid,
+        Weapper
+    }
+
+    function setExpectedValues(Order calldata _order, OrderType _orderType) public payable virtual {
+        order = _order;
+        orderType = _orderType;
+    }
+
+    receive() external payable {}
 }
 
 /**
  * @dev Simple bad price discovery contract used in tests
  *
  * This contract modifies the token id, simulates bad/malicious contract
  */
-contract PriceDiscoveryModifyTokenId is PriceDiscovery {
+contract PriceDiscoveryModifyTokenId is PriceDiscoveryMock {
     /**
      * @dev simple fulfillOrder that does not perform any checks
      * Bump token id by 1
@@ -123,7 +174,7 @@ contract PriceDiscoveryModifyTokenId is PriceDiscovery {
  *
  * This contract modifies the erc721 token, simulates bad/malicious contract
  */
-contract PriceDiscoveryModifyVoucherContract is PriceDiscovery {
+contract PriceDiscoveryModifyVoucherContract is PriceDiscoveryMock {
     Foreign721 private erc721;
 
     constructor(address _erc721) {
@@ -149,7 +200,7 @@ contract PriceDiscoveryModifyVoucherContract is PriceDiscovery {
  *
  * This contract simply does not transfer the voucher to the caller
  */
-contract PriceDiscoveryNoTransfer is PriceDiscovery {
+contract PriceDiscoveryNoTransfer is PriceDiscoveryMock {
     /**
      * @dev do nothing
      */
@@ -161,7 +212,7 @@ contract PriceDiscoveryNoTransfer is PriceDiscovery {
  *
  * This contract transfers the voucher to itself instead of the origina msg.sender
  */
-contract PriceDiscoveryTransferElsewhere is PriceDiscovery, IERC721Receiver {
+contract PriceDiscoveryTransferElsewhere is PriceDiscoveryMock, IERC721Receiver {
     /**
      * @dev invoke fulfilBuyOrder on itself, making it the msg.sender
      */