From 5f82eb92924433c3f793950dee3dc30f98479189 Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Tue, 12 Sep 2023 08:49:21 -0400
Subject: [PATCH 01/25] load test WIP

---
 engine/common/synchronization/config.go       |  1 +
 .../synchronization/engine_spam_test.go       | 62 +++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/engine/common/synchronization/config.go b/engine/common/synchronization/config.go
index 6df2b2c2167..477ce05d08c 100644
--- a/engine/common/synchronization/config.go
+++ b/engine/common/synchronization/config.go
@@ -44,6 +44,7 @@ const spamProbabilityMultiplier = 1001
 type SpamDetectionConfig struct {
 	syncRequestProbability float32
 	rangeRequestBaseProb   float32
+	batchRequestBaseProb   float32
 }
 
 func NewSpamDetectionConfig() *SpamDetectionConfig {
diff --git a/engine/common/synchronization/engine_spam_test.go b/engine/common/synchronization/engine_spam_test.go
index 313f02dd9ff..48694189f0d 100644
--- a/engine/common/synchronization/engine_spam_test.go
+++ b/engine/common/synchronization/engine_spam_test.go
@@ -3,6 +3,7 @@ package synchronization
 import (
 	"context"
 	"fmt"
+	"github.com/onflow/flow-go/model/flow"
 	"testing"
 	"time"
 
@@ -257,3 +258,64 @@ func (ss *SyncSuite) TestLoad_Process_RangeRequest_SometimesReportSpam() {
 		misbehaviorsCounter = 0 // reset counter for next subtest
 	}
 }
+
+// TestLoad_Process_BatchRequest_SometimesReportSpam is a load test that ensures that a misbehavior report is generated
+// an appropriate range of times when the base probability factor and range are set to different values.
+func (ss *SyncSuite) TestLoad_Process_BatchRequest_SometimesReportSpam() {
+	ctx, cancel := irrecoverable.NewMockSignalerContextWithCancel(ss.T(), context.Background())
+	ss.e.Start(ctx)
+	unittest.AssertClosesBefore(ss.T(), ss.e.Ready(), time.Second)
+	defer cancel()
+
+	load := 1000
+
+	type loadGroup struct {
+		batchRequestBaseProb      float32
+		expectedMisbehaviorsLower int
+		expectedMisbehaviorsUpper int
+		fromHeight                uint64
+		toHeight                  uint64
+	}
+
+	loadGroups := []loadGroup{}
+
+	// reset misbehavior report counter for each subtest
+	misbehaviorsCounter := 0
+
+	for _, loadGroup := range loadGroups {
+		for i := 0; i < load; i++ {
+			ss.T().Log("load iteration", i)
+
+			nonce, err := rand.Uint64()
+			require.NoError(ss.T(), err, "should generate nonce")
+
+			// generate origin and request message
+			originID := unittest.IdentifierFixture()
+			req := &messages.BatchRequest{
+				Nonce:    nonce,
+				BlockIDs: make([]flow.Identifier, 0),
+			}
+
+			// count misbehavior reports over the course of a load test
+			ss.con.On("ReportMisbehavior", mock.Anything).Return(mock.Anything).Maybe().Run(
+				func(args mock.Arguments) {
+					misbehaviorsCounter++
+				},
+			)
+			ss.e.spamDetectionConfig.batchRequestBaseProb = loadGroup.batchRequestBaseProb
+			require.NoError(ss.T(), ss.e.Process(channels.SyncCommittee, originID, req))
+		}
+		// check function call expectations at the end of the load test; otherwise, load test would take much longer
+		ss.core.AssertExpectations(ss.T())
+		ss.con.AssertExpectations(ss.T())
+
+		// check that correct range of misbehavior reports were generated
+		// since we're using a probabilistic approach to generate misbehavior reports, we can't guarantee the exact number,
+		// so we check that it's within an expected range
+		ss.T().Logf("misbehaviors counter after load test: %d (expected lower bound: %d expected upper bound: %d)", misbehaviorsCounter, loadGroup.expectedMisbehaviorsLower, loadGroup.expectedMisbehaviorsUpper)
+		assert.GreaterOrEqual(ss.T(), misbehaviorsCounter, loadGroup.expectedMisbehaviorsLower)
+		assert.LessOrEqual(ss.T(), misbehaviorsCounter, loadGroup.expectedMisbehaviorsUpper)
+
+		misbehaviorsCounter = 0 // reset counter for next subtest
+	}
+}

From db67457e2d6b161360c3eaa3a5fde1988e494051 Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Wed, 20 Sep 2023 02:23:34 -0500
Subject: [PATCH 02/25] godocs update

---
 engine/common/synchronization/config.go           | 8 ++++++--
 engine/common/synchronization/engine_spam_test.go | 4 ++++
 model/messages/synchronization.go                 | 6 ++++++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/engine/common/synchronization/config.go b/engine/common/synchronization/config.go
index 3d25db9b338..e22252f08e9 100644
--- a/engine/common/synchronization/config.go
+++ b/engine/common/synchronization/config.go
@@ -46,8 +46,12 @@ const spamProbabilityMultiplier = 1001
 // message types.
 type SpamDetectionConfig struct {
 
-  batchRequestBaseProb   float32
-  
+	// batchRequestBaseProb is the base probability that's used in creating the final probability of creating a
+	// misbehavior report for a BatchRequest message. This is why the word "base" is used in the name of this field,
+	// since it's not the final probability and there are other factors that determine the final probability.
+	// The reason for this is that we want to increase the probability of creating a misbehavior report for a large batch.
+	batchRequestBaseProb float32
+
 	// syncRequestProb is the probability of creating a misbehavior report for a SyncRequest message.
 	syncRequestProb float32
 
diff --git a/engine/common/synchronization/engine_spam_test.go b/engine/common/synchronization/engine_spam_test.go
index 740c919ce04..015b18c0c73 100644
--- a/engine/common/synchronization/engine_spam_test.go
+++ b/engine/common/synchronization/engine_spam_test.go
@@ -280,6 +280,10 @@ func (ss *SyncSuite) TestLoad_Process_BatchRequest_SometimesReportSpam() {
 
 	load := 1000
 
+	// each load test is a load group that contains a set of factors with unique values to test how many misbehavior reports are generated.
+	// Due to the probabilistic nature of how misbehavior reports are generated, we use an expected lower and
+	// upper range of expected misbehaviors to determine if the load test passed or failed. As long as the number of misbehavior reports
+	// falls within the expected range, the load test passes.
 	type loadGroup struct {
 		batchRequestBaseProb      float32
 		expectedMisbehaviorsLower int
diff --git a/model/messages/synchronization.go b/model/messages/synchronization.go
index 8f59468f63e..6f10c835c92 100644
--- a/model/messages/synchronization.go
+++ b/model/messages/synchronization.go
@@ -8,6 +8,8 @@ import (
 // SyncRequest is part of the synchronization protocol and represents a node on
 // the network sharing the height of its latest finalized block and requesting
 // the same information from the recipient.
+// All SyncRequest messages are validated before being processed. If validation fails, then a misbehavior report is created.
+// See synchronization.validateSyncRequestForALSP for more details.
 type SyncRequest struct {
 	Nonce  uint64
 	Height uint64
@@ -25,6 +27,8 @@ type SyncResponse struct {
 // (pulling) attempt to synchronize with the consensus state of the network. It
 // requests finalized blocks by a range of block heights, including from and to
 // heights.
+// All RangeRequest messages are validated before being processed. If validation fails, then a misbehavior report is created.
+// See synchronization.validateRangeRequestForALSP for more details.
 type RangeRequest struct {
 	Nonce      uint64
 	FromHeight uint64
@@ -34,6 +38,8 @@ type RangeRequest struct {
 // BatchRequest is part of the synchronization protocol and represents an active
 // (pulling) attempt to synchronize with the consensus state of the network. It
 // requests finalized or unfinalized blocks by a list of block IDs.
+// All BatchRequest messages are validated before being processed. If validation fails, then a misbehavior report is created.
+// See synchronization.validateBatchRequestForALSP for more details.
 type BatchRequest struct {
 	Nonce    uint64
 	BlockIDs []flow.Identifier

From 43a1c59c11e3223e5fe6276a15c661e6916afd17 Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Wed, 20 Sep 2023 03:05:36 -0500
Subject: [PATCH 03/25] load test 1 implemented - 0 block IDs

---
 engine/common/synchronization/engine.go       | 51 ++++++++++++++++---
 .../synchronization/engine_spam_test.go       |  8 ++-
 2 files changed, 50 insertions(+), 9 deletions(-)

diff --git a/engine/common/synchronization/engine.go b/engine/common/synchronization/engine.go
index e7266c694a2..e9f8ef721aa 100644
--- a/engine/common/synchronization/engine.go
+++ b/engine/common/synchronization/engine.go
@@ -208,7 +208,7 @@ func (e *Engine) Process(channel channels.Channel, originID flow.Identifier, eve
 func (e *Engine) process(channel channels.Channel, originID flow.Identifier, event interface{}) error {
 	switch message := event.(type) {
 	case *messages.BatchRequest:
-		report, valid, err := e.validateBatchRequestForALSP(channel, originID, message)
+		report, valid, err := e.validateBatchRequestForALSP(originID, message)
 		if err != nil {
 			return fmt.Errorf("failed to validate batch request from %x: %w", originID[:], err)
 		}
@@ -509,8 +509,40 @@ func (e *Engine) sendRequests(participants flow.IdentifierList, ranges []chainsy
 	}
 }
 
-// TODO: implement spam reporting similar to validateSyncRequestForALSP
-func (e *Engine) validateBatchRequestForALSP(channel channels.Channel, id flow.Identifier, batchRequest *messages.BatchRequest) (*alsp.MisbehaviorReport, bool, error) {
+// validateBatchRequestForALSP checks if a batch request should be reported as a misbehavior due to malicious intent (e.g. spamming).
+// It returns a misbehavior report and a boolean indicating whether validation passed, as well as an error.
+// Returns an error that is assumed to be irrecoverable because of internal processes that didn't allow validation to complete.
+// Returns true if the batch request is valid and should not be reported as misbehavior.
+// Returns false if either
+// a) the batch request is invalid or
+// b) the batch request is valid but should be reported as misbehavior anyway (due to probabilities) or
+// c) an error is encountered.
+func (e *Engine) validateBatchRequestForALSP(originID flow.Identifier, batchRequest *messages.BatchRequest) (*alsp.MisbehaviorReport, bool, error) {
+	// Generate a random integer between 1 and spamProbabilityMultiplier (exclusive)
+	_, err := rand.Uint32n(spamProbabilityMultiplier)
+
+	if err != nil {
+		return nil, false, fmt.Errorf("failed to generate random number from %x: %w", originID[:], err)
+	}
+
+	// check if range request is valid
+	if len(batchRequest.BlockIDs) == 0 {
+		e.log.Warn().
+			Hex("origin_id", logging.ID(originID)).
+			Str(logging.KeySuspicious, "true").
+			Str("reason", alsp.InvalidMessage.String()).
+			Msg("received invalid batch request with 0 block IDs, creating ALSP report")
+		report, err := alsp.NewMisbehaviorReport(originID, alsp.InvalidMessage)
+
+		if err != nil {
+			// failing to create the misbehavior report is unlikely. If an error is encountered while
+			// creating the misbehavior report it indicates a bug and processing can not proceed.
+			return nil, false, fmt.Errorf("failed to create misbehavior report (invalid range request) from %x: %w", originID[:], err)
+		}
+		// failed validation check and should be reported as misbehavior
+		return report, false, nil
+	}
+
 	return nil, true, nil
 }
 
@@ -519,11 +551,14 @@ func (e *Engine) validateBlockResponseForALSP(channel channels.Channel, id flow.
 	return nil, true, nil
 }
 
-// validateRangeRequestForALSP checks if a range request should be reported as a misbehavior.
+// validateRangeRequestForALSP checks if a range request should be reported as a misbehavior due to malicious intent (e.g. spamming).
 // It returns a misbehavior report and a boolean indicating whether validation passed, as well as an error.
 // Returns an error that is assumed to be irrecoverable because of internal processes that didn't allow validation to complete.
 // Returns true if the range request is valid and should not be reported as misbehavior.
-// Returns false if either a) the range request is invalid or b) the range request is valid but should be reported as misbehavior anyway (due to probabilities) or c) an error is encountered.
+// Returns false if either
+// a) the range request is invalid or
+// b) the range request is valid but should be reported as misbehavior anyway (due to probabilities) or
+// c) an error is encountered.
 func (e *Engine) validateRangeRequestForALSP(originID flow.Identifier, rangeRequest *messages.RangeRequest) (*alsp.MisbehaviorReport, bool, error) {
 	// Generate a random integer between 1 and spamProbabilityMultiplier (exclusive)
 	n, err := rand.Uint32n(spamProbabilityMultiplier)
@@ -586,11 +621,13 @@ func (e *Engine) validateRangeRequestForALSP(originID flow.Identifier, rangeRequ
 	return nil, true, nil
 }
 
-// validateSyncRequestForALSP checks if a sync request should be reported as a misbehavior.
+// validateSyncRequestForALSP checks if a sync request should be reported as a misbehavior due to malicious intent (e.g. spamming).
 // It returns a misbehavior report and a boolean indicating whether validation passed, as well as an error.
 // Returns an error that is assumed to be irrecoverable because of internal processes that didn't allow validation to complete.
 // Returns true if passed validation.
-// Returns false if either a) failed validation (due to probabilities) or b) an error is encountered.
+// Returns false if either
+// a) failed validation (due to probabilities) or
+// b) an error is encountered.
 func (e *Engine) validateSyncRequestForALSP(originID flow.Identifier) (*alsp.MisbehaviorReport, bool, error) {
 	// Generate a random integer between 1 and spamProbabilityMultiplier (exclusive)
 	n, err := rand.Uint32n(spamProbabilityMultiplier)
diff --git a/engine/common/synchronization/engine_spam_test.go b/engine/common/synchronization/engine_spam_test.go
index 015b18c0c73..ce13d13ae61 100644
--- a/engine/common/synchronization/engine_spam_test.go
+++ b/engine/common/synchronization/engine_spam_test.go
@@ -288,12 +288,16 @@ func (ss *SyncSuite) TestLoad_Process_BatchRequest_SometimesReportSpam() {
 		batchRequestBaseProb      float32
 		expectedMisbehaviorsLower int
 		expectedMisbehaviorsUpper int
-		fromHeight                uint64
-		toHeight                  uint64
+		blockIDs                  []flow.Identifier
 	}
 
 	loadGroups := []loadGroup{}
 
+	// ALWAYS REPORT SPAM FOR INVALID BATCH REQUESTS OR BATCH REQUESTS THAT ARE FAR OUTSIDE OF THE TOLERANCE
+
+	// using an empty batch request always results in a misbehavior report, no matter how small the base probability factor is
+	loadGroups = append(loadGroups, loadGroup{0.001, 1000, 1000, []flow.Identifier{}})
+
 	// reset misbehavior report counter for each subtest
 	misbehaviorsCounter := 0
 

From ade2b0bafe51fd6d9c89ee9177be7876a86416c8 Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Wed, 20 Sep 2023 04:05:42 -0500
Subject: [PATCH 04/25] load test 2 - unknown blocks

---
 engine/common/synchronization/engine.go       | 29 +++++++++++++++++--
 .../synchronization/engine_spam_test.go       |  5 +++-
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/engine/common/synchronization/engine.go b/engine/common/synchronization/engine.go
index e9f8ef721aa..d09041b7073 100644
--- a/engine/common/synchronization/engine.go
+++ b/engine/common/synchronization/engine.go
@@ -4,6 +4,7 @@ package synchronization
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"time"
 
@@ -525,7 +526,7 @@ func (e *Engine) validateBatchRequestForALSP(originID flow.Identifier, batchRequ
 		return nil, false, fmt.Errorf("failed to generate random number from %x: %w", originID[:], err)
 	}
 
-	// check if range request is valid
+	// validity check #1: if no block IDs
 	if len(batchRequest.BlockIDs) == 0 {
 		e.log.Warn().
 			Hex("origin_id", logging.ID(originID)).
@@ -537,12 +538,36 @@ func (e *Engine) validateBatchRequestForALSP(originID flow.Identifier, batchRequ
 		if err != nil {
 			// failing to create the misbehavior report is unlikely. If an error is encountered while
 			// creating the misbehavior report it indicates a bug and processing can not proceed.
-			return nil, false, fmt.Errorf("failed to create misbehavior report (invalid range request) from %x: %w", originID[:], err)
+			return nil, false, fmt.Errorf("failed to create misbehavior report (invalid batch request, no block IDs) from %x: %w", originID[:], err)
 		}
 		// failed validation check and should be reported as misbehavior
 		return report, false, nil
 	}
 
+	// validity check #2: if any block ID is unknown
+	for _, blockID := range batchRequest.BlockIDs {
+		_, err := e.requestHandler.blocks.ByID(blockID)
+		if errors.Is(err, storage.ErrNotFound) {
+			//logger.Debug().Hex("block_id", blockID[:]).Msg("skipping unknown block")
+			//continue
+
+			e.log.Warn().
+				Hex("origin_id", logging.ID(originID)).
+				Str(logging.KeySuspicious, "true").
+				Str("reason", alsp.InvalidMessage.String()).
+				Msgf("received invalid batch request with block ID %x that does not exist, creating ALSP report", blockID[:])
+			report, err := alsp.NewMisbehaviorReport(originID, alsp.InvalidMessage)
+
+			if err != nil {
+				// failing to create the misbehavior report is unlikely. If an error is encountered while
+				// creating the misbehavior report it indicates a bug and processing can not proceed.
+				return nil, false, fmt.Errorf("failed to create misbehavior report (invalid batch request, unknown block ID) from %x: %w", originID[:], err)
+			}
+			return report, false, nil
+		}
+
+	}
+
 	return nil, true, nil
 }
 
diff --git a/engine/common/synchronization/engine_spam_test.go b/engine/common/synchronization/engine_spam_test.go
index ce13d13ae61..0f5707006f0 100644
--- a/engine/common/synchronization/engine_spam_test.go
+++ b/engine/common/synchronization/engine_spam_test.go
@@ -298,6 +298,9 @@ func (ss *SyncSuite) TestLoad_Process_BatchRequest_SometimesReportSpam() {
 	// using an empty batch request always results in a misbehavior report, no matter how small the base probability factor is
 	loadGroups = append(loadGroups, loadGroup{0.001, 1000, 1000, []flow.Identifier{}})
 
+	// using a batch request with missing block ID should always result in a misbehavior report, no matter how small the base probability factor is
+	loadGroups = append(loadGroups, loadGroup{0.001, 1000, 1000, unittest.IdentifierListFixture(100)})
+
 	// reset misbehavior report counter for each subtest
 	misbehaviorsCounter := 0
 
@@ -312,7 +315,7 @@ func (ss *SyncSuite) TestLoad_Process_BatchRequest_SometimesReportSpam() {
 			originID := unittest.IdentifierFixture()
 			req := &messages.BatchRequest{
 				Nonce:    nonce,
-				BlockIDs: make([]flow.Identifier, 0),
+				BlockIDs: loadGroup.blockIDs,
 			}
 
 			// count misbehavior reports over the course of a load test

From 8ad805a0a2281d355f07ed6b8f62e37d7b8ae0e6 Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Wed, 20 Sep 2023 04:06:43 -0500
Subject: [PATCH 05/25] lint fix

---
 engine/common/synchronization/engine_spam_test.go | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/engine/common/synchronization/engine_spam_test.go b/engine/common/synchronization/engine_spam_test.go
index 0f5707006f0..6caf410bab9 100644
--- a/engine/common/synchronization/engine_spam_test.go
+++ b/engine/common/synchronization/engine_spam_test.go
@@ -3,10 +3,11 @@ package synchronization
 import (
 	"context"
 	"fmt"
-	"github.com/onflow/flow-go/model/flow"
 	"testing"
 	"time"
 
+	"github.com/onflow/flow-go/model/flow"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"

From 9b4f52947d8efefb6904041f11a1fcabcdeb5b64 Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Thu, 21 Sep 2023 02:19:41 -0500
Subject: [PATCH 06/25] core implementation, load test

---
 engine/common/synchronization/engine.go       | 53 +++++++++++--------
 .../synchronization/engine_spam_test.go       | 13 ++++-
 2 files changed, 41 insertions(+), 25 deletions(-)

diff --git a/engine/common/synchronization/engine.go b/engine/common/synchronization/engine.go
index d09041b7073..a93af454906 100644
--- a/engine/common/synchronization/engine.go
+++ b/engine/common/synchronization/engine.go
@@ -4,7 +4,6 @@ package synchronization
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"time"
 
@@ -520,13 +519,13 @@ func (e *Engine) sendRequests(participants flow.IdentifierList, ranges []chainsy
 // c) an error is encountered.
 func (e *Engine) validateBatchRequestForALSP(originID flow.Identifier, batchRequest *messages.BatchRequest) (*alsp.MisbehaviorReport, bool, error) {
 	// Generate a random integer between 1 and spamProbabilityMultiplier (exclusive)
-	_, err := rand.Uint32n(spamProbabilityMultiplier)
+	n, err := rand.Uint32n(spamProbabilityMultiplier)
 
 	if err != nil {
 		return nil, false, fmt.Errorf("failed to generate random number from %x: %w", originID[:], err)
 	}
 
-	// validity check #1: if no block IDs
+	// validity check: if no block IDs, always report as misbehavior
 	if len(batchRequest.BlockIDs) == 0 {
 		e.log.Warn().
 			Hex("origin_id", logging.ID(originID)).
@@ -544,28 +543,36 @@ func (e *Engine) validateBatchRequestForALSP(originID flow.Identifier, batchRequ
 		return report, false, nil
 	}
 
-	// validity check #2: if any block ID is unknown
-	for _, blockID := range batchRequest.BlockIDs {
-		_, err := e.requestHandler.blocks.ByID(blockID)
-		if errors.Is(err, storage.ErrNotFound) {
-			//logger.Debug().Hex("block_id", blockID[:]).Msg("skipping unknown block")
-			//continue
+	// to avoid creating a misbehavior report for every batch request received, use a probabilistic approach.
+	// The larger the batch request and base probability, the higher the probability of creating a misbehavior report.
 
-			e.log.Warn().
-				Hex("origin_id", logging.ID(originID)).
-				Str(logging.KeySuspicious, "true").
-				Str("reason", alsp.InvalidMessage.String()).
-				Msgf("received invalid batch request with block ID %x that does not exist, creating ALSP report", blockID[:])
-			report, err := alsp.NewMisbehaviorReport(originID, alsp.InvalidMessage)
-
-			if err != nil {
-				// failing to create the misbehavior report is unlikely. If an error is encountered while
-				// creating the misbehavior report it indicates a bug and processing can not proceed.
-				return nil, false, fmt.Errorf("failed to create misbehavior report (invalid batch request, unknown block ID) from %x: %w", originID[:], err)
-			}
-			return report, false, nil
-		}
+	// batchRequestProb is calculated as follows:
+	// batchRequestBaseProb * (len(batchRequest.BlockIDs) + 1) / synccore.DefaultConfig().MaxSize
+	// Example 1 (small batch of block IDs) if the batch request is for 10 blocks IDs and batchRequestBaseProb is 0.01, then the probability of
+	// creating a misbehavior report is:
+	// batchRequestBaseProb * (10+1) / synccore.DefaultConfig().MaxSize
+	// = 0.01 * 11 / 64 = 0.00171875 = 0.171875%
+	// Example 2 (large batch of block IDs) if the batch request is for 1000 block IDs and batchRequestBaseProb is 0.01, then the probability of
+	// creating a misbehavior report is:
+	// batchRequestBaseProb * (1000+1) / synccore.DefaultConfig().MaxSize
+	// = 0.01 * 1001 / 64 = 0.15640625 = 15.640625%
+	batchRequestProb := e.spamDetectionConfig.batchRequestBaseProb * (float32(len(batchRequest.BlockIDs)) + 1) / float32(synccore.DefaultConfig().MaxSize)
+	if float32(n) < batchRequestProb*spamProbabilityMultiplier {
+		// create a misbehavior report
+		e.log.Warn().
+			Hex("origin_id", logging.ID(originID)).
+			Str(logging.KeySuspicious, "true").
+			Str("reason", alsp.ResourceIntensiveRequest.String()).
+			Msgf("for %d block IDs, creating probabilistic ALSP report", len(batchRequest.BlockIDs))
+		report, err := alsp.NewMisbehaviorReport(originID, alsp.ResourceIntensiveRequest)
 
+		if err != nil {
+			// failing to create the misbehavior report is unlikely. If an error is encountered while
+			// creating the misbehavior report it indicates a bug and processing can not proceed.
+			return nil, false, fmt.Errorf("failed to create misbehavior report from %x: %w", originID[:], err)
+		}
+		// failed validation check and should be reported as misbehavior
+		return report, false, nil
 	}
 
 	return nil, true, nil
diff --git a/engine/common/synchronization/engine_spam_test.go b/engine/common/synchronization/engine_spam_test.go
index 6caf410bab9..9ff457afac0 100644
--- a/engine/common/synchronization/engine_spam_test.go
+++ b/engine/common/synchronization/engine_spam_test.go
@@ -299,8 +299,9 @@ func (ss *SyncSuite) TestLoad_Process_BatchRequest_SometimesReportSpam() {
 	// using an empty batch request always results in a misbehavior report, no matter how small the base probability factor is
 	loadGroups = append(loadGroups, loadGroup{0.001, 1000, 1000, []flow.Identifier{}})
 
-	// using a batch request with missing block ID should always result in a misbehavior report, no matter how small the base probability factor is
-	loadGroups = append(loadGroups, loadGroup{0.001, 1000, 1000, unittest.IdentifierListFixture(100)})
+	// using a very large batch request (999 block IDs) with a 10% base probability factor, expect to get misbehavior report 100% of the time (1000 in 1000 requests)
+	// expected probability factor: 0.1 * ((999 + 1)/64 = 1.5625
+	loadGroups = append(loadGroups, loadGroup{0.1, 1000, 1000, repeatedBlockIDs(unittest.BlockFixture().ID(), 999)})
 
 	// reset misbehavior report counter for each subtest
 	misbehaviorsCounter := 0
@@ -342,3 +343,11 @@ func (ss *SyncSuite) TestLoad_Process_BatchRequest_SometimesReportSpam() {
 		misbehaviorsCounter = 0 // reset counter for next subtest
 	}
 }
+
+func repeatedBlockIDs(value flow.Identifier, n int) []flow.Identifier {
+	arr := make([]flow.Identifier, n)
+	for i := 0; i < n; i++ {
+		arr[i] = value
+	}
+	return arr
+}

From 5296f6e1fd21f052c352bd3cf92d3c7df0509207 Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Thu, 21 Sep 2023 02:40:02 -0500
Subject: [PATCH 07/25] added remaining load tests

---
 .../synchronization/engine_spam_test.go       | 32 ++++++++++++++++---
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/engine/common/synchronization/engine_spam_test.go b/engine/common/synchronization/engine_spam_test.go
index 9ff457afac0..b94157aa893 100644
--- a/engine/common/synchronization/engine_spam_test.go
+++ b/engine/common/synchronization/engine_spam_test.go
@@ -272,7 +272,7 @@ func (ss *SyncSuite) TestLoad_Process_RangeRequest_SometimesReportSpam() {
 }
 
 // TestLoad_Process_BatchRequest_SometimesReportSpam is a load test that ensures that a misbehavior report is generated
-// an appropriate range of times when the base probability factor and range are set to different values.
+// an appropriate range of times when the base probability factor and number of block IDs are set to different values.
 func (ss *SyncSuite) TestLoad_Process_BatchRequest_SometimesReportSpam() {
 	ctx, cancel := irrecoverable.NewMockSignalerContextWithCancel(ss.T(), context.Background())
 	ss.e.Start(ctx)
@@ -294,14 +294,34 @@ func (ss *SyncSuite) TestLoad_Process_BatchRequest_SometimesReportSpam() {
 
 	loadGroups := []loadGroup{}
 
+	// using a very small batch request (1 block ID) with a 10% base probability factor, expect to almost never get misbehavior report, about 0.003% of the time (3 in 1000 requests)
+	// expected probability factor: 0.1 * ((10-9) + 1)/64 = 0.003125
+	loadGroups = append(loadGroups, loadGroup{0.1, 0, 15, repeatedBlockIDs(1)})
+
+	// using a small batch request (10 block IDs) with a 10% base probability factor, expect to get misbehavior report about 1.7% of the time (17 in 1000 requests)
+	// expected probability factor: 0.1 * ((11-1) + 1)/64 = 0.0171875
+	loadGroups = append(loadGroups, loadGroup{0.1, 5, 31, repeatedBlockIDs(10)})
+
+	// using a large batch request (99 block IDs) with a 10% base probability factor, expect to get misbehavior report about 15% of the time (150 in 1000 requests)
+	// expected probability factor: 0.1 * ((100-1) + 1)/64 = 0.15625
+	loadGroups = append(loadGroups, loadGroup{0.1, 110, 200, repeatedBlockIDs(99)})
+
+	// using a small batch request (10 block IDs) with a 1% base probability factor, expect to almost never get misbehavior report, about 0.17% of the time (2 in 1000 requests)
+	// expected probability factor: 0.01 * ((11-1) + 1)/64 = 0.00171875
+	loadGroups = append(loadGroups, loadGroup{0.01, 0, 7, repeatedBlockIDs(10)})
+
+	// using a very large batch request (999 block IDs) with a 1% base probability factor, expect to get misbehavior report about 15% of the time (150 in 1000 requests)
+	// expected probability factor: 0.01 * ((1000-1) + 1)/64 = 0.15625
+	loadGroups = append(loadGroups, loadGroup{0.01, 110, 200, repeatedBlockIDs(999)})
+
 	// ALWAYS REPORT SPAM FOR INVALID BATCH REQUESTS OR BATCH REQUESTS THAT ARE FAR OUTSIDE OF THE TOLERANCE
 
-	// using an empty batch request always results in a misbehavior report, no matter how small the base probability factor is
+	// using an empty batch request (0 block IDs) always results in a misbehavior report, no matter how small the base probability factor is
 	loadGroups = append(loadGroups, loadGroup{0.001, 1000, 1000, []flow.Identifier{}})
 
 	// using a very large batch request (999 block IDs) with a 10% base probability factor, expect to get misbehavior report 100% of the time (1000 in 1000 requests)
 	// expected probability factor: 0.1 * ((999 + 1)/64 = 1.5625
-	loadGroups = append(loadGroups, loadGroup{0.1, 1000, 1000, repeatedBlockIDs(unittest.BlockFixture().ID(), 999)})
+	loadGroups = append(loadGroups, loadGroup{0.1, 1000, 1000, repeatedBlockIDs(999)})
 
 	// reset misbehavior report counter for each subtest
 	misbehaviorsCounter := 0
@@ -344,10 +364,12 @@ func (ss *SyncSuite) TestLoad_Process_BatchRequest_SometimesReportSpam() {
 	}
 }
 
-func repeatedBlockIDs(value flow.Identifier, n int) []flow.Identifier {
+func repeatedBlockIDs(n int) []flow.Identifier {
+	blockID := unittest.BlockFixture().ID()
+
 	arr := make([]flow.Identifier, n)
 	for i := 0; i < n; i++ {
-		arr[i] = value
+		arr[i] = blockID
 	}
 	return arr
 }

From 71aec4464211b3ea9f667773c679fc2c83477eeb Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Tue, 3 Oct 2023 06:28:31 -0500
Subject: [PATCH 08/25] validateBatchRequestForALSP() godoc update

---
 engine/common/synchronization/engine.go | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/engine/common/synchronization/engine.go b/engine/common/synchronization/engine.go
index a93af454906..fc0b81630c8 100644
--- a/engine/common/synchronization/engine.go
+++ b/engine/common/synchronization/engine.go
@@ -510,13 +510,17 @@ func (e *Engine) sendRequests(participants flow.IdentifierList, ranges []chainsy
 }
 
 // validateBatchRequestForALSP checks if a batch request should be reported as a misbehavior due to malicious intent (e.g. spamming).
-// It returns a misbehavior report and a boolean indicating whether validation passed, as well as an error.
-// Returns an error that is assumed to be irrecoverable because of internal processes that didn't allow validation to complete.
-// Returns true if the batch request is valid and should not be reported as misbehavior.
-// Returns false if either
-// a) the batch request is invalid or
-// b) the batch request is valid but should be reported as misbehavior anyway (due to probabilities) or
-// c) an error is encountered.
+// Args:
+// - originID: the sender of the batch request
+// - batchRequest: the batch request to validate
+// Returns:
+// - *alsp.MisbehaviorReport: If any misbehavior is detected such as;
+//   - the batch request is invalid
+//   - the batch request is valid but should be reported as misbehavior anyway (due to probabilities)
+//
+// - bool: true if the batch request is valid and should not be reported as misbehavior; otherwise, false if a misbehavior is detected
+//
+// - error: If an error is encountered while validating the batch request. Error is assumed to be irrecoverable because of internal processes that didn't allow validation to complete.
 func (e *Engine) validateBatchRequestForALSP(originID flow.Identifier, batchRequest *messages.BatchRequest) (*alsp.MisbehaviorReport, bool, error) {
 	// Generate a random integer between 1 and spamProbabilityMultiplier (exclusive)
 	n, err := rand.Uint32n(spamProbabilityMultiplier)

From 6999004a1f43e6a6376442b8f4b28b55bb01bcb0 Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Tue, 3 Oct 2023 06:43:08 -0500
Subject: [PATCH 09/25] godoc updates - validateRangeRequestForALSP,
 validateSyncRequestForALSP,

---
 engine/common/synchronization/engine.go | 35 +++++++++++++++----------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/engine/common/synchronization/engine.go b/engine/common/synchronization/engine.go
index fc0b81630c8..fe4af9c55b8 100644
--- a/engine/common/synchronization/engine.go
+++ b/engine/common/synchronization/engine.go
@@ -514,7 +514,7 @@ func (e *Engine) sendRequests(participants flow.IdentifierList, ranges []chainsy
 // - originID: the sender of the batch request
 // - batchRequest: the batch request to validate
 // Returns:
-// - *alsp.MisbehaviorReport: If any misbehavior is detected such as;
+// - *alsp.MisbehaviorReport: If any misbehavior is detected such as:
 //   - the batch request is invalid
 //   - the batch request is valid but should be reported as misbehavior anyway (due to probabilities)
 //
@@ -588,13 +588,17 @@ func (e *Engine) validateBlockResponseForALSP(channel channels.Channel, id flow.
 }
 
 // validateRangeRequestForALSP checks if a range request should be reported as a misbehavior due to malicious intent (e.g. spamming).
-// It returns a misbehavior report and a boolean indicating whether validation passed, as well as an error.
-// Returns an error that is assumed to be irrecoverable because of internal processes that didn't allow validation to complete.
-// Returns true if the range request is valid and should not be reported as misbehavior.
-// Returns false if either
-// a) the range request is invalid or
-// b) the range request is valid but should be reported as misbehavior anyway (due to probabilities) or
-// c) an error is encountered.
+// Args:
+// - originID: the sender of the range request
+// - rangeRequest: the range request to validate
+// Returns:
+// - *alsp.MisbehaviorReport: If any misbehavior is detected such as:
+//   - the range request is invalid
+//   - the range request is valid but should be reported as misbehavior anyway (due to probabilities)
+//
+// - bool: true if the range request is valid and should not be reported as misbehavior; otherwise, false if a misbehavior is detected
+//
+// - error: If an error is encountered while validating the range request. Error is assumed to be irrecoverable because of internal processes that didn't allow validation to complete.
 func (e *Engine) validateRangeRequestForALSP(originID flow.Identifier, rangeRequest *messages.RangeRequest) (*alsp.MisbehaviorReport, bool, error) {
 	// Generate a random integer between 1 and spamProbabilityMultiplier (exclusive)
 	n, err := rand.Uint32n(spamProbabilityMultiplier)
@@ -658,12 +662,15 @@ func (e *Engine) validateRangeRequestForALSP(originID flow.Identifier, rangeRequ
 }
 
 // validateSyncRequestForALSP checks if a sync request should be reported as a misbehavior due to malicious intent (e.g. spamming).
-// It returns a misbehavior report and a boolean indicating whether validation passed, as well as an error.
-// Returns an error that is assumed to be irrecoverable because of internal processes that didn't allow validation to complete.
-// Returns true if passed validation.
-// Returns false if either
-// a) failed validation (due to probabilities) or
-// b) an error is encountered.
+// Args:
+// - originID: the sender of the sync request
+// Returns:
+// - *alsp.MisbehaviorReport: If any misbehavior is detected such as:
+//   - the sync request is valid but should be reported as misbehavior anyway (due to probabilities)
+//
+// - bool: true if the sync request is valid and should not be reported as misbehavior; otherwise, false if a misbehavior is detected
+//
+// - error: If an error is encountered while validating the sync request. Error is assumed to be irrecoverable because of internal processes that didn't allow validation to complete.
 func (e *Engine) validateSyncRequestForALSP(originID flow.Identifier) (*alsp.MisbehaviorReport, bool, error) {
 	// Generate a random integer between 1 and spamProbabilityMultiplier (exclusive)
 	n, err := rand.Uint32n(spamProbabilityMultiplier)

From 8728fc10698f5fa732920b295234d1211dc132c1 Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Wed, 4 Oct 2023 14:43:28 -0500
Subject: [PATCH 10/25] SpamDetectionConfig  godoc update

---
 engine/common/synchronization/config.go | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/engine/common/synchronization/config.go b/engine/common/synchronization/config.go
index e22252f08e9..5857ab73cbc 100644
--- a/engine/common/synchronization/config.go
+++ b/engine/common/synchronization/config.go
@@ -44,18 +44,23 @@ const spamProbabilityMultiplier = 1001
 // SpamDetectionConfig contains configuration parameters for spam detection for different message types.
 // The probability of creating a misbehavior report for a message of a given type is calculated differently for different
 // message types.
+// MisbehaviourReports are generated for two reasons:
+//  1. A malformed message will always produce a MisbehaviourReport, to notify ALSP of *unambiguous* spam.
+//  2. A correctly formed message may produce a MisbehaviourReport probabilistically, to notify ALSP of *ambiguous* spam.
+//     This effectively tracks the load associated with a particular sender, for this engine, and, on average,
+//     reports message load proportionally as misbehaviour to ALSP.
 type SpamDetectionConfig struct {
 
-	// batchRequestBaseProb is the base probability that's used in creating the final probability of creating a
+	// batchRequestBaseProb is the base probability in [0,1] that's used in creating the final probability of creating a
 	// misbehavior report for a BatchRequest message. This is why the word "base" is used in the name of this field,
 	// since it's not the final probability and there are other factors that determine the final probability.
 	// The reason for this is that we want to increase the probability of creating a misbehavior report for a large batch.
 	batchRequestBaseProb float32
 
-	// syncRequestProb is the probability of creating a misbehavior report for a SyncRequest message.
+	// syncRequestProb is the probability in [0,1] of creating a misbehavior report for a SyncRequest message.
 	syncRequestProb float32
 
-	// rangeRequestBaseProb is the base probability that's used in creating the final probability of creating a
+	// rangeRequestBaseProb is the base probability in [0,1] that's used in creating the final probability of creating a
 	// misbehavior report for a RangeRequest message. This is why the word "base" is used in the name of this field,
 	// since it's not the final probability and there are other factors that determine the final probability.
 	// The reason for this is that we want to increase the probability of creating a misbehavior report for a large range.

From 52ad0aa28118cc980e1ed950703207ae98d05067 Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Wed, 4 Oct 2023 15:02:36 -0500
Subject: [PATCH 11/25] spamProbabilityMultiplier reduced to 1000

---
 engine/common/synchronization/config.go | 4 ++--
 engine/common/synchronization/engine.go | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/engine/common/synchronization/config.go b/engine/common/synchronization/config.go
index 5857ab73cbc..0ab2e7dd9e0 100644
--- a/engine/common/synchronization/config.go
+++ b/engine/common/synchronization/config.go
@@ -37,9 +37,9 @@ func WithScanInterval(interval time.Duration) OptionFunc {
 	}
 }
 
-// spamProbabilityMultiplier is used to convert probability factor to an integer as well as a maximum value - 1
+// spamProbabilityMultiplier is used to convert probability factor to an integer as well as a maximum value for the
 // random number that can be generated by the random number generator.
-const spamProbabilityMultiplier = 1001
+const spamProbabilityMultiplier = 1000
 
 // SpamDetectionConfig contains configuration parameters for spam detection for different message types.
 // The probability of creating a misbehavior report for a message of a given type is calculated differently for different
diff --git a/engine/common/synchronization/engine.go b/engine/common/synchronization/engine.go
index fe4af9c55b8..0f05cb266d9 100644
--- a/engine/common/synchronization/engine.go
+++ b/engine/common/synchronization/engine.go
@@ -522,7 +522,7 @@ func (e *Engine) sendRequests(participants flow.IdentifierList, ranges []chainsy
 //
 // - error: If an error is encountered while validating the batch request. Error is assumed to be irrecoverable because of internal processes that didn't allow validation to complete.
 func (e *Engine) validateBatchRequestForALSP(originID flow.Identifier, batchRequest *messages.BatchRequest) (*alsp.MisbehaviorReport, bool, error) {
-	// Generate a random integer between 1 and spamProbabilityMultiplier (exclusive)
+	// Generate a random integer between 0 and spamProbabilityMultiplier (exclusive)
 	n, err := rand.Uint32n(spamProbabilityMultiplier)
 
 	if err != nil {
@@ -600,7 +600,7 @@ func (e *Engine) validateBlockResponseForALSP(channel channels.Channel, id flow.
 //
 // - error: If an error is encountered while validating the range request. Error is assumed to be irrecoverable because of internal processes that didn't allow validation to complete.
 func (e *Engine) validateRangeRequestForALSP(originID flow.Identifier, rangeRequest *messages.RangeRequest) (*alsp.MisbehaviorReport, bool, error) {
-	// Generate a random integer between 1 and spamProbabilityMultiplier (exclusive)
+	// Generate a random integer between 0 and spamProbabilityMultiplier (exclusive)
 	n, err := rand.Uint32n(spamProbabilityMultiplier)
 
 	if err != nil {
@@ -672,7 +672,7 @@ func (e *Engine) validateRangeRequestForALSP(originID flow.Identifier, rangeRequ
 //
 // - error: If an error is encountered while validating the sync request. Error is assumed to be irrecoverable because of internal processes that didn't allow validation to complete.
 func (e *Engine) validateSyncRequestForALSP(originID flow.Identifier) (*alsp.MisbehaviorReport, bool, error) {
-	// Generate a random integer between 1 and spamProbabilityMultiplier (exclusive)
+	// Generate a random integer between 0 and spamProbabilityMultiplier (exclusive)
 	n, err := rand.Uint32n(spamProbabilityMultiplier)
 
 	if err != nil {

From 5cd713763036c9a37ec667ba30cd7f23081a8041 Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Thu, 5 Oct 2023 08:28:53 -0500
Subject: [PATCH 12/25] removed extra line from assignment, error handling

---
 engine/common/synchronization/engine.go | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/engine/common/synchronization/engine.go b/engine/common/synchronization/engine.go
index 0f05cb266d9..8a1c3f015b6 100644
--- a/engine/common/synchronization/engine.go
+++ b/engine/common/synchronization/engine.go
@@ -524,7 +524,6 @@ func (e *Engine) sendRequests(participants flow.IdentifierList, ranges []chainsy
 func (e *Engine) validateBatchRequestForALSP(originID flow.Identifier, batchRequest *messages.BatchRequest) (*alsp.MisbehaviorReport, bool, error) {
 	// Generate a random integer between 0 and spamProbabilityMultiplier (exclusive)
 	n, err := rand.Uint32n(spamProbabilityMultiplier)
-
 	if err != nil {
 		return nil, false, fmt.Errorf("failed to generate random number from %x: %w", originID[:], err)
 	}
@@ -537,7 +536,6 @@ func (e *Engine) validateBatchRequestForALSP(originID flow.Identifier, batchRequ
 			Str("reason", alsp.InvalidMessage.String()).
 			Msg("received invalid batch request with 0 block IDs, creating ALSP report")
 		report, err := alsp.NewMisbehaviorReport(originID, alsp.InvalidMessage)
-
 		if err != nil {
 			// failing to create the misbehavior report is unlikely. If an error is encountered while
 			// creating the misbehavior report it indicates a bug and processing can not proceed.
@@ -569,7 +567,6 @@ func (e *Engine) validateBatchRequestForALSP(originID flow.Identifier, batchRequ
 			Str("reason", alsp.ResourceIntensiveRequest.String()).
 			Msgf("for %d block IDs, creating probabilistic ALSP report", len(batchRequest.BlockIDs))
 		report, err := alsp.NewMisbehaviorReport(originID, alsp.ResourceIntensiveRequest)
-
 		if err != nil {
 			// failing to create the misbehavior report is unlikely. If an error is encountered while
 			// creating the misbehavior report it indicates a bug and processing can not proceed.

From 0ee522e15c02c0eacfc727623abb0441ab79ffff Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Thu, 5 Oct 2023 10:18:38 -0500
Subject: [PATCH 13/25] removed dropping message on misbehavior

---
 engine/common/synchronization/engine.go | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/engine/common/synchronization/engine.go b/engine/common/synchronization/engine.go
index 8a1c3f015b6..4c65b4c49e5 100644
--- a/engine/common/synchronization/engine.go
+++ b/engine/common/synchronization/engine.go
@@ -219,8 +219,6 @@ func (e *Engine) process(channel channels.Channel, originID flow.Identifier, eve
 				Hex("origin_id", logging.ID(originID)).
 				Str(logging.KeySuspicious, "true").
 				Msgf("received invalid batch request from %x: %v", originID[:], valid)
-			e.metrics.InboundMessageDropped(metrics.EngineSynchronization, metrics.MessageBatchRequest)
-			return nil
 		}
 		return e.requestHandler.Process(channel, originID, event)
 	case *messages.RangeRequest:
@@ -235,8 +233,6 @@ func (e *Engine) process(channel channels.Channel, originID flow.Identifier, eve
 				Hex("origin_id", logging.ID(originID)).
 				Str(logging.KeySuspicious, "true").
 				Msgf("received invalid range request from %x: %v", originID[:], valid)
-			e.metrics.InboundMessageDropped(metrics.EngineSynchronization, metrics.MessageRangeRequest)
-			return nil
 		}
 		return e.requestHandler.Process(channel, originID, event)
 
@@ -252,8 +248,6 @@ func (e *Engine) process(channel channels.Channel, originID flow.Identifier, eve
 				Hex("origin_id", logging.ID(originID)).
 				Str(logging.KeySuspicious, "true").
 				Msgf("received invalid sync request from %x: %v", originID[:], valid)
-			e.metrics.InboundMessageDropped(metrics.EngineSynchronization, metrics.MessageSyncRequest)
-			return nil
 		}
 		return e.requestHandler.Process(channel, originID, event)
 
@@ -269,8 +263,6 @@ func (e *Engine) process(channel channels.Channel, originID flow.Identifier, eve
 				Hex("origin_id", logging.ID(originID)).
 				Str(logging.KeySuspicious, "true").
 				Msgf("received invalid block response from %x: %v", originID[:], valid)
-			e.metrics.InboundMessageDropped(metrics.EngineSynchronization, metrics.MessageBlockResponse)
-			return nil
 		}
 		return e.responseMessageHandler.Process(originID, event)
 
@@ -286,8 +278,6 @@ func (e *Engine) process(channel channels.Channel, originID flow.Identifier, eve
 				Hex("origin_id", logging.ID(originID)).
 				Str(logging.KeySuspicious, "true").
 				Msgf("received invalid sync response from %x: %v", originID[:], valid)
-			e.metrics.InboundMessageDropped(metrics.EngineSynchronization, metrics.MessageSyncResponse)
-			return nil
 		}
 		return e.responseMessageHandler.Process(originID, event)
 	default:

From eeec1a100715f9185fb6c72fc2db996ff18979c8 Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Thu, 5 Oct 2023 11:33:54 -0500
Subject: [PATCH 14/25] validateBatchRequestForALSP only returns error, logs
 load misbehaviors

---
 engine/common/synchronization/engine.go | 32 ++++++++++---------------
 utils/logging/consts.go                 |  3 +++
 2 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/engine/common/synchronization/engine.go b/engine/common/synchronization/engine.go
index 4c65b4c49e5..3271bea7b0b 100644
--- a/engine/common/synchronization/engine.go
+++ b/engine/common/synchronization/engine.go
@@ -208,18 +208,10 @@ func (e *Engine) Process(channel channels.Channel, originID flow.Identifier, eve
 func (e *Engine) process(channel channels.Channel, originID flow.Identifier, event interface{}) error {
 	switch message := event.(type) {
 	case *messages.BatchRequest:
-		report, valid, err := e.validateBatchRequestForALSP(originID, message)
+		err := e.validateBatchRequestForALSP(originID, message)
 		if err != nil {
 			return fmt.Errorf("failed to validate batch request from %x: %w", originID[:], err)
 		}
-		if !valid {
-			e.con.ReportMisbehavior(report) // report misbehavior to ALSP
-			e.log.
-				Warn().
-				Hex("origin_id", logging.ID(originID)).
-				Str(logging.KeySuspicious, "true").
-				Msgf("received invalid batch request from %x: %v", originID[:], valid)
-		}
 		return e.requestHandler.Process(channel, originID, event)
 	case *messages.RangeRequest:
 		report, valid, err := e.validateRangeRequestForALSP(originID, message)
@@ -511,11 +503,11 @@ func (e *Engine) sendRequests(participants flow.IdentifierList, ranges []chainsy
 // - bool: true if the batch request is valid and should not be reported as misbehavior; otherwise, false if a misbehavior is detected
 //
 // - error: If an error is encountered while validating the batch request. Error is assumed to be irrecoverable because of internal processes that didn't allow validation to complete.
-func (e *Engine) validateBatchRequestForALSP(originID flow.Identifier, batchRequest *messages.BatchRequest) (*alsp.MisbehaviorReport, bool, error) {
+func (e *Engine) validateBatchRequestForALSP(originID flow.Identifier, batchRequest *messages.BatchRequest) error {
 	// Generate a random integer between 0 and spamProbabilityMultiplier (exclusive)
 	n, err := rand.Uint32n(spamProbabilityMultiplier)
 	if err != nil {
-		return nil, false, fmt.Errorf("failed to generate random number from %x: %w", originID[:], err)
+		return fmt.Errorf("failed to generate random number from %x: %w", originID[:], err)
 	}
 
 	// validity check: if no block IDs, always report as misbehavior
@@ -529,10 +521,11 @@ func (e *Engine) validateBatchRequestForALSP(originID flow.Identifier, batchRequ
 		if err != nil {
 			// failing to create the misbehavior report is unlikely. If an error is encountered while
 			// creating the misbehavior report it indicates a bug and processing can not proceed.
-			return nil, false, fmt.Errorf("failed to create misbehavior report (invalid batch request, no block IDs) from %x: %w", originID[:], err)
+			return fmt.Errorf("failed to create misbehavior report (invalid batch request, no block IDs) from %x: %w", originID[:], err)
 		}
+		e.con.ReportMisbehavior(report) // report misbehavior to ALSP
 		// failed validation check and should be reported as misbehavior
-		return report, false, nil
+		return nil
 	}
 
 	// to avoid creating a misbehavior report for every batch request received, use a probabilistic approach.
@@ -553,20 +546,21 @@ func (e *Engine) validateBatchRequestForALSP(originID flow.Identifier, batchRequ
 		// create a misbehavior report
 		e.log.Warn().
 			Hex("origin_id", logging.ID(originID)).
-			Str(logging.KeySuspicious, "true").
+			Str(logging.KeyLoad, "true").
 			Str("reason", alsp.ResourceIntensiveRequest.String()).
 			Msgf("for %d block IDs, creating probabilistic ALSP report", len(batchRequest.BlockIDs))
 		report, err := alsp.NewMisbehaviorReport(originID, alsp.ResourceIntensiveRequest)
 		if err != nil {
 			// failing to create the misbehavior report is unlikely. If an error is encountered while
 			// creating the misbehavior report it indicates a bug and processing can not proceed.
-			return nil, false, fmt.Errorf("failed to create misbehavior report from %x: %w", originID[:], err)
+			return fmt.Errorf("failed to create misbehavior report from %x: %w", originID[:], err)
 		}
 		// failed validation check and should be reported as misbehavior
-		return report, false, nil
+		e.con.ReportMisbehavior(report) // report misbehavior to ALSP
+		return nil
 	}
 
-	return nil, true, nil
+	return nil
 }
 
 // TODO: implement spam reporting similar to validateSyncRequestForALSP
@@ -630,7 +624,7 @@ func (e *Engine) validateRangeRequestForALSP(originID flow.Identifier, rangeRequ
 		// create a misbehavior report
 		e.log.Warn().
 			Hex("origin_id", logging.ID(originID)).
-			Str(logging.KeySuspicious, "true").
+			Str(logging.KeyLoad, "true").
 			Str("reason", alsp.ResourceIntensiveRequest.String()).
 			Msgf("from height %d to height %d, creating probabilistic ALSP report", rangeRequest.FromHeight, rangeRequest.ToHeight)
 		report, err := alsp.NewMisbehaviorReport(originID, alsp.ResourceIntensiveRequest)
@@ -673,7 +667,7 @@ func (e *Engine) validateSyncRequestForALSP(originID flow.Identifier) (*alsp.Mis
 		// create misbehavior report
 		e.log.Warn().
 			Hex("origin_id", logging.ID(originID)).
-			Str(logging.KeySuspicious, "true").
+			Str(logging.KeyLoad, "true").
 			Str("reason", alsp.ResourceIntensiveRequest.String()).
 			Msg("creating probabilistic ALSP report")
 
diff --git a/utils/logging/consts.go b/utils/logging/consts.go
index 46f48a3c937..5bf0f0d8b6c 100644
--- a/utils/logging/consts.go
+++ b/utils/logging/consts.go
@@ -8,4 +8,7 @@ const (
 	// KeyNetworkingSecurity is a logging label that is used to flag the log event as a networking security issue.
 	// This is used to add an easily searchable label to the log events.
 	KeyNetworkingSecurity = "networking-security"
+
+	// KeyLoad is a logging label that is used to flag the log event as a load issue.
+	KeyLoad = "load"
 )

From ee502fdec3f94b3e3ce606a07286714c337d083b Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Thu, 5 Oct 2023 11:37:27 -0500
Subject: [PATCH 15/25] validateBatchRequestForALSP godoc update

---
 engine/common/synchronization/engine.go | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/engine/common/synchronization/engine.go b/engine/common/synchronization/engine.go
index 3271bea7b0b..6cafb7570a6 100644
--- a/engine/common/synchronization/engine.go
+++ b/engine/common/synchronization/engine.go
@@ -491,17 +491,12 @@ func (e *Engine) sendRequests(participants flow.IdentifierList, ranges []chainsy
 	}
 }
 
-// validateBatchRequestForALSP checks if a batch request should be reported as a misbehavior due to malicious intent (e.g. spamming).
+// validateBatchRequestForALSP checks if a batch request should be reported as a misbehavior due to malicious intent (e.g. spamming)
+// and sends misbehavior report to ALSP.
 // Args:
 // - originID: the sender of the batch request
 // - batchRequest: the batch request to validate
 // Returns:
-// - *alsp.MisbehaviorReport: If any misbehavior is detected such as:
-//   - the batch request is invalid
-//   - the batch request is valid but should be reported as misbehavior anyway (due to probabilities)
-//
-// - bool: true if the batch request is valid and should not be reported as misbehavior; otherwise, false if a misbehavior is detected
-//
 // - error: If an error is encountered while validating the batch request. Error is assumed to be irrecoverable because of internal processes that didn't allow validation to complete.
 func (e *Engine) validateBatchRequestForALSP(originID flow.Identifier, batchRequest *messages.BatchRequest) error {
 	// Generate a random integer between 0 and spamProbabilityMultiplier (exclusive)
@@ -559,7 +554,6 @@ func (e *Engine) validateBatchRequestForALSP(originID flow.Identifier, batchRequ
 		e.con.ReportMisbehavior(report) // report misbehavior to ALSP
 		return nil
 	}
-
 	return nil
 }
 

From 856e5b8bef5bf932e087bd020db7c4562bd94b11 Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Thu, 5 Oct 2023 14:56:53 -0500
Subject: [PATCH 16/25] validateRangeRequestForALSP only returns error, logs
 load misbehaviors

---
 engine/common/synchronization/engine.go | 38 +++++++++++--------------
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/engine/common/synchronization/engine.go b/engine/common/synchronization/engine.go
index 6cafb7570a6..629511df9a8 100644
--- a/engine/common/synchronization/engine.go
+++ b/engine/common/synchronization/engine.go
@@ -214,18 +214,10 @@ func (e *Engine) process(channel channels.Channel, originID flow.Identifier, eve
 		}
 		return e.requestHandler.Process(channel, originID, event)
 	case *messages.RangeRequest:
-		report, valid, err := e.validateRangeRequestForALSP(originID, message)
+		err := e.validateRangeRequestForALSP(originID, message)
 		if err != nil {
 			return fmt.Errorf("failed to validate range request from %x: %w", originID[:], err)
 		}
-		if !valid {
-			e.con.ReportMisbehavior(report) // report misbehavior to ALSP
-			e.log.
-				Warn().
-				Hex("origin_id", logging.ID(originID)).
-				Str(logging.KeySuspicious, "true").
-				Msgf("received invalid range request from %x: %v", originID[:], valid)
-		}
 		return e.requestHandler.Process(channel, originID, event)
 
 	case *messages.SyncRequest:
@@ -518,8 +510,8 @@ func (e *Engine) validateBatchRequestForALSP(originID flow.Identifier, batchRequ
 			// creating the misbehavior report it indicates a bug and processing can not proceed.
 			return fmt.Errorf("failed to create misbehavior report (invalid batch request, no block IDs) from %x: %w", originID[:], err)
 		}
-		e.con.ReportMisbehavior(report) // report misbehavior to ALSP
-		// failed validation check and should be reported as misbehavior
+		// failed unambiguous validation check and should be reported as misbehavior
+		e.con.ReportMisbehavior(report)
 		return nil
 	}
 
@@ -550,8 +542,8 @@ func (e *Engine) validateBatchRequestForALSP(originID flow.Identifier, batchRequ
 			// creating the misbehavior report it indicates a bug and processing can not proceed.
 			return fmt.Errorf("failed to create misbehavior report from %x: %w", originID[:], err)
 		}
-		// failed validation check and should be reported as misbehavior
-		e.con.ReportMisbehavior(report) // report misbehavior to ALSP
+		// failed probabilistic (load) validation check and should be reported as misbehavior
+		e.con.ReportMisbehavior(report)
 		return nil
 	}
 	return nil
@@ -574,12 +566,12 @@ func (e *Engine) validateBlockResponseForALSP(channel channels.Channel, id flow.
 // - bool: true if the range request is valid and should not be reported as misbehavior; otherwise, false if a misbehavior is detected
 //
 // - error: If an error is encountered while validating the range request. Error is assumed to be irrecoverable because of internal processes that didn't allow validation to complete.
-func (e *Engine) validateRangeRequestForALSP(originID flow.Identifier, rangeRequest *messages.RangeRequest) (*alsp.MisbehaviorReport, bool, error) {
+func (e *Engine) validateRangeRequestForALSP(originID flow.Identifier, rangeRequest *messages.RangeRequest) error {
 	// Generate a random integer between 0 and spamProbabilityMultiplier (exclusive)
 	n, err := rand.Uint32n(spamProbabilityMultiplier)
 
 	if err != nil {
-		return nil, false, fmt.Errorf("failed to generate random number from %x: %w", originID[:], err)
+		return fmt.Errorf("failed to generate random number from %x: %w", originID[:], err)
 	}
 
 	// check if range request is valid
@@ -594,10 +586,11 @@ func (e *Engine) validateRangeRequestForALSP(originID flow.Identifier, rangeRequ
 		if err != nil {
 			// failing to create the misbehavior report is unlikely. If an error is encountered while
 			// creating the misbehavior report it indicates a bug and processing can not proceed.
-			return nil, false, fmt.Errorf("failed to create misbehavior report (invalid range request) from %x: %w", originID[:], err)
+			return fmt.Errorf("failed to create misbehavior report (invalid range request) from %x: %w", originID[:], err)
 		}
-		// failed validation check and should be reported as misbehavior
-		return report, false, nil
+		// failed unambiguous validation check and should be reported as misbehavior
+		e.con.ReportMisbehavior(report)
+		return nil
 	}
 
 	// to avoid creating a misbehavior report for every range request received, use a probabilistic approach.
@@ -626,14 +619,17 @@ func (e *Engine) validateRangeRequestForALSP(originID flow.Identifier, rangeRequ
 		if err != nil {
 			// failing to create the misbehavior report is unlikely. If an error is encountered while
 			// creating the misbehavior report it indicates a bug and processing can not proceed.
-			return nil, false, fmt.Errorf("failed to create misbehavior report from %x: %w", originID[:], err)
+			return fmt.Errorf("failed to create misbehavior report from %x: %w", originID[:], err)
 		}
 		// failed validation check and should be reported as misbehavior
-		return report, false, nil
+
+		// failed probabilistic (load) validation check and should be reported as misbehavior
+		e.con.ReportMisbehavior(report)
+		return nil
 	}
 
 	// passed all validation checks with no misbehavior detected
-	return nil, true, nil
+	return nil
 }
 
 // validateSyncRequestForALSP checks if a sync request should be reported as a misbehavior due to malicious intent (e.g. spamming).

From c8d0d4a736fab190a5e769db1b462cf18ce189f1 Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Thu, 5 Oct 2023 15:10:14 -0500
Subject: [PATCH 17/25] godocs update

---
 engine/common/synchronization/engine.go | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/engine/common/synchronization/engine.go b/engine/common/synchronization/engine.go
index 629511df9a8..3d9dcf31f4c 100644
--- a/engine/common/synchronization/engine.go
+++ b/engine/common/synchronization/engine.go
@@ -483,8 +483,12 @@ func (e *Engine) sendRequests(participants flow.IdentifierList, ranges []chainsy
 	}
 }
 
-// validateBatchRequestForALSP checks if a batch request should be reported as a misbehavior due to malicious intent (e.g. spamming)
-// and sends misbehavior report to ALSP.
+// validateBatchRequestForALSP checks if a batch request should be reported as a misbehavior and sends misbehavior report to ALSP.
+// The misbehavior is due to either:
+//  1. unambiguous malicious or incorrect behavior (0 block IDs) OR
+//  2. large number of block IDs in batch request. This is more ambiguous to detect as malicious behavior because there is no way to know for sure
+//     if the sender is sending a large batch request maliciously or not, so we use a probabilistic approach to report the misbehavior.
+//
 // Args:
 // - originID: the sender of the batch request
 // - batchRequest: the batch request to validate
@@ -554,17 +558,16 @@ func (e *Engine) validateBlockResponseForALSP(channel channels.Channel, id flow.
 	return nil, true, nil
 }
 
-// validateRangeRequestForALSP checks if a range request should be reported as a misbehavior due to malicious intent (e.g. spamming).
+// validateRangeRequestForALSP checks if a range request should be reported as a misbehavior and sends misbehavior report to ALSP.
+// The misbehavior is due to either:
+//  1. unambiguous malicious or incorrect behavior (toHeight < fromHeight) OR
+//  2. large height in range request. This is more ambiguous to detect as malicious behavior because there is no way to know for sure
+//     if the sender is sending a large range request height maliciously or not, so we use a probabilistic approach to report the misbehavior.
+//
 // Args:
 // - originID: the sender of the range request
 // - rangeRequest: the range request to validate
 // Returns:
-// - *alsp.MisbehaviorReport: If any misbehavior is detected such as:
-//   - the range request is invalid
-//   - the range request is valid but should be reported as misbehavior anyway (due to probabilities)
-//
-// - bool: true if the range request is valid and should not be reported as misbehavior; otherwise, false if a misbehavior is detected
-//
 // - error: If an error is encountered while validating the range request. Error is assumed to be irrecoverable because of internal processes that didn't allow validation to complete.
 func (e *Engine) validateRangeRequestForALSP(originID flow.Identifier, rangeRequest *messages.RangeRequest) error {
 	// Generate a random integer between 0 and spamProbabilityMultiplier (exclusive)

From 78293543437c11ae3911a69cee14278c3f707aaa Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Thu, 5 Oct 2023 15:23:48 -0500
Subject: [PATCH 18/25] validateSyncRequestForALSP only returns error, logs
 misbehaviors

---
 engine/common/synchronization/engine.go | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/engine/common/synchronization/engine.go b/engine/common/synchronization/engine.go
index 3d9dcf31f4c..63698346913 100644
--- a/engine/common/synchronization/engine.go
+++ b/engine/common/synchronization/engine.go
@@ -221,18 +221,10 @@ func (e *Engine) process(channel channels.Channel, originID flow.Identifier, eve
 		return e.requestHandler.Process(channel, originID, event)
 
 	case *messages.SyncRequest:
-		report, valid, err := e.validateSyncRequestForALSP(originID)
+		err := e.validateSyncRequestForALSP(originID)
 		if err != nil {
 			return fmt.Errorf("failed to validate sync request from %x: %w", originID[:], err)
 		}
-		if !valid {
-			e.con.ReportMisbehavior(report) // report misbehavior to ALSP
-			e.log.
-				Warn().
-				Hex("origin_id", logging.ID(originID)).
-				Str(logging.KeySuspicious, "true").
-				Msgf("received invalid sync request from %x: %v", originID[:], valid)
-		}
 		return e.requestHandler.Process(channel, originID, event)
 
 	case *messages.BlockResponse:
@@ -645,12 +637,12 @@ func (e *Engine) validateRangeRequestForALSP(originID flow.Identifier, rangeRequ
 // - bool: true if the sync request is valid and should not be reported as misbehavior; otherwise, false if a misbehavior is detected
 //
 // - error: If an error is encountered while validating the sync request. Error is assumed to be irrecoverable because of internal processes that didn't allow validation to complete.
-func (e *Engine) validateSyncRequestForALSP(originID flow.Identifier) (*alsp.MisbehaviorReport, bool, error) {
+func (e *Engine) validateSyncRequestForALSP(originID flow.Identifier) error {
 	// Generate a random integer between 0 and spamProbabilityMultiplier (exclusive)
 	n, err := rand.Uint32n(spamProbabilityMultiplier)
 
 	if err != nil {
-		return nil, false, fmt.Errorf("failed to generate random number from %x: %w", originID[:], err)
+		return fmt.Errorf("failed to generate random number from %x: %w", originID[:], err)
 	}
 
 	// to avoid creating a misbehavior report for every sync request received, use a probabilistic approach.
@@ -669,13 +661,14 @@ func (e *Engine) validateSyncRequestForALSP(originID flow.Identifier) (*alsp.Mis
 		if err != nil {
 			// failing to create the misbehavior report is unlikely. If an error is encountered while
 			// creating the misbehavior report it indicates a bug and processing can not proceed.
-			return nil, false, fmt.Errorf("failed to create misbehavior report from %x: %w", originID[:], err)
+			return fmt.Errorf("failed to create misbehavior report from %x: %w", originID[:], err)
 		}
-		return report, false, nil
+		e.con.ReportMisbehavior(report)
+		return nil
 	}
 
 	// passed all validation checks with no misbehavior detected
-	return nil, true, nil
+	return nil
 }
 
 // TODO: implement spam reporting similar to validateSyncRequestForALSP

From fc479249daf279f148485bdb648586ac874e0448 Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Thu, 5 Oct 2023 16:03:46 -0500
Subject: [PATCH 19/25] godocs update

---
 engine/common/synchronization/engine.go | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/engine/common/synchronization/engine.go b/engine/common/synchronization/engine.go
index 63698346913..501c0c3c3f1 100644
--- a/engine/common/synchronization/engine.go
+++ b/engine/common/synchronization/engine.go
@@ -627,15 +627,13 @@ func (e *Engine) validateRangeRequestForALSP(originID flow.Identifier, rangeRequ
 	return nil
 }
 
-// validateSyncRequestForALSP checks if a sync request should be reported as a misbehavior due to malicious intent (e.g. spamming).
+// validateSyncRequestForALSP checks if a sync request should be reported as a misbehavior and sends misbehavior report to ALSP.
+// The misbehavior is ambiguous to detect as malicious behavior because there is no way to know for sure if the sender is sending
+// a sync request maliciously or not, so we use a probabilistic approach to report the misbehavior.
+//
 // Args:
 // - originID: the sender of the sync request
 // Returns:
-// - *alsp.MisbehaviorReport: If any misbehavior is detected such as:
-//   - the sync request is valid but should be reported as misbehavior anyway (due to probabilities)
-//
-// - bool: true if the sync request is valid and should not be reported as misbehavior; otherwise, false if a misbehavior is detected
-//
 // - error: If an error is encountered while validating the sync request. Error is assumed to be irrecoverable because of internal processes that didn't allow validation to complete.
 func (e *Engine) validateSyncRequestForALSP(originID flow.Identifier) error {
 	// Generate a random integer between 0 and spamProbabilityMultiplier (exclusive)

From eff71d5205f3ae8fd41fd998e17706d45ca78914 Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Thu, 5 Oct 2023 16:04:34 -0500
Subject: [PATCH 20/25] sync request test fix

---
 engine/common/synchronization/engine_spam_test.go | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/engine/common/synchronization/engine_spam_test.go b/engine/common/synchronization/engine_spam_test.go
index b94157aa893..0bfc8c0e7b9 100644
--- a/engine/common/synchronization/engine_spam_test.go
+++ b/engine/common/synchronization/engine_spam_test.go
@@ -49,12 +49,14 @@ func (ss *SyncSuite) TestLoad_Process_SyncRequest_HigherThanReceiver_OutsideTole
 		// if request height is higher than local finalized, we should not respond
 		req.Height = ss.head.Height + 1
 
-		// assert that HandleHeight, WithinTolerance are not called because misbehavior is reported
-		// also, check that response is never sent
-		ss.core.AssertNotCalled(ss.T(), "HandleHeight")
-		ss.core.AssertNotCalled(ss.T(), "WithinTolerance")
+		ss.core.On("HandleHeight", ss.head, req.Height)
+		ss.core.On("WithinTolerance", ss.head, req.Height).Return(false)
 		ss.con.AssertNotCalled(ss.T(), "Unicast", mock.Anything, mock.Anything)
 
+		// maybe function calls that might or might not occur over the course of the load test
+		ss.core.On("ScanPending", ss.head).Return([]chainsync.Range{}, []chainsync.Batch{}).Maybe()
+		ss.con.On("Multicast", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil).Maybe()
+
 		// count misbehavior reports over the course of a load test
 		ss.con.On("ReportMisbehavior", mock.Anything).Return(mock.Anything).Run(
 			func(args mock.Arguments) {

From 7fa258b8b582571265640bec6ca6315f761285e4 Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Thu, 5 Oct 2023 16:29:26 -0500
Subject: [PATCH 21/25] throw irrecoverable error from process() for any ALSP
 validation error

---
 engine/common/synchronization/engine.go | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/engine/common/synchronization/engine.go b/engine/common/synchronization/engine.go
index 501c0c3c3f1..4fc3c372455 100644
--- a/engine/common/synchronization/engine.go
+++ b/engine/common/synchronization/engine.go
@@ -210,27 +210,27 @@ func (e *Engine) process(channel channels.Channel, originID flow.Identifier, eve
 	case *messages.BatchRequest:
 		err := e.validateBatchRequestForALSP(originID, message)
 		if err != nil {
-			return fmt.Errorf("failed to validate batch request from %x: %w", originID[:], err)
+			irrecoverable.Throw(context.TODO(), fmt.Errorf("failed to validate batch request from %x: %w", originID[:], err))
 		}
 		return e.requestHandler.Process(channel, originID, event)
 	case *messages.RangeRequest:
 		err := e.validateRangeRequestForALSP(originID, message)
 		if err != nil {
-			return fmt.Errorf("failed to validate range request from %x: %w", originID[:], err)
+			irrecoverable.Throw(context.TODO(), fmt.Errorf("failed to validate range request from %x: %w", originID[:], err))
 		}
 		return e.requestHandler.Process(channel, originID, event)
 
 	case *messages.SyncRequest:
 		err := e.validateSyncRequestForALSP(originID)
 		if err != nil {
-			return fmt.Errorf("failed to validate sync request from %x: %w", originID[:], err)
+			irrecoverable.Throw(context.TODO(), fmt.Errorf("failed to validate sync request from %x: %w", originID[:], err))
 		}
 		return e.requestHandler.Process(channel, originID, event)
 
 	case *messages.BlockResponse:
 		report, valid, err := e.validateBlockResponseForALSP(channel, originID, message)
 		if err != nil {
-			return fmt.Errorf("failed to validate block response from %x: %w", originID[:], err)
+			irrecoverable.Throw(context.TODO(), fmt.Errorf("failed to validate block response from %x: %w", originID[:], err))
 		}
 		if !valid {
 			e.con.ReportMisbehavior(report) // report misbehavior to ALSP
@@ -245,7 +245,7 @@ func (e *Engine) process(channel channels.Channel, originID flow.Identifier, eve
 	case *messages.SyncResponse:
 		report, valid, err := e.validateSyncResponseForALSP(channel, originID, message)
 		if err != nil {
-			return fmt.Errorf("failed to validate sync response from %x: %w", originID[:], err)
+			irrecoverable.Throw(context.TODO(), fmt.Errorf("failed to validate sync response from %x: %w", originID[:], err))
 		}
 		if !valid {
 			e.con.ReportMisbehavior(report) // report misbehavior to ALSP

From d1bd216772838c942e67cb4bd70c35dac9d4ed72 Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Fri, 6 Oct 2023 15:23:03 -0500
Subject: [PATCH 22/25] validate*ResponseForALSP only return error

---
 engine/common/synchronization/engine.go | 28 ++++++-------------------
 1 file changed, 6 insertions(+), 22 deletions(-)

diff --git a/engine/common/synchronization/engine.go b/engine/common/synchronization/engine.go
index 4fc3c372455..8e6be30cb00 100644
--- a/engine/common/synchronization/engine.go
+++ b/engine/common/synchronization/engine.go
@@ -228,33 +228,17 @@ func (e *Engine) process(channel channels.Channel, originID flow.Identifier, eve
 		return e.requestHandler.Process(channel, originID, event)
 
 	case *messages.BlockResponse:
-		report, valid, err := e.validateBlockResponseForALSP(channel, originID, message)
+		err := e.validateBlockResponseForALSP(channel, originID, message)
 		if err != nil {
 			irrecoverable.Throw(context.TODO(), fmt.Errorf("failed to validate block response from %x: %w", originID[:], err))
 		}
-		if !valid {
-			e.con.ReportMisbehavior(report) // report misbehavior to ALSP
-			e.log.
-				Warn().
-				Hex("origin_id", logging.ID(originID)).
-				Str(logging.KeySuspicious, "true").
-				Msgf("received invalid block response from %x: %v", originID[:], valid)
-		}
 		return e.responseMessageHandler.Process(originID, event)
 
 	case *messages.SyncResponse:
-		report, valid, err := e.validateSyncResponseForALSP(channel, originID, message)
+		err := e.validateSyncResponseForALSP(channel, originID, message)
 		if err != nil {
 			irrecoverable.Throw(context.TODO(), fmt.Errorf("failed to validate sync response from %x: %w", originID[:], err))
 		}
-		if !valid {
-			e.con.ReportMisbehavior(report) // report misbehavior to ALSP
-			e.log.
-				Warn().
-				Hex("origin_id", logging.ID(originID)).
-				Str(logging.KeySuspicious, "true").
-				Msgf("received invalid sync response from %x: %v", originID[:], valid)
-		}
 		return e.responseMessageHandler.Process(originID, event)
 	default:
 		return fmt.Errorf("received input with type %T from %x: %w", event, originID[:], engine.IncompatibleInputTypeError)
@@ -546,8 +530,8 @@ func (e *Engine) validateBatchRequestForALSP(originID flow.Identifier, batchRequ
 }
 
 // TODO: implement spam reporting similar to validateSyncRequestForALSP
-func (e *Engine) validateBlockResponseForALSP(channel channels.Channel, id flow.Identifier, blockResponse *messages.BlockResponse) (*alsp.MisbehaviorReport, bool, error) {
-	return nil, true, nil
+func (e *Engine) validateBlockResponseForALSP(channel channels.Channel, id flow.Identifier, blockResponse *messages.BlockResponse) error {
+	return nil
 }
 
 // validateRangeRequestForALSP checks if a range request should be reported as a misbehavior and sends misbehavior report to ALSP.
@@ -670,6 +654,6 @@ func (e *Engine) validateSyncRequestForALSP(originID flow.Identifier) error {
 }
 
 // TODO: implement spam reporting similar to validateSyncRequestForALSP
-func (e *Engine) validateSyncResponseForALSP(channel channels.Channel, id flow.Identifier, syncResponse *messages.SyncResponse) (*alsp.MisbehaviorReport, bool, error) {
-	return nil, true, nil
+func (e *Engine) validateSyncResponseForALSP(channel channels.Channel, id flow.Identifier, syncResponse *messages.SyncResponse) error {
+	return nil
 }

From 5cc2f1c0bd155f48ff828413753644d514c77284 Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Sat, 7 Oct 2023 18:05:45 -0500
Subject: [PATCH 23/25] log.Debug() for logging.KeyLoad

---
 engine/common/synchronization/engine.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/engine/common/synchronization/engine.go b/engine/common/synchronization/engine.go
index 8e6be30cb00..eea0392ae99 100644
--- a/engine/common/synchronization/engine.go
+++ b/engine/common/synchronization/engine.go
@@ -511,7 +511,7 @@ func (e *Engine) validateBatchRequestForALSP(originID flow.Identifier, batchRequ
 	batchRequestProb := e.spamDetectionConfig.batchRequestBaseProb * (float32(len(batchRequest.BlockIDs)) + 1) / float32(synccore.DefaultConfig().MaxSize)
 	if float32(n) < batchRequestProb*spamProbabilityMultiplier {
 		// create a misbehavior report
-		e.log.Warn().
+		e.log.Debug().
 			Hex("origin_id", logging.ID(originID)).
 			Str(logging.KeyLoad, "true").
 			Str("reason", alsp.ResourceIntensiveRequest.String()).
@@ -588,7 +588,7 @@ func (e *Engine) validateRangeRequestForALSP(originID flow.Identifier, rangeRequ
 	rangeRequestProb := e.spamDetectionConfig.rangeRequestBaseProb * (float32(rangeRequest.ToHeight-rangeRequest.FromHeight) + 1) / float32(synccore.DefaultConfig().MaxSize)
 	if float32(n) < rangeRequestProb*spamProbabilityMultiplier {
 		// create a misbehavior report
-		e.log.Warn().
+		e.log.Debug().
 			Hex("origin_id", logging.ID(originID)).
 			Str(logging.KeyLoad, "true").
 			Str("reason", alsp.ResourceIntensiveRequest.String()).
@@ -632,7 +632,7 @@ func (e *Engine) validateSyncRequestForALSP(originID flow.Identifier) error {
 	if float32(n) < e.spamDetectionConfig.syncRequestProb*spamProbabilityMultiplier {
 
 		// create misbehavior report
-		e.log.Warn().
+		e.log.Debug().
 			Hex("origin_id", logging.ID(originID)).
 			Str(logging.KeyLoad, "true").
 			Str("reason", alsp.ResourceIntensiveRequest.String()).

From 13b5713fad0a0049cb346295b95af86d886e3a8a Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Sat, 7 Oct 2023 18:12:41 -0500
Subject: [PATCH 24/25] delete line break between error return and error
 handling

---
 engine/common/synchronization/engine.go | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/engine/common/synchronization/engine.go b/engine/common/synchronization/engine.go
index eea0392ae99..9d16924908c 100644
--- a/engine/common/synchronization/engine.go
+++ b/engine/common/synchronization/engine.go
@@ -548,7 +548,6 @@ func (e *Engine) validateBlockResponseForALSP(channel channels.Channel, id flow.
 func (e *Engine) validateRangeRequestForALSP(originID flow.Identifier, rangeRequest *messages.RangeRequest) error {
 	// Generate a random integer between 0 and spamProbabilityMultiplier (exclusive)
 	n, err := rand.Uint32n(spamProbabilityMultiplier)
-
 	if err != nil {
 		return fmt.Errorf("failed to generate random number from %x: %w", originID[:], err)
 	}
@@ -561,7 +560,6 @@ func (e *Engine) validateRangeRequestForALSP(originID flow.Identifier, rangeRequ
 			Str("reason", alsp.InvalidMessage.String()).
 			Msgf("received invalid range request from height %d is not less than the to height %d, creating ALSP report", rangeRequest.FromHeight, rangeRequest.ToHeight)
 		report, err := alsp.NewMisbehaviorReport(originID, alsp.InvalidMessage)
-
 		if err != nil {
 			// failing to create the misbehavior report is unlikely. If an error is encountered while
 			// creating the misbehavior report it indicates a bug and processing can not proceed.
@@ -594,7 +592,6 @@ func (e *Engine) validateRangeRequestForALSP(originID flow.Identifier, rangeRequ
 			Str("reason", alsp.ResourceIntensiveRequest.String()).
 			Msgf("from height %d to height %d, creating probabilistic ALSP report", rangeRequest.FromHeight, rangeRequest.ToHeight)
 		report, err := alsp.NewMisbehaviorReport(originID, alsp.ResourceIntensiveRequest)
-
 		if err != nil {
 			// failing to create the misbehavior report is unlikely. If an error is encountered while
 			// creating the misbehavior report it indicates a bug and processing can not proceed.
@@ -622,7 +619,6 @@ func (e *Engine) validateRangeRequestForALSP(originID flow.Identifier, rangeRequ
 func (e *Engine) validateSyncRequestForALSP(originID flow.Identifier) error {
 	// Generate a random integer between 0 and spamProbabilityMultiplier (exclusive)
 	n, err := rand.Uint32n(spamProbabilityMultiplier)
-
 	if err != nil {
 		return fmt.Errorf("failed to generate random number from %x: %w", originID[:], err)
 	}
@@ -639,7 +635,6 @@ func (e *Engine) validateSyncRequestForALSP(originID flow.Identifier) error {
 			Msg("creating probabilistic ALSP report")
 
 		report, err := alsp.NewMisbehaviorReport(originID, alsp.ResourceIntensiveRequest)
-
 		if err != nil {
 			// failing to create the misbehavior report is unlikely. If an error is encountered while
 			// creating the misbehavior report it indicates a bug and processing can not proceed.

From 0d1fda2e4ac7647aca4a44e086b7ed1434ca00cf Mon Sep 17 00:00:00 2001
From: Misha <15269764+gomisha@users.noreply.github.com>
Date: Wed, 11 Oct 2023 17:48:42 -0700
Subject: [PATCH 25/25] golangci-lint version update (CI fix)

---
 .github/workflows/ci.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index b5e8f75f510..2e3d9262b05 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -50,7 +50,7 @@ jobs:
       uses: golangci/golangci-lint-action@v3
       with:
         # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
-        version: v1.51
+        version: v1.54
         args: -v --build-tags relic
         working-directory: ${{ matrix.dir }}
         # https://github.com/golangci/golangci-lint-action/issues/244