From 2fe4e380d1824794b42b463c5fa589e6fdd1118a Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Fri, 9 Feb 2018 17:43:35 -0800 Subject: Abstract signature to opaque bytearray --- .../current/protocol/Exchange/Exchange.sol | 7 +- .../protocol/Exchange/MixinExchangeCore.sol | 44 ++++---- .../protocol/Exchange/MixinSignatureValidator.sol | 77 ++++++++++++++ .../Exchange/MixinSignatureValidatorEcrecover.sol | 45 -------- .../protocol/Exchange/MixinWrapperFunctions.sol | 116 +++++++-------------- .../protocol/Exchange/mixins/MExchangeCore.sol | 4 +- .../Exchange/mixins/MSignatureValidator.sol | 16 ++- 7 files changed, 148 insertions(+), 161 deletions(-) create mode 100644 packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol delete mode 100644 packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidatorEcrecover.sol diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol b/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol index 5c513341b..e46b0437f 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol @@ -17,15 +17,16 @@ */ pragma solidity ^0.4.19; +pragma experimental ABIEncoderV2; import "./MixinExchangeCore.sol"; -import "./MixinSignatureValidatorEcrecover.sol"; +import "./MixinSignatureValidator.sol"; import "./MixinSettlementProxy.sol"; import "./MixinWrapperFunctions.sol"; contract Exchange is MixinExchangeCore, - MixinSignatureValidatorEcrecover, + MixinSignatureValidator, MixinSettlementProxy, MixinWrapperFunctions { @@ -37,7 +38,7 @@ contract Exchange is ) public MixinExchangeCore() - MixinSignatureValidatorEcrecover() + MixinSignatureValidator() MixinSettlementProxy(_tokenTransferProxy, _zrxToken) MixinWrapperFunctions() { diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol index c61006374..083491a7a 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol @@ -73,19 +73,15 @@ contract MixinExchangeCore is /// @param orderAddresses Array of order's maker, taker, makerToken, takerToken, and feeRecipient. /// @param orderValues Array of order's makerTokenAmount, takerTokenAmount, makerFee, takerFee, expirationTimestampInSec, and salt. /// @param takerTokenFillAmount Desired amount of takerToken to fill. - /// @param v ECDSA signature parameter v. - /// @param r ECDSA signature parameters r. - /// @param s ECDSA signature parameters s. + /// @param signature Proof of signing order by maker. /// @return Total amount of takerToken filled in trade. function fillOrder( - address[5] orderAddresses, - uint256[6] orderValues, - uint256 takerTokenFillAmount, - uint8 v, - bytes32 r, - bytes32 s) - public - returns (uint256 takerTokenFilledAmount) + address[5] orderAddresses, + uint[6] orderValues, + uint takerTokenFillAmount, + bytes signature) + public + returns (uint256 takerTokenFilledAmount) { Order memory order = Order({ maker: orderAddresses[0], @@ -100,29 +96,37 @@ contract MixinExchangeCore is expirationTimestampInSec: orderValues[4], orderHash: getOrderHash(orderAddresses, orderValues) }); - - require(order.taker == address(0) || order.taker == msg.sender); - require(order.makerTokenAmount > 0 && order.takerTokenAmount > 0 && takerTokenFillAmount > 0); + + // Validate maker + require(order.makerTokenAmount > 0); + require(order.takerTokenAmount > 0); require(isValidSignature( - order.maker, order.orderHash, - v, - r, - s + order.maker, + signature )); + + // Validate taker + if (order.taker != address(0)) { + require(order.taker == msg.sender); + } + require(takerTokenFillAmount > 0); + // Validate order expiration if (block.timestamp >= order.expirationTimestampInSec) { LogError(uint8(Errors.ORDER_EXPIRED), order.orderHash); return 0; } - + + // Validate order availability uint256 remainingTakerTokenAmount = safeSub(order.takerTokenAmount, getUnavailableTakerTokenAmount(order.orderHash)); takerTokenFilledAmount = min256(takerTokenFillAmount, remainingTakerTokenAmount); if (takerTokenFilledAmount == 0) { LogError(uint8(Errors.ORDER_FULLY_FILLED_OR_CANCELLED), order.orderHash); return 0; } - + + // Validate fill order rounding if (isRoundingError(takerTokenFilledAmount, order.takerTokenAmount, order.makerTokenAmount)) { LogError(uint8(Errors.ROUNDING_ERROR_TOO_LARGE), order.orderHash); return 0; diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol new file mode 100644 index 000000000..919dbe312 --- /dev/null +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol @@ -0,0 +1,77 @@ +/* + + Copyright 2017 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.4.19; + +import "./mixins/MSignatureValidator.sol"; + +/// @dev Provides MSignatureValidator +contract MixinSignatureValidator is + MSignatureValidator +{ + enum SignatureType { + Invalid, + Ecrecover + } + + function isValidSignature( + bytes32 hash, + address signer, + bytes signature) + public view + returns (bool isValid) + { + require(signature.length >= 1); + + // Select signature type + SignatureType signatureType = SignatureType(uint8(signature[0])); + if (signatureType != SignatureType.Ecrecover) { + valid = false; + return; + } + + // Verify using ecrecover + require(signature.length == 66); + uint8 v = uint8(signature[1]); + bytes32 r = get32(signature, 2); + bytes32 s = get32(signature, 34); + address recovered = ecrecover( + keccak256("\x19Ethereum Signed Message:\n32", hash), + v, + r, + s + ); + isValid = signer == recovered; + } + + function get32(bytes b, uint256 index) + private pure + returns (bytes32 result) + { + require(b.length >= index + 32); + + // Arrays are prefixed by a 256 bit length parameter + index += 32; + + // Read the bytes32 from array memory + assembly { + result := mload(add(b, index)) + } + } + +} diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidatorEcrecover.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidatorEcrecover.sol deleted file mode 100644 index 7c9155921..000000000 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidatorEcrecover.sol +++ /dev/null @@ -1,45 +0,0 @@ -/* - - Copyright 2017 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.4.19; - -import "./mixins/MSignatureValidator.sol"; - -/// @dev Provides MSignatureValidator -contract MixinSignatureValidatorEcrecover is - MSignatureValidator -{ - function isValidSignature( - address signer, - bytes32 hash, - uint8 v, - bytes32 r, - bytes32 s) - public - constant - returns (bool isValid) - { - isValid = signer == ecrecover( - keccak256("\x19Ethereum Signed Message:\n32", hash), - v, - r, - s - ); - return isValid; - } -} diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index e09d93851..81a82eeec 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -17,6 +17,7 @@ */ pragma solidity ^0.4.19; +pragma experimental ABIEncoderV2; import './mixins/MExchangeCore.sol'; import "../../utils/SafeMath/SafeMath.sol"; @@ -27,29 +28,18 @@ contract MixinWrapperFunctions is SafeMath { - /// @dev Fills an order with specified parameters and ECDSA signature. Throws if specified amount not filled entirely. - /// @param orderAddresses Array of order's maker, taker, makerToken, takerToken, and feeRecipient. - /// @param orderValues Array of order's makerTokenAmount, takerTokenAmount, makerFee, takerFee, expirationTimestampInSec, and salt. - /// @param takerTokenFillAmount Desired amount of takerToken to fill. - /// @param v ECDSA signature parameter v. - /// @param r ECDSA signature parameters r. - /// @param s ECDSA signature parameters s. function fillOrKillOrder( address[5] orderAddresses, - uint256[6] orderValues, - uint256 takerTokenFillAmount, - uint8 v, - bytes32 r, - bytes32 s) + uint[6] orderValues, + uint takerTokenFillAmount, + bytes signature) public { require(fillOrder( orderAddresses, orderValues, takerTokenFillAmount, - v, - r, - s + signature ) == takerTokenFillAmount); } @@ -57,18 +47,13 @@ contract MixinWrapperFunctions is /// @param orderAddresses Array of order's maker, taker, makerToken, takerToken, and feeRecipient. /// @param orderValues Array of order's makerTokenAmount, takerTokenAmount, makerFee, takerFee, expirationTimestampInSec, and salt. /// @param takerTokenFillAmount Desired amount of takerToken to fill. - /// @param v ECDSA signature parameter v. - /// @param r ECDSA signature parameters r. - /// @param s ECDSA signature parameters s. /// @return Success if the transaction did not revert. /// @return Total amount of takerToken filled in trade. function fillOrderNoThrow( address[5] orderAddresses, uint256[6] orderValues, uint256 takerTokenFillAmount, - uint8 v, - bytes32 r, - bytes32 s) + bytes signature) public returns (bool success, uint256 takerTokenFilledAmount) { @@ -91,9 +76,16 @@ contract MixinWrapperFunctions is mstore(add(x, 292), add(orderValues, 160)) // expirationTimestampInSec mstore(add(x, 324), add(orderValues, 192)) // salt mstore(add(x, 356), takerTokenFillAmount) - mstore(add(x, 388), v) - mstore(add(x, 420), r) - mstore(add(x, 452), s) + for { + let src := signature + let dst := add(x, 388) + let end := add(add(src, mload(signature)), 32) + } lt(src, end) { + src := add(src, 32) + dst := add(dst, 32) + } { + mstore(dst, mload(src)) + } success := delegatecall( gas, // TODO: don't send all gas, save some for returning is case of throw @@ -113,16 +105,11 @@ contract MixinWrapperFunctions is /// @param orderAddresses Array of address arrays containing individual order addresses. /// @param orderValues Array of uint256 arrays containing individual order values. /// @param takerTokenFillAmounts Array of desired amounts of takerToken to fill in orders. - /// @param v Array ECDSA signature v parameters. - /// @param r Array of ECDSA signature r parameters. - /// @param s Array of ECDSA signature s parameters. function batchFillOrders( address[5][] orderAddresses, uint256[6][] orderValues, uint256[] takerTokenFillAmounts, - uint8[] v, - bytes32[] r, - bytes32[] s) + bytes[] signatures) external { for (uint256 i = 0; i < orderAddresses.length; i++) { @@ -130,9 +117,7 @@ contract MixinWrapperFunctions is orderAddresses[i], orderValues[i], takerTokenFillAmounts[i], - v[i], - r[i], - s[i] + signatures[i] ); } } @@ -141,54 +126,38 @@ contract MixinWrapperFunctions is /// @param orderAddresses Array of address arrays containing individual order addresses. /// @param orderValues Array of uint256 arrays containing individual order values. /// @param takerTokenFillAmounts Array of desired amounts of takerToken to fill in orders. - /// @param v Array ECDSA signature v parameters. - /// @param r Array of ECDSA signature r parameters. - /// @param s Array of ECDSA signature s parameters. function batchFillOrKillOrders( address[5][] orderAddresses, - uint256[6][] orderValues, - uint256[] takerTokenFillAmounts, - uint8[] v, - bytes32[] r, - bytes32[] s) - external + uint[6][] orderValues, + uint[] takerTokenFillAmounts, + bytes[] signatures) + public /// Compiler crash when set to external { for (uint256 i = 0; i < orderAddresses.length; i++) { fillOrKillOrder( orderAddresses[i], orderValues[i], takerTokenFillAmounts[i], - v[i], - r[i], - s[i] + signatures[i] ); } } - /// @dev Synchronously executes multiple calls of fillOrderNoThrow in a single transaction. - /// @param orderAddresses Array of address arrays containing individual order addresses. - /// @param orderValues Array of uint256 arrays containing individual order values. - /// @param takerTokenFillAmounts Array of desired amounts of takerToken to fill in orders. - /// @param v Array ECDSA signature v parameters. - /// @param r Array of ECDSA signature r parameters. - /// @param s Array of ECDSA signature s parameters. - function batchFillOrdersNoThrow( + + function fillOrdersUpTo( address[5][] orderAddresses, - uint256[6][] orderValues, - uint256[] takerTokenFillAmounts, - uint8[] v, - bytes32[] r, - bytes32[] s) - external + uint[6][] orderValues, + uint[] takerTokenFillAmounts, + bytes[] signatures) + public /// Stack to deep when set to external + returns (uint) { for (uint256 i = 0; i < orderAddresses.length; i++) { fillOrderNoThrow( orderAddresses[i], orderValues[i], takerTokenFillAmounts[i], - v[i], - r[i], - s[i] + signatures[i] ); } } @@ -197,17 +166,12 @@ contract MixinWrapperFunctions is /// @param orderAddresses Array of address arrays containing individual order addresses. /// @param orderValues Array of uint256 arrays containing individual order values. /// @param takerTokenFillAmount Desired total amount of takerToken to fill in orders. - /// @param v Array ECDSA signature v parameters. - /// @param r Array of ECDSA signature r parameters. - /// @param s Array of ECDSA signature s parameters. /// @return Total amount of takerTokenFillAmount filled in orders. function marketFillOrders( address[5][] orderAddresses, uint256[6][] orderValues, uint256 takerTokenFillAmount, - uint8[] v, - bytes32[] r, - bytes32[] s) + bytes[] signatures) external returns (uint256 totalTakerTokenFilledAmount) { @@ -217,9 +181,7 @@ contract MixinWrapperFunctions is orderAddresses[i], orderValues[i], safeSub(takerTokenFillAmount, totalTakerTokenFilledAmount), - v[i], - r[i], - s[i] + signatures[i] )); if (totalTakerTokenFilledAmount == takerTokenFillAmount) break; } @@ -230,17 +192,12 @@ contract MixinWrapperFunctions is /// @param orderAddresses Array of address arrays containing individual order addresses. /// @param orderValues Array of uint256 arrays containing individual order values. /// @param takerTokenFillAmount Desired total amount of takerToken to fill in orders. - /// @param v Array ECDSA signature v parameters. - /// @param r Array of ECDSA signature r parameters. - /// @param s Array of ECDSA signature s parameters. /// @return Total amount of takerTokenFillAmount filled in orders. function marketFillOrdersNoThrow( address[5][] orderAddresses, uint256[6][] orderValues, uint256 takerTokenFillAmount, - uint8[] v, - bytes32[] r, - bytes32[] s) + bytes[] signatures) external returns (uint256 totalTakerTokenFilledAmount) { @@ -250,9 +207,7 @@ contract MixinWrapperFunctions is orderAddresses[i], orderValues[i], safeSub(takerTokenFillAmount, totalTakerTokenFilledAmount), - v[i], - r[i], - s[i] + signatures[i] ); totalTakerTokenFilledAmount = safeAdd(totalTakerTokenFilledAmount, takerTokenFilledAmount); if (totalTakerTokenFilledAmount == takerTokenFillAmount) break; @@ -278,4 +233,5 @@ contract MixinWrapperFunctions is ); } } + } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol index 3977c3bbd..b7901e16b 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol @@ -24,9 +24,7 @@ contract MExchangeCore { address[5] orderAddresses, uint256[6] orderValues, uint256 takerTokenFillAmount, - uint8 v, - bytes32 r, - bytes32 s) + bytes signature) public returns (uint256 takerTokenFilledAmount); diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol index b4ca4bb48..2d21a6c86 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol @@ -20,19 +20,15 @@ pragma solidity ^0.4.19; contract MSignatureValidator { - /// @dev Verifies that an order signature is valid. - /// @param signer address of signer. - /// @param hash Signed Keccak-256 hash. - /// @param v ECDSA signature parameter v. - /// @param r ECDSA signature parameters r. - /// @param s ECDSA signature parameters s. + /// @dev Verifies that a signature is valid. + /// @param digest Message digest that is signed. + /// @param signer Address of signer. + /// @param signature Proof of signing. /// @return Validity of order signature. function isValidSignature( + bytes32 digest, address signer, - bytes32 hash, - uint8 v, - bytes32 r, - bytes32 s) + bytes signature) public view returns (bool isValid); } -- cgit v1.2.3