New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MultiAssetProxy: Allow multiple assets per side of a single order #23

Open
abandeali1 opened this Issue Nov 11, 2018 · 0 comments

Comments

Projects
None yet
1 participant
@abandeali1
Copy link
Member

abandeali1 commented Nov 11, 2018

Summary

The current AssetPoxy contracts only allow an order to buy or sell a single asset at a time. Since makerAssetData and takerAssetData are byte arrays of arbitrary length, we can actually provide enough information to allow a maker to buy or sell multiple assets with a single order. The MultiAssetProxy will allow for this and is completely backwards compatible.

Motivation

  • This has immediate applications for NFTs. For example, selling booster packs of unique cards.
  • This can be used to sell baskets of tokens (e.g a basket of stablecoins).

Specification

Requirements

  • The MultiAssetProxy should support an an arbitrary number of transfers for any asset type that is supported within the protocol (currently or in the future).
  • The MultiAssetProxy should not require users to set new approvals.
  • Ideally, the MultiAssetProxy should allow for partial fills (i.e if a maker wants to sell 2000 ZRX and 4000 BAT at once, the order can still be partially filled).

Asset data encoding

If we wish to support partial fills, the MultiAssetProxy should expect data to be encoded using the following function signature:

// 0x94cfcdd7
MultiAsset(uint256[],bytes[])

These arguments will represent an array of amounts and an array of assetDatas, where an amount at index i will correspond to the assetData at index i.

Partial Fills

Note that the amounts encoded in the top level assetData will be multiplied by the order's amount before transferring the asset. If a maker wants to sell 2000 ZRX and 4000 BAT and allow for partial fills, then amounts will equal [1, 2] and order.makerAssetAmount will be 2000. If the maker does *not *want to allow partial fills, then amounts will equal [2000, 4000] and order.makerAssetAmount will be 1.

Architecture

The MultiAssetProxy will inherit MixinAssetProxyDispatcher and MixinAuthorizable. All other AssetProxy contracts should be registered within the MultiAssetProxy, and each AssetProxy contract must also authorize the MultiAssetProxy. The MultiAssetProxy must also authorize and be registered within the Exchange contract. When an order's assetData specifies the MultiAssetProxy, the chain of calls looks like:

  1. Taker calls Exchange contract
  2. Exchange calls MultiAssetProxy
  3. MultiAssetProxy calls AssetProxy contracts [0, ..., n]

Pseudocode

pragma solidity 0.5.0;
pragma experimental ABIEncoderV2;  

import "../../utils/LibBytes/LibBytes.sol";
import "../../utils/SafeMath/SafeMath.sol";
import "../Exchange/MixinAssetProxyDispatcher.sol";
import "./MixinAuthorizable.sol";

contract MultiAssetProxy is
    SafeMath,
    MixinAssetProxyDispatcher,
    MixinAuthorizable
{
    using LibBytes for bytes;

    // Id of this proxy.
    bytes4 constant internal PROXY_ID = bytes4(keccak256("MultiAsset(uint256[],bytes[])"));

    /// @dev Transfers assets. Either succeeds or throws.
    /// @param assetData Byte array encoded for the respective asset proxy.
    /// @param from Address to transfer asset from.
    /// @param to Address to transfer asset to.
    /// @param amount Amount of asset to transfer.
    function transferFrom(
        bytes assetData,
        address from,
        address to,
        uint256 amount
    )
        external
    {
        bytes memory data = assetData.slice(4, assetData.length - 1);

        // Note: this syntax is usable with Solidity ^0.5.0. 
        // We will likely do the decoding in assembly instead for efficiency.
        (uint256[] memory amounts, bytes[] memory nestedAssetData) = abi.decode(
            data,
            (uint256[], bytes[])
        );

        for (uint i = 0; i < amounts.length; i++) {
            uint256 totalAmount = safeMul(amount, amounts[i]);
            dispatchTransferFrom(
                nestedAssetData[i],
                from,
                to,
                totalAmount
            );
        }
    }

    /// @dev Gets the proxy id associated with the proxy address.
    /// @return Proxy id.
    function getProxyId()
        external
        pure
        returns (bytes4)
    {
        return PROXY_ID;
    }
}

Issues

This addition requires a lot of extra links to be created for each AssetProxy. The current structure is suboptimal for this due to the 2 week timelock and inability to batch transactions within the AssetProxyOwner. It is possible that there may be some transaction ordering dependencies in the future since each one must be executed individually. I propose that batch transaction functionality is added to the AssetProxyOwner, although this is not immediately necessary.

@abandeali1 abandeali1 referenced this issue Nov 13, 2018

Merged

Implement MultiAssetProxy #1224

0 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment