From 80215ea1818874bcd3661259df6f2d3279cc59f2 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Wed, 23 May 2018 15:36:35 -0700 Subject: LibMem + TestLibMem + LibAssetProxyDecoder + DummyERC721Receiver --- .../current/protocol/AssetProxy/ERC20Proxy.sol | 24 +-- .../current/protocol/AssetProxy/ERC721Proxy.sol | 28 ++- .../DummyERC721Receiver/DummyERC721Receiver.sol | 62 ++++++ .../current/test/TestLibMem/TestLibMem.sol | 238 +++++++++++++++++++++ .../LibAssetProxyDecoder/LibAssetProxyDecoder.sol | 74 +++++++ .../contracts/current/utils/LibBytes/LibBytes.sol | 76 ++++++- .../src/contracts/current/utils/LibMem/LibMem.sol | 104 +++++++++ 7 files changed, 579 insertions(+), 27 deletions(-) create mode 100644 packages/contracts/src/contracts/current/test/DummyERC721Receiver/DummyERC721Receiver.sol create mode 100644 packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol create mode 100644 packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol create mode 100644 packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index 2c321e134..017f94b1a 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -20,12 +20,14 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; import "../../utils/LibBytes/LibBytes.sol"; +import "../../utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol"; import "./MixinAssetProxy.sol"; import "./MixinAuthorizable.sol"; import "../../tokens/ERC20Token/IERC20Token.sol"; contract ERC20Proxy is LibBytes, + LibAssetProxyDecoder, MixinAssetProxy, MixinAuthorizable { @@ -34,34 +36,32 @@ contract ERC20Proxy is uint8 constant PROXY_ID = 1; /// @dev Internal version of `transferFrom`. - /// @param assetMetadata Encoded byte array. + /// @param proxyData Encoded byte array. /// @param from Address to transfer asset from. /// @param to Address to transfer asset to. /// @param amount Amount of asset to transfer. function transferFromInternal( - bytes memory assetMetadata, + bytes memory proxyData, address from, address to, uint256 amount ) internal { + // Decode proxy data. + ( + uint8 proxyId, + address token + ) = decodeERC20Data(proxyData); + // Data must be intended for this proxy. uint256 length = assetMetadata.length; require( - length == 21, - LENGTH_21_REQUIRED - ); - // TODO: Is this too inflexible in the future? - require( - uint8(assetMetadata[length - 1]) == PROXY_ID, - ASSET_PROXY_ID_MISMATCH + proxyId == PROXY_ID, + PROXY_ID_MISMATCH ); - // Decode metadata. - address token = readAddress(assetMetadata, 0); - // Transfer tokens. bool success = IERC20Token(token).transferFrom(from, to, amount); require( diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index 07e01c774..f35e48eee 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -20,12 +20,14 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; import "../../utils/LibBytes/LibBytes.sol"; +import "../../utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol"; import "./MixinAssetProxy.sol"; import "./MixinAuthorizable.sol"; import "../../tokens/ERC721Token/ERC721Token.sol"; contract ERC721Proxy is LibBytes, + LibAssetProxyDecoder, MixinAssetProxy, MixinAuthorizable { @@ -33,19 +35,29 @@ contract ERC721Proxy is // Id of this proxy. uint8 constant PROXY_ID = 2; + string constant PROXY_ID_MISMATCH = "Proxy id in metadata does not match this proxy id."; + /// @dev Internal version of `transferFrom`. - /// @param assetMetadata Encoded byte array. + /// @param proxyData Encoded byte array. /// @param from Address to transfer asset from. /// @param to Address to transfer asset to. /// @param amount Amount of asset to transfer. function transferFromInternal( - bytes memory assetMetadata, + bytes memory proxyData, address from, address to, uint256 amount ) internal { + // Decode proxy data. + ( + uint8 proxyId, + address token, + uint256 tokenId, + bytes memory data + ) = decodeERC721Data(proxyData); + // Data must be intended for this proxy. uint256 length = assetMetadata.length; @@ -56,8 +68,8 @@ contract ERC721Proxy is // TODO: Is this too inflexible in the future? require( - uint8(assetMetadata[length - 1]) == PROXY_ID, - ASSET_PROXY_ID_MISMATCH + proxyId == PROXY_ID, + PROXY_ID_MISMATCH ); // There exists only 1 of each token. @@ -66,15 +78,9 @@ contract ERC721Proxy is INVALID_AMOUNT ); - // Decode metadata - address token = readAddress(assetMetadata, 0); - uint256 tokenId = readUint256(assetMetadata, 20); - // Transfer token. // Either succeeds or throws. - // @TODO: Call safeTransferFrom if there is additional - // data stored in `assetMetadata`. - ERC721Token(token).transferFrom(from, to, tokenId); + ERC721Token(token).safeTransferFrom(from, to, tokenId, data); } /// @dev Gets the proxy id associated with the proxy address. diff --git a/packages/contracts/src/contracts/current/test/DummyERC721Receiver/DummyERC721Receiver.sol b/packages/contracts/src/contracts/current/test/DummyERC721Receiver/DummyERC721Receiver.sol new file mode 100644 index 000000000..1596f3357 --- /dev/null +++ b/packages/contracts/src/contracts/current/test/DummyERC721Receiver/DummyERC721Receiver.sol @@ -0,0 +1,62 @@ +/* +The MIT License (MIT) + +Copyright (c) 2016 Smart Contract Solutions, Inc. + +Permission is hereby granted, free of charge, to any person obtaining +a copy of this software and associated documentation files (the +"Software"), to deal in the Software without restriction, including +without limitation the rights to use, copy, modify, merge, publish, +distribute, sublicense, and/or sell copies of the Software, and to +permit persons to whom the Software is furnished to do so, subject to +the following conditions: + +The above copyright notice and this permission notice shall be included +in all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. +IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY +CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, +TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE +SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. +*/ + +pragma solidity ^0.4.24; + +import "../../tokens/ERC721Token/IERC721Receiver.sol"; + +contract DummyERC721Receiver is + IERC721Receiver +{ + + event TokenReceived( + address from, + uint256 tokenId, + bytes data + ); + + /** + * @notice Handle the receipt of an NFT + * @dev The ERC721 smart contract calls this function on the recipient + * after a `safetransfer`. This function MAY throw to revert and reject the + * transfer. This function MUST use 50,000 gas or less. Return of other + * than the magic value MUST result in the transaction being reverted. + * Note: the contract address is always the message sender. + * @param _from The sending address + * @param _tokenId The NFT identifier which is being transfered + * @param _data Additional data with no specified format + * @return `bytes4(keccak256("onERC721Received(address,uint256,bytes)"))` + */ + function onERC721Received( + address _from, + uint256 _tokenId, + bytes _data) + public + returns (bytes4) + { + emit TokenReceived(_from, _tokenId, _data); + return ERC721_RECEIVED; + } +} diff --git a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol new file mode 100644 index 000000000..4cf62bf3a --- /dev/null +++ b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol @@ -0,0 +1,238 @@ +/* + + Copyright 2018 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.24; + +import "../../utils/LibMem/LibMem.sol"; +import "../../utils/LibBytes/LibBytes.sol"; + +contract TestLibMem is + LibMem, + LibBytes +{ + + function test1() + public + pure + { + // Length of array & length to copy + uint256 length = 0; + + // Create source array + bytes memory sourceArray = new bytes(length); + + // Create dest array with same contents as source array + bytes memory destArray = new bytes(length); + memcpy( + getMemAddress(destArray) + 32, // skip copying array length + getMemAddress(sourceArray) + 32, // skip copying array length + length + ); + + // Verify contents of source & dest arrays match + require( + areBytesEqual(sourceArray, destArray), + "Test #1 failed. Array contents are not the same." + ); + } + + function test2() + public + pure + { + // Length of array & length to copy + uint256 length = 1; + + // Create source array + bytes memory sourceArray = new bytes(length); + sourceArray[0] = byte(1); + + // Create dest array with same contents as source array + bytes memory destArray = new bytes(length); + memcpy( + getMemAddress(destArray) + 32, // skip copying array length + getMemAddress(sourceArray) + 32, // skip copying array length + length + ); + + // Verify contents of source & dest arrays match + require( + areBytesEqual(sourceArray, destArray), + "Test #2 failed. Array contents are not the same." + ); + } + + function test3() + public + pure + { + // Length of array & length to copy + uint256 length = 11; + + // Create source array + bytes memory sourceArray = new bytes(length); + for(uint256 i = 0; i < length; ++i) { + sourceArray[i] = byte((i % 0xF) + 1); // [1..f] + } + + // Create dest array with same contents as source array + bytes memory destArray = new bytes(length); + memcpy( + getMemAddress(destArray) + 32, // skip copying array length + getMemAddress(sourceArray) + 32, // skip copying array length + length + ); + + // Verify contents of source & dest arrays match + require( + areBytesEqual(sourceArray, destArray), + "Test #3 failed. Array contents are not the same." + ); + } + + function test4() + public + pure + { + // Length of array & length to copy + uint256 length = 32; + + // Create source array + bytes memory sourceArray = new bytes(length); + for(uint256 i = 0; i < length; ++i) { + sourceArray[i] = byte((i % 0xF) + 1); // [1..f] + } + + // Create dest array with same contents as source array + bytes memory destArray = new bytes(length); + memcpy( + getMemAddress(destArray) + 32, // skip copying array length + getMemAddress(sourceArray) + 32, // skip copying array length + length + ); + + // Verify contents of source & dest arrays match + require( + areBytesEqual(sourceArray, destArray), + "Test #4 failed. Array contents are not the same." + ); + } + + function test5() + public + pure + { + // Length of array & length to copy + uint256 length = 72; + + // Create source array + bytes memory sourceArray = new bytes(length); + for(uint256 i = 0; i < length; ++i) { + sourceArray[i] = byte((i % 0xF) + 1); // [1..f] + } + + // Create dest array with same contents as source array + bytes memory destArray = new bytes(length); + memcpy( + getMemAddress(destArray) + 32, // skip copying array length + getMemAddress(sourceArray) + 32, // skip copying array length + length + ); + + // Verify contents of source & dest arrays match + require( + areBytesEqual(sourceArray, destArray), + "Test #5 failed. Array contents are not the same." + ); + } + + + function test6() + public + pure + { + // Length of arrays + uint256 length1 = 72; + uint256 length2 = 100; + + // The full source array is used for comparisons at the end + bytes memory fullSourceArray = new bytes(length1 + length2); + + // First source array + bytes memory sourceArray1 = new bytes(length1); + for(uint256 i = 0; i < length1; ++i) { + sourceArray1[i] = byte((i % 0xF) + 1); // [1..f] + fullSourceArray[i] = byte((i % 0xF) + 1); // [1..f] + } + + // Second source array + bytes memory sourceArray2 = new bytes(length2); + for(uint256 j = 0; i < length2; ++i) { + sourceArray2[j] = byte((j % 0xF) + 1); // [1..f] + fullSourceArray[length1+j] = byte((j % 0xF) + 1); // [1..f] + } + + // Create dest array with same contents as source arrays + bytes memory destArray = new bytes(length1 + length2); + memcpy( + getMemAddress(destArray) + 32, // skip copying array length + getMemAddress(sourceArray1) + 32, // skip copying array length + length1 + ); + memcpy( + getMemAddress(destArray) + 32 + length1, // skip copying array length + sourceArray1 bytes + getMemAddress(sourceArray2) + 32, // skip copying array length + length2 + ); + + // Verify contents of source & dest arrays match + require( + areBytesEqual(fullSourceArray, destArray), + "Test #6 failed. Array contents are not the same." + ); + } + + function test7() + public + pure + { + // Length of array & length to copy + uint256 length = 72; + + // Create source array + bytes memory sourceArray = new bytes(length); + for(uint256 i = 0; i < length; ++i) { + sourceArray[i] = byte((i % 0xF) + 1); // [1..f] + } + + // Create dest array with same contents as source array + bytes memory destArray = new bytes(length); + memcpy( + getMemAddress(destArray) + 32, // skip copying array length + getMemAddress(sourceArray) + 32, // skip copying array length + length - 8 // Copy all but last byte. + ); + + // Verify contents of source & dest arrays match + // We expect this to fail + require( + areBytesEqual(sourceArray, destArray), + "Test #7 failed. Array contents are not the same." + ); + } +} diff --git a/packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol b/packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol new file mode 100644 index 000000000..ba53f2769 --- /dev/null +++ b/packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol @@ -0,0 +1,74 @@ +/* + + Copyright 2018 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.24; +pragma experimental ABIEncoderV2; + +import "../LibBytes/LibBytes.sol"; + +contract LibAssetProxyDecoder is + LibBytes +{ + + string constant INVALID_ERC20_METADATA_LENGTH = "Metadata must have a length of 21."; + string constant INVALID_ERC721_METADATA_LENGTH = "Metadata must have a length of at least 53."; + + /// @dev Decodes ERC721 Asset Proxy data + function decodeERC20Data(bytes memory proxyData) + internal + pure + returns ( + uint8 proxyId, + address token + ) + { + require( + proxyData.length == 21, + INVALID_ERC20_METADATA_LENGTH + ); + proxyId = uint8(proxyData[0]); + token = readAddress(proxyData, 1); + + return (proxyId, token); + } + + /// @dev Decodes ERC721 Asset Proxy data + function decodeERC721Data(bytes memory proxyData) + internal + pure + returns ( + uint8 proxyId, + address token, + uint256 tokenId, + bytes memory data + ) + { + require( + proxyData.length >= 53, + INVALID_ERC721_METADATA_LENGTH + ); + proxyId = uint8(proxyData[0]); + token = readAddress(proxyData, 1); + tokenId = readUint256(proxyData, 21); + if (proxyData.length > 53) { + data = readBytes(proxyData, 53); + } + + return (proxyId, token, tokenId, data); + } +} diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index df2221c93..fb8359462 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -18,7 +18,11 @@ pragma solidity ^0.4.24; -contract LibBytes { +import "../LibMem/LibMem.sol"; + +contract LibBytes is + LibMem +{ // Revert reasons string constant GT_ZERO_LENGTH_REQUIRED = "Length must be greater than 0."; @@ -42,7 +46,7 @@ contract LibBytes { // Store last byte. result = b[b.length - 1]; - + assembly { // Decrement length of byte array. let newLen := sub(mload(b), 1) @@ -125,7 +129,7 @@ contract LibBytes { require( b.length >= index + 20, // 20 is length of address GTE_20_LENGTH_REQUIRED - ); + ); // Add offset to index: // 1. Arrays are prefixed by 32-byte length parameter (add 32 to index) @@ -157,7 +161,7 @@ contract LibBytes { require( b.length >= index + 20, // 20 is length of address GTE_20_LENGTH_REQUIRED - ); + ); // Add offset to index: // 1. Arrays are prefixed by 32-byte length parameter (add 32 to index) @@ -264,6 +268,7 @@ contract LibBytes { writeBytes32(b, index, bytes32(input)); } +======= /// @dev Reads the first 4 bytes from a byte array of arbitrary length. /// @param b Byte array to read first 4 bytes from. /// @return First 4 bytes of data. @@ -281,4 +286,67 @@ contract LibBytes { } return result; } + + /// @dev Reads a uint256 value from a position in a byte array. + /// @param b Byte array containing a uint256 value. + /// @param index Index in byte array of uint256 value. + /// @return uint256 value from byte array. + function readBytes( + bytes memory b, + uint256 index + ) + internal + pure + returns (bytes memory result) + { + // Read length of nested bytes + require( + b.length >= index + 32, + GTE_32_LENGTH_REQUIRED + ); + uint256 nestedBytesLength = readUint256(b, index); + + // Assert length of is valid, given + // length of nested bytes + require( + b.length >= index + 32 + nestedBytesLength, + GTE_32_LENGTH_REQUIRED + ); + + // Allocate memory and copy value to result + result = new bytes(nestedBytesLength); + memcpy( + getMemAddress(result) + 32, // +32 skips array length + getMemAddress(b) + index + 32, // +32 skips array length + nestedBytesLength + ); + + return result; + } + + /// @dev Writes a uint256 into a specific position in a byte array. + /// @param b Byte array to insert into. + /// @param index Index in byte array of . + /// @param input uint256 to put into byte array. + function writeBytes( + bytes memory b, + uint256 index, + bytes memory input + ) + internal + pure + { + // Read length of nested bytes + require( + b.length >= index + 32 /* 32 bytes to store length */ + input.length, + GTE_32_LENGTH_REQUIRED + ); + + // Copy into + memcpy( + getMemAddress(b) + index, + getMemAddress(input), + input.length + 32 /* 32 bytes to store length */ + ); + } } diff --git a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol new file mode 100644 index 000000000..b07a5da54 --- /dev/null +++ b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol @@ -0,0 +1,104 @@ +/* + + Copyright 2018 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.24; + +contract LibMem { + + function getMemAddress(bytes memory input) + internal + pure + returns (uint256 address_) + { + assembly { + address_ := input + } + return address_; + } + + /// @dev Writes a uint256 into a specific position in a byte array. + /// @param dest memory adress to copy bytes to + function memcpy( + uint256 dest, + uint256 source, + uint256 length + ) + internal + pure + { + // Base cases + if(length == 0) return; + if(source == dest) return; + + // Copy bytes from source to dest + assembly { + // Compute number of complete words to copy + remaining bytes + let lenFullWords := div(add(length, 0x1F), 0x20) + let remainder := mod(length, 0x20) + if gt(remainder, 0) { + lenFullWords := sub(lenFullWords, 1) + } + + // Copy full words from source to dest + let offset := 0 + let maxOffset := mul(0x20, lenFullWords) + for {offset := 0} lt(offset, maxOffset) {offset := add(offset, 0x20)} { + mstore(add(dest, offset), mload(add(source, offset))) + } + + // Copy remaining bytes + if gt(remainder, 0) { + // Read a full word from source, containing X bytes to copy to dest. + // We only want to keep the X bytes, zeroing out the remaining bytes. + // We accomplish this by a right shift followed by a left shift. + // Example: + // Suppose a word of 8 bits has all 1's: [11111111] + // Let X = 7 (we want to copy the first 7 bits) + // Apply a right shift of 1: [01111111] + // Apply a left shift of 1: [11111110] + let sourceShiftFactor := exp(2, mul(8, sub(0x20, remainder))) + let sourceWord := mload(add(source, offset)) + let sourceBytes := mul(div(sourceWord, sourceShiftFactor), sourceShiftFactor) + + // Read a full word from dest, containing (32-X) bytes to retain. + // We need to zero out the remaining bytes to be overwritten by source, + // while retaining the (32-X) bytes we don't want to overwrite. + // We accomplish this by a left shift followed by a right shift. + // Example: + // Suppose a word of 8 bits has all 1's: [11111111] + // Let X = 7 (we want to free the first 7 bits, and retain the last bit) + // Apply a left shift of 1: [11111110] + // Apply a right shift of 1: [01111111] + let destShiftFactor := exp(2, mul(8, remainder)) + let destWord := mload(add(dest, offset)) + let destBytes := div(mul(destWord, destShiftFactor), destShiftFactor) + + // Combine the source and dest bytes. There should be no overlap: + // The source bytes run from [0..X-1] and the dest bytes from [X..31]. + // Example: + // Following the example from above, we have [11111110] + // from the source word and [01111111] from the dest word. + // Combine these words using to get [11111111]. + let combinedDestWord := or(sourceBytes, destBytes) + + // Store the combined word into dest + mstore(add(dest, offset), combinedDestWord) + } + } + } +} -- cgit v1.2.3 From 3d65341080177bdd436e7628a76e65774b947a38 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 24 May 2018 10:56:47 -0700 Subject: Tests for libMem --- .../current/test/TestLibMem/TestLibMem.sol | 23 ++++++++-------------- packages/contracts/src/utils/artifacts.ts | 2 ++ packages/contracts/src/utils/types.ts | 1 + 3 files changed, 11 insertions(+), 15 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol index 4cf62bf3a..0c6f8fbc9 100644 --- a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol +++ b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol @@ -27,8 +27,7 @@ contract TestLibMem is { function test1() - public - pure + external { // Length of array & length to copy uint256 length = 0; @@ -52,8 +51,7 @@ contract TestLibMem is } function test2() - public - pure + external { // Length of array & length to copy uint256 length = 1; @@ -78,8 +76,7 @@ contract TestLibMem is } function test3() - public - pure + external { // Length of array & length to copy uint256 length = 11; @@ -106,8 +103,7 @@ contract TestLibMem is } function test4() - public - pure + external { // Length of array & length to copy uint256 length = 32; @@ -134,8 +130,7 @@ contract TestLibMem is } function test5() - public - pure + external { // Length of array & length to copy uint256 length = 72; @@ -163,8 +158,7 @@ contract TestLibMem is function test6() - public - pure + external { // Length of arrays uint256 length1 = 72; @@ -208,8 +202,7 @@ contract TestLibMem is } function test7() - public - pure + external { // Length of array & length to copy uint256 length = 72; @@ -232,7 +225,7 @@ contract TestLibMem is // We expect this to fail require( areBytesEqual(sourceArray, destArray), - "Test #7 failed. Array contents are not the same." + "Test #7 failed. Array contents are not the same. This is expected." ); } } diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts index 357c66a0a..1b47f1d41 100644 --- a/packages/contracts/src/utils/artifacts.ts +++ b/packages/contracts/src/utils/artifacts.ts @@ -11,6 +11,7 @@ import * as MultiSigWallet from '../artifacts/MultiSigWallet.json'; import * as MultiSigWalletWithTimeLock from '../artifacts/MultiSigWalletWithTimeLock.json'; import * as TestAssetProxyDispatcher from '../artifacts/TestAssetProxyDispatcher.json'; import * as TestLibBytes from '../artifacts/TestLibBytes.json'; +import * as TestLibMem from '../artifacts/TestLibMem.json'; import * as TestLibs from '../artifacts/TestLibs.json'; import * as TestSignatureValidator from '../artifacts/TestSignatureValidator.json'; import * as TokenRegistry from '../artifacts/TokenRegistry.json'; @@ -31,6 +32,7 @@ export const artifacts = { MultiSigWalletWithTimeLock: (MultiSigWalletWithTimeLock as any) as ContractArtifact, TestAssetProxyDispatcher: (TestAssetProxyDispatcher as any) as ContractArtifact, TestLibBytes: (TestLibBytes as any) as ContractArtifact, + TestLibMem: (TestLibMem as any) as ContractArtifact, TestLibs: (TestLibs as any) as ContractArtifact, TestSignatureValidator: (TestSignatureValidator as any) as ContractArtifact, TokenRegistry: (TokenRegistry as any) as ContractArtifact, diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index 491890fa1..cc6f00b95 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -91,6 +91,7 @@ export enum ContractName { EtherDelta = 'EtherDelta', Arbitrage = 'Arbitrage', TestAssetProxyDispatcher = 'TestAssetProxyDispatcher', + TestLibMem = 'TestLibMem', TestLibs = 'TestLibs', TestSignatureValidator = 'TestSignatureValidator', ERC20Proxy = 'ERC20Proxy', -- cgit v1.2.3 From 9b82e2df58cfd7f4dc9954fa93167450919f457f Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 24 May 2018 12:29:45 -0700 Subject: Foundation for TestLibAssetProxyDecoder --- .../TestLibAssetProxyDecoder.sol | 50 ++++++++++++++++++++++ packages/contracts/src/utils/artifacts.ts | 2 + packages/contracts/src/utils/types.ts | 1 + 3 files changed, 53 insertions(+) create mode 100644 packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol b/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol new file mode 100644 index 000000000..ac7cd63ab --- /dev/null +++ b/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol @@ -0,0 +1,50 @@ +/* + + Copyright 2018 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.24; +pragma experimental ABIEncoderV2; + +import "../../utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol"; + +contract TestLibAssetProxyDecoder is + LibAssetProxyDecoder +{ + + /// @dev Decodes ERC721 Asset Proxy data + function publicDecodeERC20Data(bytes memory proxyData) + public + pure + returns (uint8, address) + { + return decodeERC20Data(proxyData); + } + + /// @dev Decodes ERC721 Asset Proxy data + function publicDecodeERC721Data(bytes memory proxyData) + internal + pure + returns ( + uint8, + address, + uint256, + bytes memory + ) + { + return decodeERC721Data(proxyData); + } +} diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts index 1b47f1d41..44de43a95 100644 --- a/packages/contracts/src/utils/artifacts.ts +++ b/packages/contracts/src/utils/artifacts.ts @@ -10,6 +10,7 @@ import * as MixinAuthorizable from '../artifacts/MixinAuthorizable.json'; import * as MultiSigWallet from '../artifacts/MultiSigWallet.json'; import * as MultiSigWalletWithTimeLock from '../artifacts/MultiSigWalletWithTimeLock.json'; import * as TestAssetProxyDispatcher from '../artifacts/TestAssetProxyDispatcher.json'; +import * as TestLibAssetProxyDecoder from '../artifacts/TestLibAssetProxyDecoder.json'; import * as TestLibBytes from '../artifacts/TestLibBytes.json'; import * as TestLibMem from '../artifacts/TestLibMem.json'; import * as TestLibs from '../artifacts/TestLibs.json'; @@ -31,6 +32,7 @@ export const artifacts = { MultiSigWallet: (MultiSigWallet as any) as ContractArtifact, MultiSigWalletWithTimeLock: (MultiSigWalletWithTimeLock as any) as ContractArtifact, TestAssetProxyDispatcher: (TestAssetProxyDispatcher as any) as ContractArtifact, + TestLibAssetProxyDecoder: (TestLibAssetProxyDecoder as any) as ContractArtifact, TestLibBytes: (TestLibBytes as any) as ContractArtifact, TestLibMem: (TestLibMem as any) as ContractArtifact, TestLibs: (TestLibs as any) as ContractArtifact, diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index cc6f00b95..70abb2643 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -90,6 +90,7 @@ export enum ContractName { AccountLevels = 'AccountLevels', EtherDelta = 'EtherDelta', Arbitrage = 'Arbitrage', + TestLibAssetProxyDecoder = 'TestLibAssetProxyDecoder', TestAssetProxyDispatcher = 'TestAssetProxyDispatcher', TestLibMem = 'TestLibMem', TestLibs = 'TestLibs', -- cgit v1.2.3 From bc0edd40427944349ac0c3e367c9638a1c7797dd Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 24 May 2018 13:03:17 -0700 Subject: LibAssetProxyDecoder tests --- .../current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol b/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol index ac7cd63ab..e4a7de71d 100644 --- a/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol +++ b/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol @@ -36,7 +36,7 @@ contract TestLibAssetProxyDecoder is /// @dev Decodes ERC721 Asset Proxy data function publicDecodeERC721Data(bytes memory proxyData) - internal + public pure returns ( uint8, -- cgit v1.2.3 From d9f9895b2bcd3cde09febbe0e1af31be5ddc80e2 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 24 May 2018 16:44:37 -0700 Subject: Test for onReceived erc721 callback --- packages/contracts/src/utils/artifacts.ts | 2 ++ packages/contracts/src/utils/types.ts | 1 + 2 files changed, 3 insertions(+) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts index 44de43a95..a1c8483d8 100644 --- a/packages/contracts/src/utils/artifacts.ts +++ b/packages/contracts/src/utils/artifacts.ts @@ -2,6 +2,7 @@ import { ContractArtifact } from '@0xproject/sol-compiler'; import * as AssetProxyOwner from '../artifacts/AssetProxyOwner.json'; import * as DummyERC20Token from '../artifacts/DummyERC20Token.json'; +import * as DummyERC721Receiver from '../artifacts/DummyERC721Receiver.json'; import * as DummyERC721Token from '../artifacts/DummyERC721Token.json'; import * as ERC20Proxy from '../artifacts/ERC20Proxy.json'; import * as ERC721Proxy from '../artifacts/ERC721Proxy.json'; @@ -23,6 +24,7 @@ import * as ZRX from '../artifacts/ZRXToken.json'; export const artifacts = { AssetProxyOwner: (AssetProxyOwner as any) as ContractArtifact, DummyERC20Token: (DummyERC20Token as any) as ContractArtifact, + DummyERC721Receiver: (DummyERC721Receiver as any) as ContractArtifact, DummyERC721Token: (DummyERC721Token as any) as ContractArtifact, ERC20Proxy: (ERC20Proxy as any) as ContractArtifact, ERC721Proxy: (ERC721Proxy as any) as ContractArtifact, diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index 70abb2643..cccca5705 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -97,6 +97,7 @@ export enum ContractName { TestSignatureValidator = 'TestSignatureValidator', ERC20Proxy = 'ERC20Proxy', ERC721Proxy = 'ERC721Proxy', + DummyERC721Receiver = 'DummyERC721Receiver', DummyERC721Token = 'DummyERC721Token', TestLibBytes = 'TestLibBytes', Authorizable = 'Authorizable', -- cgit v1.2.3 From 842363200b3b8aded3b03fc8e46a329ff9534e36 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Fri, 25 May 2018 00:31:03 -0700 Subject: Tons of tests around nested byte arrays and ERC721 receiver --- .../current/test/TestLibBytes/TestLibBytes.sol | 25 ++++++++++++++++++++++ .../contracts/current/utils/LibBytes/LibBytes.sol | 17 ++++++--------- 2 files changed, 32 insertions(+), 10 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index 69554605d..0bf11b03b 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -155,6 +155,7 @@ contract TestLibBytes is return b; } +======= /// @dev Reads the first 4 bytes from a byte array of arbitrary length. /// @param b Byte array to read first 4 bytes from. /// @return First 4 bytes of data. @@ -166,4 +167,28 @@ contract TestLibBytes is result = readFirst4(b); return result; } + + function publicReadBytes( + bytes memory b, + uint256 index) + public + pure + returns (bytes memory result) + { + result = readBytes(b, index); + return result; + } + + + function publicWriteBytes( + bytes memory b, + uint256 index, + bytes memory input) + public + pure + returns (bytes memory) + { + writeBytes(b, index, input); + return b; + } } diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index fb8359462..6351f1a46 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -300,24 +300,21 @@ contract LibBytes is returns (bytes memory result) { // Read length of nested bytes - require( - b.length >= index + 32, - GTE_32_LENGTH_REQUIRED - ); uint256 nestedBytesLength = readUint256(b, index); + index += 32; // Assert length of is valid, given // length of nested bytes require( - b.length >= index + 32 + nestedBytesLength, + b.length >= index + nestedBytesLength, GTE_32_LENGTH_REQUIRED ); // Allocate memory and copy value to result result = new bytes(nestedBytesLength); memcpy( - getMemAddress(result) + 32, // +32 skips array length - getMemAddress(b) + index + 32, // +32 skips array length + getMemAddress(result) + 32, // +32 skips array length + getMemAddress(b) + index + 32, nestedBytesLength ); @@ -344,9 +341,9 @@ contract LibBytes is // Copy into memcpy( - getMemAddress(b) + index, - getMemAddress(input), - input.length + 32 /* 32 bytes to store length */ + getMemAddress(b) + index + 32, // +32 to skip length of + getMemAddress(input), // include length of byte array + input.length + 32 // +32 bytes to store length ); } } -- cgit v1.2.3 From d17e031259e11e3e37cbb837245171cbdd4d219b Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Fri, 25 May 2018 17:20:00 -0700 Subject: Fixed up wording in memcpy --- packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol index b07a5da54..215a661e2 100644 --- a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol +++ b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol @@ -82,8 +82,8 @@ contract LibMem { // Example: // Suppose a word of 8 bits has all 1's: [11111111] // Let X = 7 (we want to free the first 7 bits, and retain the last bit) - // Apply a left shift of 1: [11111110] - // Apply a right shift of 1: [01111111] + // Apply a left shift of 7: [10000000] + // Apply a right shift of 7: [00000001] let destShiftFactor := exp(2, mul(8, remainder)) let destWord := mload(add(dest, offset)) let destBytes := div(mul(destWord, destShiftFactor), destShiftFactor) @@ -92,7 +92,7 @@ contract LibMem { // The source bytes run from [0..X-1] and the dest bytes from [X..31]. // Example: // Following the example from above, we have [11111110] - // from the source word and [01111111] from the dest word. + // from the source word and [00000001] from the dest word. // Combine these words using to get [11111111]. let combinedDestWord := or(sourceBytes, destBytes) -- cgit v1.2.3 From f5bc0b205c217eac8abdfee9dcdb3c4d21b5c31e Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Mon, 28 May 2018 12:50:03 +0200 Subject: Generate tests from vectors --- .../current/test/TestLibMem/TestLibMem.sol | 27 +++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol index 0c6f8fbc9..18deede4c 100644 --- a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol +++ b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol @@ -19,12 +19,33 @@ pragma solidity ^0.4.24; import "../../utils/LibMem/LibMem.sol"; -import "../../utils/LibBytes/LibBytes.sol"; contract TestLibMem is - LibMem, - LibBytes + LibMem { + function testMemcpy( + bytes mem, ///< Memory contents we want to apply memcpy to + uint256 dest, + uint256 source, + uint256 length + ) + public // not external, we need input in memory + pure + returns (bytes) + { + // Sanity check. Overflows are not checked. + require(source + length <= mem.length); + require(dest + length <= mem.length); + + // Get pointer to memory contents + uint256 offset = getMemAddress(mem) + 32; + + // Execute memcpy adjusted for memory array location + memcpy(offset + dest, offset + source, length); + + // Return modified memory contents + return mem; + } function test1() external -- cgit v1.2.3 From 76b918d40e3bb485cdd45149888f012d9ec2b67f Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Mon, 28 May 2018 13:07:04 +0200 Subject: Convert Solidity tests to vectors --- .../current/test/TestLibMem/TestLibMem.sol | 203 --------------------- 1 file changed, 203 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol index 18deede4c..64bc182f4 100644 --- a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol +++ b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol @@ -46,207 +46,4 @@ contract TestLibMem is // Return modified memory contents return mem; } - - function test1() - external - { - // Length of array & length to copy - uint256 length = 0; - - // Create source array - bytes memory sourceArray = new bytes(length); - - // Create dest array with same contents as source array - bytes memory destArray = new bytes(length); - memcpy( - getMemAddress(destArray) + 32, // skip copying array length - getMemAddress(sourceArray) + 32, // skip copying array length - length - ); - - // Verify contents of source & dest arrays match - require( - areBytesEqual(sourceArray, destArray), - "Test #1 failed. Array contents are not the same." - ); - } - - function test2() - external - { - // Length of array & length to copy - uint256 length = 1; - - // Create source array - bytes memory sourceArray = new bytes(length); - sourceArray[0] = byte(1); - - // Create dest array with same contents as source array - bytes memory destArray = new bytes(length); - memcpy( - getMemAddress(destArray) + 32, // skip copying array length - getMemAddress(sourceArray) + 32, // skip copying array length - length - ); - - // Verify contents of source & dest arrays match - require( - areBytesEqual(sourceArray, destArray), - "Test #2 failed. Array contents are not the same." - ); - } - - function test3() - external - { - // Length of array & length to copy - uint256 length = 11; - - // Create source array - bytes memory sourceArray = new bytes(length); - for(uint256 i = 0; i < length; ++i) { - sourceArray[i] = byte((i % 0xF) + 1); // [1..f] - } - - // Create dest array with same contents as source array - bytes memory destArray = new bytes(length); - memcpy( - getMemAddress(destArray) + 32, // skip copying array length - getMemAddress(sourceArray) + 32, // skip copying array length - length - ); - - // Verify contents of source & dest arrays match - require( - areBytesEqual(sourceArray, destArray), - "Test #3 failed. Array contents are not the same." - ); - } - - function test4() - external - { - // Length of array & length to copy - uint256 length = 32; - - // Create source array - bytes memory sourceArray = new bytes(length); - for(uint256 i = 0; i < length; ++i) { - sourceArray[i] = byte((i % 0xF) + 1); // [1..f] - } - - // Create dest array with same contents as source array - bytes memory destArray = new bytes(length); - memcpy( - getMemAddress(destArray) + 32, // skip copying array length - getMemAddress(sourceArray) + 32, // skip copying array length - length - ); - - // Verify contents of source & dest arrays match - require( - areBytesEqual(sourceArray, destArray), - "Test #4 failed. Array contents are not the same." - ); - } - - function test5() - external - { - // Length of array & length to copy - uint256 length = 72; - - // Create source array - bytes memory sourceArray = new bytes(length); - for(uint256 i = 0; i < length; ++i) { - sourceArray[i] = byte((i % 0xF) + 1); // [1..f] - } - - // Create dest array with same contents as source array - bytes memory destArray = new bytes(length); - memcpy( - getMemAddress(destArray) + 32, // skip copying array length - getMemAddress(sourceArray) + 32, // skip copying array length - length - ); - - // Verify contents of source & dest arrays match - require( - areBytesEqual(sourceArray, destArray), - "Test #5 failed. Array contents are not the same." - ); - } - - - function test6() - external - { - // Length of arrays - uint256 length1 = 72; - uint256 length2 = 100; - - // The full source array is used for comparisons at the end - bytes memory fullSourceArray = new bytes(length1 + length2); - - // First source array - bytes memory sourceArray1 = new bytes(length1); - for(uint256 i = 0; i < length1; ++i) { - sourceArray1[i] = byte((i % 0xF) + 1); // [1..f] - fullSourceArray[i] = byte((i % 0xF) + 1); // [1..f] - } - - // Second source array - bytes memory sourceArray2 = new bytes(length2); - for(uint256 j = 0; i < length2; ++i) { - sourceArray2[j] = byte((j % 0xF) + 1); // [1..f] - fullSourceArray[length1+j] = byte((j % 0xF) + 1); // [1..f] - } - - // Create dest array with same contents as source arrays - bytes memory destArray = new bytes(length1 + length2); - memcpy( - getMemAddress(destArray) + 32, // skip copying array length - getMemAddress(sourceArray1) + 32, // skip copying array length - length1 - ); - memcpy( - getMemAddress(destArray) + 32 + length1, // skip copying array length + sourceArray1 bytes - getMemAddress(sourceArray2) + 32, // skip copying array length - length2 - ); - - // Verify contents of source & dest arrays match - require( - areBytesEqual(fullSourceArray, destArray), - "Test #6 failed. Array contents are not the same." - ); - } - - function test7() - external - { - // Length of array & length to copy - uint256 length = 72; - - // Create source array - bytes memory sourceArray = new bytes(length); - for(uint256 i = 0; i < length; ++i) { - sourceArray[i] = byte((i % 0xF) + 1); // [1..f] - } - - // Create dest array with same contents as source array - bytes memory destArray = new bytes(length); - memcpy( - getMemAddress(destArray) + 32, // skip copying array length - getMemAddress(sourceArray) + 32, // skip copying array length - length - 8 // Copy all but last byte. - ); - - // Verify contents of source & dest arrays match - // We expect this to fail - require( - areBytesEqual(sourceArray, destArray), - "Test #7 failed. Array contents are not the same. This is expected." - ); - } } -- cgit v1.2.3 From 069b89b2084a65e6846a0722a9883af1104feb08 Mon Sep 17 00:00:00 2001 From: Remco Bloemen Date: Mon, 28 May 2018 15:24:09 +0200 Subject: Implement memcpy using masking and end-aligned words --- .../src/contracts/current/utils/LibMem/LibMem.sol | 144 ++++++++++++--------- 1 file changed, 85 insertions(+), 59 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol index 215a661e2..f7ff4ca59 100644 --- a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol +++ b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol @@ -19,7 +19,7 @@ pragma solidity ^0.4.24; contract LibMem { - + function getMemAddress(bytes memory input) internal pure @@ -30,9 +30,11 @@ contract LibMem { } return address_; } - - /// @dev Writes a uint256 into a specific position in a byte array. - /// @param dest memory adress to copy bytes to + + /// @dev Copies `length` bytes from memory location `source` to `dest`. + /// @param dest memory address to copy bytes to + /// @param source memory address to copy bytes from + /// @param length number of bytes to copy function memcpy( uint256 dest, uint256 source, @@ -41,63 +43,87 @@ contract LibMem { internal pure { - // Base cases - if(length == 0) return; - if(source == dest) return; - - // Copy bytes from source to dest - assembly { - // Compute number of complete words to copy + remaining bytes - let lenFullWords := div(add(length, 0x1F), 0x20) - let remainder := mod(length, 0x20) - if gt(remainder, 0) { - lenFullWords := sub(lenFullWords, 1) + if (length < 32) { + // Handle a partial word by reading destination and masking + // off the bits we are interested in. + // This correctly handles overlap, zero lengths and source == dest + assembly { + let mask := sub(exp(256, sub(32, length)), 1) + let s := and(mload(source), not(mask)) + let d := and(mload(dest), mask) + mstore(dest, or(s, d)) } - - // Copy full words from source to dest - let offset := 0 - let maxOffset := mul(0x20, lenFullWords) - for {offset := 0} lt(offset, maxOffset) {offset := add(offset, 0x20)} { - mstore(add(dest, offset), mload(add(source, offset))) + } else { + // Skip the O(length) loop when source == dest. + if (source == dest) { + return; } - - // Copy remaining bytes - if gt(remainder, 0) { - // Read a full word from source, containing X bytes to copy to dest. - // We only want to keep the X bytes, zeroing out the remaining bytes. - // We accomplish this by a right shift followed by a left shift. - // Example: - // Suppose a word of 8 bits has all 1's: [11111111] - // Let X = 7 (we want to copy the first 7 bits) - // Apply a right shift of 1: [01111111] - // Apply a left shift of 1: [11111110] - let sourceShiftFactor := exp(2, mul(8, sub(0x20, remainder))) - let sourceWord := mload(add(source, offset)) - let sourceBytes := mul(div(sourceWord, sourceShiftFactor), sourceShiftFactor) - - // Read a full word from dest, containing (32-X) bytes to retain. - // We need to zero out the remaining bytes to be overwritten by source, - // while retaining the (32-X) bytes we don't want to overwrite. - // We accomplish this by a left shift followed by a right shift. - // Example: - // Suppose a word of 8 bits has all 1's: [11111111] - // Let X = 7 (we want to free the first 7 bits, and retain the last bit) - // Apply a left shift of 7: [10000000] - // Apply a right shift of 7: [00000001] - let destShiftFactor := exp(2, mul(8, remainder)) - let destWord := mload(add(dest, offset)) - let destBytes := div(mul(destWord, destShiftFactor), destShiftFactor) - - // Combine the source and dest bytes. There should be no overlap: - // The source bytes run from [0..X-1] and the dest bytes from [X..31]. - // Example: - // Following the example from above, we have [11111110] - // from the source word and [00000001] from the dest word. - // Combine these words using to get [11111111]. - let combinedDestWord := or(sourceBytes, destBytes) - - // Store the combined word into dest - mstore(add(dest, offset), combinedDestWord) + + // For large copies we copy whole words at a time. The final + // word is aligned to the end of the range (instead of after the + // previous) to handle partial words. So a copy will look like this: + // + // #### + // #### + // #### + // #### + // + // We handle overlap in the source and destination range by + // changing the copying direction. This prevents us from + // overwriting parts of source that we still need to copy. + // + // This correctly handles source == dest + // + if (source > dest) { + assembly { + // We subtract 32 from `send` and `dend` because it + // is easier to compare with in the loop, and these + // are also the addresses we need for copying the + // last bytes. + length := sub(length, 32) + let send := add(source, length) + let dend := add(dest, length) + + // Remember the last 32 bytes of source + // This needs to be done here and not after the loop + // because we may have overwritten the last bytes in + // source already due to overlap. + let last := mload(send) + + // Copy whole words front to back + for {} lt(source, send) {} { + mstore(dest, mload(source)) + source := add(source, 32) + dest := add(dest, 32) + } + + // Write the last 32 bytes + mstore(dend, last) + } + } else { + assembly { + // We subtract 32 from `send` and `dend` because those + // are the starting points when copying a word at the end. + length := sub(length, 32) + let send := add(source, length) + let dend := add(dest, length) + + // Remember the first 32 bytes of source + // This needs to be done here and not after the loop + // because we may have overwritten the first bytes in + // source already due to overlap. + let first := mload(source) + + // Copy whole words back to front + for {} lt(source, send) {} { + mstore(dend, mload(send)) + send := sub(send, 32) + dend := sub(dend, 32) + } + + // Write the first 32 bytes + mstore(dest, first) + } } } } -- cgit v1.2.3 From 5db15ca54cd7e1fd90bf318d2975750dcd31cddc Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 31 May 2018 13:44:14 -0700 Subject: proxyData -> assetData --- .../LibAssetProxyDecoder/LibAssetProxyDecoder.sol | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol b/packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol index ba53f2769..ec27502a8 100644 --- a/packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol +++ b/packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol @@ -29,7 +29,7 @@ contract LibAssetProxyDecoder is string constant INVALID_ERC721_METADATA_LENGTH = "Metadata must have a length of at least 53."; /// @dev Decodes ERC721 Asset Proxy data - function decodeERC20Data(bytes memory proxyData) + function decodeERC20Data(bytes memory assetData) internal pure returns ( @@ -38,17 +38,17 @@ contract LibAssetProxyDecoder is ) { require( - proxyData.length == 21, + assetData.length == 21, INVALID_ERC20_METADATA_LENGTH ); - proxyId = uint8(proxyData[0]); - token = readAddress(proxyData, 1); + proxyId = uint8(assetData[0]); + token = readAddress(assetData, 1); return (proxyId, token); } /// @dev Decodes ERC721 Asset Proxy data - function decodeERC721Data(bytes memory proxyData) + function decodeERC721Data(bytes memory assetData) internal pure returns ( @@ -59,14 +59,14 @@ contract LibAssetProxyDecoder is ) { require( - proxyData.length >= 53, + assetData.length >= 53, INVALID_ERC721_METADATA_LENGTH ); - proxyId = uint8(proxyData[0]); - token = readAddress(proxyData, 1); - tokenId = readUint256(proxyData, 21); - if (proxyData.length > 53) { - data = readBytes(proxyData, 53); + proxyId = uint8(assetData[0]); + token = readAddress(assetData, 1); + tokenId = readUint256(assetData, 21); + if (assetData.length > 53) { + data = readBytes(assetData, 53); } return (proxyId, token, tokenId, data); -- cgit v1.2.3 From e042e0ad32cd2ac9e707cb8e52961957f58314ce Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 31 May 2018 14:06:47 -0700 Subject: Converged on naming scheme for asset data: renamed all instances of assetMetadata, proxyData, proxyMetadata to assetData --- .../current/protocol/AssetProxy/ERC20Proxy.sol | 6 ++-- .../current/protocol/AssetProxy/ERC721Proxy.sol | 6 ++-- .../protocol/AssetProxy/MixinAssetProxy.sol | 14 +++++----- .../protocol/AssetProxy/interfaces/IAssetProxy.sol | 8 +++--- .../protocol/AssetProxy/mixins/MAssetProxy.sol | 4 +-- .../current/protocol/Exchange/Exchange.sol | 4 +-- .../Exchange/MixinAssetProxyDispatcher.sol | 10 +++---- .../current/protocol/Exchange/MixinSettlement.sol | 10 +++---- .../protocol/Exchange/MixinWrapperFunctions.sol | 32 +++++++++++----------- .../Exchange/mixins/MAssetProxyDispatcher.sol | 4 +-- .../TestAssetProxyDispatcher.sol | 4 +-- .../TestLibAssetProxyDecoder.sol | 8 +++--- packages/contracts/src/utils/match_order_tester.ts | 24 ++++++++-------- 13 files changed, 67 insertions(+), 67 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index 017f94b1a..5b4367fd9 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -36,12 +36,12 @@ contract ERC20Proxy is uint8 constant PROXY_ID = 1; /// @dev Internal version of `transferFrom`. - /// @param proxyData Encoded byte array. + /// @param assetData Encoded byte array. /// @param from Address to transfer asset from. /// @param to Address to transfer asset to. /// @param amount Amount of asset to transfer. function transferFromInternal( - bytes memory proxyData, + bytes memory assetData, address from, address to, uint256 amount @@ -52,7 +52,7 @@ contract ERC20Proxy is ( uint8 proxyId, address token - ) = decodeERC20Data(proxyData); + ) = decodeERC20Data(assetData); // Data must be intended for this proxy. uint256 length = assetMetadata.length; diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index f35e48eee..e2c445463 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -38,12 +38,12 @@ contract ERC721Proxy is string constant PROXY_ID_MISMATCH = "Proxy id in metadata does not match this proxy id."; /// @dev Internal version of `transferFrom`. - /// @param proxyData Encoded byte array. + /// @param assetData Encoded byte array. /// @param from Address to transfer asset from. /// @param to Address to transfer asset to. /// @param amount Amount of asset to transfer. function transferFromInternal( - bytes memory proxyData, + bytes memory assetData, address from, address to, uint256 amount @@ -56,7 +56,7 @@ contract ERC721Proxy is address token, uint256 tokenId, bytes memory data - ) = decodeERC721Data(proxyData); + ) = decodeERC721Data(assetData); // Data must be intended for this proxy. uint256 length = assetMetadata.length; diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol index 5fa33cbef..9032658e7 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAssetProxy.sol @@ -28,12 +28,12 @@ contract MixinAssetProxy is { /// @dev Transfers assets. Either succeeds or throws. - /// @param assetMetadata Encoded byte array. + /// @param assetData Encoded byte array. /// @param from Address to transfer asset from. /// @param to Address to transfer asset to. /// @param amount Amount of asset to transfer. function transferFrom( - bytes assetMetadata, + bytes assetData, address from, address to, uint256 amount @@ -42,7 +42,7 @@ contract MixinAssetProxy is onlyAuthorized { transferFromInternal( - assetMetadata, + assetData, from, to, amount @@ -50,12 +50,12 @@ contract MixinAssetProxy is } /// @dev Makes multiple transfers of assets. Either succeeds or throws. - /// @param assetMetadata Array of byte arrays encoded for the respective asset proxy. + /// @param assetData Array of byte arrays encoded for the respective asset proxy. /// @param from Array of addresses to transfer assets from. /// @param to Array of addresses to transfer assets to. /// @param amounts Array of amounts of assets to transfer. function batchTransferFrom( - bytes[] memory assetMetadata, + bytes[] memory assetData, address[] memory from, address[] memory to, uint256[] memory amounts @@ -63,9 +63,9 @@ contract MixinAssetProxy is public onlyAuthorized { - for (uint256 i = 0; i < assetMetadata.length; i++) { + for (uint256 i = 0; i < assetData.length; i++) { transferFromInternal( - assetMetadata[i], + assetData[i], from[i], to[i], amounts[i] diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol index 7e1848889..22f43b12f 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol @@ -26,12 +26,12 @@ contract IAssetProxy is { /// @dev Transfers assets. Either succeeds or throws. - /// @param assetMetadata Byte array encoded for the respective asset proxy. + /// @param assetData Byte array encoded for the respective asset proxy. /// @param from Address to transfer asset from. /// @param to Address to transfer asset to. /// @param amount Amount of asset to transfer. function transferFrom( - bytes assetMetadata, + bytes assetData, address from, address to, uint256 amount @@ -39,12 +39,12 @@ contract IAssetProxy is external; /// @dev Makes multiple transfers of assets. Either succeeds or throws. - /// @param assetMetadata Array of byte arrays encoded for the respective asset proxy. + /// @param assetData Array of byte arrays encoded for the respective asset proxy. /// @param from Array of addresses to transfer assets from. /// @param to Array of addresses to transfer assets to. /// @param amounts Array of amounts of assets to transfer. function batchTransferFrom( - bytes[] memory assetMetadata, + bytes[] memory assetData, address[] memory from, address[] memory to, uint256[] memory amounts diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol index de9d65a53..a52cb56f9 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/mixins/MAssetProxy.sol @@ -26,12 +26,12 @@ contract MAssetProxy is { /// @dev Internal version of `transferFrom`. - /// @param assetMetadata Encoded byte array. + /// @param assetData Encoded byte array. /// @param from Address to transfer asset from. /// @param to Address to transfer asset to. /// @param amount Amount of asset to transfer. function transferFromInternal( - bytes memory assetMetadata, + bytes memory assetData, address from, address to, uint256 amount diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol b/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol index b7b308069..51f99bafa 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol @@ -40,11 +40,11 @@ contract Exchange is string constant public VERSION = "2.0.1-alpha"; // Mixins are instantiated in the order they are inherited - constructor (bytes memory _zrxProxyData) + constructor (bytes memory _zrxAssetData) public MixinExchangeCore() MixinMatchOrders() - MixinSettlement(_zrxProxyData) + MixinSettlement(_zrxAssetData) MixinSignatureValidator() MixinTransactions() MixinAssetProxyDispatcher() diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol index 8f9342739..e77d81c06 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol @@ -80,12 +80,12 @@ contract MixinAssetProxyDispatcher is } /// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws. - /// @param assetMetadata Byte array encoded for the respective asset proxy. + /// @param assetData Byte array encoded for the respective asset proxy. /// @param from Address to transfer token from. /// @param to Address to transfer token to. /// @param amount Amount of token to transfer. function dispatchTransferFrom( - bytes memory assetMetadata, + bytes memory assetData, address from, address to, uint256 amount @@ -96,16 +96,16 @@ contract MixinAssetProxyDispatcher is if (amount > 0) { // Lookup asset proxy - uint256 length = assetMetadata.length; + uint256 length = assetData.length; require( length > 0, LENGTH_GREATER_THAN_0_REQUIRED ); - uint8 assetProxyId = uint8(assetMetadata[length - 1]); + uint8 assetProxyId = uint8(assetData[length - 1]); IAssetProxy assetProxy = assetProxies[assetProxyId]; // transferFrom will either succeed or throw. - assetProxy.transferFrom(assetMetadata, from, to, amount); + assetProxy.transferFrom(assetData, from, to, amount); } } } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol index 646d3ed58..83e9dfdf4 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol @@ -40,7 +40,7 @@ contract MixinSettlement is bytes internal ZRX_PROXY_DATA; /// @dev Gets the ZRX metadata used for fee transfers. - function zrxProxyData() + function zrxAssetData() external view returns (bytes memory) @@ -48,13 +48,13 @@ contract MixinSettlement is return ZRX_PROXY_DATA; } - /// TODO: _zrxProxyData should be a constant in production. + /// TODO: _zrxAssetData should be a constant in production. /// @dev Constructor sets the metadata that will be used for paying ZRX fees. - /// @param _zrxProxyData Byte array containing ERC20 proxy id concatenated with address of ZRX. - constructor (bytes memory _zrxProxyData) + /// @param _zrxAssetData Byte array containing ERC20 proxy id concatenated with address of ZRX. + constructor (bytes memory _zrxAssetData) public { - ZRX_PROXY_DATA = _zrxProxyData; + ZRX_PROXY_DATA = _zrxAssetData; } /// @dev Settles an order by transferring assets between counterparties. diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index 0ad0710ce..0d9888703 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -91,12 +91,12 @@ contract MixinWrapperFunctions is // | | 0x0E0 | | 8. takerFeeAmount | // | | 0x100 | | 9. expirationTimeSeconds | // | | 0x120 | | 10. salt | - // | | 0x140 | | 11. Offset to makerAssetProxyMetadata (*) | - // | | 0x160 | | 12. Offset to takerAssetProxyMetadata (*) | - // | | 0x180 | 32 | makerAssetProxyMetadata Length | - // | | 0x1A0 | ** | makerAssetProxyMetadata Contents | - // | | 0x1C0 | 32 | takerAssetProxyMetadata Length | - // | | 0x1E0 | ** | takerAssetProxyMetadata Contents | + // | | 0x140 | | 11. Offset to makerAssetData (*) | + // | | 0x160 | | 12. Offset to takerAssetData (*) | + // | | 0x180 | 32 | makerAssetData Length | + // | | 0x1A0 | ** | makerAssetData Contents | + // | | 0x1C0 | 32 | takerAssetData Length | + // | | 0x1E0 | ** | takerAssetData Contents | // | | 0x200 | 32 | signature Length | // | | 0x220 | ** | signature Contents | @@ -163,43 +163,43 @@ contract MixinWrapperFunctions is mstore(add(dataAreaEnd, 0xE0), mload(add(sourceOffset, 0xE0))) // takerFeeAmount mstore(add(dataAreaEnd, 0x100), mload(add(sourceOffset, 0x100))) // expirationTimeSeconds mstore(add(dataAreaEnd, 0x120), mload(add(sourceOffset, 0x120))) // salt - mstore(add(dataAreaEnd, 0x140), mload(add(sourceOffset, 0x140))) // Offset to makerAssetProxyMetadata - mstore(add(dataAreaEnd, 0x160), mload(add(sourceOffset, 0x160))) // Offset to takerAssetProxyMetadata + mstore(add(dataAreaEnd, 0x140), mload(add(sourceOffset, 0x140))) // Offset to makerAssetData + mstore(add(dataAreaEnd, 0x160), mload(add(sourceOffset, 0x160))) // Offset to takerAssetData dataAreaEnd := add(dataAreaEnd, 0x180) sourceOffset := add(sourceOffset, 0x180) - // Write offset to + // Write offset to mstore(add(dataAreaStart, mul(10, 0x20)), sub(dataAreaEnd, dataAreaStart)) - // Calculate length of + // Calculate length of arrayLenBytes := mload(sourceOffset) sourceOffset := add(sourceOffset, 0x20) arrayLenWords := div(add(arrayLenBytes, 0x1F), 0x20) - // Write length of + // Write length of mstore(dataAreaEnd, arrayLenBytes) dataAreaEnd := add(dataAreaEnd, 0x20) - // Write contents of + // Write contents of for {let i := 0} lt(i, arrayLenWords) {i := add(i, 1)} { mstore(dataAreaEnd, mload(sourceOffset)) dataAreaEnd := add(dataAreaEnd, 0x20) sourceOffset := add(sourceOffset, 0x20) } - // Write offset to + // Write offset to mstore(add(dataAreaStart, mul(11, 0x20)), sub(dataAreaEnd, dataAreaStart)) - // Calculate length of + // Calculate length of arrayLenBytes := mload(sourceOffset) sourceOffset := add(sourceOffset, 0x20) arrayLenWords := div(add(arrayLenBytes, 0x1F), 0x20) - // Write length of + // Write length of mstore(dataAreaEnd, arrayLenBytes) dataAreaEnd := add(dataAreaEnd, 0x20) - // Write contents of + // Write contents of for {let i := 0} lt(i, arrayLenWords) {i := add(i, 1)} { mstore(dataAreaEnd, mload(sourceOffset)) dataAreaEnd := add(dataAreaEnd, 0x20) diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol index 87c5f6361..82eafb529 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol @@ -33,12 +33,12 @@ contract MAssetProxyDispatcher is ); /// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws. - /// @param assetMetadata Byte array encoded for the respective asset proxy. + /// @param assetData Byte array encoded for the respective asset proxy. /// @param from Address to transfer token from. /// @param to Address to transfer token to. /// @param amount Amount of token to transfer. function dispatchTransferFrom( - bytes memory assetMetadata, + bytes memory assetData, address from, address to, uint256 amount diff --git a/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol index 11ca0617d..2ae69e0ef 100644 --- a/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol @@ -23,12 +23,12 @@ import "../../protocol/Exchange/MixinAssetProxyDispatcher.sol"; contract TestAssetProxyDispatcher is MixinAssetProxyDispatcher { function publicDispatchTransferFrom( - bytes memory assetMetadata, + bytes memory assetData, address from, address to, uint256 amount) public { - dispatchTransferFrom(assetMetadata, from, to, amount); + dispatchTransferFrom(assetData, from, to, amount); } } diff --git a/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol b/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol index e4a7de71d..6d2866656 100644 --- a/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol +++ b/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol @@ -26,16 +26,16 @@ contract TestLibAssetProxyDecoder is { /// @dev Decodes ERC721 Asset Proxy data - function publicDecodeERC20Data(bytes memory proxyData) + function publicDecodeERC20Data(bytes memory assetData) public pure returns (uint8, address) { - return decodeERC20Data(proxyData); + return decodeERC20Data(assetData); } /// @dev Decodes ERC721 Asset Proxy data - function publicDecodeERC721Data(bytes memory proxyData) + function publicDecodeERC721Data(bytes memory assetData) public pure returns ( @@ -45,6 +45,6 @@ contract TestLibAssetProxyDecoder is bytes memory ) { - return decodeERC721Data(proxyData); + return decodeERC721Data(assetData); } } diff --git a/packages/contracts/src/utils/match_order_tester.ts b/packages/contracts/src/utils/match_order_tester.ts index f4f7f965b..fbb1b99db 100644 --- a/packages/contracts/src/utils/match_order_tester.ts +++ b/packages/contracts/src/utils/match_order_tester.ts @@ -237,11 +237,11 @@ export class MatchOrderTester { const expectedNewERC20BalancesByOwner = _.cloneDeep(erc20BalancesByOwner); const expectedNewERC721TokenIdsByOwner = _.cloneDeep(erc721TokenIdsByOwner); // Left Maker Asset (Right Taker Asset) - const makerAssetProxyIdLeft = assetProxyUtils.decodeProxyDataId(signedOrderLeft.makerAssetData); + const makerAssetProxyIdLeft = assetProxyUtils.decodeAssetDataId(signedOrderLeft.makerAssetData); if (makerAssetProxyIdLeft === AssetProxyId.ERC20) { // Decode asset data - const erc20ProxyData = assetProxyUtils.decodeERC20ProxyData(signedOrderLeft.makerAssetData); - const makerAssetAddressLeft = erc20ProxyData.tokenAddress; + const erc20AssetData = assetProxyUtils.decodeERC20AssetData(signedOrderLeft.makerAssetData); + const makerAssetAddressLeft = erc20AssetData.tokenAddress; const takerAssetAddressRight = makerAssetAddressLeft; // Left Maker expectedNewERC20BalancesByOwner[makerAddressLeft][makerAssetAddressLeft] = expectedNewERC20BalancesByOwner[ @@ -259,9 +259,9 @@ export class MatchOrderTester { ][makerAssetAddressLeft].add(expectedTransferAmounts.amountReceivedByTaker); } else if (makerAssetProxyIdLeft === AssetProxyId.ERC721) { // Decode asset data - const erc721ProxyData = assetProxyUtils.decodeERC721ProxyData(signedOrderLeft.makerAssetData); - const makerAssetAddressLeft = erc721ProxyData.tokenAddress; - const makerAssetIdLeft = erc721ProxyData.tokenId; + const erc721AssetData = assetProxyUtils.decodeERC721AssetData(signedOrderLeft.makerAssetData); + const makerAssetAddressLeft = erc721AssetData.tokenAddress; + const makerAssetIdLeft = erc721AssetData.tokenId; const takerAssetAddressRight = makerAssetAddressLeft; const takerAssetIdRight = makerAssetIdLeft; // Left Maker @@ -272,11 +272,11 @@ export class MatchOrderTester { } // Left Taker Asset (Right Maker Asset) // Note: This exchange is only between the order makers: the Taker does not receive any of the left taker asset. - const takerAssetProxyIdLeft = assetProxyUtils.decodeProxyDataId(signedOrderLeft.takerAssetData); + const takerAssetProxyIdLeft = assetProxyUtils.decodeAssetDataId(signedOrderLeft.takerAssetData); if (takerAssetProxyIdLeft === AssetProxyId.ERC20) { // Decode asset data - const erc20ProxyData = assetProxyUtils.decodeERC20ProxyData(signedOrderLeft.takerAssetData); - const takerAssetAddressLeft = erc20ProxyData.tokenAddress; + const erc20AssetData = assetProxyUtils.decodeERC20AssetData(signedOrderLeft.takerAssetData); + const takerAssetAddressLeft = erc20AssetData.tokenAddress; const makerAssetAddressRight = takerAssetAddressLeft; // Left Maker expectedNewERC20BalancesByOwner[makerAddressLeft][takerAssetAddressLeft] = expectedNewERC20BalancesByOwner[ @@ -290,9 +290,9 @@ export class MatchOrderTester { ); } else if (takerAssetProxyIdLeft === AssetProxyId.ERC721) { // Decode asset data - const erc721ProxyData = assetProxyUtils.decodeERC721ProxyData(signedOrderRight.makerAssetData); - const makerAssetAddressRight = erc721ProxyData.tokenAddress; - const makerAssetIdRight = erc721ProxyData.tokenId; + const erc721AssetData = assetProxyUtils.decodeERC721AssetData(signedOrderRight.makerAssetData); + const makerAssetAddressRight = erc721AssetData.tokenAddress; + const makerAssetIdRight = erc721AssetData.tokenId; const takerAssetAddressLeft = makerAssetAddressRight; const takerAssetIdLeft = makerAssetIdRight; // Right Maker -- cgit v1.2.3 From 249a1e6d8d129d6b067a4ddadfb4b94733ddfc07 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 31 May 2018 16:31:00 -0700 Subject: Removed the LibAssetProxyDecoder. Merged decode functions into the proxies. This way they can still be used by the forwarding contract. TestAssetDataDecoders inherits them in the same way the forwarding contract would --- .../current/protocol/AssetProxy/ERC20Proxy.sol | 24 ++++++- .../current/protocol/AssetProxy/ERC721Proxy.sol | 29 ++++++++- .../TestAssetDataDecoders.sol | 52 +++++++++++++++ .../TestLibAssetProxyDecoder.sol | 50 --------------- .../LibAssetProxyDecoder/LibAssetProxyDecoder.sol | 74 ---------------------- packages/contracts/src/utils/artifacts.ts | 4 +- packages/contracts/src/utils/types.ts | 2 +- 7 files changed, 102 insertions(+), 133 deletions(-) create mode 100644 packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol delete mode 100644 packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol delete mode 100644 packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index 5b4367fd9..11383adaf 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -20,14 +20,13 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; import "../../utils/LibBytes/LibBytes.sol"; -import "../../utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol"; +import "../../tokens/ERC20Token/IERC20Token.sol"; import "./MixinAssetProxy.sol"; import "./MixinAuthorizable.sol"; import "../../tokens/ERC20Token/IERC20Token.sol"; contract ERC20Proxy is LibBytes, - LibAssetProxyDecoder, MixinAssetProxy, MixinAuthorizable { @@ -52,7 +51,7 @@ contract ERC20Proxy is ( uint8 proxyId, address token - ) = decodeERC20Data(assetData); + ) = decodeERC20AssetData(assetData); // Data must be intended for this proxy. uint256 length = assetMetadata.length; @@ -79,4 +78,23 @@ contract ERC20Proxy is { return PROXY_ID; } + + /// @dev Decodes ERC20 Asset Proxy data + function decodeERC20AssetData(bytes memory assetData) + internal + pure + returns ( + uint8 proxyId, + address token + ) + { + require( + assetData.length == 21, + INVALID_ASSET_DATA_LENGTH + ); + proxyId = uint8(assetData[0]); + token = readAddress(assetData, 1); + + return (proxyId, token); + } } diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index e2c445463..eb23736a0 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -20,14 +20,12 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; import "../../utils/LibBytes/LibBytes.sol"; -import "../../utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol"; import "./MixinAssetProxy.sol"; import "./MixinAuthorizable.sol"; import "../../tokens/ERC721Token/ERC721Token.sol"; contract ERC721Proxy is LibBytes, - LibAssetProxyDecoder, MixinAssetProxy, MixinAuthorizable { @@ -56,7 +54,7 @@ contract ERC721Proxy is address token, uint256 tokenId, bytes memory data - ) = decodeERC721Data(assetData); + ) = decodeERC721AssetData(assetData); // Data must be intended for this proxy. uint256 length = assetMetadata.length; @@ -92,4 +90,29 @@ contract ERC721Proxy is { return PROXY_ID; } + + /// @dev Decodes ERC721 Asset Proxy data + function decodeERC721AssetData(bytes memory assetData) + internal + pure + returns ( + uint8 proxyId, + address token, + uint256 tokenId, + bytes memory data + ) + { + require( + assetData.length >= 53, + INVALID_ASSET_DATA_LENGTH + ); + proxyId = uint8(assetData[0]); + token = readAddress(assetData, 1); + tokenId = readUint256(assetData, 21); + if (assetData.length > 53) { + data = readBytes(assetData, 53); + } + + return (proxyId, token, tokenId, data); + } } diff --git a/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol b/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol new file mode 100644 index 000000000..45787d88b --- /dev/null +++ b/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol @@ -0,0 +1,52 @@ +/* + + Copyright 2018 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.24; +pragma experimental ABIEncoderV2; + +import "../../protocol/AssetProxy/ERC20Proxy.sol"; +import "../../protocol/AssetProxy/ERC721Proxy.sol"; + +contract TestAssetDataDecoders is + ERC20Proxy, + ERC721Proxy +{ + + /// @dev Decodes ERC721 Asset Proxy data + function publicDecodeERC20Data(bytes memory assetData) + public + pure + returns (uint8, address) + { + return ERC20Proxy.decodeERC20AssetData(assetData); + } + + /// @dev Decodes ERC721 Asset Proxy data + function publicDecodeERC721Data(bytes memory assetData) + public + pure + returns ( + uint8, + address, + uint256, + bytes memory + ) + { + return ERC721Proxy.decodeERC721AssetData(assetData); + } +} diff --git a/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol b/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol deleted file mode 100644 index 6d2866656..000000000 --- a/packages/contracts/src/contracts/current/test/TestLibAssetProxyDecoder/TestLibAssetProxyDecoder.sol +++ /dev/null @@ -1,50 +0,0 @@ -/* - - Copyright 2018 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.24; -pragma experimental ABIEncoderV2; - -import "../../utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol"; - -contract TestLibAssetProxyDecoder is - LibAssetProxyDecoder -{ - - /// @dev Decodes ERC721 Asset Proxy data - function publicDecodeERC20Data(bytes memory assetData) - public - pure - returns (uint8, address) - { - return decodeERC20Data(assetData); - } - - /// @dev Decodes ERC721 Asset Proxy data - function publicDecodeERC721Data(bytes memory assetData) - public - pure - returns ( - uint8, - address, - uint256, - bytes memory - ) - { - return decodeERC721Data(assetData); - } -} diff --git a/packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol b/packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol deleted file mode 100644 index ec27502a8..000000000 --- a/packages/contracts/src/contracts/current/utils/LibAssetProxyDecoder/LibAssetProxyDecoder.sol +++ /dev/null @@ -1,74 +0,0 @@ -/* - - Copyright 2018 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.24; -pragma experimental ABIEncoderV2; - -import "../LibBytes/LibBytes.sol"; - -contract LibAssetProxyDecoder is - LibBytes -{ - - string constant INVALID_ERC20_METADATA_LENGTH = "Metadata must have a length of 21."; - string constant INVALID_ERC721_METADATA_LENGTH = "Metadata must have a length of at least 53."; - - /// @dev Decodes ERC721 Asset Proxy data - function decodeERC20Data(bytes memory assetData) - internal - pure - returns ( - uint8 proxyId, - address token - ) - { - require( - assetData.length == 21, - INVALID_ERC20_METADATA_LENGTH - ); - proxyId = uint8(assetData[0]); - token = readAddress(assetData, 1); - - return (proxyId, token); - } - - /// @dev Decodes ERC721 Asset Proxy data - function decodeERC721Data(bytes memory assetData) - internal - pure - returns ( - uint8 proxyId, - address token, - uint256 tokenId, - bytes memory data - ) - { - require( - assetData.length >= 53, - INVALID_ERC721_METADATA_LENGTH - ); - proxyId = uint8(assetData[0]); - token = readAddress(assetData, 1); - tokenId = readUint256(assetData, 21); - if (assetData.length > 53) { - data = readBytes(assetData, 53); - } - - return (proxyId, token, tokenId, data); - } -} diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts index a1c8483d8..42de7c921 100644 --- a/packages/contracts/src/utils/artifacts.ts +++ b/packages/contracts/src/utils/artifacts.ts @@ -11,7 +11,7 @@ import * as MixinAuthorizable from '../artifacts/MixinAuthorizable.json'; import * as MultiSigWallet from '../artifacts/MultiSigWallet.json'; import * as MultiSigWalletWithTimeLock from '../artifacts/MultiSigWalletWithTimeLock.json'; import * as TestAssetProxyDispatcher from '../artifacts/TestAssetProxyDispatcher.json'; -import * as TestLibAssetProxyDecoder from '../artifacts/TestLibAssetProxyDecoder.json'; +import * as TestAssetDataDecoders from '../artifacts/TestAssetDataDecoders.json'; import * as TestLibBytes from '../artifacts/TestLibBytes.json'; import * as TestLibMem from '../artifacts/TestLibMem.json'; import * as TestLibs from '../artifacts/TestLibs.json'; @@ -34,7 +34,7 @@ export const artifacts = { MultiSigWallet: (MultiSigWallet as any) as ContractArtifact, MultiSigWalletWithTimeLock: (MultiSigWalletWithTimeLock as any) as ContractArtifact, TestAssetProxyDispatcher: (TestAssetProxyDispatcher as any) as ContractArtifact, - TestLibAssetProxyDecoder: (TestLibAssetProxyDecoder as any) as ContractArtifact, + TestAssetDataDecoders: (TestAssetDataDecoders as any) as ContractArtifact, TestLibBytes: (TestLibBytes as any) as ContractArtifact, TestLibMem: (TestLibMem as any) as ContractArtifact, TestLibs: (TestLibs as any) as ContractArtifact, diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index cccca5705..bb8c12088 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -90,7 +90,7 @@ export enum ContractName { AccountLevels = 'AccountLevels', EtherDelta = 'EtherDelta', Arbitrage = 'Arbitrage', - TestLibAssetProxyDecoder = 'TestLibAssetProxyDecoder', + TestAssetDataDecoders = 'TestAssetDataDecoders', TestAssetProxyDispatcher = 'TestAssetProxyDispatcher', TestLibMem = 'TestLibMem', TestLibs = 'TestLibs', -- cgit v1.2.3 From 05f1e9e3b8f5da593d949a1a18abba70568adb9c Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 31 May 2018 16:49:10 -0700 Subject: Resolved edge case in Memcpy where where send would eventually turn "negative" and wrap around. --- .../src/contracts/current/utils/LibMem/LibMem.sol | 28 +++++++++++++--------- 1 file changed, 17 insertions(+), 11 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol index f7ff4ca59..500044500 100644 --- a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol +++ b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol @@ -19,7 +19,7 @@ pragma solidity ^0.4.24; contract LibMem { - + function getMemAddress(bytes memory input) internal pure @@ -30,7 +30,7 @@ contract LibMem { } return address_; } - + /// @dev Copies `length` bytes from memory location `source` to `dest`. /// @param dest memory address to copy bytes to /// @param source memory address to copy bytes from @@ -58,7 +58,7 @@ contract LibMem { if (source == dest) { return; } - + // For large copies we copy whole words at a time. The final // word is aligned to the end of the range (instead of after the // previous) to handle partial words. So a copy will look like this: @@ -76,6 +76,9 @@ contract LibMem { // if (source > dest) { assembly { + // Record the total number of full words to copy + let nwords := div(length, 32) + // We subtract 32 from `send` and `dend` because it // is easier to compare with in the loop, and these // are also the addresses we need for copying the @@ -83,44 +86,47 @@ contract LibMem { length := sub(length, 32) let send := add(source, length) let dend := add(dest, length) - + // Remember the last 32 bytes of source // This needs to be done here and not after the loop // because we may have overwritten the last bytes in // source already due to overlap. let last := mload(send) - + // Copy whole words front to back - for {} lt(source, send) {} { + for {let i := 0} lt(i, nwords) {i := add(i, 1)} { mstore(dest, mload(source)) source := add(source, 32) dest := add(dest, 32) } - + // Write the last 32 bytes mstore(dend, last) } } else { assembly { + // Record the total number of full words to copy + let nwords := div(length, 32) + // We subtract 32 from `send` and `dend` because those // are the starting points when copying a word at the end. length := sub(length, 32) let send := add(source, length) let dend := add(dest, length) - + // Remember the first 32 bytes of source // This needs to be done here and not after the loop // because we may have overwritten the first bytes in // source already due to overlap. let first := mload(source) - + // Copy whole words back to front - for {} lt(source, send) {} { + for {let i := 0} lt(i, nwords) {i := add(i, 1)} { mstore(dend, mload(send)) send := sub(send, 32) dend := sub(dend, 32) } - + // Write the first 32 bytes mstore(dest, first) } -- cgit v1.2.3 From 3c3851c221873baf3b7fec7213324dae0c1c3351 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 31 May 2018 17:49:12 -0700 Subject: Fixed formatting in memory layout --- .../current/protocol/Exchange/MixinWrapperFunctions.sol | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index 0d9888703..f4822e814 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -91,12 +91,12 @@ contract MixinWrapperFunctions is // | | 0x0E0 | | 8. takerFeeAmount | // | | 0x100 | | 9. expirationTimeSeconds | // | | 0x120 | | 10. salt | - // | | 0x140 | | 11. Offset to makerAssetData (*) | - // | | 0x160 | | 12. Offset to takerAssetData (*) | - // | | 0x180 | 32 | makerAssetData Length | - // | | 0x1A0 | ** | makerAssetData Contents | - // | | 0x1C0 | 32 | takerAssetData Length | - // | | 0x1E0 | ** | takerAssetData Contents | + // | | 0x140 | | 11. Offset to makerAssetData (*) | + // | | 0x160 | | 12. Offset to takerAssetData (*) | + // | | 0x180 | 32 | makerAssetData Length | + // | | 0x1A0 | ** | makerAssetData Contents | + // | | 0x1C0 | 32 | takerAssetData Length | + // | | 0x1E0 | ** | takerAssetData Contents | // | | 0x200 | 32 | signature Length | // | | 0x220 | ** | signature Contents | -- cgit v1.2.3 From 8496c1cdd3bc477fcfe584adf8200f4ed35da2b0 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 31 May 2018 18:59:02 -0700 Subject: Call safeTransferFrom only when there is receiver data present --- .../src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index eb23736a0..9dac02d87 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -77,8 +77,13 @@ contract ERC721Proxy is ); // Transfer token. + // Save gas by calling safeTransferFrom only when there is data present. // Either succeeds or throws. - ERC721Token(token).safeTransferFrom(from, to, tokenId, data); + if(data.length > 0) { + ERC721Token(token).safeTransferFrom(from, to, tokenId, data); + } else { + ERC721Token(token).transferFrom(from, to, tokenId); + } } /// @dev Gets the proxy id associated with the proxy address. -- cgit v1.2.3 From f03e5c6bd12c88fffbad324fd7493d3acedea0aa Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Fri, 1 Jun 2018 11:34:45 -0700 Subject: Style audit proxies --- .../current/protocol/AssetProxy/ERC20Proxy.sol | 8 +++- .../current/protocol/AssetProxy/ERC721Proxy.sol | 33 ++++++++++++----- .../TestAssetDataDecoders.sol | 43 +++++++++++++++++----- 3 files changed, 64 insertions(+), 20 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index 11383adaf..54cbeb963 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -79,7 +79,10 @@ contract ERC20Proxy is return PROXY_ID; } - /// @dev Decodes ERC20 Asset Proxy data + /// @dev Decodes ERC20 Asset data. + /// @param assetData Encoded byte array. + /// @return proxyId Intended ERC20 proxy id. + /// @return token ERC20 token address. function decodeERC20AssetData(bytes memory assetData) internal pure @@ -88,10 +91,13 @@ contract ERC20Proxy is address token ) { + // Validate encoded data length require( assetData.length == 21, INVALID_ASSET_DATA_LENGTH ); + + // Decode data proxyId = uint8(assetData[0]); token = readAddress(assetData, 1); diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index 9dac02d87..58c23b03b 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -53,7 +53,7 @@ contract ERC721Proxy is uint8 proxyId, address token, uint256 tokenId, - bytes memory data + bytes memory receiverData ) = decodeERC721AssetData(assetData); // Data must be intended for this proxy. @@ -76,11 +76,10 @@ contract ERC721Proxy is INVALID_AMOUNT ); - // Transfer token. - // Save gas by calling safeTransferFrom only when there is data present. - // Either succeeds or throws. - if(data.length > 0) { - ERC721Token(token).safeTransferFrom(from, to, tokenId, data); + // Transfer token. Saves gas by calling safeTransferFrom only + // when there is receiverData present. Either succeeds or throws. + if(receiverData.length > 0) { + ERC721Token(token).safeTransferFrom(from, to, tokenId, receiverData); } else { ERC721Token(token).transferFrom(from, to, tokenId); } @@ -96,7 +95,13 @@ contract ERC721Proxy is return PROXY_ID; } - /// @dev Decodes ERC721 Asset Proxy data + /// @dev Decodes ERC721 Asset data. + /// @param assetData Encoded byte array. + /// @return proxyId Intended ERC721 proxy id. + /// @return token ERC721 token address. + /// @return tokenId ERC721 token id. + /// @return receiverData Additional data with no specific format, which + /// is passed to the receiving contract's onERC721Received. function decodeERC721AssetData(bytes memory assetData) internal pure @@ -104,20 +109,28 @@ contract ERC721Proxy is uint8 proxyId, address token, uint256 tokenId, - bytes memory data + bytes memory receiverData ) { + // Validate encoded data length require( assetData.length >= 53, INVALID_ASSET_DATA_LENGTH ); + + // Decode asset data proxyId = uint8(assetData[0]); token = readAddress(assetData, 1); tokenId = readUint256(assetData, 21); if (assetData.length > 53) { - data = readBytes(assetData, 53); + receiverData = readBytes(assetData, 53); } - return (proxyId, token, tokenId, data); + return ( + proxyId, + token, + tokenId, + receiverData + ); } } diff --git a/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol b/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol index 45787d88b..2c6a8fdb0 100644 --- a/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol +++ b/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol @@ -27,26 +27,51 @@ contract TestAssetDataDecoders is ERC721Proxy { - /// @dev Decodes ERC721 Asset Proxy data + /// @dev Decodes ERC20 Asset data. + /// @param assetData Encoded byte array. + /// @return proxyId Intended ERC20 proxy id. + /// @return token ERC20 token address. function publicDecodeERC20Data(bytes memory assetData) public pure - returns (uint8, address) + returns ( + uint8 proxyId, + address token + ) { - return ERC20Proxy.decodeERC20AssetData(assetData); + (proxyId, token) = decodeERC20AssetData(assetData); + return (proxyId, token); } - /// @dev Decodes ERC721 Asset Proxy data + /// @dev Decodes ERC721 Asset data. + /// @param assetData Encoded byte array. + /// @return proxyId Intended ERC721 proxy id. + /// @return token ERC721 token address. + /// @return tokenId ERC721 token id. + /// @return receiverData Additional data with no specific format, which + /// is passed to the receiving contract's onERC721Received. function publicDecodeERC721Data(bytes memory assetData) public pure returns ( - uint8, - address, - uint256, - bytes memory + uint8 proxyId, + address token, + uint256 tokenId, + bytes memory receiverData ) { - return ERC721Proxy.decodeERC721AssetData(assetData); + ( + proxyId, + token, + tokenId, + receiverData + ) = decodeERC721AssetData(assetData); + + return ( + proxyId, + token, + tokenId, + receiverData + ); } } -- cgit v1.2.3 From 3ed13150e106c19563c8e9b06621be3d44d66b6c Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Fri, 1 Jun 2018 11:54:20 -0700 Subject: Style audit for proxies + libmem + libbytes --- .../current/protocol/AssetProxy/ERC20Proxy.sol | 2 +- .../current/protocol/AssetProxy/ERC721Proxy.sol | 4 ++-- .../current/test/TestLibBytes/TestLibBytes.sol | 11 +++++++++-- .../contracts/current/test/TestLibMem/TestLibMem.sol | 15 +++++++++++---- .../src/contracts/current/utils/LibBytes/LibBytes.sol | 13 ++++++------- .../src/contracts/current/utils/LibMem/LibMem.sol | 18 +++++++++++------- 6 files changed, 40 insertions(+), 23 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index 54cbeb963..96950f1cd 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -47,7 +47,7 @@ contract ERC20Proxy is ) internal { - // Decode proxy data. + // Decode asset data. ( uint8 proxyId, address token diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index 58c23b03b..102064c15 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -48,7 +48,7 @@ contract ERC721Proxy is ) internal { - // Decode proxy data. + // Decode asset data. ( uint8 proxyId, address token, @@ -118,7 +118,7 @@ contract ERC721Proxy is INVALID_ASSET_DATA_LENGTH ); - // Decode asset data + // Decode asset data. proxyId = uint8(assetData[0]); token = readAddress(assetData, 1); tokenId = readUint256(assetData, 21); diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index 0bf11b03b..f009a6a71 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -155,7 +155,6 @@ contract TestLibBytes is return b; } -======= /// @dev Reads the first 4 bytes from a byte array of arbitrary length. /// @param b Byte array to read first 4 bytes from. /// @return First 4 bytes of data. @@ -168,6 +167,10 @@ contract TestLibBytes is return result; } + /// @dev Reads nested bytes from a specific position. + /// @param b Byte array containing nested bytes. + /// @param index Index of nested bytes. + /// @return result Nested bytes. function publicReadBytes( bytes memory b, uint256 index) @@ -179,7 +182,11 @@ contract TestLibBytes is return result; } - + /// @dev Inserts bytes at a specific position in a byte array. + /// @param b Byte array to insert into. + /// @param index Index in byte array of . + /// @param input bytes to insert. + /// @return b Updated input byte array function publicWriteBytes( bytes memory b, uint256 index, diff --git a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol index 64bc182f4..076bee24c 100644 --- a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol +++ b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol @@ -23,8 +23,15 @@ import "../../utils/LibMem/LibMem.sol"; contract TestLibMem is LibMem { + + /// @dev Copies a block of memory from one location to another. + /// @param mem Memory contents we want to apply memcpy to + /// @param dest Destination offset into . + /// @param source Source offset into . + /// @param length Length of bytes to copy from to + /// @return mem Memory contents after calling memcpy. function testMemcpy( - bytes mem, ///< Memory contents we want to apply memcpy to + bytes mem, uint256 dest, uint256 source, uint256 length @@ -36,13 +43,13 @@ contract TestLibMem is // Sanity check. Overflows are not checked. require(source + length <= mem.length); require(dest + length <= mem.length); - + // Get pointer to memory contents uint256 offset = getMemAddress(mem) + 32; - + // Execute memcpy adjusted for memory array location memcpy(offset + dest, offset + source, length); - + // Return modified memory contents return mem; } diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 6351f1a46..5610c47b3 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -268,7 +268,6 @@ contract LibBytes is writeBytes32(b, index, bytes32(input)); } -======= /// @dev Reads the first 4 bytes from a byte array of arbitrary length. /// @param b Byte array to read first 4 bytes from. /// @return First 4 bytes of data. @@ -287,10 +286,10 @@ contract LibBytes is return result; } - /// @dev Reads a uint256 value from a position in a byte array. - /// @param b Byte array containing a uint256 value. - /// @param index Index in byte array of uint256 value. - /// @return uint256 value from byte array. + /// @dev Reads nested bytes from a specific position. + /// @param b Byte array containing nested bytes. + /// @param index Index of nested bytes. + /// @return result Nested bytes. function readBytes( bytes memory b, uint256 index @@ -321,10 +320,10 @@ contract LibBytes is return result; } - /// @dev Writes a uint256 into a specific position in a byte array. + /// @dev Inserts bytes at a specific position in a byte array. /// @param b Byte array to insert into. /// @param index Index in byte array of . - /// @param input uint256 to put into byte array. + /// @param input bytes to insert. function writeBytes( bytes memory b, uint256 index, diff --git a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol index 500044500..960850725 100644 --- a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol +++ b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol @@ -18,23 +18,27 @@ pragma solidity ^0.4.24; -contract LibMem { +contract LibMem +{ + /// @dev Gets the memory address for a byte array. + /// @param input Byte array to lookup. + /// @return memoryAddress Memory address of byte array. function getMemAddress(bytes memory input) internal pure - returns (uint256 address_) + returns (uint256 memoryAddress) { assembly { - address_ := input + memoryAddress := input } - return address_; + return memoryAddress; } /// @dev Copies `length` bytes from memory location `source` to `dest`. - /// @param dest memory address to copy bytes to - /// @param source memory address to copy bytes from - /// @param length number of bytes to copy + /// @param dest memory address to copy bytes to. + /// @param source memory address to copy bytes from. + /// @param length number of bytes to copy. function memcpy( uint256 dest, uint256 source, -- cgit v1.2.3 From e4e36760952287a84f8991df8589c183036383db Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Fri, 1 Jun 2018 14:17:13 -0700 Subject: Fixed up after rebasing. Contracts build and tests pass --- .../current/protocol/AssetProxy/ERC20Proxy.sol | 7 +++---- .../current/protocol/AssetProxy/ERC721Proxy.sol | 18 ++++++------------ 2 files changed, 9 insertions(+), 16 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index 96950f1cd..4e9ae64f8 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -54,8 +54,6 @@ contract ERC20Proxy is ) = decodeERC20AssetData(assetData); // Data must be intended for this proxy. - uint256 length = assetMetadata.length; - require( proxyId == PROXY_ID, PROXY_ID_MISMATCH @@ -92,14 +90,15 @@ contract ERC20Proxy is ) { // Validate encoded data length + uint256 length = assetData.length; require( assetData.length == 21, INVALID_ASSET_DATA_LENGTH ); // Decode data - proxyId = uint8(assetData[0]); - token = readAddress(assetData, 1); + token = readAddress(assetData, 0); + proxyId = uint8(assetData[length-1]); return (proxyId, token); } diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index 102064c15..f6c3af104 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -56,15 +56,8 @@ contract ERC721Proxy is bytes memory receiverData ) = decodeERC721AssetData(assetData); - // Data must be intended for this proxy. - uint256 length = assetMetadata.length; - require( - length == 53, - LENGTH_53_REQUIRED - ); - - // TODO: Is this too inflexible in the future? + // Data must be intended for this proxy. require( proxyId == PROXY_ID, PROXY_ID_MISMATCH @@ -113,18 +106,19 @@ contract ERC721Proxy is ) { // Validate encoded data length + uint256 length = assetData.length; require( assetData.length >= 53, INVALID_ASSET_DATA_LENGTH ); // Decode asset data. - proxyId = uint8(assetData[0]); - token = readAddress(assetData, 1); - tokenId = readUint256(assetData, 21); + token = readAddress(assetData, 0); + tokenId = readUint256(assetData, 20); if (assetData.length > 53) { - receiverData = readBytes(assetData, 53); + receiverData = readBytes(assetData, 52); } + proxyId = uint8(assetData[length-1]); return ( proxyId, -- cgit v1.2.3 From a1b49d8389844c9b2d62ded91b76a23deb060ab6 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Mon, 4 Jun 2018 19:40:01 -0700 Subject: Fixed after rebase --- .../src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol | 4 ++-- .../src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol | 2 +- .../current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index 4e9ae64f8..c8e8f4587 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -56,7 +56,7 @@ contract ERC20Proxy is // Data must be intended for this proxy. require( proxyId == PROXY_ID, - PROXY_ID_MISMATCH + ASSET_PROXY_ID_MISMATCH ); // Transfer tokens. @@ -93,7 +93,7 @@ contract ERC20Proxy is uint256 length = assetData.length; require( assetData.length == 21, - INVALID_ASSET_DATA_LENGTH + LENGTH_21_REQUIRED ); // Decode data diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index f6c3af104..7de7968cc 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -109,7 +109,7 @@ contract ERC721Proxy is uint256 length = assetData.length; require( assetData.length >= 53, - INVALID_ASSET_DATA_LENGTH + LENGTH_AT_LEAST_53_REQUIRED ); // Decode asset data. diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol index e0c7fc796..80180a0d9 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol @@ -29,9 +29,9 @@ contract LibAssetProxyErrors { /// AssetProxy errors /// string constant ASSET_PROXY_ID_MISMATCH = "ASSET_PROXY_ID_MISMATCH"; // Proxy id in metadata does not match this proxy id. string constant INVALID_AMOUNT = "INVALID_AMOUNT"; // Transfer amount must equal 1. - string constant TRANSFER_FAILED = "TRANSFER_FAILED"; // Transfer failed. + string constant TRANSFER_FAILED = "TRANSFER_FAILED"; // Transfer failed. /// Length validation errors /// string constant LENGTH_21_REQUIRED = "LENGTH_21_REQUIRED"; // Byte array must have a length of 21. - string constant LENGTH_53_REQUIRED = "LENGTH_53_REQUIRED"; // Byte array must have a length of 53. + string constant LENGTH_AT_LEAST_53_REQUIRED = "LENGTH_AT_LEAST_53_REQUIRED"; // Byte array must have a length of at least 53. } -- cgit v1.2.3 From 774d831fae5809408f9ddfcf9393d579416b1bfb Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Mon, 4 Jun 2018 22:34:04 -0700 Subject: Style updates to ERC721 onReceiver --- .../src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol | 1 - .../src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol | 4 +--- .../contracts/src/contracts/current/utils/LibBytes/LibBytes.sol | 9 +++++---- 3 files changed, 6 insertions(+), 8 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index c8e8f4587..50632400e 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -20,7 +20,6 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; import "../../utils/LibBytes/LibBytes.sol"; -import "../../tokens/ERC20Token/IERC20Token.sol"; import "./MixinAssetProxy.sol"; import "./MixinAuthorizable.sol"; import "../../tokens/ERC20Token/IERC20Token.sol"; diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index 7de7968cc..21e5518c6 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -33,8 +33,6 @@ contract ERC721Proxy is // Id of this proxy. uint8 constant PROXY_ID = 2; - string constant PROXY_ID_MISMATCH = "Proxy id in metadata does not match this proxy id."; - /// @dev Internal version of `transferFrom`. /// @param assetData Encoded byte array. /// @param from Address to transfer asset from. @@ -60,7 +58,7 @@ contract ERC721Proxy is // Data must be intended for this proxy. require( proxyId == PROXY_ID, - PROXY_ID_MISMATCH + ASSET_PROXY_ID_MISMATCH ); // There exists only 1 of each token. diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 5610c47b3..8f6647d20 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -332,7 +332,8 @@ contract LibBytes is internal pure { - // Read length of nested bytes + // Assert length of is valid, given + // length of input require( b.length >= index + 32 /* 32 bytes to store length */ + input.length, GTE_32_LENGTH_REQUIRED @@ -340,9 +341,9 @@ contract LibBytes is // Copy into memcpy( - getMemAddress(b) + index + 32, // +32 to skip length of - getMemAddress(input), // include length of byte array - input.length + 32 // +32 bytes to store length + getMemAddress(b) + 32 + index, // +32 to skip length of + getMemAddress(input), // includes length of + input.length + 32 // +32 bytes to store length ); } } -- cgit v1.2.3 From b19276bb0f36059e67bb57b14e9f1e9e0efc17f2 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Tue, 5 Jun 2018 16:44:47 -0700 Subject: Fixed merge error when rebasing wrt length variable in asset data decoders --- .../src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol | 4 ++-- .../src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index 50632400e..dd25bf41a 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -91,13 +91,13 @@ contract ERC20Proxy is // Validate encoded data length uint256 length = assetData.length; require( - assetData.length == 21, + length == 21, LENGTH_21_REQUIRED ); // Decode data token = readAddress(assetData, 0); - proxyId = uint8(assetData[length-1]); + proxyId = uint8(assetData[length - 1]); return (proxyId, token); } diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index 21e5518c6..499d8d96e 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -106,14 +106,14 @@ contract ERC721Proxy is // Validate encoded data length uint256 length = assetData.length; require( - assetData.length >= 53, + length >= 53, LENGTH_AT_LEAST_53_REQUIRED ); // Decode asset data. token = readAddress(assetData, 0); tokenId = readUint256(assetData, 20); - if (assetData.length > 53) { + if (length > 53) { receiverData = readBytes(assetData, 52); } proxyId = uint8(assetData[length-1]); -- cgit v1.2.3 From 37684c6af0d2962f7c7822dd14531787bd7b4212 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Tue, 5 Jun 2018 17:30:43 -0700 Subject: Fixed a styling throughout contracts. Moved closing parenthesis for long list of function parameters to next line. --- .../protocol/Exchange/MixinWrapperFunctions.sol | 27 ++++++++++++++-------- .../Exchange/interfaces/IAssetProxyDispatcher.sol | 3 ++- .../protocol/Exchange/interfaces/ITransactions.sol | 3 ++- .../Exchange/interfaces/IWrapperFunctions.sol | 27 ++++++++++++++-------- .../test/DummyERC20Token/DummyERC20Token.sol | 3 ++- .../DummyERC721Receiver/DummyERC721Receiver.sol | 3 ++- .../test/DummyERC721Token/DummyERC721Token.sol | 3 ++- .../current/test/TestLibBytes/TestLibBytes.sol | 24 ++++++++++++------- 8 files changed, 62 insertions(+), 31 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index f4822e814..e09f80bcc 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -40,7 +40,8 @@ contract MixinWrapperFunctions is function fillOrKillOrder( LibOrder.Order memory order, uint256 takerAssetFillAmount, - bytes memory signature) + bytes memory signature + ) public returns (FillResults memory fillResults) { @@ -65,7 +66,8 @@ contract MixinWrapperFunctions is function fillOrderNoThrow( LibOrder.Order memory order, uint256 takerAssetFillAmount, - bytes memory signature) + bytes memory signature + ) public returns (FillResults memory fillResults) { @@ -264,7 +266,8 @@ contract MixinWrapperFunctions is function batchFillOrders( LibOrder.Order[] memory orders, uint256[] memory takerAssetFillAmounts, - bytes[] memory signatures) + bytes[] memory signatures + ) public { for (uint256 i = 0; i < orders.length; i++) { @@ -283,7 +286,8 @@ contract MixinWrapperFunctions is function batchFillOrKillOrders( LibOrder.Order[] memory orders, uint256[] memory takerAssetFillAmounts, - bytes[] memory signatures) + bytes[] memory signatures + ) public { for (uint256 i = 0; i < orders.length; i++) { @@ -303,7 +307,8 @@ contract MixinWrapperFunctions is function batchFillOrdersNoThrow( LibOrder.Order[] memory orders, uint256[] memory takerAssetFillAmounts, - bytes[] memory signatures) + bytes[] memory signatures + ) public { for (uint256 i = 0; i < orders.length; i++) { @@ -323,7 +328,8 @@ contract MixinWrapperFunctions is function marketSellOrders( LibOrder.Order[] memory orders, uint256 takerAssetFillAmount, - bytes[] memory signatures) + bytes[] memory signatures + ) public returns (FillResults memory totalFillResults) { @@ -366,7 +372,8 @@ contract MixinWrapperFunctions is function marketSellOrdersNoThrow( LibOrder.Order[] memory orders, uint256 takerAssetFillAmount, - bytes[] memory signatures) + bytes[] memory signatures + ) public returns (FillResults memory totalFillResults) { @@ -408,7 +415,8 @@ contract MixinWrapperFunctions is function marketBuyOrders( LibOrder.Order[] memory orders, uint256 makerAssetFillAmount, - bytes[] memory signatures) + bytes[] memory signatures + ) public returns (FillResults memory totalFillResults) { @@ -459,7 +467,8 @@ contract MixinWrapperFunctions is function marketBuyOrdersNoThrow( LibOrder.Order[] memory orders, uint256 makerAssetFillAmount, - bytes[] memory signatures) + bytes[] memory signatures + ) public returns (FillResults memory totalFillResults) { diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IAssetProxyDispatcher.sol index 3ce5ef157..2c331dc34 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IAssetProxyDispatcher.sol @@ -28,7 +28,8 @@ contract IAssetProxyDispatcher { function registerAssetProxy( uint8 assetProxyId, address newAssetProxy, - address oldAssetProxy) + address oldAssetProxy + ) external; /// @dev Gets an asset proxy. diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ITransactions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ITransactions.sol index d973bf001..2f9a5bc7c 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ITransactions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ITransactions.sol @@ -28,6 +28,7 @@ contract ITransactions { uint256 salt, address signer, bytes data, - bytes signature) + bytes signature + ) external; } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWrapperFunctions.sol index 8682b394a..acd4f359c 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IWrapperFunctions.sol @@ -33,7 +33,8 @@ contract IWrapperFunctions is function fillOrKillOrder( LibOrder.Order memory order, uint256 takerAssetFillAmount, - bytes memory signature) + bytes memory signature + ) public returns (LibFillResults.FillResults memory fillResults); @@ -46,7 +47,8 @@ contract IWrapperFunctions is function fillOrderNoThrow( LibOrder.Order memory order, uint256 takerAssetFillAmount, - bytes memory signature) + bytes memory signature + ) public returns (LibFillResults.FillResults memory fillResults); @@ -57,7 +59,8 @@ contract IWrapperFunctions is function batchFillOrders( LibOrder.Order[] memory orders, uint256[] memory takerAssetFillAmounts, - bytes[] memory signatures) + bytes[] memory signatures + ) public; /// @dev Synchronously executes multiple calls of fillOrKill. @@ -67,7 +70,8 @@ contract IWrapperFunctions is function batchFillOrKillOrders( LibOrder.Order[] memory orders, uint256[] memory takerAssetFillAmounts, - bytes[] memory signatures) + bytes[] memory signatures + ) public; /// @dev Fills an order with specified parameters and ECDSA signature. @@ -78,7 +82,8 @@ contract IWrapperFunctions is function batchFillOrdersNoThrow( LibOrder.Order[] memory orders, uint256[] memory takerAssetFillAmounts, - bytes[] memory signatures) + bytes[] memory signatures + ) public; /// @dev Synchronously executes multiple calls of fillOrder until total amount of takerAsset is sold by taker. @@ -89,7 +94,8 @@ contract IWrapperFunctions is function marketSellOrders( LibOrder.Order[] memory orders, uint256 takerAssetFillAmount, - bytes[] memory signatures) + bytes[] memory signatures + ) public returns (LibFillResults.FillResults memory totalFillResults); @@ -102,7 +108,8 @@ contract IWrapperFunctions is function marketSellOrdersNoThrow( LibOrder.Order[] memory orders, uint256 takerAssetFillAmount, - bytes[] memory signatures) + bytes[] memory signatures + ) public returns (LibFillResults.FillResults memory totalFillResults); @@ -114,7 +121,8 @@ contract IWrapperFunctions is function marketBuyOrders( LibOrder.Order[] memory orders, uint256 makerAssetFillAmount, - bytes[] memory signatures) + bytes[] memory signatures + ) public returns (LibFillResults.FillResults memory totalFillResults); @@ -127,7 +135,8 @@ contract IWrapperFunctions is function marketBuyOrdersNoThrow( LibOrder.Order[] memory orders, uint256 makerAssetFillAmount, - bytes[] memory signatures) + bytes[] memory signatures + ) public returns (LibFillResults.FillResults memory totalFillResults); diff --git a/packages/contracts/src/contracts/current/test/DummyERC20Token/DummyERC20Token.sol b/packages/contracts/src/contracts/current/test/DummyERC20Token/DummyERC20Token.sol index 0c7b18c0c..b2fe2df06 100644 --- a/packages/contracts/src/contracts/current/test/DummyERC20Token/DummyERC20Token.sol +++ b/packages/contracts/src/contracts/current/test/DummyERC20Token/DummyERC20Token.sol @@ -31,7 +31,8 @@ contract DummyERC20Token is Mintable, Ownable { string _name, string _symbol, uint256 _decimals, - uint256 _totalSupply) + uint256 _totalSupply + ) public { name = _name; diff --git a/packages/contracts/src/contracts/current/test/DummyERC721Receiver/DummyERC721Receiver.sol b/packages/contracts/src/contracts/current/test/DummyERC721Receiver/DummyERC721Receiver.sol index 1596f3357..c584d0b54 100644 --- a/packages/contracts/src/contracts/current/test/DummyERC721Receiver/DummyERC721Receiver.sol +++ b/packages/contracts/src/contracts/current/test/DummyERC721Receiver/DummyERC721Receiver.sol @@ -52,7 +52,8 @@ contract DummyERC721Receiver is function onERC721Received( address _from, uint256 _tokenId, - bytes _data) + bytes _data + ) public returns (bytes4) { diff --git a/packages/contracts/src/contracts/current/test/DummyERC721Token/DummyERC721Token.sol b/packages/contracts/src/contracts/current/test/DummyERC721Token/DummyERC721Token.sol index 369a2950d..5503eb2f2 100644 --- a/packages/contracts/src/contracts/current/test/DummyERC721Token/DummyERC721Token.sol +++ b/packages/contracts/src/contracts/current/test/DummyERC721Token/DummyERC721Token.sol @@ -34,7 +34,8 @@ contract DummyERC721Token is */ constructor ( string name, - string symbol) + string symbol + ) public ERC721Token(name, symbol) {} diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index f009a6a71..22c84504c 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -68,7 +68,8 @@ contract TestLibBytes is /// @return address from byte array. function publicReadAddress( bytes memory b, - uint256 index) + uint256 index + ) public pure returns (address result) @@ -84,7 +85,8 @@ contract TestLibBytes is function publicWriteAddress( bytes memory b, uint256 index, - address input) + address input + ) public pure returns (bytes memory) @@ -99,7 +101,8 @@ contract TestLibBytes is /// @return bytes32 value from byte array. function publicReadBytes32( bytes memory b, - uint256 index) + uint256 index + ) public pure returns (bytes32 result) @@ -115,7 +118,8 @@ contract TestLibBytes is function publicWriteBytes32( bytes memory b, uint256 index, - bytes32 input) + bytes32 input + ) public pure returns (bytes memory) @@ -130,7 +134,8 @@ contract TestLibBytes is /// @return uint256 value from byte array. function publicReadUint256( bytes memory b, - uint256 index) + uint256 index + ) public pure returns (uint256 result) @@ -146,7 +151,8 @@ contract TestLibBytes is function publicWriteUint256( bytes memory b, uint256 index, - uint256 input) + uint256 input + ) public pure returns (bytes memory) @@ -173,7 +179,8 @@ contract TestLibBytes is /// @return result Nested bytes. function publicReadBytes( bytes memory b, - uint256 index) + uint256 index + ) public pure returns (bytes memory result) @@ -190,7 +197,8 @@ contract TestLibBytes is function publicWriteBytes( bytes memory b, uint256 index, - bytes memory input) + bytes memory input + ) public pure returns (bytes memory) -- cgit v1.2.3 From f457a56d4a0a4e5a5b5b11289f65df64cf2c7f1f Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 7 Jun 2018 11:11:40 -0700 Subject: Style updates to contracts --- .../current/protocol/AssetProxy/ERC721Proxy.sol | 2 +- .../contracts/current/utils/LibBytes/LibBytes.sol | 28 +++++++++++----------- packages/contracts/src/utils/artifacts.ts | 2 +- 3 files changed, 16 insertions(+), 16 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index 499d8d96e..25136133d 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -116,7 +116,7 @@ contract ERC721Proxy is if (length > 53) { receiverData = readBytes(assetData, 52); } - proxyId = uint8(assetData[length-1]); + proxyId = uint8(assetData[length - 1]); return ( proxyId, diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 8f6647d20..282455ea0 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -25,11 +25,11 @@ contract LibBytes is { // Revert reasons - string constant GT_ZERO_LENGTH_REQUIRED = "Length must be greater than 0."; - string constant GTE_4_LENGTH_REQUIRED = "Length must be greater than or equal to 4."; - string constant GTE_20_LENGTH_REQUIRED = "Length must be greater than or equal to 20."; - 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."; + string constant GREATER_THAN_ZERO_LENGTH_REQUIRED = "GREATER_THAN_ZERO_LENGTH_REQUIRED"; + string constant GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED = "GREATER_THAN_4_LENGTH_REQUIRED"; + string constant GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED"; + string constant GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED"; + string constant GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED"; /// @dev Pops the last byte off of a byte array by modifying its length. /// @param b Byte array that will be modified. @@ -41,7 +41,7 @@ contract LibBytes is { require( b.length > 0, - GT_ZERO_LENGTH_REQUIRED + GREATER_THAN_ZERO_LENGTH_REQUIRED ); // Store last byte. @@ -65,7 +65,7 @@ contract LibBytes is { require( b.length >= 20, - GTE_20_LENGTH_REQUIRED + GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED ); // Store last 20 bytes. @@ -128,7 +128,7 @@ contract LibBytes is { require( b.length >= index + 20, // 20 is length of address - GTE_20_LENGTH_REQUIRED + GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED ); // Add offset to index: @@ -160,7 +160,7 @@ contract LibBytes is { require( b.length >= index + 20, // 20 is length of address - GTE_20_LENGTH_REQUIRED + GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED ); // Add offset to index: @@ -199,7 +199,7 @@ contract LibBytes is { require( b.length >= index + 32, - GTE_32_LENGTH_REQUIRED + GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED ); // Arrays are prefixed by a 256 bit length parameter @@ -226,7 +226,7 @@ contract LibBytes is { require( b.length >= index + 32, - GTE_32_LENGTH_REQUIRED + GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED ); // Arrays are prefixed by a 256 bit length parameter @@ -278,7 +278,7 @@ contract LibBytes is { require( b.length >= 4, - GTE_4_LENGTH_REQUIRED + GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED ); assembly { result := mload(add(b, 32)) @@ -306,7 +306,7 @@ contract LibBytes is // length of nested bytes require( b.length >= index + nestedBytesLength, - GTE_32_LENGTH_REQUIRED + GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED ); // Allocate memory and copy value to result @@ -336,7 +336,7 @@ contract LibBytes is // length of input require( b.length >= index + 32 /* 32 bytes to store length */ + input.length, - GTE_32_LENGTH_REQUIRED + GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED ); // Copy into diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts index 42de7c921..bf7221d6d 100644 --- a/packages/contracts/src/utils/artifacts.ts +++ b/packages/contracts/src/utils/artifacts.ts @@ -10,8 +10,8 @@ import * as Exchange from '../artifacts/Exchange.json'; import * as MixinAuthorizable from '../artifacts/MixinAuthorizable.json'; import * as MultiSigWallet from '../artifacts/MultiSigWallet.json'; import * as MultiSigWalletWithTimeLock from '../artifacts/MultiSigWalletWithTimeLock.json'; -import * as TestAssetProxyDispatcher from '../artifacts/TestAssetProxyDispatcher.json'; import * as TestAssetDataDecoders from '../artifacts/TestAssetDataDecoders.json'; +import * as TestAssetProxyDispatcher from '../artifacts/TestAssetProxyDispatcher.json'; import * as TestLibBytes from '../artifacts/TestLibBytes.json'; import * as TestLibMem from '../artifacts/TestLibMem.json'; import * as TestLibs from '../artifacts/TestLibs.json'; -- cgit v1.2.3 From 5bb7219f4b8d5ce22169d5139bd1c07d7b2fcafd Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 7 Jun 2018 11:43:17 -0700 Subject: Camelcase in memCopy --- .../current/test/TestLibMem/TestLibMem.sol | 8 +++--- .../contracts/current/utils/LibBytes/LibBytes.sol | 4 +-- .../src/contracts/current/utils/LibMem/LibMem.sol | 32 +++++++++++----------- 3 files changed, 22 insertions(+), 22 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol index 076bee24c..b7e2e06b8 100644 --- a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol +++ b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol @@ -25,11 +25,11 @@ contract TestLibMem is { /// @dev Copies a block of memory from one location to another. - /// @param mem Memory contents we want to apply memcpy to + /// @param mem Memory contents we want to apply memCopy to /// @param dest Destination offset into . /// @param source Source offset into . /// @param length Length of bytes to copy from to - /// @return mem Memory contents after calling memcpy. + /// @return mem Memory contents after calling memCopy. function testMemcpy( bytes mem, uint256 dest, @@ -47,8 +47,8 @@ contract TestLibMem is // Get pointer to memory contents uint256 offset = getMemAddress(mem) + 32; - // Execute memcpy adjusted for memory array location - memcpy(offset + dest, offset + source, length); + // Execute memCopy adjusted for memory array location + memCopy(offset + dest, offset + source, length); // Return modified memory contents return mem; diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 282455ea0..4f9e2f152 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -311,7 +311,7 @@ contract LibBytes is // Allocate memory and copy value to result result = new bytes(nestedBytesLength); - memcpy( + memCopy( getMemAddress(result) + 32, // +32 skips array length getMemAddress(b) + index + 32, nestedBytesLength @@ -340,7 +340,7 @@ contract LibBytes is ); // Copy into - memcpy( + memCopy( getMemAddress(b) + 32 + index, // +32 to skip length of getMemAddress(input), // includes length of input.length + 32 // +32 bytes to store length diff --git a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol index 960850725..6afb9973a 100644 --- a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol +++ b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol @@ -39,7 +39,7 @@ contract LibMem /// @param dest memory address to copy bytes to. /// @param source memory address to copy bytes from. /// @param length number of bytes to copy. - function memcpy( + function memCopy( uint256 dest, uint256 source, uint256 length @@ -81,42 +81,42 @@ contract LibMem if (source > dest) { assembly { // Record the total number of full words to copy - let nwords := div(length, 32) + let nWords := div(length, 32) - // We subtract 32 from `send` and `dend` because it + // We subtract 32 from `sEnd` and `dEnd` because it // is easier to compare with in the loop, and these // are also the addresses we need for copying the // last bytes. length := sub(length, 32) - let send := add(source, length) - let dend := add(dest, length) + let sEnd := add(source, length) + let dEnd := add(dest, length) // Remember the last 32 bytes of source // This needs to be done here and not after the loop // because we may have overwritten the last bytes in // source already due to overlap. - let last := mload(send) + let last := mload(sEnd) // Copy whole words front to back - for {let i := 0} lt(i, nwords) {i := add(i, 1)} { + for {let i := 0} lt(i, nWords) {i := add(i, 1)} { mstore(dest, mload(source)) source := add(source, 32) dest := add(dest, 32) } // Write the last 32 bytes - mstore(dend, last) + mstore(dEnd, last) } } else { assembly { // Record the total number of full words to copy - let nwords := div(length, 32) + let nWords := div(length, 32) - // We subtract 32 from `send` and `dend` because those + // We subtract 32 from `sEnd` and `dEnd` because those // are the starting points when copying a word at the end. length := sub(length, 32) - let send := add(source, length) - let dend := add(dest, length) + let sEnd := add(source, length) + let dEnd := add(dest, length) // Remember the first 32 bytes of source // This needs to be done here and not after the loop @@ -125,10 +125,10 @@ contract LibMem let first := mload(source) // Copy whole words back to front - for {let i := 0} lt(i, nwords) {i := add(i, 1)} { - mstore(dend, mload(send)) - send := sub(send, 32) - dend := sub(dend, 32) + for {let i := 0} lt(i, nWords) {i := add(i, 1)} { + mstore(dEnd, mload(sEnd)) + sEnd := sub(sEnd, 32) + dEnd := sub(dEnd, 32) } // Write the first 32 bytes -- cgit v1.2.3 From 05123ea6f48a71ac0a53dcadcbc5924cbd1d1486 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Thu, 7 Jun 2018 16:32:42 -0700 Subject: Updated LibBytes error messages --- .../src/contracts/current/utils/LibBytes/LibBytes.sol | 2 +- packages/contracts/src/utils/constants.ts | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 4f9e2f152..d1d10476f 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -26,7 +26,7 @@ contract LibBytes is // Revert reasons string constant GREATER_THAN_ZERO_LENGTH_REQUIRED = "GREATER_THAN_ZERO_LENGTH_REQUIRED"; - string constant GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED = "GREATER_THAN_4_LENGTH_REQUIRED"; + string constant GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED"; string constant GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED"; string constant GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED"; string constant GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED"; diff --git a/packages/contracts/src/utils/constants.ts b/packages/contracts/src/utils/constants.ts index fa2a4af3c..af3f26d82 100644 --- a/packages/contracts/src/utils/constants.ts +++ b/packages/contracts/src/utils/constants.ts @@ -19,11 +19,11 @@ const TESTRPC_PRIVATE_KEYS_STRINGS = [ export const constants = { INVALID_OPCODE: 'invalid opcode', REVERT: 'revert', - LIB_BYTES_GT_ZERO_LENGTH_REQUIRED: 'Length must be greater than 0.', - LIB_BYTES_GTE_4_LENGTH_REQUIRED: 'Length must be greater than or equal to 4.', - LIB_BYTES_GTE_20_LENGTH_REQUIRED: 'Length must be greater than or equal to 20.', - LIB_BYTES_GTE_32_LENGTH_REQUIRED: 'Length must be greater than or equal to 32.', - LIB_BYTES_INDEX_OUT_OF_BOUNDS: 'Specified array index is out of bounds.', + LIB_BYTES_GREATER_THAN_ZERO_LENGTH_REQUIRED: 'GREATER_THAN_ZERO_LENGTH_REQUIRED', + LIB_BYTES_GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED', + LIB_BYTES_GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED', + LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED', + LIB_BYTES_GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED', ERC20_INSUFFICIENT_BALANCE: 'Insufficient balance to complete transfer.', ERC20_INSUFFICIENT_ALLOWANCE: 'Insufficient allowance to complete transfer.', TESTRPC_NETWORK_ID: 50, -- cgit v1.2.3 From 760bab8f866ec3d5fc7627ce9bbf5c2eaaef1f36 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Fri, 8 Jun 2018 11:18:32 -0700 Subject: Implement SolidityProfiler & adapt sol-cov to work with Geth --- packages/contracts/src/utils/coverage.ts | 3 ++- packages/contracts/src/utils/profiler.ts | 27 +++++++++++++++++++++++++++ packages/contracts/src/utils/web3_wrapper.ts | 22 ++++++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 packages/contracts/src/utils/profiler.ts (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/coverage.ts b/packages/contracts/src/utils/coverage.ts index 41c83f703..de29a3ecc 100644 --- a/packages/contracts/src/utils/coverage.ts +++ b/packages/contracts/src/utils/coverage.ts @@ -14,7 +14,8 @@ export const coverage = { _getCoverageSubprovider(): CoverageSubprovider { const defaultFromAddress = devConstants.TESTRPC_FIRST_ADDRESS; const solCompilerArtifactAdapter = new SolCompilerArtifactAdapter(); - const subprovider = new CoverageSubprovider(solCompilerArtifactAdapter, defaultFromAddress); + const isVerbose = true; + const subprovider = new CoverageSubprovider(solCompilerArtifactAdapter, defaultFromAddress, isVerbose); return subprovider; }, }; diff --git a/packages/contracts/src/utils/profiler.ts b/packages/contracts/src/utils/profiler.ts new file mode 100644 index 000000000..85ee24f22 --- /dev/null +++ b/packages/contracts/src/utils/profiler.ts @@ -0,0 +1,27 @@ +import { devConstants } from '@0xproject/dev-utils'; +import { ProfilerSubprovider, SolCompilerArtifactAdapter } from '@0xproject/sol-cov'; +import * as _ from 'lodash'; + +let profilerSubprovider: ProfilerSubprovider; + +export const profiler = { + start(): void { + profiler.getProfilerSubproviderSingleton().start(); + }, + stop(): void { + profiler.getProfilerSubproviderSingleton().stop(); + }, + getProfilerSubproviderSingleton(): ProfilerSubprovider { + if (_.isUndefined(profilerSubprovider)) { + profilerSubprovider = profiler._getProfilerSubprovider(); + } + return profilerSubprovider; + }, + _getProfilerSubprovider(): ProfilerSubprovider { + const defaultFromAddress = devConstants.TESTRPC_FIRST_ADDRESS; + const solCompilerArtifactAdapter = new SolCompilerArtifactAdapter(); + const isVerbose = true; + const subprovider = new ProfilerSubprovider(solCompilerArtifactAdapter, defaultFromAddress, isVerbose); + return subprovider; + }, +}; diff --git a/packages/contracts/src/utils/web3_wrapper.ts b/packages/contracts/src/utils/web3_wrapper.ts index df9bf88c8..c475d96a9 100644 --- a/packages/contracts/src/utils/web3_wrapper.ts +++ b/packages/contracts/src/utils/web3_wrapper.ts @@ -1,8 +1,10 @@ import { devConstants, env, EnvVars, web3Factory } from '@0xproject/dev-utils'; import { prependSubprovider } from '@0xproject/subproviders'; +import { logUtils } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; import { coverage } from './coverage'; +import { profiler } from './profiler'; enum ProviderType { Ganache = 'ganache', @@ -45,9 +47,29 @@ const providerConfigs = testProvider === ProviderType.Ganache ? ganacheConfigs : export const provider = web3Factory.getRpcProvider(providerConfigs); const isCoverageEnabled = env.parseBoolean(EnvVars.SolidityCoverage); +const isProfilerEnabled = env.parseBoolean(EnvVars.SolidityProfiler); +if (isCoverageEnabled && isProfilerEnabled) { + throw new Error( + `Unfortunately for now you can't enable both coverage and profiler at the same time. They both use coverage.json file and there is no way to configure that.`, + ); +} if (isCoverageEnabled) { const coverageSubprovider = coverage.getCoverageSubproviderSingleton(); prependSubprovider(provider, coverageSubprovider); } +if (isProfilerEnabled) { + if (testProvider === ProviderType.Ganache) { + logUtils.warn( + "Gas costs in Ganache traces are incorrect and we don't recommend using it for profiling. Please switch to Geth", + ); + process.exit(1); + } + const profilerSubprovider = profiler.getProfilerSubproviderSingleton(); + logUtils.log( + "By default profilerSubprovider is stopped so that you don't get noise from setup code. Don't forget to start it before the code you want to profile and stop it afterwards", + ); + profilerSubprovider.stop(); + prependSubprovider(provider, profilerSubprovider); +} export const web3Wrapper = new Web3Wrapper(provider); -- cgit v1.2.3