Merge pull request #717 from 0xProject/feature/contracts/erc721proxy-always-safeTransferFrom

ERC721Proxy Always call safeTransferFrom

2 files changed, 7 insertions, 9 deletions

diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol
index d09aba599..9dc9e6525 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol
@@ -53,13 +53,7 @@ contract MixinERC721Transfer is
             bytes memory receiverData
         ) = decodeERC721AssetData(assetData);
 
-        // Transfer token. Saves gas by calling safeTransferFrom only
-        // when there is receiverData present. Either succeeds or throws.
-        if (receiverData.length > 0) {
-            ERC721Token(token).safeTransferFrom(from, to, tokenId, receiverData);
-        } else {
-            ERC721Token(token).transferFrom(from, to, tokenId);
-        }
+        ERC721Token(token).safeTransferFrom(from, to, tokenId, receiverData);
     }
 
     /// @dev Decodes ERC721 Asset data.
diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts
index 9760d3b9c..2c27f7382 100644
--- a/packages/contracts/test/asset_proxy/proxies.ts
+++ b/packages/contracts/test/asset_proxy/proxies.ts
@@ -276,7 +276,7 @@ describe('Asset Transfer Proxies', () => {
                 expect(newOwnerMakerAsset).to.be.bignumber.equal(takerAddress);
             });
 
-            it('should not call onERC721Received when transferring to a smart contract without receiver data', async () => {
+            it('should call onERC721Received when transferring to a smart contract without receiver data', async () => {
                 // Construct ERC721 asset data
                 const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
                 const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
@@ -297,7 +297,11 @@ describe('Asset Transfer Proxies', () => {
                 const logDecoder = new LogDecoder(web3Wrapper, erc721Receiver.address);
                 const tx = await logDecoder.getTxWithDecodedLogsAsync(txHash);
                 // Verify that no log was emitted by erc721 receiver
-                expect(tx.logs.length).to.be.equal(0);
+                expect(tx.logs.length).to.be.equal(1);
+                const tokenReceivedLog = tx.logs[0] as LogWithDecodedArgs<TokenReceivedContractEventArgs>;
+                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);
                 // Verify transfer was successful
                 const newOwnerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
                 expect(newOwnerMakerAsset).to.be.bignumber.equal(erc721Receiver.address);