diff options
3 files changed, 77 insertions, 63 deletions
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol index 76de961bc..1022b1fe7 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol @@ -83,13 +83,13 @@ contract MixinSignatureValidator is ); SignatureType signatureType = SignatureType(uint8(signature[0])); - // Variables are not scoped in Solidity + // Variables are not scoped in Solidity. uint8 v; bytes32 r; bytes32 s; address recovered; - // Always illegal signature + // 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 @@ -98,7 +98,7 @@ contract MixinSignatureValidator is // NOTE: Reason cannot be assigned to a variable because of https://github.com/ethereum/solidity/issues/4051 revert("Illegal signature type."); - // Always invalid signature + // 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. @@ -110,20 +110,14 @@ contract MixinSignatureValidator is isValid = false; return isValid; - // Implicitly signed by caller - // The signer has initiated the call. In the case of non-contract - // accounts it means the transaction itself was signed. - // Example: let's say for a particular operation three signatures - // A, B and C are required. To submit the transaction, A and B can - // give a signature to C, who can then submit the transaction using - // `Caller` for his own signature. Or A and C can sign and B can - // submit using `Caller`. Having `Caller` allows this flexibility. - } else if (signatureType == SignatureType.Caller) { - require( - signature.length == 1, - INVALID_SIGNATURE_LENGTH - ); - isValid = signer == msg.sender; + // Signature using EIP712 + } else if (signatureType == SignatureType.EIP712) { + require(signature.length == 66); + v = uint8(signature[1]); + r = get32(signature, 2); + s = get32(signature, 34); + recovered = ecrecover(hash, v, r, s); + isValid = signer == recovered; return isValid; // Signed using web3.eth_sign @@ -144,42 +138,29 @@ contract MixinSignatureValidator is isValid = signer == recovered; return isValid; - // Signature using EIP712 - } else if (signatureType == SignatureType.EIP712) { - require( - signature.length == 66, - INVALID_SIGNATURE_LENGTH - ); - v = uint8(signature[1]); - r = readBytes32(signature, 2); - s = readBytes32(signature, 34); - recovered = ecrecover(hash, v, r, s); - isValid = signer == recovered; + // Implicitly signed by signer of Ethereum transaction. + // This is useful when initiating a call from a contract that might + // otherwise require multiple signatures. + // Example: Contract A calls Exchange.fillOrder using `executeTransaction`. + // This would normally require the user to sign both an Ethereum transaction + // and a 0x transaction. By using the TxOrigin signature type, the signature + // for the Ethereum transaction will encompass both signatures. + } else if (signatureType == SignatureType.TxOrigin) { + require(signature.length == 1); + isValid = signer == tx.origin; return isValid; - // Signature from Trezor hardware wallet - // It differs from web3.eth_sign in the encoding of message length - // (Bitcoin varint encoding vs ascii-decimal, the latter is not - // self-terminating which leads to ambiguities). - // See also: - // https://en.bitcoin.it/wiki/Protocol_documentation#Variable_length_integer - // https://github.com/trezor/trezor-mcu/blob/master/firmware/ethereum.c#L602 - // https://github.com/trezor/trezor-mcu/blob/master/firmware/crypto.c#L36 - } else if (signatureType == SignatureType.Trezor) { - require( - signature.length == 66, - INVALID_SIGNATURE_LENGTH - ); - v = uint8(signature[1]); - r = readBytes32(signature, 2); - s = readBytes32(signature, 34); - recovered = ecrecover( - keccak256("\x19Ethereum Signed Message:\n\x41", hash), - v, - r, - s - ); - isValid = signer == recovered; + // Implicitly signed by caller. + // The signer has initiated the call. In the case of non-contract + // accounts it means the transaction itself was signed. + // Example: let's say for a particular operation three signatures + // A, B and C are required. To submit the transaction, A and B can + // give a signature to C, who can then submit the transaction using + // `Caller` for his own signature. Or A and C can sign and B can + // submit using `Caller`. Having `Caller` allows this flexibility. + } else if (signatureType == SignatureType.Caller) { + require(signature.length == 1); + isValid = signer == msg.sender; return isValid; // Signature verified by signer contract. @@ -219,6 +200,36 @@ contract MixinSignatureValidator is ); return isValid; + // Signer signed hash previously using the preSign function. + } else if (signatureType == SignatureType.PreSigned) { + isValid = preSigned[hash][signer]; + return isValid; + + // Signature from Trezor hardware wallet. + // It differs from web3.eth_sign in the encoding of message length + // (Bitcoin varint encoding vs ascii-decimal, the latter is not + // self-terminating which leads to ambiguities). + // See also: + // https://en.bitcoin.it/wiki/Protocol_documentation#Variable_length_integer + // https://github.com/trezor/trezor-mcu/blob/master/firmware/ethereum.c#L602 + // https://github.com/trezor/trezor-mcu/blob/master/firmware/crypto.c#L36 + } else if (signatureType == SignatureType.Trezor) { + require( + signature.length == 66, + INVALID_SIGNATURE_LENGTH + ); + v = uint8(signature[1]); + r = readBytes32(signature, 2); + s = readBytes32(signature, 34); + recovered = ecrecover( + keccak256("\x19Ethereum Signed Message:\n\x41", hash), + v, + r, + s + ); + isValid = signer == recovered; + return isValid; + // Signer signed hash previously using the preSign function } else if (signatureType == SignatureType.PreSigned) { isValid = preSigned[hash][signer]; 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 33424ff84..a13f4524b 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol @@ -25,15 +25,16 @@ contract MSignatureValidator is { // Allowed signature types. enum SignatureType { - Illegal, // Default value - Invalid, - Caller, - Ecrecover, - EIP712, - Trezor, - Signer, - Validator, - PreSigned + Illegal, // 0x00, default value + Invalid, // 0x01 + EIP712, // 0x02 + Ecrecover, // 0x03 + TxOrigin, // 0x04 + Caller, // 0x05 + Signer, // 0x06 + Validator, // 0x07 + PreSigned, // 0x08 + Trezor // 0x09 } /// @dev Verifies that a signature is valid. diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index 90f90ec27..cd2d128f1 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -116,11 +116,13 @@ export enum ContractName { export enum SignatureType { Illegal, Invalid, - Caller, - Ecrecover, EIP712, - Trezor, + Ecrecover, + TxOrigin, + Caller, Contract, + PreSigned, + Trezor, } export interface SignedTransaction { |