diff options
Diffstat (limited to 'packages')
| -rw-r--r-- | packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol | 2 | ||||
| -rw-r--r-- | packages/contracts/src/2.0.0/forwarder/MixinTransfer.sol | 6 | ||||
| -rw-r--r-- | packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol | 96 | ||||
| -rw-r--r-- | packages/contracts/test/asset_proxy/proxies.ts | 91 | ||||
| -rw-r--r-- | packages/contracts/test/exchange/core.ts | 24 | ||||
| -rw-r--r-- | packages/migrations/src/utils/constants.ts | 2 | ||||
| -rw-r--r-- | packages/order-utils/src/asset_data_utils.ts | 11 | ||||
| -rw-r--r-- | packages/types/src/index.ts | 3 |
8 files changed, 36 insertions, 199 deletions
diff --git a/packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol b/packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol index eadeaf5ba..cb0ed5422 100644 --- a/packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol +++ b/packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol @@ -40,7 +40,7 @@ contract MixinForwarderCore is MForwarderCore { bytes4 constant internal ERC20_DATA_ID = bytes4(keccak256("ERC20Token(address)")); - bytes4 constant internal ERC721_DATA_ID = bytes4(keccak256("ERC721Token(address,uint256,bytes)")); + bytes4 constant internal ERC721_DATA_ID = bytes4(keccak256("ERC721Token(address,uint256)")); uint256 constant internal MAX_UINT = 2**256 - 1; constructor () diff --git a/packages/contracts/src/2.0.0/forwarder/MixinTransfer.sol b/packages/contracts/src/2.0.0/forwarder/MixinTransfer.sol index 6c49330f2..bebfc976b 100644 --- a/packages/contracts/src/2.0.0/forwarder/MixinTransfer.sol +++ b/packages/contracts/src/2.0.0/forwarder/MixinTransfer.sol @@ -109,12 +109,10 @@ contract MixinTransfer is // Decode asset data. address token = assetData.readAddress(16); uint256 tokenId = assetData.readUint256(36); - bytes memory receiverData = assetData.readBytesWithLength(100); - IERC721Token(token).safeTransferFrom( + IERC721Token(token).transferFrom( address(this), to, - tokenId, - receiverData + tokenId ); } } diff --git a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol index 1f9958b43..53f5a14e0 100644 --- a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol @@ -26,7 +26,7 @@ contract ERC721Proxy is MixinAuthorizable { // Id of this proxy. - bytes4 constant internal PROXY_ID = bytes4(keccak256("ERC721Token(address,uint256,bytes)")); + bytes4 constant internal PROXY_ID = bytes4(keccak256("ERC721Token(address,uint256)")); // solhint-disable-next-line payable-fallback function () @@ -83,19 +83,15 @@ contract ERC721Proxy is // WARNING: The ABIv2 specification allows additional padding between // the Params and Data section. This will result in a larger // offset to assetData. - + // Asset data itself is encoded as follows: // // | Area | Offset | Length | Contents | // |----------|--------|---------|-------------------------------------| // | Header | 0 | 4 | function selector | - // | Params | | 3 * 32 | function parameters: | + // | Params | | 2 * 32 | function parameters: | // | | 4 | 12 + 20 | 1. token address | // | | 36 | | 2. tokenId | - // | | 68 | | 3. offset to receiverData (*) | - // | Data | | | receiverData: | - // | | 100 | 32 | receiverData Length | - // | | 132 | ** | receiverData Contents | // We construct calldata for the `token.safeTransferFrom` ABI. // The layout of this calldata is in the table below. @@ -103,14 +99,10 @@ contract ERC721Proxy is // | Area | Offset | Length | Contents | // |----------|--------|---------|-------------------------------------| // | Header | 0 | 4 | function selector | - // | Params | | 4 * 32 | function parameters: | + // | Params | | 3 * 32 | function parameters: | // | | 4 | | 1. from | // | | 36 | | 2. to | // | | 68 | | 3. tokenId | - // | | 100 | | 4. offset to receiverData (*) | - // | Data | | | receiverData: | - // | | 132 | 32 | receiverData Length | - // | | 164 | ** | receiverData Contents | // There exists only 1 of each token. // require(amount == 1, "INVALID_AMOUNT") @@ -122,76 +114,32 @@ contract ERC721Proxy is mstore(96, 0) revert(0, 100) } - - // Require assetData to be at least 132 bytes - let offset := calldataload(4) - if lt(calldataload(add(offset, 4)), 132) { - // Revert with `Error("LENGTH_GREATER_THAN_131_REQUIRED")` - mstore(0, 0x08c379a000000000000000000000000000000000000000000000000000000000) - mstore(32, 0x0000002000000000000000000000000000000000000000000000000000000000) - mstore(64, 0x000000204c454e4754485f475245415445525f5448414e5f3133315f52455155) - mstore(96, 0x4952454400000000000000000000000000000000000000000000000000000000) - revert(0, 100) - } - - /////// Setup State /////// - // `cdStart` is the start of the calldata for - // `token.safeTransferFrom` (equal to free memory ptr). - let cdStart := mload(64) - // `dataAreaLength` is the total number of words - // needed to store `receiverData` - // As-per the ABI spec, this value is padded up to - // the nearest multiple of 32, - // and includes 32-bytes for length. - // It's calculated as folows: - // - Unpadded length in bytes = `mload(receiverData) + 32` - // - Add 31 to convert rounding down to rounding up. - // Combined with the previous and this is `63`. - // - Round down to nearest multiple of 32 by clearing - // bits 0x1F. This is done with `and` and a mask. /////// Setup Header Area /////// // This area holds the 4-byte `transferFromSelector`. // Any trailing data in transferFromSelector will be // overwritten in the next `mstore` call. - mstore(cdStart, 0xb88d4fde00000000000000000000000000000000000000000000000000000000) + mstore(0, 0x23b872dd00000000000000000000000000000000000000000000000000000000) /////// Setup Params Area /////// - // Each parameter is padded to 32-bytes. - // The entire Params Area is 128 bytes. - // Notes: - // 1. A 20-byte mask is applied to addresses - // to zero-out the unused bytes. - // 2. The offset to `receiverData` is the length - // of the Params Area (128 bytes). - - let length := calldataload(add(offset, 136)) - let token := calldataload(add(offset, 40)) - - // Round length up to multiple of 32 - length := and(add(length, 31), 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFE0) - - // Copy `from` and `to` - calldatacopy(add(cdStart, 4), 36, 64) - - // TokenId - mstore(add(cdStart, 68), calldataload(add(offset, 72))) - - // Offset to receiverData - mstore(add(cdStart, 100), 128) - - // receiverData (including length) - calldatacopy(add(cdStart, 132), add(offset, 136), add(length, 32)) - - /////// Call `token.safeTransferFrom` using the calldata /////// + // We copy the fields `from` and `to` in bulk + // from our own calldata to the new calldata. + calldatacopy(4, 36, 64) + + // Copy `tokenId` field from our own calldata to the new calldata. + let assetDataOffset := calldataload(4) + calldatacopy(68, add(assetDataOffset, 72), 32) + + /////// Call `token.transferFrom` using the calldata /////// + let token := calldataload(add(assetDataOffset, 40)) let success := call( - gas, // forward all gas - token, // call address of token contract - 0, // don't send any ETH - cdStart, // pointer to start of input - add(length, 164), // length of input - 0, // write output to null - 0 // output size is 0 bytes + gas, // forward all gas + token, // call address of token contract + 0, // don't send any ETH + 0, // pointer to start of input + 100, // length of input + 0, // write output to null + 0 // output size is 0 bytes ) if success { return(0, 0) diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts index 39674a030..62215f08d 100644 --- a/packages/contracts/test/asset_proxy/proxies.ts +++ b/packages/contracts/test/asset_proxy/proxies.ts @@ -1,17 +1,12 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils'; -import { assetDataUtils, generatePseudoRandomSalt } from '@0xproject/order-utils'; +import { assetDataUtils } from '@0xproject/order-utils'; import { RevertReason } 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'; import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token'; -import { - DummyERC721ReceiverContract, - DummyERC721ReceiverTokenReceivedEventArgs, -} from '../../generated_contract_wrappers/dummy_erc721_receiver'; +import { DummyERC721ReceiverContract } from '../../generated_contract_wrappers/dummy_erc721_receiver'; import { DummyERC721TokenContract } from '../../generated_contract_wrappers/dummy_erc721_token'; import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy'; import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy'; @@ -23,7 +18,6 @@ import { constants } from '../utils/constants'; import { ERC20Wrapper } from '../utils/erc20_wrapper'; import { ERC721Wrapper } from '../utils/erc721_wrapper'; import { LogDecoder } from '../utils/log_decoder'; -import { typeEncodingUtils } from '../utils/type_encoding_utils'; import { provider, txDefaults, web3Wrapper } from '../utils/web3_wrapper'; chaiSetup.configure(); @@ -253,7 +247,7 @@ describe('Asset Transfer Proxies', () => { expect(newOwnerMakerAsset).to.be.bignumber.equal(takerAddress); }); - it('should call onERC721Received when transferring to a smart contract without receiver data', async () => { + it('should not call onERC721Received when transferring to a smart contract', async () => { // Construct ERC721 asset data const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); // Verify pre-condition @@ -277,85 +271,12 @@ describe('Asset Transfer Proxies', () => { }), ); // Verify that no log was emitted by erc721 receiver - expect(tx.logs.length).to.be.equal(1); - const tokenReceivedLog = tx.logs[0] as LogWithDecodedArgs<DummyERC721ReceiverTokenReceivedEventArgs>; - expect(tokenReceivedLog.args.from).to.be.equal(makerAddress); - expect(tokenReceivedLog.args.tokenId).to.be.bignumber.equal(erc721MakerTokenId); - expect(tokenReceivedLog.args.data).to.be.equal(constants.NULL_BYTES); + expect(tx.logs.length).to.be.equal(0); // Verify transfer was successful const newOwnerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); expect(newOwnerMakerAsset).to.be.bignumber.equal(erc721Receiver.address); }); - it('should call onERC721Received when transferring to a smart contract with receiver data', async () => { - // Construct ERC721 asset data - const receiverData = ethUtil.bufferToHex(typeEncodingUtils.encodeUint256(generatePseudoRandomSalt())); - const encodedAssetData = assetDataUtils.encodeERC721AssetData( - erc721Token.address, - erc721MakerTokenId, - receiverData, - ); - // 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 data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( - encodedAssetData, - makerAddress, - erc721Receiver.address, - amount, - ); - const logDecoder = new LogDecoder(web3Wrapper, erc721Receiver.address); - const tx = await logDecoder.getTxWithDecodedLogsAsync( - await web3Wrapper.sendTransactionAsync({ - to: erc721Proxy.address, - data, - from: exchangeAddress, - gas: constants.MAX_TRANSFER_FROM_GAS, - }), - ); - // Validate log emitted by erc721 receiver - expect(tx.logs.length).to.be.equal(1); - const tokenReceivedLog = tx.logs[0] as LogWithDecodedArgs<DummyERC721ReceiverTokenReceivedEventArgs>; - expect(tokenReceivedLog.args.from).to.be.equal(makerAddress); - expect(tokenReceivedLog.args.tokenId).to.be.bignumber.equal(erc721MakerTokenId); - expect(tokenReceivedLog.args.data).to.be.equal(receiverData); - // Verify transfer was successful - const newOwnerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId); - expect(newOwnerMakerAsset).to.be.bignumber.equal(erc721Receiver.address); - }); - - it('should throw if there is receiver data but contract does not have onERC721Received', async () => { - // Construct ERC721 asset data - const receiverData = ethUtil.bufferToHex(typeEncodingUtils.encodeUint256(generatePseudoRandomSalt())); - const encodedAssetData = assetDataUtils.encodeERC721AssetData( - erc721Token.address, - erc721MakerTokenId, - receiverData, - ); - // 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 data = assetProxyInterface.transferFrom.getABIEncodedTransactionData( - encodedAssetData, - makerAddress, - erc20Proxy.address, // the ERC20 proxy does not have an ERC721 receiver - amount, - ); - return expectTransactionFailedAsync( - web3Wrapper.sendTransactionAsync({ - to: erc721Proxy.address, - data, - from: exchangeAddress, - gas: constants.MAX_TRANSFER_FROM_GAS, - }), - RevertReason.TransferFailed, - ); - }); - it('should throw if transferring 0 amount of a token', async () => { // Construct ERC721 asset data const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId); @@ -454,9 +375,9 @@ describe('Asset Transfer Proxies', () => { }); }); - it('should have an id of 0x08e937fa', async () => { + it('should have an id of 0x02571792', async () => { const proxyId = await erc721Proxy.getProxyId.callAsync(); - const expectedProxyId = '0x08e937fa'; + const expectedProxyId = '0x02571792'; expect(proxyId).to.equal(expectedProxyId); }); }); diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts index 33246a681..b324988cc 100644 --- a/packages/contracts/test/exchange/core.ts +++ b/packages/contracts/test/exchange/core.ts @@ -456,30 +456,6 @@ describe('Exchange core', () => { RevertReason.RoundingError, ); }); - - it('should throw if assetData has a length < 132', async () => { - // Construct Exchange parameters - const makerAssetId = erc721MakerAssetIds[0]; - const takerAssetId = erc721TakerAssetIds[0]; - const makerAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, makerAssetId).slice(0, -2); - signedOrder = await orderFactory.newSignedOrderAsync({ - makerAssetAmount: new BigNumber(1), - takerAssetAmount: new BigNumber(1), - makerAssetData, - takerAssetData: assetDataUtils.encodeERC721AssetData(erc721Token.address, takerAssetId), - }); - // Verify pre-conditions - const initialOwnerMakerAsset = await erc721Token.ownerOf.callAsync(makerAssetId); - expect(initialOwnerMakerAsset).to.be.bignumber.equal(makerAddress); - const initialOwnerTakerAsset = await erc721Token.ownerOf.callAsync(takerAssetId); - expect(initialOwnerTakerAsset).to.be.bignumber.equal(takerAddress); - // Call Exchange - const takerAssetFillAmount = signedOrder.takerAssetAmount; - return expectTransactionFailedAsync( - exchangeWrapper.fillOrderAsync(signedOrder, takerAddress, { takerAssetFillAmount }), - RevertReason.LengthGreaterThan131Required, - ); - }); }); describe('getOrderInfo', () => { diff --git a/packages/migrations/src/utils/constants.ts b/packages/migrations/src/utils/constants.ts index 068122e28..53ce2f5f1 100644 --- a/packages/migrations/src/utils/constants.ts +++ b/packages/migrations/src/utils/constants.ts @@ -9,7 +9,7 @@ export const constants = { ASSET_PROXY_OWNER_TIMELOCK: new BigNumber(0), ASSET_PROXY_OWNER_CONFIRMATIONS: new BigNumber(1), ERC20_PROXY_ID: '0xf47261b0', - ERC721_PROXY_ID: '0x08e937fa', + ERC721_PROXY_ID: '0x02571792', NULL_ADDRESS: '0x0000000000000000000000000000000000000000', RPC_URL: 'http://localhost:8545', KOVAN_NETWORK_ID: 42, diff --git a/packages/order-utils/src/asset_data_utils.ts b/packages/order-utils/src/asset_data_utils.ts index a9601e9ea..0c0b59548 100644 --- a/packages/order-utils/src/asset_data_utils.ts +++ b/packages/order-utils/src/asset_data_utils.ts @@ -50,14 +50,13 @@ export const assetDataUtils = { * @param tokenId The ERC721 tokenId to encode * @return The hex encoded assetData string */ - encodeERC721AssetData(tokenAddress: string, tokenId: BigNumber, receiverData?: string): string { + encodeERC721AssetData(tokenAddress: string, tokenId: BigNumber): string { // TODO: Pass `tokendId` as a BigNumber. return ethUtil.bufferToHex( ethAbi.simpleEncode( - 'ERC721Token(address,uint256,bytes)', + 'ERC721Token(address,uint256)', tokenAddress, `0x${tokenId.toString(constants.BASE_16)}`, - ethUtil.toBuffer(receiverData || '0x'), ), ); }, @@ -83,15 +82,11 @@ export const assetDataUtils = { }), but got ${assetProxyId}`, ); } - const [tokenAddress, tokenId, receiverData] = ethAbi.rawDecode( - ['address', 'uint256', 'bytes'], - data.slice(constants.SELECTOR_LENGTH), - ); + const [tokenAddress, tokenId] = ethAbi.rawDecode(['address', 'uint256'], data.slice(constants.SELECTOR_LENGTH)); return { assetProxyId, tokenAddress: ethUtil.addHexPrefix(tokenAddress), tokenId: new BigNumber(tokenId.toString()), - receiverData: ethUtil.bufferToHex(receiverData), }; }, /** diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 72e2a726b..19f2b1a23 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -159,7 +159,7 @@ export interface ECSignature { export enum AssetProxyId { ERC20 = '0xf47261b0', - ERC721 = '0x08e937fa', + ERC721 = '0x02571792', } export interface ERC20AssetData { @@ -171,7 +171,6 @@ export interface ERC721AssetData { assetProxyId: string; tokenAddress: string; tokenId: BigNumber; - receiverData: string; } export enum RevertReason { |