From 2f300e6fa885f095bc4d98c7c2f6e3e20e11a058 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 4 Dec 2024 17:28:53 +0000 Subject: [PATCH] fix: Role Admin should be handling provers, as before --- packages/contracts-rfq/contracts/AdminV2.sol | 4 +-- .../contracts/interfaces/IAdminV2.sol | 4 +-- .../test/FastBridgeV2.Management.t.sol | 34 +++++++++---------- .../test/FastBridgeV2.Src.Base.t.sol | 5 +-- .../FastBridgeV2.MulticallTarget.t.sol | 1 - .../test/integration/TokenZapV1.t.sol | 1 - 6 files changed, 24 insertions(+), 25 deletions(-) diff --git a/packages/contracts-rfq/contracts/AdminV2.sol b/packages/contracts-rfq/contracts/AdminV2.sol index 645ea4524d..1b495c5d9f 100644 --- a/packages/contracts-rfq/contracts/AdminV2.sol +++ b/packages/contracts-rfq/contracts/AdminV2.sol @@ -90,7 +90,7 @@ contract AdminV2 is AccessControlEnumerable, IAdminV2, IAdminV2Errors { } /// @inheritdoc IAdminV2 - function addProver(address prover) external onlyRole(GOVERNOR_ROLE) { + function addProver(address prover) external onlyRole(DEFAULT_ADMIN_ROLE) { if (getActiveProverID(prover) != 0) revert ProverAlreadyActive(); ProverInfo storage $ = _proverInfos[prover]; // Add the prover to the list of all provers and record its id (its position + 1), @@ -109,7 +109,7 @@ contract AdminV2 is AccessControlEnumerable, IAdminV2, IAdminV2Errors { } /// @inheritdoc IAdminV2 - function removeProver(address prover) external onlyRole(GOVERNOR_ROLE) { + function removeProver(address prover) external onlyRole(DEFAULT_ADMIN_ROLE) { if (getActiveProverID(prover) == 0) revert ProverNotActive(); // We never remove provers from the list of all provers to preserve their IDs, // so we just need to reset the activeFrom timestamp. diff --git a/packages/contracts-rfq/contracts/interfaces/IAdminV2.sol b/packages/contracts-rfq/contracts/interfaces/IAdminV2.sol index ca22639900..feefff1174 100644 --- a/packages/contracts-rfq/contracts/interfaces/IAdminV2.sol +++ b/packages/contracts-rfq/contracts/interfaces/IAdminV2.sol @@ -11,10 +11,10 @@ interface IAdminV2 { event ProverRemoved(address prover); event ProverTimeoutApplied(address prover, uint256 inactiveUntilTimestamp); - /// @notice Allows the governor to add a new prover to the contract. + /// @notice Allows the role admin to add a new prover to the contract. function addProver(address prover) external; - /// @notice Allows the governor to remove a prover from the contract. + /// @notice Allows the role admin to remove a prover from the contract. function removeProver(address prover) external; /// @notice Allows the governor to set the cancel delay. The cancel delay is the time period after the transaction diff --git a/packages/contracts-rfq/test/FastBridgeV2.Management.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Management.t.sol index 668dd8088b..0250610956 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.Management.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.Management.t.sol @@ -119,7 +119,7 @@ contract FastBridgeV2ManagementTest is FastBridgeV2Test { uint256 proverAtime = block.timestamp; vm.expectEmit(address(fastBridge)); emit ProverAdded(proverA); - addProver(governor, proverA); + addProver(admin, proverA); assertEq(fastBridge.getActiveProverID(proverA), 1); assertEq(fastBridge.getActiveProverID(proverB), 0); address[] memory provers = fastBridge.getProvers(); @@ -136,7 +136,7 @@ contract FastBridgeV2ManagementTest is FastBridgeV2Test { uint256 proverBtime = block.timestamp; vm.expectEmit(address(fastBridge)); emit ProverAdded(proverB); - addProver(governor, proverB); + addProver(admin, proverB); assertEq(fastBridge.getActiveProverID(proverA), 1); assertEq(fastBridge.getActiveProverID(proverB), 2); address[] memory provers = fastBridge.getProvers(); @@ -154,7 +154,7 @@ contract FastBridgeV2ManagementTest is FastBridgeV2Test { uint256 proverBtime = block.timestamp; vm.expectEmit(address(fastBridge)); emit ProverAdded(proverB); - addProver(governor, proverB); + addProver(admin, proverB); assertEq(fastBridge.getActiveProverID(proverA), 0); assertEq(fastBridge.getActiveProverID(proverB), 2); address[] memory provers = fastBridge.getProvers(); @@ -167,7 +167,7 @@ contract FastBridgeV2ManagementTest is FastBridgeV2Test { uint256 proverAtime = block.timestamp; vm.expectEmit(address(fastBridge)); emit ProverAdded(proverA); - addProver(governor, proverA); + addProver(admin, proverA); assertEq(fastBridge.getActiveProverID(proverA), 1); assertEq(fastBridge.getActiveProverID(proverB), 2); provers = fastBridge.getProvers(); @@ -178,24 +178,24 @@ contract FastBridgeV2ManagementTest is FastBridgeV2Test { checkProverInfo(proverB, 2, proverBtime); } - function test_addProver_revertNotGovernor(address caller) public { - vm.assume(caller != governor); - expectUnauthorized(caller, fastBridge.GOVERNOR_ROLE()); + function test_addProver_revertNotAdmin(address caller) public { + vm.assume(caller != admin); + expectUnauthorized(caller, fastBridge.DEFAULT_ADMIN_ROLE()); addProver(caller, proverA); } function test_addProver_revertAlreadyActive() public { test_addProver(); vm.expectRevert(ProverAlreadyActive.selector); - addProver(governor, proverA); + addProver(admin, proverA); } function test_addProver_revertTooManyProvers() public { for (uint256 i = 0; i < type(uint16).max; i++) { - addProver(governor, address(uint160(i))); + addProver(admin, address(uint160(i))); } vm.expectRevert(ProverCapacityExceeded.selector); - addProver(governor, proverA); + addProver(admin, proverA); } // ═══════════════════════════════════════════════ REMOVE PROVER ═══════════════════════════════════════════════════ @@ -205,7 +205,7 @@ contract FastBridgeV2ManagementTest is FastBridgeV2Test { uint256 proverBtime = block.timestamp; vm.expectEmit(address(fastBridge)); emit ProverRemoved(proverA); - removeProver(governor, proverA); + removeProver(admin, proverA); assertEq(fastBridge.getActiveProverID(proverA), 0); assertEq(fastBridge.getActiveProverID(proverB), 2); address[] memory provers = fastBridge.getProvers(); @@ -219,7 +219,7 @@ contract FastBridgeV2ManagementTest is FastBridgeV2Test { test_removeProver(); vm.expectEmit(address(fastBridge)); emit ProverRemoved(proverB); - removeProver(governor, proverB); + removeProver(admin, proverB); assertEq(fastBridge.getActiveProverID(proverA), 0); assertEq(fastBridge.getActiveProverID(proverB), 0); address[] memory provers = fastBridge.getProvers(); @@ -228,21 +228,21 @@ contract FastBridgeV2ManagementTest is FastBridgeV2Test { checkProverInfo(proverB, 2, 0); } - function test_removeProver_revertNotGovernor(address caller) public { - vm.assume(caller != governor); - expectUnauthorized(caller, fastBridge.GOVERNOR_ROLE()); + function test_removeProver_revertNotAdmin(address caller) public { + vm.assume(caller != admin); + expectUnauthorized(caller, fastBridge.DEFAULT_ADMIN_ROLE()); removeProver(caller, proverA); } function test_removeProver_revertNeverBeenActive() public { vm.expectRevert(ProverNotActive.selector); - removeProver(governor, proverA); + removeProver(admin, proverA); } function test_removeProver_revertNotActive() public { test_removeProver(); vm.expectRevert(ProverNotActive.selector); - removeProver(governor, proverA); + removeProver(admin, proverA); } // ═════════════════════════════════════════════ SET CANCEL DELAY ══════════════════════════════════════════════════ diff --git a/packages/contracts-rfq/test/FastBridgeV2.Src.Base.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Src.Base.t.sol index 0abd31b012..ddc357f781 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.Src.Base.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.Src.Base.t.sol @@ -27,12 +27,13 @@ abstract contract FastBridgeV2SrcBaseTest is FastBridgeV2Test { } function configureFastBridge() public virtual override { + fastBridge.addProver(relayerA); + fastBridge.addProver(relayerB); + fastBridge.grantRole(fastBridge.GUARD_ROLE(), guard); fastBridge.grantRole(fastBridge.CANCELER_ROLE(), canceler); fastBridge.grantRole(fastBridge.GOVERNOR_ROLE(), address(this)); - fastBridge.addProver(relayerA); - fastBridge.addProver(relayerB); fastBridge.setCancelDelay(PERMISSIONLESS_CANCEL_DELAY); fastBridge.setProverTimeout(PROVER_TIMEOUT); } diff --git a/packages/contracts-rfq/test/integration/FastBridgeV2.MulticallTarget.t.sol b/packages/contracts-rfq/test/integration/FastBridgeV2.MulticallTarget.t.sol index db2753f4a8..794d0c9210 100644 --- a/packages/contracts-rfq/test/integration/FastBridgeV2.MulticallTarget.t.sol +++ b/packages/contracts-rfq/test/integration/FastBridgeV2.MulticallTarget.t.sol @@ -9,7 +9,6 @@ import {IFastBridge, MulticallTargetIntegrationTest} from "./MulticallTarget.t.s contract FastBridgeV2MulticallTargetTest is MulticallTargetIntegrationTest { function deployAndConfigureFastBridge() public override returns (address) { FastBridgeV2 fb = new FastBridgeV2(address(this)); - fb.grantRole(fb.GOVERNOR_ROLE(), address(this)); fb.addProver(relayer); return address(fb); } diff --git a/packages/contracts-rfq/test/integration/TokenZapV1.t.sol b/packages/contracts-rfq/test/integration/TokenZapV1.t.sol index 4fb2501e8f..e1edfc50e4 100644 --- a/packages/contracts-rfq/test/integration/TokenZapV1.t.sol +++ b/packages/contracts-rfq/test/integration/TokenZapV1.t.sol @@ -45,7 +45,6 @@ abstract contract TokenZapV1IntegrationTest is Test { function setUp() public virtual { fastBridge = new FastBridgeV2(address(this)); - fastBridge.grantRole(fastBridge.GOVERNOR_ROLE(), address(this)); fastBridge.addProver(relayer); srcToken = new MockERC20("SRC", 18);