Global shared state validation - Duplicated message issue

p2p/MessageValidation/Rules.md

@@ -120,7 +120,7 @@ var (
 
 	ErrPartialSigOneSigner              		        = Error{text: "partial signature message with len(signers) != 1", reject: true}
 	ErrTooManyPartialSignatureMessages  		        = Error{text: "too many signatures for cluster in partial signature message"}
-    ErrTooManyEqualValidatorIndicesInPartialSignatures  = Error{text: "validator index appears too many times in partial signature message", reject: true}
+	ErrTooManyEqualValidatorIndicesInPartialSignatures  = Error{text: "validator index appears too many times in partial signature message", reject: true}
 	ErrNoPartialSignatureMessages       		        = Error{text: "no partial signature messages", reject: true}
 	ErrInconsistentSigners              		        = Error{text: "inconsistent signers", reject: true}
 	ErrValidatorIndexMismatch           		        = Error{text: "validator index mismatch"}
@@ -137,6 +137,16 @@ var (
 
 The main structure is the `MessageValidation` structure which has a `ValidatePubsubMessage` function to serve as a handle for the GossipSub extended validator.
 
+The function calls `ValidateMessage` which recursively calls every validation chain: Syntax -> Semantics -> QBFT Semantics | Partial Signature Semantics -> QBFT Logic -> Duty Rules.
+All rules are verified against the peer-specific state, and after it:
+- If there's any error, the appropriate `Ignore` or `Reject` is returned.
+- Else, the message is again validated but this time against the global shared state.
+This prevents the node from propagating duplicated messages. Then:
+  - In case there's any error, `Ignore` is returned (even if the triggered rule has `reject: true`) as we can't blame the peer for the error.
+  - Else, `Accept` is returned.
+
+More about the global shared state validation is discussed [below](#global-shared-state-validations).
+
 ```go
 
 const (
@@ -175,27 +185,35 @@ func (mv *MessageValidation) Validate(_ context.Context, _ peer.ID, pmsg *pubsub
 
 	// Check error
 	if err != nil {
-
-		var valErr Error
-		if errors.As(err, &valErr) {
-			// Update state
-			peerState.OnError(valErr)
-
-			if valErr.Reject() {
-				// Reject
-				return pubsub.ValidationReject
-			} else {
-				// Ignore
-				return pubsub.ValidationIgnore
-			}
-		} else {
-			panic(err)
-		}
+		return GetPubSubValidatoinResult(peerState, err)
 	} else {
+		// If the message is successful after all rules are tested on the peer-specific state,
+		// test it as well on the global shared state to avoid propagating duplicated
+		err = mv.ValidateAgainstGlobalSharedState(pmsg)
+		if err != nil {
+			return pubsub.ValidationIgnore
+        }
 		return pubsub.ValidationAccept
 	}
 }
 
+func GetPubSubValidatoinResult(peerState *PeerState, err error) pubsub.ValidationResult {
+    var valErr Error
+    if errors.As(err, &valErr) {
+        // Update state
+        peerState.OnError(valErr)
+        if valErr.Reject() {
+            // Reject
+            return pubsub.ValidationReject
+        } else {
+            // Ignore
+            return pubsub.ValidationIgnore
+        }
+    } else {
+        panic(err)
+    }
+}
+
 func (mv *MessageValidation) VerifyMessageSignature(pmsg *pubsub.Message) error {
 	// Already verified
 	signedSSVMessage := &types.SignedSSVMessage{}
@@ -1064,6 +1082,61 @@ func (mv *MessageValidation) ValidatePartialSigMessagesByDutyLogic(peerID peer.I
 
 - Proposal and round-change justifications were not included because they are too complex to implement at the message validation level. The cost of adding this complexity is not justified since the message count check already prevents any related attack.
 
+### Global Shared State Validations
+
+All message validation procedures MUST follow the order and outcome-directed handling below, using RFC 2119 conventions for requirement levels.
+
+**1. Global Shared State Validation**
+
+Upon receipt of a message, the node MUST first validate the message against all protocol rules using the *global shared state*. This state reflects an aggregate view of all messages accepted by the node, regardless of which peer sent them.
+
+- If the message passes all rules in the global shared state, the node MUST proceed to accept the message.
+- If any rule is triggered during global shared state validation:
+    - If the corresponding rule may later result in rejection when applied to peer-specific state (i.e., is classified as REJECT in this document), the node MUST proceed to validate the message against *peer-specific state* for this peer:
+        - If the message fails the peer-specific check for this rule, the node MUST REJECT the message according to the rule's classification.
+        - If the message passes the peer-specific check but failed the global check, the node MUST IGNORE the message. This ensures the peer is not penalized for a condition it cannot control.
+    - If the rule is not a REJECT rule, the node MUST IGNORE the message.
+
+**2. Peer-Specific State Validation**
+
+For completeness, prior to any global state check, messages MAY be initially checked against peer-specific state for any checks directly related to penalizing peer misbehavior relative to their historical activity. Peer-specific checks MUST only result in penalization or rejection if the message definitively violates protocol rules in the context of that peer’s known state.
+
+**Rationale and Security Considerations**
+
+This approach is required to prevent scenarios such as the *Covert Attack*, illustrated below, where malicious actors distribute logically conflicting messages to different peers to avoid immediate penalization, while still causing protocol violations affecting a node’s global state.
+
+![Covert Attack](images/covert_attack.png)
+
+**Correctness**: The global shared state is intended to reflect what other peers would understand as any given node’s state (i.e., `A.global_shared_state == B.peer_specific_state[A]`, assuming synchronization). By enforcing global state checks first, nodes avoid creating and propagating protocol-violating conditions, even in the face of asymmetric message delivery.
+
+**Pseudocode Summary**:
+
+```mermaid
+flowchart LR
+    A[Receive message from peer]
+    B[Rules validation with global shared state]
+    C{Any rule triggered?}
+    D[Is rule 'reject'-typed?]
+    D2[Peer-specific check for this rule]
+    E[Reject message]
+    F[Ignore message]
+    G[Accept message]
+
+    A --> B
+    B --> C
+    C -- No --> G
+    C -- Yes --> D
+    D -- Yes --> D2
+    D2 -- Fail --> E
+    D2 -- Pass --> F
+    D -- No --> F
+```
+
+**Summary**: The node MUST apply all rule checks first to the global shared state. For any rule that, if violated in peer-specific state, would cause rejection, a secondary peer-specific check MUST be performed. If the message fails that peer-specific check, it MUST be REJECTED. If it passes the peer-specific check but still violates the rule globally, the message MUST be IGNORED. If it passes all checks globally, it MUST be ACCEPTED.
+
+This flow strictly prevents penalization of peers for state they do not control, while ensuring the node does not violate protocol integrity due to global state inconsistencies.
+
+
 
 ### Rules suggestions for future
 - Priority-based message handling: priority based on type and sender.