Skip to content

refactor: rename BLSMessage to ConsensusMessage#413

Merged
akhi3030 merged 4 commits into
masterfrom
akhi3030/consensus-message
Aug 22, 2025
Merged

refactor: rename BLSMessage to ConsensusMessage#413
akhi3030 merged 4 commits into
masterfrom
akhi3030/consensus-message

Conversation

@akhi3030
Copy link
Copy Markdown
Contributor

@akhi3030 akhi3030 commented Aug 21, 2025

Problem

BLS happens to be implementation detail. It should not be captured in the name of the message.

Summary of Changes

Renames BLSMessage to ConsensusMessage

Fixes #

Closes #365.

@akhi3030 akhi3030 force-pushed the akhi3030/consensus-message branch 6 times, most recently from e64dc17 to f907485 Compare August 21, 2025 13:40
@akhi3030 akhi3030 changed the title wip refactor: rename BLSMessage to ConsensusMessage Aug 21, 2025
@akhi3030 akhi3030 marked this pull request as ready for review August 21, 2025 13:45
@akhi3030 akhi3030 force-pushed the akhi3030/consensus-message branch from f907485 to 6d6afe2 Compare August 21, 2025 13:46
@akhi3030 akhi3030 enabled auto-merge (squash) August 21, 2025 13:49
Comment thread runtime/src/vote_sender_types.rs
Copy link
Copy Markdown
Contributor

@AshwinSekar AshwinSekar left a comment

Choose a reason for hiding this comment

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

LGTM, pending conclusion of wen's issue

Comment thread core/src/tpu.rs Outdated
root_bank_cache,
verified_vote_sender.clone(),
bls_verified_message_sender,
consensus_message_sender,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The incoming messages are also of type ConsensusMessage, I do prefer to keep verified in the name here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated in 5fcf524.

Comment thread votor/src/voting_utils.rs Outdated
| Self::WaitToVoteSlot(_)
| Self::NoRankFound => false,
Self::Tx(_) | Self::BLSMessage(_) => false,
Self::Tx(_) | Self::Consensus(_) => false,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this Consensus instead of ConsensusMessage?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I thought it was a bit verbose. Updated in 5fcf524.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, can approve after conflicts are resolved.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Merged the latest master into the branch and resolved the conflicts.

@akhi3030 akhi3030 requested a review from wen-coding August 22, 2025 07:49
@akhi3030 akhi3030 merged commit 4c36c9b into master Aug 22, 2025
3 of 7 checks passed
@akhi3030 akhi3030 deleted the akhi3030/consensus-message branch August 22, 2025 16:43
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.

Rename BLSMessage to ConsensusMessage or AlpenglowMessage or something appropriate

3 participants