Global shared state validation - Duplicated message issue #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Open

MatheusFranco99 wants to merge 2 commits into boole-msg-validation from duplicate-msg-protection

Contributor

MatheusFranco99 commented Jan 16, 2026

Overview

This PR solves the issue with message validation caused by a duplicated node running,
which can cause a honest node to propagate duplicated messages.


          add global shared state validation

c239a6c

MatheusFranco99 requested a review from GalRogozinski

January 16, 2026 18:22

MatheusFranco99 self-assigned this

MatheusFranco99 changed the title ~~add global shared state validation~~ Global shared state validation - Duplicated message issue

iurii-ssv reviewed

View reviewed changes

iurii-ssv left a comment

LGTM, 1 other thing I would also maybe cover in this write-up is about serializing message validation, namely:

we receive many different messages from different peers in parallel
to validate the message against peer-state (and record validation results for future re-use) we acquire a lock identified by peerID+msgID and release it once we are done with peer-state validation
to validate the message against global-state (and record validation results for future re-use) we acquire a lock identified by msgID and release it once we are done with global-state validation

p2p/MessageValidation/Rules.md Outdated

    
              ![Covert Attack](images/covert_attack.png)

              This image illustrates the *Covert Attack*, in which a peer malicious sends two "logically duplicated" messages

iurii-ssv Jan 25, 2026

in which a peer malicious sends -> in which a malicious peer sends

p2p/MessageValidation/Rules.md Outdated

    
              Most importantly, note that such a global shared state is exactly what others should understand as this peer's specific-state

              (i.e. `A.global_shared_state == B.peer_specific_state[A]` for any A and B with appropriate synchronization).

              Thus, if the message is accepted against such a state, other peers will also accept it.

iurii-ssv Jan 25, 2026

Thus, if the message is accepted against such a state, other peers will also accept it. -> Thus, if peer A accepts the message against his global state, other peers will also accept the broadcast of this message from peer A.


          improve wording and optimize

4464b48

GalRogozinski requested review from dknopik and y0sher

January 26, 2026 09:38

iurii-ssv reviewed

View reviewed changes

iurii-ssv left a comment

Left some suggestions

p2p/MessageValidation/Rules.md

Comment on lines +141 to +148

    
              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).

iurii-ssv Jan 26, 2026

This section hasn't been changed to reflect the check is inverted now (now we check the global state first, then we check peer-specific state) ?

Also, throughout this document we are using "Ignore" and "IGNORE" (and same for rejects) interchangeably ... probably better to use just one of these.

p2p/MessageValidation/Rules.md

    
              - 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

iurii-ssv Jan 26, 2026

Since this section describes validation in general (not just global-state validation) maybe change the header to something like Global Shared State Validations -> Global vs Peer State Validations

p2p/MessageValidation/Rules.md

    
                  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.

iurii-ssv Jan 26, 2026

I think here we are talking about global state:

For any rule that, if violated in peer-specific state, would cause rejection -> For any rule that, if violated in global state, would cause ignore or reject

... but this whole summary doesn't look 100% correct to me, I would remove it entirely given that it just tries to make the detailed description provided above shorter while actually "making it harder to understand how the validation flow is supposed to work in step-by-step manner".

p2p/MessageValidation/Rules.md

Comment on lines +1100 to +1102

    
              **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.

iurii-ssv Jan 26, 2026 •

edited

Loading

Since section-1. already describes how to use peer-specific state to validate messages this whole section-2.:

isn't really adding anything new
and I would even say it only adds confusion because it says messages MAY be initially checked against peer-specific state while the section 1. says that we MUST check the global state first (the node MUST first validate the message against all protocol rules using the *global shared state*)

iurii-ssv reviewed

View reviewed changes

p2p/MessageValidation/Rules.md

Comment on lines +1095 to +1098

    
                  - 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.

iurii-ssv Jan 26, 2026 •

edited

Loading

There are 2 types of "REJECT rules":

those that DO NOT require an additional check against peer-state (eg. Error{text: "signer is not expected", reject: true})
those that DO require, which is basically what 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 describes

so I would change/expand this section like this:

    - If the rule is not a REJECT rule, the node MUST IGNORE the message.
    - If the rule is a REJECT rule and it doesn't require an additional check agaisnt **peer-specific state**, the node MUST REJECT the message.
	- If the rule is a REJECT rule and it does require an additional check agaisnt **peer-specific state**, 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet