Skip to content

Stop sending decided message (by iurii)#590

Draft
iurii-ssv wants to merge 5 commits intomainfrom
remove-decided-sending-iurii
Draft

Stop sending decided message (by iurii)#590
iurii-ssv wants to merge 5 commits intomainfrom
remove-decided-sending-iurii

Conversation

@iurii-ssv
Copy link
Contributor

Replaces (builds on #548)

@iurii-ssv iurii-ssv marked this pull request as draft December 6, 2025 08:19
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 6, 2025

Greptile Overview

Greptile Summary

Removes broadcasting of decided messages after QBFT consensus reaches a decision. This change eliminates redundant network traffic since nodes already receive commit messages and can determine consensus locally.

Major changes:

  • Removed broadcastDecided() method and its invocation in UponExistingInstanceMsg
  • Deleted SortedDecided test that validated signer ordering in broadcasted messages
  • Removed testBroadcastedDecided validation function from test infrastructure
  • Removed documentation for deleted tests

Note: The BroadcastedDecided field remains in DecidedState struct across 17 test files, but is no longer validated. This field is now effectively dead code that could be cleaned up in a follow-up PR.

Confidence Score: 4/5

  • Safe to merge with minor cleanup considerations
  • Changes cleanly remove broadcasting functionality without affecting core consensus logic. Tests pass and the removal aligns with optimization goals. Deducted one point because BroadcastedDecided field remains unused in 17+ test files as dead code.
  • No files require special attention - the changes are straightforward removals

Important Files Changed

File Analysis

Filename Score Overview
qbft/controller.go 5/5 Removed broadcastDecided method and its call after consensus decision - clean removal with no side effects
qbft/spectest/tests/commit/sorted_decided.go 5/5 Deleted test file that validated sorted signers in broadcasted decided messages - no longer needed
qbft/spectest/tests/controller_spectest.go 4/5 Removed testBroadcastedDecided validation function and imports - BroadcastedDecided field remains in struct but is no longer validated

Sequence Diagram

sequenceDiagram
    participant Client
    participant Controller
    participant Instance
    participant Network
    
    Note over Controller,Instance: Before PR (Broadcasting Decided)
    Client->>Controller: ProcessMsg(commit msg)
    Controller->>Instance: ProcessMsg(commit msg)
    Instance-->>Controller: decided=true, decidedMsg
    Controller->>Network: Broadcast(decidedMsg)
    Controller-->>Client: decidedMsg
    
    Note over Controller,Instance: After PR (No Broadcasting)
    Client->>Controller: ProcessMsg(commit msg)
    Controller->>Instance: ProcessMsg(commit msg)
    Instance-->>Controller: decided=true, decidedMsg
    Note over Controller,Network: Broadcasting removed
    Controller-->>Client: decidedMsg
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

5 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@github-actions
Copy link

github-actions bot commented Feb 6, 2026

This pull request has been marked as stale due to 60 days of inactivity.

@github-actions github-actions bot added the stale label Feb 6, 2026
@iurii-ssv iurii-ssv removed the stale label Feb 6, 2026
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