[FAB-3727] Orderer restart broken

There was a programming error introduced during the consortium
refactoring which caused an interface return value to be checked for
nil, which returns false even when the underlying value is nil because
the dynamic type is a pointer kind.

This error caused the orderer to recognize all channels as the system
channel, and would cause a fatal error at startup.

This changeset modifies the config interface to return a boolean along
with the interface value, indicating whether the interface value was
populated.

Change-Id: Ia9bb89b765c7e8f35742895d2ac1dd2396d03908
Signed-off-by: Jason Yellick <[email protected]>

Author: Jason Yellick

common/configtx/api/api.go

@@ -62,7 +62,8 @@ type Resources interface {
 	OrdererConfig() config.Orderer
 
 	// ConsortiumsConfig() returns the config.Consortiums for the channel
-	ConsortiumsConfig() config.Consortiums
+	// and whether the consortiums config exists
+	ConsortiumsConfig() (config.Consortiums, bool)
 
 	// ApplicationConfig returns the configtxapplication.SharedConfig for the channel
 	ApplicationConfig() config.Application

common/configtx/initializer.go

@@ -56,9 +56,15 @@ func (r *resources) ApplicationConfig() config.Application {
 	return r.configRoot.Application()
 }
 
-// ConsortiumsConfig returns the api.ConsortiumsConfig for the chain
-func (r *resources) ConsortiumsConfig() config.Consortiums {
-	return r.configRoot.Consortiums()
+// ConsortiumsConfig returns the api.ConsortiumsConfig for the chain and whether or not
+// this channel contains consortiums config
+func (r *resources) ConsortiumsConfig() (config.Consortiums, bool) {
+	result := r.configRoot.Consortiums()
+	if result == nil {
+		return nil, false
+	}
+
+	return result, true
 }
 
 // MSPManager returns the msp.MSPManager for the chain

common/mocks/configtx/configtx.go

@@ -66,8 +66,8 @@ func (r *Resources) ApplicationConfig() config.Application {
 	return r.ApplicationConfigVal
 }
 
-func (r *Resources) ConsortiumsConfig() config.Consortiums {
-	return r.ConsortiumsConfigVal
+func (r *Resources) ConsortiumsConfig() (config.Consortiums, bool) {
+	return r.ConsortiumsConfigVal, r.ConsortiumsConfigVal != nil
 }
 
 // Returns the MSPManagerVal

orderer/multichain/manager.go

@@ -100,18 +100,18 @@ func NewManagerImpl(ledgerFactory ledger.Factory, consenters map[string]Consente
 	for _, chainID := range existingChains {
 		rl, err := ledgerFactory.GetOrCreate(chainID)
 		if err != nil {
-			logger.Fatalf("Ledger factory reported chainID %s but could not retrieve it: %s", chainID, err)
+			logger.Panicf("Ledger factory reported chainID %s but could not retrieve it: %s", chainID, err)
 		}
 		configTx := getConfigTx(rl)
 		if configTx == nil {
-			logger.Fatalf("Could not find config transaction for chain %s", chainID)
+			logger.Panicf("Could not find config transaction for chain %s", chainID)
 		}
 		ledgerResources := ml.newLedgerResources(configTx)
 		chainID := ledgerResources.ChainID()
 
-		if ledgerResources.ConsortiumsConfig() != nil {
+		if _, ok := ledgerResources.ConsortiumsConfig(); ok {
 			if ml.systemChannelID != "" {
-				logger.Fatalf("There appear to be two system chains %s and %s", ml.systemChannelID, chainID)
+				logger.Panicf("There appear to be two system chains %s and %s", ml.systemChannelID, chainID)
 			}
 			chain := newChainSupport(createSystemChainFilters(ml, ledgerResources),
 				ledgerResources,
@@ -156,14 +156,14 @@ func (ml *multiLedger) newLedgerResources(configTx *cb.Envelope) *ledgerResource
 	initializer := configtx.NewInitializer()
 	configManager, err := configtx.NewManagerImpl(configTx, initializer, nil)
 	if err != nil {
-		logger.Fatalf("Error creating configtx manager and handlers: %s", err)
+		logger.Panicf("Error creating configtx manager and handlers: %s", err)
 	}
 
 	chainID := configManager.ChainID()
 
 	ledger, err := ml.ledgerFactory.GetOrCreate(chainID)
 	if err != nil {
-		logger.Fatalf("Error getting ledger for %s", chainID)
+		logger.Panicf("Error getting ledger for %s", chainID)
 	}
 
 	return &ledgerResources{
@@ -237,8 +237,8 @@ func (ml *multiLedger) NewChannelConfig(envConfigUpdate *cb.Envelope) (configtxa
 	}
 
 	applicationGroup := cb.NewConfigGroup()
-	consortiumsConfig := ml.systemChannel.ConsortiumsConfig()
-	if consortiumsConfig == nil {
+	consortiumsConfig, ok := ml.systemChannel.ConsortiumsConfig()
+	if !ok {
 		return nil, fmt.Errorf("The ordering system channel does not appear to support creating channels")
 	}
 

orderer/multichain/manager_test.go

@@ -133,7 +133,7 @@ func TestGetConfigTxFailure(t *testing.T) {
 
 }
 
-// This test essentially brings the entire system up and is ultimately what main.go will replicate
+// This test checks to make sure the orderer refuses to come up if it cannot find a system channel
 func TestNoSystemChain(t *testing.T) {
 	defer func() {
 		if recover() == nil {
@@ -149,6 +149,24 @@ func TestNoSystemChain(t *testing.T) {
 	NewManagerImpl(lf, consenters, mockCrypto())
 }
 
+// This test checks to make sure that the orderer refuses to come up if there are multiple system channels
+func TestMultiSystemChannel(t *testing.T) {
+	lf := ramledger.New(10)
+
+	for _, id := range []string{"foo", "bar"} {
+		rl, err := lf.GetOrCreate(id)
+		assert.NoError(t, err)
+
+		err = rl.Append(provisional.New(conf).GenesisBlockForChannel(id))
+		assert.NoError(t, err)
+	}
+
+	consenters := make(map[string]Consenter)
+	consenters[conf.Orderer.OrdererType] = &mockConsenter{}
+
+	assert.Panics(t, func() { NewManagerImpl(lf, consenters, mockCrypto()) }, "Two system channels should have caused panic")
+}
+
 // This test essentially brings the entire system up and is ultimately what main.go will replicate
 func TestManagerImpl(t *testing.T) {
 	lf, rl := NewRAMLedgerAndFactory(10)