diff --git a/.changeset/metal-walls-doubt.md b/.changeset/metal-walls-doubt.md new file mode 100644 index 00000000..dc674c67 --- /dev/null +++ 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 index e737d093..13977e96 100644 --- 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 index 5d53f260..1350e404 100644 --- 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 new file mode 100644 index 00000000..b1c3c433 --- /dev/null +++ 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 index d5032a86..8e4af959 100644 --- 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';