diff options
6 files changed, 223 insertions, 32 deletions
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 <b> 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 <input> into <b> memcpy( - getMemAddress(b) + index, - getMemAddress(input), - input.length + 32 /* 32 bytes to store length */ + getMemAddress(b) + index + 32, // +32 to skip length of <b> + getMemAddress(input), // include length of byte array + input.length + 32 // +32 bytes to store length ); } } diff --git a/packages/contracts/test/asset_proxy/decoder.ts b/packages/contracts/test/asset_proxy/decoder.ts index 4416334d1..0f1413ff1 100644 --- a/packages/contracts/test/asset_proxy/decoder.ts +++ b/packages/contracts/test/asset_proxy/decoder.ts @@ -56,21 +56,45 @@ describe('LibAssetProxyDecoder', () => { expect(decodedTokenAddress).to.be.equal(expectedDecodedProxyData.tokenAddress); }); - it('should correctly decode ERC721 proxy data)', async () => { + it('should correctly decode ERC721 proxy data', async () => { const tokenId = ZeroEx.generatePseudoRandomSalt(); const encodedProxyData = assetProxyUtils.encodeERC721ProxyData(testAddress, tokenId); const expectedDecodedProxyData = assetProxyUtils.decodeERC721ProxyData(encodedProxyData); let decodedAssetProxyId: number; let decodedTokenAddress: string; let decodedTokenId: BigNumber; + let decodedData: string; [ decodedAssetProxyId, decodedTokenAddress, decodedTokenId, + decodedData, ] = await testAssetProxyDecoder.publicDecodeERC721Data.callAsync(encodedProxyData); expect(decodedAssetProxyId).to.be.equal(expectedDecodedProxyData.assetProxyId); expect(decodedTokenAddress).to.be.equal(expectedDecodedProxyData.tokenAddress); expect(decodedTokenId).to.be.bignumber.equal(expectedDecodedProxyData.tokenId); + expect(decodedData).to.be.equal(expectedDecodedProxyData.data); + }); + + it('should correctly decode ERC721 proxy data with receiver data', async () => { + const tokenId = ZeroEx.generatePseudoRandomSalt(); + const data = ethUtil.bufferToHex(assetProxyUtils.encodeUint256(ZeroEx.generatePseudoRandomSalt())) + 'FFFF'; + const encodedProxyData = assetProxyUtils.encodeERC721ProxyData(testAddress, tokenId, data); + const expectedDecodedProxyData = assetProxyUtils.decodeERC721ProxyData(encodedProxyData); + let decodedAssetProxyId: number; + let decodedTokenAddress: string; + let decodedTokenId: BigNumber; + let decodedData: string; + [ + decodedAssetProxyId, + decodedTokenAddress, + decodedTokenId, + decodedData, + ] = await testAssetProxyDecoder.publicDecodeERC721Data.callAsync(encodedProxyData); + expect(decodedAssetProxyId).to.be.equal(expectedDecodedProxyData.assetProxyId); + expect(decodedTokenAddress).to.be.equal(expectedDecodedProxyData.tokenAddress); + expect(decodedTokenId).to.be.bignumber.equal(expectedDecodedProxyData.tokenId); + expect(decodedData).to.be.equal(expectedDecodedProxyData.data); }); }); }); diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts index d7f27deb4..4995e95a0 100644 --- a/packages/contracts/test/asset_proxy/proxies.ts +++ b/packages/contracts/test/asset_proxy/proxies.ts @@ -261,7 +261,7 @@ describe('Asset Transfer Proxies', () => { }); }); - describe.only('Transfer Proxy - ERC721', () => { + describe('Transfer Proxy - ERC721', () => { describe('transferFrom', () => { it('should successfully transfer tokens', async () => { // Construct metadata for ERC721 proxy @@ -289,7 +289,7 @@ describe('Asset Transfer Proxies', () => { expect(newOwnerMakerAsset).to.be.bignumber.equal(takerAddress); }); - it.only('should call onERC721Received when transferring to a smart contract', async () => { + it('should call onERC721Received when transferring to a smart contract', async () => { // Construct metadata for ERC721 proxy const encodedProxyMetadata = assetProxyUtils.encodeERC721ProxyData( erc721Token.address, @@ -326,9 +326,11 @@ describe('Asset Transfer Proxies', () => { it('should call onERC721Received when transferring to a smart contract and receive extra data', async () => { // Construct metadata for ERC721 proxy + const data = ethUtil.bufferToHex(assetProxyUtils.encodeUint256(ZeroEx.generatePseudoRandomSalt())); const encodedProxyMetadata = assetProxyUtils.encodeERC721ProxyData( erc721Token.address, erc721MakerTokenId, + data, ); // Verify pre-condition const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); @@ -348,14 +350,17 @@ describe('Asset Transfer Proxies', () => { tx.logs = _.filter(tx.logs, log => log.address === erc721Receiver.address); const logDecoder = new LogDecoder(constants.TESTRPC_NETWORK_ID); tx.logs = _.map(tx.logs, log => logDecoder.decodeLogOrThrow(log)); - // Verify erc721 receiver log emitted - console.log(tx.logs); + // Validate log emitted by erc721 receiver + expect(tx.logs.length).to.be.equal(1); + const tokenReceivedLog = tx.logs[0] as LogWithDecodedArgs<TokenReceivedContractEventArgs>; + expect(tokenReceivedLog.args.from).to.be.equal(makerAddress); + expect(tokenReceivedLog.args.tokenId).to.be.bignumber.equal(erc721MakerTokenId); + expect(tokenReceivedLog.args.data).to.be.equal(data); // Verify transfer was successful const newOwnerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); expect(newOwnerMakerAsset).to.be.bignumber.equal(erc721Receiver.address); }); - /* it('should throw if receiving contract does not have onERC721Received', async () => { // Construct metadata for ERC721 proxy const encodedProxyMetadata = assetProxyUtils.encodeERC721ProxyData( @@ -368,17 +373,16 @@ describe('Asset Transfer Proxies', () => { // Perform a transfer from makerAddress to takerAddress const erc20Balances = await erc20Wrapper.getBalancesAsync(); const amount = new BigNumber(1); - await erc721Proxy.transferFrom.sendTransactionAsync( - encodedProxyMetadata, - makerAddress, - takerAddress, - amount, - { from: exchangeAddress }, - ); - // Verify transfer was successful - const newOwnerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); - expect(newOwnerMakerAsset).to.be.bignumber.equal(takerAddress); - });*/ + return expect( + erc721Proxy.transferFrom.sendTransactionAsync( + encodedProxyMetadata, + makerAddress, + erc20Proxy.address, // the ERC20 proxy does not have an ERC721 receiver + amount, + { from: exchangeAddress }, + ), + ).to.be.rejectedWith(constants.REVERT); + }); it('should throw if transferring 0 amount of a token', async () => { // Construct metadata for ERC721 proxy diff --git a/packages/contracts/test/libraries/lib_bytes.ts b/packages/contracts/test/libraries/lib_bytes.ts index 0996cdc84..a6e3c7d37 100644 --- a/packages/contracts/test/libraries/lib_bytes.ts +++ b/packages/contracts/test/libraries/lib_bytes.ts @@ -6,7 +6,8 @@ import ethUtil = require('ethereumjs-util'); import { TestLibBytesContract } from '../../src/generated_contract_wrappers/test_lib_bytes'; import { artifacts } from '../../src/utils/artifacts'; -import { expectRevertOrOtherErrorAsync } from '../../src/utils/assertions'; +import { expectRevertOrOtherErrorAsync, expectRevertOrOtherErrorAsync } from '../../src/utils/assertions'; +import { assetProxyUtils } from '../../src/utils/asset_proxy_utils'; import { chaiSetup } from '../../src/utils/chai_setup'; import { constants } from '../../src/utils/constants'; import { provider, txDefaults, web3Wrapper } from '../../src/utils/web3_wrapper'; @@ -28,6 +29,15 @@ describe('LibBytes', () => { let testAddress: string; const testBytes32 = '0x102030405060708090a0b0c0d0e0f0102030405060708090a0b0c0d0e0f01020'; const testUint256 = new BigNumber(testBytes32, 16); + let shortData: string; + let shortTestBytes: string; + let shortTestBytesAsBuffer: Buffer; + let wordOfData: string; + let wordOfTestBytes: string; + let wordOfTestBytesAsBuffer: Buffer; + let longData: string; + let longTestBytes: string; + let longTestBytesAsBuffer: Buffer; before(async () => { await blockchainLifecycle.startAsync(); @@ -48,6 +58,26 @@ describe('LibBytes', () => { expect(byteArrayLongerThan32BytesLength).to.be.greaterThan(32); const testBytes32Length = ethUtil.toBuffer(testBytes32).byteLength; expect(testBytes32Length).to.be.equal(32); + // Create short test bytes + shortData = '0xffffaa'; + const encodedShortData = ethUtil.toBuffer(shortData); + const shortDataLength = new BigNumber(encodedShortData.byteLength); + const encodedShortDataLength = assetProxyUtils.encodeUint256(shortDataLength); + shortTestBytesAsBuffer = Buffer.concat([encodedShortDataLength, encodedShortData]); + shortTestBytes = ethUtil.bufferToHex(shortTestBytesAsBuffer); + // Create test bytes one word in length + wordOfData = ethUtil.bufferToHex(assetProxyUtils.encodeUint256(ZeroEx.generatePseudoRandomSalt())); + const encodedWordOfData = ethUtil.toBuffer(wordOfData); + const wordOfDataLength = new BigNumber(encodedWordOfData.byteLength); + const encodedWordOfDataLength = assetProxyUtils.encodeUint256(wordOfDataLength); + wordOfTestBytesAsBuffer = Buffer.concat([encodedWordOfDataLength, encodedWordOfData]); + wordOfTestBytes = ethUtil.bufferToHex(wordOfTestBytesAsBuffer); + // Create long test bytes (combines short test bytes with word of test bytes) + longData = ethUtil.bufferToHex(Buffer.concat([encodedShortData, encodedWordOfData])); + const longDataLength = new BigNumber(encodedShortData.byteLength + encodedWordOfData.byteLength); + const encodedLongDataLength = assetProxyUtils.encodeUint256(longDataLength); + longTestBytesAsBuffer = Buffer.concat([encodedLongDataLength, encodedShortData, encodedWordOfData]); + longTestBytes = ethUtil.bufferToHex(longTestBytesAsBuffer); }); beforeEach(async () => { await blockchainLifecycle.startAsync(); @@ -285,6 +315,7 @@ describe('LibBytes', () => { }); */ +======= describe('readFirst4', () => { // AssertionError: expected promise to be rejected with an error including 'revert' but it was fulfilled with '0x08c379a0' it('should revert if byte array has a length < 4', async () => { @@ -300,4 +331,92 @@ describe('LibBytes', () => { expect(first4Bytes).to.equal(expectedFirst4Bytes); }); }); + + describe('readBytes', () => { + it('should successfully read short, nested array of bytes when it takes up the whole array', async () => { + const testBytesOffset = new BigNumber(0); + 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 shortDataAsBuffer = ethUtil.toBuffer(shortData); + const combinedByteArrayBuffer = Buffer.concat([prefixByteArrayBuffer, shortTestBytesAsBuffer]); + const combinedByteArray = ethUtil.bufferToHex(combinedByteArrayBuffer); + const testUint256Offset = new BigNumber(prefixByteArrayBuffer.byteLength); + 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 wordOfDataAsBuffer = ethUtil.toBuffer(wordOfData); + const combinedByteArrayBuffer = Buffer.concat([prefixByteArrayBuffer, wordOfTestBytesAsBuffer]); + const combinedByteArray = ethUtil.bufferToHex(combinedByteArrayBuffer); + const testUint256Offset = new BigNumber(prefixByteArrayBuffer.byteLength); + 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 longDataAsBuffer = ethUtil.toBuffer(longData); + const combinedByteArrayBuffer = Buffer.concat([prefixByteArrayBuffer, longTestBytesAsBuffer]); + const combinedByteArray = ethUtil.bufferToHex(combinedByteArrayBuffer); + const testUint256Offset = new BigNumber(prefixByteArrayBuffer.byteLength); + 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); + return expect(libBytes.publicReadBytes.callAsync(byteArrayShorterThan32Bytes, offset)).to.be.rejectedWith( + constants.REVERT, + ); + }); + + it('should fail if we store a nested byte array length, without a nested byte array)', async () => { + const offset = new BigNumber(0); + return expect(libBytes.publicReadBytes.callAsync(testBytes32, offset)).to.be.rejectedWith(constants.REVERT); + }); + + 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 expect( + libBytes.publicReadBytes.callAsync(byteArrayShorterThan32Bytes, badOffset), + ).to.be.rejectedWith(constants.REVERT); + }); + + 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 expect(libBytes.publicReadBytes.callAsync(testBytes32, badOffset)).to.be.rejectedWith( + constants.REVERT, + ); + }); + }); + + /// @TODO Implement test cases for writeUint256. Test template below. + /// Currently, the generated contract wrappers do not support this library's write methods. + /* + describe('writeBytes', () => { + it('should successfully write bytes when it takes up the whole array)', async () => {}); + it('should successfully write bytes when it is offset in the array)', async () => {}); + it('should fail if the byte array is too short to hold the nested bytes)', async () => {}); + it('should fail if the length between the offset and end of the byte array is too short to hold the nested bytes)', async () => {}); + }); + */ }); diff --git a/packages/order-utils/src/asset_proxy_utils.ts b/packages/order-utils/src/asset_proxy_utils.ts index 55f2d56df..8255376a1 100644 --- a/packages/order-utils/src/asset_proxy_utils.ts +++ b/packages/order-utils/src/asset_proxy_utils.ts @@ -2,6 +2,7 @@ import { AssetProxyId, ERC20ProxyData, ERC721ProxyData, ProxyData } from '@0xpro import { BigNumber } from '@0xproject/utils'; import BN = require('bn.js'); import ethUtil = require('ethereumjs-util'); +import * as _ from 'lodash'; const ERC20_PROXY_METADATA_BYTE_LENGTH = 21; const ERC721_PROXY_METADATA_BYTE_LENGTH = 53; @@ -74,19 +75,25 @@ export const assetProxyUtils = { }; return erc20ProxyData; }, - encodeERC721ProxyData(tokenAddress: string, tokenId: BigNumber): string { + encodeERC721ProxyData(tokenAddress: string, tokenId: BigNumber, data?: string): string { const encodedAssetProxyId = assetProxyUtils.encodeAssetProxyId(AssetProxyId.ERC721); const encodedAddress = assetProxyUtils.encodeAddress(tokenAddress); const encodedTokenId = assetProxyUtils.encodeUint256(tokenId); const encodedMetadata = Buffer.concat([encodedAddress, encodedTokenId, encodedAssetProxyId]); + if (!_.isUndefined(data)) { + const encodedData = ethUtil.toBuffer(data); + const dataLength = new BigNumber(encodedData.byteLength); + const encodedDataLength = assetProxyUtils.encodeUint256(dataLength); + encodedMetadata = Buffer.concat([encodedMetadata, encodedDataLength, encodedData]); + } const encodedMetadataHex = ethUtil.bufferToHex(encodedMetadata); return encodedMetadataHex; }, decodeERC721ProxyData(proxyData: string): ERC721ProxyData { const encodedProxyMetadata = ethUtil.toBuffer(proxyData); - if (encodedProxyMetadata.byteLength !== ERC721_PROXY_METADATA_BYTE_LENGTH) { + if (encodedProxyMetadata.byteLength < ERC721_PROXY_METADATA_BYTE_LENGTH) { throw new Error( - `Could not decode ERC20 Proxy Data. Expected length of encoded data to be 53. Got ${ + `Could not decode ERC20 Proxy Data. Expected length of encoded data to be at least 53. Got ${ encodedProxyMetadata.byteLength }`, ); @@ -106,10 +113,25 @@ export const assetProxyUtils = { const tokenIdOffset = ERC721_PROXY_METADATA_BYTE_LENGTH - 1; const encodedTokenId = encodedProxyMetadata.slice(addressOffset, tokenIdOffset); const tokenId = assetProxyUtils.decodeUint256(encodedTokenId); - const erc721ProxyData = { + const nullData = '0x'; + let data = nullData; + if (encodedProxyMetadata.byteLength > 53) { + const encodedDataLength = encodedProxyMetadata.slice(53, 85); + const dataLength = assetProxyUtils.decodeUint256(encodedDataLength); + const expectedDataLength = new BigNumber(encodedProxyMetadata.byteLength - 85); + if (!dataLength.equals(expectedDataLength)) { + throw new Error( + `Data length (${dataLength}) does not match actual length of data (${expectedDataLength})`, + ); + } + const encodedData = encodedProxyMetadata.slice(85); + data = ethUtil.bufferToHex(encodedData); + } + const erc721ProxyData: ERC721ProxyData = { assetProxyId, tokenAddress, tokenId, + data, }; return erc721ProxyData; }, |