From 76db11d996230e6ea69b64ad308d8c93a6a6eb54 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 4 Dec 2024 19:39:41 +0000 Subject: [PATCH 1/2] feat: configurable `deployBlock` --- packages/contracts-rfq/contracts/AdminV2.sol | 19 +++++++++++++++++++ .../contracts-rfq/contracts/FastBridgeV2.sol | 6 +----- .../contracts/interfaces/IAdminV2.sol | 6 ++++++ 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/packages/contracts-rfq/contracts/AdminV2.sol b/packages/contracts-rfq/contracts/AdminV2.sol index 64806426c3..14ad93a3bc 100644 --- a/packages/contracts-rfq/contracts/AdminV2.sol +++ b/packages/contracts-rfq/contracts/AdminV2.sol @@ -60,6 +60,12 @@ contract AdminV2 is AccessControlEnumerable, IAdminV2, IAdminV2Errors { /// @notice The delay period after which a transaction can be permissionlessly cancelled. uint256 public cancelDelay; + /// @notice The block number at which the contract was deployed. + /// @dev This used to be immutable in V1, but was adjusted to be mutable in V2 for chains like Arbitrum that + /// implement the `block.number` as the underlying L1 block number rather than the chain's native block number. + /// This is exposed for conveniece for off-chain indexers that need to know the deployment block. + uint256 public deployBlock; + /// @notice This variable is deprecated and should not be used. /// @dev Use ZapNative V2 requests instead. uint256 public immutable chainGasAmount = 0; @@ -67,6 +73,7 @@ contract AdminV2 is AccessControlEnumerable, IAdminV2, IAdminV2Errors { constructor(address defaultAdmin) { _grantRole(DEFAULT_ADMIN_ROLE, defaultAdmin); _setCancelDelay(DEFAULT_CANCEL_DELAY); + _setDeployBlock(block.number); } /// @inheritdoc IAdminV2 @@ -74,6 +81,11 @@ contract AdminV2 is AccessControlEnumerable, IAdminV2, IAdminV2Errors { _setCancelDelay(newCancelDelay); } + /// @inheritdoc IAdminV2 + function setDeployBlock(uint256 blockNumber) external onlyRole(DEFAULT_ADMIN_ROLE) { + _setDeployBlock(blockNumber); + } + /// @inheritdoc IAdminV2 function setProtocolFeeRate(uint256 newFeeRate) external onlyRole(GOVERNOR_ROLE) { if (newFeeRate > FEE_RATE_MAX) revert FeeRateAboveMax(); @@ -106,4 +118,11 @@ contract AdminV2 is AccessControlEnumerable, IAdminV2, IAdminV2Errors { cancelDelay = newCancelDelay; emit CancelDelayUpdated(oldCancelDelay, newCancelDelay); } + + /// @notice Internal logic to set the deploy block. Security checks are performed outside of this function. + /// @dev This function is marked as private to prevent child contracts from calling it directly. + function _setDeployBlock(uint256 blockNumber) private { + deployBlock = blockNumber; + emit DeployBlockSet(blockNumber); + } } diff --git a/packages/contracts-rfq/contracts/FastBridgeV2.sol b/packages/contracts-rfq/contracts/FastBridgeV2.sol index f5e9cbd363..836c26262a 100644 --- a/packages/contracts-rfq/contracts/FastBridgeV2.sol +++ b/packages/contracts-rfq/contracts/FastBridgeV2.sol @@ -52,14 +52,10 @@ contract FastBridgeV2 is AdminV2, MulticallTarget, IFastBridgeV2, IFastBridgeV2E /// @notice This variable is deprecated and should not be used. /// @dev Replaced by senderNonces. uint256 public immutable nonce = 0; - /// @notice The block number at which this contract was deployed. - uint256 public immutable deployBlock; /// @notice Initializes the FastBridgeV2 contract with the provided default admin, /// sets the default cancel delay, and records the deploy block number. - constructor(address defaultAdmin) AdminV2(defaultAdmin) { - deployBlock = block.number; - } + constructor(address defaultAdmin) AdminV2(defaultAdmin) {} // ══════════════════════════════════════ EXTERNAL MUTABLE (USER FACING) ═══════════════════════════════════════════ diff --git a/packages/contracts-rfq/contracts/interfaces/IAdminV2.sol b/packages/contracts-rfq/contracts/interfaces/IAdminV2.sol index 90115d2f49..0720cce4ac 100644 --- a/packages/contracts-rfq/contracts/interfaces/IAdminV2.sol +++ b/packages/contracts-rfq/contracts/interfaces/IAdminV2.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.4; interface IAdminV2 { event CancelDelayUpdated(uint256 oldCancelDelay, uint256 newCancelDelay); + event DeployBlockSet(uint256 blockNumber); event FeeRateUpdated(uint256 oldFeeRate, uint256 newFeeRate); event FeesSwept(address token, address recipient, uint256 amount); @@ -10,6 +11,11 @@ interface IAdminV2 { /// deadline during which a transaction can be permissionlessly cancelled if it hasn't been proven by any Relayer. function setCancelDelay(uint256 newCancelDelay) external; + /// @notice Allows the default admin to set the deploy block. + /// @dev This is only relevant for chains like Arbitrum that implement the `block.number` as the underlying L1 + /// block number rather than the chain's native block number. + function setDeployBlock(uint256 blockNumber) external; + /// @notice Allows the governor to set the protocol fee rate. The protocol fee is taken from the origin /// amount and is only applied to completed and claimed transactions. /// @dev The protocol fee is abstracted away from the relayers; they always operate using the amounts after fees. From c65b45152b15d1a54acbe9ad9492ba041d2941f8 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 4 Dec 2024 19:39:48 +0000 Subject: [PATCH 2/2] test: add coverage --- .../test/FastBridgeV2.Management.t.sol | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/packages/contracts-rfq/test/FastBridgeV2.Management.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Management.t.sol index ab9ea59341..4c61883aa1 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.Management.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.Management.t.sol @@ -18,6 +18,7 @@ contract FastBridgeV2ManagementTest is FastBridgeV2Test, IAdminV2Errors { address public governorA = makeAddr("Governor A"); event CancelDelayUpdated(uint256 oldCancelDelay, uint256 newCancelDelay); + event DeployBlockSet(uint256 blockNumber); event FeeRateUpdated(uint256 oldFeeRate, uint256 newFeeRate); event FeesSwept(address token, address recipient, uint256 amount); @@ -46,6 +47,11 @@ contract FastBridgeV2ManagementTest is FastBridgeV2Test, IAdminV2Errors { fastBridge.setCancelDelay(newCancelDelay); } + function setDeployBlock(address caller, uint256 blockNumber) public { + vm.prank(caller); + fastBridge.setDeployBlock(blockNumber); + } + function setProtocolFeeRate(address caller, uint256 newFeeRate) public { vm.prank(caller); fastBridge.setProtocolFeeRate(newFeeRate); @@ -68,8 +74,10 @@ contract FastBridgeV2ManagementTest is FastBridgeV2Test, IAdminV2Errors { setGovernor(caller, governorA); } - function test_defaultCancelDelay() public view { + function test_defaultValues() public view { assertEq(fastBridge.cancelDelay(), DEFAULT_CANCEL_DELAY); + assertEq(fastBridge.deployBlock(), block.number); + assertEq(fastBridge.protocolFeeRate(), 0); } // ═════════════════════════════════════════════ SET CANCEL DELAY ══════════════════════════════════════════════════ @@ -100,6 +108,21 @@ contract FastBridgeV2ManagementTest is FastBridgeV2Test, IAdminV2Errors { setCancelDelay(caller, 4 days); } + // ═════════════════════════════════════════════ SET DEPLOY BLOCK ══════════════════════════════════════════════════ + + function test_setDeployBlock() public { + vm.expectEmit(address(fastBridge)); + emit DeployBlockSet(123_456); + setDeployBlock(admin, 123_456); + assertEq(fastBridge.deployBlock(), 123_456); + } + + function test_setDeployBlock_revertNotAdmin(address caller) public { + vm.assume(caller != admin); + expectUnauthorized(caller, fastBridge.DEFAULT_ADMIN_ROLE()); + setDeployBlock(caller, 123_456); + } + // ═══════════════════════════════════════════ SET PROTOCOL FEE RATE ═══════════════════════════════════════════════ function test_setProtocolFeeRate() public {