From 2e6970aa453d601fc66c5155563a8bb7876e7098 Mon Sep 17 00:00:00 2001 From: shotaronowhere Date: Thu, 2 Feb 2023 09:49:48 +0000 Subject: [PATCH 1/2] fix: revert mistaken variable naming --- .../src/FastBridgeReceiverOnEthereum.sol | 14 +- contracts/src/FastBridgeReceiverOnGnosis.sol | 12 +- contracts/src/FastBridgeReceiverOnPolygon.sol | 12 +- contracts/src/FastBridgeSender.sol | 12 +- contracts/src/SafeBridgeRouterToGnosis.sol | 12 +- contracts/src/SafeBridgeRouterToPolygon.sol | 12 +- contracts/src/test/FastBridgeReceiverMock.sol | 328 +++++++++++++++++- contracts/test/integration/index.ts | 4 +- 8 files changed, 358 insertions(+), 48 deletions(-) diff --git a/contracts/src/FastBridgeReceiverOnEthereum.sol b/contracts/src/FastBridgeReceiverOnEthereum.sol index 26dbd1fd..74daeda4 100644 --- a/contracts/src/FastBridgeReceiverOnEthereum.sol +++ b/contracts/src/FastBridgeReceiverOnEthereum.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT /** - * @authors: [@jaybuidl, @shotaronowhere, @hrishibhat, @adi274] + * @authors: [@jaybuidl, @shotaronowhere, @hrishibhat] * @reviewers: [] * @auditors: [] * @bounties: [] @@ -36,9 +36,9 @@ contract FastBridgeReceiverOnEthereum is IFastBridgeReceiver, ISafeBridgeReceive // * Views * // // ************************************* // - function isSentBySafeBridge() internal view virtual override returns (bool) { + function isSentBySafeBridge() internal view override returns (bool) { IOutbox outbox = IOutbox(inbox.bridge().activeOutbox()); - return (msg.sender == address(outbox) && outbox.l2ToL1Sender() == fastBridgeSender); + return outbox.l2ToL1Sender() == safeBridgeSender; } /** @@ -46,20 +46,20 @@ contract FastBridgeReceiverOnEthereum is IFastBridgeReceiver, ISafeBridgeReceive * @param _deposit The deposit amount to submit a claim in wei. * @param _epochPeriod The duration of each epoch. * @param _challengePeriod The duration of the period allowing to challenge a claim. - * @param _fastBridgeSender The address of the Safe Bridge Sender on the connecting chain. + * @param _safeBridgeSender The address of the Safe Bridge Sender on the connecting chain. * @param _inbox Ethereum receiver specific: The address of the inbox contract on Ethereum. */ constructor( uint256 _deposit, uint256 _epochPeriod, uint256 _challengePeriod, - address _fastBridgeSender, + address _safeBridgeSender, address _inbox // Ethereum receiver specific ) { deposit = _deposit; epochPeriod = _epochPeriod; challengePeriod = _challengePeriod; - fastBridgeSender = _fastBridgeSender; + safeBridgeSender = _safeBridgeSender; inbox = IInbox(_inbox); // Ethereum receiver specific } @@ -95,7 +95,7 @@ contract FastBridgeReceiverOnEthereum is IFastBridgeReceiver, ISafeBridgeReceive uint256 public immutable deposit; // The deposit required to submit a claim or challenge uint256 public immutable override epochPeriod; // Epochs mark the period between potential batches of messages. uint256 public immutable override challengePeriod; // Epochs mark the period between potential batches of messages. - address public immutable fastBridgeSender; // The address of the Safe Bridge Sender on the connecting chain. + address public immutable safeBridgeSender; // The address of the Safe Bridge Sender on the connecting chain. mapping(uint256 => bytes32) public fastInbox; // epoch => validated batch merkle root(optimistically, or challenged and verified with the safe bridge) mapping(uint256 => Claim) public claims; // epoch => claim diff --git a/contracts/src/FastBridgeReceiverOnGnosis.sol b/contracts/src/FastBridgeReceiverOnGnosis.sol index 4ed63052..9602e1bd 100644 --- a/contracts/src/FastBridgeReceiverOnGnosis.sol +++ b/contracts/src/FastBridgeReceiverOnGnosis.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT /** - * @authors: [@jaybuidl, @shotaronowhere, @hrishibhat, @adi274] + * @authors: [@jaybuidl, @shotaronowhere, @hrishibhat] * @reviewers: [] * @auditors: [] * @bounties: [] @@ -36,7 +36,7 @@ contract FastBridgeReceiverOnGnosis is IFastBridgeReceiver, ISafeBridgeReceiver // ************************************* // function isSentBySafeBridge() internal view override returns (bool) { - return (msg.sender == address(amb)) && (amb.messageSender() == fastBridgeSender); + return (msg.sender == address(amb)) && (amb.messageSender() == safeBridgeSender); } /** @@ -44,20 +44,20 @@ contract FastBridgeReceiverOnGnosis is IFastBridgeReceiver, ISafeBridgeReceiver * @param _deposit The deposit amount to submit a claim in wei. * @param _epochPeriod The duration of each epoch. * @param _challengePeriod The duration of the period allowing to challenge a claim. - * @param _fastBridgeSender The address of the Safe Bridge Sender on the connecting chain. + * @param _safeBridgeSender The address of the Safe Bridge Sender on the connecting chain. * @param _amb The AMB contract on Gnosis Chain. */ constructor( uint256 _deposit, uint256 _epochPeriod, uint256 _challengePeriod, - address _fastBridgeSender, // Gnosis receiver specific + address _safeBridgeSender, // Gnosis receiver specific address _amb // Gnosis receiver specific ) { deposit = _deposit; epochPeriod = _epochPeriod; challengePeriod = _challengePeriod; - fastBridgeSender = _fastBridgeSender; + safeBridgeSender = _safeBridgeSender; amb = IAMB(_amb); // Gnosis receiver specific } @@ -93,7 +93,7 @@ contract FastBridgeReceiverOnGnosis is IFastBridgeReceiver, ISafeBridgeReceiver uint256 public immutable deposit; // The deposit required to submit a claim or challenge uint256 public immutable override epochPeriod; // Epochs mark the period between potential batches of messages. uint256 public immutable override challengePeriod; // Epochs mark the period between potential batches of messages. - address public immutable fastBridgeSender; // The address of the Safe Bridge Sender on the connecting chain. + address public immutable safeBridgeSender; // The address of the Safe Bridge Sender on the connecting chain. mapping(uint256 => bytes32) public fastInbox; // epoch => validated batch merkle root(optimistically, or challenged and verified with the safe bridge) mapping(uint256 => Claim) public claims; // epoch => claim diff --git a/contracts/src/FastBridgeReceiverOnPolygon.sol b/contracts/src/FastBridgeReceiverOnPolygon.sol index c0b77286..59bc8336 100644 --- a/contracts/src/FastBridgeReceiverOnPolygon.sol +++ b/contracts/src/FastBridgeReceiverOnPolygon.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT /** - * @authors: [@jaybuidl, @shotaronowhere, @hrishibhat, @adi274] + * @authors: [@jaybuidl, @shotaronowhere, @hrishibhat] * @reviewers: [] * @auditors: [] * @bounties: [] @@ -38,21 +38,21 @@ contract FastBridgeReceiverOnPolygon is FxBaseChildTunnel, IFastBridgeReceiver, * @param _deposit The deposit amount to submit a claim in wei. * @param _epochPeriod The duration of each epoch. * @param _challengePeriod The duration of the period allowing to challenge a claim. - * @param _fastBridgeSender The address of the Safe Bridge Sender on the connecting chain. fxRootTunnel contract in ethereum + * @param _safeBridgeSender The address of the Safe Bridge Sender on the connecting chain. fxRootTunnel contract in ethereum * @param _fxChild The the fxChild contract on Polygon Chain. */ constructor( uint256 _deposit, uint256 _epochPeriod, uint256 _challengePeriod, - address _fastBridgeSender, // Polygon receiver specific + address _safeBridgeSender, // Polygon receiver specific address _fxChild // Polygon receiver specific ) FxBaseChildTunnel(_fxChild) { deposit = _deposit; epochPeriod = _epochPeriod; challengePeriod = _challengePeriod; - fastBridgeSender = _fastBridgeSender; - setFxRootTunnel(_fastBridgeSender); + safeBridgeSender = _safeBridgeSender; + setFxRootTunnel(_safeBridgeSender); } // ************************************** // @@ -87,7 +87,7 @@ contract FastBridgeReceiverOnPolygon is FxBaseChildTunnel, IFastBridgeReceiver, uint256 public immutable deposit; // The deposit required to submit a claim or challenge uint256 public immutable override epochPeriod; // Epochs mark the period between potential batches of messages. uint256 public immutable override challengePeriod; // Epochs mark the period between potential batches of messages. - address public immutable fastBridgeSender; // The address of the Safe Bridge Sender on the connecting chain. + address public immutable safeBridgeSender; // The address of the Safe Bridge Sender on the connecting chain. mapping(uint256 => bytes32) public fastInbox; // epoch => validated batch merkle root(optimistically, or challenged and verified with the safe bridge) mapping(uint256 => Claim) public claims; // epoch => claim diff --git a/contracts/src/FastBridgeSender.sol b/contracts/src/FastBridgeSender.sol index 73bdbda0..b6475b4e 100644 --- a/contracts/src/FastBridgeSender.sol +++ b/contracts/src/FastBridgeSender.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT /** - * @authors: [@jaybuidl, @shotaronowhere, @adi274] + * @authors: [@jaybuidl, @shotaronowhere] * @reviewers: [] * @auditors: [] * @bounties: [] @@ -44,7 +44,7 @@ contract FastBridgeSender is IFastBridgeSender, ISafeBridgeSender { bytes4 methodSelector = ISafeBridgeReceiver.verifySafeBatch.selector; bytes memory safeMessageData = abi.encodeWithSelector(methodSelector, _epoch, batchMerkleRoot); - bytes32 ticketID = _sendSafe(fastBridgeReceiver, safeMessageData); + bytes32 ticketID = _sendSafe(safeBridgeReceiver, safeMessageData); emit SentSafe(_epoch, ticketID); } @@ -56,11 +56,11 @@ contract FastBridgeSender is IFastBridgeSender, ISafeBridgeSender { /** * @dev Constructor. * @param _epochPeriod The duration between epochs. - * @param _fastBridgeReceiver The the Safe Bridge Router on Ethereum to the receiving chain. + * @param _safeBridgeReceiver The the Safe Bridge Router on Ethereum to the receiving chain. */ - constructor(uint256 _epochPeriod, address _fastBridgeReceiver) { + constructor(uint256 _epochPeriod, address _safeBridgeReceiver) { epochPeriod = _epochPeriod; - fastBridgeReceiver = _fastBridgeReceiver; + safeBridgeReceiver = _safeBridgeReceiver; unchecked { currentBatchID = block.timestamp / _epochPeriod - 1; } @@ -79,7 +79,7 @@ contract FastBridgeSender is IFastBridgeSender, ISafeBridgeSender { uint256 public immutable epochPeriod; // Epochs mark the period between potential batches of messages. uint256 public currentBatchID; mapping(uint256 => bytes32) public fastOutbox; // epoch count => merkle root of batched messages - address public immutable fastBridgeReceiver; + address public immutable safeBridgeReceiver; // merkle tree representation of a batch of messages // supports 2^64 messages. diff --git a/contracts/src/SafeBridgeRouterToGnosis.sol b/contracts/src/SafeBridgeRouterToGnosis.sol index 90946342..0b04ca4f 100644 --- a/contracts/src/SafeBridgeRouterToGnosis.sol +++ b/contracts/src/SafeBridgeRouterToGnosis.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT /** - * @authors: [@shotaronowhere, @jaybuidl, @adi274] + * @authors: [@shotaronowhere, @jaybuidl] * @reviewers: [] * @auditors: [] * @bounties: [] @@ -25,25 +25,25 @@ contract SafeBridgeRouter is ISafeBridgeRouter { IInbox public immutable inbox; // The address of the Arbitrum Inbox contract. IAMB public immutable amb; // The address of the AMB contract on Ethereum. - address public immutable fastBridgeSender; // The address of the Fast Bridge sender on Arbitrum. + address public immutable safeBridgeSender; // The address of the Safe Bridge sender on Arbitrum. address public immutable fastBridgeReceiverOnGnosisChain; // The address of the Fast Bridge Receiver on Gnosis Chain. /** * @dev Constructor. * @param _inbox The address of the inbox contract on Ethereum. * @param _amb The address of the AMB contract on Ethereum. - * @param _fastBridgeSender The fast bridge sender on Arbitrum. + * @param _safeBridgeSender The safe bridge sender on Arbitrum. * @param _fastBridgeReceiverOnGnosisChain The fast bridge receiver on Gnosis Chain. */ constructor( IInbox _inbox, IAMB _amb, - address _fastBridgeSender, + address _safeBridgeSender, address _fastBridgeReceiverOnGnosisChain ) { inbox = _inbox; amb = _amb; - fastBridgeSender = _fastBridgeSender; + safeBridgeSender = _safeBridgeSender; fastBridgeReceiverOnGnosisChain = _fastBridgeReceiverOnGnosisChain; } @@ -74,6 +74,6 @@ contract SafeBridgeRouter is ISafeBridgeRouter { function isSentBySafeBridge() internal view override returns (bool) { IOutbox outbox = IOutbox(inbox.bridge().activeOutbox()); - return outbox.l2ToL1Sender() == fastBridgeSender; + return outbox.l2ToL1Sender() == safeBridgeSender; } } diff --git a/contracts/src/SafeBridgeRouterToPolygon.sol b/contracts/src/SafeBridgeRouterToPolygon.sol index 15deb005..56653e4b 100644 --- a/contracts/src/SafeBridgeRouterToPolygon.sol +++ b/contracts/src/SafeBridgeRouterToPolygon.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT /** - * @authors: [@shotaronowhere, @hrishibhat, @jaybuidl, @adi274] + * @authors: [@shotaronowhere, @hrishibhat, @jaybuidl] * @reviewers: [] * @auditors: [] * @bounties: [] @@ -24,24 +24,24 @@ contract SafeBridgeRouterToPolygon is ISafeBridgeRouter, FxBaseRootTunnel { // ************************************* // IInbox public immutable inbox; // The address of the Arbitrum Inbox contract. - address public immutable fastBridgeSender; // The address of the Safe Bridge sender on Arbitrum. + address public immutable safeBridgeSender; // The address of the Safe Bridge sender on Arbitrum. /** * @dev Constructor. * @param _inbox The address of the inbox contract on Ethereum. * @param _fxRoot The address of the fxRoot contract in Ethereum. - * @param _fastBridgeSender The safe bridge sender on Arbitrum. + * @param _safeBridgeSender The safe bridge sender on Arbitrum. * @param _fastBridgeReceiverOnPolygon The fast bridge receiver on Polygon Chain. */ constructor( IInbox _inbox, address _checkpointManager, address _fxRoot, - address _fastBridgeSender, + address _safeBridgeSender, address _fastBridgeReceiverOnPolygon ) FxBaseRootTunnel(_checkpointManager, _fxRoot) { inbox = _inbox; - fastBridgeSender = _fastBridgeSender; + safeBridgeSender = _safeBridgeSender; setFxChildTunnel(_fastBridgeReceiverOnPolygon); } @@ -80,6 +80,6 @@ contract SafeBridgeRouterToPolygon is ISafeBridgeRouter, FxBaseRootTunnel { function isSentBySafeBridge() internal view override returns (bool) { IOutbox outbox = IOutbox(inbox.bridge().activeOutbox()); - return outbox.l2ToL1Sender() == fastBridgeSender; + return outbox.l2ToL1Sender() == safeBridgeSender; } } diff --git a/contracts/src/test/FastBridgeReceiverMock.sol b/contracts/src/test/FastBridgeReceiverMock.sol index 9c981c47..553f4d4a 100644 --- a/contracts/src/test/FastBridgeReceiverMock.sol +++ b/contracts/src/test/FastBridgeReceiverMock.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT /** - * @authors: [@adi274] + * @authors: [@jaybuidl, @shotaronowhere, @hrishibhat] * @reviewers: [] * @auditors: [] * @bounties: [] @@ -10,26 +10,336 @@ pragma solidity ^0.8.0; -import "../FastBridgeReceiverOnEthereum.sol"; +import "../interfaces/IFastBridgeReceiver.sol"; +import "../interfaces/ISafeBridgeReceiver.sol"; +import "../canonical/arbitrum/IInbox.sol"; +import "../canonical/arbitrum/IOutbox.sol"; /** - * Fast Bridge Receiver - * Counterpart of `Fast Sender` + * Fast Receiver On Ethereum + * Counterpart of `FastSenderFromArbitrum` */ -contract FastBridgeReceiverOnEthereumMock is FastBridgeReceiverOnEthereum { +contract FastBridgeReceiverOnEthereumMock is IFastBridgeReceiver, ISafeBridgeReceiver { // **************************************** // - // * Hardhat Specific * // + // * * // + // * Ethereum Receiver Specific * // + // * * // // **************************************** // + + // ************************************* // + // * Storage * // + // ************************************* // + + IInbox public immutable inbox; // The address of the Arbitrum Inbox contract. + + // ************************************* // + // * Views * // + // ************************************* // + function isSentBySafeBridge() internal view override returns (bool) { IOutbox outbox = IOutbox(inbox.bridge().activeOutbox()); - return outbox.l2ToL1Sender() == fastBridgeSender; + return outbox.l2ToL1Sender() == safeBridgeSender; } + /** + * @dev Constructor. + * @param _deposit The deposit amount to submit a claim in wei. + * @param _epochPeriod The duration of each epoch. + * @param _challengePeriod The duration of the period allowing to challenge a claim. + * @param _safeBridgeSender The address of the Safe Bridge Sender on the connecting chain. + * @param _inbox Ethereum receiver specific: The address of the inbox contract on Ethereum. + */ constructor( uint256 _deposit, uint256 _epochPeriod, uint256 _challengePeriod, - address _fastBridgeSender, + address _safeBridgeSender, address _inbox // Ethereum receiver specific - ) FastBridgeReceiverOnEthereum(_deposit, _epochPeriod, _challengePeriod, _fastBridgeSender, _inbox) {} + ) { + deposit = _deposit; + epochPeriod = _epochPeriod; + challengePeriod = _challengePeriod; + safeBridgeSender = _safeBridgeSender; + inbox = IInbox(_inbox); // Ethereum receiver specific + } + + // ************************************** // + // * * // + // * General Receiver * // + // * * // + // ************************************** // + + // ************************************* // + // * Enums / Structs * // + // ************************************* // + + struct Claim { + bytes32 batchMerkleRoot; + address bridger; + uint32 timestamp; + bool honest; + bool verificationAttempted; + bool depositAndRewardWithdrawn; + } + + struct Challenge { + address challenger; + bool honest; + bool depositAndRewardWithdrawn; + } + + // ************************************* // + // * Storage * // + // ************************************* // + + uint256 public immutable deposit; // The deposit required to submit a claim or challenge + uint256 public immutable override epochPeriod; // Epochs mark the period between potential batches of messages. + uint256 public immutable override challengePeriod; // Epochs mark the period between potential batches of messages. + address public immutable safeBridgeSender; // The address of the Safe Bridge Sender on the connecting chain. + + mapping(uint256 => bytes32) public fastInbox; // epoch => validated batch merkle root(optimistically, or challenged and verified with the safe bridge) + mapping(uint256 => Claim) public claims; // epoch => claim + mapping(uint256 => Challenge) public challenges; // epoch => challenge + mapping(uint256 => mapping(uint256 => bytes32)) public relayed; // epoch => packed replay bitmap + + // ************************************* // + // * State Modifiers * // + // ************************************* // + + /** + * @dev Submit a claim about the `_batchMerkleRoot` for the last completed epoch from the Fast Bridge and submit a deposit. The `_batchMerkleRoot` should match the one on the sending side otherwise the sender will lose his deposit. + * @param _epoch The epoch in which the batch to claim. + * @param _batchMerkleRoot The batch merkle root claimed for the last completed epoch. + */ + function claim(uint256 _epoch, bytes32 _batchMerkleRoot) external payable override { + require(msg.value >= deposit, "Insufficient claim deposit."); + require(_batchMerkleRoot != bytes32(0), "Invalid claim."); + + uint256 epochNow = block.timestamp / epochPeriod; + // allow claim about current or previous epoch + require(_epoch == epochNow || _epoch == epochNow - 1, "Invalid epoch."); + require(claims[_epoch].bridger == address(0), "Claim already made for most recent finalized epoch."); + + claims[_epoch] = Claim({ + batchMerkleRoot: _batchMerkleRoot, + bridger: msg.sender, + timestamp: uint32(block.timestamp), + honest: false, + verificationAttempted: false, + depositAndRewardWithdrawn: false + }); + emit ClaimReceived(_epoch, _batchMerkleRoot); + } + + /** + * @dev Submit a challenge for the claim of the current epoch's Fast Bridge batch merkleroot state and submit a deposit. The `batchMerkleRoot` in the claim already made for the last finalized epoch should be different from the one on the sending side, otherwise the sender will lose his deposit. + * @param _epoch The epoch of the claim to challenge. + */ + function challenge(uint256 _epoch) external payable override { + require(msg.value >= deposit, "Not enough claim deposit"); + + // Can only challenge the only active claim, about the previous epoch + require(claims[_epoch].bridger != address(0), "No claim to challenge."); + require(block.timestamp < uint256(claims[_epoch].timestamp) + challengePeriod, "Challenge period elapsed."); + + challenges[_epoch] = Challenge({challenger: msg.sender, honest: false, depositAndRewardWithdrawn: false}); + emit ClaimChallenged(_epoch); + } + + /** + * @dev Resolves the optimistic claim for '_epoch'. + * @param _epoch The epoch of the optimistic claim. + */ + function verifyBatch(uint256 _epoch) external override { + Claim storage claim = claims[_epoch]; + require(claim.bridger != address(0), "Invalid epoch, no claim to verify."); + require(claim.verificationAttempted == false, "Optimistic verification already attempted."); + require( + block.timestamp > uint256(claims[_epoch].timestamp) + challengePeriod, + "Challenge period has not yet elapsed." + ); + + if (challenges[_epoch].challenger == address(0)) { + // Optimistic happy path + claim.honest = true; + fastInbox[_epoch] = claim.batchMerkleRoot; + emit BatchVerified(_epoch, true); + } else { + emit BatchVerified(_epoch, false); + } + claim.verificationAttempted = true; + } + + /** + * Note: Access restricted to the Safe Bridge. + * @dev Resolves any challenge of the optimistic claim for '_epoch'. + * @param _epoch The epoch to verify. + * @param _batchMerkleRoot The true batch merkle root for the epoch. + */ + function verifySafeBatch(uint256 _epoch, bytes32 _batchMerkleRoot) external override { + require(isSentBySafeBridge(), "Access not allowed: SafeBridgeSender only."); + + fastInbox[_epoch] = _batchMerkleRoot; + + // Corner cases: + // a) No claim submitted, + // b) Receiving the root of an empty batch, + // c) Batch root is zero. + if (claims[_epoch].bridger != address(0)) { + if (_batchMerkleRoot == claims[_epoch].batchMerkleRoot) { + claims[_epoch].honest = true; + } else { + claims[_epoch].honest = false; + challenges[_epoch].honest = true; + } + } + emit BatchSafeVerified(_epoch, claims[_epoch].honest, challenges[_epoch].honest); + } + + /** + * @dev Verifies merkle proof for the given message and associated nonce for the epoch and relays the message. + * @param _epoch The epoch in which the message was batched by the bridge. + * @param _proof The merkle proof to prove the membership of the message and nonce in the merkle tree for the epoch. + * @param _message The data on the cross-domain chain for the message. + */ + function verifyAndRelayMessage( + uint256 _epoch, + bytes32[] calldata _proof, + bytes calldata _message + ) external override { + bytes32 batchMerkleRoot = fastInbox[_epoch]; + require(batchMerkleRoot != bytes32(0), "Invalid epoch."); + + // Claim assessment if any + require(validateProof(_proof, sha256(_message), batchMerkleRoot) == true, "Invalid proof."); + require(_checkReplayAndRelay(_epoch, _message), "Failed to call contract"); // Checks-Effects-Interaction + } + + /** + * @dev Sends the deposit back to the Bridger if their claim is not successfully challenged. Includes a portion of the Challenger's deposit if unsuccessfully challenged. + * @param _epoch The epoch associated with the claim deposit to withraw. + */ + function withdrawClaimDeposit(uint256 _epoch) external override { + Claim storage claim = claims[_epoch]; + + require(claim.bridger != address(0), "Claim does not exist"); + require(claim.honest == true, "Claim failed."); + require(claim.depositAndRewardWithdrawn == false, "Claim deposit and any rewards already withdrawn."); + + uint256 amount = deposit; + if (challenges[_epoch].challenger != address(0) && challenges[_epoch].honest == false) { + amount += deposit / 2; // half burnt + } + + claim.depositAndRewardWithdrawn = true; + emit ClaimDepositWithdrawn(_epoch, claim.bridger); + + payable(claim.bridger).send(amount); // Use of send to prevent reverting fallback. User is responsibility for accepting ETH. + // Checks-Effects-Interaction + } + + /** + * @dev Sends the deposit back to the Challenger if their challenge is successful. Includes a portion of the Bridger's deposit. + * @param _epoch The epoch associated with the challenge deposit to withraw. + */ + function withdrawChallengeDeposit(uint256 _epoch) external override { + Challenge storage challenge = challenges[_epoch]; + + require(challenge.challenger != address(0), "Challenge does not exist"); + require(challenge.honest == true, "Challenge failed."); + require(challenge.depositAndRewardWithdrawn == false, "Challenge deposit and rewards already withdrawn."); + + uint256 amount = deposit; + if (claims[_epoch].bridger != address(0) && claims[_epoch].honest == false) { + amount += deposit / 2; // half burnt + } + + challenge.depositAndRewardWithdrawn = true; + emit ChallengeDepositWithdrawn(_epoch, challenge.challenger); + + payable(challenge.challenger).send(amount); // Use of send to prevent reverting fallback. User is responsibility for accepting ETH. + // Checks-Effects-Interaction + } + + // ********************************** // + // * Merkle Proof * // + // ********************************** // + + /** + * @dev Validates membership of leaf in merkle tree with merkle proof. + * Note: Inlined from `merkle/MerkleProof.sol` for performance. + * @param proof The merkle proof. + * @param leaf The leaf to validate membership in merkle tree. + * @param merkleRoot The root of the merkle tree. + */ + function validateProof( + bytes32[] memory proof, + bytes32 leaf, + bytes32 merkleRoot + ) internal pure returns (bool) { + return (merkleRoot == calculateRoot(proof, leaf)); + } + + /** + * @dev Calculates merkle root from proof. + * @param proof The merkle proof. + * @param leaf The leaf to validate membership in merkle tree.. + */ + function calculateRoot(bytes32[] memory proof, bytes32 leaf) private pure returns (bytes32) { + uint256 proofLength = proof.length; + require(proofLength <= 32, "Invalid Proof"); + bytes32 h = leaf; + for (uint256 i = 0; i < proofLength; i++) { + bytes32 proofElement = proof[i]; + // effecient hash + if (proofElement > h) + assembly { + mstore(0x00, h) + mstore(0x20, proofElement) + h := keccak256(0x00, 0x40) + } + else + assembly { + mstore(0x00, proofElement) + mstore(0x20, h) + h := keccak256(0x00, 0x40) + } + } + return h; + } + + // ************************************* // + // * Public Views * // + // ************************************* // + + /** + * @dev Returns the `start` and `end` time of challenge period for this `epoch`. + * @param _epoch The epoch of the claim to request the challenge period. + * @return start The start time of the challenge period. + * @return end The end time of the challenge period. + */ + function claimChallengePeriod(uint256 _epoch) external view override returns (uint256 start, uint256 end) { + // start begins latest after the claim deadline expiry + // however can begin as soon as a claim is made + // can only challenge the only active claim, about the previous epoch + start = claims[_epoch].timestamp; + end = start + challengePeriod; + } + + // ************************ // + // * Internal * // + // ************************ // + + function _checkReplayAndRelay(uint256 _epoch, bytes calldata _messageData) internal returns (bool success) { + // Decode the receiver gateway address from the data encoded by the IFastBridgeSender + (uint256 nonce, address receiver, bytes memory data) = abi.decode(_messageData, (uint256, address, bytes)); + uint256 index = nonce / 256; + uint256 offset = nonce % 256; + bytes32 replay = relayed[_epoch][index]; + require(((replay >> offset) & bytes32(uint256(1))) == 0, "Message already relayed"); + relayed[_epoch][index] = replay | bytes32(1 << offset); + emit MessageRelayed(_epoch, nonce); + (success, ) = receiver.call(data); + // Checks-Effects-Interaction + } } diff --git a/contracts/test/integration/index.ts b/contracts/test/integration/index.ts index 81bdc759..86fb07c0 100644 --- a/contracts/test/integration/index.ts +++ b/contracts/test/integration/index.ts @@ -65,13 +65,13 @@ describe("Integration tests", async () => { // FastBridgeSender expect(await fastBridgeSender.arbSys()).to.equal(arbsysMock.address); expect(await fastBridgeSender.epochPeriod()).to.equal(EPOCH_PERIOD); - expect(await fastBridgeSender.fastBridgeReceiver()).to.equal(fastBridgeReceiver.address); + expect(await fastBridgeSender.safeBridgeReceiver()).to.equal(fastBridgeReceiver.address); // FastBridgeReceiver expect(await fastBridgeReceiver.deposit()).to.equal(ONE_TENTH_ETH); expect(await fastBridgeReceiver.epochPeriod()).to.equal(EPOCH_PERIOD); expect(await fastBridgeReceiver.challengePeriod()).to.equal(CHALLENGE_PERIOD); - expect(await fastBridgeReceiver.fastBridgeSender()).to.equal(fastBridgeSender.address); + expect(await fastBridgeReceiver.safeBridgeSender()).to.equal(fastBridgeSender.address); expect(await fastBridgeReceiver.inbox()).to.equal(inbox.address); // ReceiverGateway From 0d501798364cf5339a1702aa733be2758ae42e8e Mon Sep 17 00:00:00 2001 From: jaybuidl Date: Thu, 2 Feb 2023 14:50:19 +0000 Subject: [PATCH 2/2] fix: revert focused on the bad variable rename only --- .../src/FastBridgeReceiverOnEthereum.sol | 6 +- contracts/src/test/FastBridgeReceiverMock.sol | 303 +----------------- 2 files changed, 7 insertions(+), 302 deletions(-) diff --git a/contracts/src/FastBridgeReceiverOnEthereum.sol b/contracts/src/FastBridgeReceiverOnEthereum.sol index 74daeda4..2c6df07d 100644 --- a/contracts/src/FastBridgeReceiverOnEthereum.sol +++ b/contracts/src/FastBridgeReceiverOnEthereum.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT /** - * @authors: [@jaybuidl, @shotaronowhere, @hrishibhat] + * @authors: [@jaybuidl, @shotaronowhere, @hrishibhat, @adi274] * @reviewers: [] * @auditors: [] * @bounties: [] @@ -36,9 +36,9 @@ contract FastBridgeReceiverOnEthereum is IFastBridgeReceiver, ISafeBridgeReceive // * Views * // // ************************************* // - function isSentBySafeBridge() internal view override returns (bool) { + function isSentBySafeBridge() internal view virtual override returns (bool) { IOutbox outbox = IOutbox(inbox.bridge().activeOutbox()); - return outbox.l2ToL1Sender() == safeBridgeSender; + return msg.sender == address(outbox) && outbox.l2ToL1Sender() == safeBridgeSender; } /** diff --git a/contracts/src/test/FastBridgeReceiverMock.sol b/contracts/src/test/FastBridgeReceiverMock.sol index 553f4d4a..25a9847b 100644 --- a/contracts/src/test/FastBridgeReceiverMock.sol +++ b/contracts/src/test/FastBridgeReceiverMock.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT /** - * @authors: [@jaybuidl, @shotaronowhere, @hrishibhat] + * @authors: [@jaybuidl, @shotaronowhere, @hrishibhat, @adi274] * @reviewers: [] * @auditors: [] * @bounties: [] @@ -10,28 +10,19 @@ pragma solidity ^0.8.0; -import "../interfaces/IFastBridgeReceiver.sol"; -import "../interfaces/ISafeBridgeReceiver.sol"; -import "../canonical/arbitrum/IInbox.sol"; -import "../canonical/arbitrum/IOutbox.sol"; +import "../FastBridgeReceiverOnEthereum.sol"; /** * Fast Receiver On Ethereum * Counterpart of `FastSenderFromArbitrum` */ -contract FastBridgeReceiverOnEthereumMock is IFastBridgeReceiver, ISafeBridgeReceiver { +contract FastBridgeReceiverOnEthereumMock is FastBridgeReceiverOnEthereum { // **************************************** // // * * // // * Ethereum Receiver Specific * // // * * // // **************************************** // - // ************************************* // - // * Storage * // - // ************************************* // - - IInbox public immutable inbox; // The address of the Arbitrum Inbox contract. - // ************************************* // // * Views * // // ************************************* // @@ -55,291 +46,5 @@ contract FastBridgeReceiverOnEthereumMock is IFastBridgeReceiver, ISafeBridgeRec uint256 _challengePeriod, address _safeBridgeSender, address _inbox // Ethereum receiver specific - ) { - deposit = _deposit; - epochPeriod = _epochPeriod; - challengePeriod = _challengePeriod; - safeBridgeSender = _safeBridgeSender; - inbox = IInbox(_inbox); // Ethereum receiver specific - } - - // ************************************** // - // * * // - // * General Receiver * // - // * * // - // ************************************** // - - // ************************************* // - // * Enums / Structs * // - // ************************************* // - - struct Claim { - bytes32 batchMerkleRoot; - address bridger; - uint32 timestamp; - bool honest; - bool verificationAttempted; - bool depositAndRewardWithdrawn; - } - - struct Challenge { - address challenger; - bool honest; - bool depositAndRewardWithdrawn; - } - - // ************************************* // - // * Storage * // - // ************************************* // - - uint256 public immutable deposit; // The deposit required to submit a claim or challenge - uint256 public immutable override epochPeriod; // Epochs mark the period between potential batches of messages. - uint256 public immutable override challengePeriod; // Epochs mark the period between potential batches of messages. - address public immutable safeBridgeSender; // The address of the Safe Bridge Sender on the connecting chain. - - mapping(uint256 => bytes32) public fastInbox; // epoch => validated batch merkle root(optimistically, or challenged and verified with the safe bridge) - mapping(uint256 => Claim) public claims; // epoch => claim - mapping(uint256 => Challenge) public challenges; // epoch => challenge - mapping(uint256 => mapping(uint256 => bytes32)) public relayed; // epoch => packed replay bitmap - - // ************************************* // - // * State Modifiers * // - // ************************************* // - - /** - * @dev Submit a claim about the `_batchMerkleRoot` for the last completed epoch from the Fast Bridge and submit a deposit. The `_batchMerkleRoot` should match the one on the sending side otherwise the sender will lose his deposit. - * @param _epoch The epoch in which the batch to claim. - * @param _batchMerkleRoot The batch merkle root claimed for the last completed epoch. - */ - function claim(uint256 _epoch, bytes32 _batchMerkleRoot) external payable override { - require(msg.value >= deposit, "Insufficient claim deposit."); - require(_batchMerkleRoot != bytes32(0), "Invalid claim."); - - uint256 epochNow = block.timestamp / epochPeriod; - // allow claim about current or previous epoch - require(_epoch == epochNow || _epoch == epochNow - 1, "Invalid epoch."); - require(claims[_epoch].bridger == address(0), "Claim already made for most recent finalized epoch."); - - claims[_epoch] = Claim({ - batchMerkleRoot: _batchMerkleRoot, - bridger: msg.sender, - timestamp: uint32(block.timestamp), - honest: false, - verificationAttempted: false, - depositAndRewardWithdrawn: false - }); - emit ClaimReceived(_epoch, _batchMerkleRoot); - } - - /** - * @dev Submit a challenge for the claim of the current epoch's Fast Bridge batch merkleroot state and submit a deposit. The `batchMerkleRoot` in the claim already made for the last finalized epoch should be different from the one on the sending side, otherwise the sender will lose his deposit. - * @param _epoch The epoch of the claim to challenge. - */ - function challenge(uint256 _epoch) external payable override { - require(msg.value >= deposit, "Not enough claim deposit"); - - // Can only challenge the only active claim, about the previous epoch - require(claims[_epoch].bridger != address(0), "No claim to challenge."); - require(block.timestamp < uint256(claims[_epoch].timestamp) + challengePeriod, "Challenge period elapsed."); - - challenges[_epoch] = Challenge({challenger: msg.sender, honest: false, depositAndRewardWithdrawn: false}); - emit ClaimChallenged(_epoch); - } - - /** - * @dev Resolves the optimistic claim for '_epoch'. - * @param _epoch The epoch of the optimistic claim. - */ - function verifyBatch(uint256 _epoch) external override { - Claim storage claim = claims[_epoch]; - require(claim.bridger != address(0), "Invalid epoch, no claim to verify."); - require(claim.verificationAttempted == false, "Optimistic verification already attempted."); - require( - block.timestamp > uint256(claims[_epoch].timestamp) + challengePeriod, - "Challenge period has not yet elapsed." - ); - - if (challenges[_epoch].challenger == address(0)) { - // Optimistic happy path - claim.honest = true; - fastInbox[_epoch] = claim.batchMerkleRoot; - emit BatchVerified(_epoch, true); - } else { - emit BatchVerified(_epoch, false); - } - claim.verificationAttempted = true; - } - - /** - * Note: Access restricted to the Safe Bridge. - * @dev Resolves any challenge of the optimistic claim for '_epoch'. - * @param _epoch The epoch to verify. - * @param _batchMerkleRoot The true batch merkle root for the epoch. - */ - function verifySafeBatch(uint256 _epoch, bytes32 _batchMerkleRoot) external override { - require(isSentBySafeBridge(), "Access not allowed: SafeBridgeSender only."); - - fastInbox[_epoch] = _batchMerkleRoot; - - // Corner cases: - // a) No claim submitted, - // b) Receiving the root of an empty batch, - // c) Batch root is zero. - if (claims[_epoch].bridger != address(0)) { - if (_batchMerkleRoot == claims[_epoch].batchMerkleRoot) { - claims[_epoch].honest = true; - } else { - claims[_epoch].honest = false; - challenges[_epoch].honest = true; - } - } - emit BatchSafeVerified(_epoch, claims[_epoch].honest, challenges[_epoch].honest); - } - - /** - * @dev Verifies merkle proof for the given message and associated nonce for the epoch and relays the message. - * @param _epoch The epoch in which the message was batched by the bridge. - * @param _proof The merkle proof to prove the membership of the message and nonce in the merkle tree for the epoch. - * @param _message The data on the cross-domain chain for the message. - */ - function verifyAndRelayMessage( - uint256 _epoch, - bytes32[] calldata _proof, - bytes calldata _message - ) external override { - bytes32 batchMerkleRoot = fastInbox[_epoch]; - require(batchMerkleRoot != bytes32(0), "Invalid epoch."); - - // Claim assessment if any - require(validateProof(_proof, sha256(_message), batchMerkleRoot) == true, "Invalid proof."); - require(_checkReplayAndRelay(_epoch, _message), "Failed to call contract"); // Checks-Effects-Interaction - } - - /** - * @dev Sends the deposit back to the Bridger if their claim is not successfully challenged. Includes a portion of the Challenger's deposit if unsuccessfully challenged. - * @param _epoch The epoch associated with the claim deposit to withraw. - */ - function withdrawClaimDeposit(uint256 _epoch) external override { - Claim storage claim = claims[_epoch]; - - require(claim.bridger != address(0), "Claim does not exist"); - require(claim.honest == true, "Claim failed."); - require(claim.depositAndRewardWithdrawn == false, "Claim deposit and any rewards already withdrawn."); - - uint256 amount = deposit; - if (challenges[_epoch].challenger != address(0) && challenges[_epoch].honest == false) { - amount += deposit / 2; // half burnt - } - - claim.depositAndRewardWithdrawn = true; - emit ClaimDepositWithdrawn(_epoch, claim.bridger); - - payable(claim.bridger).send(amount); // Use of send to prevent reverting fallback. User is responsibility for accepting ETH. - // Checks-Effects-Interaction - } - - /** - * @dev Sends the deposit back to the Challenger if their challenge is successful. Includes a portion of the Bridger's deposit. - * @param _epoch The epoch associated with the challenge deposit to withraw. - */ - function withdrawChallengeDeposit(uint256 _epoch) external override { - Challenge storage challenge = challenges[_epoch]; - - require(challenge.challenger != address(0), "Challenge does not exist"); - require(challenge.honest == true, "Challenge failed."); - require(challenge.depositAndRewardWithdrawn == false, "Challenge deposit and rewards already withdrawn."); - - uint256 amount = deposit; - if (claims[_epoch].bridger != address(0) && claims[_epoch].honest == false) { - amount += deposit / 2; // half burnt - } - - challenge.depositAndRewardWithdrawn = true; - emit ChallengeDepositWithdrawn(_epoch, challenge.challenger); - - payable(challenge.challenger).send(amount); // Use of send to prevent reverting fallback. User is responsibility for accepting ETH. - // Checks-Effects-Interaction - } - - // ********************************** // - // * Merkle Proof * // - // ********************************** // - - /** - * @dev Validates membership of leaf in merkle tree with merkle proof. - * Note: Inlined from `merkle/MerkleProof.sol` for performance. - * @param proof The merkle proof. - * @param leaf The leaf to validate membership in merkle tree. - * @param merkleRoot The root of the merkle tree. - */ - function validateProof( - bytes32[] memory proof, - bytes32 leaf, - bytes32 merkleRoot - ) internal pure returns (bool) { - return (merkleRoot == calculateRoot(proof, leaf)); - } - - /** - * @dev Calculates merkle root from proof. - * @param proof The merkle proof. - * @param leaf The leaf to validate membership in merkle tree.. - */ - function calculateRoot(bytes32[] memory proof, bytes32 leaf) private pure returns (bytes32) { - uint256 proofLength = proof.length; - require(proofLength <= 32, "Invalid Proof"); - bytes32 h = leaf; - for (uint256 i = 0; i < proofLength; i++) { - bytes32 proofElement = proof[i]; - // effecient hash - if (proofElement > h) - assembly { - mstore(0x00, h) - mstore(0x20, proofElement) - h := keccak256(0x00, 0x40) - } - else - assembly { - mstore(0x00, proofElement) - mstore(0x20, h) - h := keccak256(0x00, 0x40) - } - } - return h; - } - - // ************************************* // - // * Public Views * // - // ************************************* // - - /** - * @dev Returns the `start` and `end` time of challenge period for this `epoch`. - * @param _epoch The epoch of the claim to request the challenge period. - * @return start The start time of the challenge period. - * @return end The end time of the challenge period. - */ - function claimChallengePeriod(uint256 _epoch) external view override returns (uint256 start, uint256 end) { - // start begins latest after the claim deadline expiry - // however can begin as soon as a claim is made - // can only challenge the only active claim, about the previous epoch - start = claims[_epoch].timestamp; - end = start + challengePeriod; - } - - // ************************ // - // * Internal * // - // ************************ // - - function _checkReplayAndRelay(uint256 _epoch, bytes calldata _messageData) internal returns (bool success) { - // Decode the receiver gateway address from the data encoded by the IFastBridgeSender - (uint256 nonce, address receiver, bytes memory data) = abi.decode(_messageData, (uint256, address, bytes)); - uint256 index = nonce / 256; - uint256 offset = nonce % 256; - bytes32 replay = relayed[_epoch][index]; - require(((replay >> offset) & bytes32(uint256(1))) == 0, "Message already relayed"); - relayed[_epoch][index] = replay | bytes32(1 << offset); - emit MessageRelayed(_epoch, nonce); - (success, ) = receiver.call(data); - // Checks-Effects-Interaction - } + ) FastBridgeReceiverOnEthereum(_deposit, _epochPeriod, _challengePeriod, _safeBridgeSender, _inbox) {} }