diff options
Diffstat (limited to 'packages/contracts')
6 files changed, 55 insertions, 24 deletions
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol index 6c018683e..8ef961a81 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol @@ -43,7 +43,8 @@ contract MixinSignatureValidator is function preSign( bytes32 hash, address signer, - bytes signature) + bytes signature + ) external { require( @@ -56,7 +57,10 @@ contract MixinSignatureValidator is /// @dev Approves a Validator contract to verify signatures on signer's behalf. /// @param validator Address of Validator contract. /// @param approval Approval or disapproval of Validator contract. - function approveSignatureValidator(address validator, bool approval) + function approveSignatureValidator( + address validator, + bool approval + ) external { allowedValidators[msg.sender][validator] = approval; @@ -70,19 +74,19 @@ contract MixinSignatureValidator is function isValidSignature( bytes32 hash, address signer, - bytes memory signature) + bytes memory signature + ) internal view returns (bool isValid) { // TODO: Domain separation: make hash depend on role. (Taker sig should not be valid as maker sig, etc.) - require( signature.length >= 1, INVALID_SIGNATURE_LENGTH ); - // Pop last byte off of + // Pop last byte off of signature byte array. SignatureType signatureType = SignatureType(uint8(popByte(signature))); // Variables are not scoped in Solidity. @@ -90,7 +94,7 @@ contract MixinSignatureValidator is bytes32 r; bytes32 s; address recovered; - + // 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 @@ -173,7 +177,9 @@ contract MixinSignatureValidator is // | 0x00 + x | 20 | Address of validator contract | // | 0x14 + x | 1 | Signature type is always "\x06" | } else if (signatureType == SignatureType.Validator) { + // Pop last 20 bytes off of signature byte array. address validator = popAddress(signature); + // Ensure signer has approved validator. if (!allowedValidators[signer][validator]) { return false; } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol index d28df11d8..12bba1ef2 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol @@ -43,7 +43,8 @@ contract MixinTransactions is uint256 salt, address signer, bytes data, - bytes signature) + bytes signature + ) external { // Prevent reentrancy diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index cd5206eb8..b835e70d0 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -30,7 +30,7 @@ contract TestLibBytes is /// @return The byte that was popped off. function publicPopByte(bytes memory b) public - returns (bytes memory, byte result) + returns (bytes memory, bytes1 result) { result = popByte(b); return (b, result); diff --git a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol index 11576fd89..034222915 100644 --- a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol +++ b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol @@ -27,7 +27,9 @@ contract Whitelist is Ownable { // Revert reasons - string constant ADDRESS_NOT_WHITELISTED = "Address not whitelisted."; + string constant MAKER_NOT_WHITELISTED = "Maker address not whitelisted."; + string constant TAKER_NOT_WHITELISTED = "Taker address not whitelisted."; + string constant INVALID_SENDER = "Sender must equal transaction origin."; // Mapping of address => whitelist status. mapping (address => bool) public isWhitelisted; @@ -42,7 +44,7 @@ contract Whitelist is public { EXCHANGE = IExchange(_exchange); - TX_ORIGIN_SIGNATURE = abi.encodePacked(VALIDATOR_SIGNATURE_BYTE, address(this)); + TX_ORIGIN_SIGNATURE = abi.encodePacked(address(this), VALIDATOR_SIGNATURE_BYTE); } /// @dev Adds or removes an address from the whitelist. @@ -67,24 +69,28 @@ contract Whitelist is LibOrder.Order memory order, uint256 takerAssetFillAmount, uint256 salt, - bytes memory orderSignature) + bytes memory orderSignature + ) public { address takerAddress = msg.sender; // This contract must be the entry point for the transaction. - require(takerAddress == tx.origin); + require( + takerAddress == tx.origin, + INVALID_SENDER + ); // Check if maker is on the whitelist. require( isWhitelisted[order.makerAddress], - ADDRESS_NOT_WHITELISTED + MAKER_NOT_WHITELISTED ); // Check if taker is on the whitelist. require( isWhitelisted[takerAddress], - ADDRESS_NOT_WHITELISTED + TAKER_NOT_WHITELISTED ); // Encode arguments into byte array. @@ -111,7 +117,8 @@ contract Whitelist is function isValidSignature( bytes32 hash, address signer, - bytes signature) + bytes signature + ) external view returns (bool isValid) diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index f0a622fe4..df2221c93 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -32,19 +32,23 @@ contract LibBytes { /// @return The byte that was popped off. function popByte(bytes memory b) internal - returns (byte result) + pure + returns (bytes1 result) { require( b.length > 0, GT_ZERO_LENGTH_REQUIRED ); + // Store last byte. result = b[b.length - 1]; + assembly { + // Decrement length of byte array. let newLen := sub(mload(b), 1) mstore(b, newLen) } - result; + return result; } /// @dev Pops the last 20 bytes off of a byte array by modifying its length. @@ -52,6 +56,7 @@ contract LibBytes { /// @return The 20 byte address that was popped off. function popAddress(bytes memory b) internal + pure returns (address result) { require( @@ -59,9 +64,11 @@ contract LibBytes { GTE_20_LENGTH_REQUIRED ); + // Store last 20 bytes. result = readAddress(b, b.length - 20); assembly { + // Subtract 20 from byte array length. let newLen := sub(mload(b), 20) mstore(b, newLen) } @@ -72,7 +79,10 @@ contract LibBytes { /// @param lhs First byte array to compare. /// @param rhs Second byte array to compare. /// @return True if arrays are the same. False otherwise. - function areBytesEqual(bytes memory lhs, bytes memory rhs) + function areBytesEqual( + bytes memory lhs, + bytes memory rhs + ) internal pure returns (bool equal) @@ -106,7 +116,8 @@ contract LibBytes { /// @return address from byte array. function readAddress( bytes memory b, - uint256 index) + uint256 index + ) internal pure returns (address result) @@ -138,7 +149,8 @@ contract LibBytes { function writeAddress( bytes memory b, uint256 index, - address input) + address input + ) internal pure { @@ -175,7 +187,8 @@ contract LibBytes { /// @return bytes32 value from byte array. function readBytes32( bytes memory b, - uint256 index) + uint256 index + ) internal pure returns (bytes32 result) @@ -202,7 +215,8 @@ contract LibBytes { function writeBytes32( bytes memory b, uint256 index, - bytes32 input) + bytes32 input + ) internal pure { @@ -226,7 +240,8 @@ contract LibBytes { /// @return uint256 value from byte array. function readUint256( bytes memory b, - uint256 index) + uint256 index + ) internal pure returns (uint256 result) @@ -241,7 +256,8 @@ contract LibBytes { function writeUint256( bytes memory b, uint256 index, - uint256 input) + uint256 input + ) internal pure { diff --git a/packages/contracts/test/libraries/lib_bytes.ts b/packages/contracts/test/libraries/lib_bytes.ts index 4eedad25d..e817951ab 100644 --- a/packages/contracts/test/libraries/lib_bytes.ts +++ b/packages/contracts/test/libraries/lib_bytes.ts @@ -90,6 +90,7 @@ describe('LibBytes', () => { 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( |