From 7932f416341a227db295c33f232efde89fd2c50c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CF=87=C2=B2?= <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 18 Nov 2024 17:00:47 +0000 Subject: [PATCH] feat(contracts-rfq): rework permisionless cancellation [SLT-489] (#3382) * feat: start with identical AdminV2 * refactor: refund -> cancel * test: update the tests, add coverage for deprecated method * feat: scaffold `setCancelDelay` * test: coverage for `calcelDelay` management * feat: configurable cancel delay * refactor: custom error `FeeRateAboveMax` * test: chainGasAmount deprecation * feat: deprecate `chainGasAmount` * refactor: drop `UniversalTokenLib` in AdminV2 * refactor: event before external call, docs * relabel RELAYER_ROLE -> PROVER_ROLE * retain RELAYER_ROLE in addtn to PROVER_ROLE, for offchain perms * refactor: RELAYER_ROLE -> QUOTER_ROLE, docs * docs: AdminV2 other constants --------- Co-authored-by: parodime --- packages/contracts-rfq/contracts/AdminV2.sol | 104 +++++++++++++++ .../contracts-rfq/contracts/FastBridgeV2.sol | 80 ++++++------ .../contracts/interfaces/IAdminV2.sol | 14 +++ .../contracts/interfaces/IAdminV2Errors.sol | 7 ++ .../contracts/interfaces/IFastBridgeV2.sol | 6 + .../test/FastBridgeV2.GasBench.Src.t.sol | 20 +-- .../test/FastBridgeV2.Management.t.sol | 78 ++++++++---- .../test/FastBridgeV2.Src.Base.t.sol | 16 ++- .../test/FastBridgeV2.Src.RefundV1.t.sol | 12 ++ .../contracts-rfq/test/FastBridgeV2.Src.t.sol | 118 +++++++++--------- .../contracts-rfq/test/FastBridgeV2.t.sol | 2 +- .../FastBridgeV2.MulticallTarget.t.sol | 2 +- 12 files changed, 317 insertions(+), 142 deletions(-) create mode 100644 packages/contracts-rfq/contracts/AdminV2.sol create mode 100644 packages/contracts-rfq/contracts/interfaces/IAdminV2.sol create mode 100644 packages/contracts-rfq/contracts/interfaces/IAdminV2Errors.sol create mode 100644 packages/contracts-rfq/test/FastBridgeV2.Src.RefundV1.t.sol diff --git a/packages/contracts-rfq/contracts/AdminV2.sol b/packages/contracts-rfq/contracts/AdminV2.sol new file mode 100644 index 0000000000..5a8cf447ad --- /dev/null +++ b/packages/contracts-rfq/contracts/AdminV2.sol @@ -0,0 +1,104 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {IAdminV2} from "./interfaces/IAdminV2.sol"; +import {IAdminV2Errors} from "./interfaces/IAdminV2Errors.sol"; + +import {AccessControlEnumerable} from "@openzeppelin/contracts/access/extensions/AccessControlEnumerable.sol"; +import {SafeERC20, IERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; +import {Address} from "@openzeppelin/contracts/utils/Address.sol"; + +contract AdminV2 is AccessControlEnumerable, IAdminV2, IAdminV2Errors { + using SafeERC20 for IERC20; + + /// @notice Address reserved for native gas token (ETH on Ethereum and most L2s, AVAX on Avalanche, etc) + address public constant NATIVE_GAS_TOKEN = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE; + + /// @notice Role identifier for Quoter API's off-chain authentication. + /// @dev Only addresses with this role can post FastBridge quotes to the API. + bytes32 public constant QUOTER_ROLE = keccak256("QUOTER_ROLE"); + + /// @notice Role identifier for Prover's on-chain authentication in FastBridge. + /// @dev Only addresses with this role can provide proofs that a FastBridge request has been relayed. + bytes32 public constant PROVER_ROLE = keccak256("PROVER_ROLE"); + + /// @notice Role identifier for Guard's on-chain authentication in FastBridge. + /// @dev Only addresses with this role can dispute submitted relay proofs during the dispute period. + bytes32 public constant GUARD_ROLE = keccak256("GUARD_ROLE"); + + /// @notice Role identifier for Canceler's on-chain authentication in FastBridge. + /// @dev Only addresses with this role can cancel a FastBridge transaction without the cancel delay. + bytes32 public constant CANCELER_ROLE = keccak256("CANCELER_ROLE"); + + /// @notice Role identifier for Governor's on-chain administrative authority. + /// @dev Only addresses with this role can perform administrative tasks within the contract. + bytes32 public constant GOVERNOR_ROLE = keccak256("GOVERNOR_ROLE"); + + /// @notice Denominator for fee rates, represents 100%. + uint256 public constant FEE_BPS = 1e6; + /// @notice Maximum protocol fee rate: 1% on origin amount. + uint256 public constant FEE_RATE_MAX = 0.01e6; + + /// @notice Minimum cancel delay that can be set by the governor. + uint256 public constant MIN_CANCEL_DELAY = 1 hours; + /// @notice Default cancel delay set during the contract deployment. + uint256 public constant DEFAULT_CANCEL_DELAY = 1 days; + + /// @notice Protocol fee rate taken on origin amount deposited in origin chain + uint256 public protocolFeeRate; + + /// @notice Protocol fee amounts accumulated + mapping(address => uint256) public protocolFees; + + /// @notice Delay for a transaction after which it could be permisionlessly cancelled + uint256 public cancelDelay; + + /// @notice This is deprecated and should not be used. + /// @dev Use ZapNative V2 requests instead. + uint256 public immutable chainGasAmount = 0; + + constructor(address _owner) { + _grantRole(DEFAULT_ADMIN_ROLE, _owner); + _setCancelDelay(DEFAULT_CANCEL_DELAY); + } + + /// @notice Allows the contract governor to set the cancel delay. The cancel delay is the time after the transaction + /// deadline after which it can be permissionlessly cancelled, if it hasn't been proven by any of the Relayers. + function setCancelDelay(uint256 newCancelDelay) external onlyRole(GOVERNOR_ROLE) { + _setCancelDelay(newCancelDelay); + } + + /// @notice Allows the contract governor to set the protocol fee rate. The protocol fee is taken from the origin + /// amount only for completed and claimed transactions. + /// @dev The protocol fee is abstracted away from the relayers, they always operate using the amounts after fees: + /// what they see as the origin amount emitted in the log is what they get credited with. + function setProtocolFeeRate(uint256 newFeeRate) external onlyRole(GOVERNOR_ROLE) { + if (newFeeRate > FEE_RATE_MAX) revert FeeRateAboveMax(); + uint256 oldFeeRate = protocolFeeRate; + protocolFeeRate = newFeeRate; + emit FeeRateUpdated(oldFeeRate, newFeeRate); + } + + /// @notice Allows the contract governor to sweep the accumulated protocol fees in the contract. + function sweepProtocolFees(address token, address recipient) external onlyRole(GOVERNOR_ROLE) { + uint256 feeAmount = protocolFees[token]; + if (feeAmount == 0) return; // skip if no accumulated fees + + protocolFees[token] = 0; + emit FeesSwept(token, recipient, feeAmount); + /// Sweep the fees as the last transaction action + if (token == NATIVE_GAS_TOKEN) { + Address.sendValue(payable(recipient), feeAmount); + } else { + IERC20(token).safeTransfer(recipient, feeAmount); + } + } + + /// @notice Internal function to set the cancel delay. Security checks are performed outside of this function. + function _setCancelDelay(uint256 newCancelDelay) private { + if (newCancelDelay < MIN_CANCEL_DELAY) revert CancelDelayBelowMin(); + uint256 oldCancelDelay = cancelDelay; + cancelDelay = newCancelDelay; + emit CancelDelayUpdated(oldCancelDelay, newCancelDelay); + } +} diff --git a/packages/contracts-rfq/contracts/FastBridgeV2.sol b/packages/contracts-rfq/contracts/FastBridgeV2.sol index 26a2a1b3d7..a5aa64851a 100644 --- a/packages/contracts-rfq/contracts/FastBridgeV2.sol +++ b/packages/contracts-rfq/contracts/FastBridgeV2.sol @@ -1,12 +1,9 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.24; -import {SafeERC20, IERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; -import {Address} from "@openzeppelin/contracts/utils/Address.sol"; - import {BridgeTransactionV2Lib} from "./libs/BridgeTransactionV2.sol"; -import {Admin} from "./Admin.sol"; +import {AdminV2} from "./AdminV2.sol"; import {IFastBridge} from "./interfaces/IFastBridge.sol"; import {IFastBridgeV2} from "./interfaces/IFastBridgeV2.sol"; import {IFastBridgeV2Errors} from "./interfaces/IFastBridgeV2Errors.sol"; @@ -14,20 +11,17 @@ import {IZapRecipient} from "./interfaces/IZapRecipient.sol"; import {MulticallTarget} from "./utils/MulticallTarget.sol"; +import {SafeERC20, IERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; +import {Address} from "@openzeppelin/contracts/utils/Address.sol"; + /// @notice FastBridgeV2 is a contract for bridging tokens across chains. -contract FastBridgeV2 is Admin, MulticallTarget, IFastBridgeV2, IFastBridgeV2Errors { +contract FastBridgeV2 is AdminV2, MulticallTarget, IFastBridgeV2, IFastBridgeV2Errors { using BridgeTransactionV2Lib for bytes; using SafeERC20 for IERC20; - /// @notice Address reserved for native gas token (ETH on Ethereum and most L2s, AVAX on Avalanche, etc) - address public constant NATIVE_GAS_TOKEN = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE; - /// @notice Dispute period for relayed transactions uint256 public constant DISPUTE_PERIOD = 30 minutes; - /// @notice Delay for a transaction after which it could be permisionlessly refunded - uint256 public constant REFUND_DELAY = 7 days; - /// @notice Minimum deadline period to relay a requested bridge transaction uint256 public constant MIN_DEADLINE_PERIOD = 30 minutes; @@ -47,7 +41,7 @@ contract FastBridgeV2 is Admin, MulticallTarget, IFastBridgeV2, IFastBridgeV2Err /// @notice the block the contract was deployed at uint256 public immutable deployBlock; - constructor(address _owner) Admin(_owner) { + constructor(address _owner) AdminV2(_owner) { deployBlock = block.number; } @@ -104,33 +98,10 @@ contract FastBridgeV2 is Admin, MulticallTarget, IFastBridgeV2, IFastBridgeV2Err emit BridgeProofDisputed(transactionId, disputedRelayer); } + /// Note: this function is deprecated and will be removed in a future version. /// @inheritdoc IFastBridge function refund(bytes calldata request) external { - request.validateV2(); - bytes32 transactionId = keccak256(request); - BridgeTxDetails storage $ = bridgeTxDetails[transactionId]; - // Can only refund a REQUESTED transaction after its deadline expires - if ($.status != BridgeStatus.REQUESTED) revert StatusIncorrect(); - uint256 deadline = request.deadline(); - // Permissionless refund is only allowed after REFUND_DELAY on top of the deadline - if (!hasRole(REFUNDER_ROLE, msg.sender)) deadline += REFUND_DELAY; - if (block.timestamp <= deadline) revert DeadlineNotExceeded(); - // Update status to REFUNDED and return the full amount (collateral + protocol fees) to the original sender. - // The protocol fees are only updated when the transaction is claimed, so we don't need to update them here. - // Note: this is a storage write - $.status = BridgeStatus.REFUNDED; - - address to = request.originSender(); - address token = request.originToken(); - uint256 amount = request.originAmount() + request.originFeeAmount(); - // Emit the event before any external calls - emit BridgeDepositRefunded(transactionId, to, token, amount); - // Complete the user refund as the last transaction action - if (token == NATIVE_GAS_TOKEN) { - Address.sendValue(payable(to), amount); - } else { - IERC20(token).safeTransfer(to, amount); - } + cancel(request); } /// @inheritdoc IFastBridge @@ -306,7 +277,7 @@ contract FastBridgeV2 is Admin, MulticallTarget, IFastBridgeV2, IFastBridgeV2Err } /// @inheritdoc IFastBridgeV2 - function prove(bytes32 transactionId, bytes32 destTxHash, address relayer) public onlyRole(RELAYER_ROLE) { + function prove(bytes32 transactionId, bytes32 destTxHash, address relayer) public onlyRole(PROVER_ROLE) { BridgeTxDetails storage $ = bridgeTxDetails[transactionId]; // Can only prove a REQUESTED transaction @@ -363,6 +334,35 @@ contract FastBridgeV2 is Admin, MulticallTarget, IFastBridgeV2, IFastBridgeV2Err } } + /// @inheritdoc IFastBridgeV2 + function cancel(bytes calldata request) public { + request.validateV2(); + bytes32 transactionId = keccak256(request); + BridgeTxDetails storage $ = bridgeTxDetails[transactionId]; + // Can only cancel a REQUESTED transaction after its deadline expires + if ($.status != BridgeStatus.REQUESTED) revert StatusIncorrect(); + uint256 deadline = request.deadline(); + // Permissionless cancel is only allowed after `cancelDelay` on top of the deadline + if (!hasRole(CANCELER_ROLE, msg.sender)) deadline += cancelDelay; + if (block.timestamp <= deadline) revert DeadlineNotExceeded(); + // Update status to REFUNDED and return the full amount (collateral + protocol fees) to the original sender. + // The protocol fees are only updated when the transaction is claimed, so we don't need to update them here. + // Note: this is a storage write + $.status = BridgeStatus.REFUNDED; + + address to = request.originSender(); + address token = request.originToken(); + uint256 amount = request.originAmount() + request.originFeeAmount(); + // Emit the event before any external calls + emit BridgeDepositRefunded(transactionId, to, token, amount); + // Complete the user cancel as the last transaction action + if (token == NATIVE_GAS_TOKEN) { + Address.sendValue(payable(to), amount); + } else { + IERC20(token).safeTransfer(to, amount); + } + } + /// @inheritdoc IFastBridgeV2 function bridgeStatuses(bytes32 transactionId) public view returns (BridgeStatus status) { return bridgeTxDetails[transactionId].status; @@ -383,8 +383,8 @@ contract FastBridgeV2 is Admin, MulticallTarget, IFastBridgeV2, IFastBridgeV2Err } /// @notice Takes the bridged asset from the user into FastBridgeV2 custody. It will be later - /// claimed by the relayer who completed the relay on destination chain, or refunded back to the user, - /// should no one complete the relay. + /// claimed by the relayer who completed the relay on destination chain, or transferred back to the user + /// via the cancel function should no one complete the relay. function _takeBridgedUserAsset(address token, uint256 amount) internal returns (uint256 amountTaken) { if (token == NATIVE_GAS_TOKEN) { // For the native gas token, we just need to check that the supplied msg.value is correct. diff --git a/packages/contracts-rfq/contracts/interfaces/IAdminV2.sol b/packages/contracts-rfq/contracts/interfaces/IAdminV2.sol new file mode 100644 index 0000000000..1a30434161 --- /dev/null +++ b/packages/contracts-rfq/contracts/interfaces/IAdminV2.sol @@ -0,0 +1,14 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.4; + +interface IAdminV2 { + event CancelDelayUpdated(uint256 oldCancelDelay, uint256 newCancelDelay); + event FeeRateUpdated(uint256 oldFeeRate, uint256 newFeeRate); + event FeesSwept(address token, address recipient, uint256 amount); + + function setCancelDelay(uint256 newCancelDelay) external; + + function setProtocolFeeRate(uint256 newFeeRate) external; + + function sweepProtocolFees(address token, address recipient) external; +} diff --git a/packages/contracts-rfq/contracts/interfaces/IAdminV2Errors.sol b/packages/contracts-rfq/contracts/interfaces/IAdminV2Errors.sol new file mode 100644 index 0000000000..445087c87c --- /dev/null +++ b/packages/contracts-rfq/contracts/interfaces/IAdminV2Errors.sol @@ -0,0 +1,7 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.4; + +interface IAdminV2Errors { + error CancelDelayBelowMin(); + error FeeRateAboveMax(); +} diff --git a/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol b/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol index f6e7076d7e..258369b191 100644 --- a/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol +++ b/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol @@ -88,6 +88,12 @@ interface IFastBridgeV2 is IFastBridge { /// @notice Can only send funds to the relayer address on the proof. /// @param request The encoded bridge transaction to claim on origin chain function claim(bytes memory request) external; + + /// @notice Cancels an outstanding bridge transaction in case optimistic bridging failed and returns the full amount + /// to the original sender. + /// @param request The encoded bridge transaction to refund + function cancel(bytes memory request) external; + /// @notice Checks if a transaction has been relayed /// @param transactionId The ID of the transaction to check /// @return True if the transaction has been relayed, false otherwise diff --git a/packages/contracts-rfq/test/FastBridgeV2.GasBench.Src.t.sol b/packages/contracts-rfq/test/FastBridgeV2.GasBench.Src.t.sol index 0ffa7d8d5e..087be93105 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.GasBench.Src.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.GasBench.Src.t.sol @@ -152,19 +152,19 @@ contract FastBridgeV2GasBenchmarkSrcTest is FastBridgeV2SrcBaseTest { assertEq(srcToken.balanceOf(address(fastBridge)), initialFastBridgeBalanceToken); } - function test_refundPermissioned_token() public { + function test_cancelPermissioned_token() public { bytes32 txId = getTxId(bridgedTokenTx); skipTimeAtLeast({time: DEADLINE}); - refund({caller: refunder, bridgeTx: bridgedTokenTx}); + cancel({caller: canceler, bridgeTx: bridgedTokenTx}); assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REFUNDED); assertEq(srcToken.balanceOf(userA), initialUserBalanceToken + tokenParams.originAmount); assertEq(srcToken.balanceOf(address(fastBridge)), initialFastBridgeBalanceToken - tokenParams.originAmount); } - function test_refundPermissionless_token() public { + function test_cancelPermissionless_token() public { bytes32 txId = getTxId(bridgedTokenTx); - skipTimeAtLeast({time: DEADLINE + PERMISSIONLESS_REFUND_DELAY}); - refund({caller: userB, bridgeTx: bridgedTokenTx}); + skipTimeAtLeast({time: DEADLINE + PERMISSIONLESS_CANCEL_DELAY}); + cancel({caller: userB, bridgeTx: bridgedTokenTx}); assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REFUNDED); assertEq(srcToken.balanceOf(userA), initialUserBalanceToken + tokenParams.originAmount); assertEq(srcToken.balanceOf(address(fastBridge)), initialFastBridgeBalanceToken - tokenParams.originAmount); @@ -236,19 +236,19 @@ contract FastBridgeV2GasBenchmarkSrcTest is FastBridgeV2SrcBaseTest { assertEq(address(fastBridge).balance, initialFastBridgeBalanceEth); } - function test_refundPermissioned_eth() public { + function test_cancelPermissioned_eth() public { bytes32 txId = getTxId(bridgedEthTx); skipTimeAtLeast({time: DEADLINE}); - refund({caller: refunder, bridgeTx: bridgedEthTx}); + cancel({caller: canceler, bridgeTx: bridgedEthTx}); assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REFUNDED); assertEq(userA.balance, initialUserBalanceEth + ethParams.originAmount); assertEq(address(fastBridge).balance, initialFastBridgeBalanceEth - ethParams.originAmount); } - function test_refundPermissionless_eth() public { + function test_cancelPermissionless_eth() public { bytes32 txId = getTxId(bridgedEthTx); - skipTimeAtLeast({time: DEADLINE + PERMISSIONLESS_REFUND_DELAY}); - refund({caller: userB, bridgeTx: bridgedEthTx}); + skipTimeAtLeast({time: DEADLINE + PERMISSIONLESS_CANCEL_DELAY}); + cancel({caller: userB, bridgeTx: bridgedEthTx}); assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REFUNDED); assertEq(userA.balance, initialUserBalanceEth + ethParams.originAmount); assertEq(address(fastBridge).balance, initialFastBridgeBalanceEth - ethParams.originAmount); diff --git a/packages/contracts-rfq/test/FastBridgeV2.Management.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Management.t.sol index 58502618b9..ab9ea59341 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.Management.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.Management.t.sol @@ -1,19 +1,25 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; +import {IAdmin} from "../contracts/interfaces/IAdmin.sol"; +import {IAdminV2Errors} from "../contracts/interfaces/IAdminV2Errors.sol"; + import {FastBridgeV2, FastBridgeV2Test} from "./FastBridgeV2.t.sol"; // solhint-disable func-name-mixedcase, ordering -contract FastBridgeV2ManagementTest is FastBridgeV2Test { +contract FastBridgeV2ManagementTest is FastBridgeV2Test, IAdminV2Errors { uint256 public constant FEE_RATE_MAX = 1e4; // 1% bytes32 public constant GOVERNOR_ROLE = keccak256("GOVERNOR_ROLE"); + uint256 public constant MIN_CANCEL_DELAY = 1 hours; + uint256 public constant DEFAULT_CANCEL_DELAY = 1 days; + address public admin = makeAddr("Admin"); address public governorA = makeAddr("Governor A"); + event CancelDelayUpdated(uint256 oldCancelDelay, uint256 newCancelDelay); event FeeRateUpdated(uint256 oldFeeRate, uint256 newFeeRate); event FeesSwept(address token, address recipient, uint256 amount); - event ChainGasAmountUpdated(uint256 oldChainGasAmount, uint256 newChainGasAmount); function deployFastBridge() public override returns (FastBridgeV2) { return new FastBridgeV2(admin); @@ -35,6 +41,11 @@ contract FastBridgeV2ManagementTest is FastBridgeV2Test { fastBridge.grantRole(GOVERNOR_ROLE, newGovernor); } + function setCancelDelay(address caller, uint256 newCancelDelay) public { + vm.prank(caller); + fastBridge.setCancelDelay(newCancelDelay); + } + function setProtocolFeeRate(address caller, uint256 newFeeRate) public { vm.prank(caller); fastBridge.setProtocolFeeRate(newFeeRate); @@ -45,11 +56,6 @@ contract FastBridgeV2ManagementTest is FastBridgeV2Test { fastBridge.sweepProtocolFees(token, recipient); } - function setChainGasAmount(address caller, uint256 newChainGasAmount) public { - vm.prank(caller); - fastBridge.setChainGasAmount(newChainGasAmount); - } - function test_grantGovernorRole() public { assertFalse(fastBridge.hasRole(GOVERNOR_ROLE, governorA)); setGovernor(admin, governorA); @@ -62,6 +68,38 @@ contract FastBridgeV2ManagementTest is FastBridgeV2Test { setGovernor(caller, governorA); } + function test_defaultCancelDelay() public view { + assertEq(fastBridge.cancelDelay(), DEFAULT_CANCEL_DELAY); + } + + // ═════════════════════════════════════════════ SET CANCEL DELAY ══════════════════════════════════════════════════ + + function test_setCancelDelay() public { + vm.expectEmit(address(fastBridge)); + emit CancelDelayUpdated(DEFAULT_CANCEL_DELAY, 4 days); + setCancelDelay(governor, 4 days); + assertEq(fastBridge.cancelDelay(), 4 days); + } + + function test_setCancelDelay_twice() public { + test_setCancelDelay(); + vm.expectEmit(address(fastBridge)); + emit CancelDelayUpdated(4 days, 8 days); + setCancelDelay(governor, 8 days); + assertEq(fastBridge.cancelDelay(), 8 days); + } + + function test_setCancelDelay_revertBelowMin() public { + vm.expectRevert(IAdminV2Errors.CancelDelayBelowMin.selector); + setCancelDelay(governor, MIN_CANCEL_DELAY - 1); + } + + function test_setCancelDelay_revertNotGovernor(address caller) public { + vm.assume(caller != governor); + expectUnauthorized(caller, fastBridge.GOVERNOR_ROLE()); + setCancelDelay(caller, 4 days); + } + // ═══════════════════════════════════════════ SET PROTOCOL FEE RATE ═══════════════════════════════════════════════ function test_setProtocolFeeRate() public { @@ -80,7 +118,7 @@ contract FastBridgeV2ManagementTest is FastBridgeV2Test { } function test_setProtocolFeeRate_revert_tooHigh() public { - vm.expectRevert("newFeeRate > max"); + vm.expectRevert(IAdminV2Errors.FeeRateAboveMax.selector); setProtocolFeeRate(governor, FEE_RATE_MAX + 1); } @@ -118,24 +156,14 @@ contract FastBridgeV2ManagementTest is FastBridgeV2Test { // ═══════════════════════════════════════════ SET CHAIN GAS AMOUNT ════════════════════════════════════════════════ - function test_setChainGasAmount() public { - vm.expectEmit(address(fastBridge)); - emit ChainGasAmountUpdated(0, 123); - setChainGasAmount(governor, 123); - assertEq(fastBridge.chainGasAmount(), 123); + function test_chainGasAmountZero() public view { + assertEq(fastBridge.chainGasAmount(), 0); } - function test_setChainGasAmount_twice() public { - test_setChainGasAmount(); - vm.expectEmit(address(fastBridge)); - emit ChainGasAmountUpdated(123, 456); - setChainGasAmount(governor, 456); - assertEq(fastBridge.chainGasAmount(), 456); - } - - function test_setChainGasAmount_revertNotGovernor(address caller) public { - vm.assume(caller != governor); - expectUnauthorized(caller, fastBridge.GOVERNOR_ROLE()); - setChainGasAmount(caller, 123); + function test_setChainGasAmount_revert() public { + // Generic revert: this function should not be in the V2 interface + vm.expectRevert(); + vm.prank(governor); + IAdmin(address(fastBridge)).setChainGasAmount(123); } } diff --git a/packages/contracts-rfq/test/FastBridgeV2.Src.Base.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Src.Base.t.sol index f7a4d66633..2d655aa49d 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.Src.Base.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.Src.Base.t.sol @@ -9,7 +9,8 @@ import {FastBridgeV2, FastBridgeV2Test, IFastBridge, IFastBridgeV2} from "./Fast abstract contract FastBridgeV2SrcBaseTest is FastBridgeV2Test { uint256 public constant MIN_DEADLINE = 30 minutes; uint256 public constant CLAIM_DELAY = 30 minutes; - uint256 public constant PERMISSIONLESS_REFUND_DELAY = 7 days; + // Use a value different from the default to ensure it's being set correctly. + uint256 public constant PERMISSIONLESS_CANCEL_DELAY = 13.37 hours; uint256 public constant LEFTOVER_BALANCE = 10 ether; uint256 public constant INITIAL_PROTOCOL_FEES_TOKEN = 456_789; @@ -25,10 +26,13 @@ abstract contract FastBridgeV2SrcBaseTest is FastBridgeV2Test { } function configureFastBridge() public virtual override { - fastBridge.grantRole(fastBridge.RELAYER_ROLE(), relayerA); - fastBridge.grantRole(fastBridge.RELAYER_ROLE(), relayerB); + fastBridge.grantRole(fastBridge.PROVER_ROLE(), relayerA); + fastBridge.grantRole(fastBridge.PROVER_ROLE(), relayerB); fastBridge.grantRole(fastBridge.GUARD_ROLE(), guard); - fastBridge.grantRole(fastBridge.REFUNDER_ROLE(), refunder); + fastBridge.grantRole(fastBridge.CANCELER_ROLE(), canceler); + + fastBridge.grantRole(fastBridge.GOVERNOR_ROLE(), address(this)); + fastBridge.setCancelDelay(PERMISSIONLESS_CANCEL_DELAY); } function mintTokens() public virtual override { @@ -92,9 +96,9 @@ abstract contract FastBridgeV2SrcBaseTest is FastBridgeV2Test { fastBridge.dispute(txId); } - function refund(address caller, IFastBridgeV2.BridgeTransactionV2 memory bridgeTx) public { + function cancel(address caller, IFastBridgeV2.BridgeTransactionV2 memory bridgeTx) public virtual { vm.prank({msgSender: caller, txOrigin: caller}); - fastBridge.refund(BridgeTransactionV2Lib.encodeV2(bridgeTx)); + fastBridge.cancel(BridgeTransactionV2Lib.encodeV2(bridgeTx)); } function test_nonce() public view { diff --git a/packages/contracts-rfq/test/FastBridgeV2.Src.RefundV1.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Src.RefundV1.t.sol new file mode 100644 index 0000000000..3b26f83c55 --- /dev/null +++ b/packages/contracts-rfq/test/FastBridgeV2.Src.RefundV1.t.sol @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {FastBridgeV2SrcTest, BridgeTransactionV2Lib, IFastBridgeV2} from "./FastBridgeV2.Src.t.sol"; + +// solhint-disable func-name-mixedcase, ordering +contract FastBridgeV2SrcRefundV1Test is FastBridgeV2SrcTest { + function cancel(address caller, IFastBridgeV2.BridgeTransactionV2 memory bridgeTx) public virtual override { + vm.prank({msgSender: caller, txOrigin: caller}); + fastBridge.refund(BridgeTransactionV2Lib.encodeV2(bridgeTx)); + } +} diff --git a/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol index 15f0ac44c5..539fe94c4d 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol @@ -341,7 +341,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2SrcBaseTest { function test_prove_revert_statusRefunded() public { bridge({caller: userA, msgValue: 0, params: tokenParams}); skip(DEADLINE + 1); - refund({caller: refunder, bridgeTx: tokenTx}); + cancel({caller: canceler, bridgeTx: tokenTx}); vm.expectRevert(StatusIncorrect.selector); prove({caller: relayerA, bridgeTx: tokenTx, destTxHash: hex"01"}); } @@ -349,7 +349,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2SrcBaseTest { function test_prove_revert_callerNotRelayer(address caller) public { vm.assume(caller != relayerA && caller != relayerB); bridge({caller: userA, msgValue: 0, params: tokenParams}); - expectUnauthorized(caller, fastBridge.RELAYER_ROLE()); + expectUnauthorized(caller, fastBridge.PROVER_ROLE()); prove({caller: caller, bridgeTx: tokenTx, destTxHash: hex"01"}); } @@ -455,7 +455,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2SrcBaseTest { bytes32 txId = getTxId(tokenTx); bridge({caller: userA, msgValue: 0, params: tokenParams}); skip(DEADLINE + 1); - refund({caller: refunder, bridgeTx: tokenTx}); + cancel({caller: canceler, bridgeTx: tokenTx}); vm.expectRevert(StatusIncorrect.selector); prove({caller: relayerB, transactionId: txId, destTxHash: hex"01", relayer: relayerA}); } @@ -464,7 +464,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2SrcBaseTest { bytes32 txId = getTxId(tokenTx); vm.assume(caller != relayerA && caller != relayerB); bridge({caller: userA, msgValue: 0, params: tokenParams}); - expectUnauthorized(caller, fastBridge.RELAYER_ROLE()); + expectUnauthorized(caller, fastBridge.PROVER_ROLE()); prove({caller: caller, transactionId: txId, destTxHash: hex"01", relayer: relayerA}); } @@ -690,7 +690,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2SrcBaseTest { bytes32 txId = getTxId(tokenTx); bridge({caller: userA, msgValue: 0, params: tokenParams}); skip(DEADLINE + 1); - refund({caller: refunder, bridgeTx: tokenTx}); + cancel({caller: canceler, bridgeTx: tokenTx}); vm.expectRevert(StatusIncorrect.selector); fastBridge.canClaim(txId, relayerA); vm.expectRevert(StatusIncorrect.selector); @@ -797,14 +797,14 @@ contract FastBridgeV2SrcTest is FastBridgeV2SrcBaseTest { bytes32 txId = getTxId(tokenTx); bridge({caller: userA, msgValue: 0, params: tokenParams}); skip(DEADLINE + 1); - refund({caller: refunder, bridgeTx: tokenTx}); + cancel({caller: canceler, bridgeTx: tokenTx}); vm.expectRevert(StatusIncorrect.selector); dispute({caller: guard, txId: txId}); } - // ══════════════════════════════════════════════════ REFUND ═══════════════════════════════════════════════════════ + // ══════════════════════════════════════════════════ CANCEL ═══════════════════════════════════════════════════════ - function checkStatusAndProofAfterRefund(bytes32 txId) public view { + function checkStatusAndProofAfterCancel(bytes32 txId) public view { assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REFUNDED); (IFastBridgeV2.BridgeStatus status, uint32 destChainId, uint256 proofBlockTimestamp, address proofRelayer) = fastBridge.bridgeTxDetails(txId); @@ -817,160 +817,160 @@ contract FastBridgeV2SrcTest is FastBridgeV2SrcBaseTest { assertEq(proofRelayer, address(0)); } - function test_refund_token() public { + function test_cancel_token() public { bytes32 txId = getTxId(tokenTx); bridge({caller: userA, msgValue: 0, params: tokenParams}); skip(DEADLINE + 1); expectBridgeDepositRefunded({bridgeParams: tokenParams, txId: txId}); - refund({caller: refunder, bridgeTx: tokenTx}); - checkStatusAndProofAfterRefund(txId); + cancel({caller: canceler, bridgeTx: tokenTx}); + checkStatusAndProofAfterCancel(txId); assertEq(fastBridge.protocolFees(address(srcToken)), INITIAL_PROTOCOL_FEES_TOKEN); assertEq(srcToken.balanceOf(userA), LEFTOVER_BALANCE + tokenParams.originAmount); assertEq(srcToken.balanceOf(address(fastBridge)), INITIAL_PROTOCOL_FEES_TOKEN); } /// @notice Deposit should be refunded to the BridgeParams.sender, regardless of the actual caller - function test_refund_token_diffSender() public { + function test_cancel_token_diffSender() public { bytes32 txId = getTxId(tokenTx); bridge({caller: userB, msgValue: 0, params: tokenParams}); skip(DEADLINE + 1); expectBridgeDepositRefunded({bridgeParams: tokenParams, txId: txId}); - refund({caller: refunder, bridgeTx: tokenTx}); - checkStatusAndProofAfterRefund(txId); + cancel({caller: canceler, bridgeTx: tokenTx}); + checkStatusAndProofAfterCancel(txId); assertEq(fastBridge.protocolFees(address(srcToken)), INITIAL_PROTOCOL_FEES_TOKEN); assertEq(srcToken.balanceOf(userA), LEFTOVER_BALANCE + 2 * tokenParams.originAmount); assertEq(srcToken.balanceOf(userB), LEFTOVER_BALANCE); assertEq(srcToken.balanceOf(address(fastBridge)), INITIAL_PROTOCOL_FEES_TOKEN); } - function test_refund_token_longDelay() public { + function test_cancel_token_longDelay() public { bytes32 txId = getTxId(tokenTx); bridge({caller: userA, msgValue: 0, params: tokenParams}); skip(DEADLINE + 30 days); expectBridgeDepositRefunded({bridgeParams: tokenParams, txId: txId}); - refund({caller: refunder, bridgeTx: tokenTx}); - checkStatusAndProofAfterRefund(txId); + cancel({caller: canceler, bridgeTx: tokenTx}); + checkStatusAndProofAfterCancel(txId); assertEq(fastBridge.protocolFees(address(srcToken)), INITIAL_PROTOCOL_FEES_TOKEN); assertEq(srcToken.balanceOf(userA), LEFTOVER_BALANCE + tokenParams.originAmount); assertEq(srcToken.balanceOf(address(fastBridge)), INITIAL_PROTOCOL_FEES_TOKEN); } - function test_refund_token_permisionless(address caller) public { - vm.assume(caller != refunder); + function test_cancel_token_permisionless(address caller) public { + vm.assume(caller != canceler); bytes32 txId = getTxId(tokenTx); bridge({caller: userA, msgValue: 0, params: tokenParams}); - skip(DEADLINE + PERMISSIONLESS_REFUND_DELAY + 1); + skip(DEADLINE + PERMISSIONLESS_CANCEL_DELAY + 1); expectBridgeDepositRefunded({bridgeParams: tokenParams, txId: txId}); - refund({caller: caller, bridgeTx: tokenTx}); - checkStatusAndProofAfterRefund(txId); + cancel({caller: caller, bridgeTx: tokenTx}); + checkStatusAndProofAfterCancel(txId); assertEq(fastBridge.protocolFees(address(srcToken)), INITIAL_PROTOCOL_FEES_TOKEN); assertEq(srcToken.balanceOf(userA), LEFTOVER_BALANCE + tokenParams.originAmount); assertEq(srcToken.balanceOf(address(fastBridge)), INITIAL_PROTOCOL_FEES_TOKEN); } - function test_refund_eth() public { + function test_cancel_eth() public { bridge({caller: userA, msgValue: 0, params: tokenParams}); bytes32 txId = getTxId(ethTx); bridge({caller: userA, msgValue: ethParams.originAmount, params: ethParams}); skip(DEADLINE + 1); expectBridgeDepositRefunded({bridgeParams: ethParams, txId: txId}); - refund({caller: refunder, bridgeTx: ethTx}); - checkStatusAndProofAfterRefund(txId); + cancel({caller: canceler, bridgeTx: ethTx}); + checkStatusAndProofAfterCancel(txId); assertEq(fastBridge.protocolFees(ETH_ADDRESS), INITIAL_PROTOCOL_FEES_ETH); assertEq(userA.balance, LEFTOVER_BALANCE + ethParams.originAmount); assertEq(address(fastBridge).balance, INITIAL_PROTOCOL_FEES_ETH); } /// @notice Deposit should be refunded to the BridgeParams.sender, regardless of the actual caller - function test_refund_eth_diffSender() public { + function test_cancel_eth_diffSender() public { bridge({caller: userA, msgValue: 0, params: tokenParams}); bytes32 txId = getTxId(ethTx); bridge({caller: userB, msgValue: ethParams.originAmount, params: ethParams}); skip(DEADLINE + 1); expectBridgeDepositRefunded({bridgeParams: ethParams, txId: txId}); - refund({caller: refunder, bridgeTx: ethTx}); - checkStatusAndProofAfterRefund(txId); + cancel({caller: canceler, bridgeTx: ethTx}); + checkStatusAndProofAfterCancel(txId); assertEq(fastBridge.protocolFees(ETH_ADDRESS), INITIAL_PROTOCOL_FEES_ETH); assertEq(userA.balance, LEFTOVER_BALANCE + 2 * ethParams.originAmount); assertEq(userB.balance, LEFTOVER_BALANCE); assertEq(address(fastBridge).balance, INITIAL_PROTOCOL_FEES_ETH); } - function test_refund_eth_longDelay() public { + function test_cancel_eth_longDelay() public { bridge({caller: userA, msgValue: 0, params: tokenParams}); bytes32 txId = getTxId(ethTx); bridge({caller: userA, msgValue: ethParams.originAmount, params: ethParams}); skip(DEADLINE + 30 days); expectBridgeDepositRefunded({bridgeParams: ethParams, txId: txId}); - refund({caller: refunder, bridgeTx: ethTx}); - checkStatusAndProofAfterRefund(txId); + cancel({caller: canceler, bridgeTx: ethTx}); + checkStatusAndProofAfterCancel(txId); assertEq(fastBridge.protocolFees(ETH_ADDRESS), INITIAL_PROTOCOL_FEES_ETH); assertEq(userA.balance, LEFTOVER_BALANCE + ethParams.originAmount); assertEq(address(fastBridge).balance, INITIAL_PROTOCOL_FEES_ETH); } - function test_refund_eth_permisionless(address caller) public { - vm.assume(caller != refunder); + function test_cancel_eth_permisionless(address caller) public { + vm.assume(caller != canceler); bytes32 txId = getTxId(ethTx); bridge({caller: userA, msgValue: 0, params: tokenParams}); bridge({caller: userA, msgValue: ethParams.originAmount, params: ethParams}); - skip(DEADLINE + PERMISSIONLESS_REFUND_DELAY + 1); + skip(DEADLINE + PERMISSIONLESS_CANCEL_DELAY + 1); expectBridgeDepositRefunded({bridgeParams: ethParams, txId: txId}); - refund({caller: caller, bridgeTx: ethTx}); - checkStatusAndProofAfterRefund(txId); + cancel({caller: caller, bridgeTx: ethTx}); + checkStatusAndProofAfterCancel(txId); assertEq(fastBridge.protocolFees(ETH_ADDRESS), INITIAL_PROTOCOL_FEES_ETH); assertEq(userA.balance, LEFTOVER_BALANCE + ethParams.originAmount); assertEq(address(fastBridge).balance, INITIAL_PROTOCOL_FEES_ETH); } - function test_refund_revert_zeroDelay() public { + function test_cancel_revert_zeroDelay() public { bridge({caller: userA, msgValue: 0, params: tokenParams}); vm.expectRevert(DeadlineNotExceeded.selector); - refund({caller: refunder, bridgeTx: tokenTx}); + cancel({caller: canceler, bridgeTx: tokenTx}); } - function test_refund_revert_justBeforeDeadline() public { + function test_cancel_revert_justBeforeDeadline() public { bridge({caller: userA, msgValue: 0, params: tokenParams}); skip(DEADLINE); vm.expectRevert(DeadlineNotExceeded.selector); - refund({caller: refunder, bridgeTx: tokenTx}); + cancel({caller: canceler, bridgeTx: tokenTx}); } - function test_refund_revert_justBeforeDeadline_permisionless(address caller) public { - vm.assume(caller != refunder); + function test_cancel_revert_justBeforeDeadline_permisionless(address caller) public { + vm.assume(caller != canceler); bridge({caller: userA, msgValue: 0, params: tokenParams}); - skip(DEADLINE + PERMISSIONLESS_REFUND_DELAY); + skip(DEADLINE + PERMISSIONLESS_CANCEL_DELAY); vm.expectRevert(DeadlineNotExceeded.selector); - refund({caller: caller, bridgeTx: tokenTx}); + cancel({caller: caller, bridgeTx: tokenTx}); } - function test_refund_revert_statusNull() public { + function test_cancel_revert_statusNull() public { vm.expectRevert(StatusIncorrect.selector); - refund({caller: refunder, bridgeTx: ethTx}); + cancel({caller: canceler, bridgeTx: ethTx}); } - function test_refund_revert_statusProven() public { + function test_cancel_revert_statusProven() public { bridge({caller: userA, msgValue: 0, params: tokenParams}); prove({caller: relayerA, bridgeTx: tokenTx, destTxHash: hex"01"}); vm.expectRevert(StatusIncorrect.selector); - refund({caller: refunder, bridgeTx: tokenTx}); + cancel({caller: canceler, bridgeTx: tokenTx}); } - function test_refund_revert_statusClaimed() public { + function test_cancel_revert_statusClaimed() public { bridge({caller: userA, msgValue: 0, params: tokenParams}); prove({caller: relayerA, bridgeTx: tokenTx, destTxHash: hex"01"}); skip(CLAIM_DELAY + 1); claim({caller: relayerA, bridgeTx: tokenTx, to: relayerA}); vm.expectRevert(StatusIncorrect.selector); - refund({caller: refunder, bridgeTx: tokenTx}); + cancel({caller: canceler, bridgeTx: tokenTx}); } - function test_refund_revert_statusRefunded() public { + function test_cancel_revert_statusRefunded() public { bridge({caller: userA, msgValue: 0, params: tokenParams}); skip(DEADLINE + 1); - refund({caller: refunder, bridgeTx: tokenTx}); + cancel({caller: canceler, bridgeTx: tokenTx}); vm.expectRevert(StatusIncorrect.selector); - refund({caller: refunder, bridgeTx: tokenTx}); + cancel({caller: canceler, bridgeTx: tokenTx}); } // ═════════════════════════════════════════════ INVALID PAYLOADS ══════════════════════════════════════════════════ @@ -1032,22 +1032,22 @@ contract FastBridgeV2SrcTest is FastBridgeV2SrcBaseTest { fastBridge.claim(mockRequestV3, relayerB); } - function test_refund_revert_requestV1() public { + function test_cancel_revert_requestV1() public { // V1 doesn't have any version field expectRevertUnsupportedVersion(0); vm.prank({msgSender: relayerA, txOrigin: relayerA}); - fastBridge.refund(mockRequestV1); + fastBridge.cancel(mockRequestV1); } - function test_refund_revert_invalidRequestV2() public { + function test_cancel_revert_invalidRequestV2() public { expectRevertInvalidEncodedTx(); vm.prank({msgSender: relayerA, txOrigin: relayerA}); - fastBridge.refund(invalidRequestV2); + fastBridge.cancel(invalidRequestV2); } - function test_refund_revert_requestV3() public { + function test_cancel_revert_requestV3() public { expectRevertUnsupportedVersion(3); vm.prank({msgSender: relayerA, txOrigin: relayerA}); - fastBridge.refund(mockRequestV3); + fastBridge.cancel(mockRequestV3); } } diff --git a/packages/contracts-rfq/test/FastBridgeV2.t.sol b/packages/contracts-rfq/test/FastBridgeV2.t.sol index d3f51e4ca9..706f05d681 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.t.sol @@ -36,7 +36,7 @@ abstract contract FastBridgeV2Test is Test, IFastBridgeV2Errors { address public userA = makeAddr("User A"); address public userB = makeAddr("User B"); address public governor = makeAddr("Governor"); - address public refunder = makeAddr("Refunder"); + address public canceler = makeAddr("Canceler"); IFastBridgeV2.BridgeTransactionV2 internal tokenTx; IFastBridgeV2.BridgeTransactionV2 internal ethTx; diff --git a/packages/contracts-rfq/test/integration/FastBridgeV2.MulticallTarget.t.sol b/packages/contracts-rfq/test/integration/FastBridgeV2.MulticallTarget.t.sol index 0cdb61e358..f58dceb310 100644 --- a/packages/contracts-rfq/test/integration/FastBridgeV2.MulticallTarget.t.sol +++ b/packages/contracts-rfq/test/integration/FastBridgeV2.MulticallTarget.t.sol @@ -9,7 +9,7 @@ import {MulticallTargetIntegrationTest, IFastBridge} from "./MulticallTarget.t.s contract FastBridgeV2MulticallTargetTest is MulticallTargetIntegrationTest { function deployAndConfigureFastBridge() public override returns (address) { FastBridgeV2 fastBridge = new FastBridgeV2(address(this)); - fastBridge.grantRole(fastBridge.RELAYER_ROLE(), relayer); + fastBridge.grantRole(fastBridge.PROVER_ROLE(), relayer); return address(fastBridge); }