Improve permission granularity for message blacklisting (#168)

Closes #163
FuelLabs · Apr 15, 2024 · 5c9a8e1 · 5c9a8e1
1 parent 9941f5a
commit 5c9a8e1
Show file tree

Hide file tree

Showing 5 changed files with 76 additions and 6 deletions.
diff --git a/.changeset/metal-walls-doubt.md b/.changeset/metal-walls-doubt.md
@@ -0,0 +1,5 @@
+---
+'@fuel-bridge/solidity-contracts': patch
+---
+
+Improve granularity of blacklisting permissions for messages in FuelMessagePortal
diff --git a/packages/solidity-contracts/contracts/fuelchain/FuelMessagePortal/v3/FuelMessagePortalV3.sol b/packages/solidity-contracts/contracts/fuelchain/FuelMessagePortal/v3/FuelMessagePortalV3.sol
@@ -20,12 +20,16 @@ contract FuelMessagePortalV3 is FuelMessagePortalV2 {
         withdrawalsPaused = true;
     }
 
-    function unpauseWithdrawals() external payable onlyRole(PAUSER_ROLE) {
+    function unpauseWithdrawals() external payable onlyRole(DEFAULT_ADMIN_ROLE) {
         withdrawalsPaused = false;
     }
 
-    function setMessageBlacklist(bytes32 messageId, bool value) external payable onlyRole(PAUSER_ROLE) {
-        messageIsBlacklisted[messageId] = value;
+    function addMessageToBlacklist(bytes32 messageId) external payable onlyRole(PAUSER_ROLE) {
+        messageIsBlacklisted[messageId] = true;
+    }
+
+    function removeMessageFromBlacklist(bytes32 messageId) external payable onlyRole(DEFAULT_ADMIN_ROLE) {
+        messageIsBlacklisted[messageId] = false;
     }
 
     ///////////////////////////////////////

diff --git a/packages/solidity-contracts/test/messagesIncomingV3.test.ts b/packages/solidity-contracts/test/messagesIncomingV3.test.ts
@@ -18,6 +18,7 @@ import type {
   FuelMessagePortalV3,
 } from '../typechain';
 
+import { createRandomWalletWithFunds } from './utils';
 import { addressToB256, b256ToAddress } from './utils/addressConversion';
 import { createBlock } from './utils/createBlock';
 import type { TreeNode } from './utils/merkle';
@@ -389,7 +390,31 @@ describe('FuelMessagePortalV3 - Incoming messages', () => {
       });
     });
 
-    describe('setMessageBlacklist', () => {
+    describe('addMessageToBlacklist', () => {
+      it('can only be called by pauser role', async () => {
+        const mallory = await createRandomWalletWithFunds();
+
+        const [msgID] = generateProof(
+          messageEOA,
+          blockHeaders,
+          prevBlockNodes,
+          blockIds,
+          messageNodes
+        );
+
+        const PAUSER_ROLE = await fuelMessagePortal.PAUSER_ROLE();
+
+        const tx = fuelMessagePortal
+          .connect(mallory)
+          .addMessageToBlacklist(msgID);
+
+        const expectedErrorMsg =
+          `AccessControl: account ${mallory.address.toLowerCase()} ` +
+          `is missing role ${PAUSER_ROLE}`;
+
+        await expect(tx).to.be.revertedWith(expectedErrorMsg);
+      });
+
       it('prevents withdrawals', async () => {
         // Blacklisted message
         {
@@ -402,7 +427,7 @@ describe('FuelMessagePortalV3 - Incoming messages', () => {
               messageNodes
             );
 
-          await fuelMessagePortal.setMessageBlacklist(msgID, true);
+          await fuelMessagePortal.addMessageToBlacklist(msgID);
 
           const relayTx = fuelMessagePortal.relayMessage(
             messageEOA,
@@ -440,7 +465,31 @@ describe('FuelMessagePortalV3 - Incoming messages', () => {
           );
           await expect(relayTx).to.not.be.reverted;
         }
+      });
+    });
+
+    describe('removeMessageFromBlacklist', () => {
+      it('can only be called by admin role', async () => {
+        const mallory = await createRandomWalletWithFunds();
+        const [msgID] = generateProof(
+          messageEOA,
+          blockHeaders,
+          prevBlockNodes,
+          blockIds,
+          messageNodes
+        );
+
+        const ADMIN_ROLE = await fuelMessagePortal.DEFAULT_ADMIN_ROLE();
+        const tx = fuelMessagePortal
+          .connect(mallory)
+          .removeMessageFromBlacklist(msgID);
 
+        const expectedErrorMsg =
+          `AccessControl: account ${mallory.address.toLowerCase()} ` +
+          `is missing role ${ADMIN_ROLE}`;
+        expect(tx).to.be.revertedWith(expectedErrorMsg);
+      });
+      it('restores ability to withdraw', async () => {
         // Whitelist back the blacklisted message
         {
           const [msgID, msgBlockHeader, blockInRoot, msgInBlock] =
@@ -457,7 +506,7 @@ describe('FuelMessagePortalV3 - Incoming messages', () => {
           await fuelMessagePortal.depositETH(messageEOA.recipient, {
             value: depositedAmount,
           });
-          await fuelMessagePortal.setMessageBlacklist(msgID, false);
+          await fuelMessagePortal.removeMessageFromBlacklist(msgID);
 
           const relayTx = fuelMessagePortal.relayMessage(
             messageEOA,

diff --git a/packages/solidity-contracts/test/utils/createRandomWalletWithFunds.ts b/packages/solidity-contracts/test/utils/createRandomWalletWithFunds.ts
@@ -0,0 +1,11 @@
+import { setBalance } from '@nomicfoundation/hardhat-network-helpers';
+import { Wallet, parseEther } from 'ethers';
+import hre from 'hardhat';
+
+export async function createRandomWalletWithFunds(funds = parseEther('10')) {
+  const wallet = Wallet.createRandom(hre.ethers.provider);
+
+  await setBalance(wallet.address, funds);
+
+  return wallet;
+}
diff --git a/packages/solidity-contracts/test/utils/index.ts b/packages/solidity-contracts/test/utils/index.ts
@@ -4,3 +4,4 @@ export * from './encodeErc20DepositMessage';
 export * from './impersonateAccount';
 export * from './merkle';
 export * from './deployProxy';
+export * from './createRandomWalletWithFunds';