Migrate constructor logic of Bridge into an initialize function (#405)

* Add witness verification on Bridge

* Use segwit txn as test data in Bridge contract test

* Change deposit parameters to a struct in Bridge

* Change L1BlockHashList address in Bridge

* Remove block header from Bridge contract

* Fix witness utils bugs

* Witness inclusion in Bridge

* Further tests for Bridge

* Rename signature count in Bridge

* Change Bridge's storage structure to have `owner` in first storage slot

* Rearrange variables in storage on Bridge

* Migrate constructor logic to `initialize` in Bridge

* Fix merging errors

* Add revert on reinitialize test case

* Add natspec to `initialize` 

* Remove constructor from Bridge

* Remove already initialized check in deposit

* Remove constructor from Bridge harness

* Revert "Remove constructor from Bridge harness"

This reverts commit ee6e051.

* Revert "Remove already initialized check in deposit"

This reverts commit 8fc0940.

* Remove constructor from Bridge harness in test

* Remove already initialized check from `deposit` 

* Remove duplicate imports

crates/evm/src/evm/system_contracts/src/Bridge.sol

@@ -5,15 +5,13 @@ import "bitcoin-spv/solidity/contracts/ValidateSPV.sol";
 import "bitcoin-spv/solidity/contracts/BTCUtils.sol";
 import "../lib/WitnessUtils.sol";
 import "../lib/Ownable.sol";
-
-
 import "./MerkleTree.sol";
 import "./L1BlockHashList.sol";
 
 /// @title Bridge contract of Clementine
 /// @author Citrea
 
-contract Bridge is MerkleTree, Ownable {
+contract Bridge is Ownable, MerkleTree {
     using BTCUtils for bytes;
     using BytesLib for bytes;
 
@@ -31,13 +29,16 @@ contract Bridge is MerkleTree, Ownable {
 
     L1BlockHashList public constant BLOCK_HASH_LIST = L1BlockHashList(address(0x3100000000000000000000000000000000000001)); 
 
-    bytes public depositScript;
-    bytes public scriptSuffix;
+    bool public initialized;
     uint256 public constant DEPOSIT_AMOUNT = 1 ether;
     address public operator;
+    uint256 public requiredSigsCount;
+    bytes public depositScript;
+    bytes public scriptSuffix;
+
     mapping(bytes32 => bool) public blockHashes;
     mapping(bytes32 => bool) public spentWtxIds;
-    uint256 public requiredSigsCount;
+
 
     event Deposit(bytes32 wtxId, uint256 timestamp);
     event Withdrawal(bytes32  bitcoin_address, uint32 indexed leafIndex, uint256 timestamp);
@@ -49,7 +50,24 @@ contract Bridge is MerkleTree, Ownable {
         _;
     }
 
-    constructor(uint32 _levels) MerkleTree(_levels) {}
+    /// @notice Initializes the bridge contract, caches the sublevels of the withdrawal tree and sets the deposit script
+    /// @param _levels The depth of the Merkle tree
+    /// @param _depositScript The deposit script expected in the witness field for all L1 deposits
+    /// @param _scriptSuffix The suffix of the deposit script that follows the receiver address
+    /// @param _requiredSigsCount The number of signatures that is contained in the deposit script
+    function initialize(uint32 _levels, bytes calldata _depositScript, bytes calldata _scriptSuffix, uint256 _requiredSigsCount) external onlyOwner {
+        require(!initialized, "Contract is already initialized");
+        require(_requiredSigsCount != 0, "Verifier count cannot be 0");
+        require(_depositScript.length != 0, "Deposit script cannot be empty");
+
+        initialized = true;
+        initializeTree(_levels);
+        depositScript = _depositScript;
+        scriptSuffix = _scriptSuffix;
+        requiredSigsCount = _requiredSigsCount;
+
+        emit DepositScriptUpdate(_depositScript, _scriptSuffix, _requiredSigsCount);
+    }
 
     /// @notice Sets the expected deposit script of the deposit transaction on Bitcoin, contained in the witness
     /// @dev Deposit script contains a fixed script that checks signatures of verifiers and pushes EVM address of the receiver
@@ -72,7 +90,9 @@ contract Bridge is MerkleTree, Ownable {
     function deposit(
         DepositParams calldata p
     ) external onlyOperator {
-        require(requiredSigsCount != 0, "Contract is not initialized");
+        // We don't need to check if the contract is initialized, as without an `initialize` call and `deposit` calls afterwards,
+        // only the system caller can execute a transaction on Citrea, as no addresses have any balance. Thus there's no risk of 
+        // `deposit` being called before `initialize` maliciously.
 
         bytes32 wtxId = WitnessUtils.calculateWtxId(p.version, p.flag, p.vin, p.vout, p.witness, p.locktime);
         require(!spentWtxIds[wtxId], "wtxId already spent");

crates/evm/src/evm/system_contracts/src/MerkleTree.sol

@@ -4,17 +4,13 @@ pragma solidity ^0.8.13;
 contract MerkleTree {
     bytes32 public constant ZERO_VALUE = 0xcb0c9f4264546b15be9801ecb11df7e43bfc6841609fc1e4e9de5b3a5973af38; // keccak256("CITREA")
 
-    uint32 public immutable levels;
+    uint32 public levels;
     mapping(uint256 => bytes32) filledSubtrees;
     bytes32 root;
     uint32 nextIndex;
 
-    constructor(uint32 _levels) {
+    function initializeTree(uint32 _levels) internal {
         levels = _levels;
-        initializeTree();
-    }
-
-    function initializeTree() internal {
         for (uint32 i = 0; i < levels; i++) {
             filledSubtrees[i] = zeros(i);
         }

crates/evm/src/evm/system_contracts/test/Bridge.t.sol

@@ -12,7 +12,6 @@ import "bitcoin-spv/solidity/contracts/BTCUtils.sol";
 // !!! - Write fuzz tests for deposit and withdraw actions with random Bitcoin txns if this goes to production
 
 contract BridgeHarness is Bridge {
-    constructor(uint32 _levels) Bridge(_levels) {}
     // Overriding in harness is needed as internal functions are not accessible in the test
     function isBytesEqual_(bytes memory a, bytes memory b) public pure returns (bool result) {
         result = super.isBytesEqual(a, b);
@@ -23,7 +22,6 @@ contract BridgeTest is Test {
     uint256 constant DEPOSIT_AMOUNT = 1 ether;
     BridgeHarness public bridge;
     bytes2 flag = hex"0001";
-
     bytes4 version = hex"02000000";
     bytes vin = hex"01d4d6c5c94583a0505dd0c1eb64760ba2a6a391f6da3164094ed8bcac190b7d6c0000000000fdffffff";
     bytes vout = hex"0378dcf50500000000225120081bb55c845b1b14b8580a0246764d53d4aa579645c67568d8375c71f687a2ce4a01000000000000220020340a847f2a890d208f6c7a21811116134bd2b01cc1d46a999e61da195f6b8a3b4a010000000000002200204ae81572f06e1b88fd5ced7a1a000945432e83e1551e6f721ee9c00b8cc33260";
@@ -43,7 +41,8 @@ contract BridgeTest is Test {
     bytes32 mockBlockhash = keccak256("CITREA_TEST");
 
     function setUp() public {
-        bridge = new BridgeHarness(31);
+        bridge = new BridgeHarness();
+        bridge.initialize(31, depositScript, scriptSuffix, 5);
         vm.deal(address(bridge), 21_000_000 ether);
         address block_hash_list_impl = address(new L1BlockHashList());
         L1BlockHashList l1BlockHashList = bridge.BLOCK_HASH_LIST();
@@ -59,8 +58,6 @@ contract BridgeTest is Test {
 
         // Arbitrary blockhash as this is mock 
         l1BlockHashList.setBlockInfo(mockBlockhash, witnessRoot);
-
-        bridge.setDepositScript(depositScript, scriptSuffix, 5);
     }
 
     function testZeros() public view {
@@ -208,6 +205,11 @@ contract BridgeTest is Test {
         bridge.setOperator(operator);
     }
 
+    function testCannotReinitialize() public {
+        vm.expectRevert("Contract is already initialized");
+        bridge.initialize(31, depositScript, scriptSuffix, 5);
+    }
+
     function testBytesEqual() public {
         bytes memory a = hex"1234";
         bytes memory b = hex"1234";