Add OrderValidator and WalletOrderValidator signature strategies to Exchange

[dorothy-zbornak]

Description

This adds support for two new order signature strategies to the Exchange: SignatureType.OrderValidator and SignatureType.WalletOrderValidator.

The new signature callbacks are similar to the existing Validator and Wallet strategies, but instead of passing just a bytes32 orderHash to the validator's isValidSignature(), we also pass an Order order struct to the validating contract's isValidOrderSignature() function (see IWallet and IOrderValidator).

Here is a sample validator contract for use with either SignatureType.OrderValidator or SignatureType.WalletOrderValidator (they have identical function signatures):

pragma solidity ^0.5.5;
pragma experimental ABIEncoderV2;

contract OrderValidator {
    
    // 0x order structure
    struct Order {
        address makerAddress;
        address takerAddress;
        address feeRecipientAddress;
        address senderAddress;
        uint256 makerAssetAmount;
        uint256 takerAssetAmount;
        uint256 makerFee;
        uint256 takerFee;
        uint256 expirationTimeSeconds;
        uint256 salt;
        bytes makerAssetData;
        bytes takerAssetData;
    }

    /// @dev Validate an order AND signature.
    /// @param order The full order.
    /// @param orderHash The order hash.
    /// @param signature Signature data for the order.
    /// @return `true` if valid.
    function isValidOrderSignature(
        Order calldata order,
        bytes32 orderHash,
        bytes calldata signature
    )
        external 
        view
        returns (bool isValid)
    {
        // Validate the order
        // ...
        return true;
    }
}

Other notes:

• Just like SignatureType.Validator and SignatureType.Wallet, the validator contract is called via staticall() and will fail if the validator contract attempts to update state.
• OrderValidators must be registered for a maker ahead of time via setOrderValidatorApproval(), which is distinct from setSignatureValidatorApproval() used by Validator.
• The public function isValidSignature() has been supplanted by isValidHashSignature() and
  isValidOrderSignature().
• Transactions use isValidHashSignature(), so they do not support the new signature strategies (which wouldn't make sense anyway).

Testing instructions

Types of changes

• New feature (non-breaking change which adds functionality)

Checklist:

• Prefix PR title with [WIP] if necessary.
• Add tests to cover changes as needed.
• Update documentation as needed.
• Add new entries to the relevant CHANGELOG.jsons.

[coveralls]

Coverage decreased (-0.009%) to 84.342% when pulling 9d08d67 on feature/3.0/exchange/order-validator-signature into 07fe677 on 3.0.

[abandeali1]

At some point we should also add the Wallet equivalent of this signature type, but that can be in a separate PR.

[abandeali1]

orderHash and signerAddress both feel redundant to me since they can be derived from the order.

[abandeali1]

I can see an argument for keeping orderHash since it prevents duplication of the hash calculation, though.

[dorothy-zbornak]

Yeah, signerAddress can definitely go.

I see this as potentially supplanting the SignatureType.Validator scheme, so it's likely validators will want the orderHash anyway.

[abandeali1]

Using the same mapping for both types of validators could be a bit of a security flaw. What if a user wants to use one of the validation functions but not the other? This is probably also an argument for separating the interfaces into 2 different contracts (I know I said otherwise earlier - sorry for the thrash).

[dorothy-zbornak]

Fixed in new version.

[dorothy-zbornak]

At some point we should also add the Wallet equivalent of this signature type, but that can be in a separate PR.

I can do that in this one. The work is fairly similar. Stay tuned. 📺

[abandeali1]

A few small nits, then looks good to me!

[abandeali1]

I'm surprised this actually compiles. Will have to check out the most recent version of Solidity :)

[abandeali1]

This is never actually used internally, right? We should make it external if not.

[abandeali1]

Typo

[abandeali1]

Nit: I'd default to putting any function calls with >2 arguments onto multiple lines.

[abandeali1]

Same as above.