Fix: Remove useless batch recipient fee

G7DAO · Jan 15, 2025 · 6b529d7 · 6b529d7
1 parent eeddb97
commit 6b529d7
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 96 deletions.
diff --git a/contracts/payments/PaymentRouterNative.sol b/contracts/payments/PaymentRouterNative.sol
@@ -338,39 +338,6 @@ contract PaymentRouterNative is
         return (config.active, config.percentage);
     }
 
-    /**
-     * @notice Batch updates fee recipients
-     * @param recipients Array of recipient addresses
-     * @param percentages Array of corresponding percentages
-     */
-    function batchSetFeeRecipients(
-        address[] calldata recipients,
-        uint256[] calldata percentages
-    ) external onlyRole(MANAGER_ROLE) {
-        if (recipients.length != percentages.length) revert InvalidPaymentId();
-
-        uint256 totalPercentage;
-        for (uint256 i = 0; i < recipients.length; i++) {
-            if (recipients[i] == address(0)) revert InvalidRecipientAddress();
-            if (percentages[i] > HUNDRED_PERCENT) revert InvalidPercentage();
-            totalPercentage += percentages[i];
-        }
-        if (totalPercentage > HUNDRED_PERCENT) revert TotalPercentageExceedsLimit();
-
-        for (uint256 i = 0; i < recipients.length; i++) {
-            if (!feeRecipients[recipients[i]].active) {
-                feeRecipientAddresses.push(recipients[i]);
-            }
-
-            feeRecipients[recipients[i]] = FeeRecipient({
-                active: true,
-                percentage: percentages[i]
-            });
-
-            emit FeeRecipientUpdated(recipients[i], percentages[i]);
-        }
-    }
-
     /**
      * @notice Makes a payment for a specific ID
      * @param id The payment ID

diff --git a/package.json b/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"
 }
diff --git a/test/hardhatTests/paymentRouterNative.test.ts b/test/hardhatTests/paymentRouterNative.test.ts
@@ -27,9 +27,12 @@ describe('PaymentRouterNative', function () {
         const recipient1Fee = 3000;
 
         // Setup fee recipients
-        await paymentRouter.connect(managerWallet).batchSetFeeRecipients(
-            [multiSigWallet.address, recipient1.address],
-            [defaultMultisigFee, recipient1Fee] // 70% and 30%
+        await paymentRouter.connect(managerWallet).setFeeRecipient(
+            multiSigWallet.address, defaultMultisigFee
+        );
+
+        await paymentRouter.connect(managerWallet).setFeeRecipient(
+            recipient1.address, recipient1Fee
         );
 
         const chainId = (await ethers.provider.getNetwork()).chainId;
@@ -286,7 +289,7 @@ describe('PaymentRouterNative', function () {
         });
 
         it('Should remove fee recipient correctly', async function () {
-            const { paymentRouter, managerWallet, recipient1 } = await loadFixture(deployPaymentRouterFixture);
+            const { paymentRouter, managerWallet, multiSigWallet, recipient1 } = await loadFixture(deployPaymentRouterFixture);
 
             // First remove recipient1 (30%)
             await paymentRouter.connect(managerWallet).removeFeeRecipient(recipient1.address);
@@ -298,6 +301,13 @@ describe('PaymentRouterNative', function () {
             // Verify only multisig remains with 70%
             const totalPercentage = await paymentRouter.getTotalFeePercentage();
             expect(totalPercentage).to.equal(7000);
+
+            // Second remove multisig 70%
+            await paymentRouter.connect(managerWallet).removeFeeRecipient(multiSigWallet.address);
+
+            // Verify total percentage is 0
+            const totalPercentageAfterRemoval = await paymentRouter.getTotalFeePercentage();
+            expect(totalPercentageAfterRemoval).to.equal(0);
         });
 
         it('Should get all active fee recipients correctly', async function () {
@@ -317,39 +327,6 @@ describe('PaymentRouterNative', function () {
             expect(percentages[1]).to.equal(3000);
         });
 
-        it('Should batch set fee recipients correctly', async function () {
-            const { paymentRouter, managerWallet,  recipient1, multiSigWallet } = await loadFixture(deployPaymentRouterFixture);
-
-
-            // First remove recipient1 (30%)
-            await paymentRouter.connect(managerWallet).removeFeeRecipient(recipient1.address);
-
-            // Second remove multisig 70%
-            await paymentRouter.connect(managerWallet).removeFeeRecipient(multiSigWallet.address);
-
-            const newRecipients = await Promise.all([
-                ethers.Wallet.createRandom(),
-                ethers.Wallet.createRandom(),
-                ethers.Wallet.createRandom()
-            ]);
-
-            await paymentRouter.connect(managerWallet).batchSetFeeRecipients(
-                newRecipients.map(r => r.address),
-                [4000, 3000, 3000] // 40%, 30%, 30%
-            );
-
-            // Verify total percentage
-            const totalPercentage = await paymentRouter.getTotalFeePercentage();
-            expect(totalPercentage).to.equal(10000);
-
-            // Verify individual recipients
-            for (let i = 0; i < newRecipients.length; i++) {
-                const [active, percentage] = await paymentRouter.getFeeRecipient(newRecipients[i].address);
-                expect(active).to.be.true;
-                expect(percentage).to.equal(i === 0 ? 4000 : 3000);
-            }
-        });
-
         describe('Error cases', function () {
             it('Should revert when setting zero address as recipient', async function () {
                 const { paymentRouter, managerWallet } = await loadFixture(deployPaymentRouterFixture);
@@ -375,31 +352,6 @@ describe('PaymentRouterNative', function () {
                     paymentRouter.connect(managerWallet).removeFeeRecipient(nonExistentRecipient.address)
                 ).to.be.revertedWithCustomError(paymentRouter, 'FeeRecipientDoesNotExist');
             });
-
-            it('Should revert when batch setting with mismatched arrays', async function () {
-                const { paymentRouter, managerWallet } = await loadFixture(deployPaymentRouterFixture);
-                const recipients = [await ethers.Wallet.createRandom(), await ethers.Wallet.createRandom()];
-                const percentages = [5000];
-
-                await expect(
-                    paymentRouter.connect(managerWallet).batchSetFeeRecipients(
-                        recipients.map(r => r.address),
-                        percentages
-                    )
-                ).to.be.revertedWithCustomError(paymentRouter, 'InvalidPaymentId');
-            });
-
-            it('Should revert when batch total exceeds 100%', async function () {
-                const { paymentRouter, managerWallet } = await loadFixture(deployPaymentRouterFixture);
-                const recipients = [await ethers.Wallet.createRandom(), await ethers.Wallet.createRandom()];
-
-                await expect(
-                    paymentRouter.connect(managerWallet).batchSetFeeRecipients(
-                        recipients.map(r => r.address),
-                        [6000, 5000] // 110%
-                    )
-                ).to.be.revertedWithCustomError(paymentRouter, 'TotalPercentageExceedsLimit');
-            });
         });
 
         it('Should remove fee recipient from array and prevent duplicates when re-adding', async function () {