aboutsummaryrefslogtreecommitdiffstats
path: root/packages/contracts
diff options
context:
space:
mode:
Diffstat (limited to 'packages/contracts')
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol58
-rw-r--r--packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol31
-rw-r--r--packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol2
-rw-r--r--packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol54
-rw-r--r--packages/contracts/src/utils/signing_utils.ts4
-rw-r--r--packages/contracts/test/exchange/core.ts5
-rw-r--r--packages/contracts/test/exchange/signature_validator.ts8
-rw-r--r--packages/contracts/test/libraries/lib_bytes.ts47
8 files changed, 103 insertions, 106 deletions
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
index 423f306cb..6c018683e 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -81,7 +81,9 @@ contract MixinSignatureValidator is
signature.length >= 1,
INVALID_SIGNATURE_LENGTH
);
- SignatureType signatureType = SignatureType(uint8(signature[0]));
+
+ // Pop last byte off of
+ SignatureType signatureType = SignatureType(uint8(popByte(signature)));
// Variables are not scoped in Solidity.
uint8 v;
@@ -104,7 +106,7 @@ contract MixinSignatureValidator is
// a correctly formatted but incorrect signature.
} else if (signatureType == SignatureType.Invalid) {
require(
- signature.length == 1,
+ signature.length == 0,
INVALID_SIGNATURE_LENGTH
);
isValid = false;
@@ -113,12 +115,12 @@ contract MixinSignatureValidator is
// Signature using EIP712
} else if (signatureType == SignatureType.EIP712) {
require(
- signature.length == 66,
+ signature.length == 65,
INVALID_SIGNATURE_LENGTH
);
- v = uint8(signature[1]);
- r = readBytes32(signature, 2);
- s = readBytes32(signature, 34);
+ v = uint8(signature[0]);
+ r = readBytes32(signature, 1);
+ s = readBytes32(signature, 33);
recovered = ecrecover(hash, v, r, s);
isValid = signer == recovered;
return isValid;
@@ -126,12 +128,12 @@ contract MixinSignatureValidator is
// Signed using web3.eth_sign
} else if (signatureType == SignatureType.Ecrecover) {
require(
- signature.length == 66,
+ signature.length == 65,
INVALID_SIGNATURE_LENGTH
);
- v = uint8(signature[1]);
- r = readBytes32(signature, 2);
- s = readBytes32(signature, 34);
+ v = uint8(signature[0]);
+ r = readBytes32(signature, 1);
+ s = readBytes32(signature, 33);
recovered = ecrecover(
keccak256("\x19Ethereum Signed Message:\n32", hash),
v,
@@ -151,7 +153,7 @@ contract MixinSignatureValidator is
// submit using `Caller`. Having `Caller` allows this flexibility.
} else if (signatureType == SignatureType.Caller) {
require(
- signature.length == 1,
+ signature.length == 0,
INVALID_SIGNATURE_LENGTH
);
isValid = signer == msg.sender;
@@ -160,37 +162,25 @@ contract MixinSignatureValidator is
// Signature verified by signer contract.
// If used with an order, the maker of the order is the signer contract.
} else if (signatureType == SignatureType.Signer) {
- // Pass in signature without signature type.
- bytes memory signatureWithoutType = deepCopyBytes(
- signature,
- 1,
- signature.length - 1
- );
- isValid = ISigner(signer).isValidSignature(hash, signatureWithoutType);
+ isValid = ISigner(signer).isValidSignature(hash, signature);
return isValid;
// Signature verified by validator contract.
// If used with an order, the maker of the order can still be an EOA.
// A signature using this type should be encoded as:
- // | Offset | Length | Contents |
- // | 0x00 | 1 | Signature type is always "\x06" |
- // | 0x01 | 20 | Address of validator contract |
- // | 0x15 | ** | Signature to validate |
+ // | Offset | Length | Contents |
+ // | 0x00 | x | Signature to validate |
+ // | 0x00 + x | 20 | Address of validator contract |
+ // | 0x14 + x | 1 | Signature type is always "\x06" |
} else if (signatureType == SignatureType.Validator) {
- address validator = readAddress(signature, 1);
+ address validator = popAddress(signature);
if (!allowedValidators[signer][validator]) {
return false;
}
- // Pass in signature without type or validator address.
- bytes memory signatureWithoutTypeOrAddress = deepCopyBytes(
- signature,
- 21,
- signature.length - 21
- );
isValid = IValidator(validator).isValidSignature(
hash,
signer,
- signatureWithoutTypeOrAddress
+ signature
);
return isValid;
@@ -209,12 +199,12 @@ contract MixinSignatureValidator is
// https://github.com/trezor/trezor-mcu/blob/master/firmware/crypto.c#L36
} else if (signatureType == SignatureType.Trezor) {
require(
- signature.length == 66,
+ signature.length == 65,
INVALID_SIGNATURE_LENGTH
);
- v = uint8(signature[1]);
- r = readBytes32(signature, 2);
- s = readBytes32(signature, 34);
+ v = uint8(signature[0]);
+ r = readBytes32(signature, 1);
+ s = readBytes32(signature, 33);
recovered = ecrecover(
keccak256("\x19Ethereum Signed Message:\n\x41", hash),
v,
diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol
index b3ed1f620..cd5206eb8 100644
--- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol
+++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol
@@ -25,21 +25,26 @@ contract TestLibBytes is
LibBytes
{
- /// @dev Performs a deep copy of a section of a byte array.
- /// @param b Byte array that will be sliced.
- /// @param startIndex Index of first byte to copy.
- /// @param endIndex Index of last byte to copy + 1.
- /// @return A deep copy of b from startIndex to endIndex.
- function publicDeepCopyBytes(
- bytes memory b,
- uint256 startIndex,
- uint256 endIndex)
+ /// @dev Pops the last byte off of a byte array by modifying its length.
+ /// @param b Byte array that will be modified.
+ /// @return The byte that was popped off.
+ function publicPopByte(bytes memory b)
public
- pure
- returns (bytes memory copy)
+ returns (bytes memory, byte result)
+ {
+ result = popByte(b);
+ return (b, result);
+ }
+
+ /// @dev Pops the last 20 bytes off of a byte array by modifying its length.
+ /// @param b Byte array that will be modified.
+ /// @return The 20 byte address that was popped off.
+ function publicPopAddress(bytes memory b)
+ public
+ returns (bytes memory, address result)
{
- copy = deepCopyBytes(b, startIndex, endIndex);
- return copy;
+ result = popAddress(b);
+ return (b, result);
}
/// @dev Tests equality of two byte arrays.
diff --git a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
index 3bd3179e5..11576fd89 100644
--- a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
+++ b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol
@@ -75,7 +75,7 @@ contract Whitelist is
// This contract must be the entry point for the transaction.
require(takerAddress == tx.origin);
- // Check if maker is on the whitelist
+ // Check if maker is on the whitelist.
require(
isWhitelisted[order.makerAddress],
ADDRESS_NOT_WHITELISTED
diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
index 5c4fdbb23..f0a622fe4 100644
--- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
+++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
@@ -27,39 +27,45 @@ contract LibBytes {
string constant GTE_32_LENGTH_REQUIRED = "Length must be greater than or equal to 32.";
string constant INDEX_OUT_OF_BOUNDS = "Specified array index is out of bounds.";
- /// @dev Performs a deep copy of a section of a byte array.
- /// @param b Byte array that will be copied.
- /// @param index Index of first byte to copy.
- /// @param len Number of bytes to copy starting from index.
- /// @return A deep copy `len` bytes of b starting from index.
- function deepCopyBytes(
- bytes memory b,
- uint256 index,
- uint256 len)
+ /// @dev Pops the last byte off of a byte array by modifying its length.
+ /// @param b Byte array that will be modified.
+ /// @return The byte that was popped off.
+ function popByte(bytes memory b)
internal
- pure
- returns (bytes memory)
+ returns (byte result)
{
require(
- b.length >= index + len,
- INDEX_OUT_OF_BOUNDS
+ b.length > 0,
+ GT_ZERO_LENGTH_REQUIRED
);
- bytes memory copy = new bytes(len);
+ result = b[b.length - 1];
+ assembly {
+ let newLen := sub(mload(b), 1)
+ mstore(b, newLen)
+ }
+ result;
+ }
- // Arrays are prefixed by a 256 bit length parameter
- index += 32;
+ /// @dev Pops the last 20 bytes off of a byte array by modifying its length.
+ /// @param b Byte array that will be modified.
+ /// @return The 20 byte address that was popped off.
+ function popAddress(bytes memory b)
+ internal
+ returns (address result)
+ {
+ require(
+ b.length >= 20,
+ GTE_20_LENGTH_REQUIRED
+ );
+
+ result = readAddress(b, b.length - 20);
assembly {
- // Start storing onto copy after length field
- let storeStartIndex := add(copy, 32)
- // Start loading from b at index
- let startLoadIndex := add(b, index)
- for {let i := 0} lt(i, len) {i := add(i, 32)} {
- mstore(add(storeStartIndex, i), mload(add(startLoadIndex, i)))
- }
+ let newLen := sub(mload(b), 20)
+ mstore(b, newLen)
}
- return copy;
+ return result;
}
/// @dev Tests equality of two byte arrays.
diff --git a/packages/contracts/src/utils/signing_utils.ts b/packages/contracts/src/utils/signing_utils.ts
index 61ab1f138..4c36c8310 100644
--- a/packages/contracts/src/utils/signing_utils.ts
+++ b/packages/contracts/src/utils/signing_utils.ts
@@ -8,19 +8,19 @@ export const signingUtils = {
const prefixedMessage = ethUtil.hashPersonalMessage(message);
const ecSignature = ethUtil.ecsign(prefixedMessage, privateKey);
const signature = Buffer.concat([
- ethUtil.toBuffer(signatureType),
ethUtil.toBuffer(ecSignature.v),
ecSignature.r,
ecSignature.s,
+ ethUtil.toBuffer(signatureType),
]);
return signature;
} else if (signatureType === SignatureType.EIP712) {
const ecSignature = ethUtil.ecsign(message, privateKey);
const signature = Buffer.concat([
- ethUtil.toBuffer(signatureType),
ethUtil.toBuffer(ecSignature.v),
ecSignature.r,
ecSignature.s,
+ ethUtil.toBuffer(signatureType),
]);
return signature;
} else {
diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts
index be8d14cb0..bc476a5ee 100644
--- a/packages/contracts/test/exchange/core.ts
+++ b/packages/contracts/test/exchange/core.ts
@@ -460,10 +460,11 @@ describe('Exchange core', () => {
makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 18),
});
+ const v = ethUtil.toBuffer(signedOrder.signature.slice(0, 4));
const invalidR = ethUtil.sha3('invalidR');
const invalidS = ethUtil.sha3('invalidS');
- const signatureTypeAndV = signedOrder.signature.slice(0, 6);
- const invalidSigBuff = Buffer.concat([ethUtil.toBuffer(signatureTypeAndV), invalidR, invalidS]);
+ const signatureType = ethUtil.toBuffer(`0x${signedOrder.signature.slice(-2)}`);
+ const invalidSigBuff = Buffer.concat([v, invalidR, invalidS, signatureType]);
const invalidSigHex = `0x${invalidSigBuff.toString('hex')}`;
signedOrder.signature = invalidSigHex;
return expect(exchangeWrapper.fillOrderAsync(signedOrder, takerAddress)).to.be.rejectedWith(
diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts
index 376fff438..ca679e344 100644
--- a/packages/contracts/test/exchange/signature_validator.ts
+++ b/packages/contracts/test/exchange/signature_validator.ts
@@ -75,13 +75,11 @@ describe('MixinSignatureValidator', () => {
});
it('should return false with an invalid signature', async () => {
+ const v = ethUtil.toBuffer(signedOrder.signature.slice(0, 4));
const invalidR = ethUtil.sha3('invalidR');
const invalidS = ethUtil.sha3('invalidS');
- const invalidSigBuff = Buffer.concat([
- ethUtil.toBuffer(signedOrder.signature.slice(0, 6)),
- invalidR,
- invalidS,
- ]);
+ const signatureType = ethUtil.toBuffer(`0x${signedOrder.signature.slice(-2)}`);
+ const invalidSigBuff = Buffer.concat([v, invalidR, invalidS, signatureType]);
const invalidSigHex = `0x${invalidSigBuff.toString('hex')}`;
signedOrder.signature = invalidSigHex;
const orderHashHex = orderUtils.getOrderHashHex(signedOrder);
diff --git a/packages/contracts/test/libraries/lib_bytes.ts b/packages/contracts/test/libraries/lib_bytes.ts
index c85cee5e3..4eedad25d 100644
--- a/packages/contracts/test/libraries/lib_bytes.ts
+++ b/packages/contracts/test/libraries/lib_bytes.ts
@@ -22,6 +22,7 @@ describe('LibBytes', () => {
let owner: string;
let libBytes: TestLibBytesContract;
const byteArrayShorterThan32Bytes = '0x012345';
+ const byteArrayShorterThan20Bytes = byteArrayShorterThan32Bytes;
const byteArrayLongerThan32Bytes =
'0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef';
const byteArrayLongerThan32BytesFirstBytesSwapped =
@@ -60,39 +61,35 @@ describe('LibBytes', () => {
await blockchainLifecycle.revertAsync();
});
- describe('deepCopyBytes', () => {
- const byteArrayLongerThan32BytesLen = (byteArrayLongerThan32Bytes.length - 2) / 2;
- it('should return a byte array of length 0 if len is 0', async () => {
- const index = new BigNumber(0);
- const len = new BigNumber(0);
- const copy = await libBytes.publicDeepCopyBytes.callAsync(byteArrayLongerThan32Bytes, index, len);
- expect(copy).to.equal(constants.NULL_BYTES);
+ describe('popByte', () => {
+ it('should revert if length is 0', async () => {
+ return expect(libBytes.publicPopByte.callAsync(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 () => {
- const index = new BigNumber(0);
- const len = new BigNumber(byteArrayLongerThan32BytesLen + 1);
- return expect(
- libBytes.publicDeepCopyBytes.callAsync(byteArrayLongerThan32Bytes, index, len),
- ).to.be.rejectedWith(constants.REVERT);
+ it('should pop the last byte from the input and return it', async () => {
+ const [newBytes, poppedByte] = await libBytes.publicPopByte.callAsync(byteArrayLongerThan32Bytes);
+ const expectedNewBytes = byteArrayLongerThan32Bytes.slice(0, -2);
+ const expectedPoppedByte = `0x${byteArrayLongerThan32Bytes.slice(-2)}`;
+ expect(newBytes).to.equal(expectedNewBytes);
+ expect(poppedByte).to.equal(expectedPoppedByte);
});
+ });
- it('should copy the entire byte array if index = 0 and len = b.length', async () => {
- const index = new BigNumber(0);
- const len = new BigNumber(byteArrayLongerThan32BytesLen);
- const copy = await libBytes.publicDeepCopyBytes.callAsync(byteArrayLongerThan32Bytes, index, len);
- expect(copy).to.equal(byteArrayLongerThan32Bytes);
+ describe('popAddress', () => {
+ it('should revert if length is less than 20', async () => {
+ return expect(libBytes.publicPopAddress.callAsync(byteArrayShorterThan20Bytes)).to.be.rejectedWith(
+ constants.REVERT,
+ );
});
- it('should copy part of the byte array if area to copy is less than b.length', async () => {
- const index = new BigNumber(10);
- const len = new BigNumber(4);
- const copy = await libBytes.publicDeepCopyBytes.callAsync(byteArrayLongerThan32Bytes, index, len);
- const expectedCopy = `0x${byteArrayLongerThan32Bytes.slice(22, 30)}`;
- expect(copy).to.equal(expectedCopy);
+ it('should pop the last 20 bytes from the input and return it', async () => {
+ const [newBytes, poppedAddress] = await libBytes.publicPopAddress.callAsync(byteArrayLongerThan32Bytes);
+ const expectedNewBytes = byteArrayLongerThan32Bytes.slice(0, -40);
+ const expectedPoppedAddress = `0x${byteArrayLongerThan32Bytes.slice(-40)}`;
+ expect(newBytes).to.equal(expectedNewBytes);
+ expect(poppedAddress).to.equal(expectedPoppedAddress);
});
});
-
describe('areBytesEqual', () => {
it('should return true if byte arrays are equal (both arrays < 32 bytes)', async () => {
const areBytesEqual = await libBytes.publicAreBytesEqual.callAsync(