Skip to content

Commit 4298d72

Browse files
authored
fix(smartbft): implement Errored() on BFTChain (#5451)
Signed-off-by: Shubham Singh <shubhsoch@gmail.com>
1 parent 1718a16 commit 4298d72

2 files changed

Lines changed: 61 additions & 2 deletions

File tree

orderer/consensus/smartbft/chain.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,9 @@ type BFTChain struct {
8484
statusReportMutex sync.Mutex
8585
consensusRelation types2.ConsensusRelation
8686
status types2.Status
87+
88+
exitCLock sync.RWMutex
89+
exitC chan struct{}
8790
}
8891

8992
// NewChain creates new BFT Smart chain
@@ -128,6 +131,7 @@ func NewChain(
128131
Logger: logger,
129132
consensusRelation: types2.ConsensusRelationConsenter,
130133
status: types2.StatusActive,
134+
exitC: make(chan struct{}),
131135
Metrics: &Metrics{
132136
ClusterSize: metrics.ClusterSize.With("channel", support.ChannelID()),
133137
CommittedBlockNumber: metrics.CommittedBlockNumber.With("channel", support.ChannelID()),
@@ -445,8 +449,9 @@ func (c *BFTChain) WaitReady() error {
445449
// This is especially useful for the Deliver client, who must terminate waiting
446450
// clients when the consenter is not up to date.
447451
func (c *BFTChain) Errored() <-chan struct{} {
448-
// TODO: Implement Errored
449-
return nil
452+
c.exitCLock.RLock()
453+
defer c.exitCLock.RUnlock()
454+
return c.exitC
450455
}
451456

452457
// Start should allocate whatever resources are needed for staying up to date with the chain.
@@ -462,6 +467,14 @@ func (c *BFTChain) Start() {
462467
// Halt frees the resources which were allocated for this Chain.
463468
func (c *BFTChain) Halt() {
464469
c.Logger.Infof("Shutting down chain")
470+
c.exitCLock.Lock()
471+
select {
472+
case <-c.exitC:
473+
// already closed
474+
default:
475+
close(c.exitC)
476+
}
477+
c.exitCLock.Unlock()
465478
c.consensus.Stop()
466479
}
467480

orderer/consensus/smartbft/chain_test.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,3 +636,49 @@ func removeNodeFromConfig(t *testing.T, lastConfigBlock *cb.Block, nodeId uint32
636636
}),
637637
}
638638
}
639+
640+
// TestErrored verifies Errored() returns an open channel before Halt and a closed one after; double-Halt is safe.
641+
func TestErrored(t *testing.T) {
642+
dir := t.TempDir()
643+
channelId := "testchannel"
644+
645+
networkSetupInfo := NewNetworkSetupInfo(t, channelId, dir)
646+
nodeMap := networkSetupInfo.CreateNodes(1)
647+
node := nodeMap[1]
648+
649+
chain := node.Chain
650+
651+
// before Halt: Errored() must return a non-nil, open channel.
652+
errC := chain.Errored()
653+
require.NotNil(t, errC)
654+
select {
655+
case <-errC:
656+
t.Fatal("Errored channel should not be closed before Halt")
657+
default:
658+
}
659+
660+
node.Start()
661+
node.State.WaitLedgerHeightToBe(1)
662+
663+
chain.Halt()
664+
665+
// after Halt: the channel returned before Halt must now be closed.
666+
select {
667+
case <-errC:
668+
// expected
669+
default:
670+
t.Fatal("Errored channel should be closed after Halt")
671+
}
672+
673+
// calling Halt a second time must not panic.
674+
require.NotPanics(t, chain.Halt)
675+
676+
// Errored() after Halt must still return a closed channel.
677+
require.NotNil(t, chain.Errored())
678+
select {
679+
case <-chain.Errored():
680+
// expected
681+
default:
682+
t.Fatal("Errored channel should be closed after repeated Halt")
683+
}
684+
}

0 commit comments

Comments
 (0)