Adding signatures for maker acceptance into pool

2019-06-21 16:35:52 -07:00
parent e2a2f932f1
commit 1f2e94b585
9 changed files with 324 additions and 37 deletions
--- a/contracts/staking/contracts/src/core/MixinPools.sol
+++ b/contracts/staking/contracts/src/core/MixinPools.sol
@@ -23,13 +23,15 @@ import "@0x/contracts-utils/contracts/src/SafeMath.sol";
 import "../immutable/MixinConstants.sol";
 import "../interfaces/IStakingEvents.sol";
 import "./MixinRewardVault.sol";
+import "./MixinSignatureValidator.sol";

 contract MixinPools is
    SafeMath,
    IStakingEvents,
    MixinConstants,
    MixinStorage,
-    MixinRewardVault
+    MixinRewardVault,
+    MixinSignatureValidator
 {

    function _getNextPoolId()
@@ -65,7 +67,8 @@ contract MixinPools is
    function _addMakerToPool(
        bytes32 poolId,
        address makerAddress,
-        bytes memory makeSignature,
+        bytes32 hashSignedByMaker,
+        bytes memory makerSignature,
        address operatorAddress
    )
        internal
@@ -75,6 +78,11 @@ contract MixinPools is
            "BAD_POOL_OPERATOR"
        );

+        require(
+            _isValidSignature(hashSignedByMaker, makerAddress, makerSignature),
+            "INVALID_MAKER_SIGNATURE"
+        );
+
        _recordMaker(poolId, makerAddress);
    }

--- a/contracts/staking/contracts/src/core/MixinSignatureValidator.sol
+++ b/contracts/staking/contracts/src/core/MixinSignatureValidator.sol
@@ -0,0 +1,197 @@
+/*
+  Copyright 2018 ZeroEx Intl.
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+    http://www.apache.org/licenses/LICENSE-2.0
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+*/
+
+pragma solidity ^0.5.5;
+
+import "@0x/contracts-utils/contracts/src/LibBytes.sol";
+import "../interfaces/IStructs.sol";
+import "../interfaces/IWallet.sol";
+
+
+contract MixinSignatureValidator is
+    IStructs
+{
+    using LibBytes for bytes;
+
+    /// @dev Verifies that a hash has been signed by the given signer.
+    /// @param hash Any 32 byte hash.
+    /// @param signerAddress Address that should have signed the given hash.
+    /// @param signature Proof that the hash has been signed by signer.
+    /// @return True if the address recovered from the provided signature matches the input signer address.
+    function _isValidSignature(
+        bytes32 hash,
+        address signerAddress,
+        bytes memory signature
+    )
+        internal
+        view
+        returns (bool isValid)
+    {
+        require(
+            signature.length > 0,
+            "LENGTH_GREATER_THAN_0_REQUIRED"
+        );
+
+        // Pop last byte off of signature byte array.
+        uint8 signatureTypeRaw = uint8(signature.popLastByte());
+
+        // Ensure signature is supported
+        require(
+            signatureTypeRaw < uint8(SignatureType.NSignatureTypes),
+            "SIGNATURE_UNSUPPORTED"
+        );
+
+        SignatureType signatureType = SignatureType(signatureTypeRaw);
+
+        // Variables are not scoped in Solidity.
+        uint8 v;
+        bytes32 r;
+        bytes32 s;
+        address recovered;
+
+        // Always illegal signature.
+        // This is always an implicit option since a signer can create a
+        // signature array with invalid type or length. We may as well make
+        // it an explicit option. This aids testing and analysis. It is
+        // also the initialization value for the enum type.
+        if (signatureType == SignatureType.Illegal) {
+            revert("SIGNATURE_ILLEGAL");
+
+        // Always invalid signature.
+        // Like Illegal, this is always implicitly available and therefore
+        // offered explicitly. It can be implicitly created by providing
+        // a correctly formatted but incorrect signature.
+        } else if (signatureType == SignatureType.Invalid) {
+            require(
+                signature.length == 0,
+                "LENGTH_0_REQUIRED"
+            );
+            isValid = false;
+            return isValid;
+
+        // Signature using EIP712
+        } else if (signatureType == SignatureType.EIP712) {
+            require(
+                signature.length == 65,
+                "LENGTH_65_REQUIRED"
+            );
+            v = uint8(signature[0]);
+            r = signature.readBytes32(1);
+            s = signature.readBytes32(33);
+            recovered = ecrecover(
+                hash,
+                v,
+                r,
+                s
+            );
+            isValid = signerAddress == recovered;
+            return isValid;
+
+        // Signed using web3.eth_sign
+        } else if (signatureType == SignatureType.EthSign) {
+            require(
+                signature.length == 65,
+                "LENGTH_65_REQUIRED"
+            );
+            v = uint8(signature[0]);
+            r = signature.readBytes32(1);
+            s = signature.readBytes32(33);
+            recovered = ecrecover(
+                keccak256(abi.encodePacked(
+                    "\x19Ethereum Signed Message:\n32",
+                    hash
+                )),
+                v,
+                r,
+                s
+            );
+            isValid = signerAddress == recovered;
+            return isValid;
+
+        // Signature verified by wallet contract.
+        // If used with an order, the maker of the order is the wallet contract.
+        } else if (signatureType == SignatureType.Wallet) {
+            isValid = isValidWalletSignature(
+                hash,
+                signerAddress,
+                signature
+            );
+            return isValid;
+
+        // Signature verified by wallet contract.
+        // If used with an order, the maker of the order is the wallet contract.
+        } else if (signatureType == SignatureType.Wallet) {
+            isValid = isValidWalletSignature(
+                hash,
+                signerAddress,
+                signature
+            );
+            return isValid;
+        }
+
+        // Anything else is illegal (We do not return false because
+        // the signature may actually be valid, just not in a format
+        // that we currently support. In this case returning false
+        // may lead the caller to incorrectly believe that the
+        // signature was invalid.)
+        revert("SIGNATURE_UNSUPPORTED");
+    }
+
+    /// @dev Verifies signature using logic defined by Wallet contract.
+    /// @param hash Any 32 byte hash.
+    /// @param walletAddress Address that should have signed the given hash
+    ///                      and defines its own signature verification method.
+    /// @param signature Proof that the hash has been signed by signer.
+    /// @return True if signature is valid for given wallet..
+    function isValidWalletSignature(
+        bytes32 hash,
+        address walletAddress,
+        bytes memory signature
+    )
+        internal
+        view
+        returns (bool isValid)
+    {
+        bytes memory callData = abi.encodeWithSelector(
+            IWallet(walletAddress).isValidSignature.selector,
+            hash,
+            signature
+        );
+        assembly {
+            let cdStart := add(callData, 32)
+            let success := staticcall(
+                gas,              // forward all gas
+                walletAddress,    // address of Wallet contract
+                cdStart,          // pointer to start of input
+                mload(callData),  // length of input
+                cdStart,          // write output over input
+                32                // output size is 32 bytes
+            )
+
+            switch success
+            case 0 {
+                // Revert with `Error("WALLET_ERROR")`
+                mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000)
+                mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000)
+                mstore(64, 0x0000000c57414c4c45545f4552524f5200000000000000000000000000000000)
+                mstore(96, 0)
+                revert(0, 100)
+            }
+            case 1 {
+                // Signature is valid if call did not revert and returned true
+                isValid := mload(cdStart)
+            }
+        }
+        return isValid;
+    }
+}
--- a/contracts/staking/contracts/src/interfaces/IStructs.sol
+++ b/contracts/staking/contracts/src/interfaces/IStructs.sol
@@ -21,6 +21,16 @@ pragma solidity ^0.5.5;

 interface IStructs {

+    // Allowed signature types.
+    enum SignatureType {
+        Illegal,            // 0x00, default value
+        Invalid,            // 0x01
+        EIP712,             // 0x02
+        EthSign,            // 0x03
+        Wallet,             // 0x04
+        NSignatureTypes     // 0x05, number of signature types. Always leave at end.
+    }
+
    struct Timelock {
        uint64 lockedAt;
        uint96 total;
--- a/contracts/staking/contracts/src/interfaces/IWallet.sol
+++ b/contracts/staking/contracts/src/interfaces/IWallet.sol
@@ -0,0 +1,30 @@
+/*
+  Copyright 2018 ZeroEx Intl.
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+    http://www.apache.org/licenses/LICENSE-2.0
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+*/
+
+pragma solidity ^0.5.5;
+
+
+interface IWallet /* is EIP-1271 */ {
+
+    /// @dev Verifies that a signature is valid.
+    /// @param hash Message hash that is signed.
+    /// @param signature Proof of signing.
+    /// @return The function selector for this function
+    function isValidSignature(
+        bytes32 hash,
+        bytes calldata signature
+    )
+        external
+        view
+        returns (bytes32 isValidSignatureSelector);
+}
--- a/contracts/staking/contracts/src/vaults/RewardVault.sol
+++ b/contracts/staking/contracts/src/vaults/RewardVault.sol
@@ -31,6 +31,7 @@ contract RewardVault is
    MixinConstants,
    MixinVaultCore
 {
+    // @TODO -- ADD README's TO EACH DIRECTORY

    using LibSafeMath for uint256;
    using LibSafeMath96Bit for uint96;
--- a/contracts/staking/contracts/src/wrappers/MixinPoolsWrapper.sol
+++ b/contracts/staking/contracts/src/wrappers/MixinPoolsWrapper.sol
@@ -46,6 +46,7 @@ contract MixinPoolsWrapper is
    function addMakerToPool(
        bytes32 poolId,
        address makerAddress,
+        bytes32 makerSignedHash,
        bytes calldata makerSignature
    )
        external
@@ -54,6 +55,7 @@ contract MixinPoolsWrapper is
        _addMakerToPool(
            poolId,
            makerAddress,
+            makerSignedHash,
            makerSignature,
            msg.sender
        );
--- a/contracts/staking/contracts/src/wrappers/MixinSignatureValidatorWrapper.sol
+++ b/contracts/staking/contracts/src/wrappers/MixinSignatureValidatorWrapper.sol
@@ -0,0 +1,39 @@
+/*
+  Copyright 2018 ZeroEx Intl.
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+    http://www.apache.org/licenses/LICENSE-2.0
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+*/
+
+pragma solidity ^0.5.5;
+
+import "../core/MixinSignatureValidator.sol";
+
+
+contract MixinSignatureValidatorWrapper is
+    MixinSignatureValidator
+{
+
+    /// @dev Verifies that a hash has been signed by the given signer.
+    /// @param hash Any 32 byte hash.
+    /// @param signerAddress Address that should have signed the given hash.
+    /// @param signature Proof that the hash has been signed by signer.
+    /// @return True if the address recovered from the provided signature matches the input signer address.
+    function isValidSignature(
+        bytes32 hash,
+        address signerAddress,
+        bytes memory signature
+    )
+        internal
+        view
+        returns (bool isValid)
+    {
+        return _isValidSignature(hash, signerAddress, signature);
+    }
+}
--- a/contracts/staking/test/core_test.ts
+++ b/contracts/staking/test/core_test.ts
@@ -609,14 +609,14 @@ describe('Staking Core', () => {
            ];
            await Promise.all([
                // pool 0
-                stakingWrapper.addMakerToPoolAsync(poolIds[0], makersByPoolId[0][0], "0x00", poolOperators[0]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[0], makersByPoolId[0][0], "0x00", "0x00", poolOperators[0]),
                // pool 1
-                stakingWrapper.addMakerToPoolAsync(poolIds[1], makersByPoolId[1][0], "0x00", poolOperators[1]),
-                stakingWrapper.addMakerToPoolAsync(poolIds[1], makersByPoolId[1][1], "0x00", poolOperators[1]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[1], makersByPoolId[1][0], "0x00", "0x00", poolOperators[1]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[1], makersByPoolId[1][1], "0x00", "0x00", poolOperators[1]),
                // pool 2
-                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][0], "0x00", poolOperators[2]),
-                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][1], "0x00", poolOperators[2]),
-                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][2], "0x00", poolOperators[2]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][0], "0x00", "0x00", poolOperators[2]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][1], "0x00", "0x00", poolOperators[2]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][2], "0x00", "0x00", poolOperators[2]),
            ]);
            ///// 2 PAY FEES /////
            await Promise.all([
@@ -787,7 +787,7 @@ describe('Staking Core', () => {
            // add maker to pool
            const makerAddress = makers[0];
            const makerSignature = "0x";
-            await stakingWrapper.addMakerToPoolAsync(poolId, makerAddress, makerSignature, operatorAddress);
+            await stakingWrapper.addMakerToPoolAsync(poolId, makerAddress, "0x00", makerSignature, operatorAddress);
            // check the pool id of the maker
            const poolIdOfMaker = await stakingWrapper.getMakerPoolId(makerAddress);
            expect(poolIdOfMaker).to.be.equal(poolId);
@@ -796,7 +796,7 @@ describe('Staking Core', () => {
            expect(makerAddressesForPool).to.be.deep.equal([makerAddress]);
            // try to add the same maker address again
            await expectTransactionFailedAsync(
-                stakingWrapper.addMakerToPoolAsync(poolId, makerAddress, makerSignature, operatorAddress),
+                stakingWrapper.addMakerToPoolAsync(poolId, makerAddress, "0x00", makerSignature, operatorAddress),
                RevertReason.MakerAddressAlreadyRegistered
            );
            // try to add a new maker address from an address other than the pool operator
@@ -804,7 +804,7 @@ describe('Staking Core', () => {
            const anotherMakerAddress = makers[1];
            const anotherMakerSignature = "0x";
            await expectTransactionFailedAsync(
-                stakingWrapper.addMakerToPoolAsync(poolId, anotherMakerAddress, anotherMakerSignature, notOperatorAddress),
+                stakingWrapper.addMakerToPoolAsync(poolId, anotherMakerAddress, "0x00", anotherMakerSignature, notOperatorAddress),
                RevertReason.OnlyCallableByPoolOperator
            );
            // try to remove the maker address from an address other than the operator
@@ -860,14 +860,14 @@ describe('Staking Core', () => {
            ];
            await Promise.all([
                // pool 0
-                stakingWrapper.addMakerToPoolAsync(poolIds[0], makersByPoolId[0][0], "0x00", poolOperators[0]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[0], makersByPoolId[0][0], "0x00", "0x00", poolOperators[0]),
                // pool 1
-                stakingWrapper.addMakerToPoolAsync(poolIds[1], makersByPoolId[1][0], "0x00", poolOperators[1]),
-                stakingWrapper.addMakerToPoolAsync(poolIds[1], makersByPoolId[1][1], "0x00", poolOperators[1]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[1], makersByPoolId[1][0], "0x00", "0x00", poolOperators[1]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[1], makersByPoolId[1][1], "0x00", "0x00", poolOperators[1]),
                // pool 2
-                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][0], "0x00", poolOperators[2]),
-                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][1], "0x00", poolOperators[2]),
-                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][2], "0x00", poolOperators[2]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][0], "0x00", "0x00", poolOperators[2]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][1], "0x00", "0x00", poolOperators[2]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][2], "0x00", "0x00", poolOperators[2]),
            ]);
            ///// 2 PAY FEES /////
            await Promise.all([
@@ -1008,14 +1008,14 @@ describe('Staking Core', () => {
            ];
            await Promise.all([
                // pool 0
-                stakingWrapper.addMakerToPoolAsync(poolIds[0], makersByPoolId[0][0], "0x00", poolOperators[0]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[0], makersByPoolId[0][0], "0x00", "0x00", poolOperators[0]),
                // pool 1
-                stakingWrapper.addMakerToPoolAsync(poolIds[1], makersByPoolId[1][0], "0x00", poolOperators[1]),
-                stakingWrapper.addMakerToPoolAsync(poolIds[1], makersByPoolId[1][1], "0x00", poolOperators[1]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[1], makersByPoolId[1][0], "0x00", "0x00", poolOperators[1]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[1], makersByPoolId[1][1], "0x00", "0x00", poolOperators[1]),
                // pool 2
-                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][0], "0x00", poolOperators[2]),
-                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][1], "0x00", poolOperators[2]),
-                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][2], "0x00", poolOperators[2]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][0], "0x00", "0x00", poolOperators[2]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][1], "0x00", "0x00", poolOperators[2]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][2], "0x00", "0x00", poolOperators[2]),
            ]);
            ///// 2 PAY FEES /////
            await Promise.all([
@@ -1252,14 +1252,14 @@ describe('Staking Core', () => {
            ];
            await Promise.all([
                // pool 0
-                stakingWrapper.addMakerToPoolAsync(poolIds[0], makersByPoolId[0][0], "0x00", poolOperators[0]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[0], makersByPoolId[0][0], "0x00", "0x00", poolOperators[0]),
                // pool 1
-                stakingWrapper.addMakerToPoolAsync(poolIds[1], makersByPoolId[1][0], "0x00", poolOperators[1]),
-                stakingWrapper.addMakerToPoolAsync(poolIds[1], makersByPoolId[1][1], "0x00", poolOperators[1]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[1], makersByPoolId[1][0], "0x00", "0x00", poolOperators[1]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[1], makersByPoolId[1][1], "0x00", "0x00", poolOperators[1]),
                // pool 2
-                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][0], "0x00", poolOperators[2]),
-                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][1], "0x00", poolOperators[2]),
-                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][2], "0x00", poolOperators[2]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][0], "0x00", "0x00", poolOperators[2]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][1], "0x00", "0x00", poolOperators[2]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][2], "0x00", "0x00", poolOperators[2]),
            ]);
            ///// 2 PAY FEES /////
            await Promise.all([
@@ -1438,14 +1438,14 @@ describe('Staking Core', () => {
            ];
            await Promise.all([
                // pool 0
-                stakingWrapper.addMakerToPoolAsync(poolIds[0], makersByPoolId[0][0], "0x00", poolOperators[0]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[0], makersByPoolId[0][0], "0x00", "0x00", poolOperators[0]),
                // pool 1
-                stakingWrapper.addMakerToPoolAsync(poolIds[1], makersByPoolId[1][0], "0x00", poolOperators[1]),
-                stakingWrapper.addMakerToPoolAsync(poolIds[1], makersByPoolId[1][1], "0x00", poolOperators[1]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[1], makersByPoolId[1][0], "0x00", "0x00", poolOperators[1]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[1], makersByPoolId[1][1], "0x00", "0x00", poolOperators[1]),
                // pool 2
-                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][0], "0x00", poolOperators[2]),
-                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][1], "0x00", poolOperators[2]),
-                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][2], "0x00", poolOperators[2]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][0], "0x00", "0x00", poolOperators[2]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][1], "0x00", "0x00", poolOperators[2]),
+                stakingWrapper.addMakerToPoolAsync(poolIds[2], makersByPoolId[2][2], "0x00", "0x00", poolOperators[2]),
            ]);
            ///// 2 PAY FEES /////
            await Promise.all([
--- a/contracts/staking/test/utils/staking_wrapper.ts
+++ b/contracts/staking/test/utils/staking_wrapper.ts
@@ -251,8 +251,8 @@ export class StakingWrapper {
        const poolId = (createPoolLog as any).args.poolId;
        return poolId;
    }
-    public async addMakerToPoolAsync(poolId: string, makerAddress: string, makerSignature: string, operatorAddress: string): Promise<TransactionReceiptWithDecodedLogs> {
-        const calldata = this.getStakingContract().addMakerToPool.getABIEncodedTransactionData(poolId, makerAddress, makerSignature);
+    public async addMakerToPoolAsync(poolId: string, makerAddress: string, makerSignedHash: string, makerSignature: string, operatorAddress: string): Promise<TransactionReceiptWithDecodedLogs> {
+        const calldata = this.getStakingContract().addMakerToPool.getABIEncodedTransactionData(poolId, makerAddress, makerSignedHash, makerSignature);
        const txReceipt = await this._executeTransactionAsync(calldata, operatorAddress);
        return txReceipt;
    }