Merge pull request #1585 from thesandboxgame/custom-order-recipient

Custom order recipient

packages/marketplace/contracts/ExchangeCore.sol

@@ -170,16 +170,8 @@ abstract contract ExchangeCore is Initializable, ITransferManager {
         LibOrder.FillResult memory newFill = _parseOrdersSetFillEmitMatch(sender, orderLeft, orderRight);
 
         doTransfers(
-            ITransferManager.DealSide(
-                LibAsset.Asset(makeMatch, newFill.leftValue),
-                orderLeft.maker,
-                orderLeft.makeRecipient
-            ),
-            ITransferManager.DealSide(
-                LibAsset.Asset(takeMatch, newFill.rightValue),
-                orderRight.maker,
-                orderRight.makeRecipient
-            ),
+            ITransferManager.DealSide(LibAsset.Asset(makeMatch, newFill.leftValue), orderLeft.maker),
+            ITransferManager.DealSide(LibAsset.Asset(takeMatch, newFill.rightValue), orderRight.maker),
             LibAsset.getFeeSide(makeMatch.assetClass, takeMatch.assetClass)
         );
     }

packages/marketplace/contracts/OrderValidator.sol

@@ -42,7 +42,6 @@ contract OrderValidator is IOrderValidator, Initializable, EIP712Upgradeable, Wh
     /// @param sender Address of the order sender.
     function validate(LibOrder.Order calldata order, bytes memory signature, address sender) external view {
         require(order.maker != address(0), "no maker");
-        require(order.makeRecipient != address(0), "no recipient");
 
         LibOrder.validateOrderTime(order);
         _verifyWhitelists(order.makeAsset);

packages/marketplace/contracts/TransferManager.sol

@@ -98,11 +98,14 @@ abstract contract TransferManager is Initializable, ITransferManager {
             paymentSide = right;
             nftSide = left;
         }
+
+        (address paymentSideRecipient, address nftSideRecipient) = _getRecipients(paymentSide, nftSide);
+
         // Transfer NFT or left side if FeeSide.NONE
-        _transfer(nftSide.asset, nftSide.account, paymentSide.recipient);
+        _transfer(nftSide.asset, nftSide.account, paymentSideRecipient);
         // Transfer ERC20 or right side if FeeSide.NONE
         if (feeSide == LibAsset.FeeSide.NONE || _mustSkipFees(paymentSide.account)) {
-            _transfer(paymentSide.asset, paymentSide.account, nftSide.recipient);
+            _transfer(paymentSide.asset, paymentSide.account, nftSideRecipient);
         } else {
             _doTransfersWithFeesAndRoyalties(paymentSide, nftSide);
         }
@@ -138,6 +141,26 @@ abstract contract TransferManager is Initializable, ITransferManager {
         emit DefaultFeeReceiverSet(newDefaultFeeReceiver);
     }
 
+    function _getRecipients(
+        DealSide memory paymentSide,
+        DealSide memory nftSide
+    ) internal pure returns (address paymentSideRecipient, address nftSideRecipient) {
+        address decodedPaymentSideRecipient = LibAsset.decodeRecipient(paymentSide.asset.assetType);
+        address decodedNftSideRecipient = LibAsset.decodeRecipient(nftSide.asset.assetType);
+
+        if (decodedPaymentSideRecipient != address(0)) {
+            paymentSideRecipient = decodedPaymentSideRecipient;
+        } else {
+            paymentSideRecipient = paymentSide.account;
+        }
+
+        if (decodedNftSideRecipient != address(0)) {
+            nftSideRecipient = decodedNftSideRecipient;
+        } else {
+            nftSideRecipient = nftSide.account;
+        }
+    }
+
     /// @notice Transfer protocol fees and royalties.
     /// @param paymentSide DealSide of the fee-side order
     /// @param nftSide DealSide of the nft-side order
@@ -155,7 +178,8 @@ abstract contract TransferManager is Initializable, ITransferManager {
             remainder = _transferPercentage(remainder, paymentSide, defaultFeeReceiver, fees, PROTOCOL_FEE_MULTIPLIER);
         }
         if (remainder > 0) {
-            _transfer(LibAsset.Asset(paymentSide.asset.assetType, remainder), paymentSide.account, nftSide.recipient);
+            (, address nftSideRecipient) = _getRecipients(paymentSide, nftSide);
+            _transfer(LibAsset.Asset(paymentSide.asset.assetType, remainder), paymentSide.account, nftSideRecipient);
         }
     }
 
@@ -178,13 +202,14 @@ abstract contract TransferManager is Initializable, ITransferManager {
         DealSide memory nftSide
     ) internal returns (uint256) {
         (address token, uint256 tokenId) = LibAsset.decodeToken(nftSide.asset.assetType);
+        (, address nftSideRecipient) = _getRecipients(paymentSide, nftSide);
         IRoyaltiesProvider.Part[] memory royalties = royaltiesRegistry.getRoyalties(token, tokenId);
         uint256 totalRoyalties;
         uint256 len = royalties.length;
         for (uint256 i; i < len; i++) {
             IRoyaltiesProvider.Part memory r = royalties[i];
             totalRoyalties = totalRoyalties + r.basisPoints;
-            if (r.account == nftSide.recipient) {
+            if (r.account == nftSideRecipient) {
                 // We just skip the transfer because the nftSide will get the full payment anyway.
                 continue;
             }

packages/marketplace/contracts/interfaces/ITransferManager.sol

@@ -13,7 +13,6 @@ abstract contract ITransferManager {
     struct DealSide {
         LibAsset.Asset asset; // The asset associated with this side of the deal.
         address account; // The account address associated with this side of the deal.
-        address recipient; // The account address receiving the tokens
     }
 
     /// @notice Executes the asset transfers associated with two matched orders.

packages/marketplace/contracts/libraries/LibAsset.sol

@@ -104,4 +104,18 @@ library LibAsset {
     function decodeAddress(AssetType memory assetType) internal pure returns (address) {
         return abi.decode(assetType.data, (address));
     }
+
+    /// @notice Decode the recipient address from an AssetType.
+    /// @param assetType The asset type to decode.
+    /// @return The address of the recipient, or zero address if not present.
+    function decodeRecipient(AssetType memory assetType) internal pure returns (address) {
+        bytes memory data = assetType.data;
+        if (data.length == 96) {
+            // 3 * 32 bytes (address, uint256, address)
+            (, , address recipient) = abi.decode(data, (address, uint256, address));
+            return recipient;
+        } else {
+            return address(0);
+        }
+    }
 }

packages/marketplace/contracts/libraries/LibOrder.sol

@@ -11,7 +11,7 @@ import {LibMath} from "./LibMath.sol";
 library LibOrder {
     bytes32 internal constant ORDER_TYPEHASH =
         keccak256(
-            "Order(address maker,Asset makeAsset,address taker,Asset takeAsset,address makeRecipient,uint256 salt,uint256 start,uint256 end)Asset(AssetType assetType,uint256 value)AssetType(uint256 assetClass,bytes data)"
+            "Order(address maker,Asset makeAsset,address taker,Asset takeAsset,uint256 salt,uint256 start,uint256 end)Asset(AssetType assetType,uint256 value)AssetType(uint256 assetClass,bytes data)"
         );
 
     /// @dev Represents the structure of an order.
@@ -20,7 +20,6 @@ library LibOrder {
         LibAsset.Asset makeAsset; // Asset the maker is providing.
         address taker; // Address of the taker.
         LibAsset.Asset takeAsset; // Asset the taker is providing.
-        address makeRecipient; // recipient address for maker.
         uint256 salt; // Random number to ensure unique order hash.
         uint256 start; // Timestamp when the order becomes valid.
         uint256 end; // Timestamp when the order expires.
@@ -40,7 +39,6 @@ library LibOrder {
             keccak256(
                 abi.encode(
                     order.maker,
-                    order.makeRecipient,
                     LibAsset.hash(order.makeAsset.assetType),
                     LibAsset.hash(order.takeAsset.assetType),
                     order.salt
@@ -61,7 +59,6 @@ library LibOrder {
                     LibAsset.hash(order.makeAsset),
                     order.taker,
                     LibAsset.hash(order.takeAsset),
-                    order.makeRecipient,
                     order.salt,
                     order.start,
                     order.end

packages/marketplace/docs/Exchange.md

@@ -181,6 +181,22 @@ In the extreme cases:
   gets the benefit because she sells at 2000 MATIC each ASSET. Alice gets more
   MATIC than expected.
 
+### Custom recipients
+
+The protocol allows both buyer and seller to set custom recipients for the
+exchange.
+
+Inside the `LibAsset` library, the `Asset` struct contains a `data` field which
+is a `bytes` type. We encode the recipient address into that field.
+
+Look up `decodeRecipient` function in `LibAsset` to see how to decode the
+recipient address. Assets like ERC20 only have the token address in the data so
+we need to add a uint256 and a recipient address to make it 96 bytes.
+
+When transfers are happening, the recipient is decoded from the `Asset` struct,
+and if it's not the zero address, then the transfer will be done to the decoded
+recipient address otherwise it will be sent to the original deal side address.
+
 ### Order salt
 
 Orders contain a salt value that ensure each order is unique, even for the same

packages/marketplace/test/OrderValidator.test.ts

@@ -325,58 +325,6 @@ describe('OrderValidator.sol', function () {
     ).to.not.be.reverted;
   });
 
-  it('should not validate if recipient is zero', async function () {
-    const makerAsset = await AssetERC721(ERC721Contract, 100);
-    const takerAsset = await AssetERC20(ERC20Contract, 100);
-    const order = await OrderDefault(
-      user1,
-      makerAsset,
-      ZeroAddress,
-      takerAsset,
-      1,
-      0,
-      0,
-      ZeroAddress
-    );
-
-    const signature = await signOrder(order, user1, OrderValidatorAsUser);
-
-    await expect(
-      OrderValidatorAsUser.validate(order, signature, user2.getAddress())
-    ).to.be.revertedWith('no recipient');
-  });
-
-  it('should not validate if recipient is changed after signature', async function () {
-    const makerAsset = await AssetERC721(ERC721Contract, 100);
-    const takerAsset = await AssetERC20(ERC20Contract, 100);
-    const order = await OrderDefault(
-      user1,
-      makerAsset,
-      ZeroAddress,
-      takerAsset,
-      1,
-      0,
-      0
-    );
-
-    const signature = await signOrder(order, user1, OrderValidatorAsUser);
-
-    const newOrder = await OrderDefault(
-      user1,
-      makerAsset,
-      ZeroAddress,
-      takerAsset,
-      1,
-      0,
-      0,
-      await user2.getAddress()
-    );
-
-    await expect(
-      OrderValidatorAsUser.validate(newOrder, signature, user2.getAddress())
-    ).to.be.revertedWith('signature verification error');
-  });
-
   it('should not set permission for token if caller is not owner', async function () {
     await expect(OrderValidatorAsUser.enableRole(TSBRole)).to.revertedWith(
       `AccessControl: account ${(