From 1348e636f79c68da30ea40fc4c8a9fe7d381385c Mon Sep 17 00:00:00 2001 From: parodime Date: Mon, 23 Sep 2024 15:32:00 -0400 Subject: [PATCH 01/11] status/proof mapping packing [SLT-186] --- .../contracts-rfq/contracts/FastBridgeV2.sol | 51 +++++++-------- .../contracts/interfaces/IFastBridgeV2.sol | 29 +++++++++ .../contracts-rfq/test/FastBridgeV2.Src.t.sol | 62 +++++++++---------- .../contracts-rfq/test/FastBridgeV2.t.sol | 1 + 4 files changed, 87 insertions(+), 56 deletions(-) diff --git a/packages/contracts-rfq/contracts/FastBridgeV2.sol b/packages/contracts-rfq/contracts/FastBridgeV2.sol index 7f938f28f1..352b2df2bb 100644 --- a/packages/contracts-rfq/contracts/FastBridgeV2.sol +++ b/packages/contracts-rfq/contracts/FastBridgeV2.sol @@ -23,18 +23,9 @@ contract FastBridgeV2 is Admin, IFastBridgeV2 { /// @notice Minimum deadline period to relay a requested bridge transaction uint256 public constant MIN_DEADLINE_PERIOD = 30 minutes; - enum BridgeStatus { - NULL, // doesn't exist yet - REQUESTED, - RELAYER_PROVED, - RELAYER_CLAIMED, - REFUNDED - } /// @notice Status of the bridge tx on origin chain - mapping(bytes32 => BridgeStatus) public bridgeStatuses; - /// @notice Proof of relayed bridge tx on origin chain - mapping(bytes32 => BridgeProof) public bridgeProofs; + mapping(bytes32 => BridgeTxDetails) public bridgeTxDetails; /// @notice Whether bridge has been relayed on destination chain mapping(bytes32 => bool) public bridgeRelays; @@ -43,6 +34,16 @@ contract FastBridgeV2 is Admin, IFastBridgeV2 { // @dev the block the contract was deployed at uint256 public immutable deployBlock; + function bridgeStatuses(bytes32 transactionId) public view returns (BridgeStatus status) + { + return bridgeTxDetails[transactionId].status; + } + + function bridgeProofs(bytes32 transactionId) public view returns (BridgeProof memory proof) + { + return bridgeTxDetails[transactionId].proof; + } + constructor(address _owner) Admin(_owner) { deployBlock = block.number; } @@ -109,7 +110,7 @@ contract FastBridgeV2 is Admin, IFastBridgeV2 { }) ); bytes32 transactionId = keccak256(request); - bridgeStatuses[transactionId] = BridgeStatus.REQUESTED; + bridgeTxDetails[transactionId].status = BridgeStatus.REQUESTED; emit BridgeRequested( transactionId, @@ -183,9 +184,9 @@ contract FastBridgeV2 is Admin, IFastBridgeV2 { function prove(bytes memory request, bytes32 destTxHash, address relayer) public onlyRole(RELAYER_ROLE) { bytes32 transactionId = keccak256(request); // update bridge tx status given proof provided - if (bridgeStatuses[transactionId] != BridgeStatus.REQUESTED) revert StatusIncorrect(); - bridgeStatuses[transactionId] = BridgeStatus.RELAYER_PROVED; - bridgeProofs[transactionId] = BridgeProof({timestamp: uint96(block.timestamp), relayer: relayer}); // overflow ok + if (bridgeTxDetails[transactionId].status != BridgeStatus.REQUESTED) revert StatusIncorrect(); + bridgeTxDetails[transactionId].status = BridgeStatus.RELAYER_PROVED; + bridgeTxDetails[transactionId].proof = BridgeProof({timestamp: uint96(block.timestamp), relayer: relayer}); // overflow ok emit BridgeProofProvided(transactionId, relayer, destTxHash); } @@ -204,8 +205,8 @@ contract FastBridgeV2 is Admin, IFastBridgeV2 { /// @inheritdoc IFastBridge function canClaim(bytes32 transactionId, address relayer) external view returns (bool) { - if (bridgeStatuses[transactionId] != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); - BridgeProof memory proof = bridgeProofs[transactionId]; + if (bridgeTxDetails[transactionId].status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); + BridgeProof memory proof = bridgeTxDetails[transactionId].proof; if (proof.relayer != relayer) revert SenderIncorrect(); return _timeSince(proof) > DISPUTE_PERIOD; } @@ -221,9 +222,9 @@ contract FastBridgeV2 is Admin, IFastBridgeV2 { BridgeTransaction memory transaction = getBridgeTransaction(request); // update bridge tx status if able to claim origin collateral - if (bridgeStatuses[transactionId] != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); + if (bridgeTxDetails[transactionId].status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); - BridgeProof memory proof = bridgeProofs[transactionId]; + BridgeProof memory proof = bridgeTxDetails[transactionId].proof; // if "to" is zero addr, permissionlessly send funds to proven relayer if (to == address(0)) { @@ -234,7 +235,7 @@ contract FastBridgeV2 is Admin, IFastBridgeV2 { if (_timeSince(proof) <= DISPUTE_PERIOD) revert DisputePeriodNotPassed(); - bridgeStatuses[transactionId] = BridgeStatus.RELAYER_CLAIMED; + bridgeTxDetails[transactionId].status = BridgeStatus.RELAYER_CLAIMED; // update protocol fees if origin fee amount exists if (transaction.originFeeAmount > 0) protocolFees[transaction.originToken] += transaction.originFeeAmount; @@ -249,12 +250,12 @@ contract FastBridgeV2 is Admin, IFastBridgeV2 { /// @inheritdoc IFastBridge function dispute(bytes32 transactionId) external onlyRole(GUARD_ROLE) { - if (bridgeStatuses[transactionId] != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); - if (_timeSince(bridgeProofs[transactionId]) > DISPUTE_PERIOD) revert DisputePeriodPassed(); + if (bridgeTxDetails[transactionId].status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); + if (_timeSince(bridgeTxDetails[transactionId].proof) > DISPUTE_PERIOD) revert DisputePeriodPassed(); // @dev relayer gets slashed effectively if dest relay has gone thru - bridgeStatuses[transactionId] = BridgeStatus.REQUESTED; - delete bridgeProofs[transactionId]; + bridgeTxDetails[transactionId].status = BridgeStatus.REQUESTED; + delete bridgeTxDetails[transactionId].proof; emit BridgeProofDisputed(transactionId, msg.sender); } @@ -273,8 +274,8 @@ contract FastBridgeV2 is Admin, IFastBridgeV2 { } // set status to refunded if still in requested state - if (bridgeStatuses[transactionId] != BridgeStatus.REQUESTED) revert StatusIncorrect(); - bridgeStatuses[transactionId] = BridgeStatus.REFUNDED; + if (bridgeTxDetails[transactionId].status != BridgeStatus.REQUESTED) revert StatusIncorrect(); + bridgeTxDetails[transactionId].status = BridgeStatus.REFUNDED; // transfer origin collateral back to original sender address to = transaction.originSender; diff --git a/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol b/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol index 979551919a..19b65ac6d5 100644 --- a/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol +++ b/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol @@ -20,4 +20,33 @@ interface IFastBridgeV2 is IFastBridge { /// @param request The encoded bridge transaction to claim on origin chain function claim(bytes memory request) external; + + + + + + + enum BridgeStatus { + NULL, // doesn't exist yet + REQUESTED, + RELAYER_PROVED, + RELAYER_CLAIMED, + REFUNDED + } + + struct BridgeTxDetails { + BridgeStatus status; + BridgeProof proof; + } + + + /// @notice Returns the status of a bridge transaction + /// @param transactionId The ID of the bridge transaction + /// @return The status of the bridge transaction + function bridgeStatuses(bytes32 transactionId) external view returns (BridgeStatus); + + /// @notice Returns the timestamp and relayer of a bridge proof + /// @param transactionId The ID of the bridge transaction + /// @return The timestamp and relayer address of the bridge proof + function bridgeProofs(bytes32 transactionId) external view returns (uint96, address); } diff --git a/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol index 8a0106d0c0..586b2b9691 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol @@ -14,7 +14,7 @@ import { ZeroAddress } from "../contracts/libs/Errors.sol"; -import {FastBridgeV2, FastBridgeV2Test, IFastBridge} from "./FastBridgeV2.t.sol"; +import {FastBridgeV2, FastBridgeV2Test, IFastBridge, IFastBridgeV2} from "./FastBridgeV2.t.sol"; // solhint-disable func-name-mixedcase, ordering contract FastBridgeV2SrcTest is FastBridgeV2Test { @@ -167,7 +167,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { }); } - function assertEq(FastBridgeV2.BridgeStatus a, FastBridgeV2.BridgeStatus b) public pure { + function assertEq(IFastBridgeV2.BridgeStatus a, IFastBridgeV2.BridgeStatus b) public pure { assertEq(uint8(a), uint8(b)); } @@ -183,7 +183,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { bytes32 txId = getTxId(tokenTx); expectBridgeRequested(tokenTx, txId); bridge({caller: userA, msgValue: 0, params: tokenParams}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REQUESTED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REQUESTED); checkTokenBalancesAfterBridge(userA); } @@ -191,7 +191,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { bytes32 txId = getTxId(tokenTx); expectBridgeRequested(tokenTx, txId); bridge({caller: userB, msgValue: 0, params: tokenParams}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REQUESTED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REQUESTED); assertEq(srcToken.balanceOf(userA), LEFTOVER_BALANCE + tokenParams.originAmount); checkTokenBalancesAfterBridge(userB); } @@ -208,7 +208,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { bytes32 txId = getTxId(ethTx); expectBridgeRequested(ethTx, txId); bridge({caller: userA, msgValue: ethParams.originAmount, params: ethParams}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REQUESTED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REQUESTED); checkEthBalancesAfterBridge(userA); } @@ -218,7 +218,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { bytes32 txId = getTxId(ethTx); expectBridgeRequested(ethTx, txId); bridge({caller: userB, msgValue: ethParams.originAmount, params: ethParams}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REQUESTED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REQUESTED); assertEq(userA.balance, LEFTOVER_BALANCE + ethParams.originAmount); checkEthBalancesAfterBridge(userB); } @@ -233,7 +233,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { bytes32 txId = getTxId(ethTx); expectBridgeRequested(ethTx, txId); bridge({caller: userB, msgValue: ethParams.originAmount, params: ethParams}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REQUESTED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REQUESTED); checkEthBalancesAfterBridge(userB); } @@ -309,7 +309,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { bridge({caller: userA, msgValue: 0, params: tokenParams}); expectBridgeProofProvided({txId: txId, relayer: relayerA, destTxHash: hex"01"}); prove({caller: relayerA, bridgeTx: tokenTx, destTxHash: hex"01"}); - (uint96 timestamp, address relayer) = fastBridge.bridgeProofs(txId); + (uint96 timestamp, address relayer) = (fastBridge.bridgeProofs(txId).timestamp, fastBridge.bridgeProofs(txId).relayer); assertEq(timestamp, block.timestamp); assertEq(relayer, relayerA); assertEq(srcToken.balanceOf(address(fastBridge)), INITIAL_PROTOCOL_FEES_TOKEN + tokenParams.originAmount); @@ -323,7 +323,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { bridge({caller: userA, msgValue: ethParams.originAmount, params: ethParams}); expectBridgeProofProvided({txId: txId, relayer: relayerA, destTxHash: hex"01"}); prove({caller: relayerA, bridgeTx: ethTx, destTxHash: hex"01"}); - (uint96 timestamp, address relayer) = fastBridge.bridgeProofs(txId); + (uint96 timestamp, address relayer) = (fastBridge.bridgeProofs(txId).timestamp, fastBridge.bridgeProofs(txId).relayer); assertEq(timestamp, block.timestamp); assertEq(relayer, relayerA); assertEq(address(fastBridge).balance, INITIAL_PROTOCOL_FEES_ETH + ethParams.originAmount); @@ -382,7 +382,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { assertTrue(fastBridge.canClaim(txId, relayerA)); expectBridgeDepositClaimed({bridgeTx: tokenTx, txId: txId, relayer: relayerA, to: relayerA}); claim({caller: relayerA, bridgeTx: tokenTx, to: relayerA}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.RELAYER_CLAIMED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.RELAYER_CLAIMED); checkTokenBalancesAfterClaim(relayerA); } @@ -394,7 +394,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { skip(CLAIM_DELAY + 1); expectBridgeDepositClaimed({bridgeTx: tokenTx, txId: txId, relayer: relayerA, to: relayerA}); claim({caller: caller, bridgeTx: tokenTx}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.RELAYER_CLAIMED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.RELAYER_CLAIMED); checkTokenBalancesAfterClaim(relayerA); } @@ -406,7 +406,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { skip(CLAIM_DELAY + 1); expectBridgeDepositClaimed({bridgeTx: tokenTx, txId: txId, relayer: relayerA, to: relayerA}); claim({caller: caller, bridgeTx: tokenTx, to: address(0)}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.RELAYER_CLAIMED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.RELAYER_CLAIMED); checkTokenBalancesAfterClaim(relayerA); } @@ -417,7 +417,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { skip(CLAIM_DELAY + 1); expectBridgeDepositClaimed({bridgeTx: tokenTx, txId: txId, relayer: relayerA, to: claimTo}); claim({caller: relayerA, bridgeTx: tokenTx, to: claimTo}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.RELAYER_CLAIMED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.RELAYER_CLAIMED); assertEq(srcToken.balanceOf(relayerA), 0); checkTokenBalancesAfterClaim(claimTo); } @@ -429,7 +429,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { skip(CLAIM_DELAY + 30 days); expectBridgeDepositClaimed({bridgeTx: tokenTx, txId: txId, relayer: relayerA, to: relayerA}); claim({caller: relayerA, bridgeTx: tokenTx, to: relayerA}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.RELAYER_CLAIMED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.RELAYER_CLAIMED); checkTokenBalancesAfterClaim(relayerA); } @@ -448,7 +448,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { assertTrue(fastBridge.canClaim(txId, relayerA)); expectBridgeDepositClaimed({bridgeTx: ethTx, txId: txId, relayer: relayerA, to: relayerA}); claim({caller: relayerA, bridgeTx: ethTx, to: relayerA}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.RELAYER_CLAIMED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.RELAYER_CLAIMED); checkEthBalancesAfterClaim(relayerA); } @@ -461,7 +461,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { skip(CLAIM_DELAY + 1); expectBridgeDepositClaimed({bridgeTx: ethTx, txId: txId, relayer: relayerA, to: relayerA}); claim({caller: caller, bridgeTx: ethTx}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.RELAYER_CLAIMED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.RELAYER_CLAIMED); checkEthBalancesAfterClaim(relayerA); } @@ -474,7 +474,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { skip(CLAIM_DELAY + 1); expectBridgeDepositClaimed({bridgeTx: ethTx, txId: txId, relayer: relayerA, to: relayerA}); claim({caller: caller, bridgeTx: ethTx, to: address(0)}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.RELAYER_CLAIMED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.RELAYER_CLAIMED); checkEthBalancesAfterClaim(relayerA); } @@ -486,7 +486,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { skip(CLAIM_DELAY + 1); expectBridgeDepositClaimed({bridgeTx: ethTx, txId: txId, relayer: relayerA, to: claimTo}); claim({caller: relayerA, bridgeTx: ethTx, to: claimTo}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.RELAYER_CLAIMED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.RELAYER_CLAIMED); checkEthBalancesAfterClaim(claimTo); } @@ -498,7 +498,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { skip(CLAIM_DELAY + 30 days); expectBridgeDepositClaimed({bridgeTx: ethTx, txId: txId, relayer: relayerA, to: relayerA}); claim({caller: relayerA, bridgeTx: ethTx, to: relayerA}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.RELAYER_CLAIMED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.RELAYER_CLAIMED); checkEthBalancesAfterClaim(relayerA); } @@ -578,7 +578,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { prove({caller: relayerA, bridgeTx: tokenTx, destTxHash: hex"01"}); expectBridgeProofDisputed({txId: txId, guard: guard}); dispute({caller: guard, txId: txId}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REQUESTED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REQUESTED); assertEq(fastBridge.protocolFees(address(srcToken)), INITIAL_PROTOCOL_FEES_TOKEN); assertEq(srcToken.balanceOf(address(fastBridge)), INITIAL_PROTOCOL_FEES_TOKEN + tokenParams.originAmount); } @@ -590,7 +590,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { skip(CLAIM_DELAY); expectBridgeProofDisputed({txId: txId, guard: guard}); dispute({caller: guard, txId: txId}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REQUESTED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REQUESTED); assertEq(fastBridge.protocolFees(address(srcToken)), INITIAL_PROTOCOL_FEES_TOKEN); assertEq(srcToken.balanceOf(address(fastBridge)), INITIAL_PROTOCOL_FEES_TOKEN + tokenParams.originAmount); } @@ -602,7 +602,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { prove({caller: relayerA, bridgeTx: ethTx, destTxHash: hex"01"}); expectBridgeProofDisputed({txId: txId, guard: guard}); dispute({caller: guard, txId: txId}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REQUESTED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REQUESTED); assertEq(fastBridge.protocolFees(ETH_ADDRESS), INITIAL_PROTOCOL_FEES_ETH); assertEq(address(fastBridge).balance, INITIAL_PROTOCOL_FEES_ETH + ethParams.originAmount); } @@ -615,7 +615,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { skip(CLAIM_DELAY); expectBridgeProofDisputed({txId: txId, guard: guard}); dispute({caller: guard, txId: txId}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REQUESTED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REQUESTED); assertEq(fastBridge.protocolFees(ETH_ADDRESS), INITIAL_PROTOCOL_FEES_ETH); assertEq(address(fastBridge).balance, INITIAL_PROTOCOL_FEES_ETH + ethParams.originAmount); } @@ -678,7 +678,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { skip(DEADLINE + 1); expectBridgeDepositRefunded({bridgeParams: tokenParams, txId: txId}); refund({caller: refunder, bridgeTx: tokenTx}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REFUNDED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REFUNDED); 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); @@ -691,7 +691,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { skip(DEADLINE + 1); expectBridgeDepositRefunded({bridgeParams: tokenParams, txId: txId}); refund({caller: refunder, bridgeTx: tokenTx}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REFUNDED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REFUNDED); assertEq(fastBridge.protocolFees(address(srcToken)), INITIAL_PROTOCOL_FEES_TOKEN); assertEq(srcToken.balanceOf(userA), LEFTOVER_BALANCE + 2 * tokenParams.originAmount); assertEq(srcToken.balanceOf(userB), LEFTOVER_BALANCE); @@ -704,7 +704,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { skip(DEADLINE + 30 days); expectBridgeDepositRefunded({bridgeParams: tokenParams, txId: txId}); refund({caller: refunder, bridgeTx: tokenTx}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REFUNDED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REFUNDED); 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); @@ -717,7 +717,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { skip(DEADLINE + PERMISSIONLESS_REFUND_DELAY + 1); expectBridgeDepositRefunded({bridgeParams: tokenParams, txId: txId}); refund({caller: caller, bridgeTx: tokenTx}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REFUNDED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REFUNDED); 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); @@ -730,7 +730,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { skip(DEADLINE + 1); expectBridgeDepositRefunded({bridgeParams: ethParams, txId: txId}); refund({caller: refunder, bridgeTx: ethTx}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REFUNDED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REFUNDED); assertEq(fastBridge.protocolFees(ETH_ADDRESS), INITIAL_PROTOCOL_FEES_ETH); assertEq(address(userA).balance, LEFTOVER_BALANCE + ethParams.originAmount); assertEq(address(fastBridge).balance, INITIAL_PROTOCOL_FEES_ETH); @@ -744,7 +744,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { skip(DEADLINE + 1); expectBridgeDepositRefunded({bridgeParams: ethParams, txId: txId}); refund({caller: refunder, bridgeTx: ethTx}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REFUNDED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REFUNDED); assertEq(fastBridge.protocolFees(ETH_ADDRESS), INITIAL_PROTOCOL_FEES_ETH); assertEq(address(userA).balance, LEFTOVER_BALANCE + 2 * ethParams.originAmount); assertEq(address(userB).balance, LEFTOVER_BALANCE); @@ -758,7 +758,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { skip(DEADLINE + 30 days); expectBridgeDepositRefunded({bridgeParams: ethParams, txId: txId}); refund({caller: refunder, bridgeTx: ethTx}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REFUNDED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REFUNDED); assertEq(fastBridge.protocolFees(ETH_ADDRESS), INITIAL_PROTOCOL_FEES_ETH); assertEq(address(userA).balance, LEFTOVER_BALANCE + ethParams.originAmount); assertEq(address(fastBridge).balance, INITIAL_PROTOCOL_FEES_ETH); @@ -772,7 +772,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { skip(DEADLINE + PERMISSIONLESS_REFUND_DELAY + 1); expectBridgeDepositRefunded({bridgeParams: ethParams, txId: txId}); refund({caller: caller, bridgeTx: ethTx}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REFUNDED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REFUNDED); assertEq(fastBridge.protocolFees(ETH_ADDRESS), INITIAL_PROTOCOL_FEES_ETH); assertEq(address(userA).balance, LEFTOVER_BALANCE + ethParams.originAmount); assertEq(address(fastBridge).balance, INITIAL_PROTOCOL_FEES_ETH); diff --git a/packages/contracts-rfq/test/FastBridgeV2.t.sol b/packages/contracts-rfq/test/FastBridgeV2.t.sol index 4fe357e3f2..480e1778e7 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.t.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.20; import {IFastBridge} from "../contracts/interfaces/IFastBridge.sol"; +import {IFastBridgeV2} from "../contracts/interfaces/IFastBridgeV2.sol"; import {FastBridgeV2} from "../contracts/FastBridgeV2.sol"; import {MockERC20} from "./MockERC20.sol"; From 1c5dd7de13bbde7c0bde7619357b201bea33b6ad Mon Sep 17 00:00:00 2001 From: parodime Date: Mon, 23 Sep 2024 15:47:59 -0400 Subject: [PATCH 02/11] fix BridgeProofs def to use struct --- packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol b/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol index 19b65ac6d5..6fd419b23b 100644 --- a/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol +++ b/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol @@ -48,5 +48,5 @@ interface IFastBridgeV2 is IFastBridge { /// @notice Returns the timestamp and relayer of a bridge proof /// @param transactionId The ID of the bridge transaction /// @return The timestamp and relayer address of the bridge proof - function bridgeProofs(bytes32 transactionId) external view returns (uint96, address); + function bridgeProofs(bytes32 transactionId) external view returns (BridgeProof memory); } From 504583568284c8d6bfc4d8f1ba82575080c70728 Mon Sep 17 00:00:00 2001 From: parodime Date: Wed, 25 Sep 2024 05:35:39 -0400 Subject: [PATCH 03/11] WIP - evaluating ProofDetail struct approach --- .../contracts-rfq/contracts/FastBridgeV2.sol | 27 ++++++++++--------- .../contracts/interfaces/IFastBridgeV2.sol | 16 +++++------ .../contracts-rfq/test/FastBridgeV2.Src.t.sol | 6 +++-- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/packages/contracts-rfq/contracts/FastBridgeV2.sol b/packages/contracts-rfq/contracts/FastBridgeV2.sol index 352b2df2bb..2d48f42ad0 100644 --- a/packages/contracts-rfq/contracts/FastBridgeV2.sol +++ b/packages/contracts-rfq/contracts/FastBridgeV2.sol @@ -23,7 +23,6 @@ contract FastBridgeV2 is Admin, IFastBridgeV2 { /// @notice Minimum deadline period to relay a requested bridge transaction uint256 public constant MIN_DEADLINE_PERIOD = 30 minutes; - /// @notice Status of the bridge tx on origin chain mapping(bytes32 => BridgeTxDetails) public bridgeTxDetails; /// @notice Whether bridge has been relayed on destination chain @@ -34,16 +33,17 @@ contract FastBridgeV2 is Admin, IFastBridgeV2 { // @dev the block the contract was deployed at uint256 public immutable deployBlock; - function bridgeStatuses(bytes32 transactionId) public view returns (BridgeStatus status) - { + function bridgeStatuses(bytes32 transactionId) public view returns (BridgeStatus status) { return bridgeTxDetails[transactionId].status; } - - function bridgeProofs(bytes32 transactionId) public view returns (BridgeProof memory proof) - { - return bridgeTxDetails[transactionId].proof; + + function bridgeProofs(bytes32 transactionId) public view returns (BridgeProof memory proof) { + return BridgeProof({ + timestamp: bridgeTxDetails[transactionId].proof.blockTimestamp, + relayer: bridgeTxDetails[transactionId].proof.relayer + }); } - + constructor(address _owner) Admin(_owner) { deployBlock = block.number; } @@ -186,7 +186,8 @@ contract FastBridgeV2 is Admin, IFastBridgeV2 { // update bridge tx status given proof provided if (bridgeTxDetails[transactionId].status != BridgeStatus.REQUESTED) revert StatusIncorrect(); bridgeTxDetails[transactionId].status = BridgeStatus.RELAYER_PROVED; - bridgeTxDetails[transactionId].proof = BridgeProof({timestamp: uint96(block.timestamp), relayer: relayer}); // overflow ok + bridgeTxDetails[transactionId].proof = + ProofDetail({blockTimestamp: uint40(block.timestamp), blockNumber: uint48(block.number), relayer: relayer}); // overflow ok emit BridgeProofProvided(transactionId, relayer, destTxHash); } @@ -197,16 +198,16 @@ contract FastBridgeV2 is Admin, IFastBridgeV2 { /// proof.timestamp < type(uint96).max via unchecked statement /// @param proof The bridge proof /// @return delta Time delta since proof submitted - function _timeSince(BridgeProof memory proof) internal view returns (uint256 delta) { + function _timeSince(ProofDetail memory proof) internal view returns (uint256 delta) { unchecked { - delta = uint96(block.timestamp) - proof.timestamp; + delta = uint40(block.timestamp) - proof.blockTimestamp; } } /// @inheritdoc IFastBridge function canClaim(bytes32 transactionId, address relayer) external view returns (bool) { if (bridgeTxDetails[transactionId].status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); - BridgeProof memory proof = bridgeTxDetails[transactionId].proof; + ProofDetail memory proof = bridgeTxDetails[transactionId].proof; if (proof.relayer != relayer) revert SenderIncorrect(); return _timeSince(proof) > DISPUTE_PERIOD; } @@ -224,7 +225,7 @@ contract FastBridgeV2 is Admin, IFastBridgeV2 { // update bridge tx status if able to claim origin collateral if (bridgeTxDetails[transactionId].status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); - BridgeProof memory proof = bridgeTxDetails[transactionId].proof; + ProofDetail memory proof = bridgeTxDetails[transactionId].proof; // if "to" is zero addr, permissionlessly send funds to proven relayer if (to == address(0)) { diff --git a/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol b/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol index 6fd419b23b..cf4b6ca6db 100644 --- a/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol +++ b/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol @@ -4,7 +4,6 @@ pragma solidity ^0.8.20; import {IFastBridge} from "./IFastBridge.sol"; interface IFastBridgeV2 is IFastBridge { - /// @notice Relays destination side of bridge transaction by off-chain relayer /// @param request The encoded bridge transaction to relay on destination chain /// @param relayer The address of the relaying entity which should have control of the origin funds when claimed @@ -20,12 +19,6 @@ interface IFastBridgeV2 is IFastBridge { /// @param request The encoded bridge transaction to claim on origin chain function claim(bytes memory request) external; - - - - - - enum BridgeStatus { NULL, // doesn't exist yet REQUESTED, @@ -34,12 +27,17 @@ interface IFastBridgeV2 is IFastBridge { REFUNDED } + struct ProofDetail { + uint40 blockTimestamp; + uint48 blockNumber; + address relayer; + } + struct BridgeTxDetails { BridgeStatus status; - BridgeProof proof; + ProofDetail proof; } - /// @notice Returns the status of a bridge transaction /// @param transactionId The ID of the bridge transaction /// @return The status of the bridge transaction diff --git a/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol index 586b2b9691..4b565ca6b8 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol @@ -309,7 +309,8 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { bridge({caller: userA, msgValue: 0, params: tokenParams}); expectBridgeProofProvided({txId: txId, relayer: relayerA, destTxHash: hex"01"}); prove({caller: relayerA, bridgeTx: tokenTx, destTxHash: hex"01"}); - (uint96 timestamp, address relayer) = (fastBridge.bridgeProofs(txId).timestamp, fastBridge.bridgeProofs(txId).relayer); + (uint96 timestamp, address relayer) = + (fastBridge.bridgeProofs(txId).timestamp, fastBridge.bridgeProofs(txId).relayer); assertEq(timestamp, block.timestamp); assertEq(relayer, relayerA); assertEq(srcToken.balanceOf(address(fastBridge)), INITIAL_PROTOCOL_FEES_TOKEN + tokenParams.originAmount); @@ -323,7 +324,8 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { bridge({caller: userA, msgValue: ethParams.originAmount, params: ethParams}); expectBridgeProofProvided({txId: txId, relayer: relayerA, destTxHash: hex"01"}); prove({caller: relayerA, bridgeTx: ethTx, destTxHash: hex"01"}); - (uint96 timestamp, address relayer) = (fastBridge.bridgeProofs(txId).timestamp, fastBridge.bridgeProofs(txId).relayer); + (uint96 timestamp, address relayer) = + (fastBridge.bridgeProofs(txId).timestamp, fastBridge.bridgeProofs(txId).relayer); assertEq(timestamp, block.timestamp); assertEq(relayer, relayerA); assertEq(address(fastBridge).balance, INITIAL_PROTOCOL_FEES_ETH + ethParams.originAmount); From 7f5c260aad202ea9fba20d0accccd302196c15eb Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 25 Sep 2024 17:09:37 +0100 Subject: [PATCH 04/11] fix: overflow in parity tests --- packages/contracts-rfq/test/FastBridge.t.sol | 34 +++++++++++-------- .../test/FastBridgeV2.Parity.t.sol | 15 ++++++++ 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/packages/contracts-rfq/test/FastBridge.t.sol b/packages/contracts-rfq/test/FastBridge.t.sol index 2d90779c96..a571f50b40 100644 --- a/packages/contracts-rfq/test/FastBridge.t.sol +++ b/packages/contracts-rfq/test/FastBridge.t.sol @@ -1,5 +1,6 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.13; +// solhint-disable import "forge-std/Test.sol"; import "forge-std/console2.sol"; @@ -51,6 +52,19 @@ contract FastBridgeTest is Test { ethUSDC.mint(dstUser, 100 * 10 ** 6); } + function assertCorrectProof( + bytes32 transactionId, + uint256 expectedTimestamp, + address expectedRelayer + ) + internal + virtual + { + (uint96 timestamp, address relayer) = fastBridge.bridgeProofs(transactionId); + assertEq(timestamp, uint96(expectedTimestamp)); + assertEq(relayer, expectedRelayer); + } + function _getBridgeRequestAndId( uint256 chainId, uint256 currentNonce, @@ -1349,9 +1363,7 @@ contract FastBridgeTest is Test { fastBridge.prove(request, fakeTxnHash); // We check if the bridge transaction proof timestamp is set to the timestamp at which the proof was provided - (uint96 _timestamp, address _oldRelayer) = fastBridge.bridgeProofs(transactionId); - assertEq(_timestamp, uint96(block.timestamp)); - assertEq(_oldRelayer, relayer); + assertCorrectProof(transactionId, block.timestamp, relayer); // We check if the bridge status is RELAYER_PROVED assertEq(uint256(fastBridge.bridgeStatuses(transactionId)), uint256(FastBridge.BridgeStatus.RELAYER_PROVED)); @@ -1383,9 +1395,7 @@ contract FastBridgeTest is Test { fastBridge.prove(request, fakeTxnHash); // We check if the bridge transaction proof timestamp is set to the timestamp at which the proof was provided - (uint96 _timestamp, address _oldRelayer) = fastBridge.bridgeProofs(transactionId); - assertEq(_timestamp, uint96(block.timestamp)); - assertEq(_oldRelayer, relayer); + assertCorrectProof(transactionId, block.timestamp, relayer); // We stop a prank to contain within test vm.stopPrank(); @@ -1414,9 +1424,7 @@ contract FastBridgeTest is Test { fastBridge.prove(request, fakeTxnHash); // We check if the bridge transaction proof timestamp is set to the timestamp at which the proof was provided - (uint96 _timestamp, address _oldRelayer) = fastBridge.bridgeProofs(transactionId); - assertEq(_timestamp, uint96(block.timestamp)); - assertEq(_oldRelayer, relayer); + assertCorrectProof(transactionId, block.timestamp, relayer); // We stop a prank to contain within test vm.stopPrank(); @@ -1713,10 +1721,8 @@ contract FastBridgeTest is Test { fastBridge.dispute(transactionId); // check status and proofs updated - (uint96 _timestamp, address _oldRelayer) = fastBridge.bridgeProofs(transactionId); assertEq(uint256(fastBridge.bridgeStatuses(transactionId)), uint256(FastBridge.BridgeStatus.REQUESTED)); - assertEq(_timestamp, 0); - assertEq(_oldRelayer, address(0)); + assertCorrectProof(transactionId, 0, address(0)); // We stop a prank to contain within test vm.stopPrank(); @@ -1739,10 +1745,8 @@ contract FastBridgeTest is Test { fastBridge.dispute(transactionId); // check status and proofs updated - (uint96 _timestamp, address _oldRelayer) = fastBridge.bridgeProofs(transactionId); assertEq(uint256(fastBridge.bridgeStatuses(transactionId)), uint256(FastBridge.BridgeStatus.REQUESTED)); - assertEq(_timestamp, 0); - assertEq(_oldRelayer, address(0)); + assertCorrectProof(transactionId, 0, address(0)); // We stop a prank to contain within test vm.stopPrank(); diff --git a/packages/contracts-rfq/test/FastBridgeV2.Parity.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Parity.t.sol index d92e4f8ac5..d7c19cc77a 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.Parity.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.Parity.t.sol @@ -12,6 +12,21 @@ contract FastBridgeV2ParityTest is FastBridgeTest { return deployCode({what: "FastBridgeV2", args: abi.encode(owner)}); } + /// @notice We use uint40 for the timestamps in FastBridgeV2 + function assertCorrectProof( + bytes32 transactionId, + uint256 expectedTimestamp, + address expectedRelayer + ) + internal + virtual + override + { + (uint96 timestamp, address relayer) = fastBridge.bridgeProofs(transactionId); + assertEq(timestamp, uint40(expectedTimestamp)); + assertEq(relayer, expectedRelayer); + } + /// @notice Relay function is no longer permissioned, so we skip this test function test_failedRelayNotRelayer() public virtual override { vm.skip(true); From dc2dab507812546c11e024eeef039f99be03bacf Mon Sep 17 00:00:00 2001 From: parodime Date: Wed, 25 Sep 2024 14:43:56 -0400 Subject: [PATCH 05/11] refactor BridgeTxDetails w/o addtl ProofDetail struct --- .../contracts-rfq/contracts/FastBridgeV2.sol | 68 ++++++++++--------- .../contracts/interfaces/IFastBridgeV2.sol | 10 +-- 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/packages/contracts-rfq/contracts/FastBridgeV2.sol b/packages/contracts-rfq/contracts/FastBridgeV2.sol index 2d48f42ad0..0afd87dcbd 100644 --- a/packages/contracts-rfq/contracts/FastBridgeV2.sol +++ b/packages/contracts-rfq/contracts/FastBridgeV2.sol @@ -37,11 +37,9 @@ contract FastBridgeV2 is Admin, IFastBridgeV2 { return bridgeTxDetails[transactionId].status; } - function bridgeProofs(bytes32 transactionId) public view returns (BridgeProof memory proof) { - return BridgeProof({ - timestamp: bridgeTxDetails[transactionId].proof.blockTimestamp, - relayer: bridgeTxDetails[transactionId].proof.relayer - }); + function bridgeProofs(bytes32 transactionId) public view returns (BridgeProof memory) { + BridgeTxDetails storage _thisBridgeTxDetails = bridgeTxDetails[transactionId]; + return BridgeProof(_thisBridgeTxDetails.proofBlockTimestamp, _thisBridgeTxDetails.proofRelayer); } constructor(address _owner) Admin(_owner) { @@ -185,31 +183,33 @@ contract FastBridgeV2 is Admin, IFastBridgeV2 { bytes32 transactionId = keccak256(request); // update bridge tx status given proof provided if (bridgeTxDetails[transactionId].status != BridgeStatus.REQUESTED) revert StatusIncorrect(); - bridgeTxDetails[transactionId].status = BridgeStatus.RELAYER_PROVED; - bridgeTxDetails[transactionId].proof = - ProofDetail({blockTimestamp: uint40(block.timestamp), blockNumber: uint48(block.number), relayer: relayer}); // overflow ok + BridgeTxDetails storage _thisBridgeTxDetails = bridgeTxDetails[transactionId]; + _thisBridgeTxDetails.status = BridgeStatus.RELAYER_PROVED; + _thisBridgeTxDetails.proofBlockTimestamp = uint40(block.timestamp); + _thisBridgeTxDetails.proofBlockNumber = uint48(block.number); + _thisBridgeTxDetails.proofRelayer = relayer; emit BridgeProofProvided(transactionId, relayer, destTxHash); } /// @notice Calculates time since proof submitted - /// @dev proof.timestamp stores casted uint96(block.timestamp) block timestamps for gas optimization - /// _timeSince(proof) can accomodate rollover case when block.timestamp > type(uint96).max but - /// proof.timestamp < type(uint96).max via unchecked statement - /// @param proof The bridge proof + /// @dev proof.timestamp stores casted uint40(block.timestamp) block timestamps for gas optimization + /// _timeSince(proof) can accomodate rollover case when block.timestamp > type(uint40).max but + /// proof.timestamp < type(uint40).max via unchecked statement + /// @param proofBlockTimestamp The bridge proof block timestamp /// @return delta Time delta since proof submitted - function _timeSince(ProofDetail memory proof) internal view returns (uint256 delta) { + function _timeSince(uint40 proofBlockTimestamp) internal view returns (uint256 delta) { unchecked { - delta = uint40(block.timestamp) - proof.blockTimestamp; + delta = uint40(block.timestamp) - proofBlockTimestamp; } } /// @inheritdoc IFastBridge function canClaim(bytes32 transactionId, address relayer) external view returns (bool) { - if (bridgeTxDetails[transactionId].status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); - ProofDetail memory proof = bridgeTxDetails[transactionId].proof; - if (proof.relayer != relayer) revert SenderIncorrect(); - return _timeSince(proof) > DISPUTE_PERIOD; + BridgeTxDetails storage _thisBridgeTxDetails = bridgeTxDetails[transactionId]; + if (_thisBridgeTxDetails.status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); + if (_thisBridgeTxDetails.proofRelayer != relayer) revert SenderIncorrect(); + return _timeSince(_thisBridgeTxDetails.proofBlockTimestamp) > DISPUTE_PERIOD; } /// @inheritdoc IFastBridgeV2 @@ -222,21 +222,21 @@ contract FastBridgeV2 is Admin, IFastBridgeV2 { bytes32 transactionId = keccak256(request); BridgeTransaction memory transaction = getBridgeTransaction(request); - // update bridge tx status if able to claim origin collateral - if (bridgeTxDetails[transactionId].status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); + BridgeTxDetails storage _thisBridgeTxDetails = bridgeTxDetails[transactionId]; - ProofDetail memory proof = bridgeTxDetails[transactionId].proof; + // update bridge tx status if able to claim origin collateral + if (_thisBridgeTxDetails.status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); // if "to" is zero addr, permissionlessly send funds to proven relayer if (to == address(0)) { - to = proof.relayer; - } else if (proof.relayer != msg.sender) { + to = _thisBridgeTxDetails.proofRelayer; + } else if (_thisBridgeTxDetails.proofRelayer != msg.sender) { revert SenderIncorrect(); } - if (_timeSince(proof) <= DISPUTE_PERIOD) revert DisputePeriodNotPassed(); + if (_timeSince(_thisBridgeTxDetails.proofBlockTimestamp) <= DISPUTE_PERIOD) revert DisputePeriodNotPassed(); - bridgeTxDetails[transactionId].status = BridgeStatus.RELAYER_CLAIMED; + _thisBridgeTxDetails.status = BridgeStatus.RELAYER_CLAIMED; // update protocol fees if origin fee amount exists if (transaction.originFeeAmount > 0) protocolFees[transaction.originToken] += transaction.originFeeAmount; @@ -246,17 +246,21 @@ contract FastBridgeV2 is Admin, IFastBridgeV2 { uint256 amount = transaction.originAmount; token.universalTransfer(to, amount); - emit BridgeDepositClaimed(transactionId, proof.relayer, to, token, amount); + emit BridgeDepositClaimed(transactionId, _thisBridgeTxDetails.proofRelayer, to, token, amount); } /// @inheritdoc IFastBridge function dispute(bytes32 transactionId) external onlyRole(GUARD_ROLE) { - if (bridgeTxDetails[transactionId].status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); - if (_timeSince(bridgeTxDetails[transactionId].proof) > DISPUTE_PERIOD) revert DisputePeriodPassed(); + BridgeTxDetails storage _thisBridgeTxDetails = bridgeTxDetails[transactionId]; + + if (_thisBridgeTxDetails.status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); + if (_timeSince(_thisBridgeTxDetails.proofBlockTimestamp) > DISPUTE_PERIOD) revert DisputePeriodPassed(); // @dev relayer gets slashed effectively if dest relay has gone thru - bridgeTxDetails[transactionId].status = BridgeStatus.REQUESTED; - delete bridgeTxDetails[transactionId].proof; + _thisBridgeTxDetails.status = BridgeStatus.REQUESTED; + _thisBridgeTxDetails.proofRelayer = address(0); + _thisBridgeTxDetails.proofBlockTimestamp = 0; + _thisBridgeTxDetails.proofBlockNumber = 0; emit BridgeProofDisputed(transactionId, msg.sender); } @@ -266,6 +270,8 @@ contract FastBridgeV2 is Admin, IFastBridgeV2 { bytes32 transactionId = keccak256(request); BridgeTransaction memory transaction = getBridgeTransaction(request); + BridgeTxDetails storage _thisBridgeTxDetails = bridgeTxDetails[transactionId]; + if (hasRole(REFUNDER_ROLE, msg.sender)) { // Refunder can refund if deadline has passed if (block.timestamp <= transaction.deadline) revert DeadlineNotExceeded(); @@ -275,7 +281,7 @@ contract FastBridgeV2 is Admin, IFastBridgeV2 { } // set status to refunded if still in requested state - if (bridgeTxDetails[transactionId].status != BridgeStatus.REQUESTED) revert StatusIncorrect(); + if (_thisBridgeTxDetails.status != BridgeStatus.REQUESTED) revert StatusIncorrect(); bridgeTxDetails[transactionId].status = BridgeStatus.REFUNDED; // transfer origin collateral back to original sender diff --git a/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol b/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol index cf4b6ca6db..ecdf1dd26d 100644 --- a/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol +++ b/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol @@ -27,15 +27,11 @@ interface IFastBridgeV2 is IFastBridge { REFUNDED } - struct ProofDetail { - uint40 blockTimestamp; - uint48 blockNumber; - address relayer; - } - struct BridgeTxDetails { BridgeStatus status; - ProofDetail proof; + uint40 proofBlockTimestamp; + uint48 proofBlockNumber; + address proofRelayer; } /// @notice Returns the status of a bridge transaction From ddc1f1f54430c26689d7a5c18278067e509e687e Mon Sep 17 00:00:00 2001 From: jw Date: Fri, 27 Sep 2024 11:57:55 -0400 Subject: [PATCH 06/11] master merge conflicts --- .../contracts-rfq/contracts/FastBridgeV2.sol | 10 +++--- .../contracts/interfaces/IFastBridgeV2.sol | 36 ++++++++++--------- .../contracts-rfq/test/FastBridgeV2.Src.t.sol | 7 ++-- .../contracts-rfq/test/FastBridgeV2.t.sol | 1 - 4 files changed, 26 insertions(+), 28 deletions(-) diff --git a/packages/contracts-rfq/contracts/FastBridgeV2.sol b/packages/contracts-rfq/contracts/FastBridgeV2.sol index 5bfe9537a9..cf12ce4215 100644 --- a/packages/contracts-rfq/contracts/FastBridgeV2.sol +++ b/packages/contracts-rfq/contracts/FastBridgeV2.sol @@ -38,9 +38,9 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { return bridgeTxDetails[transactionId].status; } - function bridgeProofs(bytes32 transactionId) public view returns (BridgeProof memory) { + function bridgeProofs(bytes32 transactionId) public view returns (uint96 timestamp, address relayer) { BridgeTxDetails storage _thisBridgeTxDetails = bridgeTxDetails[transactionId]; - return BridgeProof(_thisBridgeTxDetails.proofBlockTimestamp, _thisBridgeTxDetails.proofRelayer); + return (uint96(_thisBridgeTxDetails.proofBlockTimestamp), _thisBridgeTxDetails.proofRelayer); } constructor(address _owner) Admin(_owner) { @@ -280,12 +280,12 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { function refund(bytes memory request) external { bytes32 transactionId = keccak256(request); - if (bridgeStatuses[transactionId] != BridgeStatus.REQUESTED) revert StatusIncorrect(); - BridgeTransaction memory transaction = getBridgeTransaction(request); BridgeTxDetails storage _thisBridgeTxDetails = bridgeTxDetails[transactionId]; + if (_thisBridgeTxDetails.status != BridgeStatus.REQUESTED) revert StatusIncorrect(); + if (hasRole(REFUNDER_ROLE, msg.sender)) { // Refunder can refund if deadline has passed if (block.timestamp <= transaction.deadline) revert DeadlineNotExceeded(); @@ -295,7 +295,7 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { } // if all checks passed, set to REFUNDED status - bridgeStatuses[transactionId] = BridgeStatus.REFUNDED; + _thisBridgeTxDetails.status = BridgeStatus.REFUNDED; // transfer origin collateral back to original sender address to = transaction.originSender; diff --git a/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol b/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol index 1aa3e9f235..35c6173224 100644 --- a/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol +++ b/packages/contracts-rfq/contracts/interfaces/IFastBridgeV2.sol @@ -4,6 +4,21 @@ pragma solidity ^0.8.20; import {IFastBridge} from "./IFastBridge.sol"; interface IFastBridgeV2 is IFastBridge { + enum BridgeStatus { + NULL, // doesn't exist yet + REQUESTED, + RELAYER_PROVED, + RELAYER_CLAIMED, + REFUNDED + } + + struct BridgeTxDetails { + BridgeStatus status; + uint40 proofBlockTimestamp; + uint48 proofBlockNumber; + address proofRelayer; + } + struct BridgeRelay { uint48 blockNumber; uint48 blockTimestamp; @@ -29,28 +44,15 @@ interface IFastBridgeV2 is IFastBridge { /// @param transactionId The ID of the transaction to check /// @return True if the transaction has been relayed, false otherwise function bridgeRelays(bytes32 transactionId) external view returns (bool); - enum BridgeStatus { - NULL, // doesn't exist yet - REQUESTED, - RELAYER_PROVED, - RELAYER_CLAIMED, - REFUNDED - } - - struct BridgeTxDetails { - BridgeStatus status; - uint40 proofBlockTimestamp; - uint48 proofBlockNumber; - address proofRelayer; - } /// @notice Returns the status of a bridge transaction /// @param transactionId The ID of the bridge transaction - /// @return The status of the bridge transaction + /// @return BridgeStatus Status of the bridge transaction function bridgeStatuses(bytes32 transactionId) external view returns (BridgeStatus); /// @notice Returns the timestamp and relayer of a bridge proof /// @param transactionId The ID of the bridge transaction - /// @return The timestamp and relayer address of the bridge proof - function bridgeProofs(bytes32 transactionId) external view returns (BridgeProof memory); + /// @return timestamp The timestamp of the bridge proof + /// @return relayer The relayer address of the bridge proof + function bridgeProofs(bytes32 transactionId) external view returns (uint96 timestamp, address relayer); } diff --git a/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol index f54e3ab5c4..560e2436b5 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol @@ -14,7 +14,6 @@ import { ZeroAddress } from "../contracts/libs/Errors.sol"; - import {FastBridgeV2, FastBridgeV2Test, IFastBridge, IFastBridgeV2} from "./FastBridgeV2.t.sol"; // solhint-disable func-name-mixedcase, ordering @@ -313,8 +312,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { bridge({caller: userA, msgValue: 0, params: tokenParams}); expectBridgeProofProvided({txId: txId, relayer: relayerA, destTxHash: hex"01"}); prove({caller: relayerA, bridgeTx: tokenTx, destTxHash: hex"01"}); - (uint96 timestamp, address relayer) = - (fastBridge.bridgeProofs(txId).timestamp, fastBridge.bridgeProofs(txId).relayer); + (uint96 timestamp, address relayer) = fastBridge.bridgeProofs(txId); assertEq(timestamp, block.timestamp); assertEq(relayer, relayerA); assertEq(srcToken.balanceOf(address(fastBridge)), INITIAL_PROTOCOL_FEES_TOKEN + tokenParams.originAmount); @@ -328,8 +326,7 @@ contract FastBridgeV2SrcTest is FastBridgeV2Test { bridge({caller: userA, msgValue: ethParams.originAmount, params: ethParams}); expectBridgeProofProvided({txId: txId, relayer: relayerA, destTxHash: hex"01"}); prove({caller: relayerA, bridgeTx: ethTx, destTxHash: hex"01"}); - (uint96 timestamp, address relayer) = - (fastBridge.bridgeProofs(txId).timestamp, fastBridge.bridgeProofs(txId).relayer); + (uint96 timestamp, address relayer) = fastBridge.bridgeProofs(txId); assertEq(timestamp, block.timestamp); assertEq(relayer, relayerA); assertEq(address(fastBridge).balance, INITIAL_PROTOCOL_FEES_ETH + ethParams.originAmount); diff --git a/packages/contracts-rfq/test/FastBridgeV2.t.sol b/packages/contracts-rfq/test/FastBridgeV2.t.sol index b61d18f45a..8517a2dbf8 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.t.sol @@ -2,7 +2,6 @@ pragma solidity ^0.8.20; import {IFastBridge} from "../contracts/interfaces/IFastBridge.sol"; -import {IFastBridgeV2} from "../contracts/interfaces/IFastBridgeV2.sol"; import {IFastBridgeV2Errors} from "../contracts/interfaces/IFastBridgeV2Errors.sol"; import {FastBridgeV2} from "../contracts/FastBridgeV2.sol"; From 582941f0763f7e6c326261ea521aeae427bc442b Mon Sep 17 00:00:00 2001 From: jw Date: Fri, 27 Sep 2024 12:30:30 -0400 Subject: [PATCH 07/11] restore bad lint warning --- packages/contracts-rfq/test/FastBridgeV2.t.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/contracts-rfq/test/FastBridgeV2.t.sol b/packages/contracts-rfq/test/FastBridgeV2.t.sol index 8517a2dbf8..b61d18f45a 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.t.sol @@ -2,6 +2,7 @@ pragma solidity ^0.8.20; import {IFastBridge} from "../contracts/interfaces/IFastBridge.sol"; +import {IFastBridgeV2} from "../contracts/interfaces/IFastBridgeV2.sol"; import {IFastBridgeV2Errors} from "../contracts/interfaces/IFastBridgeV2Errors.sol"; import {FastBridgeV2} from "../contracts/FastBridgeV2.sol"; From ef3d1f711d2b37b6900c404067478a5dff412eed Mon Sep 17 00:00:00 2001 From: jw Date: Fri, 27 Sep 2024 13:18:32 -0400 Subject: [PATCH 08/11] switch to fully qualified bridgeTxDetails instead of storage refs --- .../contracts-rfq/contracts/FastBridgeV2.sol | 53 ++++++++----------- 1 file changed, 22 insertions(+), 31 deletions(-) diff --git a/packages/contracts-rfq/contracts/FastBridgeV2.sol b/packages/contracts-rfq/contracts/FastBridgeV2.sol index cf12ce4215..5520d3710a 100644 --- a/packages/contracts-rfq/contracts/FastBridgeV2.sol +++ b/packages/contracts-rfq/contracts/FastBridgeV2.sol @@ -39,8 +39,7 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { } function bridgeProofs(bytes32 transactionId) public view returns (uint96 timestamp, address relayer) { - BridgeTxDetails storage _thisBridgeTxDetails = bridgeTxDetails[transactionId]; - return (uint96(_thisBridgeTxDetails.proofBlockTimestamp), _thisBridgeTxDetails.proofRelayer); + return (uint96(bridgeTxDetails[transactionId].proofBlockTimestamp), bridgeTxDetails[transactionId].proofRelayer); } constructor(address _owner) Admin(_owner) { @@ -194,11 +193,10 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { function prove(bytes32 transactionId, bytes32 destTxHash, address relayer) public onlyRole(RELAYER_ROLE) { // update bridge tx status given proof provided if (bridgeTxDetails[transactionId].status != BridgeStatus.REQUESTED) revert StatusIncorrect(); - BridgeTxDetails storage _thisBridgeTxDetails = bridgeTxDetails[transactionId]; - _thisBridgeTxDetails.status = BridgeStatus.RELAYER_PROVED; - _thisBridgeTxDetails.proofBlockTimestamp = uint40(block.timestamp); - _thisBridgeTxDetails.proofBlockNumber = uint48(block.number); - _thisBridgeTxDetails.proofRelayer = relayer; + bridgeTxDetails[transactionId].status = BridgeStatus.RELAYER_PROVED; + bridgeTxDetails[transactionId].proofBlockTimestamp = uint40(block.timestamp); + bridgeTxDetails[transactionId].proofBlockNumber = uint48(block.number); + bridgeTxDetails[transactionId].proofRelayer = relayer; emit BridgeProofProvided(transactionId, relayer, destTxHash); } @@ -217,10 +215,9 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { /// @inheritdoc IFastBridge function canClaim(bytes32 transactionId, address relayer) external view returns (bool) { - BridgeTxDetails storage _thisBridgeTxDetails = bridgeTxDetails[transactionId]; - if (_thisBridgeTxDetails.status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); - if (_thisBridgeTxDetails.proofRelayer != relayer) revert SenderIncorrect(); - return _timeSince(_thisBridgeTxDetails.proofBlockTimestamp) > DISPUTE_PERIOD; + if (bridgeTxDetails[transactionId].status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); + if (bridgeTxDetails[transactionId].proofRelayer != relayer) revert SenderIncorrect(); + return _timeSince(bridgeTxDetails[transactionId].proofBlockTimestamp) > DISPUTE_PERIOD; } /// @inheritdoc IFastBridgeV2 @@ -233,21 +230,19 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { bytes32 transactionId = keccak256(request); BridgeTransaction memory transaction = getBridgeTransaction(request); - BridgeTxDetails storage _thisBridgeTxDetails = bridgeTxDetails[transactionId]; - // update bridge tx status if able to claim origin collateral - if (_thisBridgeTxDetails.status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); + if (bridgeTxDetails[transactionId].status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); // if "to" is zero addr, permissionlessly send funds to proven relayer if (to == address(0)) { - to = _thisBridgeTxDetails.proofRelayer; - } else if (_thisBridgeTxDetails.proofRelayer != msg.sender) { + to = bridgeTxDetails[transactionId].proofRelayer; + } else if (bridgeTxDetails[transactionId].proofRelayer != msg.sender) { revert SenderIncorrect(); } - if (_timeSince(_thisBridgeTxDetails.proofBlockTimestamp) <= DISPUTE_PERIOD) revert DisputePeriodNotPassed(); + if (_timeSince(bridgeTxDetails[transactionId].proofBlockTimestamp) <= DISPUTE_PERIOD) revert DisputePeriodNotPassed(); - _thisBridgeTxDetails.status = BridgeStatus.RELAYER_CLAIMED; + bridgeTxDetails[transactionId].status = BridgeStatus.RELAYER_CLAIMED; // update protocol fees if origin fee amount exists if (transaction.originFeeAmount > 0) protocolFees[transaction.originToken] += transaction.originFeeAmount; @@ -257,21 +252,19 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { uint256 amount = transaction.originAmount; token.universalTransfer(to, amount); - emit BridgeDepositClaimed(transactionId, _thisBridgeTxDetails.proofRelayer, to, token, amount); + emit BridgeDepositClaimed(transactionId, bridgeTxDetails[transactionId].proofRelayer, to, token, amount); } /// @inheritdoc IFastBridge function dispute(bytes32 transactionId) external onlyRole(GUARD_ROLE) { - BridgeTxDetails storage _thisBridgeTxDetails = bridgeTxDetails[transactionId]; - - if (_thisBridgeTxDetails.status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); - if (_timeSince(_thisBridgeTxDetails.proofBlockTimestamp) > DISPUTE_PERIOD) revert DisputePeriodPassed(); + if (bridgeTxDetails[transactionId].status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); + if (_timeSince(bridgeTxDetails[transactionId].proofBlockTimestamp) > DISPUTE_PERIOD) revert DisputePeriodPassed(); // @dev relayer gets slashed effectively if dest relay has gone thru - _thisBridgeTxDetails.status = BridgeStatus.REQUESTED; - _thisBridgeTxDetails.proofRelayer = address(0); - _thisBridgeTxDetails.proofBlockTimestamp = 0; - _thisBridgeTxDetails.proofBlockNumber = 0; + bridgeTxDetails[transactionId].status = BridgeStatus.REQUESTED; + bridgeTxDetails[transactionId].proofRelayer = address(0); + bridgeTxDetails[transactionId].proofBlockTimestamp = 0; + bridgeTxDetails[transactionId].proofBlockNumber = 0; emit BridgeProofDisputed(transactionId, msg.sender); } @@ -282,9 +275,7 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { BridgeTransaction memory transaction = getBridgeTransaction(request); - BridgeTxDetails storage _thisBridgeTxDetails = bridgeTxDetails[transactionId]; - - if (_thisBridgeTxDetails.status != BridgeStatus.REQUESTED) revert StatusIncorrect(); + if (bridgeTxDetails[transactionId].status != BridgeStatus.REQUESTED) revert StatusIncorrect(); if (hasRole(REFUNDER_ROLE, msg.sender)) { // Refunder can refund if deadline has passed @@ -295,7 +286,7 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { } // if all checks passed, set to REFUNDED status - _thisBridgeTxDetails.status = BridgeStatus.REFUNDED; + bridgeTxDetails[transactionId].status = BridgeStatus.REFUNDED; // transfer origin collateral back to original sender address to = transaction.originSender; From 2f64c9422fc414f042d28294a0461fade6a2934f Mon Sep 17 00:00:00 2001 From: jw Date: Fri, 27 Sep 2024 13:29:32 -0400 Subject: [PATCH 09/11] rearrange for lint (120 char line) --- packages/contracts-rfq/contracts/FastBridgeV2.sol | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/contracts-rfq/contracts/FastBridgeV2.sol b/packages/contracts-rfq/contracts/FastBridgeV2.sol index 5520d3710a..3036650b62 100644 --- a/packages/contracts-rfq/contracts/FastBridgeV2.sol +++ b/packages/contracts-rfq/contracts/FastBridgeV2.sol @@ -39,7 +39,9 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { } function bridgeProofs(bytes32 transactionId) public view returns (uint96 timestamp, address relayer) { - return (uint96(bridgeTxDetails[transactionId].proofBlockTimestamp), bridgeTxDetails[transactionId].proofRelayer); + uint96 proofBlockTimestamp = bridgeTxDetails[transactionId].proofBlockTimestamp; + address proofRelayer = bridgeTxDetails[transactionId].proofRelayer; + return (proofBlockTimestamp, proofRelayer); } constructor(address _owner) Admin(_owner) { @@ -240,7 +242,9 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { revert SenderIncorrect(); } - if (_timeSince(bridgeTxDetails[transactionId].proofBlockTimestamp) <= DISPUTE_PERIOD) revert DisputePeriodNotPassed(); + if (_timeSince(bridgeTxDetails[transactionId].proofBlockTimestamp) <= DISPUTE_PERIOD) { + revert DisputePeriodNotPassed(); + } bridgeTxDetails[transactionId].status = BridgeStatus.RELAYER_CLAIMED; @@ -258,7 +262,9 @@ 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) revert DisputePeriodPassed(); + if (_timeSince(bridgeTxDetails[transactionId].proofBlockTimestamp) > DISPUTE_PERIOD) { + revert DisputePeriodPassed(); + } // @dev relayer gets slashed effectively if dest relay has gone thru bridgeTxDetails[transactionId].status = BridgeStatus.REQUESTED; From fbd7043db3d78ca9f0a36cbbd207cf9b01f2b9e6 Mon Sep 17 00:00:00 2001 From: parodime Date: Fri, 27 Sep 2024 14:15:35 -0400 Subject: [PATCH 10/11] test fixes --- .../test/FastBridgeV2.GasBench.Src.t.sol | 47 ++++++++++--------- .../contracts-rfq/test/FastBridgeV2.Src.t.sol | 7 +-- .../contracts-rfq/test/FastBridgeV2.t.sol | 1 + 3 files changed, 27 insertions(+), 28 deletions(-) diff --git a/packages/contracts-rfq/test/FastBridgeV2.GasBench.Src.t.sol b/packages/contracts-rfq/test/FastBridgeV2.GasBench.Src.t.sol index 424fdd5e2e..42c0cbf7e2 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.GasBench.Src.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.GasBench.Src.t.sol @@ -1,7 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -import {FastBridgeV2, FastBridgeV2SrcBaseTest, IFastBridge} from "./FastBridgeV2.Src.Base.t.sol"; +import {FastBridgeV2SrcBaseTest, IFastBridge} from "./FastBridgeV2.Src.Base.t.sol"; +import {IFastBridgeV2} from "../contracts/interfaces/IFastBridgeV2.sol"; // solhint-disable func-name-mixedcase, ordering /// @notice This test is used to estimate the gas cost of FastBridgeV2 source chain functions. @@ -63,12 +64,12 @@ contract FastBridgeV2GasBenchmarkSrcTest is FastBridgeV2SrcBaseTest { prove({caller: relayerA, bridgeTx: provenTokenTx, destTxHash: hex"01"}); prove({caller: relayerB, transactionId: getTxId(provenEthTx), destTxHash: hex"02", relayer: relayerA}); // Status checks - assertEq(fastBridge.bridgeStatuses(getTxId(bridgedTokenTx)), FastBridgeV2.BridgeStatus.REQUESTED); - assertEq(fastBridge.bridgeStatuses(getTxId(bridgedEthTx)), FastBridgeV2.BridgeStatus.REQUESTED); - assertEq(fastBridge.bridgeStatuses(getTxId(provenTokenTx)), FastBridgeV2.BridgeStatus.RELAYER_PROVED); - assertEq(fastBridge.bridgeStatuses(getTxId(provenEthTx)), FastBridgeV2.BridgeStatus.RELAYER_PROVED); - assertEq(fastBridge.bridgeStatuses(getTxId(tokenTx)), FastBridgeV2.BridgeStatus.NULL); - assertEq(fastBridge.bridgeStatuses(getTxId(ethTx)), FastBridgeV2.BridgeStatus.NULL); + assertEq(fastBridge.bridgeStatuses(getTxId(bridgedTokenTx)), IFastBridgeV2.BridgeStatus.REQUESTED); + assertEq(fastBridge.bridgeStatuses(getTxId(bridgedEthTx)), IFastBridgeV2.BridgeStatus.REQUESTED); + assertEq(fastBridge.bridgeStatuses(getTxId(provenTokenTx)), IFastBridgeV2.BridgeStatus.RELAYER_PROVED); + assertEq(fastBridge.bridgeStatuses(getTxId(provenEthTx)), IFastBridgeV2.BridgeStatus.RELAYER_PROVED); + assertEq(fastBridge.bridgeStatuses(getTxId(tokenTx)), IFastBridgeV2.BridgeStatus.NULL); + assertEq(fastBridge.bridgeStatuses(getTxId(ethTx)), IFastBridgeV2.BridgeStatus.NULL); } function skipBlocksExactly(uint256 blocks) public { @@ -86,7 +87,7 @@ contract FastBridgeV2GasBenchmarkSrcTest is FastBridgeV2SrcBaseTest { function test_bridge_token() public { bridge({caller: userA, msgValue: 0, params: tokenParams}); - assertEq(fastBridge.bridgeStatuses(getTxId(tokenTx)), FastBridgeV2.BridgeStatus.REQUESTED); + assertEq(fastBridge.bridgeStatuses(getTxId(tokenTx)), IFastBridgeV2.BridgeStatus.REQUESTED); assertEq(srcToken.balanceOf(userA), initialUserBalanceToken - tokenParams.originAmount); assertEq(srcToken.balanceOf(address(fastBridge)), initialFastBridgeBalanceToken + tokenParams.originAmount); } @@ -94,7 +95,7 @@ contract FastBridgeV2GasBenchmarkSrcTest is FastBridgeV2SrcBaseTest { function test_prove_token() public { bytes32 txId = getTxId(bridgedTokenTx); prove({caller: relayerA, bridgeTx: bridgedTokenTx, destTxHash: hex"03"}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.RELAYER_PROVED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.RELAYER_PROVED); (uint96 timestamp, address relayer) = fastBridge.bridgeProofs(txId); assertEq(timestamp, block.timestamp); assertEq(relayer, relayerA); @@ -104,7 +105,7 @@ contract FastBridgeV2GasBenchmarkSrcTest is FastBridgeV2SrcBaseTest { function test_proveWithAddress_token() public { bytes32 txId = getTxId(bridgedTokenTx); prove({caller: relayerB, transactionId: txId, destTxHash: hex"03", relayer: relayerA}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.RELAYER_PROVED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.RELAYER_PROVED); (uint96 timestamp, address relayer) = fastBridge.bridgeProofs(txId); assertEq(timestamp, block.timestamp); assertEq(relayer, relayerA); @@ -113,7 +114,7 @@ contract FastBridgeV2GasBenchmarkSrcTest is FastBridgeV2SrcBaseTest { function test_claim_token() public { skipTimeAtLeast({time: CLAIM_DELAY + 1}); claim({caller: relayerA, bridgeTx: provenTokenTx}); - assertEq(fastBridge.bridgeStatuses(getTxId(provenTokenTx)), FastBridgeV2.BridgeStatus.RELAYER_CLAIMED); + assertEq(fastBridge.bridgeStatuses(getTxId(provenTokenTx)), IFastBridgeV2.BridgeStatus.RELAYER_CLAIMED); assertEq(srcToken.balanceOf(relayerA), INITIAL_RELAYER_BALANCE + tokenTx.originAmount); assertEq(srcToken.balanceOf(address(fastBridge)), initialFastBridgeBalanceToken - tokenTx.originAmount); } @@ -121,7 +122,7 @@ contract FastBridgeV2GasBenchmarkSrcTest is FastBridgeV2SrcBaseTest { function test_claimWithAddress_token() public { skipTimeAtLeast({time: CLAIM_DELAY + 1}); claim({caller: relayerA, bridgeTx: provenTokenTx, to: relayerB}); - assertEq(fastBridge.bridgeStatuses(getTxId(provenTokenTx)), FastBridgeV2.BridgeStatus.RELAYER_CLAIMED); + assertEq(fastBridge.bridgeStatuses(getTxId(provenTokenTx)), IFastBridgeV2.BridgeStatus.RELAYER_CLAIMED); assertEq(srcToken.balanceOf(relayerB), INITIAL_RELAYER_BALANCE + tokenTx.originAmount); assertEq(srcToken.balanceOf(address(fastBridge)), initialFastBridgeBalanceToken - tokenTx.originAmount); } @@ -129,7 +130,7 @@ contract FastBridgeV2GasBenchmarkSrcTest is FastBridgeV2SrcBaseTest { function test_dispute_token() public { bytes32 txId = getTxId(provenTokenTx); dispute({caller: guard, txId: txId}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REQUESTED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REQUESTED); assertEq(srcToken.balanceOf(address(fastBridge)), initialFastBridgeBalanceToken); } @@ -137,7 +138,7 @@ contract FastBridgeV2GasBenchmarkSrcTest is FastBridgeV2SrcBaseTest { bytes32 txId = getTxId(bridgedTokenTx); skipTimeAtLeast({time: DEADLINE}); refund({caller: refunder, bridgeTx: bridgedTokenTx}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REFUNDED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REFUNDED); assertEq(srcToken.balanceOf(userA), initialUserBalanceToken + tokenParams.originAmount); assertEq(srcToken.balanceOf(address(fastBridge)), initialFastBridgeBalanceToken - tokenParams.originAmount); } @@ -146,7 +147,7 @@ contract FastBridgeV2GasBenchmarkSrcTest is FastBridgeV2SrcBaseTest { bytes32 txId = getTxId(bridgedTokenTx); skipTimeAtLeast({time: DEADLINE + PERMISSIONLESS_REFUND_DELAY}); refund({caller: userB, bridgeTx: bridgedTokenTx}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REFUNDED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REFUNDED); assertEq(srcToken.balanceOf(userA), initialUserBalanceToken + tokenParams.originAmount); assertEq(srcToken.balanceOf(address(fastBridge)), initialFastBridgeBalanceToken - tokenParams.originAmount); } @@ -155,7 +156,7 @@ contract FastBridgeV2GasBenchmarkSrcTest is FastBridgeV2SrcBaseTest { function test_bridge_eth() public { bridge({caller: userA, msgValue: ethParams.originAmount, params: ethParams}); - assertEq(fastBridge.bridgeStatuses(getTxId(ethTx)), FastBridgeV2.BridgeStatus.REQUESTED); + assertEq(fastBridge.bridgeStatuses(getTxId(ethTx)), IFastBridgeV2.BridgeStatus.REQUESTED); assertEq(userA.balance, initialUserBalanceEth - ethParams.originAmount); assertEq(address(fastBridge).balance, initialFastBridgeBalanceEth + ethParams.originAmount); } @@ -163,7 +164,7 @@ contract FastBridgeV2GasBenchmarkSrcTest is FastBridgeV2SrcBaseTest { function test_prove_eth() public { bytes32 txId = getTxId(bridgedEthTx); prove({caller: relayerA, bridgeTx: bridgedEthTx, destTxHash: hex"03"}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.RELAYER_PROVED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.RELAYER_PROVED); (uint96 timestamp, address relayer) = fastBridge.bridgeProofs(txId); assertEq(timestamp, block.timestamp); assertEq(relayer, relayerA); @@ -173,7 +174,7 @@ contract FastBridgeV2GasBenchmarkSrcTest is FastBridgeV2SrcBaseTest { function test_proveWithAddress_eth() public { bytes32 txId = getTxId(bridgedEthTx); prove({caller: relayerB, transactionId: txId, destTxHash: hex"03", relayer: relayerA}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.RELAYER_PROVED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.RELAYER_PROVED); (uint96 timestamp, address relayer) = fastBridge.bridgeProofs(txId); assertEq(timestamp, block.timestamp); assertEq(relayer, relayerA); @@ -183,7 +184,7 @@ contract FastBridgeV2GasBenchmarkSrcTest is FastBridgeV2SrcBaseTest { function test_claim_eth() public { skipTimeAtLeast({time: CLAIM_DELAY + 1}); claim({caller: relayerA, bridgeTx: provenEthTx}); - assertEq(fastBridge.bridgeStatuses(getTxId(provenEthTx)), FastBridgeV2.BridgeStatus.RELAYER_CLAIMED); + assertEq(fastBridge.bridgeStatuses(getTxId(provenEthTx)), IFastBridgeV2.BridgeStatus.RELAYER_CLAIMED); assertEq(relayerA.balance, INITIAL_RELAYER_BALANCE + ethTx.originAmount); assertEq(address(fastBridge).balance, initialFastBridgeBalanceEth - ethTx.originAmount); } @@ -191,7 +192,7 @@ contract FastBridgeV2GasBenchmarkSrcTest is FastBridgeV2SrcBaseTest { function test_claimWithAddress_eth() public { skipTimeAtLeast({time: CLAIM_DELAY + 1}); claim({caller: relayerA, bridgeTx: provenEthTx, to: relayerB}); - assertEq(fastBridge.bridgeStatuses(getTxId(provenEthTx)), FastBridgeV2.BridgeStatus.RELAYER_CLAIMED); + assertEq(fastBridge.bridgeStatuses(getTxId(provenEthTx)), IFastBridgeV2.BridgeStatus.RELAYER_CLAIMED); assertEq(relayerB.balance, INITIAL_RELAYER_BALANCE + ethTx.originAmount); assertEq(address(fastBridge).balance, initialFastBridgeBalanceEth - ethTx.originAmount); } @@ -199,7 +200,7 @@ contract FastBridgeV2GasBenchmarkSrcTest is FastBridgeV2SrcBaseTest { function test_dispute_eth() public { bytes32 txId = getTxId(provenEthTx); dispute({caller: guard, txId: txId}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REQUESTED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REQUESTED); assertEq(address(fastBridge).balance, initialFastBridgeBalanceEth); } @@ -207,7 +208,7 @@ contract FastBridgeV2GasBenchmarkSrcTest is FastBridgeV2SrcBaseTest { bytes32 txId = getTxId(bridgedEthTx); skipTimeAtLeast({time: DEADLINE}); refund({caller: refunder, bridgeTx: bridgedEthTx}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REFUNDED); + assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REFUNDED); assertEq(userA.balance, initialUserBalanceEth + ethParams.originAmount); assertEq(address(fastBridge).balance, initialFastBridgeBalanceEth - ethParams.originAmount); } @@ -216,7 +217,7 @@ contract FastBridgeV2GasBenchmarkSrcTest is FastBridgeV2SrcBaseTest { bytes32 txId = getTxId(bridgedEthTx); skipTimeAtLeast({time: DEADLINE + PERMISSIONLESS_REFUND_DELAY}); refund({caller: userB, bridgeTx: bridgedEthTx}); - assertEq(fastBridge.bridgeStatuses(txId), FastBridgeV2.BridgeStatus.REFUNDED); + 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.Src.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol index 322b8589d6..086f9906aa 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol @@ -14,7 +14,8 @@ import { ZeroAddress } from "../contracts/libs/Errors.sol"; -import {FastBridgeV2, FastBridgeV2Test, IFastBridge, IFastBridgeV2} from "./FastBridgeV2.t.sol"; +import {FastBridgeV2SrcBaseTest} from "./FastBridgeV2.Src.Base.t.sol"; +import {IFastBridge, IFastBridgeV2} from "./FastBridgeV2.t.sol"; // solhint-disable func-name-mixedcase, ordering contract FastBridgeV2SrcTest is FastBridgeV2SrcBaseTest { @@ -96,10 +97,6 @@ contract FastBridgeV2SrcTest is FastBridgeV2SrcBaseTest { }); } - function assertEq(IFastBridgeV2.BridgeStatus a, IFastBridgeV2.BridgeStatus b) public pure { - assertEq(uint8(a), uint8(b)); - } - // ══════════════════════════════════════════════════ BRIDGE ═══════════════════════════════════════════════════════ function checkTokenBalancesAfterBridge(address caller) public view { diff --git a/packages/contracts-rfq/test/FastBridgeV2.t.sol b/packages/contracts-rfq/test/FastBridgeV2.t.sol index b61d18f45a..80c1354fa8 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.t.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.20; import {IFastBridge} from "../contracts/interfaces/IFastBridge.sol"; import {IFastBridgeV2} from "../contracts/interfaces/IFastBridgeV2.sol"; + import {IFastBridgeV2Errors} from "../contracts/interfaces/IFastBridgeV2Errors.sol"; import {FastBridgeV2} from "../contracts/FastBridgeV2.sol"; From 78c84a8fcb74561555f43dc2d354bbb711f22535 Mon Sep 17 00:00:00 2001 From: parodime Date: Fri, 27 Sep 2024 15:40:06 -0400 Subject: [PATCH 11/11] code cleanup --- packages/contracts-rfq/contracts/FastBridgeV2.sol | 5 ++--- packages/contracts-rfq/test/FastBridgeV2.Src.t.sol | 13 ------------- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/packages/contracts-rfq/contracts/FastBridgeV2.sol b/packages/contracts-rfq/contracts/FastBridgeV2.sol index 3036650b62..e649ab0ec0 100644 --- a/packages/contracts-rfq/contracts/FastBridgeV2.sol +++ b/packages/contracts-rfq/contracts/FastBridgeV2.sol @@ -39,9 +39,8 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { } function bridgeProofs(bytes32 transactionId) public view returns (uint96 timestamp, address relayer) { - uint96 proofBlockTimestamp = bridgeTxDetails[transactionId].proofBlockTimestamp; - address proofRelayer = bridgeTxDetails[transactionId].proofRelayer; - return (proofBlockTimestamp, proofRelayer); + timestamp = bridgeTxDetails[transactionId].proofBlockTimestamp; + relayer = bridgeTxDetails[transactionId].proofRelayer; } constructor(address _owner) Admin(_owner) { diff --git a/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol index 086f9906aa..e0eb6d6ca3 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol @@ -1,19 +1,6 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.20; -import { - AmountIncorrect, - ChainIncorrect, - DisputePeriodNotPassed, - DisputePeriodPassed, - DeadlineNotExceeded, - DeadlineTooShort, - MsgValueIncorrect, - SenderIncorrect, - StatusIncorrect, - ZeroAddress -} from "../contracts/libs/Errors.sol"; - import {FastBridgeV2SrcBaseTest} from "./FastBridgeV2.Src.Base.t.sol"; import {IFastBridge, IFastBridgeV2} from "./FastBridgeV2.t.sol";