Skip to content

Don't verify a future proposal a second time when it is already scheduled - #576

Merged
yacovm merged 2 commits into
mainfrom
doubleVerify
Sep 9, 2026
Merged

Don't verify a future proposal a second time when it is already scheduled#576
yacovm merged 2 commits into
mainfrom
doubleVerify

Conversation

@yacovm

@yacovm yacovm commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

When a verification task consumes a parked finalization, the commit calls startRound, which schedules the next round's proposal from within the task. The outer maybeLoadFutureMessages loop then re-iterates for the new round and dispatches the same proposal again, because the task that would remove it from the future messages map has not run yet.

Skip proposals already marked as being processed, and clear that mark even when verification fails so a failed proposal can still be re-dispatched.

@yacovm
yacovm force-pushed the doubleVerify branch 2 times, most recently from 34e48e4 to aff33ce Compare September 3, 2026 20:50
@samliok

samliok commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

consumes a parked finalization

very claudish word

Comment thread simplex/epoch.go
for from, messagesFromNode := range e.futureMessages {
if msgs, exists := messagesFromNode[round]; exists {
if msgs.proposal != nil {
if msgs.proposal != nil && !msgs.proposalBeingProcessed {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in your other pr we have something similar https://github.com/ava-labs/Simplex/pull/570/changes. where we have a pending rounds for blocks being processed. i'm wondering if we can simplify these two bugs into one solution?

also why can't we just delete the message from future messages when we send it to handleBlockMessage?

@yacovm yacovm Sep 3, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two bugs are unrelated, but I did use this idea 👍 : take a look here.

@yacovm
yacovm force-pushed the doubleVerify branch 3 times, most recently from 4c1184a to e9e2e3f Compare September 4, 2026 18:18
@yacovm
yacovm force-pushed the doubleVerify branch 2 times, most recently from bd870a1 to ff68902 Compare September 8, 2026 22:13
@yacovm

yacovm commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator Author
[09-08|22:15:10.518] WARN testutil/logger.go:87 Too many blocks being verified to ingest another one {"pendingBlocks": 500}
    replication_backpressure_test.go:121: handling the replication response with a full verification queue returned: too many blocks being verified to ingest another one: 500 pending verifications (max 500)
    replication_backpressure_test.go:129: 
        	Error Trace:	/home/runner/work/Simplex/Simplex/simplex/replication_backpressure_test.go:129
        	Error:      	Condition never satisfied
        	Test:       	TestReplicationSurvivesFullVerificationQueue
        	Messages:   	node never committed the replicated sequences after the verification queue drained

…uled

When a verification task consumes a parked finalization, the commit calls
startRound, which schedules the next round's proposal from within the task.
The outer maybeLoadFutureMessages loop then re-iterates for the new round
and dispatches the same proposal again, because the task that would remove
it from the future messages map has not run yet.

Skip proposals already marked as being processed, and clear that mark even
when verification fails so a failed proposal can still be re-dispatched.

Signed-off-by: Yacov Manevich <yacov.manevich@avalabs.org>
@yacovm
yacovm merged commit f5f1936 into main Sep 9, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants