Skip to content

Conversation

@kstroobants
Copy link
Contributor

@kstroobants kstroobants commented Jul 29, 2025

Fixes #DXP-499

What

  • Updated the PendingState class in backend/consensus/base.py to handle leader-only transactions by setting the number of validators to 1. And modified the ProposingState class to no longer clear validators for leader-only transactions. So, we do not choose the 5 validators and then remove 4.
  • Adjusted the CommittingState and RevealingState classes to account for leader-only transactions when emitting events.
  • Updated the determine_consensus_from_votes function in backend/consensus/utils.py to handle leader-only transactions by returning a majority agree result. This way the transaction gets accepted and the contract gets deployed. If we did not do this then no votes means no majority which means undetermined state.
  • Enhanced the TransactionsProcessor class in backend/database_handler/transactions_processor.py to correctly process leader-only transactions.
  • Updated the TransactionItem.vue component in the frontend to correctly display the votes.

Why

  • To fix a bug related to leader-only transactions not being handled correctly in the consensus process.

Testing done

  • Tested in the local studio but running transactions.

Decisions made

Checks

  • I have tested this code
  • I have reviewed my own PR
  • I have created an issue for this PR
  • I have set a descriptive PR title compliant with conventional commits

Reviewing tips

  • Focus on the changes.

User facing release notes

  • Improved handling of leader-only transactions in the consensus process.
  • Enhanced frontend display of transaction votes.

Summary by CodeRabbit

  • New Features

    • Enhanced handling for transactions marked as "leader only," ensuring they follow a streamlined consensus process with single-validator execution and distinct event signaling.
  • Bug Fixes

    • Improved display logic in the transaction simulator to accurately show leader results and vote statuses, providing clearer consensus history for users.

@kstroobants kstroobants self-assigned this Jul 29, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 29, 2025

Walkthrough

The changes implement specialized handling for leader_only transactions throughout the consensus process, affecting validator selection, vote collection, and event emission. The consensus determination function now takes a leader_only flag, and relevant backend and frontend logic is updated to consistently process and display consensus results for such transactions.

Changes

Cohort / File(s) Change Summary
Consensus State Handling
backend/consensus/base.py
Updated consensus state methods to treat leader_only transactions differently in validator selection, transaction execution, vote handling, and event emission. Logic for handling votes and receipts is now conditional on the leader_only flag.
Consensus Utility Function
backend/consensus/utils.py
Modified determine_consensus_from_votes to accept a leader_only flag and return consensus immediately for such transactions, bypassing normal vote evaluation. Function signature updated accordingly.
Transaction Processing
backend/database_handler/transactions_processor.py
Updated consensus result processing to pass the leader_only flag to the consensus determination function, and clarified leader result validation logic. Ensures consistent consensus handling for leader_only transactions in both round data and result processing.
Frontend Consensus Display
frontend/src/components/Simulator/TransactionItem.vue
Adjusted consensus history rendering to correctly display leader results and validator votes for transactions, reflecting new backend logic and simplifying vote status display.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Frontend
    participant Backend
    participant ConsensusEngine

    User->>Frontend: Initiates transaction (may be leader_only)
    Frontend->>Backend: Submits transaction
    Backend->>ConsensusEngine: Processes transaction
    alt leader_only transaction
        ConsensusEngine->>ConsensusEngine: Select 1 validator (leader)
        ConsensusEngine->>ConsensusEngine: Skip multi-validator voting
        ConsensusEngine->>Backend: Return immediate consensus (majority agree)
    else normal transaction
        ConsensusEngine->>ConsensusEngine: Select initial validators
        ConsensusEngine->>ConsensusEngine: Collect votes
        ConsensusEngine->>Backend: Return consensus based on votes
    end
    Backend->>Frontend: Return consensus result
    Frontend->>User: Display consensus history and vote results
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~15 minutes

Suggested labels

run-tests

Suggested reviewers

  • cristiam86

Poem

A leader stands alone, so bold,
Deciding fates as stories unfold.
No need for votes from all the crew,
The consensus path is swift and true.
With frontend flair and backend might,
The code now treats the leader right!
🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 128e60f and 1fe5c7c.

📒 Files selected for processing (4)
  • backend/consensus/base.py (4 hunks)
  • backend/consensus/utils.py (1 hunks)
  • backend/database_handler/transactions_processor.py (3 hunks)
  • frontend/src/components/Simulator/TransactionItem.vue (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Load Tests / load-test
  • GitHub Check: test
  • GitHub Check: Sherlock AI Review
  • GitHub Check: backend-unit-tests
🔇 Additional comments (14)
backend/consensus/utils.py (2)

5-7: LGTM: Function signature update is well-structured.

The addition of the leader_only parameter is correctly typed and positioned. The parameter naming is clear and follows the existing codebase conventions.


18-19: Verify leader-only transactions short-circuit to MAJORITY_AGREE

I performed a global search and found no other references to the leader_only flag in the consensus workflow. As it stands, backend/consensus/utils.py (lines 18–19) will always return ConsensusResult.MAJORITY_AGREE for leader-only transactions, bypassing any vote tally.

Please confirm that:

  • Leader-only transactions are intended to skip validator voting and be treated as implicitly agreed.
  • This aligns with the overall consensus protocol specification.
  • If this is by design, consider adding a clarifying code comment or a unit test in backend/consensus/utils.py to document and verify this shortcut.
frontend/src/components/Simulator/TransactionItem.vue (4)

422-426: LGTM: Improved conditional rendering logic.

The updated conditional logic properly checks for both the existence and minimum length of leader_result before rendering. This prevents potential runtime errors when accessing leader_result[1].


438-438: LGTM: Consistent data structure access.

Accessing history.leader_result[1].node_config.address is consistent with the conditional check and the updated data structure expectations.


442-455: LGTM: Simplified and clearer vote display logic.

The vote status rendering has been improved by:

  • Removing redundant conditional branches
  • Using explicit templates for each vote type
  • Accessing the correct data element (leader_result[1].vote)

This makes the code more readable and maintainable.


430-432: No changes needed for leader_result indexing
The backend’s transactions_processor.py already populates leader_result as a list of receipt dicts and safely accesses index 1 when len(leader_result) > 1. This matches the front-end’s check:

v-if="history?.leader_result && history.leader_result.length > 1"

No further adjustments are required.

backend/database_handler/transactions_processor.py (3)

311-315: LGTM: Excellent defensive programming.

The added safety checks prevent potential IndexError when accessing leader_result[1]. The conditions properly verify:

  • leader_result key exists
  • leader_result is not None
  • leader_result has more than one element

This is consistent with the frontend changes and prevents runtime errors.


342-345: LGTM: Consistent parameter passing for consensus determination.

The addition of transaction_data["leader_only"] as a parameter to determine_consensus_from_votes ensures that consensus determination properly respects the leader-only transaction status. This aligns perfectly with the changes made to the consensus utility function.


550-552: LGTM: Consistent leader_only parameter usage.

The addition of transaction_data["leader_only"] parameter maintains consistency with the updated determine_consensus_from_votes function signature and ensures proper consensus determination for leader-only transactions throughout the processing pipeline.

backend/consensus/base.py (5)

1831-1836: LGTM!

The implementation correctly sets the number of validators to 1 for leader-only transactions, ensuring only the leader participates in the consensus process.


1998-2007: LGTM!

Correctly skips redundant leader transaction execution and vote recording for leader-only transactions, as the leader has already executed in the ProposingState.


2031-2043: LGTM!

Appropriately skips vote commitment event emission for the leader in leader-only transactions, maintaining consistency with the execution logic.


2096-2100: LGTM!

Correctly skips vote reveal event emission for the leader in leader-only transactions, maintaining consistency with the CommittingState implementation.


2091-2093: Function signature verified and changes approved.

The determine_consensus_from_votes function in backend/consensus/utils.py (now defined as

def determine_consensus_from_votes(
    votes_list: list[str], leader_only: bool
) -> ConsensusResult:
    …
)

(lines 5–7) correctly accepts the leader_only flag. The call in backend/consensus/base.py (lines 2091–2093) now passes context.transaction.leader_only as intended, and the consensus logic handles leader-only transactions appropriately. No further changes required.

  • backend/consensus/utils.py: Signature includes leader_only: bool
  • backend/consensus/base.py: Passing context.transaction.leader_only
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch dxp-499-fix-incorrect-rotation-behavior-on-execution-error

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
50.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@sherlock-ai-beta
Copy link

PR Summary

Here's a concise summary of the changes in the Pull Request:

  • Purely documentation/comment updates across multiple contracts (Messages, Transactions, ConsensusMain, MockGenStaking) with no functional changes
  • Key documentation improvements include:
    • Enhanced descriptions of security mechanisms and ownership transfers in Messages contract
    • Clarification of message processing flow and access control systems
  • No code changes to:
    • Function implementations
    • State variables/mappings
    • Modifiers or events
  • All contract behaviors remain identical before and after changes
  • Possible purposes:
    • Improved code readability/documentation
    • Cosmetic/formatting adjustments (though not visible in diffs)

Recommendations for developers:

  1. Verify if any intended functional changes were accidentally omitted
  2. Confirm new descriptions accurately reflect contract purposes
  3. Check if further documentation updates are needed elsewhere
  4. Determine if this was intentionally a documentation-only PR

@sherlock-ai-beta
Copy link

Sherlock AI Findings

The automated tool completed its analysis of the codebase and found no potential security issues.

Next Steps: No immediate actions are required. Continue monitoring the codebase with future scans.

Full report available at: https://ai.sherlock.xyz/runs/7c9eb8bc-cc6b-4ed4-8325-bcd04452c01a

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