diff options
Diffstat (limited to 'packages/contracts')
27 files changed, 532 insertions, 345 deletions
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index dd25bf41a..ddcd78e93 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -47,16 +47,7 @@ contract ERC20Proxy is internal { // Decode asset data. - ( - uint8 proxyId, - address token - ) = decodeERC20AssetData(assetData); - - // Data must be intended for this proxy. - require( - proxyId == PROXY_ID, - ASSET_PROXY_ID_MISMATCH - ); + address token = readAddress(assetData, 0); // Transfer tokens. bool success = IERC20Token(token).transferFrom(from, to, amount); @@ -75,30 +66,4 @@ contract ERC20Proxy is { return PROXY_ID; } - - /// @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 - returns ( - uint8 proxyId, - address token - ) - { - // Validate encoded data length - uint256 length = assetData.length; - require( - length == 21, - LENGTH_21_REQUIRED - ); - - // Decode data - 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 25136133d..861fac2c1 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -46,30 +46,22 @@ contract ERC721Proxy is ) internal { + // There exists only 1 of each token. + require( + amount == 1, + INVALID_AMOUNT + ); + // Decode asset data. ( - uint8 proxyId, address token, uint256 tokenId, bytes memory receiverData ) = decodeERC721AssetData(assetData); - - // Data must be intended for this proxy. - require( - proxyId == PROXY_ID, - ASSET_PROXY_ID_MISMATCH - ); - - // There exists only 1 of each token. - require( - amount == 1, - INVALID_AMOUNT - ); - // Transfer token. Saves gas by calling safeTransferFrom only // when there is receiverData present. Either succeeds or throws. - if(receiverData.length > 0) { + if (receiverData.length > 0) { ERC721Token(token).safeTransferFrom(from, to, tokenId, receiverData); } else { ERC721Token(token).transferFrom(from, to, tokenId); @@ -97,29 +89,19 @@ contract ERC721Proxy is internal pure returns ( - uint8 proxyId, address token, uint256 tokenId, bytes memory receiverData ) { - // Validate encoded data length - uint256 length = assetData.length; - require( - length >= 53, - LENGTH_AT_LEAST_53_REQUIRED - ); - // Decode asset data. token = readAddress(assetData, 0); tokenId = readUint256(assetData, 20); - if (length > 53) { + if (assetData.length > 52) { receiverData = readBytes(assetData, 52); } - proxyId = uint8(assetData[length - 1]); return ( - proxyId, token, tokenId, receiverData 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 80180a0d9..65bdacdb7 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol @@ -27,11 +27,6 @@ contract LibAssetProxyErrors { string constant AUTHORIZED_ADDRESS_MISMATCH = "AUTHORIZED_ADDRESS_MISMATCH"; // Address at index does not match given target address. /// 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. - - /// Length validation errors /// - string constant LENGTH_21_REQUIRED = "LENGTH_21_REQUIRED"; // Byte array must have a length of 21. - string constant LENGTH_AT_LEAST_53_REQUIRED = "LENGTH_AT_LEAST_53_REQUIRED"; // Byte array must have a length of at least 53. } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol index e77d81c06..9e0246303 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol @@ -19,12 +19,14 @@ pragma solidity ^0.4.24; import "../../utils/Ownable/Ownable.sol"; +import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibExchangeErrors.sol"; import "./mixins/MAssetProxyDispatcher.sol"; import "../AssetProxy/interfaces/IAssetProxy.sol"; contract MixinAssetProxyDispatcher is Ownable, + LibBytes, LibExchangeErrors, MAssetProxyDispatcher { @@ -81,11 +83,13 @@ contract MixinAssetProxyDispatcher is /// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws. /// @param assetData Byte array encoded for the respective asset proxy. + /// @param assetProxyId Id of assetProxy to dispach to. /// @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 assetData, + uint8 assetProxyId, address from, address to, uint256 amount @@ -94,16 +98,8 @@ contract MixinAssetProxyDispatcher is { // Do nothing if no amount should be transferred. if (amount > 0) { - - // Lookup asset proxy - uint256 length = assetData.length; - require( - length > 0, - LENGTH_GREATER_THAN_0_REQUIRED - ); - uint8 assetProxyId = uint8(assetData[length - 1]); + // Lookup assetProxy IAssetProxy assetProxy = assetProxies[assetProxyId]; - // transferFrom will either succeed or throw. assetProxy.transferFrom(assetData, from, to, amount); } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol index 12b57d99f..0a0f0209a 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol @@ -108,9 +108,6 @@ contract MixinExchangeCore is // Compute proportional fill amounts fillResults = calculateFillResults(order, takerAssetFilledAmount); - // Settle order - settleOrder(order, takerAddress, fillResults); - // Update exchange internal state updateFilledState( order, @@ -119,6 +116,10 @@ contract MixinExchangeCore is orderInfo.orderTakerAssetFilledAmount, fillResults ); + + // Settle order + settleOrder(order, takerAddress, fillResults); + return fillResults; } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol index ed76287e0..517b743fe 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol @@ -14,7 +14,6 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; -import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibMath.sol"; import "./libs/LibOrder.sol"; import "./libs/LibFillResults.sol"; @@ -25,7 +24,6 @@ import "./mixins/MSettlement.sol"; import "./mixins/MTransactions.sol"; contract MixinMatchOrders is - LibBytes, LibMath, LibExchangeErrors, MExchangeCore, @@ -94,14 +92,6 @@ contract MixinMatchOrders is rightSignature ); - // Settle matched orders. Succeeds or throws. - settleMatchedOrders( - leftOrder, - rightOrder, - takerAddress, - matchedFillResults - ); - // Update exchange state updateFilledState( leftOrder, @@ -117,6 +107,14 @@ contract MixinMatchOrders is rightOrderInfo.orderTakerAssetFilledAmount, matchedFillResults.right ); + + // Settle matched orders. Succeeds or throws. + settleMatchedOrders( + leftOrder, + rightOrder, + takerAddress, + matchedFillResults + ); return matchedFillResults; } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol index 83e9dfdf4..29a9c87bd 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol @@ -19,6 +19,7 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; +import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibMath.sol"; import "./libs/LibFillResults.sol"; import "./libs/LibOrder.sol"; @@ -28,25 +29,18 @@ import "./mixins/MSettlement.sol"; import "./mixins/MAssetProxyDispatcher.sol"; contract MixinSettlement is + LibBytes, LibMath, LibExchangeErrors, MMatchOrders, MSettlement, MAssetProxyDispatcher { - // ZRX metadata used for fee transfers. + // ZRX address encoded as a byte array. // This will be constant throughout the life of the Exchange contract, // since ZRX will always be transferred via the ERC20 AssetProxy. - bytes internal ZRX_PROXY_DATA; - - /// @dev Gets the ZRX metadata used for fee transfers. - function zrxAssetData() - external - view - returns (bytes memory) - { - return ZRX_PROXY_DATA; - } + bytes internal ZRX_ASSET_DATA; + uint8 constant ZRX_PROXY_ID = 1; /// TODO: _zrxAssetData should be a constant in production. /// @dev Constructor sets the metadata that will be used for paying ZRX fees. @@ -54,7 +48,7 @@ contract MixinSettlement is constructor (bytes memory _zrxAssetData) public { - ZRX_PROXY_DATA = _zrxAssetData; + ZRX_ASSET_DATA = _zrxAssetData; } /// @dev Settles an order by transferring assets between counterparties. @@ -68,26 +62,33 @@ contract MixinSettlement is ) internal { + uint8 makerAssetProxyId = uint8(popLastByte(order.makerAssetData)); + uint8 takerAssetProxyId = uint8(popLastByte(order.takerAssetData)); + bytes memory zrxAssetData = ZRX_ASSET_DATA; dispatchTransferFrom( order.makerAssetData, + makerAssetProxyId, order.makerAddress, takerAddress, fillResults.makerAssetFilledAmount ); dispatchTransferFrom( order.takerAssetData, + takerAssetProxyId, takerAddress, order.makerAddress, fillResults.takerAssetFilledAmount ); dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + ZRX_PROXY_ID, order.makerAddress, order.feeRecipientAddress, fillResults.makerFeePaid ); dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + ZRX_PROXY_ID, takerAddress, order.feeRecipientAddress, fillResults.takerFeePaid @@ -107,21 +108,27 @@ contract MixinSettlement is ) internal { + uint8 leftMakerAssetProxyId = uint8(popLastByte(leftOrder.makerAssetData)); + uint8 rightMakerAssetProxyId = uint8(popLastByte(rightOrder.makerAssetData)); + bytes memory zrxAssetData = ZRX_ASSET_DATA; // Order makers and taker dispatchTransferFrom( leftOrder.makerAssetData, + leftMakerAssetProxyId, leftOrder.makerAddress, rightOrder.makerAddress, matchedFillResults.right.takerAssetFilledAmount ); dispatchTransferFrom( rightOrder.makerAssetData, + rightMakerAssetProxyId, rightOrder.makerAddress, leftOrder.makerAddress, matchedFillResults.left.takerAssetFilledAmount ); dispatchTransferFrom( leftOrder.makerAssetData, + leftMakerAssetProxyId, leftOrder.makerAddress, takerAddress, matchedFillResults.leftMakerAssetSpreadAmount @@ -129,13 +136,15 @@ contract MixinSettlement is // Maker fees dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + ZRX_PROXY_ID, leftOrder.makerAddress, leftOrder.feeRecipientAddress, matchedFillResults.left.makerFeePaid ); dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + ZRX_PROXY_ID, rightOrder.makerAddress, rightOrder.feeRecipientAddress, matchedFillResults.right.makerFeePaid @@ -144,7 +153,8 @@ contract MixinSettlement is // Taker fees if (leftOrder.feeRecipientAddress == rightOrder.feeRecipientAddress) { dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + ZRX_PROXY_ID, takerAddress, leftOrder.feeRecipientAddress, safeAdd( @@ -154,13 +164,15 @@ contract MixinSettlement is ); } else { dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + ZRX_PROXY_ID, takerAddress, leftOrder.feeRecipientAddress, matchedFillResults.left.takerFeePaid ); dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + ZRX_PROXY_ID, takerAddress, rightOrder.feeRecipientAddress, matchedFillResults.right.takerFeePaid diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol index 48a0c5552..1a556dfe2 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol @@ -93,7 +93,7 @@ contract MixinSignatureValidator is ); // Pop last byte off of signature byte array. - SignatureType signatureType = SignatureType(uint8(popByte(signature))); + SignatureType signatureType = SignatureType(uint8(popLastByte(signature))); // Variables are not scoped in Solidity. uint8 v; @@ -183,7 +183,7 @@ contract MixinSignatureValidator is // | 0x14 + x | 1 | Signature type is always "\x06" | } else if (signatureType == SignatureType.Validator) { // Pop last 20 bytes off of signature byte array. - address validator = popAddress(signature); + address validator = popLast20Bytes(signature); // Ensure signer has approved validator. if (!allowedValidators[signer][validator]) { return false; diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index e09f80bcc..a7849f4cb 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -19,7 +19,6 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; -import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibMath.sol"; import "./libs/LibOrder.sol"; import "./libs/LibFillResults.sol"; @@ -27,7 +26,6 @@ import "./libs/LibExchangeErrors.sol"; import "./mixins/MExchangeCore.sol"; contract MixinWrapperFunctions is - LibBytes, LibMath, LibFillResults, LibExchangeErrors, @@ -174,6 +172,7 @@ contract MixinWrapperFunctions is mstore(add(dataAreaStart, mul(10, 0x20)), sub(dataAreaEnd, dataAreaStart)) // Calculate length of <order.makerAssetData> + sourceOffset := mload(add(order, 0x140)) // makerAssetData arrayLenBytes := mload(sourceOffset) sourceOffset := add(sourceOffset, 0x20) arrayLenWords := div(add(arrayLenBytes, 0x1F), 0x20) @@ -193,6 +192,7 @@ contract MixinWrapperFunctions is mstore(add(dataAreaStart, mul(11, 0x20)), sub(dataAreaEnd, dataAreaStart)) // Calculate length of <order.takerAssetData> + sourceOffset := mload(add(order, 0x160)) // takerAssetData arrayLenBytes := mload(sourceOffset) sourceOffset := add(sourceOffset, 0x20) arrayLenWords := div(add(arrayLenBytes, 0x1F), 0x20) @@ -333,14 +333,13 @@ contract MixinWrapperFunctions is public returns (FillResults memory totalFillResults) { + bytes memory takerAssetData = orders[0].takerAssetData; + for (uint256 i = 0; i < orders.length; i++) { - // Token being sold by taker must be the same for each order - // TODO: optimize by only using takerAssetData for first order. - require( - areBytesEqual(orders[i].takerAssetData, orders[0].takerAssetData), - ASSET_DATA_MISMATCH - ); + // We assume that asset being sold by taker is the same for each order. + // Rather than passing this in as calldata, we use the takerAssetData from the first order in all later orders. + orders[i].takerAssetData = takerAssetData; // Calculate the remaining amount of takerAsset to sell uint256 remainingTakerAssetFillAmount = safeSub(takerAssetFillAmount, totalFillResults.takerAssetFilledAmount); @@ -352,6 +351,14 @@ contract MixinWrapperFunctions is signatures[i] ); + // HACK: the proxyId is "popped" from the byte array before a fill is settled + // by subtracting from the length of the array. Since the popped byte is + // still in memory, we can "unpop" it by incrementing the length of the byte array. + assembly { + let len := mload(takerAssetData) + mstore(takerAssetData, add(len, 1)) + } + // Update amounts filled and fees paid by maker and taker addFillResults(totalFillResults, singleFillResults); @@ -377,14 +384,13 @@ contract MixinWrapperFunctions is public returns (FillResults memory totalFillResults) { + bytes memory takerAssetData = orders[0].takerAssetData; + for (uint256 i = 0; i < orders.length; i++) { - // Token being sold by taker must be the same for each order - // TODO: optimize by only using takerAssetData for first order. - require( - areBytesEqual(orders[i].takerAssetData, orders[0].takerAssetData), - ASSET_DATA_MISMATCH - ); + // We assume that asset being sold by taker is the same for each order. + // Rather than passing this in as calldata, we use the takerAssetData from the first order in all later orders. + orders[i].takerAssetData = takerAssetData; // Calculate the remaining amount of takerAsset to sell uint256 remainingTakerAssetFillAmount = safeSub(takerAssetFillAmount, totalFillResults.takerAssetFilledAmount); @@ -420,14 +426,13 @@ contract MixinWrapperFunctions is public returns (FillResults memory totalFillResults) { + bytes memory makerAssetData = orders[0].makerAssetData; + for (uint256 i = 0; i < orders.length; i++) { - // Token being bought by taker must be the same for each order - // TODO: optimize by only using makerAssetData for first order. - require( - areBytesEqual(orders[i].makerAssetData, orders[0].makerAssetData), - ASSET_DATA_MISMATCH - ); + // We assume that asset being bought by taker is the same for each order. + // Rather than passing this in as calldata, we copy the makerAssetData from the first order onto all later orders. + orders[i].makerAssetData = makerAssetData; // Calculate the remaining amount of makerAsset to buy uint256 remainingMakerAssetFillAmount = safeSub(makerAssetFillAmount, totalFillResults.makerAssetFilledAmount); @@ -447,6 +452,14 @@ contract MixinWrapperFunctions is signatures[i] ); + // HACK: the proxyId is "popped" from the byte array before a fill is settled + // by subtracting from the length of the array. Since the popped byte is + // still in memory, we can "unpop" it by incrementing the length of the byte array. + assembly { + let len := mload(makerAssetData) + mstore(makerAssetData, add(len, 1)) + } + // Update amounts filled and fees paid by maker and taker addFillResults(totalFillResults, singleFillResults); @@ -472,14 +485,13 @@ contract MixinWrapperFunctions is public returns (FillResults memory totalFillResults) { + bytes memory makerAssetData = orders[0].makerAssetData; + for (uint256 i = 0; i < orders.length; i++) { - // Token being bought by taker must be the same for each order - // TODO: optimize by only using makerAssetData for first order. - require( - areBytesEqual(orders[i].makerAssetData, orders[0].makerAssetData), - ASSET_DATA_MISMATCH - ); + // We assume that asset being bought by taker is the same for each order. + // Rather than passing this in as calldata, we copy the makerAssetData from the first order onto all later orders. + orders[i].makerAssetData = makerAssetData; // Calculate the remaining amount of makerAsset to buy uint256 remainingMakerAssetFillAmount = safeSub(makerAssetFillAmount, totalFillResults.makerAssetFilledAmount); diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol index 2b4bbeec4..48dd0f8be 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol @@ -25,7 +25,6 @@ contract LibExchangeErrors { string constant INVALID_TAKER = "INVALID_TAKER"; // Invalid takerAddress. string constant INVALID_SENDER = "INVALID_SENDER"; // Invalid `msg.sender`. string constant INVALID_ORDER_SIGNATURE = "INVALID_ORDER_SIGNATURE"; // Signature validation failed. - string constant ASSET_DATA_MISMATCH = "ASSET_DATA_MISMATCH"; // Asset data must be the same for each order. /// fillOrder validation errors /// string constant INVALID_TAKER_AMOUNT = "INVALID_TAKER_AMOUNT"; // takerAssetFillAmount cannot equal 0. 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 82eafb529..d16a830f4 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol @@ -34,11 +34,13 @@ contract MAssetProxyDispatcher is /// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws. /// @param assetData Byte array encoded for the respective asset proxy. + /// @param assetProxyId Id of assetProxy to dispach to. /// @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 assetData, + uint8 assetProxyId, address from, address to, uint256 amount diff --git a/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol b/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol index 2c6a8fdb0..5987291d2 100644 --- a/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol +++ b/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol @@ -23,26 +23,8 @@ import "../../protocol/AssetProxy/ERC20Proxy.sol"; import "../../protocol/AssetProxy/ERC721Proxy.sol"; contract TestAssetDataDecoders is - ERC20Proxy, ERC721Proxy { - - /// @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 proxyId, - address token - ) - { - (proxyId, token) = decodeERC20AssetData(assetData); - return (proxyId, token); - } - /// @dev Decodes ERC721 Asset data. /// @param assetData Encoded byte array. /// @return proxyId Intended ERC721 proxy id. @@ -54,21 +36,18 @@ contract TestAssetDataDecoders is public pure returns ( - uint8 proxyId, address token, uint256 tokenId, bytes memory receiverData ) { ( - proxyId, token, tokenId, receiverData ) = decodeERC721AssetData(assetData); return ( - proxyId, token, tokenId, receiverData diff --git a/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol index 2ae69e0ef..d469a07f0 100644 --- a/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol @@ -24,11 +24,12 @@ import "../../protocol/Exchange/MixinAssetProxyDispatcher.sol"; contract TestAssetProxyDispatcher is MixinAssetProxyDispatcher { function publicDispatchTransferFrom( bytes memory assetData, + uint8 assetProxyId, address from, address to, uint256 amount) public { - dispatchTransferFrom(assetData, from, to, amount); + dispatchTransferFrom(assetData, assetProxyId, from, to, amount); } } diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index 22c84504c..6f1898acd 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -28,24 +28,24 @@ contract TestLibBytes is /// @dev Pops the last byte off of a byte array by modifying its length. /// @param b Byte array that will be modified. /// @return The byte that was popped off. - function publicPopByte(bytes memory b) + function publicPopLastByte(bytes memory b) public pure returns (bytes memory, bytes1 result) { - result = popByte(b); + result = popLastByte(b); return (b, result); } /// @dev Pops the last 20 bytes off of a byte array by modifying its length. /// @param b Byte array that will be modified. /// @return The 20 byte address that was popped off. - function publicPopAddress(bytes memory b) + function publicPopLast20Bytes(bytes memory b) public pure returns (bytes memory, address result) { - result = popAddress(b); + result = popLast20Bytes(b); return (b, result); } @@ -62,6 +62,21 @@ contract TestLibBytes is return equal; } + /// @dev Performs a deep copy of a byte array onto another byte array of greater than or equal length. + /// @param dest Byte array that will be overwritten with source bytes. + /// @param source Byte array to copy onto dest bytes. + function publicDeepCopyBytes( + bytes memory dest, + bytes memory source + ) + public + pure + returns (bytes memory) + { + deepCopyBytes(dest, source); + return dest; + } + /// @dev Reads an address from a position in a byte array. /// @param b Byte array containing an address. /// @param index Index in byte array of address. diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index d1d10476f..10d7ce41a 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -30,11 +30,12 @@ contract LibBytes is 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"; + string constant GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_SOURCE_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. /// @return The byte that was popped off. - function popByte(bytes memory b) + function popLastByte(bytes memory b) internal pure returns (bytes1 result) @@ -58,7 +59,7 @@ contract LibBytes is /// @dev Pops the last 20 bytes off of a byte array by modifying its length. /// @param b Byte array that will be modified. /// @return The 20 byte address that was popped off. - function popAddress(bytes memory b) + function popLast20Bytes(bytes memory b) internal pure returns (address result) @@ -79,41 +80,6 @@ contract LibBytes is return result; } - /// @dev Tests equality of two byte arrays. - /// @param lhs First byte array to compare. - /// @param rhs Second byte array to compare. - /// @return True if arrays are the same. False otherwise. - function areBytesEqual( - bytes memory lhs, - bytes memory rhs - ) - internal - pure - returns (bool equal) - { - assembly { - // Get the number of words occupied by <lhs> - let lenFullWords := div(add(mload(lhs), 0x1F), 0x20) - - // Add 1 to the number of words, to account for the length field - lenFullWords := add(lenFullWords, 0x1) - - // Test equality word-by-word. - // Terminates early if there is a mismatch. - for {let i := 0} lt(i, lenFullWords) {i := add(i, 1)} { - let lhsWord := mload(add(lhs, mul(i, 0x20))) - let rhsWord := mload(add(rhs, mul(i, 0x20))) - equal := eq(lhsWord, rhsWord) - if eq(equal, 0) { - // Break - i := lenFullWords - } - } - } - - return equal; - } - /// @dev Reads an address from a position in a byte array. /// @param b Byte array containing an address. /// @param index Index in byte array of address. @@ -346,4 +312,62 @@ contract LibBytes is input.length + 32 // +32 bytes to store <input> length ); } + + /// @dev Tests equality of two byte arrays. + /// @param lhs First byte array to compare. + /// @param rhs Second byte array to compare. + /// @return True if arrays are the same. False otherwise. + function areBytesEqual( + bytes memory lhs, + bytes memory rhs + ) + internal + pure + returns (bool equal) + { + assembly { + // Get the number of words occupied by <lhs> + let lenFullWords := div(add(mload(lhs), 0x1F), 0x20) + + // Add 1 to the number of words, to account for the length field + lenFullWords := add(lenFullWords, 0x1) + + // Test equality word-by-word. + // Terminates early if there is a mismatch. + for {let i := 0} lt(i, lenFullWords) {i := add(i, 1)} { + let lhsWord := mload(add(lhs, mul(i, 0x20))) + let rhsWord := mload(add(rhs, mul(i, 0x20))) + equal := eq(lhsWord, rhsWord) + if eq(equal, 0) { + // Break + i := lenFullWords + } + } + } + + return equal; + } + + /// @dev Performs a deep copy of a byte array onto another byte array of greater than or equal length. + /// @param dest Byte array that will be overwritten with source bytes. + /// @param source Byte array to copy onto dest bytes. + function deepCopyBytes( + bytes memory dest, + bytes memory source + ) + internal + pure + { + uint256 sourceLen = source.length; + // Dest length must be >= source length, or some bytes would not be copied. + require( + dest.length >= sourceLen, + GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED + ); + memCopy( + getMemAddress(dest) + 32, // +32 to skip length of <dest> + getMemAddress(source) + 32, // +32 to skip length of <source> + sourceLen + ); + } } diff --git a/packages/contracts/src/utils/constants.ts b/packages/contracts/src/utils/constants.ts index af3f26d82..ec3c8fd36 100644 --- a/packages/contracts/src/utils/constants.ts +++ b/packages/contracts/src/utils/constants.ts @@ -24,10 +24,14 @@ export const constants = { 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', + LIB_BYTES_GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED', ERC20_INSUFFICIENT_BALANCE: 'Insufficient balance to complete transfer.', ERC20_INSUFFICIENT_ALLOWANCE: 'Insufficient allowance to complete transfer.', TESTRPC_NETWORK_ID: 50, - AWAIT_TRANSACTION_MINED_MS: 100, + // Note(albrow): In practice V8 and most other engines limit the minimum + // interval for setInterval to 10ms. We still set it to 0 here in order to + // ensure we always use the minimum interval. + AWAIT_TRANSACTION_MINED_MS: 0, MAX_ETHERTOKEN_WITHDRAW_GAS: 43000, MAX_TOKEN_TRANSFERFROM_GAS: 80000, MAX_TOKEN_APPROVE_GAS: 60000, diff --git a/packages/contracts/src/utils/exchange_wrapper.ts b/packages/contracts/src/utils/exchange_wrapper.ts index e74b91fe2..38dcb891a 100644 --- a/packages/contracts/src/utils/exchange_wrapper.ts +++ b/packages/contracts/src/utils/exchange_wrapper.ts @@ -165,14 +165,14 @@ export class ExchangeWrapper { public async marketBuyOrdersNoThrowAsync( orders: SignedOrder[], from: string, - opts: { makerAssetFillAmount: BigNumber }, + opts: { makerAssetFillAmount: BigNumber; gas?: number }, ): Promise<TransactionReceiptWithDecodedLogs> { const params = formatters.createMarketBuyOrders(orders, opts.makerAssetFillAmount); const txHash = await this._exchange.marketBuyOrdersNoThrow.sendTransactionAsync( params.orders, params.makerAssetFillAmount, params.signatures, - { from }, + { from, gas: opts.gas }, ); const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; diff --git a/packages/contracts/src/utils/formatters.ts b/packages/contracts/src/utils/formatters.ts index 1035f2d7c..32e4787d6 100644 --- a/packages/contracts/src/utils/formatters.ts +++ b/packages/contracts/src/utils/formatters.ts @@ -2,6 +2,7 @@ import { SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import * as _ from 'lodash'; +import { constants } from './constants'; import { orderUtils } from './order_utils'; import { BatchCancelOrders, BatchFillOrders, MarketBuyOrders, MarketSellOrders } from './types'; @@ -28,8 +29,11 @@ export const formatters = { signatures: [], takerAssetFillAmount, }; - _.forEach(signedOrders, signedOrder => { + _.forEach(signedOrders, (signedOrder, i) => { const orderWithoutExchangeAddress = orderUtils.getOrderWithoutExchangeAddress(signedOrder); + if (i !== 0) { + orderWithoutExchangeAddress.takerAssetData = constants.NULL_BYTES; + } marketSellOrders.orders.push(orderWithoutExchangeAddress); marketSellOrders.signatures.push(signedOrder.signature); }); @@ -41,8 +45,11 @@ export const formatters = { signatures: [], makerAssetFillAmount, }; - _.forEach(signedOrders, signedOrder => { + _.forEach(signedOrders, (signedOrder, i) => { const orderWithoutExchangeAddress = orderUtils.getOrderWithoutExchangeAddress(signedOrder); + if (i !== 0) { + orderWithoutExchangeAddress.makerAssetData = constants.NULL_BYTES; + } marketBuyOrders.orders.push(orderWithoutExchangeAddress); marketBuyOrders.signatures.push(signedOrder.signature); }); diff --git a/packages/contracts/src/utils/order_utils.ts b/packages/contracts/src/utils/order_utils.ts index 40643fa75..019f6e74b 100644 --- a/packages/contracts/src/utils/order_utils.ts +++ b/packages/contracts/src/utils/order_utils.ts @@ -1,6 +1,7 @@ import { OrderWithoutExchangeAddress, SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; +import { constants } from './constants'; import { CancelOrder, MatchOrder } from './types'; export const orderUtils = { @@ -50,6 +51,8 @@ export const orderUtils = { leftSignature: signedOrderLeft.signature, rightSignature: signedOrderRight.signature, }; + fill.right.makerAssetData = constants.NULL_BYTES; + fill.right.takerAssetData = constants.NULL_BYTES; return fill; }, }; diff --git a/packages/contracts/test/asset_proxy/decoder.ts b/packages/contracts/test/asset_proxy/decoder.ts index d4fae1601..875d55daa 100644 --- a/packages/contracts/test/asset_proxy/decoder.ts +++ b/packages/contracts/test/asset_proxy/decoder.ts @@ -42,33 +42,19 @@ describe('TestAssetDataDecoders', () => { }); describe('Asset Data Decoders', () => { - it('should correctly decode ERC20 asset data)', async () => { - const encodedAssetData = assetProxyUtils.encodeERC20AssetData(testAddress); - const expectedDecodedAssetData = assetProxyUtils.decodeERC20AssetData(encodedAssetData); - let decodedAssetProxyId: number; - let decodedTokenAddress: string; - [decodedAssetProxyId, decodedTokenAddress] = await testAssetProxyDecoder.publicDecodeERC20Data.callAsync( - encodedAssetData, - ); - expect(decodedAssetProxyId).to.be.equal(expectedDecodedAssetData.assetProxyId); - expect(decodedTokenAddress).to.be.equal(expectedDecodedAssetData.tokenAddress); - }); - it('should correctly decode ERC721 asset data', async () => { const tokenId = generatePseudoRandomSalt(); const encodedAssetData = assetProxyUtils.encodeERC721AssetData(testAddress, tokenId); + const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2); const expectedDecodedAssetData = assetProxyUtils.decodeERC721AssetData(encodedAssetData); - let decodedAssetProxyId: number; let decodedTokenAddress: string; let decodedTokenId: BigNumber; let decodedData: string; [ - decodedAssetProxyId, decodedTokenAddress, decodedTokenId, decodedData, - ] = await testAssetProxyDecoder.publicDecodeERC721Data.callAsync(encodedAssetData); - expect(decodedAssetProxyId).to.be.equal(expectedDecodedAssetData.assetProxyId); + ] = await testAssetProxyDecoder.publicDecodeERC721Data.callAsync(encodedAssetDataWithoutProxyId); expect(decodedTokenAddress).to.be.equal(expectedDecodedAssetData.tokenAddress); expect(decodedTokenId).to.be.bignumber.equal(expectedDecodedAssetData.tokenId); expect(decodedData).to.be.equal(expectedDecodedAssetData.receiverData); @@ -84,17 +70,14 @@ describe('TestAssetDataDecoders', () => { const receiverData = receiverDataFirst32Bytes + receiverDataExtraBytes; const encodedAssetData = assetProxyUtils.encodeERC721AssetData(testAddress, tokenId, receiverData); const expectedDecodedAssetData = assetProxyUtils.decodeERC721AssetData(encodedAssetData); - let decodedAssetProxyId: number; let decodedTokenAddress: string; let decodedTokenId: BigNumber; let decodedReceiverData: string; [ - decodedAssetProxyId, decodedTokenAddress, decodedTokenId, decodedReceiverData, ] = await testAssetProxyDecoder.publicDecodeERC721Data.callAsync(encodedAssetData); - expect(decodedAssetProxyId).to.be.equal(expectedDecodedAssetData.assetProxyId); expect(decodedTokenAddress).to.be.equal(expectedDecodedAssetData.tokenAddress); expect(decodedTokenId).to.be.bignumber.equal(expectedDecodedAssetData.tokenId); expect(decodedReceiverData).to.be.equal(expectedDecodedAssetData.receiverData); diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts index 08376ccfb..9760d3b9c 100644 --- a/packages/contracts/test/asset_proxy/proxies.ts +++ b/packages/contracts/test/asset_proxy/proxies.ts @@ -96,12 +96,13 @@ describe('Asset Transfer Proxies', () => { it('should successfully transfer tokens', async () => { // Construct ERC20 asset data const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address); + const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2); // Perform a transfer from makerAddress to takerAddress const erc20Balances = await erc20Wrapper.getBalancesAsync(); const amount = new BigNumber(10); await web3Wrapper.awaitTransactionSuccessAsync( await erc20Proxy.transferFrom.sendTransactionAsync( - encodedAssetData, + encodedAssetDataWithoutProxyId, makerAddress, takerAddress, amount, @@ -122,12 +123,13 @@ describe('Asset Transfer Proxies', () => { it('should do nothing if transferring 0 amount of a token', async () => { // Construct ERC20 asset data const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address); + const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2); // Perform a transfer from makerAddress to takerAddress const erc20Balances = await erc20Wrapper.getBalancesAsync(); const amount = new BigNumber(0); await web3Wrapper.awaitTransactionSuccessAsync( await erc20Proxy.transferFrom.sendTransactionAsync( - encodedAssetData, + encodedAssetDataWithoutProxyId, makerAddress, takerAddress, amount, @@ -172,12 +174,19 @@ describe('Asset Transfer Proxies', () => { it('should throw if requesting address is not authorized', async () => { // Construct ERC20 asset data const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address); + const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(10); return expectRevertOrAlwaysFailingTransactionAsync( - erc20Proxy.transferFrom.sendTransactionAsync(encodedAssetData, makerAddress, takerAddress, amount, { - from: notAuthorized, - }), + erc20Proxy.transferFrom.sendTransactionAsync( + encodedAssetDataWithoutProxyId, + makerAddress, + takerAddress, + amount, + { + from: notAuthorized, + }, + ), ); }); }); @@ -187,9 +196,10 @@ describe('Asset Transfer Proxies', () => { const erc20Balances = await erc20Wrapper.getBalancesAsync(); const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address); + const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2); const amount = new BigNumber(10); const numTransfers = 2; - const assetData = _.times(numTransfers, () => encodedAssetData); + const assetData = _.times(numTransfers, () => encodedAssetDataWithoutProxyId); const fromAddresses = _.times(numTransfers, () => makerAddress); const toAddresses = _.times(numTransfers, () => takerAddress); const amounts = _.times(numTransfers, () => amount); @@ -218,9 +228,10 @@ describe('Asset Transfer Proxies', () => { it('should throw if not called by an authorized address', async () => { const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address); + const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2); const amount = new BigNumber(10); const numTransfers = 2; - const assetData = _.times(numTransfers, () => encodedAssetData); + const assetData = _.times(numTransfers, () => encodedAssetDataWithoutProxyId); const fromAddresses = _.times(numTransfers, () => makerAddress); const toAddresses = _.times(numTransfers, () => takerAddress); const amounts = _.times(numTransfers, () => amount); @@ -244,6 +255,7 @@ describe('Asset Transfer Proxies', () => { it('should successfully transfer tokens', async () => { // Construct ERC721 asset data const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); + const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2); // Verify pre-condition const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress); @@ -251,7 +263,7 @@ describe('Asset Transfer Proxies', () => { const amount = new BigNumber(1); await web3Wrapper.awaitTransactionSuccessAsync( await erc721Proxy.transferFrom.sendTransactionAsync( - encodedAssetData, + encodedAssetDataWithoutProxyId, makerAddress, takerAddress, amount, @@ -267,13 +279,14 @@ describe('Asset Transfer Proxies', () => { it('should not call onERC721Received when transferring to a smart contract without receiver data', async () => { // Construct ERC721 asset data const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); + const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2); // Verify pre-condition const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(1); const txHash = await erc721Proxy.transferFrom.sendTransactionAsync( - encodedAssetData, + encodedAssetDataWithoutProxyId, makerAddress, erc721Receiver.address, amount, @@ -298,13 +311,14 @@ describe('Asset Transfer Proxies', () => { erc721MakerTokenId, receiverData, ); + const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2); // Verify pre-condition const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(1); const txHash = await erc721Proxy.transferFrom.sendTransactionAsync( - encodedAssetData, + encodedAssetDataWithoutProxyId, makerAddress, erc721Receiver.address, amount, @@ -333,6 +347,7 @@ describe('Asset Transfer Proxies', () => { erc721MakerTokenId, receiverData, ); + const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2); // Verify pre-condition const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress); @@ -340,7 +355,7 @@ describe('Asset Transfer Proxies', () => { const amount = new BigNumber(1); return expectRevertOrAlwaysFailingTransactionAsync( erc721Proxy.transferFrom.sendTransactionAsync( - encodedAssetData, + encodedAssetDataWithoutProxyId, makerAddress, erc20Proxy.address, // the ERC20 proxy does not have an ERC721 receiver amount, @@ -352,6 +367,7 @@ describe('Asset Transfer Proxies', () => { it('should throw if transferring 0 amount of a token', async () => { // Construct ERC721 asset data const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); + const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2); // Verify pre-condition const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress); @@ -359,7 +375,7 @@ describe('Asset Transfer Proxies', () => { const amount = new BigNumber(0); return expectRevertOrAlwaysFailingTransactionAsync( erc721Proxy.transferFrom.sendTransactionAsync( - encodedAssetData, + encodedAssetDataWithoutProxyId, makerAddress, takerAddress, amount, @@ -371,6 +387,7 @@ describe('Asset Transfer Proxies', () => { it('should throw if transferring > 1 amount of a token', async () => { // Construct ERC721 asset data const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); + const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2); // Verify pre-condition const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress); @@ -378,7 +395,7 @@ describe('Asset Transfer Proxies', () => { const amount = new BigNumber(500); return expectRevertOrAlwaysFailingTransactionAsync( erc721Proxy.transferFrom.sendTransactionAsync( - encodedAssetData, + encodedAssetDataWithoutProxyId, makerAddress, takerAddress, amount, @@ -390,6 +407,7 @@ describe('Asset Transfer Proxies', () => { it('should throw if allowances are too low', async () => { // Construct ERC721 asset data const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); + const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2); // Remove transfer approval for makerAddress. await web3Wrapper.awaitTransactionSuccessAsync( await erc721Token.setApprovalForAll.sendTransactionAsync(erc721Proxy.address, false, { @@ -400,20 +418,27 @@ describe('Asset Transfer Proxies', () => { // Perform a transfer; expect this to fail. const amount = new BigNumber(1); return expectRevertOrAlwaysFailingTransactionAsync( - erc20Proxy.transferFrom.sendTransactionAsync(encodedAssetData, makerAddress, takerAddress, amount, { - from: notAuthorized, - }), + erc20Proxy.transferFrom.sendTransactionAsync( + encodedAssetDataWithoutProxyId, + makerAddress, + takerAddress, + amount, + { + from: notAuthorized, + }, + ), ); }); it('should throw if requesting address is not authorized', async () => { // Construct ERC721 asset data const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); + const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(1); return expectRevertOrAlwaysFailingTransactionAsync( erc721Proxy.transferFrom.sendTransactionAsync( - encodedAssetData, + encodedAssetDataWithoutProxyId, makerAddress, takerAddress, amount, @@ -430,8 +455,8 @@ describe('Asset Transfer Proxies', () => { const numTransfers = 2; const assetData = [ - assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdA), - assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdB), + assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdA).slice(0, -2), + assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdB).slice(0, -2), ]; const fromAddresses = _.times(numTransfers, () => makerAddress); const toAddresses = _.times(numTransfers, () => takerAddress); @@ -462,8 +487,8 @@ describe('Asset Transfer Proxies', () => { const numTransfers = 2; const assetData = [ - assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdA), - assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdB), + assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdA).slice(0, -2), + assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdB).slice(0, -2), ]; const fromAddresses = _.times(numTransfers, () => makerAddress); const toAddresses = _.times(numTransfers, () => takerAddress); @@ -484,3 +509,4 @@ describe('Asset Transfer Proxies', () => { }); }); // tslint:enable:no-unnecessary-type-assertion +// tslint:disable:max-file-line-count diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index 53b98c755..63c2fa6c0 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -87,7 +87,7 @@ describe('Exchange core', () => { artifacts.Exchange, provider, txDefaults, - assetProxyUtils.encodeERC20AssetData(zrxToken.address), + zrxToken.address, ); exchangeWrapper = new ExchangeWrapper(exchange, provider); await exchangeWrapper.registerAssetProxyAsync(AssetProxyId.ERC20, erc20Proxy.address, owner); diff --git a/packages/contracts/test/exchange/dispatcher.ts b/packages/contracts/test/exchange/dispatcher.ts index 9e113e47d..abbfd7ac7 100644 --- a/packages/contracts/test/exchange/dispatcher.ts +++ b/packages/contracts/test/exchange/dispatcher.ts @@ -276,12 +276,14 @@ describe('AssetProxyDispatcher', () => { ); // Construct metadata for ERC20 proxy const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address); + const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2); // Perform a transfer from makerAddress to takerAddress const erc20Balances = await erc20Wrapper.getBalancesAsync(); const amount = new BigNumber(10); await web3Wrapper.awaitTransactionSuccessAsync( await assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync( - encodedAssetData, + encodedAssetDataWithoutProxyId, + AssetProxyId.ERC20, makerAddress, takerAddress, amount, @@ -302,11 +304,13 @@ describe('AssetProxyDispatcher', () => { it('should throw if dispatching to unregistered proxy', async () => { // Construct metadata for ERC20 proxy const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address); + const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2); // Perform a transfer from makerAddress to takerAddress const amount = new BigNumber(10); return expectRevertOrAlwaysFailingTransactionAsync( assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync( - encodedAssetData, + encodedAssetDataWithoutProxyId, + AssetProxyId.ERC20, makerAddress, takerAddress, amount, diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts index 18a46187f..b8dca04fd 100644 --- a/packages/contracts/test/exchange/match_orders.ts +++ b/packages/contracts/test/exchange/match_orders.ts @@ -96,7 +96,7 @@ describe('matchOrders', () => { artifacts.Exchange, provider, txDefaults, - assetProxyUtils.encodeERC20AssetData(zrxToken.address), + zrxToken.address, ); exchangeWrapper = new ExchangeWrapper(exchange, provider); await exchangeWrapper.registerAssetProxyAsync(AssetProxyId.ERC20, erc20Proxy.address, owner); diff --git a/packages/contracts/test/exchange/transactions.ts b/packages/contracts/test/exchange/transactions.ts index 12390ce01..21ff123e5 100644 --- a/packages/contracts/test/exchange/transactions.ts +++ b/packages/contracts/test/exchange/transactions.ts @@ -72,7 +72,7 @@ describe('Exchange transactions', () => { artifacts.Exchange, provider, txDefaults, - assetProxyUtils.encodeERC20AssetData(zrxToken.address), + zrxToken.address, ); exchangeWrapper = new ExchangeWrapper(exchange, provider); await exchangeWrapper.registerAssetProxyAsync(AssetProxyId.ERC20, erc20Proxy.address, owner); diff --git a/packages/contracts/test/exchange/wrapper.ts b/packages/contracts/test/exchange/wrapper.ts index b66cff90a..abba1ac4f 100644 --- a/packages/contracts/test/exchange/wrapper.ts +++ b/packages/contracts/test/exchange/wrapper.ts @@ -81,7 +81,7 @@ describe('Exchange wrappers', () => { artifacts.Exchange, provider, txDefaults, - assetProxyUtils.encodeERC20AssetData(zrxToken.address), + zrxToken.address, ); exchangeWrapper = new ExchangeWrapper(exchange, provider); await exchangeWrapper.registerAssetProxyAsync(AssetProxyId.ERC20, erc20Proxy.address, owner); @@ -712,6 +712,10 @@ describe('Exchange wrappers', () => { ); await exchangeWrapper.marketSellOrdersNoThrowAsync(signedOrders, takerAddress, { takerAssetFillAmount, + // HACK(albrow): We need to hardcode the gas estimate here because + // the Geth gas estimator doesn't work with the way we use + // delegatecall and swallow errors. + gas: 6000000, }); const newBalances = await erc20Wrapper.getBalancesAsync(); @@ -781,20 +785,49 @@ describe('Exchange wrappers', () => { expect(newBalances).to.be.deep.equal(erc20Balances); }); - it('should throw when a signedOrder does not use the same takerAssetAddress', async () => { + it('should not fill a signedOrder that does not use the same takerAssetAddress', async () => { signedOrders = [ orderFactory.newSignedOrder(), + orderFactory.newSignedOrder(), orderFactory.newSignedOrder({ takerAssetData: assetProxyUtils.encodeERC20AssetData(zrxToken.address), }), - orderFactory.newSignedOrder(), ]; + const takerAssetFillAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(100000), 18); + const filledSignedOrders = signedOrders.slice(0, -1); + _.forEach(filledSignedOrders, signedOrder => { + erc20Balances[makerAddress][defaultMakerAssetAddress] = erc20Balances[makerAddress][ + defaultMakerAssetAddress + ].minus(signedOrder.makerAssetAmount); + erc20Balances[makerAddress][defaultTakerAssetAddress] = erc20Balances[makerAddress][ + defaultTakerAssetAddress + ].add(signedOrder.takerAssetAmount); + erc20Balances[makerAddress][zrxToken.address] = erc20Balances[makerAddress][zrxToken.address].minus( + signedOrder.makerFee, + ); + erc20Balances[takerAddress][defaultMakerAssetAddress] = erc20Balances[takerAddress][ + defaultMakerAssetAddress + ].add(signedOrder.makerAssetAmount); + erc20Balances[takerAddress][defaultTakerAssetAddress] = erc20Balances[takerAddress][ + defaultTakerAssetAddress + ].minus(signedOrder.takerAssetAmount); + erc20Balances[takerAddress][zrxToken.address] = erc20Balances[takerAddress][zrxToken.address].minus( + signedOrder.takerFee, + ); + erc20Balances[feeRecipientAddress][zrxToken.address] = erc20Balances[feeRecipientAddress][ + zrxToken.address + ].add(signedOrder.makerFee.add(signedOrder.takerFee)); + }); + await exchangeWrapper.marketSellOrdersNoThrowAsync(signedOrders, takerAddress, { + takerAssetFillAmount, + // HACK(albrow): We need to hardcode the gas estimate here because + // the Geth gas estimator doesn't work with the way we use + // delegatecall and swallow errors. + gas: 600000, + }); - return expectRevertOrAlwaysFailingTransactionAsync( - exchangeWrapper.marketSellOrdersNoThrowAsync(signedOrders, takerAddress, { - takerAssetFillAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1000), 18), - }), - ); + const newBalances = await erc20Wrapper.getBalancesAsync(); + expect(newBalances).to.be.deep.equal(erc20Balances); }); }); @@ -894,6 +927,10 @@ describe('Exchange wrappers', () => { ); await exchangeWrapper.marketBuyOrdersNoThrowAsync(signedOrders, takerAddress, { makerAssetFillAmount, + // HACK(albrow): We need to hardcode the gas estimate here because + // the Geth gas estimator doesn't work with the way we use + // delegatecall and swallow errors. + gas: 600000, }); const newBalances = await erc20Wrapper.getBalancesAsync(); @@ -926,8 +963,8 @@ describe('Exchange wrappers', () => { ); }); - it('should fill all signedOrders if cannot fill entire takerAssetFillAmount', async () => { - const takerAssetFillAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(100000), 18); + it('should fill all signedOrders if cannot fill entire makerAssetFillAmount', async () => { + const makerAssetFillAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(100000), 18); _.forEach(signedOrders, signedOrder => { erc20Balances[makerAddress][defaultMakerAssetAddress] = erc20Balances[makerAddress][ defaultMakerAssetAddress @@ -951,8 +988,8 @@ describe('Exchange wrappers', () => { zrxToken.address ].add(signedOrder.makerFee.add(signedOrder.takerFee)); }); - await exchangeWrapper.marketSellOrdersNoThrowAsync(signedOrders, takerAddress, { - takerAssetFillAmount, + await exchangeWrapper.marketBuyOrdersNoThrowAsync(signedOrders, takerAddress, { + makerAssetFillAmount, // HACK(albrow): We need to hardcode the gas estimate here because // the Geth gas estimator doesn't work with the way we use // delegatecall and swallow errors. @@ -963,20 +1000,50 @@ describe('Exchange wrappers', () => { expect(newBalances).to.be.deep.equal(erc20Balances); }); - it('should throw when a signedOrder does not use the same makerAssetAddress', async () => { + it('should not fill a signedOrder that does not use the same makerAssetAddress', async () => { signedOrders = [ orderFactory.newSignedOrder(), + orderFactory.newSignedOrder(), orderFactory.newSignedOrder({ makerAssetData: assetProxyUtils.encodeERC20AssetData(zrxToken.address), }), - orderFactory.newSignedOrder(), ]; - return expectRevertOrAlwaysFailingTransactionAsync( - exchangeWrapper.marketBuyOrdersNoThrowAsync(signedOrders, takerAddress, { - makerAssetFillAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1000), 18), - }), - ); + const makerAssetFillAmount = Web3Wrapper.toBaseUnitAmount(new BigNumber(100000), 18); + const filledSignedOrders = signedOrders.slice(0, -1); + _.forEach(filledSignedOrders, signedOrder => { + erc20Balances[makerAddress][defaultMakerAssetAddress] = erc20Balances[makerAddress][ + defaultMakerAssetAddress + ].minus(signedOrder.makerAssetAmount); + erc20Balances[makerAddress][defaultTakerAssetAddress] = erc20Balances[makerAddress][ + defaultTakerAssetAddress + ].add(signedOrder.takerAssetAmount); + erc20Balances[makerAddress][zrxToken.address] = erc20Balances[makerAddress][zrxToken.address].minus( + signedOrder.makerFee, + ); + erc20Balances[takerAddress][defaultMakerAssetAddress] = erc20Balances[takerAddress][ + defaultMakerAssetAddress + ].add(signedOrder.makerAssetAmount); + erc20Balances[takerAddress][defaultTakerAssetAddress] = erc20Balances[takerAddress][ + defaultTakerAssetAddress + ].minus(signedOrder.takerAssetAmount); + erc20Balances[takerAddress][zrxToken.address] = erc20Balances[takerAddress][zrxToken.address].minus( + signedOrder.takerFee, + ); + erc20Balances[feeRecipientAddress][zrxToken.address] = erc20Balances[feeRecipientAddress][ + zrxToken.address + ].add(signedOrder.makerFee.add(signedOrder.takerFee)); + }); + await exchangeWrapper.marketBuyOrdersNoThrowAsync(signedOrders, takerAddress, { + makerAssetFillAmount, + // HACK(albrow): We need to hardcode the gas estimate here because + // the Geth gas estimator doesn't work with the way we use + // delegatecall and swallow errors. + gas: 600000, + }); + + const newBalances = await erc20Wrapper.getBalancesAsync(); + expect(newBalances).to.be.deep.equal(erc20Balances); }); }); diff --git a/packages/contracts/test/libraries/lib_bytes.ts b/packages/contracts/test/libraries/lib_bytes.ts index 2fefb7aeb..a31a4789c 100644 --- a/packages/contracts/test/libraries/lib_bytes.ts +++ b/packages/contracts/test/libraries/lib_bytes.ts @@ -4,6 +4,7 @@ import { BigNumber } from '@0xproject/utils'; import BN = require('bn.js'); import * as chai from 'chai'; import ethUtil = require('ethereumjs-util'); +import * as _ from 'lodash'; import { TestLibBytesContract } from '../../src/generated_contract_wrappers/test_lib_bytes'; import { artifacts } from '../../src/utils/artifacts'; @@ -27,8 +28,11 @@ describe('LibBytes', () => { const byteArrayLongerThan32BytesLastBytesSwapped = '0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abefcd'; let testAddress: string; + let testAddressB: string; const testBytes32 = '0x102030405060708090a0b0c0d0e0f0102030405060708090a0b0c0d0e0f01020'; + const testBytes32B = '0x534877abd8443578526845cdfef020047528759477fedef87346527659aced32'; const testUint256 = new BigNumber(testBytes32, 16); + const testUint256B = new BigNumber(testBytes32B, 16); let shortData: string; let shortTestBytes: string; let shortTestBytesAsBuffer: Buffer; @@ -49,6 +53,7 @@ describe('LibBytes', () => { // Setup accounts & addresses const accounts = await web3Wrapper.getAvailableAddressesAsync(); testAddress = accounts[1]; + testAddressB = accounts[2]; // Deploy LibBytes libBytes = await TestLibBytesContract.deployFrom0xArtifactAsync(artifacts.TestLibBytes, provider, txDefaults); // Verify lengths of test data @@ -86,16 +91,15 @@ describe('LibBytes', () => { await blockchainLifecycle.revertAsync(); }); - describe('popByte', () => { + describe('popLastByte', () => { it('should revert if length is 0', async () => { return expectRevertOrOtherErrorAsync( - libBytes.publicPopByte.callAsync(constants.NULL_BYTES), + libBytes.publicPopLastByte.callAsync(constants.NULL_BYTES), constants.LIB_BYTES_GREATER_THAN_ZERO_LENGTH_REQUIRED, ); }); - it('should pop the last byte from the input and return it', async () => { - const [newBytes, poppedByte] = await libBytes.publicPopByte.callAsync(byteArrayLongerThan32Bytes); + const [newBytes, poppedByte] = await libBytes.publicPopLastByte.callAsync(byteArrayLongerThan32Bytes); const expectedNewBytes = byteArrayLongerThan32Bytes.slice(0, -2); const expectedPoppedByte = `0x${byteArrayLongerThan32Bytes.slice(-2)}`; expect(newBytes).to.equal(expectedNewBytes); @@ -103,16 +107,15 @@ describe('LibBytes', () => { }); }); - describe('popAddress', () => { + describe('popLast20Bytes', () => { it('should revert if length is less than 20', async () => { return expectRevertOrOtherErrorAsync( - libBytes.publicPopAddress.callAsync(byteArrayShorterThan20Bytes), + libBytes.publicPopLast20Bytes.callAsync(byteArrayShorterThan20Bytes), constants.LIB_BYTES_GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED, ); }); - it('should pop the last 20 bytes from the input and return it', async () => { - const [newBytes, poppedAddress] = await libBytes.publicPopAddress.callAsync(byteArrayLongerThan32Bytes); + const [newBytes, poppedAddress] = await libBytes.publicPopLast20Bytes.callAsync(byteArrayLongerThan32Bytes); const expectedNewBytes = byteArrayLongerThan32Bytes.slice(0, -40); const expectedPoppedAddress = `0x${byteArrayLongerThan32Bytes.slice(-40)}`; expect(newBytes).to.equal(expectedNewBytes); @@ -128,7 +131,6 @@ describe('LibBytes', () => { ); return expect(areBytesEqual).to.be.true(); }); - it('should return true if byte arrays are equal (both arrays > 32 bytes)', async () => { const areBytesEqual = await libBytes.publicAreBytesEqual.callAsync( byteArrayLongerThan32Bytes, @@ -136,7 +138,6 @@ describe('LibBytes', () => { ); return expect(areBytesEqual).to.be.true(); }); - it('should return false if byte arrays are not equal (first array < 32 bytes, second array > 32 bytes)', async () => { const areBytesEqual = await libBytes.publicAreBytesEqual.callAsync( byteArrayShorterThan32Bytes, @@ -144,7 +145,6 @@ describe('LibBytes', () => { ); return expect(areBytesEqual).to.be.false(); }); - it('should return false if byte arrays are not equal (first array > 32 bytes, second array < 32 bytes)', async () => { const areBytesEqual = await libBytes.publicAreBytesEqual.callAsync( byteArrayLongerThan32Bytes, @@ -152,7 +152,6 @@ describe('LibBytes', () => { ); return expect(areBytesEqual).to.be.false(); }); - it('should return false if byte arrays are not equal (same length, but a byte in first word differs)', async () => { const areBytesEqual = await libBytes.publicAreBytesEqual.callAsync( byteArrayLongerThan32BytesFirstBytesSwapped, @@ -160,7 +159,6 @@ describe('LibBytes', () => { ); return expect(areBytesEqual).to.be.false(); }); - it('should return false if byte arrays are not equal (same length, but a byte in last word differs)', async () => { const areBytesEqual = await libBytes.publicAreBytesEqual.callAsync( byteArrayLongerThan32BytesLastBytesSwapped, @@ -170,15 +168,50 @@ describe('LibBytes', () => { }); }); + describe('deepCopyBytes', () => { + it('should revert if dest is shorter than source', async () => { + return expectRevertOrOtherErrorAsync( + libBytes.publicDeepCopyBytes.callAsync(byteArrayShorterThan32Bytes, byteArrayLongerThan32Bytes), + constants.LIB_BYTES_GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED, + ); + }); + it('should overwrite dest with source if source and dest have equal length', async () => { + const zeroedByteArrayLongerThan32Bytes = `0x${_.repeat('0', byteArrayLongerThan32Bytes.length - 2)}`; + const zeroedBytesAfterCopy = await libBytes.publicDeepCopyBytes.callAsync( + zeroedByteArrayLongerThan32Bytes, + byteArrayLongerThan32Bytes, + ); + return expect(zeroedBytesAfterCopy).to.be.equal(byteArrayLongerThan32Bytes); + }); + it('should overwrite the leftmost len(source) bytes of dest if dest is larger than source', async () => { + const zeroedByteArrayLongerThan32Bytes = `0x${_.repeat('0', byteArrayLongerThan32Bytes.length * 2)}`; + const zeroedBytesAfterCopy = await libBytes.publicDeepCopyBytes.callAsync( + zeroedByteArrayLongerThan32Bytes, + byteArrayLongerThan32Bytes, + ); + const copiedBytes = zeroedBytesAfterCopy.slice(0, byteArrayLongerThan32Bytes.length); + return expect(copiedBytes).to.be.equal(byteArrayLongerThan32Bytes); + }); + it('should not overwrite the rightmost bytes of dest if dest is larger than source', async () => { + const zeroedByteArrayLongerThan32Bytes = `0x${_.repeat('0', byteArrayLongerThan32Bytes.length * 2)}`; + const zeroedBytesAfterCopy = await libBytes.publicDeepCopyBytes.callAsync( + zeroedByteArrayLongerThan32Bytes, + byteArrayLongerThan32Bytes, + ); + const expectedNotCopiedBytes = zeroedByteArrayLongerThan32Bytes.slice(byteArrayLongerThan32Bytes.length); + const notCopiedBytes = zeroedBytesAfterCopy.slice(byteArrayLongerThan32Bytes.length); + return expect(notCopiedBytes).to.be.equal(expectedNotCopiedBytes); + }); + }); + describe('readAddress', () => { - it('should successfully read address when the address takes up the whole array)', async () => { + it('should successfully read address when the address takes up the whole array', async () => { const byteArray = ethUtil.addHexPrefix(testAddress); const testAddressOffset = new BigNumber(0); const address = await libBytes.publicReadAddress.callAsync(byteArray, testAddressOffset); return expect(address).to.be.equal(testAddress); }); - - it('should successfully read address when it is offset in the array)', async () => { + it('should successfully read address when it is offset in the array', async () => { const addressByteArrayBuffer = ethUtil.toBuffer(testAddress); const prefixByteArrayBuffer = ethUtil.toBuffer('0xabcdef'); const combinedByteArrayBuffer = Buffer.concat([prefixByteArrayBuffer, addressByteArrayBuffer]); @@ -187,8 +220,7 @@ describe('LibBytes', () => { const address = await libBytes.publicReadAddress.callAsync(combinedByteArray, testAddressOffset); return expect(address).to.be.equal(testAddress); }); - - it('should fail if the byte array is too short to hold an address)', async () => { + it('should fail if the byte array is too short to hold an address', async () => { const shortByteArray = '0xabcdef'; const offset = new BigNumber(0); return expectRevertOrOtherErrorAsync( @@ -196,9 +228,8 @@ describe('LibBytes', () => { constants.LIB_BYTES_GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED, ); }); - - it('should fail if the length between the offset and end of the byte array is too short to hold an address)', async () => { - const byteArray = ethUtil.addHexPrefix(testAddress); + it('should fail if the length between the offset and end of the byte array is too short to hold an address', async () => { + const byteArray = testAddress; const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength); return expectRevertOrOtherErrorAsync( libBytes.publicReadAddress.callAsync(byteArray, badOffset), @@ -207,43 +238,73 @@ describe('LibBytes', () => { }); }); - /// @TODO Implement test cases for writeAddress. Test template below. - /// Currently, the generated contract wrappers do not support this library's write methods. - /* describe('writeAddress', () => { - it('should successfully write address when the address takes up the whole array)', async () => {}); - it('should successfully write address when it is offset in the array)', async () => {}); - it('should fail if the byte array is too short to hold an address)', async () => {}); - it('should fail if the length between the offset and end of the byte array is too short to hold an address)', async () => {}); + it('should successfully write address when the address takes up the whole array', async () => { + const byteArray = testAddress; + const testAddressOffset = new BigNumber(0); + const newByteArray = await libBytes.publicWriteAddress.callAsync( + byteArray, + testAddressOffset, + testAddressB, + ); + return expect(newByteArray).to.be.equal(testAddressB); + }); + it('should successfully write address when it is offset in the array', async () => { + const addressByteArrayBuffer = ethUtil.toBuffer(testAddress); + const prefixByteArrayBuffer = ethUtil.toBuffer('0xabcdef'); + const combinedByteArrayBuffer = Buffer.concat([prefixByteArrayBuffer, addressByteArrayBuffer]); + const combinedByteArray = ethUtil.bufferToHex(combinedByteArrayBuffer); + const testAddressOffset = new BigNumber(prefixByteArrayBuffer.byteLength); + const newByteArray = await libBytes.publicWriteAddress.callAsync( + combinedByteArray, + testAddressOffset, + testAddressB, + ); + const newByteArrayBuffer = ethUtil.toBuffer(newByteArray); + const addressFromOffsetBuffer = newByteArrayBuffer.slice(prefixByteArrayBuffer.byteLength); + const addressFromOffset = ethUtil.addHexPrefix(ethUtil.bufferToHex(addressFromOffsetBuffer)); + return expect(addressFromOffset).to.be.equal(testAddressB); + }); + it('should fail if the byte array is too short to hold an address', async () => { + const offset = new BigNumber(0); + return expectRevertOrOtherErrorAsync( + libBytes.publicWriteAddress.callAsync(byteArrayShorterThan20Bytes, offset, testAddress), + constants.LIB_BYTES_GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED, + ); + }); + it('should fail if the length between the offset and end of the byte array is too short to hold an address', async () => { + const byteArray = byteArrayLongerThan32Bytes; + const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength); + return expectRevertOrOtherErrorAsync( + libBytes.publicWriteAddress.callAsync(byteArray, badOffset, testAddress), + constants.LIB_BYTES_GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED, + ); + }); }); - */ describe('readBytes32', () => { - it('should successfully read bytes32 when the bytes32 takes up the whole array)', async () => { + it('should successfully read bytes32 when the bytes32 takes up the whole array', async () => { const testBytes32Offset = new BigNumber(0); const bytes32 = await libBytes.publicReadBytes32.callAsync(testBytes32, testBytes32Offset); return expect(bytes32).to.be.equal(testBytes32); }); - it('should successfully read bytes32 when it is offset in the array)', async () => { const bytes32ByteArrayBuffer = ethUtil.toBuffer(testBytes32); const prefixByteArrayBuffer = ethUtil.toBuffer('0xabcdef'); const combinedByteArrayBuffer = Buffer.concat([prefixByteArrayBuffer, bytes32ByteArrayBuffer]); const combinedByteArray = ethUtil.bufferToHex(combinedByteArrayBuffer); - const testAddressOffset = new BigNumber(prefixByteArrayBuffer.byteLength); - const bytes32 = await libBytes.publicReadBytes32.callAsync(combinedByteArray, testAddressOffset); + const testBytes32Offset = new BigNumber(prefixByteArrayBuffer.byteLength); + const bytes32 = await libBytes.publicReadBytes32.callAsync(combinedByteArray, testBytes32Offset); return expect(bytes32).to.be.equal(testBytes32); }); - - it('should fail if the byte array is too short to hold a bytes32)', async () => { + it('should fail if the byte array is too short to hold a bytes32', async () => { const offset = new BigNumber(0); return expectRevertOrOtherErrorAsync( libBytes.publicReadBytes32.callAsync(byteArrayShorterThan32Bytes, offset), constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, ); }); - - it('should fail if the length between the offset and end of the byte array is too short to hold a bytes32)', async () => { + it('should fail if the length between the offset and end of the byte array is too short to hold a bytes32', async () => { const badOffset = new BigNumber(ethUtil.toBuffer(testBytes32).byteLength); return expectRevertOrOtherErrorAsync( libBytes.publicReadBytes32.callAsync(testBytes32, badOffset), @@ -252,19 +313,52 @@ describe('LibBytes', () => { }); }); - /// @TODO Implement test cases for writeBytes32. Test template below. - /// Currently, the generated contract wrappers do not support this library's write methods. - /* describe('writeBytes32', () => { - it('should successfully write bytes32 when the address takes up the whole array)', async () => {}); - it('should successfully write bytes32 when it is offset in the array)', async () => {}); - it('should fail if the byte array is too short to hold a bytes32)', async () => {}); - it('should fail if the length between the offset and end of the byte array is too short to hold a bytes32)', async () => {}); + it('should successfully write bytes32 when the address takes up the whole array', async () => { + const byteArray = testBytes32; + const testBytes32Offset = new BigNumber(0); + const newByteArray = await libBytes.publicWriteBytes32.callAsync( + byteArray, + testBytes32Offset, + testBytes32B, + ); + return expect(newByteArray).to.be.equal(testBytes32B); + }); + it('should successfully write bytes32 when it is offset in the array', async () => { + const bytes32ByteArrayBuffer = ethUtil.toBuffer(testBytes32); + const prefixByteArrayBuffer = ethUtil.toBuffer('0xabcdef'); + const combinedByteArrayBuffer = Buffer.concat([prefixByteArrayBuffer, bytes32ByteArrayBuffer]); + const combinedByteArray = ethUtil.bufferToHex(combinedByteArrayBuffer); + const testBytes32Offset = new BigNumber(prefixByteArrayBuffer.byteLength); + const newByteArray = await libBytes.publicWriteBytes32.callAsync( + combinedByteArray, + testBytes32Offset, + testBytes32B, + ); + const newByteArrayBuffer = ethUtil.toBuffer(newByteArray); + const bytes32FromOffsetBuffer = newByteArrayBuffer.slice(prefixByteArrayBuffer.byteLength); + const bytes32FromOffset = ethUtil.addHexPrefix(ethUtil.bufferToHex(bytes32FromOffsetBuffer)); + return expect(bytes32FromOffset).to.be.equal(testBytes32B); + }); + it('should fail if the byte array is too short to hold a bytes32', async () => { + const offset = new BigNumber(0); + return expectRevertOrOtherErrorAsync( + libBytes.publicWriteBytes32.callAsync(byteArrayShorterThan32Bytes, offset, testBytes32), + constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, + ); + }); + it('should fail if the length between the offset and end of the byte array is too short to hold a bytes32', async () => { + const byteArray = byteArrayLongerThan32Bytes; + const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength); + return expectRevertOrOtherErrorAsync( + libBytes.publicWriteBytes32.callAsync(byteArray, badOffset, testBytes32), + constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, + ); + }); }); - */ describe('readUint256', () => { - it('should successfully read uint256 when the uint256 takes up the whole array)', async () => { + it('should successfully read uint256 when the uint256 takes up the whole array', async () => { const formattedTestUint256 = new BN(testUint256.toString(10)); const testUint256AsBuffer = ethUtil.toBuffer(formattedTestUint256); const byteArray = ethUtil.bufferToHex(testUint256AsBuffer); @@ -272,8 +366,7 @@ describe('LibBytes', () => { const uint256 = await libBytes.publicReadUint256.callAsync(byteArray, testUint256Offset); return expect(uint256).to.bignumber.equal(testUint256); }); - - it('should successfully read uint256 when it is offset in the array)', async () => { + it('should successfully read uint256 when it is offset in the array', async () => { const prefixByteArrayBuffer = ethUtil.toBuffer('0xabcdef'); const formattedTestUint256 = new BN(testUint256.toString(10)); const testUint256AsBuffer = ethUtil.toBuffer(formattedTestUint256); @@ -283,16 +376,14 @@ describe('LibBytes', () => { const uint256 = await libBytes.publicReadUint256.callAsync(combinedByteArray, testUint256Offset); return expect(uint256).to.bignumber.equal(testUint256); }); - - it('should fail if the byte array is too short to hold a uint256)', async () => { + it('should fail if the byte array is too short to hold a uint256', async () => { const offset = new BigNumber(0); return expectRevertOrOtherErrorAsync( libBytes.publicReadUint256.callAsync(byteArrayShorterThan32Bytes, offset), constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, ); }); - - it('should fail if the length between the offset and end of the byte array is too short to hold a uint256)', async () => { + it('should fail if the length between the offset and end of the byte array is too short to hold a uint256', async () => { const formattedTestUint256 = new BN(testUint256.toString(10)); const testUint256AsBuffer = ethUtil.toBuffer(formattedTestUint256); const byteArray = ethUtil.bufferToHex(testUint256AsBuffer); @@ -304,16 +395,53 @@ describe('LibBytes', () => { }); }); - /// @TODO Implement test cases for writeUint256. Test template below. - /// Currently, the generated contract wrappers do not support this library's write methods. - /* describe('writeUint256', () => { - it('should successfully write uint256 when the address takes up the whole array)', async () => {}); - it('should successfully write uint256 when it is offset in the array)', async () => {}); - it('should fail if the byte array is too short to hold a uint256)', async () => {}); - it('should fail if the length between the offset and end of the byte array is too short to hold a uint256)', async () => {}); + it('should successfully write uint256 when the address takes up the whole array', async () => { + const byteArray = testBytes32; + const testUint256Offset = new BigNumber(0); + const newByteArray = await libBytes.publicWriteUint256.callAsync( + byteArray, + testUint256Offset, + testUint256B, + ); + const newByteArrayAsUint256 = new BigNumber(newByteArray, 16); + return expect(newByteArrayAsUint256).to.be.bignumber.equal(testUint256B); + }); + it('should successfully write uint256 when it is offset in the array', async () => { + const bytes32ByteArrayBuffer = ethUtil.toBuffer(testBytes32); + const prefixByteArrayBuffer = ethUtil.toBuffer('0xabcdef'); + const combinedByteArrayBuffer = Buffer.concat([prefixByteArrayBuffer, bytes32ByteArrayBuffer]); + const combinedByteArray = ethUtil.bufferToHex(combinedByteArrayBuffer); + const testUint256Offset = new BigNumber(prefixByteArrayBuffer.byteLength); + const newByteArray = await libBytes.publicWriteUint256.callAsync( + combinedByteArray, + testUint256Offset, + testUint256B, + ); + const newByteArrayBuffer = ethUtil.toBuffer(newByteArray); + const uint256FromOffsetBuffer = newByteArrayBuffer.slice(prefixByteArrayBuffer.byteLength); + const uint256FromOffset = new BigNumber( + ethUtil.addHexPrefix(ethUtil.bufferToHex(uint256FromOffsetBuffer)), + 16, + ); + return expect(uint256FromOffset).to.be.bignumber.equal(testUint256B); + }); + it('should fail if the byte array is too short to hold a uint256', async () => { + const offset = new BigNumber(0); + return expectRevertOrOtherErrorAsync( + libBytes.publicWriteUint256.callAsync(byteArrayShorterThan32Bytes, offset, testUint256), + constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, + ); + }); + it('should fail if the length between the offset and end of the byte array is too short to hold a uint256', async () => { + const byteArray = byteArrayLongerThan32Bytes; + const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength); + return expectRevertOrOtherErrorAsync( + libBytes.publicWriteUint256.callAsync(byteArray, badOffset, testUint256), + constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, + ); + }); }); - */ describe('readFirst4', () => { // AssertionError: expected promise to be rejected with an error including 'revert' but it was fulfilled with '0x08c379a0' @@ -337,7 +465,6 @@ describe('LibBytes', () => { const bytes = await libBytes.publicReadBytes.callAsync(shortTestBytes, testBytesOffset); return expect(bytes).to.be.equal(shortData); }); - it('should successfully read short, nested array of bytes when it is offset in the array', async () => { const prefixByteArrayBuffer = ethUtil.toBuffer('0xabcdef'); const combinedByteArrayBuffer = Buffer.concat([prefixByteArrayBuffer, shortTestBytesAsBuffer]); @@ -346,13 +473,11 @@ describe('LibBytes', () => { const bytes = await libBytes.publicReadBytes.callAsync(combinedByteArray, testUint256Offset); return expect(bytes).to.be.equal(shortData); }); - it('should successfully read a nested array of bytes - one word in length - when it takes up the whole array', async () => { const testBytesOffset = new BigNumber(0); const bytes = await libBytes.publicReadBytes.callAsync(wordOfTestBytes, testBytesOffset); return expect(bytes).to.be.equal(wordOfData); }); - it('should successfully read a nested array of bytes - one word in length - when it is offset in the array', async () => { const prefixByteArrayBuffer = ethUtil.toBuffer('0xabcdef'); const combinedByteArrayBuffer = Buffer.concat([prefixByteArrayBuffer, wordOfTestBytesAsBuffer]); @@ -361,13 +486,11 @@ describe('LibBytes', () => { const bytes = await libBytes.publicReadBytes.callAsync(combinedByteArray, testUint256Offset); return expect(bytes).to.be.equal(wordOfData); }); - it('should successfully read long, nested array of bytes when it takes up the whole array', async () => { const testBytesOffset = new BigNumber(0); const bytes = await libBytes.publicReadBytes.callAsync(longTestBytes, testBytesOffset); return expect(bytes).to.be.equal(longData); }); - it('should successfully read long, nested array of bytes when it is offset in the array', async () => { const prefixByteArrayBuffer = ethUtil.toBuffer('0xabcdef'); const combinedByteArrayBuffer = Buffer.concat([prefixByteArrayBuffer, longTestBytesAsBuffer]); @@ -376,7 +499,6 @@ describe('LibBytes', () => { const bytes = await libBytes.publicReadBytes.callAsync(combinedByteArray, testUint256Offset); return expect(bytes).to.be.equal(longData); }); - it('should fail if the byte array is too short to hold the length of a nested byte array', async () => { // The length of the nested array is 32 bytes. By storing less than 32 bytes, a length cannot be read. const offset = new BigNumber(0); @@ -385,7 +507,6 @@ describe('LibBytes', () => { constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, ); }); - it('should fail if we store a nested byte array length, without a nested byte array', async () => { const offset = new BigNumber(0); return expectRevertOrOtherErrorAsync( @@ -393,7 +514,6 @@ describe('LibBytes', () => { constants.LIB_BYTES_GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED, ); }); - it('should fail if the length between the offset and end of the byte array is too short to hold the length of a nested byte array', async () => { const badOffset = new BigNumber(ethUtil.toBuffer(byteArrayShorterThan32Bytes).byteLength); return expectRevertOrOtherErrorAsync( @@ -401,7 +521,6 @@ describe('LibBytes', () => { constants.LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED, ); }); - it('should fail if the length between the offset and end of the byte array is too short to hold the nested byte array', async () => { const badOffset = new BigNumber(ethUtil.toBuffer(testBytes32).byteLength); return expectRevertOrOtherErrorAsync( @@ -419,7 +538,6 @@ describe('LibBytes', () => { const bytesRead = await libBytes.publicReadBytes.callAsync(bytesWritten, testBytesOffset); return expect(bytesRead).to.be.equal(shortData); }); - it('should successfully write short, nested array of bytes when it is offset in the array', async () => { // Write a prefix to the array const prefixData = '0xabcdef'; @@ -436,7 +554,6 @@ describe('LibBytes', () => { const bytes = await libBytes.publicReadBytes.callAsync(bytesWritten, testBytesOffset); return expect(bytes).to.be.equal(shortData); }); - it('should successfully write a nested array of bytes - one word in length - when it takes up the whole array', async () => { const testBytesOffset = new BigNumber(0); const emptyByteArray = ethUtil.bufferToHex(new Buffer(wordOfTestBytesAsBuffer.byteLength)); @@ -444,7 +561,6 @@ describe('LibBytes', () => { const bytesRead = await libBytes.publicReadBytes.callAsync(bytesWritten, testBytesOffset); return expect(bytesRead).to.be.equal(wordOfData); }); - it('should successfully write a nested array of bytes - one word in length - when it is offset in the array', async () => { // Write a prefix to the array const prefixData = '0xabcdef'; @@ -461,7 +577,6 @@ describe('LibBytes', () => { const bytes = await libBytes.publicReadBytes.callAsync(bytesWritten, testBytesOffset); return expect(bytes).to.be.equal(wordOfData); }); - it('should successfully write a long, nested bytes when it takes up the whole array', async () => { const testBytesOffset = new BigNumber(0); const emptyByteArray = ethUtil.bufferToHex(new Buffer(longTestBytesAsBuffer.byteLength)); @@ -469,7 +584,6 @@ describe('LibBytes', () => { const bytesRead = await libBytes.publicReadBytes.callAsync(bytesWritten, testBytesOffset); return expect(bytesRead).to.be.equal(longData); }); - it('should successfully write long, nested array of bytes when it is offset in the array', async () => { // Write a prefix to the array const prefixData = '0xabcdef'; @@ -486,7 +600,6 @@ describe('LibBytes', () => { const bytes = await libBytes.publicReadBytes.callAsync(bytesWritten, testBytesOffset); return expect(bytes).to.be.equal(longData); }); - it('should fail if the byte array is too short to hold the length of a nested byte array', async () => { const offset = new BigNumber(0); const emptyByteArray = ethUtil.bufferToHex(new Buffer(1)); @@ -495,7 +608,6 @@ describe('LibBytes', () => { constants.LIB_BYTES_GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED, ); }); - it('should fail if the length between the offset and end of the byte array is too short to hold the length of a nested byte array)', async () => { const emptyByteArray = ethUtil.bufferToHex(new Buffer(shortTestBytesAsBuffer.byteLength)); const badOffset = new BigNumber(ethUtil.toBuffer(shortTestBytesAsBuffer).byteLength); |