Removed receiverData and onReceive callback from ERC721 proxy.

This commit is contained in:
Greg Hysen
2018-07-13 14:13:16 -07:00
parent e2fb49a8f8
commit 9f74feb347
3 changed files with 25 additions and 178 deletions

View File

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

View File

@@ -1,17 +1,13 @@
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';
@@ -253,7 +249,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 +273,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);

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