diff --git a/src/ERC721TransferLib.sol b/src/ERC721TransferLib.sol index 71aec94..bc8ea05 100644 --- a/src/ERC721TransferLib.sol +++ b/src/ERC721TransferLib.sol @@ -26,6 +26,11 @@ library ERC721TransferLib { token.addr.transferFrom(parties.seller, parties.buyer, token.id); } + /// @dev Transfers the token from `parties.seller` to `parties.buyer` using `safeTransferFrom()`. + function _safeTransfer(ERC721Token memory token, Parties memory parties, bytes memory data) internal { + token.addr.safeTransferFrom(parties.seller, parties.buyer, token.id, data); + } + /// @dev Represents multiple ERC721 tokens within the same contract. struct MultiERC721Token { IERC721 addr; @@ -40,6 +45,14 @@ library ERC721TransferLib { _transfer(tokens, _reusableTransferCallData(parties)); } + /** + * @dev Transfers all tokens from `parties.seller` to `parties.buyer` using `safeTransferFrom()`. + * @param tokens Any number of ERC721 tokens from a single contract; ids MUST be distinct across the entire array. + */ + function _safeTransfer(MultiERC721Token memory tokens, Parties memory parties, bytes memory data) internal { + _transfer(tokens, _reusableSafeTransferCallData(parties, data)); + } + /** * @dev Transfers all tokens from `parties.seller` to `parties.buyer`. * @param tokens An _array_ of MultiERC721Token structs, representing any number of tokens. {addr,id} pairs MUST be @@ -52,6 +65,18 @@ library ERC721TransferLib { } } + /** + * @dev Transfers all tokens from `parties.seller` to `parties.buyer` using `safeTransferFrom()`. + * @param tokens An _array_ of MultiERC721Token structs, representing any number of tokens. {addr,id} pairs MUST be + * distinct across the entire array. + */ + function _safeTransfer(MultiERC721Token[] memory tokens, Parties memory parties, bytes memory data) internal { + bytes memory callData = _reusableSafeTransferCallData(parties, data); + for (uint256 i = 0; i < tokens.length; ++i) { + _transfer(tokens[i], callData); + } + } + /** * @dev Returns calldata for `ERC721.transferFrom(parties.seller, parties.buyer, 0)`, which can be reused to avoid * unnecessary memory expansion when transferring large numbers of tokens. @@ -60,9 +85,24 @@ library ERC721TransferLib { return abi.encodeCall(IERC721.transferFrom, (parties.seller, parties.buyer, 0)); } + /** + * @dev Returns calldata for `ERC721.safeTransferFrom(parties.seller, parties.buyer, 0, data)`. See + * `_reusableTransferCallData()` for rationale. + */ + function _reusableSafeTransferCallData(Parties memory parties, bytes memory data) + private + pure + returns (bytes memory) + { + // Since `safeTransferFrom()` has an overload, we can't use `abi.encodeCall()` as it's ambiguous. + return abi.encodeWithSignature( + "safeTransferFrom(address,address,uint256,bytes)", parties.seller, parties.buyer, 0, data + ); + } + /** * @param token Contract and IDs of tokens to be transferred. - * @param reusableCallData Output of `_reusableTransferCallData()`. + * @param reusableCallData Output of `_reusableTransferCallData()` or `_reusableSafeTransferCallData()`. */ function _transfer(MultiERC721Token memory token, bytes memory reusableCallData) private { address addr = address(token.addr); diff --git a/test/ERC721TransferLib.t.sol b/test/ERC721TransferLib.t.sol index 386d3db..6cb0f2e 100644 --- a/test/ERC721TransferLib.t.sol +++ b/test/ERC721TransferLib.t.sol @@ -9,6 +9,16 @@ import {ERC721TransferLib, IERC721} from "../src/ERC721TransferLib.sol"; import {Parties} from "../src/TypesAndConstants.sol"; import {IERC721Errors} from "@openzeppelin/contracts/interfaces/draft-IERC6093.sol"; +import {IERC721Receiver} from "@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol"; + +contract ERC721Receiver is IERC721Receiver { + mapping(uint256 => bytes) public tokenData; + + function onERC721Received(address, address, uint256 tokenId, bytes calldata data) external returns (bytes4) { + tokenData[tokenId] = data; + return IERC721Receiver.onERC721Received.selector; + } +} contract ERC721Transferrer { function transfer(ERC721TransferLib.MultiERC721Token[] memory tokens, Parties memory parties) external { @@ -29,10 +39,18 @@ contract ERC721TransferLibTest is Test, ITestEvents { uint256[TOKENS_PER_CONTRACT][NUM_CONTRACTS] calldata ids, Parties memory parties ) public { + vm.assume(parties.buyer.code.length == 0); // otherwise safeTransferFrom() will fail + ERC721TransferLib.MultiERC721Token[] memory tokens = _testSetup(deploySalts, ids, parties); - function(ERC721TransferLib.MultiERC721Token[] memory, Parties memory)[3] memory funcs = - [_transferBatch, _transferPerContract, _transferIndividually]; + function(ERC721TransferLib.MultiERC721Token[] memory, Parties memory)[6] memory funcs = [ + _transferBatch, + _safeTransferBatch, + _transferPerContract, + _safeTransferPerContract, + _transferIndividually, + _safeTransferIndividually + ]; for (uint256 i = 0; i < funcs.length; ++i) { uint256 snap = vm.snapshot(); @@ -51,6 +69,19 @@ contract ERC721TransferLibTest is Test, ITestEvents { tokens._transfer(parties); } + function _safeTransferBatch(ERC721TransferLib.MultiERC721Token[] memory tokens, Parties memory parties) internal { + _safeTransferBatchWithData(tokens, parties, ""); + } + + /// @dev The `WithData` suffix avoids overloading so the functions can be placed in arrays for test suites. + function _safeTransferBatchWithData( + ERC721TransferLib.MultiERC721Token[] memory tokens, + Parties memory parties, + bytes memory data + ) internal { + tokens._safeTransfer(parties, data); + } + function _transferPerContract(ERC721TransferLib.MultiERC721Token[] memory tokens, Parties memory parties) internal { @@ -59,6 +90,22 @@ contract ERC721TransferLibTest is Test, ITestEvents { } } + function _safeTransferPerContract(ERC721TransferLib.MultiERC721Token[] memory tokens, Parties memory parties) + internal + { + _safeTransferPerContractWithData(tokens, parties, ""); + } + + function _safeTransferPerContractWithData( + ERC721TransferLib.MultiERC721Token[] memory tokens, + Parties memory parties, + bytes memory data + ) internal { + for (uint256 i = 0; i < tokens.length; ++i) { + tokens[i]._safeTransfer(parties, data); + } + } + function _transferIndividually(ERC721TransferLib.MultiERC721Token[] memory tokens, Parties memory parties) internal { @@ -69,6 +116,24 @@ contract ERC721TransferLibTest is Test, ITestEvents { } } + function _safeTransferIndividually(ERC721TransferLib.MultiERC721Token[] memory tokens, Parties memory parties) + internal + { + _safeTransferIndividuallyWithData(tokens, parties, ""); + } + + function _safeTransferIndividuallyWithData( + ERC721TransferLib.MultiERC721Token[] memory tokens, + Parties memory parties, + bytes memory data + ) internal { + for (uint256 i = 0; i < tokens.length; ++i) { + for (uint256 j = 0; j < tokens[i].ids.length; ++j) { + ERC721TransferLib.ERC721Token({addr: tokens[i].addr, id: tokens[i].ids[j]})._safeTransfer(parties, data); + } + } + } + function testGas( bytes32[NUM_CONTRACTS] calldata deploySalts, uint256[TOKENS_PER_CONTRACT][NUM_CONTRACTS] calldata ids, @@ -160,6 +225,40 @@ contract ERC721TransferLibTest is Test, ITestEvents { tokens._transfer(parties); } + function testSafeTransferDataPropagation(uint256 tokenId, address seller, bytes32 buyerSalt, bytes memory data) + external + { + vm.assume(seller != address(0)); + + ERC721Receiver buyer = new ERC721Receiver{salt: buyerSalt}(); + Parties memory parties = Parties({seller: seller, buyer: address(buyer)}); + + Token t = new Token(); + t.mint(seller, tokenId); + + ERC721TransferLib.MultiERC721Token[] memory tokens = new ERC721TransferLib.MultiERC721Token[](1); + tokens[0].addr = IERC721(t); + tokens[0].ids = new uint256[](1); + tokens[0].ids[0] = tokenId; + + function(ERC721TransferLib.MultiERC721Token[] memory, Parties memory, bytes memory)[3] memory funcs = + [_safeTransferBatchWithData, _safeTransferPerContractWithData, _safeTransferIndividuallyWithData]; + + for (uint256 i = 0; i < funcs.length; ++i) { + uint256 snap = vm.snapshot(); + _assertOwner(tokens, parties.seller); + assertEq(buyer.tokenData(tokenId), "", "safeTransfer() data recorder empty"); + + vm.startPrank(parties.seller); + funcs[i](tokens, parties, data); + vm.stopPrank(); + + _assertOwner(tokens, parties.buyer); + assertEq(buyer.tokenData(tokenId), data, "safeTransfer() data propagated"); + vm.revertTo(snap); + } + } + function testNoCodeAtTokenAddress( bytes32[NUM_CONTRACTS] calldata deploySalts, uint256[TOKENS_PER_CONTRACT][NUM_CONTRACTS] calldata ids,