simulators/ethereum/engine: Fix Geth Node Closing

simulators/ethereum/engine/client/node/node.go

@@ -554,7 +554,7 @@ func (n *GethNode) ReOrgBackBlockChain(N uint64, currentBlock *types.Header) (*t
 
 func (n *GethNode) SubscribeP2PEvents() {
 	eventChan := make(chan *p2p.PeerEvent)
-	n.node.Server().SubscribeEvents(eventChan)
+	subscription := n.node.Server().SubscribeEvents(eventChan)
 	for {
 		select {
 		case event := <-eventChan:
@@ -573,6 +573,7 @@ func (n *GethNode) SubscribeP2PEvents() {
 			}
 
 		case <-n.running.Done():
+			subscription.Unsubscribe()
 			return
 		}
 	}

simulators/ethereum/engine/clmock/clmock.go

@@ -156,6 +156,7 @@ func NewCLMocker(t *hivesim.T, slotsToSafe, slotsToFinalized, safeSlotsToImportO
 func (cl *CLMocker) AddEngineClient(ec client.EngineClient) {
 	cl.EngineClientsLock.Lock()
 	defer cl.EngineClientsLock.Unlock()
+	cl.Logf("CLMocker: Adding engine client %v", ec.ID())
 	cl.EngineClients = append(cl.EngineClients, ec)
 }
 
@@ -164,6 +165,7 @@ func (cl *CLMocker) RemoveEngineClient(ec client.EngineClient) {
 	cl.EngineClientsLock.Lock()
 	defer cl.EngineClientsLock.Unlock()
 
+	cl.Logf("CLMocker: Removing engine client %v", ec.ID())
 	for i, engine := range cl.EngineClients {
 		if engine.ID() == ec.ID() {
 			cl.EngineClients = append(cl.EngineClients[:i], cl.EngineClients[i+1:]...)
@@ -176,9 +178,11 @@ func (cl *CLMocker) RemoveEngineClient(ec client.EngineClient) {
 // Close all the engine clients
 func (cl *CLMocker) CloseClients() {
 	for _, engine := range cl.EngineClients {
+		cl.Logf("CLMocker: Closing engine client %v", engine.ID())
 		if err := engine.Close(); err != nil {
 			panic(err)
 		}
+		cl.Logf("CLMocker: Closed engine client %v", engine.ID())
 	}
 }
 

simulators/ethereum/engine/suites/transition/tests.go

@@ -947,10 +947,10 @@ func GenerateMergeTestSpec(mergeTestSpec MergeTestSpec) test.Spec {
 		for i, secondaryClientSpec := range mergeTestSpec.SecondaryClientSpecs {
 			// Start the secondary client with the alternative chain
 			secondaryClient, err := secondaryClientSpec.ClientStarter.StartClient(t.T, t.CLMock.TestContext, t.Genesis, t.ClientParams, t.ClientFiles, t.Engine)
-			t.Logf("INFO (%s): Started secondary client: %v", t.TestName, secondaryClient.ID())
 			if err != nil {
 				t.Fatalf("FAIL (%s): Unable to start secondary client: %v", t.TestName, err)
 			}
+			t.Logf("INFO (%s): Started secondary client: %v", t.TestName, secondaryClient.ID())
 			defer t.HandleClientPostRunVerification(secondaryClient)
 			secondaryClients[i] = ClientSpec{
 				Client: secondaryClient,
@@ -961,6 +961,15 @@ func GenerateMergeTestSpec(mergeTestSpec MergeTestSpec) test.Spec {
 		// Start a secondary clients with alternative PoW chains
 		for _, cs := range secondaryClients {
 			if cs.Spec.SkipAddingToCLMocker {
+				// This client is not added to the CLMocker, so we don't need to
+				// wait for it to reach TTD, but we still need to close it
+				defer func(c client.EngineClient) {
+					t.Logf("INFO (%s): Closing secondary engine client: %v", t.TestName, c.ID())
+					if err := c.Close(); err != nil {
+						t.Logf("WARN (%s): Error while closing engine client: %v", t.TestName, err)
+					}
+					t.Logf("INFO (%s): Closed secondary engine client: %v", t.TestName, c.ID())
+				}(cs.Client)
 				continue
 			}