fix: Ignore duplicate p2p messages post-validation#18027
Closed
spalladino wants to merge 1 commit intov2from
Closed
fix: Ignore duplicate p2p messages post-validation#18027spalladino wants to merge 1 commit intov2from
spalladino wants to merge 1 commit intov2from
Conversation
Contributor
Author
|
Closing in favor of #18034 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Today we have 3 deduplication mechanisms for messages:
fastMsgIdcache, where gossipsub deduplicates messages based on a hash computed over the compressed message buffer:msgIdcache, where gossipsub deduplicates messages based on a hash computed over the topic and the decompressed message buffer:msgIdSeenValidatorswe manually implement inLibP2PService, where we deduplicate based on themsgIdagain (not using the customp2pMessageIdentifierfromGossipable), but with a larger cache, based on capacity instead of TTL:All of these deduplications run after any validation is done. This PR adds an additional check for duplicates after validation, where we deduplicate against the attestation and tx pools, using the object identifiers:
Returning
Ignoreto gossipsub causes it to delete the message from itsmcache(but not from itsseenCache) without penalizing the sender, and instructs it to not re-broadcast the message.Note that we cannot rely on the identifiers before validation since they could be forged: eg an attacker could pick up an identifier for a valid message, change its data with garbage, and forward it so it fails validation. When the real message is received afterwards, it gets rejected because it had previously failed. We need to run these id-based deduplications after validation so we know that the identifier corresponds to the payload.
This change prevents a malicious message originator from slightly modifying part of its message and broadcasting it across the network repeatedly, causing it to be re-processed and re-broadcasted. Eg a malicious attester could rely on non-deterministic ecdsa signatures to produce different valid signatures for the same attestation and broadcast them all. This change would only accept the first attestation and reject all others.
Fixes A-206