Remove TxOrigin signature type, modify whitelist to use Validator signature type

This commit is contained in:
Amir Bandeali
2018-05-21 10:04:17 -07:00
parent 4b71c65aea
commit 6d462fc961
7 changed files with 35 additions and 35 deletions

View File

@@ -31,10 +31,10 @@ contract MixinSignatureValidator is
{ {
// Mapping of hash => signer => signed // Mapping of hash => signer => signed
mapping(bytes32 => mapping(address => bool)) preSigned; mapping (bytes32 => mapping (address => bool)) preSigned;
// Mapping of signer => validator => approved // Mapping of signer => validator => approved
mapping(address => mapping (address => bool)) allowedValidators; mapping (address => mapping (address => bool)) allowedValidators;
/// @dev Approves a hash on-chain using any valid signature type. /// @dev Approves a hash on-chain using any valid signature type.
/// After presigning a hash, the preSign signature type will become valid for that hash and signer. /// After presigning a hash, the preSign signature type will become valid for that hash and signer.
@@ -141,21 +141,6 @@ contract MixinSignatureValidator is
isValid = signer == recovered; isValid = signer == recovered;
return isValid; return isValid;
// 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,
INVALID_SIGNATURE_LENGTH
);
isValid = signer == tx.origin;
return isValid;
// Implicitly signed by caller. // Implicitly signed by caller.
// The signer has initiated the call. In the case of non-contract // The signer has initiated the call. In the case of non-contract
// accounts it means the transaction itself was signed. // accounts it means the transaction itself was signed.
@@ -188,7 +173,7 @@ contract MixinSignatureValidator is
// If used with an order, the maker of the order can still be an EOA. // If used with an order, the maker of the order can still be an EOA.
// A signature using this type should be encoded as: // A signature using this type should be encoded as:
// | Offset | Length | Contents | // | Offset | Length | Contents |
// | 0x00 | 1 | Signature type is always "\x07" | // | 0x00 | 1 | Signature type is always "\x06" |
// | 0x01 | 20 | Address of validator contract | // | 0x01 | 20 | Address of validator contract |
// | 0x15 | ** | Signature to validate | // | 0x15 | ** | Signature to validate |
} else if (signatureType == SignatureType.Validator) { } else if (signatureType == SignatureType.Validator) {

View File

@@ -29,12 +29,11 @@ contract MSignatureValidator is
Invalid, // 0x01 Invalid, // 0x01
EIP712, // 0x02 EIP712, // 0x02
Ecrecover, // 0x03 Ecrecover, // 0x03
TxOrigin, // 0x04 Caller, // 0x04
Caller, // 0x05 Signer, // 0x05
Signer, // 0x06 Validator, // 0x06
Validator, // 0x07 PreSigned, // 0x07
PreSigned, // 0x08 Trezor // 0x08
Trezor // 0x09
} }
/// @dev Verifies that a signature is valid. /// @dev Verifies that a signature is valid.

View File

@@ -35,13 +35,14 @@ contract Whitelist is
// Exchange contract. // Exchange contract.
IExchange EXCHANGE; IExchange EXCHANGE;
// TxOrigin signature type is the 5th value in enum SignatureType and has a length of 1. byte constant VALIDATOR_SIGNATURE_BYTE = "\x06";
bytes constant TX_ORIGIN_SIGNATURE = "\x04"; bytes TX_ORIGIN_SIGNATURE;
constructor (address _exchange) constructor (address _exchange)
public public
{ {
EXCHANGE = IExchange(_exchange); EXCHANGE = IExchange(_exchange);
TX_ORIGIN_SIGNATURE = abi.encodePacked(VALIDATOR_SIGNATURE_BYTE, address(this));
} }
/// @dev Adds or removes an address from the whitelist. /// @dev Adds or removes an address from the whitelist.
@@ -102,4 +103,19 @@ contract Whitelist is
TX_ORIGIN_SIGNATURE TX_ORIGIN_SIGNATURE
); );
} }
}
/// @dev Verifies signer is same as signer of current Ethereum transaction.
/// @param signer Address that should have signed the given hash.
/// @param signature Proof of signing.
/// @return Validity of order signature.
function isValidSignature(
bytes32 hash,
address signer,
bytes signature)
external
view
returns (bool isValid)
{
return signer == tx.origin;
}
}

View File

@@ -40,10 +40,6 @@ contract LibBytes {
pure pure
returns (bytes memory) returns (bytes memory)
{ {
require(
len > 0,
GT_ZERO_LENGTH_REQUIRED
);
require( require(
b.length >= index + len, b.length >= index + len,
INDEX_OUT_OF_BOUNDS INDEX_OUT_OF_BOUNDS

View File

@@ -28,6 +28,7 @@ export const constants = {
DUMMY_TOKEN_SYMBOL: '', DUMMY_TOKEN_SYMBOL: '',
DUMMY_TOKEN_DECIMALS: new BigNumber(18), DUMMY_TOKEN_DECIMALS: new BigNumber(18),
DUMMY_TOKEN_TOTAL_SUPPLY: new BigNumber(0), DUMMY_TOKEN_TOTAL_SUPPLY: new BigNumber(0),
NULL_BYTES: '0x',
NUM_DUMMY_ERC20_TO_DEPLOY: 3, NUM_DUMMY_ERC20_TO_DEPLOY: 3,
NUM_DUMMY_ERC721_TO_DEPLOY: 1, NUM_DUMMY_ERC721_TO_DEPLOY: 1,
NUM_ERC721_TOKENS_TO_MINT: 2, NUM_ERC721_TOKENS_TO_MINT: 2,

View File

@@ -218,6 +218,10 @@ describe('Exchange transactions', () => {
txDefaults, txDefaults,
exchange.address, exchange.address,
); );
const isApproved = true;
await exchange.approveSignatureValidator.sendTransactionAsync(whitelist.address, isApproved, {
from: takerAddress,
});
const defaultOrderParams = { const defaultOrderParams = {
...constants.STATIC_ORDER_PARAMS, ...constants.STATIC_ORDER_PARAMS,
senderAddress: whitelist.address, senderAddress: whitelist.address,

View File

@@ -62,12 +62,11 @@ describe('LibBytes', () => {
describe('deepCopyBytes', () => { describe('deepCopyBytes', () => {
const byteArrayLongerThan32BytesLen = (byteArrayLongerThan32Bytes.length - 2) / 2; const byteArrayLongerThan32BytesLen = (byteArrayLongerThan32Bytes.length - 2) / 2;
it('should throw if length of copy is 0', async () => { it('should return a byte array of length 0 if len is 0', async () => {
const index = new BigNumber(0); const index = new BigNumber(0);
const len = new BigNumber(0); const len = new BigNumber(0);
return expect( const copy = await libBytes.publicDeepCopyBytes.callAsync(byteArrayLongerThan32Bytes, index, len);
libBytes.publicDeepCopyBytes.callAsync(byteArrayLongerThan32Bytes, index, len), expect(copy).to.equal(constants.NULL_BYTES);
).to.be.rejectedWith(constants.REVERT);
}); });
it('should throw if start index + length to copy is greater than length of byte array', async () => { it('should throw if start index + length to copy is greater than length of byte array', async () => {