From 21197e8980dc9c5c8014178517cec60b04fd47b3 Mon Sep 17 00:00:00 2001
From: Piotr Macek <4007944+piotrm50@users.noreply.github.com>
Date: Wed, 5 Jun 2024 14:58:52 +0200
Subject: [PATCH] Remove blocks that failed to be appended from the ticker.

---
 pkg/protocol/engine/blockdag/events.go               |  5 +++++
 .../engine/blockdag/inmemoryblockdag/blockdag.go     |  1 +
 pkg/protocol/engine/engine.go                        | 12 ++++++++++--
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/pkg/protocol/engine/blockdag/events.go b/pkg/protocol/engine/blockdag/events.go
index ef95f5f70..d6ccd6223 100644
--- a/pkg/protocol/engine/blockdag/events.go
+++ b/pkg/protocol/engine/blockdag/events.go
@@ -3,6 +3,7 @@ package blockdag
 import (
 	"github.com/iotaledger/hive.go/runtime/event"
 	"github.com/iotaledger/iota-core/pkg/protocol/engine/blocks"
+	iotago "github.com/iotaledger/iota.go/v4"
 )
 
 // Events is a collection of Tangle related Events.
@@ -19,6 +20,9 @@ type Events struct {
 	// MissingBlockAppended is triggered when a previously missing Block was appended.
 	MissingBlockAppended *event.Event1[*blocks.Block]
 
+	// BlockNotAppended is triggered when an incoming Block could not be successfully appended.
+	BlockNotAppended *event.Event1[iotago.BlockID]
+
 	// BlockInvalid is triggered when a Block is found to be invalid.
 	BlockInvalid *event.Event2[*blocks.Block, error]
 
@@ -32,6 +36,7 @@ var NewEvents = event.CreateGroupConstructor(func() (newEvents *Events) {
 		BlockSolid:           event.New1[*blocks.Block](),
 		BlockMissing:         event.New1[*blocks.Block](),
 		MissingBlockAppended: event.New1[*blocks.Block](),
+		BlockNotAppended:     event.New1[iotago.BlockID](),
 		BlockInvalid:         event.New2[*blocks.Block, error](),
 	}
 })
diff --git a/pkg/protocol/engine/blockdag/inmemoryblockdag/blockdag.go b/pkg/protocol/engine/blockdag/inmemoryblockdag/blockdag.go
index 0067c0a97..c20df6f91 100644
--- a/pkg/protocol/engine/blockdag/inmemoryblockdag/blockdag.go
+++ b/pkg/protocol/engine/blockdag/inmemoryblockdag/blockdag.go
@@ -47,6 +47,7 @@ func NewProvider(opts ...options.Option[BlockDAG]) module.Provider[*engine.Engin
 
 			e.Events.PreSolidFilter.BlockPreAllowed.Hook(func(block *model.Block) {
 				if _, _, err := b.Append(block); err != nil {
+					b.events.BlockNotAppended.Trigger(block.ID())
 					b.LogError("failed to append block", "blockID", block.ID(), "issuer", block.ProtocolBlock().Header.IssuerID, "err", err)
 				}
 			}, event.WithWorkerPool(wp))
diff --git a/pkg/protocol/engine/engine.go b/pkg/protocol/engine/engine.go
index ca993ed0c..4f5c6f47b 100644
--- a/pkg/protocol/engine/engine.go
+++ b/pkg/protocol/engine/engine.go
@@ -585,15 +585,23 @@ func (e *Engine) setupEvictionState() {
 
 func (e *Engine) setupBlockRequester() {
 	e.Events.BlockRequester.LinkTo(e.BlockRequester.Events)
-
+	wp := e.Workers.CreatePool("BlockRequester", workerpool.WithWorkerCount(1)) // Using just 1 worker to avoid contention
 	// We need to hook to make sure that the request is created before the block arrives to avoid a race condition
 	// where we try to delete the request again before it is created. Thus, continuing to request forever.
 	e.Events.BlockDAG.BlockMissing.Hook(func(block *blocks.Block) {
 		e.BlockRequester.StartTicker(block.ID())
 	})
+
 	e.Events.BlockDAG.MissingBlockAppended.Hook(func(block *blocks.Block) {
 		e.BlockRequester.StopTicker(block.ID())
-	}, event.WithWorkerPool(e.Workers.CreatePool("BlockRequester", workerpool.WithWorkerCount(1)))) // Using just 1 worker to avoid contention
+	}, event.WithWorkerPool(wp))
+
+	// Remove the block from the ticker if it failed to be appended.
+	// It's executed for all blocks to avoid locking twice:
+	// once to check if the block has the ticker and then again to remove it.
+	e.Events.BlockDAG.BlockNotAppended.Hook(func(blockID iotago.BlockID) {
+		e.BlockRequester.StopTicker(blockID)
+	}, event.WithWorkerPool(wp))
 }
 
 func (e *Engine) setupPruning() {