fix: add copilot suggestions

Author: Luisfc68

src/CollateralManagement.sol

@@ -50,6 +50,9 @@ contract CollateralManagementContract is
     /// @param oldFlyoverDiscovery The previous FlyoverDiscovery address
     /// @param newFlyoverDiscovery The new FlyoverDiscovery address
     event FlyoverDiscoverySet(address indexed oldFlyoverDiscovery, address indexed newFlyoverDiscovery);
+    /// @notice Emitted when an error occurs while removing a provider from the FlyoverDiscovery contract
+    /// @param reason The error message
+    event FlyoverDiscoveryError(bytes reason);
     /// @notice Emitted when the minimum collateral is set
     /// @param oldMinCollateral The old minimum collateral
     /// @param newMinCollateral The new minimum collateral
@@ -146,6 +149,7 @@ contract CollateralManagementContract is
         address flyoverDiscovery
     ) external reinitializer(2) onlyRole(DEFAULT_ADMIN_ROLE) {
         if (address(flyoverDiscovery).code.length == 0) revert Flyover.NoContract(flyoverDiscovery);
+        emit FlyoverDiscoverySet(address(_flyoverDiscovery), flyoverDiscovery);
         _flyoverDiscovery = IFlyoverDiscovery(flyoverDiscovery);
     }
 
@@ -390,7 +394,12 @@ contract CollateralManagementContract is
     /// @param providerAddress The address of the provider
     function _notifyCollateralWithdrawal(address providerAddress) private {
         if (address(_flyoverDiscovery) != address(0)) {
-            _flyoverDiscovery.removeProvider(providerAddress);
+            // try / catch  is to avoid blocking the collateral withdrawal if the discovery is misconfigured
+            // solhint-disable-next-line no-empty-blocks
+            try _flyoverDiscovery.removeProvider(providerAddress) {} 
+            catch (bytes memory reason) {
+                emit FlyoverDiscoveryError(reason);
+            }
         }
     }
 

src/FlyoverDiscovery.sol

@@ -80,12 +80,16 @@ contract FlyoverDiscovery is
     // solhint-disable-next-line comprehensive-interface,func-name-mixedcase
     function initializeV2_1_0() external reinitializer(2) onlyRole(DEFAULT_ADMIN_ROLE) {
         Flyover.LiquidityProvider storage lp;
+        bool providerExists;
         for (uint i = 1; i < lastProviderId + 1; ++i) {
             lp = _liquidityProviders[i];
-            if (_collateralManagement.isRegistered(lp.providerType, lp.providerAddress)) {
-                _addActiveProvider(i);
+            providerExists = lp.providerAddress != address(0);
+            if (providerExists) {
                 _registrationStates[lp.providerAddress] = RegistrationState.Approved;
             }
+            if (providerExists && _collateralManagement.isRegistered(lp.providerType, lp.providerAddress)) {
+                _addActiveProvider(i);
+            }
         }
     }
 

test/discovery/ActiveListing.t.sol

@@ -273,6 +273,41 @@ contract ActiveListingTest is DiscoveryTestBase {
         _assertProvidersExclude(lp);
     }
 
+    function test_WithdrawCollateral_SucceedsWhenRemoveProviderReverts()
+        public
+    {
+        deployDiscovery();
+        address lp = makeAddr("revertCallbackLp");
+        _registerAndApprove(
+            lp,
+            "LP",
+            Flyover.ProviderType.PegIn,
+            MIN_COLLATERAL
+        );
+
+        RemoveProviderRevertMock brokenDiscovery = new RemoveProviderRevertMock();
+        vm.prank(owner);
+        collateralManagement.setFlyoverDiscovery(address(brokenDiscovery));
+
+        uint256 balanceBefore = lp.balance;
+
+        vm.prank(lp);
+        collateralManagement.resign();
+        vm.roll(block.number + TEST_RESIGN_DELAY_BLOCKS);
+
+        vm.expectEmit(true, true, false, true);
+        emit CollateralManagementContract.FlyoverDiscoveryError(
+            abi.encodeWithSelector(
+                RemoveProviderRevertMock.RemoveProviderFailed.selector
+            )
+        );
+        vm.prank(lp);
+        collateralManagement.withdrawCollateral();
+
+        assertEq(lp.balance, balanceBefore + MIN_COLLATERAL);
+        assertEq(collateralManagement.getPegInCollateral(lp), 0);
+    }
+
     // ============ Initialization — FlyoverDiscovery ============
 
     function test_InitializeV2_1_0_EmptyStateSucceeds() public {
@@ -411,6 +446,37 @@ contract ActiveListingTest is DiscoveryTestBase {
 
         assertEq(discovery.getProviders().length, 1);
         _assertProvidersContain(keptLp);
+
+        assertEq(
+            uint256(discovery.getRegistrationState(pendingLp)),
+            uint256(IFlyoverDiscovery.RegistrationState.Pending),
+            "pending-only slots must not be backfilled to Approved"
+        );
+
+        assertEq(
+            uint256(discovery.getRegistrationState(resignedLp)),
+            uint256(IFlyoverDiscovery.RegistrationState.Approved),
+            "resigned LPs with discovery records should be marked Approved"
+        );
+        assertEq(discovery.getProvider(resignedLp).providerAddress, resignedLp);
+        assertFalse(
+            discovery.isOperational(Flyover.ProviderType.PegIn, resignedLp)
+        );
+        _assertProvidersExclude(resignedLp);
+
+        assertEq(
+            uint256(discovery.getRegistrationState(withdrawnLp)),
+            uint256(IFlyoverDiscovery.RegistrationState.Approved),
+            "withdrawn LPs with discovery records should be marked Approved"
+        );
+        assertEq(
+            discovery.getProvider(withdrawnLp).providerAddress,
+            withdrawnLp
+        );
+        assertFalse(
+            discovery.isOperational(Flyover.ProviderType.PegIn, withdrawnLp)
+        );
+        _assertProvidersExclude(withdrawnLp);
     }
 
     function test_InitializeV2_1_0_BackfillIgnoresInflatedLastProviderId()
@@ -453,6 +519,11 @@ contract ActiveListingTest is DiscoveryTestBase {
             MIN_COLLATERAL
         );
 
+        vm.expectEmit(true, true, false, true);
+        emit CollateralManagementContract.FlyoverDiscoverySet(
+            address(0),
+            address(discovery)
+        );
         vm.prank(owner);
         collateralManagement.initializeV2_1_0(address(discovery));
 
@@ -537,6 +608,11 @@ contract ActiveListingTest is DiscoveryTestBase {
         cm.initializeV2_1_0(address(discoveryA));
         discoveryA.initializeV2_1_0();
         cm.grantRole(cm.COLLATERAL_ADDER(), address(discoveryB));
+        vm.expectEmit(true, true, false, true);
+        emit CollateralManagementContract.FlyoverDiscoverySet(
+            address(discoveryA),
+            address(discoveryB)
+        );
         cm.setFlyoverDiscovery(address(discoveryB));
         discoveryB.initializeV2_1_0();
         vm.stopPrank();
@@ -677,3 +753,12 @@ contract ActiveListingTest is DiscoveryTestBase {
         assertEq(discovery.getProviders().length, 0);
     }
 }
+
+/// @dev Discovery stand-in that always reverts on removeProvider
+contract RemoveProviderRevertMock {
+    error RemoveProviderFailed();
+
+    function removeProvider(address) external pure {
+        revert RemoveProviderFailed();
+    }
+}