Merge pull request #877 from 0xProject/feature/contracts/removeERC721Callback

Removed receiverData and `onReceive` callback from ERC721 proxy.
This commit is contained in:
Amir Bandeali
2018-07-22 18:14:09 -05:00
committed by GitHub
8 changed files with 36 additions and 199 deletions

View File

@@ -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);
});
});

View File

@@ -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', () => {