diff --git a/nonvalidator/non_validator.go b/nonvalidator/non_validator.go index 0d36505d..a918fecf 100644 --- a/nonvalidator/non_validator.go +++ b/nonvalidator/non_validator.go @@ -17,23 +17,24 @@ import ( ) type finalizedSeq struct { - block common.Block + // blocks holds one block per leader that proposed for this sequence. + // Rounds that share a sequence can have different leaders, so several blocks may be stored. + blocks map[string]common.Block finalization *common.Finalization } func (f *finalizedSeq) String() string { seq := uint64(0) digest := common.Digest{} - if f.block != nil { - seq = f.block.BlockHeader().Seq - digest = f.block.BlockHeader().Digest + for _, block := range f.blocks { + seq = block.BlockHeader().Seq } if f.finalization != nil { seq = f.finalization.Finalization.Seq digest = f.finalization.Finalization.Digest } - return fmt.Sprintf("FinalizedSeq {BlockDigest: %s, Seq: %d, BlockExists %t, FinalizationExists %t}", digest, seq, f.block != nil, f.finalization != nil) + return fmt.Sprintf("FinalizedSeq {FinalizationDigest: %s, Seq: %d, NumBlocks %d, FinalizationExists %t}", digest, seq, len(f.blocks), f.finalization != nil) } type Config struct { @@ -198,18 +199,15 @@ func (n *NonValidator) handleBlock(block common.Block, from common.NodeID) error } incomplete, ok := n.incompleteSequences[bh.Seq] - // we have not received any blocks or finalizations for this sequence if !ok { - incompleteSeq := &finalizedSeq{ - block: block, - } - n.incompleteSequences[bh.Seq] = incompleteSeq - n.Logger.Debug("Stored incomplete sequence", zap.Stringer("Sequence", incompleteSeq)) - return nil + incomplete = &finalizedSeq{blocks: make(map[string]common.Block)} + n.incompleteSequences[bh.Seq] = incomplete } - // Duplicate block, or finalization not yet received. - if incomplete.block != nil || incomplete.finalization == nil { + // Without a finalization we cannot tell which block is the right one, so keep one per node. + if incomplete.finalization == nil { + incomplete.blocks[string(from)] = block + n.Logger.Debug("Stored incomplete sequence", zap.Stringer("Sequence", incomplete)) return nil } @@ -223,8 +221,6 @@ func (n *NonValidator) handleBlock(block common.Block, from common.NodeID) error return nil } - incomplete.block = block - n.maybeValidateNextEpoch(block) return n.scheduleNewFinalizedBlockTask(block, incomplete.finalization) } @@ -362,18 +358,9 @@ func (n *NonValidator) handleFinalization(finalization *common.Finalization, fro } incomplete, ok := n.incompleteSequences[bh.Seq] - if !ok || (incomplete.finalization == nil || incomplete.block == nil) { - n.sequenceReplicator.ReceivedFutureFinalization(finalization, n.nextSeqToCommit()) - } - if !ok { - // we have not received anything for this sequence - incompleteSeq := &finalizedSeq{ - finalization: finalization, - } - n.incompleteSequences[bh.Seq] = incompleteSeq - n.Logger.Debug("Stored incomplete sequence", zap.Stringer("Sequence", incompleteSeq)) - return nil + incomplete = &finalizedSeq{blocks: make(map[string]common.Block)} + n.incompleteSequences[bh.Seq] = incomplete } // Duplicate finalization received. @@ -402,35 +389,27 @@ func (n *NonValidator) handleFinalization(finalization *common.Finalization, fro default: // The current finalization in incompleteSequences belongs to a Telock // because stored.Epoch < bh.Epoch - n.Logger.Debug("Dropping stored Telock sequence", zap.Stringer("Sequence", incomplete)) - incomplete.block = nil - n.sequenceReplicator.ReceivedFutureFinalization(finalization, n.nextSeqToCommit()) + n.Logger.Debug("Dropping stored Telock finalization", zap.Stringer("Sequence", incomplete)) } } incomplete.finalization = finalization + // Let the replicator know this sequence is finalized in case no stored block matches it. + n.sequenceReplicator.ReceivedFutureFinalization(finalization, n.nextSeqToCommit()) - // No block received yet for this sequence. - if incomplete.block == nil { - return nil - } + // Blocks arriving from now on are checked against the finalization directly, so the stored ones can be released. + blocks := incomplete.blocks + incomplete.blocks = nil - digest := incomplete.block.BlockHeader().Digest - if !bytes.Equal(bh.Digest[:], digest[:]) { - n.Logger.Debug( - "Received a block from the leader of a round whose digest mismatches the finalization", - zap.Stringer("Finalization Digest", bh.Digest), - zap.Stringer("Block digest", digest), - zap.Stringer("From", from), - ) - - incomplete.block = nil - n.sequenceReplicator.ReceivedFutureFinalization(finalization, n.nextSeqToCommit()) - return nil + for _, block := range blocks { + if block.BlockHeader().Digest == bh.Digest { + n.maybeValidateNextEpoch(block) + return n.scheduleNewFinalizedBlockTask(block, finalization) + } } - n.maybeValidateNextEpoch(incomplete.block) - return n.scheduleNewFinalizedBlockTask(incomplete.block, incomplete.finalization) + n.Logger.Debug("No stored block matches the finalization", zap.Stringer("Sequence", incomplete), zap.Int("Stored Blocks", len(blocks))) + return nil } func (n *NonValidator) scheduleNewFinalizedBlockTask(block common.Block, finalization *common.Finalization) error { diff --git a/nonvalidator/non_validator_test.go b/nonvalidator/non_validator_test.go index 0ccf79a0..98f3c731 100644 --- a/nonvalidator/non_validator_test.go +++ b/nonvalidator/non_validator_test.go @@ -175,6 +175,66 @@ func TestHandleMessages(t *testing.T) { }, expectedHeight: 3, }, + { + name: "adversary sends block for same sequence as real block", + setup: func(t *testing.T) (*testChain, []*messageInfo) { + tc := newSeededChain(t, testNodes, 2) + b3 := tc.appendBlock() + junk := testutil.NewTestBlock(common.ProtocolMetadata{ + Round: b3.Metadata.Round + 1, + Seq: b3.Metadata.Seq, + Epoch: b3.Metadata.Epoch, + Prev: b3.Metadata.Prev, + }, common.Blacklist{}) + + return tc, []*messageInfo{ + blockMsg(t, junk, testNodes), + blockMsg(t, b3, testNodes), + finalizationMsg(t, b3, testNodes), + } + }, + expectedHeight: 4, + }, + { + name: "junk block then finalization then real block", + setup: func(t *testing.T) (*testChain, []*messageInfo) { + tc := newSeededChain(t, testNodes, 2) + b3 := tc.appendBlock() + junk := testutil.NewTestBlock(common.ProtocolMetadata{ + Round: b3.Metadata.Round + 1, + Seq: b3.Metadata.Seq, + Epoch: b3.Metadata.Epoch, + Prev: b3.Metadata.Prev, + }, common.Blacklist{}) + + return tc, []*messageInfo{ + blockMsg(t, junk, testNodes), + finalizationMsg(t, b3, testNodes), + blockMsg(t, b3, testNodes), + } + }, + expectedHeight: 4, + }, + { + name: "real block then junk block then finalization", + setup: func(t *testing.T) (*testChain, []*messageInfo) { + tc := newSeededChain(t, testNodes, 2) + b3 := tc.appendBlock() + junk := testutil.NewTestBlock(common.ProtocolMetadata{ + Round: b3.Metadata.Round + 1, + Seq: b3.Metadata.Seq, + Epoch: b3.Metadata.Epoch, + Prev: b3.Metadata.Prev, + }, common.Blacklist{}) + + return tc, []*messageInfo{ + blockMsg(t, b3, testNodes), + blockMsg(t, junk, testNodes), + finalizationMsg(t, b3, testNodes), + } + }, + expectedHeight: 4, + }, { name: "qc does not verify", setup: func(t *testing.T) (*testChain, []*messageInfo) {