rearrange refund validation checks [SLT-185] (#3172)

* rearrange refund validation checks [SLT-185] * test: fix own incorrectly defined tests --------- Co-authored-by: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com>
synapsecns · Sep 26, 2024 · 7bcb5d9 · 7bcb5d9
1 parent ebf102c
commit 7bcb5d9
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 8 deletions.
diff --git a/packages/contracts-rfq/contracts/FastBridgeV2.sol b/packages/contracts-rfq/contracts/FastBridgeV2.sol
@@ -274,6 +274,9 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {
     /// @inheritdoc IFastBridge
     function refund(bytes memory request) external {
         bytes32 transactionId = keccak256(request);
+
+        if (bridgeStatuses[transactionId] != BridgeStatus.REQUESTED) revert StatusIncorrect();
+
         BridgeTransaction memory transaction = getBridgeTransaction(request);
 
         if (hasRole(REFUNDER_ROLE, msg.sender)) {
@@ -284,8 +287,7 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {
             if (block.timestamp <= transaction.deadline + REFUND_DELAY) revert DeadlineNotExceeded();
         }
 
-        // set status to refunded if still in requested state
-        if (bridgeStatuses[transactionId] != BridgeStatus.REQUESTED) revert StatusIncorrect();
+        // if all checks passed, set to REFUNDED status
         bridgeStatuses[transactionId] = BridgeStatus.REFUNDED;
 
         // transfer origin collateral back to original sender

diff --git a/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol
@@ -896,40 +896,37 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test {
     function test_refund_revert_zeroDelay() public {
         bridge({caller: userA, msgValue: 0, params: tokenParams});
         vm.expectRevert(DeadlineNotExceeded.selector);
-        refund({caller: refunder, bridgeTx: ethTx});
+        refund({caller: refunder, bridgeTx: tokenTx});
     }
 
     function test_refund_revert_justBeforeDeadline() public {
         bridge({caller: userA, msgValue: 0, params: tokenParams});
         skip(DEADLINE);
         vm.expectRevert(DeadlineNotExceeded.selector);
-        refund({caller: refunder, bridgeTx: ethTx});
+        refund({caller: refunder, bridgeTx: tokenTx});
     }
 
     function test_refund_revert_justBeforeDeadline_permisionless(address caller) public {
         vm.assume(caller != refunder);
         bridge({caller: userA, msgValue: 0, params: tokenParams});
         skip(DEADLINE + PERMISSIONLESS_REFUND_DELAY);
         vm.expectRevert(DeadlineNotExceeded.selector);
-        refund({caller: caller, bridgeTx: ethTx});
+        refund({caller: caller, bridgeTx: tokenTx});
     }
 
     function test_refund_revert_statusNull() public {
-        vm.skip(true); // TODO: unskip when fixed
         vm.expectRevert(StatusIncorrect.selector);
         refund({caller: refunder, bridgeTx: ethTx});
     }
 
     function test_refund_revert_statusProven() public {
-        vm.skip(true); // TODO: unskip when fixed
         bridge({caller: userA, msgValue: 0, params: tokenParams});
         prove({caller: relayerA, bridgeTx: tokenTx, destTxHash: hex"01"});
         vm.expectRevert(StatusIncorrect.selector);
         refund({caller: refunder, bridgeTx: tokenTx});
     }
 
     function test_refund_revert_statusClaimed() public {
-        vm.skip(true); // TODO: unskip when fixed
         bridge({caller: userA, msgValue: 0, params: tokenParams});
         prove({caller: relayerA, bridgeTx: tokenTx, destTxHash: hex"01"});
         skip(CLAIM_DELAY + 1);