fix: repair copyBatchBulk migration import

scripts/migration/import.ts

@@ -23,11 +23,6 @@ async function main() {
   const contractAddress = '0xcdfdC3752caaA826fE62531E0000C40546eC56A6';
   const contract = await ethers.getContractAt('PostageStamp', contractAddress);
 
-  // Add Admin Role to the contract address itself as it is calling itself with this.copyBatch function
-  const adminRole = await contract.DEFAULT_ADMIN_ROLE();
-  const tx0 = await contract.grantRole(adminRole, contractAddress);
-  console.log('Added Admin Role to contract itself : ', tx0.hash);
-
   // A numerator to keep track of the batch group number
   let groupNumber = 0;
   const batchGroups: Batch[][] = chunkArray(batches, chunkSize);

src/PostageStamp.sol

@@ -261,22 +261,42 @@ contract PostageStamp is AccessControl, Pausable {
             revert AdministratorOnly();
         }
 
+        if (!_copyBatch(_owner, _initialBalancePerChunk, _depth, _bucketDepth, _batchId, _immutable)) {
+            if (_owner == address(0)) revert ZeroAddress();
+            if (_bucketDepth == 0 || _bucketDepth >= _depth) revert InvalidDepth();
+            if (batches[_batchId].owner != address(0)) revert BatchExists();
+            if (currentTotalOutPayment() + _initialBalancePerChunk == 0) revert ZeroBalance();
+        }
+    }
+
+    function _copyBatch(
+        address _owner,
+        uint256 _initialBalancePerChunk,
+        uint8 _depth,
+        uint8 _bucketDepth,
+        bytes32 _batchId,
+        bool _immutable
+    ) internal returns (bool) {
+        if (paused()) {
+            return false;
+        }
+
         if (_owner == address(0)) {
-            revert ZeroAddress();
+            return false;
         }
 
         if (_bucketDepth == 0 || _bucketDepth >= _depth) {
-            revert InvalidDepth();
+            return false;
         }
 
         if (batches[_batchId].owner != address(0)) {
-            revert BatchExists();
+            return false;
         }
 
         uint256 totalAmount = _initialBalancePerChunk * (1 << _depth);
         uint256 normalisedBalance = currentTotalOutPayment() + (_initialBalancePerChunk);
         if (normalisedBalance == 0) {
-            revert ZeroBalance();
+            return false;
         }
 
         //update validChunkCount to remove currently expired batches
@@ -296,6 +316,8 @@ contract PostageStamp is AccessControl, Pausable {
         tree.insert(_batchId, normalisedBalance);
 
         emit BatchCreated(_batchId, totalAmount, normalisedBalance, _owner, _depth, _bucketDepth, _immutable);
+
+        return true;
     }
 
     /**
@@ -310,19 +332,16 @@ contract PostageStamp is AccessControl, Pausable {
         }
         for (uint i = 0; i < bulkBatches.length; i++) {
             ImportBatch memory _batch = bulkBatches[i];
-            try
-                this.copyBatch(
+            if (
+                !_copyBatch(
                     _batch.owner,
                     _batch.remainingBalance,
                     _batch.depth,
                     _batch.bucketDepth,
                     _batch.batchId,
                     _batch.immutableFlag
                 )
-            {
-                // Successful copyBatch call
-            } catch {
-                // copyBatch failed, handle error
+            ) {
                 emit CopyBatchFailed(i, _batch.batchId);
             }
         }

test/PostageStamp.test.ts

@@ -50,7 +50,7 @@
 };
 
 describe('PostageStamp', function () {
   let minimumPrice: number;
   describe('when deploying contract', function () {
     beforeEach(async function () {
       await deployments.fixture();
@@ -234,7 +234,7 @@
         expect(batch0).equal(await postageStampStamper.firstBatchId());
 
         const blocksElapsed = (await getBlockNumber()) - setPrice0Block;
         const expectedNormalisedBalance1 = initialPaymentPerChunk1 + blocksElapsed * price0;
 
         const nonce1 = '0x0000000000000000000000000000000000000000000000000000000000001235';
         await postageStampStamper.createBatch(
@@ -1439,5 +1439,111 @@
         expect(stamp[4]).to.equal(expectedNormalisedBalance);
       });
     });
+
+    describe('when copyBatchBulk imports batches', function () {
+      beforeEach(async function () {
+        postageStampStamper = await ethers.getContract('PostageStamp', stamper);
+        const postageStampDeployer = await ethers.getContract('PostageStamp', deployer);
+        const admin = await postageStampStamper.DEFAULT_ADMIN_ROLE();
+        await postageStampDeployer.grantRole(admin, stamper);
+
+        batch = {
+          id: '0x000000000000000000000000000000000000000000000000000000000000abcd',
+          nonce: '0x000000000000000000000000000000000000000000000000000000000000abcd',
+          initialPaymentPerChunk: 10240,
+          depth: 17,
+          immutable: false,
+          bucketDepth: 16,
+        };
+      });
+
+      it('should import valid batches without granting admin role to the contract', async function () {
+        const nonce1 = '0x0000000000000000000000000000000000000000000000000000000000001234';
+        const batch1 = computeBatchId(stamper, nonce1);
+        const nonce2 = '0x0000000000000000000000000000000000000000000000000000000000001235';
+        const batch2 = computeBatchId(stamper, nonce2);
+
+        const bulkBatches = [
+          {
+            batchId: batch1,
+            owner: stamper,
+            depth: batch.depth,
+            bucketDepth: batch.bucketDepth,
+            immutableFlag: batch.immutable,
+            remainingBalance: 3300,
+          },
+          {
+            batchId: batch2,
+            owner: stamper,
+            depth: batch.depth,
+            bucketDepth: batch.bucketDepth,
+            immutableFlag: batch.immutable,
+            remainingBalance: 2200,
+          },
+        ];
+
+        const adminRole = await postageStampStamper.DEFAULT_ADMIN_ROLE();
+        expect(await postageStampStamper.hasRole(adminRole, postageStampStamper.address)).to.be.false;
+
+        await expect(postageStampStamper.copyBatchBulk(bulkBatches))
+          .to.emit(postageStampStamper, 'BatchCreated')
+          .withArgs(batch1, 3300 * 2 ** batch.depth, 3300, stamper, batch.depth, batch.bucketDepth, batch.immutable)
+          .and.to.emit(postageStampStamper, 'BatchCreated')
+          .withArgs(batch2, 2200 * 2 ** batch.depth, 2200, stamper, batch.depth, batch.bucketDepth, batch.immutable);
+
+        expect((await postageStampStamper.batches(batch1)).owner).to.equal(stamper);
+        expect((await postageStampStamper.batches(batch2)).owner).to.equal(stamper);
+      });
+
+      it('should emit CopyBatchFailed for invalid batches and continue importing valid ones', async function () {
+        const validNonce = '0x0000000000000000000000000000000000000000000000000000000000001234';
+        const validBatchId = computeBatchId(stamper, validNonce);
+        const duplicateBatchId = batch.nonce;
+
+        await postageStampStamper.copyBatch(
+          stamper,
+          batch.initialPaymentPerChunk,
+          batch.depth,
+          batch.bucketDepth,
+          duplicateBatchId,
+          batch.immutable
+        );
+
+        const bulkBatches = [
+          {
+            batchId: validBatchId,
+            owner: stamper,
+            depth: batch.depth,
+            bucketDepth: batch.bucketDepth,
+            immutableFlag: batch.immutable,
+            remainingBalance: 3300,
+          },
+          {
+            batchId: duplicateBatchId,
+            owner: stamper,
+            depth: batch.depth,
+            bucketDepth: batch.bucketDepth,
+            immutableFlag: batch.immutable,
+            remainingBalance: 2200,
+          },
+        ];
+
+        await expect(postageStampStamper.copyBatchBulk(bulkBatches))
+          .to.emit(postageStampStamper, 'BatchCreated')
+          .withArgs(
+            validBatchId,
+            3300 * 2 ** batch.depth,
+            3300,
+            stamper,
+            batch.depth,
+            batch.bucketDepth,
+            batch.immutable
+          )
+          .and.to.emit(postageStampStamper, 'CopyBatchFailed')
+          .withArgs(1, duplicateBatchId);
+
+        expect((await postageStampStamper.batches(validBatchId)).owner).to.equal(stamper);
+      });
+    });
   });
 });