From 6a913eb807985fe9c83b3069c17556f17df65683 Mon Sep 17 00:00:00 2001 From: parodime Date: Wed, 2 Oct 2024 12:07:38 -0400 Subject: [PATCH 1/4] sender nonces [SLT-183] --- .../contracts-rfq/contracts/FastBridgeV2.sol | 13 +++++++++---- .../test/FastBridgeV2.Src.Base.t.sol | 5 +++++ .../contracts-rfq/test/FastBridgeV2.Src.t.sol | 18 +++++++++++++++++- 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/packages/contracts-rfq/contracts/FastBridgeV2.sol b/packages/contracts-rfq/contracts/FastBridgeV2.sol index 67221da8d8..3402b2a4f3 100644 --- a/packages/contracts-rfq/contracts/FastBridgeV2.sol +++ b/packages/contracts-rfq/contracts/FastBridgeV2.sol @@ -28,9 +28,8 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { mapping(bytes32 => BridgeTxDetails) public bridgeTxDetails; /// @notice Relay details on destination chain mapping(bytes32 => BridgeRelay) public bridgeRelayDetails; - - /// @dev to prevent replays - uint256 public nonce; + /// @notice Unique bridge nonces tracked per originSender + mapping(address => uint256) public senderNonces; // @dev the block the contract was deployed at uint256 public immutable deployBlock; @@ -113,6 +112,12 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { return _timeSince(bridgeTxDetails[transactionId].proofBlockTimestamp) > DISPUTE_PERIOD; } + /// @notice This function is deprecated and should not be used. + /// @dev Replaced by senderNonces + function nonce() external pure returns (uint256) { + return 0; + } + /// @inheritdoc IFastBridge function getBridgeTransaction(bytes memory request) external pure returns (BridgeTransaction memory) { // Note: when passing V2 request, this will decode the V1 fields correctly since the new fields were @@ -158,7 +163,7 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { originFeeAmount: originFeeAmount, sendChainGas: params.sendChainGas, deadline: params.deadline, - nonce: nonce++, // increment nonce on every bridge + nonce: senderNonces[params.sender]++, // increment nonce on every bridge exclusivityRelayer: paramsV2.quoteRelayer, // We checked exclusivityEndTime to be in range (0 .. params.deadline] above, so can safely cast exclusivityEndTime: uint256(exclusivityEndTime) diff --git a/packages/contracts-rfq/test/FastBridgeV2.Src.Base.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Src.Base.t.sol index 9a45037d54..3347f9e5fa 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.Src.Base.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.Src.Base.t.sol @@ -95,6 +95,11 @@ abstract contract FastBridgeV2SrcBaseTest is FastBridgeV2Test { fastBridge.refund(abi.encode(bridgeTx)); } + function test_nonce() public view { + // deprecated. should always return zero in FbV2. + assertEq(fastBridge.nonce(), 0); + } + function assertEq(FastBridgeV2.BridgeStatus a, FastBridgeV2.BridgeStatus b) public pure { assertEq(uint8(a), uint8(b)); } diff --git a/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol index dd01387d72..c2a5902133 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.Src.t.sol @@ -103,6 +103,8 @@ contract FastBridgeV2SrcTest is FastBridgeV2SrcBaseTest { expectBridgeRequested(tokenTx, txId); expectBridgeQuoteDetails(txId, tokenParamsV2.quoteId); bridge({caller: userA, msgValue: 0, params: tokenParams}); + assertEq(fastBridge.senderNonces(userA), 1); + assertEq(fastBridge.senderNonces(userB), 0); assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REQUESTED); checkTokenBalancesAfterBridge(userA); } @@ -112,6 +114,8 @@ contract FastBridgeV2SrcTest is FastBridgeV2SrcBaseTest { expectBridgeRequested(tokenTx, txId); expectBridgeQuoteDetails(txId, tokenParamsV2.quoteId); bridge({caller: userB, msgValue: 0, params: tokenParams}); + assertEq(fastBridge.senderNonces(userA), 1); + assertEq(fastBridge.senderNonces(userB), 0); assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REQUESTED); assertEq(srcToken.balanceOf(userA), LEFTOVER_BALANCE + tokenParams.originAmount); checkTokenBalancesAfterBridge(userB); @@ -126,10 +130,14 @@ contract FastBridgeV2SrcTest is FastBridgeV2SrcBaseTest { function test_bridge_eth() public { // bridge token first to match the nonce bridge({caller: userA, msgValue: 0, params: tokenParams}); + assertEq(fastBridge.senderNonces(userA), 1); + assertEq(fastBridge.senderNonces(userB), 0); bytes32 txId = getTxId(ethTx); expectBridgeRequested(ethTx, txId); expectBridgeQuoteDetails(txId, ethParamsV2.quoteId); bridge({caller: userA, msgValue: ethParams.originAmount, params: ethParams}); + assertEq(fastBridge.senderNonces(userA), 2); + assertEq(fastBridge.senderNonces(userB), 0); assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REQUESTED); checkEthBalancesAfterBridge(userA); } @@ -137,18 +145,24 @@ contract FastBridgeV2SrcTest is FastBridgeV2SrcBaseTest { function test_bridge_eth_diffSender() public { // bridge token first to match the nonce bridge({caller: userA, msgValue: 0, params: tokenParams}); + assertEq(fastBridge.senderNonces(userA), 1); + assertEq(fastBridge.senderNonces(userB), 0); bytes32 txId = getTxId(ethTx); expectBridgeRequested(ethTx, txId); expectBridgeQuoteDetails(txId, ethParamsV2.quoteId); + // bridge for user A as sender, called by userB bridge({caller: userB, msgValue: ethParams.originAmount, params: ethParams}); + assertEq(fastBridge.senderNonces(userA), 2); + assertEq(fastBridge.senderNonces(userB), 0); assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REQUESTED); assertEq(userA.balance, LEFTOVER_BALANCE + ethParams.originAmount); checkEthBalancesAfterBridge(userB); } function test_bridge_userSpecificNonce() public { - vm.skip(true); // TODO: unskip when implemented bridge({caller: userA, msgValue: 0, params: tokenParams}); + assertEq(fastBridge.senderNonces(userA), 1); + assertEq(fastBridge.senderNonces(userB), 0); // UserB nonce is 0 ethTx.nonce = 0; ethParams.sender = userB; @@ -157,6 +171,8 @@ contract FastBridgeV2SrcTest is FastBridgeV2SrcBaseTest { expectBridgeRequested(ethTx, txId); expectBridgeQuoteDetails(txId, ethParamsV2.quoteId); bridge({caller: userB, msgValue: ethParams.originAmount, params: ethParams}); + assertEq(fastBridge.senderNonces(userA), 1); + assertEq(fastBridge.senderNonces(userB), 1); assertEq(fastBridge.bridgeStatuses(txId), IFastBridgeV2.BridgeStatus.REQUESTED); checkEthBalancesAfterBridge(userB); } From 15ca0deffc95535b5590c3d8c16f9a9fb1510b20 Mon Sep 17 00:00:00 2001 From: parodime Date: Wed, 2 Oct 2024 13:04:16 -0400 Subject: [PATCH 2/4] refactor nonce test for codecov --- packages/contracts-rfq/test/FastBridgeV2.Src.Base.t.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/contracts-rfq/test/FastBridgeV2.Src.Base.t.sol b/packages/contracts-rfq/test/FastBridgeV2.Src.Base.t.sol index 3347f9e5fa..e63f292194 100644 --- a/packages/contracts-rfq/test/FastBridgeV2.Src.Base.t.sol +++ b/packages/contracts-rfq/test/FastBridgeV2.Src.Base.t.sol @@ -96,8 +96,9 @@ abstract contract FastBridgeV2SrcBaseTest is FastBridgeV2Test { } function test_nonce() public view { + uint256 result = fastBridge.nonce(); // deprecated. should always return zero in FbV2. - assertEq(fastBridge.nonce(), 0); + assertEq(result, 0); } function assertEq(FastBridgeV2.BridgeStatus a, FastBridgeV2.BridgeStatus b) public pure { From 52da9698eb9b294008e18726a7427e477d832730 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 2 Oct 2024 19:06:23 +0100 Subject: [PATCH 3/4] refactor: make `nonce` an immutable var --- packages/contracts-rfq/contracts/FastBridgeV2.sol | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/contracts-rfq/contracts/FastBridgeV2.sol b/packages/contracts-rfq/contracts/FastBridgeV2.sol index 3402b2a4f3..5a764a7c95 100644 --- a/packages/contracts-rfq/contracts/FastBridgeV2.sol +++ b/packages/contracts-rfq/contracts/FastBridgeV2.sol @@ -31,7 +31,10 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { /// @notice Unique bridge nonces tracked per originSender mapping(address => uint256) public senderNonces; - // @dev the block the contract was deployed at + /// @notice This is deprecated and should not be used. + /// @dev Replaced by senderNonces + uint256 public immutable nonce = 0; + /// @notice the block the contract was deployed at uint256 public immutable deployBlock; constructor(address _owner) Admin(_owner) { @@ -112,12 +115,6 @@ contract FastBridgeV2 is Admin, IFastBridgeV2, IFastBridgeV2Errors { return _timeSince(bridgeTxDetails[transactionId].proofBlockTimestamp) > DISPUTE_PERIOD; } - /// @notice This function is deprecated and should not be used. - /// @dev Replaced by senderNonces - function nonce() external pure returns (uint256) { - return 0; - } - /// @inheritdoc IFastBridge function getBridgeTransaction(bytes memory request) external pure returns (BridgeTransaction memory) { // Note: when passing V2 request, this will decode the V1 fields correctly since the new fields were From 0cb26c429de8b74219b2767aae40ad8c0d0c982f Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Wed, 2 Oct 2024 19:06:59 +0100 Subject: [PATCH 4/4] chore: ignore local coverage report --- packages/contracts-rfq/.gitignore | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/contracts-rfq/.gitignore b/packages/contracts-rfq/.gitignore index 5726149ffb..7c3b19de11 100644 --- a/packages/contracts-rfq/.gitignore +++ b/packages/contracts-rfq/.gitignore @@ -1,3 +1,5 @@ flattened .deployments -broadcast \ No newline at end of file +broadcast + +lcov.info \ No newline at end of file