Fix: Remove recipients from the array

contracts/payments/PaymentRouterNative.sol

@@ -254,9 +254,21 @@ contract PaymentRouterNative is
     function removeFeeRecipient(address recipient) external onlyRole(MANAGER_ROLE) {
         if (!feeRecipients[recipient].active) revert FeeRecipientDoesNotExist();
 
+        // Remove from mapping
         feeRecipients[recipient].active = false;
         feeRecipients[recipient].percentage = 0;
 
+        // Remove from array by replacing with last element and popping
+        for (uint256 i = 0; i < feeRecipientAddresses.length; i++) {
+            if (feeRecipientAddresses[i] == recipient) {
+                if (i != feeRecipientAddresses.length - 1) {
+                    feeRecipientAddresses[i] = feeRecipientAddresses[feeRecipientAddresses.length - 1];
+                }
+                feeRecipientAddresses.pop();
+                break;
+            }
+        }
+
         emit FeeRecipientRemoved(recipient);
     }
 

package.json

@@ -102,5 +102,6 @@
         "ethers": "^6.13.4",
         "hardhat-contract-sizer": "^2.10.0",
         "viem": "^2.21.51"
-    }
+    },
+    "packageManager": "pnpm@9.15.2+sha512.93e57b0126f0df74ce6bff29680394c0ba54ec47246b9cf321f0121d8d9bb03f750a705f24edc3c1180853afd7c2c3b94196d0a3d53d3e069d9e2793ef11f321"
 }

test/hardhatTests/paymentRouterNative.test.ts

@@ -244,7 +244,7 @@ describe('PaymentRouterNative', function () {
         });
 
         it('Should revert when non-manager tries to pause', async function () {
-            const { paymentRouter, deployer, user1 } = await loadFixture(deployPaymentRouterFixture);
+            const { paymentRouter, deployer, managerWallet, user1 } = await loadFixture(deployPaymentRouterFixture);
 
             await paymentRouter.connect(deployer).setPaymentConfig(PAYMENT_ID, PAYMENT_AMOUNT, PAYMENT_URI);
 
@@ -401,5 +401,121 @@ describe('PaymentRouterNative', function () {
                 ).to.be.revertedWithCustomError(paymentRouter, 'TotalPercentageExceedsLimit');
             });
         });
+
+        it('Should remove fee recipient from array and prevent duplicates when re-adding', async function () {
+            const { paymentRouter, managerWallet, recipient1 } = await loadFixture(deployPaymentRouterFixture);
+
+            // Get initial array length
+            const [initialRecipients] = await paymentRouter.getFeeRecipients();
+            const initialLength = initialRecipients.length;
+
+            // Remove recipient1
+            await paymentRouter.connect(managerWallet).removeFeeRecipient(recipient1.address);
+
+            // Verify array length decreased
+            const [afterRemovalRecipients] = await paymentRouter.getFeeRecipients();
+            expect(afterRemovalRecipients.length).to.equal(initialLength - 1);
+            expect(afterRemovalRecipients).to.not.include(recipient1.address);
+
+            // Re-add the same recipient
+            await paymentRouter.connect(managerWallet).setFeeRecipient(recipient1.address, 3000);
+
+            // Verify array length increased by 1 and recipient is present
+            const [finalRecipients] = await paymentRouter.getFeeRecipients();
+            expect(finalRecipients.length).to.equal(initialLength);
+            expect(finalRecipients).to.include(recipient1.address);
+
+            // Verify only one instance exists
+            const recipientCount = finalRecipients.filter(r => r === recipient1.address).length;
+            expect(recipientCount).to.equal(1);
+        });
+
+        it('Should handle multiple recipient removals and additions correctly', async function () {
+            const { paymentRouter, managerWallet, multiSigWallet, recipient1 } = await loadFixture(deployPaymentRouterFixture);
+
+            // Remove both initial recipients
+            await paymentRouter.connect(managerWallet).removeFeeRecipient(recipient1.address);
+            await paymentRouter.connect(managerWallet).removeFeeRecipient(multiSigWallet.address);
+
+            // Verify array is empty
+            const [emptyRecipients] = await paymentRouter.getFeeRecipients();
+            expect(emptyRecipients.length).to.equal(0);
+
+            // Add three new recipients
+            const newRecipients = await Promise.all([
+                ethers.Wallet.createRandom(),
+                ethers.Wallet.createRandom(),
+                ethers.Wallet.createRandom()
+            ]);
+
+            for (let i = 0; i < newRecipients.length; i++) {
+                await paymentRouter.connect(managerWallet).setFeeRecipient(
+                    newRecipients[i].address,
+                    3333 // ~33.33% each
+                );
+            }
+
+            // Verify all new recipients are present
+            const [finalRecipients, finalPercentages] = await paymentRouter.getFeeRecipients();
+            expect(finalRecipients.length).to.equal(3);
+
+            for (const recipient of newRecipients) {
+                expect(finalRecipients).to.include(recipient.address);
+            }
+
+            // Remove middle recipient
+            await paymentRouter.connect(managerWallet).removeFeeRecipient(newRecipients[1].address);
+
+            // Verify array is updated correctly
+            const [afterMiddleRemovalRecipients] = await paymentRouter.getFeeRecipients();
+            expect(afterMiddleRemovalRecipients.length).to.equal(2);
+            expect(afterMiddleRemovalRecipients).to.include(newRecipients[0].address);
+            expect(afterMiddleRemovalRecipients).to.include(newRecipients[2].address);
+            expect(afterMiddleRemovalRecipients).to.not.include(newRecipients[1].address);
+        });
+
+        it('Should prevent duplicate payments when adding same recipient multiple times', async function () {
+            const { paymentRouter, managerWallet, deployer, user1, recipient1 } = await loadFixture(deployPaymentRouterFixture);
+
+            // Setup a payment configuration
+            const paymentAmount = ethers.parseEther('1.0');
+            await paymentRouter.connect(deployer).setPaymentConfig(PAYMENT_ID, paymentAmount, PAYMENT_URI);
+
+            // Remove all existing recipients
+            const [initialRecipients] = await paymentRouter.getFeeRecipients();
+            for (const recipient of initialRecipients) {
+                await paymentRouter.connect(managerWallet).removeFeeRecipient(recipient);
+            }
+
+            // Add recipient1 twice with 100% fee
+            await paymentRouter.connect(managerWallet).setFeeRecipient(recipient1.address, 10000); // First addition
+            await paymentRouter.connect(managerWallet).setFeeRecipient(recipient1.address, 10000); // Second addition
+
+            // Verify recipient appears only once in the array
+            const [recipients] = await paymentRouter.getFeeRecipients();
+            const recipientCount = recipients.filter(r => r === recipient1.address).length;
+            expect(recipientCount).to.equal(1, 'Recipient address should appear exactly once in the array');
+
+            // Get recipient's initial balance
+            const initialBalance = await ethers.provider.getBalance(recipient1.address);
+
+            // Generate signature for payment
+            const chainId = (await ethers.provider.getNetwork()).chainId;
+            const { seed, signature, nonce } = await generateRandomSeed({
+                smartContractAddress: await paymentRouter.getAddress(),
+                chainId: chainId,
+                decode: true,
+                address: user1.address,
+                signer: deployer,
+                rawData: { type: 'string[]', data: [BOX_ID] },
+            });
+
+            // Make payment
+            await paymentRouter.connect(user1).pay(PAYMENT_ID, nonce, seed, signature, { value: paymentAmount });
+
+            // Check recipient received exactly the payment amount once
+            const finalBalance = await ethers.provider.getBalance(recipient1.address);
+            expect(finalBalance - initialBalance).to.equal(paymentAmount, 'Recipient should receive payment exactly once');
+        });
     });
 });