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

Audit #10

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
4 changes: 2 additions & 2 deletions core/ibft.go
Original file line number Diff line number Diff line change
Expand Up @@ -687,7 +687,7 @@ func (i *IBFT) validateProposal(msg *proto.Message, view *proto.View) bool {
hash := messages.ExtractProposalHash(certificate.ProposalMessage)

roundsAndPreparedBlockHashes = append(roundsAndPreparedBlockHashes, roundHashTuple{
round: rcMessage.View.Round,
round: certificate.ProposalMessage.View.Round,
hash: hash,
})
}
Expand All @@ -704,7 +704,7 @@ func (i *IBFT) validateProposal(msg *proto.Message, view *proto.View) bool {
)

for _, tuple := range roundsAndPreparedBlockHashes {
if tuple.round > maxRound {
if tuple.round >= maxRound {
maxRound = tuple.round
expectedHash = tuple.hash
}
Expand Down
118 changes: 118 additions & 0 deletions core/ibft_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,124 @@ func TestRunNewRound_Validator_NonZero(t *testing.T) {
}
}

func TestRunNewRound_Round1_Accepts_Round0_Proposal(t *testing.T) {
t.Parallel()

var (
log mockLogger
backend mockBackend
transport mockTransport
msgs mockMessages

Copy link
Member

Choose a reason for hiding this comment

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

Why not define the handlers here in-line?
It's much cleaner, and more in line with our other tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Inlined

024d328

notifyCh = make(chan uint64, 1)

ctx context.Context

round1ProposalMsg *proto.Message

round0Proposer = []byte("round 0 proposer")
Copy link
Member

Choose a reason for hiding this comment

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

This is the only value that is shared in the test (checked against the proposal), the rest (below) are only used to initialize fields.

Please inline them where they are used

round1Proposer = []byte("round 1 proposer")
round0Proposal = []byte("round 0 proposal")
round0ProposalHash = []byte("round 0 proposal hash")
)

ctx, cancelFn := context.WithCancel(context.Background())

// backend
backend.isProposerFn = func(from []byte, _ uint64, round uint64) bool {
switch round {
case 0:
return bytes.Equal(from, round0Proposer)
case 1:
return bytes.Equal(from, round1Proposer)
}

return false
}

// messages
msgs.subscribeFn = func(_ messages.SubscriptionDetails) *messages.Subscription {
return &messages.Subscription{
SubCh: notifyCh,
}
}

msgs.getValidMessagesFn = func(_ *proto.View, _ proto.MessageType, isValid func(message *proto.Message) bool) []*proto.Message {
if !isValid(round1ProposalMsg) {
return nil
}

return []*proto.Message{round1ProposalMsg}
}

msgs.unsubscribeFn = func(_ messages.SubscriptionID) { cancelFn() }

round1ProposalMsg = &proto.Message{
View: &proto.View{Round: 1},
From: round1Proposer,
Type: proto.MessageType_PREPREPARE,
Payload: &proto.Message_PreprepareData{
PreprepareData: &proto.PrePrepareMessage{
Proposal: round0Proposal,
ProposalHash: round0ProposalHash,
Certificate: &proto.RoundChangeCertificate{
RoundChangeMessages: []*proto.Message{
{
View: &proto.View{Round: 0},
Type: proto.MessageType_ROUND_CHANGE,
Payload: &proto.Message_RoundChangeData{
RoundChangeData: &proto.RoundChangeMessage{
LastPreparedProposedBlock: round0Proposal,
LatestPreparedCertificate: &proto.PreparedCertificate{
ProposalMessage: &proto.Message{
View: &proto.View{Round: 0},
From: round0Proposer,
Type: proto.MessageType_PREPREPARE,
Payload: &proto.Message_PreprepareData{
PreprepareData: &proto.PrePrepareMessage{
Proposal: round0Proposal,
ProposalHash: round0ProposalHash,
},
},
},
PrepareMessages: []*proto.Message{
{
View: &proto.View{Round: 0},
From: []byte("some validator"),
Type: proto.MessageType_PREPARE,
Payload: &proto.Message_PrepareData{
PrepareData: &proto.PrepareMessage{
ProposalHash: round0ProposalHash,
},
},
},
},
},
},
},
},
}},
}},
}

i := NewIBFT(log, backend, transport)
i.messages = msgs
i.state.setView(&proto.View{Round: 1})

// Make sure the notification is sent out
notifyCh <- 1

i.wg.Add(1)
i.startRound(ctx)
i.wg.Wait()

// Make sure the node moves to prepare state
assert.Equal(t, prepare, i.state.name)

// Make sure the accepted proposal is the one that was sent out
assert.Equal(t, round0Proposal, i.state.getProposal())
}

// TestRunPrepare checks that the node behaves correctly
// in prepare state
func TestRunPrepare(t *testing.T) {
Expand Down