aboutsummaryrefslogtreecommitdiffstats
path: root/packages
diff options
context:
space:
mode:
authorAmir Bandeali <abandeali1@gmail.com>2018-07-23 07:14:09 +0800
committerGitHub <noreply@github.com>2018-07-23 07:14:09 +0800
commit195d11f9d0bef49494b05198c259c7314cc9097d (patch)
tree7ff0ad80b4d5b84c365b4a8a12c321098ca4860c /packages
parent06f61949f93eb83ea7d6456f6bbf18cb6f6dab7a (diff)
parent9aa49e59d0b4a29fb9943fe2dcc04fd2630041b9 (diff)
downloaddexon-0x-contracts-195d11f9d0bef49494b05198c259c7314cc9097d.tar
dexon-0x-contracts-195d11f9d0bef49494b05198c259c7314cc9097d.tar.gz
dexon-0x-contracts-195d11f9d0bef49494b05198c259c7314cc9097d.tar.bz2
dexon-0x-contracts-195d11f9d0bef49494b05198c259c7314cc9097d.tar.lz
dexon-0x-contracts-195d11f9d0bef49494b05198c259c7314cc9097d.tar.xz
dexon-0x-contracts-195d11f9d0bef49494b05198c259c7314cc9097d.tar.zst
dexon-0x-contracts-195d11f9d0bef49494b05198c259c7314cc9097d.zip
Merge pull request #877 from 0xProject/feature/contracts/removeERC721Callback
Removed receiverData and `onReceive` callback from ERC721 proxy.
Diffstat (limited to 'packages')
-rw-r--r--packages/contracts/src/2.0.0/forwarder/MixinForwarderCore.sol2
-rw-r--r--packages/contracts/src/2.0.0/forwarder/MixinTransfer.sol6
-rw-r--r--packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol96
-rw-r--r--packages/contracts/test/asset_proxy/proxies.ts91
-rw-r--r--packages/contracts/test/exchange/core.ts24
-rw-r--r--packages/migrations/src/utils/constants.ts2
-rw-r--r--packages/order-utils/src/asset_data_utils.ts11
-rw-r--r--packages/types/src/index.ts3
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 {