Skip to content
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

StaticCallAssetProxy #39

Closed
dorothy-zbornak opened this issue May 3, 2019 · 6 comments
Closed

StaticCallAssetProxy #39

dorothy-zbornak opened this issue May 3, 2019 · 6 comments
Labels
protocol-transition-complete Protocol has transitioned to the new state vote-accepted Vote for this change has been accepted

Comments

@dorothy-zbornak
Copy link

dorothy-zbornak commented May 3, 2019

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

  • Protect a CDP from being liquidated and incurring a 13% liquidation fee by creating a 0x order to sell your CDP at a slight discount IF AND ONLY IF (IFF) the collateral-to-DAI ratio for your CDP is below 155%. Liquidation occurs when collateral-to-DAI falls below 150%. Note that this would rely on MakerDAO's on-chain ETH/DAI price feed or some other oracle solution. Similar to CDP Saver but native to 0x.
  • Purchase a Decentraland Estate IFF it contains specific parcels of LAND.
  • Offer to purchase a parcel of Decentraland LAND IFF neighboring parcel located at [x,y] is NOT owned by a specific address (“bad neighbor” clause).

Challenges

  • It is not very intuitive to treat this feature as an “asset,” as it can have no value. Without proper safeguarding, it's possible to trade a real asset for a worthless StaticCall “asset.”
  • Complex, non-aggregate state checks require deploying a contract.
  • The validity of an order can be much more difficult to anticipate in a general sense, as any state outside the assets being traded can render an order invalid. Issuing an eth_call fill is the only way to generally validate these asset bundles.
  • The maker dictates the state guarantees on the asset, which might not exactly align with the taker's interests. Some kind of Exchange wrapper/extension might be helpful in these cases.
  • 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.
@abandeali1 abandeali1 added 3.0 and removed 3.0 labels May 3, 2019
@dekz dekz changed the title [3.0] StaticCallAssetProxy StaticCallAssetProxy May 6, 2019
@AusIV
Copy link

AusIV commented May 7, 2019

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
Copy link
Member

dekz commented May 8, 2019

@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
Copy link

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
Copy link

AusIV commented May 8, 2019

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
Copy link
Contributor

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
Copy link
Member

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.

@dekz dekz added vote-started Voting for this ZEIP is on-going vote-accepted Vote for this change has been accepted and removed vote-started Voting for this ZEIP is on-going 3.0 labels Jul 22, 2019
@willwarren89 willwarren89 added the protocol-transition-complete Protocol has transitioned to the new state label Sep 7, 2019
@dekz dekz closed this as completed Sep 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
protocol-transition-complete Protocol has transitioned to the new state vote-accepted Vote for this change has been accepted
Projects
None yet
Development

No branches or pull requests

6 participants