has not check commitVotes

semuxproject / semux-core

Semux Core

https://www.semux.org

MIT License

76 stars 32 forks source link

has not check commitVotes #303

Closed VictorECDSA closed 3 years ago

VictorECDSA commented 3 years ago

In the Finalize stage, the number of precommitVotes is checked instead of the number of commitVotes. Does that mean that even if there is no commit stage, as long as timeout triggers enterFinalize, the block will grow? Thank you!

tell-1c commented 3 years ago

pls, give a link to code for reference this issue.

VictorECDSA commented 3 years ago

pls, give a link to code for reference this issue.

src/main/java/org/semux/consensus/SemuxBft.java

    protected void enterFinalize() {

        Optional<byte[]> blockHash = precommitVotes.anyApproved();

I think here should be commitVotes.anyApproved();

thx!

honeycrypto commented 3 years ago

Reopening it, this was not resolved in ec75fc9, but #306

VictorECDSA commented 3 years ago

Reopening it, this was not resolved in ec75fc9, but #306

I think #306 has not resolved this problem

honeycrypto commented 3 years ago

Yup, I ment this commit solved other issue

semuxgo commented 3 years ago

There are two paths that could lead to enterFinalize:

When 2/3 of the validators have signalled COMMIT vote (see the 2/3 vote check here)
Time out after the commit phase (see this code)

Compared with the original PBFT algorithm, SemuxBFT works as follows:

PRE_PREPARE message     -> BFT_PROPOSAL message

PREPARE message         -> BFT_VOTE message (VALIDATE vote)

<prepared certificate>  -> 2/3 VALIATE certificate

COMMIT message          -> BFT_VOTE message (PRECOMMIT vote)

<commited certificate>  -> 2/3 PRECOMMIT certificate

Commit state change     -> Broadcast BFT_VOTE message (COMMIT vote)
                           Wait for 2/3 COMMIT votes or timeout (whichever comes first)
                           Commit state change

As you see, the COMMIT votes are mainly for network synchronization purpose. It is not part of the certificates and is not related to COMMIT messages in PBFT.

VictorECDSA commented 3 years ago

There are two paths that could lead to enterFinalize:

When 2/3 of the validators have signalled COMMIT vote (see the 2/3 vote check here)

Time out after the commit phase (see this code)

Compared with the original PBFT algorithm, SemuxBFT works as follows:
PRE_PREPARE message     -> BFT_PROPOSAL message

PREPARE message         -> BFT_VOTE message (VALIDATE vote)

<prepared certificate>  -> 2/3 VALIATE certificate

COMMIT message          -> BFT_VOTE message (PRECOMMIT vote)

<commited certificate>  -> 2/3 PRECOMMIT certificate

Commit state change     -> Broadcast BFT_VOTE message (COMMIT vote)
                           Wait for 2/3 COMMIT votes or timeout (whichever comes first)
                           Commit state change
As you see, the COMMIT votes are mainly for network synchronization purpose. It is not part of the certificates and is not related to COMMIT messages in PBFT.

I see. Thank you