address review comments

Signed-off-by: May.Buzaglo <May.Buzaglo@ibm.com>

Author: MayRosenbaum

config/config.go

@@ -806,12 +806,12 @@ func (config *Configuration) BuildNewConfiguration(block *common.Block) (*Config
 
 	consensusMetadata, err := ReadConsensusMetadataFromConfigBlock(block)
 	if err != nil {
-		return nil, errors.Errorf("failed applying new config, failed to read consensusMetadata from last config block, block number is %d", block.Header.Number)
+		return nil, errors.Wrapf(err, "failed applying new config, failed to read consensusMetadata from last config block, block number is %d", block.Header.Number)
 	}
 
 	err = proto.Unmarshal(consensusMetadata, newConfig.SharedConfig)
 	if err != nil {
-		return nil, errors.Errorf("failed applying new config, failed to unmarshal consensusMetadata to a shared configuration")
+		return nil, errors.Wrapf(err, "failed applying new config, failed to unmarshal consensusMetadata to a shared configuration")
 	}
 	return newConfig, nil
 }

config/reconfig.go

@@ -22,33 +22,42 @@ type NodeConfig interface {
 	GetTlsCert() []byte
 }
 
-type NodeConfigWthSign interface {
+type NodeConfigWithSign interface {
 	NodeConfig
 	GetSignCert() []byte
 }
 
-// IsPartyEvicted checks if a party exists in the configuration and returns true if it is evicted or not.
-func IsPartyEvicted(partyID types.PartyID, newConfig *Configuration) bool {
-	newSharedPartyConfig := FindParty(partyID, newConfig)
-	return newSharedPartyConfig == nil
+// IsPartyEvicted returns true if the given party does not appear in the provided configuration.
+func IsPartyEvicted(partyID types.PartyID, newConfig *Configuration) (bool, error) {
+	newSharedPartyConfig, err := FindParty(partyID, newConfig)
+	if err != nil {
+		return false, err
+	}
+	return newSharedPartyConfig == nil, nil
 }
 
 // FindParty returns the PartyConfig associated with the given partyID from the shared configuration.
-// It returns nil if the party is not found.
-func FindParty(partyID types.PartyID, config *Configuration) *config_protos.PartyConfig {
+// It returns nil if the party is not found and returns error if the provided configuration is nil or incomplete.
+func FindParty(partyID types.PartyID, config *Configuration) (*config_protos.PartyConfig, error) {
+	if config == nil {
+		return nil, errors.New("the provided configuration is nil")
+	}
+	if config.SharedConfig == nil {
+		return nil, errors.New("the provided configuration has nil shared config")
+	}
 	for _, party := range config.SharedConfig.PartiesConfig {
 		if types.PartyID(party.PartyID) == partyID {
-			return party
+			return party, nil
 		}
 	}
-	return nil
+	return nil, nil
 }
 
-// IsNodeConfigChangeRestartRequired reports whether a restart is required due to changed between current configuration and new configuration.
-// A restart is required if any of the following fields differ:
+// IsNodeConfigChangeRestartRequired reports whether a restart is required due to configuration updates.
+// A restart is required if any of the following parts were updated:
 //   - host or port
 //   - TLS certificate
-//   - sign certificate (if both configs implement NodeConfigWithSignCert)
+//   - sign certificate (this is checked if both configs implement NodeConfigWithSign)
 //
 // Both arguments must represent the same node type and be non-nil.
 func IsNodeConfigChangeRestartRequired(currentConfig, newConfig NodeConfig) (bool, error) {
@@ -60,18 +69,19 @@ func IsNodeConfigChangeRestartRequired(currentConfig, newConfig NodeConfig) (boo
 		return false, errors.New("new config is nil")
 	}
 
+	extendedCurrConfig, currOK := currentConfig.(NodeConfigWithSign)
+	extendedNewConfig, newOK := newConfig.(NodeConfigWithSign)
+	if currOK != newOK {
+		return false, errors.New("type mismatch: current node config and new node config are not from the same type")
+	}
+
 	currAddr := net.JoinHostPort(currentConfig.GetHost(), strconv.Itoa(int(currentConfig.GetPort())))
 	newAddr := net.JoinHostPort(newConfig.GetHost(), strconv.Itoa(int(newConfig.GetPort())))
 
 	if currAddr != newAddr || !bytes.Equal(currentConfig.GetTlsCert(), newConfig.GetTlsCert()) {
 		return true, nil
 	}
 
-	extendedCurrConfig, currOK := currentConfig.(NodeConfigWthSign)
-	extendedNewConfig, newOK := newConfig.(NodeConfigWthSign)
-	if currOK != newOK {
-		return false, errors.New("type mismatch: current node config and new node config are not from the same type")
-	}
 	if currOK && !bytes.Equal(extendedCurrConfig.GetSignCert(), extendedNewConfig.GetSignCert()) {
 		return true, nil
 	}

config/reconfig_test.go

@@ -4,11 +4,13 @@ Copyright IBM Corp. All Rights Reserved.
 SPDX-License-Identifier: Apache-2.0
 */
 
-package config
+package config_test
 
 import (
 	"testing"
 
+	"github.com/hyperledger/fabric-x-orderer/config"
+
 	"github.com/hyperledger/fabric-x-orderer/common/types"
 	"github.com/hyperledger/fabric-x-orderer/config/protos"
 	"github.com/stretchr/testify/require"
@@ -19,16 +21,20 @@ func TestIsPartyEvicted(t *testing.T) {
 	partyConfig := &protos.PartyConfig{
 		PartyID: 2,
 	}
-	config := &Configuration{
+	conf := &config.Configuration{
 		SharedConfig: &protos.SharedConfig{
 			PartiesConfig: []*protos.PartyConfig{partyConfig},
 			MaxPartyID:    1,
 		},
 	}
 
-	require.True(t, IsPartyEvicted(partyID, config))
+	isPartyEvicted, err := config.IsPartyEvicted(partyID, conf)
+	require.NoError(t, err)
+	require.True(t, isPartyEvicted)
 	partyID = types.PartyID(2)
-	require.False(t, IsPartyEvicted(partyID, config))
+	isPartyEvicted, err = config.IsPartyEvicted(partyID, conf)
+	require.NoError(t, err)
+	require.False(t, isPartyEvicted)
 }
 
 func TestIsNodeConfigChangeRestartRequired(t *testing.T) {
@@ -45,20 +51,20 @@ func TestIsNodeConfigChangeRestartRequired(t *testing.T) {
 		TlsCert: []byte("cert"),
 	}
 
-	isRestartRequired, err := IsNodeConfigChangeRestartRequired(currRouterConfig, newRouterConfig)
+	isRestartRequired, err := config.IsNodeConfigChangeRestartRequired(currRouterConfig, newRouterConfig)
 	require.NoError(t, err)
 	require.False(t, isRestartRequired)
 
 	newRouterConfig.Port = 5070
 
-	isRestartRequired, err = IsNodeConfigChangeRestartRequired(currRouterConfig, newRouterConfig)
+	isRestartRequired, err = config.IsNodeConfigChangeRestartRequired(currRouterConfig, newRouterConfig)
 	require.NoError(t, err)
 	require.True(t, isRestartRequired)
 
 	newRouterConfig.Port = 5060
 	newRouterConfig.TlsCert = []byte("TLSCert")
 
-	isRestartRequired, err = IsNodeConfigChangeRestartRequired(currRouterConfig, newRouterConfig)
+	isRestartRequired, err = config.IsNodeConfigChangeRestartRequired(currRouterConfig, newRouterConfig)
 	require.NoError(t, err)
 	require.True(t, isRestartRequired)
 
@@ -79,12 +85,12 @@ func TestIsNodeConfigChangeRestartRequired(t *testing.T) {
 		TlsCert:  []byte("TLSCert"),
 	}
 
-	isRestartRequired, err = IsNodeConfigChangeRestartRequired(currBatcherConfig, newBatcherConfig)
+	isRestartRequired, err = config.IsNodeConfigChangeRestartRequired(currBatcherConfig, newBatcherConfig)
 	require.NoError(t, err)
 	require.False(t, isRestartRequired)
 
 	newBatcherConfig.SignCert = []byte("NewSignCert")
-	isRestartRequired, err = IsNodeConfigChangeRestartRequired(currBatcherConfig, newBatcherConfig)
+	isRestartRequired, err = config.IsNodeConfigChangeRestartRequired(currBatcherConfig, newBatcherConfig)
 	require.NoError(t, err)
 	require.True(t, isRestartRequired)
 }