diff options
8 files changed, 34 insertions, 53 deletions
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index 96950f1cd..4e9ae64f8 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -54,8 +54,6 @@ contract ERC20Proxy is ) = decodeERC20AssetData(assetData); // Data must be intended for this proxy. - uint256 length = assetMetadata.length; - require( proxyId == PROXY_ID, PROXY_ID_MISMATCH @@ -92,14 +90,15 @@ contract ERC20Proxy is ) { // Validate encoded data length + uint256 length = assetData.length; require( assetData.length == 21, INVALID_ASSET_DATA_LENGTH ); // Decode data - proxyId = uint8(assetData[0]); - token = readAddress(assetData, 1); + token = readAddress(assetData, 0); + proxyId = uint8(assetData[length-1]); return (proxyId, token); } diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index 102064c15..f6c3af104 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -56,15 +56,8 @@ contract ERC721Proxy is bytes memory receiverData ) = decodeERC721AssetData(assetData); - // Data must be intended for this proxy. - uint256 length = assetMetadata.length; - require( - length == 53, - LENGTH_53_REQUIRED - ); - - // TODO: Is this too inflexible in the future? + // Data must be intended for this proxy. require( proxyId == PROXY_ID, PROXY_ID_MISMATCH @@ -113,18 +106,19 @@ contract ERC721Proxy is ) { // Validate encoded data length + uint256 length = assetData.length; require( assetData.length >= 53, INVALID_ASSET_DATA_LENGTH ); // Decode asset data. - proxyId = uint8(assetData[0]); - token = readAddress(assetData, 1); - tokenId = readUint256(assetData, 21); + token = readAddress(assetData, 0); + tokenId = readUint256(assetData, 20); if (assetData.length > 53) { - receiverData = readBytes(assetData, 53); + receiverData = readBytes(assetData, 52); } + proxyId = uint8(assetData[length-1]); return ( proxyId, diff --git a/packages/contracts/test/asset_proxy/decoder.ts b/packages/contracts/test/asset_proxy/decoder.ts index e395c04c1..6336f69ae 100644 --- a/packages/contracts/test/asset_proxy/decoder.ts +++ b/packages/contracts/test/asset_proxy/decoder.ts @@ -1,9 +1,10 @@ -import { LogWithDecodedArgs, TransactionReceiptWithDecodedLogs, ZeroEx } from '0x.js'; import { BlockchainLifecycle, devConstants, web3Factory } from '@0xproject/dev-utils'; +import { generatePseudoRandomSalt } from '@0xproject/order-utils'; import { BigNumber } from '@0xproject/utils'; import { Web3Wrapper } from '@0xproject/web3-wrapper'; import BN = require('bn.js'); import * as chai from 'chai'; +import { LogWithDecodedArgs, TransactionReceiptWithDecodedLogs } from 'ethereum-types'; import ethUtil = require('ethereumjs-util'); import * as Web3 from 'web3'; @@ -57,7 +58,7 @@ describe('TestAssetDataDecoders', () => { }); it('should correctly decode ERC721 asset data', async () => { - const tokenId = ZeroEx.generatePseudoRandomSalt(); + const tokenId = generatePseudoRandomSalt(); const encodedAssetData = assetProxyUtils.encodeERC721AssetData(testAddress, tokenId); const expectedDecodedAssetData = assetProxyUtils.decodeERC721AssetData(encodedAssetData); let decodedAssetProxyId: number; @@ -77,9 +78,9 @@ describe('TestAssetDataDecoders', () => { }); it('should correctly decode ERC721 asset data with receiver data', async () => { - const tokenId = ZeroEx.generatePseudoRandomSalt(); + const tokenId = generatePseudoRandomSalt(); const receiverData = - ethUtil.bufferToHex(assetProxyUtils.encodeUint256(ZeroEx.generatePseudoRandomSalt())) + 'FFFF'; + ethUtil.bufferToHex(assetProxyUtils.encodeUint256(generatePseudoRandomSalt())) + 'FFFF'; const encodedAssetData = assetProxyUtils.encodeERC721AssetData(testAddress, tokenId, receiverData); const expectedDecodedAssetData = assetProxyUtils.decodeERC721AssetData(encodedAssetData); let decodedAssetProxyId: number; diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts index e8c598935..6dc652383 100644 --- a/packages/contracts/test/asset_proxy/proxies.ts +++ b/packages/contracts/test/asset_proxy/proxies.ts @@ -1,8 +1,10 @@ -import { LogWithDecodedArgs, ZeroEx } from '0x.js'; import { BlockchainLifecycle } from '@0xproject/dev-utils'; import { assetProxyUtils } from '@0xproject/order-utils'; +import { generatePseudoRandomSalt } from '@0xproject/order-utils'; +import { AssetProxyId } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import * as chai from 'chai'; +import { LogWithDecodedArgs } from 'ethereum-types'; import ethUtil = require('ethereumjs-util'); import * as _ from 'lodash'; @@ -82,20 +84,11 @@ describe('Asset Transfer Proxies', () => { }), constants.AWAIT_TRANSACTION_MINED_MS, ); - - await erc721Proxy.addAuthorizedAddress.sendTransactionAsync(exchangeAddress, { - from: owner, - }); - erc721Receiver = await DummyERC721ReceiverContract.deployFrom0xArtifactAsync( artifacts.DummyERC721Receiver, provider, txDefaults, ); - - zeroEx = new ZeroEx(provider, { - networkId: constants.TESTRPC_NETWORK_ID, - }); }); beforeEach(async () => { await blockchainLifecycle.startAsync(); @@ -298,10 +291,8 @@ describe('Asset Transfer Proxies', () => { ); // Parse transaction logs - const tx = await zeroEx.awaitTransactionMinedAsync(txHash); - 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)); + const logDecoder = new LogDecoder(web3Wrapper, erc721Receiver.address); + const tx = await logDecoder.getTxWithDecodedLogsAsync(txHash); // Verify that no log was emitted by erc721 receiver expect(tx.logs.length).to.be.equal(0); // Verify transfer was successful @@ -311,9 +302,7 @@ describe('Asset Transfer Proxies', () => { it('should call onERC721Received when transferring to a smart contract with receiver data', async () => { // Construct ERC721 asset data - const receiverData = ethUtil.bufferToHex( - assetProxyUtils.encodeUint256(ZeroEx.generatePseudoRandomSalt()), - ); + const receiverData = ethUtil.bufferToHex(assetProxyUtils.encodeUint256(generatePseudoRandomSalt())); const encodedAssetData = assetProxyUtils.encodeERC721AssetData( erc721Token.address, erc721MakerTokenId, @@ -333,10 +322,8 @@ describe('Asset Transfer Proxies', () => { { from: exchangeAddress }, ); // Parse transaction logs - const tx = await zeroEx.awaitTransactionMinedAsync(txHash); - 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)); + const logDecoder = new LogDecoder(web3Wrapper, erc721Receiver.address); + const tx = await logDecoder.getTxWithDecodedLogsAsync(txHash); // Validate log emitted by erc721 receiver expect(tx.logs.length).to.be.equal(1); const tokenReceivedLog = tx.logs[0] as LogWithDecodedArgs<TokenReceivedContractEventArgs>; @@ -350,9 +337,7 @@ describe('Asset Transfer Proxies', () => { it('should throw if there is receiver data but contract does not have onERC721Received', async () => { // Construct ERC721 asset data - const receiverData = ethUtil.bufferToHex( - assetProxyUtils.encodeUint256(ZeroEx.generatePseudoRandomSalt()), - ); + const receiverData = ethUtil.bufferToHex(assetProxyUtils.encodeUint256(generatePseudoRandomSalt())); const encodedAssetData = assetProxyUtils.encodeERC721AssetData( erc721Token.address, erc721MakerTokenId, diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index 8d3c3a940..53b98c755 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -845,7 +845,7 @@ describe('Exchange core', () => { const makerAssetId = erc721MakerAssetIds[0]; signedOrder = orderFactory.newSignedOrder({ makerAssetAmount: new BigNumber(1), - takerAssetAmount: ZeroEx.toBaseUnitAmount(new BigNumber(100), 18), + takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), makerAssetData: assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerAssetId), takerAssetData: assetProxyUtils.encodeERC20AssetData(defaultTakerAssetAddress), }); @@ -885,7 +885,7 @@ describe('Exchange core', () => { const takerAssetId = erc721TakerAssetIds[0]; signedOrder = orderFactory.newSignedOrder({ takerAssetAmount: new BigNumber(1), - makerAssetAmount: ZeroEx.toBaseUnitAmount(new BigNumber(100), 18), + makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 18), takerAssetData: assetProxyUtils.encodeERC721AssetData(erc721Token.address, takerAssetId), makerAssetData: assetProxyUtils.encodeERC20AssetData(defaultMakerAssetAddress), }); diff --git a/packages/contracts/test/exchange/transactions.ts b/packages/contracts/test/exchange/transactions.ts index 8c5bc7bec..12390ce01 100644 --- a/packages/contracts/test/exchange/transactions.ts +++ b/packages/contracts/test/exchange/transactions.ts @@ -224,8 +224,8 @@ describe('Exchange transactions', () => { exchangeAddress: exchange.address, makerAddress, feeRecipientAddress, - makerAssetData: assetProxyUtils.encodeERC20ProxyData(defaultMakerTokenAddress), - takerAssetData: assetProxyUtils.encodeERC20ProxyData(defaultTakerTokenAddress), + makerAssetData: assetProxyUtils.encodeERC20AssetData(defaultMakerTokenAddress), + takerAssetData: assetProxyUtils.encodeERC20AssetData(defaultTakerTokenAddress), }; whitelistOrderFactory = new OrderFactory(makerPrivateKey, defaultOrderParams); }); diff --git a/packages/contracts/test/libraries/lib_bytes.ts b/packages/contracts/test/libraries/lib_bytes.ts index 9fe3a1a57..1a23483ba 100644 --- a/packages/contracts/test/libraries/lib_bytes.ts +++ b/packages/contracts/test/libraries/lib_bytes.ts @@ -1,4 +1,6 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; +import { generatePseudoRandomSalt } from '@0xproject/order-utils'; +import { AssetProxyId } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import BN = require('bn.js'); import * as chai from 'chai'; @@ -66,7 +68,7 @@ describe('LibBytes', () => { shortTestBytesAsBuffer = Buffer.concat([encodedShortDataLength, encodedShortData]); shortTestBytes = ethUtil.bufferToHex(shortTestBytesAsBuffer); // Create test bytes one word in length - wordOfData = ethUtil.bufferToHex(assetProxyUtils.encodeUint256(ZeroEx.generatePseudoRandomSalt())); + wordOfData = ethUtil.bufferToHex(assetProxyUtils.encodeUint256(generatePseudoRandomSalt())); const encodedWordOfData = ethUtil.toBuffer(wordOfData); const wordOfDataLength = new BigNumber(encodedWordOfData.byteLength); const encodedWordOfDataLength = assetProxyUtils.encodeUint256(wordOfDataLength); @@ -315,7 +317,6 @@ 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 () => { diff --git a/packages/order-utils/src/asset_proxy_utils.ts b/packages/order-utils/src/asset_proxy_utils.ts index 1f1e49f0f..a12be83a3 100644 --- a/packages/order-utils/src/asset_proxy_utils.ts +++ b/packages/order-utils/src/asset_proxy_utils.ts @@ -79,13 +79,14 @@ export const assetProxyUtils = { const encodedAssetProxyId = assetProxyUtils.encodeAssetProxyId(AssetProxyId.ERC721); const encodedAddress = assetProxyUtils.encodeAddress(tokenAddress); const encodedTokenId = assetProxyUtils.encodeUint256(tokenId); - const encodedMetadata = Buffer.concat([encodedAddress, encodedTokenId, encodedAssetProxyId]); + let encodedMetadata = Buffer.concat([encodedAddress, encodedTokenId]); 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]); } + encodedMetadata = Buffer.concat([encodedMetadata, encodedAssetProxyId]); const encodedMetadataHex = ethUtil.bufferToHex(encodedMetadata); return encodedMetadataHex; }, @@ -116,7 +117,7 @@ export const assetProxyUtils = { const nullData = '0x'; let data = nullData; if (encodedAssetData.byteLength > 53) { - const encodedDataLength = encodedAssetData.slice(53, 85); + const encodedDataLength = encodedAssetData.slice(52, 84); const dataLength = assetProxyUtils.decodeUint256(encodedDataLength); const expectedDataLength = new BigNumber(encodedAssetData.byteLength - 85); if (!dataLength.equals(expectedDataLength)) { @@ -124,7 +125,7 @@ export const assetProxyUtils = { `Data length (${dataLength}) does not match actual length of data (${expectedDataLength})`, ); } - const encodedData = encodedAssetData.slice(85); + const encodedData = encodedAssetData.slice(84, expectedDataLength.toNumber() + 84); data = ethUtil.bufferToHex(encodedData); } const erc721AssetData: ERC721AssetData = { |