0xProject / ZEIPs

0x Improvement Proposals
Apache License 2.0
91 stars 24 forks source link

StaticCallAssetProxy #39

Closed dorothy-zbornak closed 4 years ago

dorothy-zbornak commented 5 years ago

Summary

We introduce a new asset proxy for validating an order bundle during settlement through an arbitrary, read-only callback.

Motivation

One issue when trading non-fungible items (kitties, composables, CDPs) is that many of these assets are stateful. Their value is derived from the state at a given point in time. If this state were to be modified, the value of the asset is also potentially modified.

We wish to implement a method to guarantee for the buyer of an asset that the state remains desirable throughout the entire settlement process.

StaticCallProxyAssetData Fields

struct StaticCallProxyAssetData {
    // Asset proxy ID.
    bytes4 assetProxyId, // bytes4(keccak256("StaticCallAssetProxy(...)")),
    // Address of the call target.
    address callTarget,
    // ABI-encoded call data to pass to the call target,
    // generated by `abi.encodeWithSelector()`. 
    bytes callData,
    // Keccak256 hash of the expected, ABI-encoded result of the call.
    // Will revert if the hash of the actual result does not match.
    bytes32 callResultHash,
}`

Usage

The StaticCallAssetProxy “asset” is intended to be mixed with real assets within a MultiAssetProxy bundle, such as:

// An asset bundle that transfers a crypto kitty and verifies that it is
// ready to breed.
MultiAssetProxyAssetData(
    // Amounts of each "asset". Because StaticCallAssetProxy assets 
    // cannot modify state, we should only call them once.
    [1, 1],
    // The "assets" in this bundle.
    [
        // The crypto ktty ERC721 asset to transfer.
        ERC721AssetProxyAssetData(CRYPTO_KITTIES_CONTRACT, kittyId),
        // Validation callback to verify that `kittyId` is ready to breed.
        StaticCallAssetProxy(
            // `callTarget`
            CRYPTO_KITTIES_CONTRACT,
            // `callData`
            abi.encodeWithSelector(
                // Call the `isReadyToBreed()` function.
                bytes4(keccak256("isReadyToBreed(bytes32)")),
                // Pass `kittyId`.
                kittyId
            ),
            // `callResult`: `isReadyToBreed(kittyId)` should return `true`.
            keccak256(abi.encode(true))
        )
    ]
)

When the MultiAssetProxy executes the transfer of this asset, the StaticCallAssetProxy will perform the following read-only check:

keccak256(assetData.callTarget.staticcall(assetData.callData)) == assetData.callResultHash 

If the above condition is not satisfied, the asset proxy will revert, aborting the entire trade. A maker can even chain multiple StaticCallAssetProxy assets inside the MultiAssetProxy bundle to perform multiple state checks, or they can use a bespoke contract to aggregate all checks (or perform more complex checks) in a single call.

Altogether, this allows the maker to establish some sort of state guarantees on the asset(s) being traded. And, since callbacks can access any on-chain state, we imagine some pretty creative uses could arise.

StaticCallAssetProxy Implementation

A pseudo-code implementation of the asset proxy would look like:

contract StaticCallAssetProxy is
    MixinAuthorizable
{
    /// @dev The main entry point for the asset proxy.
    function transferFrom(
        bytes StaticCallAssetProxyData assetData,
        // These parameters are ignored/have no relevance.
        address from,
        address to,
        uint256 amount
    )
        external
        view
        onlyAuthorized
    {
        (bool success, bytes returnData) = assetData.callTarget.staticcall(
            assetData.callData
        );

        // Abort if the call reverted or tried to modify state.
        require(success, returnData);

        // Abort if the hash of the return data does not match what is expected.
        require(keccak256(returnData) == callData.callResultHash);
    }
}

Example Use Cases

Challenges

AusIV commented 5 years ago

Looking at this with my order-book pruning hat on, this could be very challenging to support in a generic way. OpenRelay prunes our orderbook by monitoring events. If an order specifies a predicate function we've never seen, we don't even know if there are events we could monitor that would tell us when the predicate would invalidate the order, let alone what those events look like.

I could see implementing support for a specific set of predicate functions we know how to monitor, but as described I don't think we could support this in an open-ended way.

dekz commented 5 years ago

@AusIV This is definitely a challenge. It's a challenge we've already encountered with certain ERC20 tokens which can have Maker restrictions. In the wild we've seen "proof of use", "blacklisted" addresses which aren't picked up by standard events.

The best way to test the validity, and something that is already supported in 0x.js, is to simulate the underlying transfer.

We understand that there are tradeoffs in adding this behaviour, there are use cases where an event is not possible (post-dated orders).

Our job in the tooling is to also provide a succinct and efficient way of bulk checking these orders. Working towards standardised predicates will help keep this efficient.

pointtoken commented 5 years ago

While there are NFTs that keep their metadata onchain, the prevailing model for NFTs is to point to meta-data about the asset off-chain through a URI, so as to reduce gas costs. As such, the NFT use case for this ZEIP may not have as many uses without some kind of an oracle, which I imagine is beyond the scope of the ZEIP? Unless, we could perhaps specify how an oracle would be implemented, providing a standard, similar to how the tokenURI is (sort of) standardized with the ERC721 spec. Is that work worth doing as part of this ZEIP?

AusIV commented 5 years ago

The best way to test the validity, and something that is already supported in 0x.js, is to simulate the underlying transfer.

Right, and we always encourage people to validate orders before they submit them to the blockchain - there's always going to be the risk of latency meaning an order we serve is unfillable by the time a user tries to fill it, but we try not to keep unfillable orders on our books. If we allow these orders with arbitrary predicates, our only option would be to periodically retest them, which works if we just have a handful, but doesn't scale very well.

To be clear, I'm not opposing this feature at all. I think it's very useful in some narrower use cases, I just don't think there's any way to build an event-based order watcher that handles this asset type in a generic way, and I think this is the first 0x asset type where that's the case.

I am curious how the 0x order watcher plans to handle this.

willwarren89 commented 5 years ago

If we allow these orders with arbitrary predicates, our only option would be to periodically retest them, which works if we just have a handful, but doesn't scale very well.

I think it is unlikely that you will find a heterogeneous liquidity pool where some orders put these checks in place and others do not. My guess is that stateful assets will be offered on specialized, niche marketplaces that only accept orders if a specific set of checks are in place. The marketplaces will want to protect their users from trades producing unexpected results and so they will take care of these checks on their users' behalf.

abandeali1 commented 5 years ago

The from, to, and amount fields are currently ignored, but could be useful to the callback— for example, to check the KYC status of the taker.

How important is this use case for everyone? The current thinking is to actually split this out into 2 separate proxies in order to support this. The first would work exactly as described, but could not be used for whitelists of any kind because the taker is probably not known in advance. The second would expect the receiving contract to have a specific interface and would forward the from, to, and amount params. It could look something like:

function onStaticCallReceived(
    bytes calldata staticCallData,
    address from,
    address to,
    uint256 amount
)
    external
    view
{
    require(
        isWhitelisted[from],
        "FROM_NOT_WHITELISTED"
    );
    require(
        isWhitelisted[to],
        "TO_NOT_WHITELISTED"
    );
}

Alternatively, we can have a single proxy and just append from, to, and amount to the staticCallData so that it can be optionally used. This feels a bit hacky and encoding/decoding this type of assetData would get a little tricky.