EDT-3069 Altered the ref impl to do a standard call to the receiver f…

…unctions rather than "catch" the revert with low level calls. This has the side effect of not having your own revert reasons, so commented as such. Tested with Solidity 0.5.9.
pelith · May 28, 2019 · 6147143 · 6147143
1 parent f899a35
commit 6147143
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 50 deletions.
diff --git a/contracts/ERC1155.sol b/contracts/ERC1155.sol
@@ -189,57 +189,15 @@ contract ERC1155 is IERC1155, ERC165, CommonConstants
 
     function _doSafeTransferAcceptanceCheck(address _operator, address _from, address _to, uint256 _id, uint256 _value, bytes memory _data) internal {
 
-        (bool success, bytes memory returnData) = _to.call(
-            abi.encodeWithSignature(
-                "onERC1155Received(address,address,uint256,uint256,bytes)",
-                _operator,
-                _from,
-                _id,
-                _value,
-                _data
-            )
-        );
-        (success); // ignore warning on unused var
-        bytes4 receiverRet = 0x0;
-        if(returnData.length > 0) {
-            assembly {
-                receiverRet := mload(add(returnData, 32))
-            }
-        }
-
-        if (receiverRet == ERC1155_ACCEPTED) {
-            // dest was a receiver and all good, do nothing.
-        } else {
-            // dest was a receiver and rejected, revert.
-            revert("Receiver contract did not accept the transfer.");
-        }
+        // Note: if the below reverts in the onERC1155Received function of the _to address you will have an undefined revert reason returned rather than the one in the require test.
+        // If you want predictable revert reasons consider using low level _to.call() style instead so the revert does not bubble up and you can revert yourself on the ERC1155_ACCEPTED test.
+        require(IERC1155TokenReceiver(_to).onERC1155Received(_operator, _from, _id, _value, _data) == ERC1155_ACCEPTED, "contract returned an unknown value from onERC1155Received");
     }
 
     function _doSafeBatchTransferAcceptanceCheck(address _operator, address _from, address _to, uint256[] memory _ids, uint256[] memory _values, bytes memory _data) internal {
 
-        (bool success, bytes memory returnData) = _to.call(
-            abi.encodeWithSignature(
-                "onERC1155BatchReceived(address,address,uint256[],uint256[],bytes)",
-                _operator,
-                _from,
-                _ids,
-                _values,
-                _data
-            )
-        );
-        (success); // ignore warning on unused var
-        bytes4 receiverRet = 0x0;
-        if(returnData.length > 0) {
-            assembly {
-                receiverRet := mload(add(returnData, 32))
-            }
-        }
-
-        if (receiverRet == ERC1155_BATCH_ACCEPTED) {
-            // dest was a receiver and all good, do nothing.
-        } else {
-            // dest was a receiver and rejected, revert.
-            revert("Receiver contract did not accept the transfer.");
-        }
+        // Note: if the below reverts in the onERC1155BatchReceived function of the _to address you will have an undefined revert reason returned rather than the one in the require test.
+        // If you want predictable revert reasons consider using low level _to.call() style instead so the revert does not bubble up and you can revert yourself on the ERC1155_BATCH_ACCEPTED test.
+        require(IERC1155TokenReceiver(_to).onERC1155BatchReceived(_operator, _from, _ids, _values, _data) == ERC1155_BATCH_ACCEPTED, "contract returned an unknown value from onERC1155BatchReceived");
     }
 }
diff --git a/test/ERC1155ProxyTest.js b/test/ERC1155ProxyTest.js
@@ -252,7 +252,7 @@ contract('ERC1155ProxyTest - tests sending 1155 items to an ERC1538 supported pr
         let proxy1155Delegate = await ERC1155ReceiverDelegate.at(receiverContract.address);
 
         await proxy1155Delegate.setShouldReject(true);
-        await expectThrow(mainContract.safeBatchTransferFrom(user1, receiverContract.address, idSet, quantities, web3.utils.fromAscii(''), {from: user1}), "Receiver contract did not accept the transfer.");
+        await expectThrow(mainContract.safeBatchTransferFrom(user1, receiverContract.address, idSet, quantities, web3.utils.fromAscii(''), {from: user1}), "onERC1155BatchReceived: transfer not accepted");
         await proxy1155Delegate.setShouldReject(false);
         await testSafeBatchTransferFrom(user1, user1, receiverContract.address, idSet, quantities, web3.utils.fromAscii(''), 'testSafeBatchTransferFrom receiver 1155')
     });
@@ -265,7 +265,7 @@ contract('ERC1155ProxyTest - tests sending 1155 items to an ERC1538 supported pr
 
         // clashing with 1155 delegate data
         await proxyXXXXDelegate.setShouldRejectClash(true);
-        await expectThrow(mainContract.safeTransferFrom(user1, receiverContract.address, hammerId, 1, web3.utils.fromAscii('SomethingMeaningfull'), {from: user1}), "Receiver contract did not accept the transfer.");
+        await expectThrow(mainContract.safeTransferFrom(user1, receiverContract.address, hammerId, 1, web3.utils.fromAscii('SomethingMeaningfull'), {from: user1}), "onERC1155Received: transfer not accepted");
         await proxyXXXXDelegate.setShouldRejectClash(false);
         await testSafeTransferFrom(user1, user1, receiverContract.address, hammerId, 1, web3.utils.fromAscii('SomethingMeaningfull'), 'testSafeTransferFrom receiver 1155');
     });