Improve Consensus logic for Multi-BLS Validators with quorum #4799
Conversation
GheisMohammadi
changed the title
|
@GheisMohammadi i am good with that PR. Can you do run the same logic with Line 124 in 3b6fde4 return when the leader node already has quorum on the first onPrepare. Goal is to run didReachPrepareQuorum Line 188 in 3b6fde4 |
GheisMohammadi
force-pushed
the
fix/quorum
branch
from
3b6fde4
to
c6518ec
Compare
done |
consensus/leader.go
Outdated
| quorumPreExisting := consensus.decider.IsQuorumAchieved(quorum.Prepare) | ||
| //// Read - End | ||
|
|
||
| if quorumPreExisting { | ||
| // already have enough signatures | ||
| consensus.getLogger().Debug(). | ||
| Interface("validatorPubKeys", recvMsg.SenderPubkeys). | ||
| Msg("[OnPrepare] Received Additional Prepare Message") | ||
| return |
There was a problem hiding this comment.
when leader node with multi bls key has quorum, we hit that return code and will never hit consensus.didReachPrepareQuorum()
There was a problem hiding this comment.
if !isFirstReceivedSignature && quorumPreExisting
first time with quorum we don't return so we can hit consensus.didReachPrepareQuorum(); then subsequently we return
| quorumFromInitialSignature := hasMultiBlsKeys && isFirstReceivedSignature && quorumPreExisting | ||
| quorumPostNewSignatures := consensus.decider.IsQuorumAchieved(quorum.Prepare) | ||
| quorumFromNewSignatures := !quorumPreExisting && quorumPostNewSignatures | ||
|
|
||
| if quorumFromInitialSignature || quorumFromNewSignatures { |
There was a problem hiding this comment.
old code was fine, as we just need consensus.didReachPrepareQuorum() to hit once
There was a problem hiding this comment.
I removed return. So, it allows adding new signatures in Prepare Phase despite existing quorum. I think this would fix the issue
sophoah
changed the title
|
could you add PR description, and is this something we can write unit test (for onPrepare and onCommit) here as well ? |
|
@Frozen please review/approve |
|
@GheisMohammadi please update PR description |
done |
Issue
This PR improves the consensus logic to handle cases where validators with multiple BLS keys achieve quorum on their initial signatures or commits. Currently, such scenarios can block phase transitions because the condition
if !quorumWasMet && quorumIsMetdoesn't account for quorum being achieved immediately.To fix this, a new function,
checkFirstReceivedSignature, is introduced. It checks whether quorum is reached on the first batch of signatures or commits. The PR also updates the logic forPrepareandCommitphases to properly handle these edge cases. The PR helps to better handling of multi-BLS key validators while maintaining compatibility with single-key setups.