Merge pull request #809 from chainwayxyz/okko/bridge-upgradeable

Make Bridge upgradeable

bin/citrea/tests/e2e/mod.rs

@@ -1776,28 +1776,28 @@ async fn test_system_tx_effect_on_block_gas_limit() -> Result<(), anyhow::Error>
 
     let seq_port = seq_port_rx.await.unwrap();
     let seq_test_client = make_test_client(seq_port).await;
-    // sys tx use L1BlockHash(43615 + 73581) + Bridge(298471) = 415667 gas
+    // sys tx use L1BlockHash(48522 + 78491) + Bridge(258971) = 385984 gas
     // the block gas limit is 1_500_000 because the system txs gas limit is 1_500_000 (decided with @eyusufatik and @okkothejawa as bridge init takes 1M gas)
 
-    // 1500000 - 415667 = 1084333 gas left in block
-    // 1084333 / 21000 = 51,6... so 51 ether transfer transactions can be included in the block
+    // 1500000 - 385984 = 1114016 gas left in block
+    // 1114016 / 21000 = 53,04... so 53 ether transfer transactions can be included in the block
 
-    // send 51 ether transfer transactions
+    // send 53 ether transfer transactions
     let addr = Address::from_str("0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266").unwrap();
 
-    for _ in 0..50 {
+    for _ in 0..52 {
         let _pending = seq_test_client
             .send_eth(addr, None, None, None, 0u128)
             .await
             .unwrap();
     }
 
-    // 51th tx should be the last tx in the soft batch
+    // 53th tx should be the last tx in the soft batch
     let last_in_tx = seq_test_client
         .send_eth(addr, None, None, None, 0u128)
         .await;
 
-    // 52th tx should not be in soft batch
+    // 54th tx should not be in soft batch
     let not_in_tx = seq_test_client
         .send_eth(addr, None, None, None, 0u128)
         .await;
@@ -2097,7 +2097,7 @@ async fn transaction_failing_on_l1_is_removed_from_mempool() -> Result<(), anyho
 
     let random_wallet_address = random_wallet.address();
 
-    let second_block_base_fee = 768881663;
+    let second_block_base_fee = 768592592;
 
     let _pending = seq_test_client
         .send_eth(
@@ -3028,7 +3028,7 @@ async fn test_gas_limit_too_high() {
 
     let target_gas_limit: u64 = 30_000_000;
     let transfer_gas_limit = 21_000;
-    let system_txs_gas_used = 415_811;
+    let system_txs_gas_used = 385984;
     let tx_count = (target_gas_limit - system_txs_gas_used).div_ceil(transfer_gas_limit);
     let addr = Address::from_str("0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266").unwrap();
 

bin/citrea/tests/evm/web3_py/test.py

@@ -42,8 +42,8 @@ def test_get_balance(self):
         self.assertGreaterEqual(balance, (21 * 10 ** 6) * 10 ** 18)
 
     def test_get_storage_at(self):
-        slot = self.web3.eth.get_storage_at("0x3100000000000000000000000000000000000002", 2)
-        self.assertEqual(slot, HexBytes('0x000000000000000000000000deaddeaddeaddeaddeaddeaddeaddeaddeaddead'))
+        slot = self.web3.eth.get_storage_at("0x3100000000000000000000000000000000000002", 0)
+        self.assertEqual(slot, HexBytes('0x0000000000000000000000deaddeaddeaddeaddeaddeaddeaddeaddeaddead01'))
 
     def test_get_code(self):
         code = self.web3.eth.get_code('0x3200000000000000000000000000000000000001')

crates/evm/src/evm/system_contracts/mod.rs

@@ -80,29 +80,26 @@ impl Bridge {
         //     _depositScript: hex!("d2205daf577048c5e5a9a75d0a924ed03e226c3304f4a2f01c65ca1dab73522e6b8bad206228eba653cf1819bcfc1bc858630e5ae373eec1a9924322a5fe8445c5e76027ad201521d65f64be3f71b71ca462220f13c77b251027f6ca443a483353a96fbce222ad200fabeed269694ee83d9b3343a571202e68af65d05feda61dbed0c4bdb256a6eaad2000326d6f721c03dc5f1d8817d8f8ee890a95a2eeda0d4d9a01b1cc9b7b1b724dac00630663697472656114").into(),
         //     _scriptSuffix: hex!("0800000000000f424068").into(),
         //     _requiredSigsCount: U256::from(5),
-        //     _owner: address!("f9725b63fe14efaf7cc705ba4e5c55a03d50e940"),
         // }
         // .abi_encode()
         let params = vec![
             0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-            0, 0, 128, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-            0, 0, 0, 0, 0, 1, 96, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-            0, 0, 0, 0, 0, 0, 0, 0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 249, 114, 91, 99,
-            254, 20, 239, 175, 124, 199, 5, 186, 78, 92, 85, 160, 61, 80, 233, 64, 0, 0, 0, 0, 0,
-            0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 181, 210,
-            32, 93, 175, 87, 112, 72, 197, 229, 169, 167, 93, 10, 146, 78, 208, 62, 34, 108, 51, 4,
-            244, 162, 240, 28, 101, 202, 29, 171, 115, 82, 46, 107, 139, 173, 32, 98, 40, 235, 166,
-            83, 207, 24, 25, 188, 252, 27, 200, 88, 99, 14, 90, 227, 115, 238, 193, 169, 146, 67,
-            34, 165, 254, 132, 69, 197, 231, 96, 39, 173, 32, 21, 33, 214, 95, 100, 190, 63, 113,
-            183, 28, 164, 98, 34, 15, 19, 199, 123, 37, 16, 39, 246, 202, 68, 58, 72, 51, 83, 169,
-            111, 188, 226, 34, 173, 32, 15, 171, 238, 210, 105, 105, 78, 232, 61, 155, 51, 67, 165,
-            113, 32, 46, 104, 175, 101, 208, 95, 237, 166, 29, 190, 208, 196, 189, 178, 86, 166,
-            234, 173, 32, 0, 50, 109, 111, 114, 28, 3, 220, 95, 29, 136, 23, 216, 248, 238, 137,
-            10, 149, 162, 238, 218, 13, 77, 154, 1, 177, 204, 155, 123, 27, 114, 77, 172, 0, 99, 6,
-            99, 105, 116, 114, 101, 97, 20, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-            0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 10, 8, 0, 0, 0,
-            0, 0, 15, 66, 64, 104, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
-            0,
+            0, 0, 96, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+            0, 0, 0, 0, 1, 64, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+            0, 0, 0, 0, 0, 0, 0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+            0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 181, 210, 32, 93, 175, 87, 112, 72, 197, 229, 169,
+            167, 93, 10, 146, 78, 208, 62, 34, 108, 51, 4, 244, 162, 240, 28, 101, 202, 29, 171,
+            115, 82, 46, 107, 139, 173, 32, 98, 40, 235, 166, 83, 207, 24, 25, 188, 252, 27, 200,
+            88, 99, 14, 90, 227, 115, 238, 193, 169, 146, 67, 34, 165, 254, 132, 69, 197, 231, 96,
+            39, 173, 32, 21, 33, 214, 95, 100, 190, 63, 113, 183, 28, 164, 98, 34, 15, 19, 199,
+            123, 37, 16, 39, 246, 202, 68, 58, 72, 51, 83, 169, 111, 188, 226, 34, 173, 32, 15,
+            171, 238, 210, 105, 105, 78, 232, 61, 155, 51, 67, 165, 113, 32, 46, 104, 175, 101,
+            208, 95, 237, 166, 29, 190, 208, 196, 189, 178, 86, 166, 234, 173, 32, 0, 50, 109, 111,
+            114, 28, 3, 220, 95, 29, 136, 23, 216, 248, 238, 137, 10, 149, 162, 238, 218, 13, 77,
+            154, 1, 177, 204, 155, 123, 27, 114, 77, 172, 0, 99, 6, 99, 105, 116, 114, 101, 97, 20,
+            0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
+            0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 10, 8, 0, 0, 0, 0, 0, 15, 66, 64, 104, 0, 0, 0,
+            0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
         ];
 
         let mut func_selector = Vec::with_capacity(4 + params.len());

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

@@ -4,13 +4,14 @@ pragma solidity ^0.8.13;
 import "bitcoin-spv/solidity/contracts/ValidateSPV.sol";
 import "bitcoin-spv/solidity/contracts/BTCUtils.sol";
 import "../lib/WitnessUtils.sol";
-import "../lib/Ownable.sol";
 import "./BitcoinLightClient.sol";
+import "openzeppelin-contracts-upgradeable/contracts/proxy/utils/UUPSUpgradeable.sol";
+import "openzeppelin-contracts-upgradeable/contracts/access/Ownable2StepUpgradeable.sol";
 
-/// @title Bridge contract of Clementine
+/// @title Bridge contract for the Citrea end of Citrea <> Bitcoin bridge
 /// @author Citrea
 
-contract Bridge is Ownable {
+contract Bridge is UUPSUpgradeable, Ownable2StepUpgradeable {
     using BTCUtils for bytes;
     using BytesLib for bytes;
 
@@ -58,7 +59,7 @@ contract Bridge is Ownable {
     /// @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(bytes calldata _depositScript, bytes calldata _scriptSuffix, uint256 _requiredSigsCount, address _owner) external onlySystem {
+    function initialize(bytes calldata _depositScript, bytes calldata _scriptSuffix, uint256 _requiredSigsCount) external onlySystem {
         require(!initialized, "Contract is already initialized");
         require(_requiredSigsCount != 0, "Verifier count cannot be 0");
         require(_depositScript.length != 0, "Deposit script cannot be empty");
@@ -70,7 +71,6 @@ contract Bridge is Ownable {
 
         // Set initial operator to SYSTEM_CALLER so that Citrea can get operational by starting to process deposits
         operator = SYSTEM_CALLER;
-        owner = _owner;
 
         emit OperatorUpdated(address(0), SYSTEM_CALLER);
         emit DepositScriptUpdate(_depositScript, _scriptSuffix, _requiredSigsCount);
@@ -92,7 +92,7 @@ contract Bridge is Ownable {
         emit DepositScriptUpdate(_depositScript, _scriptSuffix, _requiredSigsCount);
     }
 
-    /// @notice Checks if funds 1 BTC is sent to the bridge multisig on Bitcoin, and if so, sends 1 cBTC to the receiver
+    /// @notice Checks if the deposit amount is sent to the bridge multisig on Bitcoin, and if so, sends the deposit amount to the receiver
     /// @param p The deposit parameters that contains the info of the deposit transaction on Bitcoin
     function deposit(
         DepositParams calldata p
@@ -190,4 +190,6 @@ contract Bridge is Ownable {
         bytes20 _addr = bytes20(_script.slice(offset, 20));
         return address(uint160(_addr));
     }
+
+    function _authorizeUpgrade(address) internal override onlyOwner {}
 }

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

@@ -6,6 +6,8 @@ import "forge-std/console.sol";
 
 import "../src/Bridge.sol";
 import "bitcoin-spv/solidity/contracts/BTCUtils.sol";
+import "openzeppelin-contracts/contracts/proxy/ERC1967/ERC1967Proxy.sol";
+
 
 // !!! WARNINGS:
 // !!! - Update `testDepositThenWithdraw` and `testBatchWithdraw` with proper testing of withdrawal tree root if this goes to production
@@ -18,9 +20,15 @@ contract BridgeHarness is Bridge {
     }
 }
 
+contract FalseBridge is Bridge {
+    function falseFunc() public pure returns (bytes32) {
+        return keccak256("false");
+    }
+}
+
 contract BridgeTest is Test {
     uint256 constant DEPOSIT_AMOUNT = 0.01 ether;
-    BridgeHarness public bridge;
+    BridgeHarness public bridge = BridgeHarness(address(0x3100000000000000000000000000000000000002));
     bytes2 flag = hex"0001";
     bytes4 version = hex"02000000";
     bytes vin = hex"01d4d6c5c94583a0505dd0c1eb64760ba2a6a391f6da3164094ed8bcac190b7d6c0000000000fdffffff";
@@ -44,17 +52,23 @@ contract BridgeTest is Test {
     BitcoinLightClient bitcoinLightClient;
 
     function setUp() public {
-        bridge = new BridgeHarness();
+        address bridgeImpl = address(new BridgeHarness());
+        address erc1967_impl = address(new ERC1967Proxy(bridgeImpl, ""));
+        vm.etch(address(bridge), erc1967_impl.code);
+        bytes32 IMPLEMENTATION_SLOT = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;
+        bytes32 OWNER_SLOT = 0x9016d09d72d40fdae2fd8ceac6b6234c7706214fd39c1cd1e609a0528c199300;
+        vm.store(address(bridge), IMPLEMENTATION_SLOT, bytes32(uint256(uint160(bridgeImpl))));
+        vm.store(address(bridge), OWNER_SLOT, bytes32(uint256(uint160(owner))));
         vm.prank(SYSTEM_CALLER);
-        bridge.initialize(depositScript, scriptSuffix, 5, owner);
+        bridge.initialize(depositScript, scriptSuffix, 5);
         vm.deal(address(bridge), 21_000_000 ether);
         address lightClient_impl = address(new BitcoinLightClient());
         bitcoinLightClient = bridge.LIGHT_CLIENT();
         vm.etch(address(bitcoinLightClient), lightClient_impl.code);
 
         vm.startPrank(SYSTEM_CALLER);
         bitcoinLightClient.initializeBlockNumber(INITIAL_BLOCK_NUMBER);
-        // Arbitrary blockhash as this is mock 
+        // Arbitrary blockhash as this is mock
         bitcoinLightClient.setBlockInfo(mockBlockhash, witnessRoot);
         vm.stopPrank();
 
@@ -185,7 +199,7 @@ contract BridgeTest is Test {
     function testCannotReinitialize() public {
         vm.expectRevert("Contract is already initialized");
         vm.prank(SYSTEM_CALLER);
-        bridge.initialize(depositScript, scriptSuffix, 5, owner);
+        bridge.initialize(depositScript, scriptSuffix, 5);
     }
 
     function testCanChangeOperatorAndDeposit() public {
@@ -233,6 +247,32 @@ contract BridgeTest is Test {
         assertEq(5, bridge.requiredSigsCount());
     }
 
+    function testUpgrade() public {
+        address falseBridgeImpl = address(new FalseBridge());
+        vm.prank(owner);
+        bridge.upgradeToAndCall(falseBridgeImpl, "");
+        assertEq(FalseBridge(address(bridge)).falseFunc(), keccak256("false"));
+    }
+
+    function testNonOwnerCannotUpgrade() public {
+        address falseBridgeImpl = address(new FalseBridge());
+        vm.prank(user);
+        vm.expectRevert();
+        bridge.upgradeToAndCall(falseBridgeImpl, "");
+    }
+
+    function testOwnerCanChangeAndUpgrade() public {
+        address falseBridgeImpl = address(new FalseBridge());
+        vm.stopPrank();
+        address newOwner = makeAddr("citrea_new_owner");
+        vm.prank(owner);
+        bridge.transferOwnership(newOwner);
+        vm.startPrank(newOwner);
+        bridge.acceptOwnership();
+        bridge.upgradeToAndCall(falseBridgeImpl, "");
+        assertEq(FalseBridge(address(bridge)).falseFunc(), keccak256("false"));
+    }
+
     function isKeccakEqual(bytes memory a, bytes memory b) public pure returns (bool result) {
         result = keccak256(abi.encodePacked(a)) == keccak256(abi.encodePacked(b));
     }