fix(bootstrap): do not fail mark bootstrap

- As outlined in #83, a cross-chain message that can't/won't be retried must not fail. Hence, all calls to `markBootstrapped` on `Bootstrap` should not fail. Fixes #89 - Separately, sending multiple LZ messages in one transaction is convoluted and hence disabled. As a consequence, marking bootstrap on all chains is no longer supported; instead, the caller must provide the LZ chain ID. Along similar lines, the caller must provide the native fee for this transaction.
ExocoreNetwork · Sep 2, 2024 · 662835d · 662835d
1 parent 021ea9d
commit 662835d
Show file tree

Hide file tree

Showing 13 changed files with 252 additions and 144 deletions.
diff --git a/script/11_SetPeers.s.sol b/script/11_SetPeers.s.sol
@@ -2,6 +2,7 @@ pragma solidity ^0.8.19;
 
 import {Bootstrap} from "../src/core/Bootstrap.sol";
 import {ExocoreGateway} from "../src/core/ExocoreGateway.sol";
+import {GatewayStorage} from "../src/storage/GatewayStorage.sol";
 
 import {BaseScript} from "./BaseScript.sol";
 import "forge-std/Script.sol";
@@ -53,12 +54,9 @@ contract SetPeersAndUpgrade is BaseScript {
 
         vm.selectFork(exocore);
         vm.startBroadcast(exocoreValidatorSet.privateKey);
-        // fund the gateway
-        if (exocoreGatewayAddr.balance < 1 ether) {
-            (bool sent,) = exocoreGatewayAddr.call{value: 1 ether}("");
-            require(sent, "Failed to send Ether");
-        }
-        gateway.markBootstrapOnAllChains();
+        uint256 nativeFee =
+            exocoreGateway.quote(clientChainId, abi.encodePacked(GatewayStorage.Action.REQUEST_MARK_BOOTSTRAP, ""));
+        exocoreGateway.markBootstrap{value: nativeFee}(clientChainId);
     }
 
 }
diff --git a/script/14_CorrectBootstrapErrors.s.sol b/script/14_CorrectBootstrapErrors.s.sol
@@ -29,6 +29,8 @@ contract CorrectBootstrapErrors is BaseScript {
     address wstETH;
     address proxyAddress;
     address proxyAdmin;
+    address clientGatewayLogic;
+    bytes initialization;
 
     function setUp() public virtual override {
         // load keys
@@ -62,6 +64,9 @@ contract CorrectBootstrapErrors is BaseScript {
         require(address(vaultImplementation) != address(0), "vault implementation should not be empty");
         vaultBeacon = UpgradeableBeacon(stdJson.readAddress(deployed, ".clientChain.vaultBeacon"));
         require(address(vaultBeacon) != address(0), "vault beacon should not be empty");
+        clientGatewayLogic = stdJson.readAddress(deployed, ".clientChain.clientGatewayLogic");
+        require(clientGatewayLogic != address(0), "client gateway should not be empty");
+        initialization = abi.encodeCall(ClientChainGateway.initialize, (payable(exocoreValidatorSet.addr)));
     }
 
     function run() public {
@@ -81,7 +86,9 @@ contract CorrectBootstrapErrors is BaseScript {
                 block.timestamp + 168 hours,
                 2 seconds,
                 emptyList,
-                address(proxyAdmin)
+                address(proxyAdmin),
+                address(clientGateway),
+                initialization
             )
         );
         proxyAdmin.upgradeAndCall(ITransparentUpgradeableProxy(proxyAddress), address(bootstrapLogic), data);

diff --git a/script/7_DeployBootstrap.s.sol b/script/7_DeployBootstrap.s.sol
@@ -60,6 +60,21 @@ contract DeployBootstrapOnly is BaseScript {
         Bootstrap bootstrapLogic = new Bootstrap(
             address(clientChainLzEndpoint), exocoreChainId, address(vaultBeacon), address(beaconProxyBytecode)
         );
+        // client chain constructor (upgrade details)
+        capsuleImplementation = new ExoCapsule();
+        capsuleBeacon = new UpgradeableBeacon(address(capsuleImplementation));
+        ClientChainGateway clientGatewayLogic = new ClientChainGateway(
+            address(clientChainLzEndpoint),
+            exocoreChainId,
+            address(beaconOracle),
+            address(vaultBeacon),
+            address(capsuleBeacon),
+            address(beaconProxyBytecode)
+        );
+        // then the client chain initialization
+        address[] memory emptyList;
+        bytes memory initialization =
+            abi.encodeWithSelector(clientGatewayLogic.initialize.selector, exocoreValidatorSet.addr, emptyList);
         // bootstrap implementation
         Bootstrap bootstrap = Bootstrap(
             payable(
@@ -75,7 +90,9 @@ contract DeployBootstrapOnly is BaseScript {
                                 block.timestamp + 168 hours,
                                 2 seconds,
                                 whitelistTokens, // vault is auto deployed
-                                address(proxyAdmin)
+                                address(proxyAdmin),
+                                address(clientGatewayLogic),
+                                initialization
                             )
                         )
                     )
@@ -86,23 +103,6 @@ contract DeployBootstrapOnly is BaseScript {
         // initialize proxyAdmin with bootstrap address
         proxyAdmin.initialize(address(bootstrap));
 
-        // now, focus on the client chain constructor
-        capsuleImplementation = new ExoCapsule();
-        capsuleBeacon = new UpgradeableBeacon(address(capsuleImplementation));
-        ClientChainGateway clientGatewayLogic = new ClientChainGateway(
-            address(clientChainLzEndpoint),
-            exocoreChainId,
-            address(beaconOracle),
-            address(vaultBeacon),
-            address(capsuleBeacon),
-            address(beaconProxyBytecode)
-        );
-        // then the client chain initialization
-        address[] memory emptyList;
-        bytes memory initialization =
-            abi.encodeWithSelector(clientGatewayLogic.initialize.selector, exocoreValidatorSet.addr, emptyList);
-        bootstrap.setClientChainGatewayLogic(address(clientGatewayLogic), initialization);
-
         vm.stopBroadcast();
 
         string memory clientChainContracts = "clientChainContracts";

diff --git a/script/integration/1_DeployBootstrap.s.sol b/script/integration/1_DeployBootstrap.s.sol
@@ -130,7 +130,9 @@ contract DeployContracts is Script {
                                 block.timestamp + 3 minutes,
                                 1 seconds,
                                 whitelistTokens,
-                                address(proxyAdmin)
+                                address(proxyAdmin),
+                                address(0x1), // these values don't matter for the localnet generate.js test
+                                bytes("123456")
                             )
                         )
                     )

diff --git a/src/core/Bootstrap.sol b/src/core/Bootstrap.sol
@@ -63,7 +63,9 @@ contract Bootstrap is
         uint256 spawnTime_,
         uint256 offsetDuration_,
         address[] calldata whitelistTokens_,
-        address customProxyAdmin_
+        address customProxyAdmin_,
+        address clientChainGatewayLogic_,
+        bytes calldata clientChainInitializationData_
     ) external initializer {
         if (owner == address(0)) {
             revert Errors.ZeroAddress();
@@ -83,6 +85,7 @@ contract Bootstrap is
 
         customProxyAdmin = customProxyAdmin_;
         bootstrapped = false;
+        _setClientChainGatewayLogic(clientChainGatewayLogic_, clientChainInitializationData_);
 
         // msg.sender is not the proxy admin but the transparent proxy itself, and hence,
         // cannot be used here. we must require a separate owner. since the Exocore validator
@@ -546,30 +549,32 @@ contract Bootstrap is
     /// initialization data must be set. The contract must not have been bootstrapped before.
     /// Once it is marked bootstrapped, the implementation of the contract is upgraded to the
     /// client chain gateway logic contract.
+    /// @dev This call can never fail, since such failures are not handled by ExocoreGateway.
     function markBootstrapped() public onlyCalledFromThis whenNotPaused {
         // whenNotPaused is applied so that the upgrade does not proceed without unpausing it.
         // LZ checks made so far include:
         // lzReceive called by endpoint
         // correct address on remote (peer match)
         // chainId match
         // nonce match, which requires that inbound nonce is uint64(1).
-        // TSS checks are not super clear since they can be set by anyone
-        // but at this point that does not matter since it is not fully implemented anyway.
         if (block.timestamp < spawnTime) {
-            revert Errors.BootstrapNotSpawnTime();
+            // technically never possible unless the block producer does some time-based shenanigans.
+            emit BootstrapNotTimeYet();
+            return;
         }
-        if (bootstrapped) {
-            revert Errors.BootstrapAlreadyBootstrapped();
-        }
-        if (clientChainGatewayLogic == address(0)) {
-            revert Errors.ZeroAddress();
-        }
-        ICustomProxyAdmin(customProxyAdmin).changeImplementation(
+        // bootstrapped = true is only actioned by the clientchaingateway after upgrade
+        // so no need to check for that here
+        try ICustomProxyAdmin(customProxyAdmin).changeImplementation(
             // address(this) is storage address and not logic address. so it is a proxy.
             ITransparentUpgradeableProxy(address(this)),
             clientChainGatewayLogic,
             clientChainInitializationData
-        );
+        ) {
+            emit Bootstrapped();
+        } catch {
+            // to allow retries, never fail
+            emit BootstrapUpgradeFailed();
+        }
         emit Bootstrapped();
     }
 
@@ -584,9 +589,22 @@ contract Bootstrap is
         public
         onlyOwner
     {
+        _setClientChainGatewayLogic(_clientChainGatewayLogic, _clientChainInitializationData);
+    }
+
+    /// @dev Internal version of `setClientChainGatewayLogic`.
+    /// @param _clientChainGatewayLogic The address of the new client chain gateway logic
+    /// contract.
+    /// @param _clientChainInitializationData The initialization data to be used when setting up
+    /// the new logic contract.
+    function _setClientChainGatewayLogic(
+        address _clientChainGatewayLogic,
+        bytes calldata _clientChainInitializationData
+    ) internal {
         if (_clientChainGatewayLogic == address(0)) {
             revert Errors.ZeroAddress();
         }
+        // selector is 4 bytes long
         if (_clientChainInitializationData.length < 4) {
             revert Errors.BootstrapClientChainDataMalformed();
         }

diff --git a/src/core/ExocoreGateway.sol b/src/core/ExocoreGateway.sol
@@ -98,29 +98,20 @@ contract ExocoreGateway is
         _unpause();
     }
 
-    /// @notice Marks the bootstrap on all chains.
-    /// @dev This function obtains a list of client chain ids from the precompile, and then
-    /// sends a `REQUEST_MARK_BOOTSTRAP` to all of them. In response, the Bootstrap contract
-    /// on those chains should upgrade itself to the ClientChainGateway contract.
-    /// This function should be the first to be called after the LZ infrastructure is ready.
-    // TODO: call this function automatically, either within the initializer (which requires
-    // setPeer) or be triggered by Golang after the contract is deployed.
-    // For manual calls, this function should be called immediately after deployment and
-    // then never needs to be called again.
-    function markBootstrapOnAllChains() public whenNotPaused nonReentrant {
-        (bool success, uint32[] memory chainIndices) = ASSETS_CONTRACT.getClientChains();
-        if (!success) {
-            revert Errors.ExocoreGatewayFailedToGetClientChainIds();
-        }
-        for (uint256 i = 0; i < chainIndices.length; ++i) {
-            uint32 chainIndex = chainIndices[i];
-            if (!chainToBootstrapped[chainIndex]) {
-                _sendInterchainMsg(chainIndex, Action.REQUEST_MARK_BOOTSTRAP, "", true);
-                // TODO: should this be marked only upon receiving a response?
-                chainToBootstrapped[chainIndex] = true;
-                emit BootstrapRequestSent(chainIndex);
-            }
-        }
+    /// @notice Sends a request to mark the bootstrap on a chain.
+    /// @param chainIndex The index of the chain.
+    /// @dev This function is useful if the bootstrap failed on a chain and needs to be retried.
+    function markBootstrap(uint32 chainIndex) public payable whenNotPaused nonReentrant {
+        _markBootstrap(chainIndex);
+    }
+
+    /// @dev Internal function to mark the bootstrap on a chain.
+    /// @param chainIndex The index of the chain.
+    function _markBootstrap(uint32 chainIndex) internal {
+        // we don't track that a request was sent to a chain to allow for retrials
+        // if the transaction fails on the destination chain
+        _sendInterchainMsg(chainIndex, Action.REQUEST_MARK_BOOTSTRAP, "", false);
+        emit BootstrapRequestSent(chainIndex);
     }
 
     /// @inheritdoc IExocoreGateway

diff --git a/src/interfaces/IExocoreGateway.sol b/src/interfaces/IExocoreGateway.sol
@@ -58,4 +58,11 @@ interface IExocoreGateway is IOAppReceiver, IOAppCore {
         string[] calldata metaData
     ) external payable;
 
+    /// @notice Marks the network as bootstrapped, on the client chain.
+    /// @dev Causes an upgrade of the Bootstrap contract to the ClientChainGateway contract.
+    /// @dev Only works if LZ infrastructure is set up and SetPeer has been called.
+    /// @dev This is payable because it requires a fee to be paid to LZ.
+    /// @param clientChainId The LayerZero chain id of the client chain.
+    function markBootstrap(uint32 clientChainId) external payable;
+
 }
diff --git a/src/libraries/Errors.sol b/src/libraries/Errors.sol
@@ -94,12 +94,6 @@ library Errors {
     /// @dev Bootstrap: no ether required for delegation/undelegation
     error BootstrapNoEtherForDelegation();
 
-    /// @dev Bootstrap: not yet in the bootstrap time
-    error BootstrapNotSpawnTime();
-
-    /// @dev Bootstrap: not yet bootstrapped
-    error BootstrapAlreadyBootstrapped();
-
     /// @dev Bootstrap: client chain initialization data is malformed
     error BootstrapClientChainDataMalformed();
 

diff --git a/src/storage/BootstrapStorage.sol b/src/storage/BootstrapStorage.sol
@@ -227,6 +227,15 @@ contract BootstrapStorage is GatewayStorage {
     /// trigger this event.
     event Bootstrapped();
 
+    /// @notice Emitted when a mark bootstrap call is received before the spawn time.
+    /// @dev This event is triggered when a mark bootstrap call is received before the spawn time.
+    event BootstrapNotTimeYet();
+
+    /// @notice Emitted if the bootstrap upgrade to client chain gateway fails.
+    /// @dev This event is triggered if the upgrade from Bootstrap to Client Chain Gateway fails. It is not an error
+    /// intentionally to prevent blocking the system.
+    event BootstrapUpgradeFailed();
+
     /// @notice Emitted when the client chain gateway logic + implementation are updated.
     /// @dev This event is triggered whenever the client chain gateway logic and implementation are updated. It may be
     /// used, before bootstrapping is complete, to upgrade the client chain gateway logic for upgrades or other bugs.

diff --git a/src/storage/ExocoreGatewayStorage.sol b/src/storage/ExocoreGatewayStorage.sol
@@ -43,10 +43,6 @@ contract ExocoreGatewayStorage is GatewayStorage {
     /// @dev The msg.value for all the destination chains.
     uint128 internal constant DESTINATION_MSG_VALUE = 0;
 
-    /// @notice A mapping from client chain IDs to whether the chain has been bootstrapped.
-    /// @dev Used to ensure no repeated bootstrap requests are sent.
-    mapping(uint32 clienChainId => bool) public chainToBootstrapped;
-
     /// @notice Emitted when a precompile call fails.
     /// @param precompile Address of the precompile contract.
     /// @param nonce The LayerZero nonce