add multiple signature types (#384)

* add multiple signature types

* fixes

* fixes

contracts/lib/TypedSignature.sol

@@ -0,0 +1,107 @@
+/*
+
+    Copyright 2018 dYdX Trading Inc.
+
+    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.4.24;
+pragma experimental "v0.5.0";
+
+
+/**
+ * @title TypedSignature
+ * @author dYdX
+ *
+ * Allows for ecrecovery of signed hashes with three different prepended messages:
+ * 1) ""
+ * 2) "\x19Ethereum Signed Message:\n32"
+ * 3) "\x19Ethereum Signed Message:\n\x20"
+ */
+library TypedSignature {
+
+    enum SignatureType {
+        INVALID,
+        ECRECOVER_NULL,
+        ECRECOVER_DEC,
+        ECRECOVER_HEX,
+        UNSUPPORTED
+    }
+
+    // prepended message with the length of the signed hash in hexadecimal
+    bytes constant private PREPEND_HEX = "\x19Ethereum Signed Message:\n\x20";
+
+    // prepended message with the length of the signed hash in decimal
+    bytes constant private PREPEND_DEC = "\x19Ethereum Signed Message:\n32";
+
+    /**
+     * Gives the address of the signer of a hash. Allows for three common prepended strings.
+     *
+     * @param  hash               Hash that was signed (does not include prepended message)
+     * @param  signatureWithType  Type and ECDSA signature with structure: {1:type}{1:v}{32:r}{32:s}
+     * @return                    address of the signer of the hash
+     */
+    function recover(
+        bytes32 hash,
+        bytes signatureWithType
+    )
+        internal
+        pure
+        returns (address)
+    {
+        require(
+            signatureWithType.length == 66,
+            "SignatureValidator#validateSignature: invalid signature length"
+        );
+
+        uint8 rawSigType = uint8(signatureWithType[0]);
+
+        require(
+            rawSigType > uint8(SignatureType.INVALID),
+            "SignatureValidator#validateSignature: invalid signature type"
+        );
+        require(
+            rawSigType < uint8(SignatureType.UNSUPPORTED),
+            "SignatureValidator#validateSignature: unsupported signature type"
+        );
+
+        SignatureType sigType = SignatureType(rawSigType);
+        uint8 v = uint8(signatureWithType[1]);
+        bytes32 r;
+        bytes32 s;
+
+        /* solium-disable-next-line security/no-inline-assembly */
+        assembly {
+            r := mload(add(signatureWithType, 34))
+            s := mload(add(signatureWithType, 66))
+        }
+
+        bytes32 signedHash;
+        if (sigType == SignatureType.ECRECOVER_DEC) {
+            signedHash = keccak256(abi.encodePacked(PREPEND_DEC, hash));
+        } else if (sigType == SignatureType.ECRECOVER_HEX) {
+            signedHash = keccak256(abi.encodePacked(PREPEND_HEX, hash));
+        } else {
+            assert(sigType == SignatureType.ECRECOVER_NULL);
+            signedHash = hash;
+        }
+
+        return ecrecover(
+            signedHash,
+            v,
+            r,
+            s
+        );
+    }
+}

contracts/margin/impl/BorrowShared.sol

@@ -20,14 +20,14 @@ pragma solidity 0.4.24;
 pragma experimental "v0.5.0";
 
 import { AddressUtils } from "zeppelin-solidity/contracts/AddressUtils.sol";
-import { ECRecovery } from "zeppelin-solidity/contracts/ECRecovery.sol";
 import { Math } from "zeppelin-solidity/contracts/math/Math.sol";
 import { SafeMath } from "zeppelin-solidity/contracts/math/SafeMath.sol";
 import { MarginCommon } from "./MarginCommon.sol";
 import { MarginState } from "./MarginState.sol";
 import { TokenProxy } from "../TokenProxy.sol";
 import { Vault } from "../Vault.sol";
 import { MathHelpers } from "../../lib/MathHelpers.sol";
+import { TypedSignature } from "../../lib/TypedSignature.sol";
 import { ExchangeWrapper } from "../interfaces/ExchangeWrapper.sol";
 import { LoanOfferingVerifier } from "../interfaces/LoanOfferingVerifier.sol";
 
@@ -40,7 +40,6 @@ import { LoanOfferingVerifier } from "../interfaces/LoanOfferingVerifier.sol";
  * Both use a Loan Offering and a DEX Order to open or increase a position.
  */
 library BorrowShared {
-    using ECRecovery for bytes32;
     using SafeMath for uint256;
 
     // ============ Structs ============
@@ -97,10 +96,11 @@ library BorrowShared {
         if (AddressUtils.isContract(transaction.loanOffering.payer)) {
             getConsentFromSmartContractLender(transaction);
         } else {
-            bytes32 messageHash = transaction.loanOffering.loanHash.toEthSignedMessageHash();
             require(
-                transaction.loanOffering.payer ==
-                messageHash.recover(transaction.loanOffering.signature),
+                transaction.loanOffering.payer == TypedSignature.recover(
+                    transaction.loanOffering.loanHash,
+                    transaction.loanOffering.signature
+                ),
                 "BorrowShared#validateTxPreSell: Invalid loan offering signature"
             );
         }

contracts/testing/TestTypedSignature.sol

@@ -0,0 +1,36 @@
+/*
+
+    Copyright 2018 dYdX Trading Inc.
+
+    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.4.24;
+pragma experimental "v0.5.0";
+
+import { TypedSignature } from "../lib/TypedSignature.sol";
+
+
+contract TestTypedSignature {
+    function recover(
+        bytes32 hash,
+        bytes signatureWithType
+    )
+        external
+        pure
+        returns (address)
+    {
+        return TypedSignature.recover(hash, signatureWithType);
+    }
+}

test/helpers/Constants.js

@@ -3,7 +3,14 @@ const web3Instance = new Web3(web3.currentProvider);
 const BigNumber = require('bignumber.js');
 
 module.exports = {
-  ORDER_TYPE:{
+  SIGNATURE_TYPE: {
+    INVALID_ZERO: 0,
+    INVALID_NONZERO: 15,
+    NUL: 1,
+    DEC: 2,
+    HEX: 3,
+  },
+  ORDER_TYPE: {
     ZERO_EX: "zeroEx",
     KYBER: "kyber",
     DIRECT: "openDirectly"

test/helpers/LoanHelper.js

@@ -2,7 +2,7 @@ const BigNumber = require('bignumber.js');
 const HeldToken = artifacts.require("TokenA");
 const OwedToken = artifacts.require("TokenB");
 const FeeToken = artifacts.require("TokenC");
-const { ADDRESSES, BIGNUMBERS, DEFAULT_SALT } = require('./Constants');
+const { ADDRESSES, BIGNUMBERS, DEFAULT_SALT, SIGNATURE_TYPE } = require('./Constants');
 const Web3 = require('web3');
 const Margin = artifacts.require("Margin");
 const promisify = require("es6-promisify");
@@ -85,7 +85,14 @@ async function signLoanOffering(loanOffering) {
   );
 
   const { v, r, s } = ethUtil.fromRpcSig(signature);
-  return ethUtil.bufferToHex(Buffer.concat([r, s, ethUtil.toBuffer(v)]));
+  return ethUtil.bufferToHex(
+    Buffer.concat([
+      ethUtil.toBuffer(SIGNATURE_TYPE.DEC),
+      ethUtil.toBuffer(v),
+      r,
+      s,
+    ])
+  );
 }
 
 module.exports = {

test/lib/TestTypedSignature.js

@@ -0,0 +1,102 @@
+const chai = require('chai');
+const expect = chai.expect;
+chai.use(require('chai-bignumber')());
+const BigNumber = require('bignumber.js');
+const promisify = require("es6-promisify");
+const ethUtil = require('ethereumjs-util');
+const Web3 = require('web3');
+const web3Instance = new Web3(web3.currentProvider);
+
+const TestTypedSignature = artifacts.require("TestTypedSignature");
+const { BIGNUMBERS, BYTES32, SIGNATURE_TYPE } = require('../helpers/Constants');
+const { expectThrow } = require('../helpers/ExpectHelper');
+
+contract('TestTypedSignature', accounts => {
+  let contract;
+  const rawKey = "43f2ee33c522046e80b67e96ceb84a05b60b9434b0ee2e3ae4b1311b9f5dcc46";
+  const privateKey = new Buffer(rawKey, "hex");
+  const signer = "0x" + ethUtil.privateToAddress(privateKey).toString("hex");
+  const hash = BYTES32.TEST[0];
+  const PROPER_SIG_LENGTH = 66;
+
+  before(async () => {
+    contract = await TestTypedSignature.new();
+  });
+
+  describe('INVALID', () => {
+    it("fails when invalid type (zero)", async () => {
+      const signature = await promisify(web3Instance.eth.sign)(hash, accounts[0]);
+      const { v, r, s } = ethUtil.fromRpcSig(signature);
+      const signatureWithType = getSignature(SIGNATURE_TYPE.INVALID_ZERO, v, r, s);
+      await expectThrow(contract.recover(hash, signatureWithType));
+    });
+
+    it("fails when invalid type (nonzero)", async () => {
+      const signature = await promisify(web3Instance.eth.sign)(hash, accounts[0]);
+      const { v, r, s } = ethUtil.fromRpcSig(signature);
+      const signatureWithType = getSignature(SIGNATURE_TYPE.INVALID_NONZERO, v, r, s);
+      await expectThrow(contract.recover(hash, signatureWithType));
+    });
+
+    it("fails when too short", async () => {
+      const n = Buffer.from("12345678123456781234567812345678");
+      const tooShort = Buffer.concat([n, n]);
+      const signatureWithType = ethUtil.bufferToHex(tooShort);
+      expect(tooShort.length).to.be.lt(PROPER_SIG_LENGTH);
+      await expectThrow(contract.recover(hash, signatureWithType));
+    });
+
+    it("fails when too long", async () => {
+      const n = Buffer.from("12345678123456781234567812345678");
+      const tooLong = Buffer.concat([n, n, n, n]);
+      const signatureWithType = ethUtil.bufferToHex(tooLong);
+      expect(tooLong.length).to.be.gt(PROPER_SIG_LENGTH);
+      await expectThrow(contract.recover(hash, signatureWithType));
+    });
+  });
+
+  describe('ECRECOVERY_NUL', () => {
+    it("returns the correct signer", async () => {
+      const ecSignature = ethUtil.ecsign(ethUtil.toBuffer(hash), privateKey);
+      const { v, r, s } = ecSignature;
+      const signatureWithType = getSignature(SIGNATURE_TYPE.NUL, v, r, s);
+      const retVal = await contract.recover.call(hash, signatureWithType);
+      expect(retVal).to.equal(signer);
+    });
+  });
+
+  describe('ECRECOVERY_DEC', () => {
+    it("returns the correct signer", async () => {
+      const signer = accounts[0];
+      const signature = await promisify(web3Instance.eth.sign)(hash, accounts[0]);
+      const { v, r, s } = ethUtil.fromRpcSig(signature);
+
+      const signatureWithType = getSignature(SIGNATURE_TYPE.DEC, v, r, s);
+      const retVal = await contract.recover(hash, signatureWithType);
+      expect(retVal).to.equal(signer);
+    });
+  });
+
+  describe('ECRECOVERY_HEX', () => {
+    it("returns the correct signer", async () => {
+      const packed = "\x19Ethereum Signed Message:\n\x20" + ethUtil.toBuffer(hash);
+      const hash2 = "0x" + ethUtil.sha3(Buffer.from(packed, 32)).toString("hex");
+      const ecSignature = ethUtil.ecsign(ethUtil.toBuffer(hash2), privateKey);
+      const { v, r, s } = ecSignature;
+      const signatureWithType = getSignature(SIGNATURE_TYPE.HEX, v, r, s);
+      const retVal = await contract.recover.call(hash, signatureWithType);
+      expect(retVal).to.equal(signer);
+    });
+  });
+});
+
+function getSignature(type, v, r, s) {
+  return ethUtil.bufferToHex(
+    Buffer.concat([
+      ethUtil.toBuffer(type),
+      ethUtil.toBuffer(v),
+      r,
+      s,
+    ])
+  );
+}