From 774d831fae5809408f9ddfcf9393d579416b1bfb Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Mon, 4 Jun 2018 22:34:04 -0700 Subject: Style updates to ERC721 onReceiver --- packages/contracts/package.json | 2 +- .../contracts/current/protocol/AssetProxy/ERC20Proxy.sol | 1 - .../contracts/current/protocol/AssetProxy/ERC721Proxy.sol | 4 +--- .../src/contracts/current/utils/LibBytes/LibBytes.sol | 9 +++++---- packages/contracts/test/asset_proxy/decoder.ts | 14 ++++++++++++-- packages/contracts/test/asset_proxy/proxies.ts | 3 ++- packages/order-utils/src/asset_proxy_utils.ts | 10 ++++++---- 7 files changed, 27 insertions(+), 16 deletions(-) (limited to 'packages') diff --git a/packages/contracts/package.json b/packages/contracts/package.json index e436ae15e..b533a22ce 100644 --- a/packages/contracts/package.json +++ b/packages/contracts/package.json @@ -30,7 +30,7 @@ "test:circleci": "yarn test" }, "config": { - "abis": "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Token|DummyERC721Receiver|ERC20Proxy|ERC721Proxy|Exchange|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetDataDecoders|TestAssetProxyDispatcher|TestLibBytes|TestLibMem|TestLibs|TestSignatureValidator|TokenRegistry|Whitelist|WETH9|ZRXToken).json" + "abis": "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetDataDecoders|TestAssetProxyDispatcher|TestLibBytes|TestLibMem|TestLibs|TestSignatureValidator|TokenRegistry|Whitelist|WETH9|ZRXToken).json" }, "repository": { "type": "git", diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index c8e8f4587..50632400e 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -20,7 +20,6 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; import "../../utils/LibBytes/LibBytes.sol"; -import "../../tokens/ERC20Token/IERC20Token.sol"; import "./MixinAssetProxy.sol"; import "./MixinAuthorizable.sol"; import "../../tokens/ERC20Token/IERC20Token.sol"; diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index 7de7968cc..21e5518c6 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -33,8 +33,6 @@ contract ERC721Proxy is // Id of this proxy. uint8 constant PROXY_ID = 2; - string constant PROXY_ID_MISMATCH = "Proxy id in metadata does not match this proxy id."; - /// @dev Internal version of `transferFrom`. /// @param assetData Encoded byte array. /// @param from Address to transfer asset from. @@ -60,7 +58,7 @@ contract ERC721Proxy is // Data must be intended for this proxy. require( proxyId == PROXY_ID, - PROXY_ID_MISMATCH + ASSET_PROXY_ID_MISMATCH ); // There exists only 1 of each token. diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 5610c47b3..8f6647d20 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -332,7 +332,8 @@ contract LibBytes is internal pure { - // Read length of nested bytes + // Assert length of is valid, given + // length of input require( b.length >= index + 32 /* 32 bytes to store length */ + input.length, GTE_32_LENGTH_REQUIRED @@ -340,9 +341,9 @@ contract LibBytes is // Copy into memcpy( - getMemAddress(b) + index + 32, // +32 to skip length of - getMemAddress(input), // include length of byte array - input.length + 32 // +32 bytes to store length + getMemAddress(b) + 32 + index, // +32 to skip length of + getMemAddress(input), // includes length of + input.length + 32 // +32 bytes to store length ); } } diff --git a/packages/contracts/test/asset_proxy/decoder.ts b/packages/contracts/test/asset_proxy/decoder.ts index f2668dfe6..6a60c07bb 100644 --- a/packages/contracts/test/asset_proxy/decoder.ts +++ b/packages/contracts/test/asset_proxy/decoder.ts @@ -24,6 +24,12 @@ describe('TestAssetDataDecoders', () => { let testAssetProxyDecoder: TestAssetDataDecodersContract; let testAddress: string; + before(async () => { + await blockchainLifecycle.startAsync(); + }); + after(async () => { + await blockchainLifecycle.revertAsync(); + }); before(async () => { // Setup accounts & addresses const accounts = await web3Wrapper.getAvailableAddressesAsync(); @@ -78,8 +84,12 @@ describe('TestAssetDataDecoders', () => { it('should correctly decode ERC721 asset data with receiver data', async () => { const tokenId = generatePseudoRandomSalt(); - const receiverData = - ethUtil.bufferToHex(assetProxyUtils.encodeUint256(generatePseudoRandomSalt())) + 'FFFF'; + const receiverDataFirst32Bytes = ethUtil.bufferToHex( + assetProxyUtils.encodeUint256(generatePseudoRandomSalt()), + ); + const receiverDataExtraBytes = 'FFFF'; + // We add extra bytes to generate a value that doesn't fit perfectly into one word + const receiverData = receiverDataFirst32Bytes + receiverDataExtraBytes; 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 52a92718b..a7dc54efc 100644 --- a/packages/contracts/test/asset_proxy/proxies.ts +++ b/packages/contracts/test/asset_proxy/proxies.ts @@ -287,7 +287,7 @@ describe('Asset Transfer Proxies', () => { amount, { from: exchangeAddress }, ); - + await web3Wrapper.awaitTransactionSuccessAsync(txHash, constants.AWAIT_TRANSACTION_MINED_MS); // Parse transaction logs const logDecoder = new LogDecoder(web3Wrapper, erc721Receiver.address); const tx = await logDecoder.getTxWithDecodedLogsAsync(txHash); @@ -319,6 +319,7 @@ describe('Asset Transfer Proxies', () => { amount, { from: exchangeAddress }, ); + await web3Wrapper.awaitTransactionSuccessAsync(txHash, constants.AWAIT_TRANSACTION_MINED_MS); // Parse transaction logs const logDecoder = new LogDecoder(web3Wrapper, erc721Receiver.address); const tx = await logDecoder.getTxWithDecodedLogsAsync(txHash); diff --git a/packages/order-utils/src/asset_proxy_utils.ts b/packages/order-utils/src/asset_proxy_utils.ts index 61a9b12e9..c7b301c1a 100644 --- a/packages/order-utils/src/asset_proxy_utils.ts +++ b/packages/order-utils/src/asset_proxy_utils.ts @@ -119,10 +119,12 @@ export const assetProxyUtils = { const tokenId = assetProxyUtils.decodeUint256(encodedTokenId); const nullData = '0x'; let receiverData = nullData; - if (encodedAssetData.byteLength > receiverDataLengthOffset + 1) { + const lengthUpToReceiverDataLength = receiverDataLengthOffset + 1; + if (encodedAssetData.byteLength > lengthUpToReceiverDataLength) { const encodedReceiverDataLength = encodedAssetData.slice(receiverDataLengthOffset, receiverDataOffset); const receiverDataLength = assetProxyUtils.decodeUint256(encodedReceiverDataLength); - const expectedReceiverDataLength = new BigNumber(encodedAssetData.byteLength - (receiverDataOffset + 1)); + const lengthUpToReceiverData = receiverDataOffset + 1; + const expectedReceiverDataLength = new BigNumber(encodedAssetData.byteLength - lengthUpToReceiverData); if (!receiverDataLength.equals(expectedReceiverDataLength)) { throw new Error( `Data length (${receiverDataLength}) does not match actual length of data (${expectedReceiverDataLength})`, @@ -167,12 +169,12 @@ export const assetProxyUtils = { return generalizedERC20AssetData; case AssetProxyId.ERC721: const erc721AssetData = assetProxyUtils.decodeERC721AssetData(assetData); - const generaliedERC721AssetData = { + const generalizedERC721AssetData = { assetProxyId, tokenAddress: erc721AssetData.tokenAddress, data: erc721AssetData.tokenId, }; - return generaliedERC721AssetData; + return generalizedERC721AssetData; default: throw new Error(`Unrecognized asset proxy id: ${assetProxyId}`); } -- cgit