Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chain stalls while scaling out from 1 to 4 nodes, changing quorum size fixes things #796

Merged
merged 34 commits into from
Sep 30, 2019
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
c8e9178
initial commit for pr
panghalamit Aug 7, 2019
bc55567
replaced quorum size in roundchange.go with ceil(2N/3) to fix chain s…
panghalamit Aug 7, 2019
a914f8a
updated quorumsize unit test to use CONSTANTS as defined
panghalamit Aug 8, 2019
3dee78e
updated to include use quorumsizewhere applicable and changed default…
panghalamit Aug 8, 2019
c34d30f
Merge branch 'master' into ibft-quorum-formula
panghalamit Aug 9, 2019
6098dc1
updated quorum formula to ceil(2N/3)
panghalamit Aug 16, 2019
90123ae
Merge branch 'master' into ibft-quorum-formula
panghalamit Aug 16, 2019
97123e2
Merge branch 'ibft-quorum-formula' of github.com:kaleido-io/quorum in…
panghalamit Aug 16, 2019
247e48f
initial commit for pr
panghalamit Aug 7, 2019
125b3eb
replaced quorum size in roundchange.go with ceil(2N/3) to fix chain s…
panghalamit Aug 7, 2019
d0a5b41
updated quorumsize unit test to use CONSTANTS as defined
panghalamit Aug 8, 2019
637d669
v2.2.5
jpmsam Aug 8, 2019
c8d3391
updated to include use quorumsizewhere applicable and changed default…
panghalamit Aug 8, 2019
807ccaa
VM read-only mode: check `operation.writes` not `op.isMutating`.
libby Aug 9, 2019
8886310
Tessera 0.10.0 - Documentation update (#801)
Krish1979 Aug 13, 2019
4d2319d
updated quorum formula to ceil(2N/3)
panghalamit Aug 16, 2019
2a3f5e1
quorum size formula doesnt take any inputs and only returns value by …
panghalamit Aug 22, 2019
b55bddd
rebased with master
panghalamit Aug 22, 2019
9d4ffd6
removed constants
panghalamit Aug 22, 2019
2073b07
cleaning up unused constants
panghalamit Aug 22, 2019
0158e45
Merge branch 'master' into ibft-quorum-formula
panghalamit Aug 22, 2019
031daa0
updated missed import
panghalamit Aug 22, 2019
a98c32f
updated with suggested changes
panghalamit Aug 22, 2019
ef496ff
Merge branch 'master' into ibft-quorum-formula
panghalamit Aug 23, 2019
68f568a
updated inconsistent testcase
panghalamit Aug 27, 2019
2cf2657
Merge branch 'ibft-quorum-formula' of github.com:kaleido-io/quorum in…
panghalamit Aug 27, 2019
b380271
Merge branch 'master' into ibft-quorum-formula
panghalamit Aug 27, 2019
f84b5f9
Adding Ceil2Nby3Block genesis config option to change the number of c…
jbhurat Sep 18, 2019
60bb862
Merge branch 'master' into ibft-quorum-formula
jimthematrix Sep 25, 2019
c05673f
Adding a nil check for newcfg.Istanbul so that geth init doesn't retu…
jbhurat Sep 25, 2019
ae62307
Merge remote-tracking branch 'jbhurat/Ceil2Nby3Block' into ibft-quoru…
jimthematrix Sep 27, 2019
75ecbf4
Removed duplicate impl of QuorumSize() in validator
jimthematrix Sep 27, 2019
9a45052
Addressing review comments from Jitu
jimthematrix Sep 30, 2019
cd0a976
Updated nodeinfo tests based on Jitu's feedback
jimthematrix Sep 30, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions consensus/istanbul/backend/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ import (
"github.com/ethereum/go-ethereum/common/hexutil"
"github.com/ethereum/go-ethereum/consensus"
"github.com/ethereum/go-ethereum/consensus/istanbul"
istanbulCore "github.com/ethereum/go-ethereum/consensus/istanbul/core"
"github.com/ethereum/go-ethereum/consensus/istanbul/validator"
istanbulCore "github.com/ethereum/go-ethereum/consensus/istanbul/core"
"github.com/ethereum/go-ethereum/core/state"
"github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/crypto/sha3"
Expand Down Expand Up @@ -289,8 +289,8 @@ func (sb *backend) verifyCommittedSeals(chain consensus.ChainReader, header *typ
}
}

// The length of validSeal should be larger than number of faulty node + 1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the master the condition is
if validSeal < 2*snap.ValSet.F()
shouldn't it be this instead?
if validSeal <= snap.ValSet.F()

if validSeal <= 2*snap.ValSet.F() {
// The validSeal number should be greater than equal to ceil(2N/3) i.e., quorum size
if validSeal < snap.ValSet.QuorumSize() {
panghalamit marked this conversation as resolved.
Show resolved Hide resolved
return errInvalidCommittedSeals
}

Expand Down
2 changes: 1 addition & 1 deletion consensus/istanbul/core/commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ func (c *core) handleCommit(msg *message, src istanbul.Validator) error {
//
// If we already have a proposal, we may have chance to speed up the consensus process
// by committing the proposal without PREPARE messages.
if c.current.Commits.Size() > 2*c.valSet.F() && c.state.Cmp(StateCommitted) < 0 {
if c.current.Commits.Size() >= c.valSet.QuorumSize() && c.state.Cmp(StateCommitted) < 0 {
// Still need to call LockHash here since state can skip Prepared state and jump directly to the Committed state.
c.current.LockHash()
c.commit()
Expand Down
10 changes: 5 additions & 5 deletions consensus/istanbul/core/commit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,18 +191,18 @@ OUTER:
if r0.state != StatePrepared {
t.Errorf("state mismatch: have %v, want %v", r0.state, StatePrepared)
}
if r0.current.Commits.Size() > 2*r0.valSet.F() {
t.Errorf("the size of commit messages should be less than %v", 2*r0.valSet.F()+1)
if r0.current.Commits.Size() >= r0.valSet.QuorumSize() {
t.Errorf("the size of commit messages should be less than %v", r0.valSet.QuorumSize())
}
if r0.current.IsHashLocked() {
t.Errorf("block should not be locked")
}
continue
}

// core should have 2F+1 prepare messages
if r0.current.Commits.Size() <= 2*r0.valSet.F() {
t.Errorf("the size of commit messages should be larger than 2F+1: size %v", r0.current.Commits.Size())
// core should have ceil(2N/3) prepare messages for N validators
if r0.current.Commits.Size() < r0.valSet.QuorumSize() {
t.Errorf("the size of commit messages should be atleast ceil(2N/3): size %v", r0.valSet.QuorumSize())
}

// check signatures large than 2F+1
panghalamit marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
2 changes: 1 addition & 1 deletion consensus/istanbul/core/prepare.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ func (c *core) handlePrepare(msg *message, src istanbul.Validator) error {

// Change to Prepared state if we've received enough PREPARE messages or it is locked
// and we are in earlier state before Prepared state.
if ((c.current.IsHashLocked() && prepare.Digest == c.current.GetLockedHash()) || c.current.GetPrepareOrCommitSize() > 2*c.valSet.F()) &&
if ((c.current.IsHashLocked() && prepare.Digest == c.current.GetLockedHash()) || c.current.GetPrepareOrCommitSize() >= c.valSet.QuorumSize()) &&
c.state.Cmp(StatePrepared) < 0 {
c.current.LockHash()
c.setState(StatePrepared)
Expand Down
12 changes: 6 additions & 6 deletions consensus/istanbul/core/prepare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,8 @@ OUTER:
if r0.state != StatePreprepared {
panghalamit marked this conversation as resolved.
Show resolved Hide resolved
t.Errorf("state mismatch: have %v, want %v", r0.state, StatePreprepared)
}
if r0.current.Prepares.Size() > 2*r0.valSet.F() {
t.Errorf("the size of PREPARE messages should be less than %v", 2*r0.valSet.F()+1)
if r0.current.Prepares.Size() >= r0.valSet.QuorumSize() {
t.Errorf("the size of PREPARE messages should be less than %v", r0.valSet.QuorumSize())
}
if r0.current.IsHashLocked() {
t.Errorf("block should not be locked")
Expand All @@ -224,12 +224,12 @@ OUTER:
continue
}

// core should have 2F+1 PREPARE messages
if r0.current.Prepares.Size() <= 2*r0.valSet.F() {
t.Errorf("the size of PREPARE messages should be larger than 2F+1: size %v", r0.current.Commits.Size())
// core should have ceil(2N/3) PREPARE messages
if r0.current.Prepares.Size() < r0.valSet.QuorumSize() {
t.Errorf("the size of PREPARE messages should be atleast ceil(2N/3) size %v", r0.current.Commits.Size())
}

// a message will be delivered to backend if 2F+1
// a message will be delivered to backend if ceil(2N/3)
if int64(len(v0.sentMsgs)) != 1 {
t.Errorf("the Send() should be called once: times %v", len(test.system.backends[0].sentMsgs))
}
Expand Down
4 changes: 2 additions & 2 deletions consensus/istanbul/core/roundchange.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@ func (c *core) handleRoundChange(msg *message, src istanbul.Validator) error {
c.sendRoundChange(roundView.Round)
}
return nil
} else if num == int(2*c.valSet.F()+1) && (c.waitingForRoundChange || cv.Round.Cmp(roundView.Round) < 0) {
// We've received 2f+1 ROUND CHANGE messages, start a new round immediately.
} else if num == c.valSet.QuorumSize() && (c.waitingForRoundChange || cv.Round.Cmp(roundView.Round) < 0) {
// We've received ceil(2N/3) ROUND CHANGE messages, start a new round immediately.
c.startNewRound(roundView.Round)
return nil
} else if cv.Round.Cmp(roundView.Round) < 0 {
Expand Down
2 changes: 2 additions & 0 deletions consensus/istanbul/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ type ValidatorSet interface {
F() int
// Get proposer policy
Policy() ProposerPolicy
// Get quorum size based on quorum formula type
QuorumSize() int
}

// ----------------------------------------------------------------------------
Expand Down
2 changes: 2 additions & 0 deletions consensus/istanbul/validator/default.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,3 +199,5 @@ func (valSet *defaultSet) Copy() istanbul.ValidatorSet {
func (valSet *defaultSet) F() int { return int(math.Ceil(float64(valSet.Size())/3)) - 1 }

func (valSet *defaultSet) Policy() istanbul.ProposerPolicy { return valSet.policy }

func (valSet *defaultSet) QuorumSize() int { return int(math.Ceil(float64(2*valSet.Size())/3)) }
11 changes: 11 additions & 0 deletions consensus/istanbul/validator/default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func TestValidatorSet(t *testing.T) {
testEmptyValSet(t)
testStickyProposer(t)
testAddAndRemoveValidator(t)
testQuorumSize(t)
}

func testNewValidatorSet(t *testing.T) {
Expand Down Expand Up @@ -206,3 +207,13 @@ func testStickyProposer(t *testing.T) {
t.Errorf("proposer mismatch: have %v, want %v", val, val2)
}
}

func testQuorumSize(t *testing.T) {
panghalamit marked this conversation as resolved.
Show resolved Hide resolved
valSet := newDefaultSet([]common.Address{}, istanbul.RoundRobin)
for i:=1; i<=1000; i++ {
valSet.AddValidator(common.StringToAddress(string(i)));
if valSet.QuorumSize() <= (valSet.Size() - valSet.QuorumSize() + valSet.F()) {
panghalamit marked this conversation as resolved.
Show resolved Hide resolved
t.Errorf("quorumSize constraint failed, expected value (QuorumSize > Size-QuorumSize+F) to be:%v, got: %v, for size: %v", true, false, valSet.Size());
}
}
}