storage refs - readability & gas impv [SLT-353] (#3303)

* storage refs - readability & gas impv * rearrange reads on dispute * read storage to local stack - claim * docs: make storage writes more explicit --------- Co-authored-by: ChiTimesChi <[email protected]>
synapsecns · Oct 18, 2024 · a0684a8 · a0684a8
1 parent 6415acd
commit a0684a8
Showing 1 changed file with 48 additions and 35 deletions.
diff --git a/packages/contracts-rfq/contracts/FastBridgeV2.sol b/packages/contracts-rfq/contracts/FastBridgeV2.sol
@@ -77,18 +77,22 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {
 
     /// @inheritdoc IFastBridge
     function dispute(bytes32 transactionId) external onlyRole(GUARD_ROLE) {
-        if (bridgeTxDetails[transactionId].status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect();
-        if (_timeSince(bridgeTxDetails[transactionId].proofBlockTimestamp) > DISPUTE_PERIOD) {
+        BridgeTxDetails storage $ = bridgeTxDetails[transactionId];
+
+        address disputedRelayer = $.proofRelayer;
+        BridgeStatus status = $.status;
+        uint40 proofBlockTimestamp = $.proofBlockTimestamp;
+
+        if (status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect();
+        if (_timeSince(proofBlockTimestamp) > DISPUTE_PERIOD) {
             revert DisputePeriodPassed();
         }
 
-        address disputedRelayer = bridgeTxDetails[transactionId].proofRelayer;
-
-        // @dev relayer gets slashed effectively if dest relay has gone thru
-        bridgeTxDetails[transactionId].status = BridgeStatus.REQUESTED;
-        bridgeTxDetails[transactionId].proofRelayer = address(0);
-        bridgeTxDetails[transactionId].proofBlockTimestamp = 0;
-        bridgeTxDetails[transactionId].proofBlockNumber = 0;
+        // Note: these are storage writes
+        $.status = BridgeStatus.REQUESTED;
+        $.proofRelayer = address(0);
+        $.proofBlockTimestamp = 0;
+        $.proofBlockNumber = 0;
 
         emit BridgeProofDisputed(transactionId, disputedRelayer);
     }
@@ -99,7 +103,9 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {
 
         BridgeTransactionV2 memory transaction = getBridgeTransactionV2(request);
 
-        if (bridgeTxDetails[transactionId].status != BridgeStatus.REQUESTED) revert StatusIncorrect();
+        BridgeTxDetails storage $ = bridgeTxDetails[transactionId];
+
+        if ($.status != BridgeStatus.REQUESTED) revert StatusIncorrect();
 
         if (hasRole(REFUNDER_ROLE, msg.sender)) {
             // Refunder can refund if deadline has passed
@@ -109,8 +115,8 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {
             if (block.timestamp <= transaction.deadline + REFUND_DELAY) revert DeadlineNotExceeded();
         }
 
-        // if all checks passed, set to REFUNDED status
-        bridgeTxDetails[transactionId].status = BridgeStatus.REFUNDED;
+        // Note: this is a storage write
+        $.status = BridgeStatus.REFUNDED;
 
         // transfer origin collateral back to original sender
         uint256 amount = transaction.originAmount + transaction.originFeeAmount;
@@ -128,9 +134,11 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {
 
     /// @inheritdoc IFastBridge
     function canClaim(bytes32 transactionId, address relayer) external view returns (bool) {
-        if (bridgeTxDetails[transactionId].status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect();
-        if (bridgeTxDetails[transactionId].proofRelayer != relayer) revert SenderIncorrect();
-        return _timeSince(bridgeTxDetails[transactionId].proofBlockTimestamp) > DISPUTE_PERIOD;
+        BridgeTxDetails storage $ = bridgeTxDetails[transactionId];
+
+        if ($.status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect();
+        if ($.proofRelayer != relayer) revert SenderIncorrect();
+        return _timeSince($.proofBlockTimestamp) > DISPUTE_PERIOD;
     }
 
     /// @inheritdoc IFastBridge
@@ -273,12 +281,15 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {
 
     /// @inheritdoc IFastBridgeV2
     function prove(bytes32 transactionId, bytes32 destTxHash, address relayer) public onlyRole(RELAYER_ROLE) {
+        BridgeTxDetails storage $ = bridgeTxDetails[transactionId];
+
         // update bridge tx status given proof provided
-        if (bridgeTxDetails[transactionId].status != BridgeStatus.REQUESTED) revert StatusIncorrect();
-        bridgeTxDetails[transactionId].status = BridgeStatus.RELAYER_PROVED;
-        bridgeTxDetails[transactionId].proofBlockTimestamp = uint40(block.timestamp);
-        bridgeTxDetails[transactionId].proofBlockNumber = uint48(block.number);
-        bridgeTxDetails[transactionId].proofRelayer = relayer;
+        if ($.status != BridgeStatus.REQUESTED) revert StatusIncorrect();
+        // Note: these are storage writes
+        $.status = BridgeStatus.RELAYER_PROVED;
+        $.proofBlockTimestamp = uint40(block.timestamp);
+        $.proofBlockNumber = uint48(block.number);
+        $.proofRelayer = relayer;
 
         emit BridgeProofProvided(transactionId, relayer, destTxHash);
     }
@@ -288,21 +299,27 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {
         bytes32 transactionId = keccak256(request);
         BridgeTransactionV2 memory transaction = getBridgeTransactionV2(request);
 
+        BridgeTxDetails storage $ = bridgeTxDetails[transactionId];
+
+        address proofRelayer = $.proofRelayer;
+        BridgeStatus status = $.status;
+        uint40 proofBlockTimestamp = $.proofBlockTimestamp;
+
         // update bridge tx status if able to claim origin collateral
-        if (bridgeTxDetails[transactionId].status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect();
+        if (status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect();
 
         // if "to" is zero addr, permissionlessly send funds to proven relayer
         if (to == address(0)) {
-            to = bridgeTxDetails[transactionId].proofRelayer;
-        } else if (bridgeTxDetails[transactionId].proofRelayer != msg.sender) {
+            to = proofRelayer;
+        } else if (proofRelayer != msg.sender) {
             revert SenderIncorrect();
         }
 
-        if (_timeSince(bridgeTxDetails[transactionId].proofBlockTimestamp) <= DISPUTE_PERIOD) {
+        if (_timeSince(proofBlockTimestamp) <= DISPUTE_PERIOD) {
             revert DisputePeriodNotPassed();
         }
-
-        bridgeTxDetails[transactionId].status = BridgeStatus.RELAYER_CLAIMED;
+        // Note: this is a storage write
+        $.status = BridgeStatus.RELAYER_CLAIMED;
 
         // update protocol fees if origin fee amount exists
         if (transaction.originFeeAmount > 0) protocolFees[transaction.originToken] += transaction.originFeeAmount;
@@ -317,22 +334,18 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors {
             IERC20(token).safeTransfer(to, amount);
         }
 
-        emit BridgeDepositClaimed(
-            transactionId,
-            bridgeTxDetails[transactionId].proofRelayer,
-            to,
-            transaction.originToken,
-            transaction.originAmount
-        );
+        emit BridgeDepositClaimed(transactionId, proofRelayer, to, transaction.originToken, transaction.originAmount);
     }
 
     function bridgeStatuses(bytes32 transactionId) public view returns (BridgeStatus status) {
         return bridgeTxDetails[transactionId].status;
     }
 
     function bridgeProofs(bytes32 transactionId) public view returns (uint96 timestamp, address relayer) {
-        timestamp = bridgeTxDetails[transactionId].proofBlockTimestamp;
-        relayer = bridgeTxDetails[transactionId].proofRelayer;
+        BridgeTxDetails storage $ = bridgeTxDetails[transactionId];
+
+        timestamp = $.proofBlockTimestamp;
+        relayer = $.proofRelayer;
     }
 
     /// @inheritdoc IFastBridgeV2