Call checkOnERC1155BatchReceived when transfering a batch of length 1

.changeset/vast-worlds-pull.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+`ERC1155`: Call `IERC1155Receiver.onERC1155BatchReceived` when performing a batch transfers with exactly one id/value in the batch.

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ### Breaking changes
 
+- `ERC1155`: Performing batch transfers with exactly one id/value in the batch no-longer calls `IERC1155Receiver.onERC1155Received`. `IERC1155Receiver.onERC1155BatchReceived` is called instead (with arrays of length one). ([#6170](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/6170))
 - `ERC1967Proxy` and `TransparentUpgradeableProxy`: Mandate initialization during construction. Deployment now reverts with `ERC1967ProxyUninitialized` if an initialize call is not provided. Developers that rely on the previous behavior and want to disable this check can do so by overriding the internal `_unsafeAllowUninitialized` function to return true. ([#5906](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/5906))
 - `ERC721` and `ERC1155`: Prevent setting an operator for `address(0)`. In the case of `ERC721` this type of operator allowance could lead to obfuscated mint permission. ([#6171](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/6171))
 - `RLP`: The `encode(bytes32)` function now encodes `bytes32` as a fixed size item and not as a scalar in `encode(uint256)`. Users must replace calls to `encode(bytes32)` with `encode(uint256(bytes32))` to preserve the same behavior. ([#6167](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/6167))

contracts/token/ERC1155/ERC1155.sol

@@ -178,23 +178,46 @@ abstract contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI, IER
      * IMPORTANT: Overriding this function is discouraged because it poses a reentrancy risk from the receiver. So any
      * update to the contract state after this function would break the check-effect-interaction pattern. Consider
      * overriding {_update} instead.
+     *
+     * NOTE: This version is kept for backward compatibility. We recommend calling the alternative version with a boolean
+     * flag in order to achieve better control over which hook to call.
      */
     function _updateWithAcceptanceCheck(
         address from,
         address to,
         uint256[] memory ids,
         uint256[] memory values,
         bytes memory data
+    ) internal virtual {
+        _updateWithAcceptanceCheck(from, to, ids, values, data, ids.length != 1);
+    }
+
+    /**
+     * @dev Version of {_update} that performs the token acceptance check by calling
+     * {IERC1155Receiver-onERC1155Received} or {IERC1155Receiver-onERC1155BatchReceived} on the receiver address if it
+     * contains code (eg. is a smart contract at the moment of execution).
+     *
+     * IMPORTANT: Overriding this function is discouraged because it poses a reentrancy risk from the receiver. So any
+     * update to the contract state after this function would break the check-effect-interaction pattern. Consider
+     * overriding {_update} instead.
+     */
+    function _updateWithAcceptanceCheck(
+        address from,
+        address to,
+        uint256[] memory ids,
+        uint256[] memory values,
+        bytes memory data,
+        bool batch
     ) internal virtual {
         _update(from, to, ids, values);
         if (to != address(0)) {
             address operator = _msgSender();
-            if (ids.length == 1) {
+            if (batch) {
+                ERC1155Utils.checkOnERC1155BatchReceived(operator, from, to, ids, values, data);
+            } else {
                 uint256 id = ids.unsafeMemoryAccess(0);
                 uint256 value = values.unsafeMemoryAccess(0);
                 ERC1155Utils.checkOnERC1155Received(operator, from, to, id, value, data);
-            } else {
-                ERC1155Utils.checkOnERC1155BatchReceived(operator, from, to, ids, values, data);
             }
         }
     }
@@ -219,7 +242,7 @@ abstract contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI, IER
             revert ERC1155InvalidSender(address(0));
         }
         (uint256[] memory ids, uint256[] memory values) = _asSingletonArrays(id, value);
-        _updateWithAcceptanceCheck(from, to, ids, values, data);
+        _updateWithAcceptanceCheck(from, to, ids, values, data, false);
     }
 
     /**
@@ -246,7 +269,7 @@ abstract contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI, IER
         if (from == address(0)) {
             revert ERC1155InvalidSender(address(0));
         }
-        _updateWithAcceptanceCheck(from, to, ids, values, data);
+        _updateWithAcceptanceCheck(from, to, ids, values, data, true);
     }
 
     /**
@@ -288,7 +311,7 @@ abstract contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI, IER
             revert ERC1155InvalidReceiver(address(0));
         }
         (uint256[] memory ids, uint256[] memory values) = _asSingletonArrays(id, value);
-        _updateWithAcceptanceCheck(address(0), to, ids, values, data);
+        _updateWithAcceptanceCheck(address(0), to, ids, values, data, false);
     }
 
     /**
@@ -307,7 +330,7 @@ abstract contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI, IER
         if (to == address(0)) {
             revert ERC1155InvalidReceiver(address(0));
         }
-        _updateWithAcceptanceCheck(address(0), to, ids, values, data);
+        _updateWithAcceptanceCheck(address(0), to, ids, values, data, true);
     }
 
     /**
@@ -325,7 +348,7 @@ abstract contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI, IER
             revert ERC1155InvalidSender(address(0));
         }
         (uint256[] memory ids, uint256[] memory values) = _asSingletonArrays(id, value);
-        _updateWithAcceptanceCheck(from, address(0), ids, values, "");
+        _updateWithAcceptanceCheck(from, address(0), ids, values, "", false);
     }
 
     /**
@@ -343,7 +366,7 @@ abstract contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI, IER
         if (from == address(0)) {
             revert ERC1155InvalidSender(address(0));
         }
-        _updateWithAcceptanceCheck(from, address(0), ids, values, "");
+        _updateWithAcceptanceCheck(from, address(0), ids, values, "", true);
     }
 
     /**

test/token/ERC1155/ERC1155.behavior.js

@@ -147,6 +147,24 @@ function shouldBehaveLikeERC1155() {
           .withArgs(this.holder, firstTokenValue, firstTokenValue + 1n, firstTokenId);
       });
 
+      it('reverts when transferring from zero address', async function () {
+        await expect(
+          this.token
+            .connect(this.holder)
+            .safeTransferFrom(ethers.ZeroAddress, this.holder, firstTokenId, firstTokenValue, '0x'),
+        )
+          .to.be.revertedWithCustomError(this.token, 'ERC1155MissingApprovalForAll')
+          .withArgs(this.holder, ethers.ZeroAddress);
+
+        await expect(
+          this.token
+            .connect(this.holder)
+            .$_safeTransferFrom(ethers.ZeroAddress, this.holder, firstTokenId, firstTokenValue, '0x'),
+        )
+          .to.be.revertedWithCustomError(this.token, 'ERC1155InvalidSender')
+          .withArgs(ethers.ZeroAddress);
+      });
+
       it('reverts when transferring to zero address', async function () {
         await expect(
           this.token
@@ -442,6 +460,36 @@ function shouldBehaveLikeERC1155() {
           .withArgs(ids2.length, tokenValues2.length);
       });
 
+      it('reverts when transferring from zero address', async function () {
+        await expect(
+          this.token
+            .connect(this.holder)
+            .safeBatchTransferFrom(
+              ethers.ZeroAddress,
+              this.holder,
+              [firstTokenId, secondTokenId],
+              [firstTokenValue, secondTokenValue],
+              '0x',
+            ),
+        )
+          .to.be.revertedWithCustomError(this.token, 'ERC1155MissingApprovalForAll')
+          .withArgs(this.holder, ethers.ZeroAddress);
+
+        await expect(
+          this.token
+            .connect(this.holder)
+            .$_safeBatchTransferFrom(
+              ethers.ZeroAddress,
+              this.holder,
+              [firstTokenId, secondTokenId],
+              [firstTokenValue, secondTokenValue],
+              '0x',
+            ),
+        )
+          .to.be.revertedWithCustomError(this.token, 'ERC1155InvalidSender')
+          .withArgs(ethers.ZeroAddress);
+      });
+
       it('reverts when transferring to zero address', async function () {
         await expect(
           this.token
@@ -484,9 +532,15 @@ function shouldBehaveLikeERC1155() {
         });
 
         it('emits a TransferBatch log', async function () {
-          await expect(this.tx)
-            .to.emit(this.token, 'TransferBatch')
-            .withArgs(this.args.operator, this.args.from, this.args.to, this.args.ids, this.args.values);
+          if (this.args.ids.length == 1) {
+            await expect(this.tx)
+              .to.emit(this.token, 'TransferSingle')
+              .withArgs(this.args.operator, this.args.from, this.args.to, this.args.ids[0], this.args.values[0]);
+          } else {
+            await expect(this.tx)
+              .to.emit(this.token, 'TransferBatch')
+              .withArgs(this.args.operator, this.args.from, this.args.to, this.args.ids, this.args.values);
+          }
         });
       }
 
@@ -566,7 +620,31 @@ function shouldBehaveLikeERC1155() {
           ]);
         });
 
-        describe('without data', function () {
+        describe('without data (batch of size = 1)', function () {
+          beforeEach(async function () {
+            this.args = {
+              operator: this.holder,
+              from: this.holder,
+              to: this.receiver,
+              ids: [firstTokenId],
+              values: [firstTokenValue],
+              data: '0x',
+            };
+            this.tx = await this.token
+              .connect(this.args.operator)
+              .safeBatchTransferFrom(this.args.from, this.args.to, this.args.ids, this.args.values, this.args.data);
+          });
+
+          batchTransferWasSuccessful();
+
+          it('calls onERC1155BatchReceived', async function () {
+            await expect(this.tx)
+              .to.emit(this.receiver, 'BatchReceived')
+              .withArgs(this.holder, this.holder, this.args.ids, this.args.values, this.args.data, anyValue);
+          });
+        });
+
+        describe('without data (batch of size > 1)', function () {
           beforeEach(async function () {
             this.args = {
               operator: this.holder,
@@ -590,7 +668,31 @@ function shouldBehaveLikeERC1155() {
           });
         });
 
-        describe('with data', function () {
+        describe('with data (batch of size = 1)', function () {
+          beforeEach(async function () {
+            this.args = {
+              operator: this.holder,
+              from: this.holder,
+              to: this.receiver,
+              ids: [firstTokenId],
+              values: [firstTokenValue],
+              data: '0xf00dd00d',
+            };
+            this.tx = await this.token
+              .connect(this.args.operator)
+              .safeBatchTransferFrom(this.args.from, this.args.to, this.args.ids, this.args.values, this.args.data);
+          });
+
+          batchTransferWasSuccessful();
+
+          it('calls onERC1155BatchReceived', async function () {
+            await expect(this.tx)
+              .to.emit(this.receiver, 'BatchReceived')
+              .withArgs(this.holder, this.holder, this.args.ids, this.args.values, this.args.data, anyValue);
+          });
+        });
+
+        describe('with data (batch of size > 1)', function () {
           beforeEach(async function () {
             this.args = {
               operator: this.holder,

test/token/ERC1155/ERC1155.test.js

@@ -2,6 +2,7 @@ const { ethers } = require('hardhat');
 const { expect } = require('chai');
 const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
 
+const { RevertType } = require('../../helpers/enums');
 const { zip } = require('../../helpers/iterate');
 const { shouldBehaveLikeERC1155 } = require('./ERC1155.behavior');
 
@@ -181,6 +182,42 @@ describe('ERC1155', function () {
       });
     });
 
+    describe('_updateWithAcceptanceCheck', function () {
+      beforeEach(async function () {
+        const factory = await ethers.getContractFactory('$ERC1155ReceiverMock');
+        this.receiver = await ethers.deployContract('$ERC1155ReceiverMock', [
+          factory.interface.getFunction('onERC1155Received').selector,
+          factory.interface.getFunction('onERC1155BatchReceived').selector,
+          RevertType.None,
+        ]);
+      });
+
+      it('calls onERC1155Received when only one token is transferred', async function () {
+        await expect(
+          this.token.$_updateWithAcceptanceCheck(ethers.ZeroAddress, this.receiver, [tokenId], [mintValue], '0x'),
+        ).to.emit(this.receiver, 'Received');
+      });
+
+      it('calls onERC1155BatchReceived when only one token is transferred and batch flag is set to true', async function () {
+        await expect(
+          this.token.$_updateWithAcceptanceCheck(
+            ethers.ZeroAddress,
+            this.receiver,
+            [tokenId],
+            [mintValue],
+            '0x',
+            ethers.Typed.bool(true),
+          ),
+        ).to.emit(this.receiver, 'BatchReceived');
+      });
+
+      it('calls onERC1155BatchReceived when more than one token is transferred', async function () {
+        await expect(
+          this.token.$_updateWithAcceptanceCheck(ethers.ZeroAddress, this.receiver, tokenBatchIds, mintValues, '0x'),
+        ).to.emit(this.receiver, 'BatchReceived');
+      });
+    });
+
     describe('_setApprovalForAll', function () {
       it("reverts when adding an operator over the zero account's tokens", async function () {
         await expect(this.token.$_setApprovalForAll(ethers.ZeroAddress, this.operator, true))