Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
23 changes: 23 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -792,3 +792,26 @@ func (config *Configuration) CheckIfAssemblerNodeExistsInSharedConfig() error {
}
return fmt.Errorf("partyID %d is not present in the shared configuration's party list", localPartyID)
}

// BuildNewConfiguration builds a new configuration based on current configuration and block
func (config *Configuration) BuildNewConfiguration(block *common.Block) (*Configuration, error) {
if config == nil {
return nil, errors.New("failed applying new config, current configuration is nil")
}
Comment on lines +796 to +800
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

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

BuildNewConfiguration is new behavior in config/config.go, but there are no tests exercising it. Please add unit tests for the success case and failure cases (e.g., invalid block / unmarshal error).

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

True, test it!


newConfig := &Configuration{
LocalConfig: config.LocalConfig,
SharedConfig: &protos.SharedConfig{},
}
Comment on lines +802 to +805
Copy link
Contributor

Choose a reason for hiding this comment

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

move that next to where you use it, below


consensusMetadata, err := ReadConsensusMetadataFromConfigBlock(block)
Copy link
Contributor

Choose a reason for hiding this comment

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

There must be a utility that returns func myFunc(block) (*protos.SharedConfig, error) somewhere...

if err != nil {
return nil, errors.Wrapf(err, "failed applying new config, failed to read consensusMetadata from last config block, block number is %d", block.Header.Number)
}
Comment on lines +807 to +810
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

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

BuildNewConfiguration uses block.Header.Number in the error message, but there is no nil check for block (or block.Header). Add validation up front to avoid panics if callers pass a nil/invalid block.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

True, first protect against a nil block, then,
True, protect against that, use block.GetHeader(),


err = proto.Unmarshal(consensusMetadata, newConfig.SharedConfig)
if err != nil {
return nil, errors.Wrapf(err, "failed applying new config, failed to unmarshal consensusMetadata to a shared configuration")
}
return newConfig, nil
}
90 changes: 90 additions & 0 deletions config/reconfig.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
Copyright IBM Corp. All Rights Reserved.

SPDX-License-Identifier: Apache-2.0
*/

package config

import (
"bytes"
"errors"
"net"
"strconv"

"github.com/hyperledger/fabric-x-orderer/common/types"
config_protos "github.com/hyperledger/fabric-x-orderer/config/protos"
)

type NodeConfig interface {
GetHost() string
GetPort() uint32
GetTlsCert() []byte
}

type NodeConfigWithSign interface {
NodeConfig
GetSignCert() []byte
}
Comment on lines +19 to +28
Copy link
Contributor

Choose a reason for hiding this comment

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

document how this will be used, reference the protos

Copy link
Contributor

Choose a reason for hiding this comment

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

Explicitly say, router and assembler are.... consenter and batcher are... etc


// 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 and returns error if the provided configuration is nil or incomplete.
func FindParty(partyID types.PartyID, config *Configuration) (*config_protos.PartyConfig, error) {
Comment on lines +39 to +41
Copy link
Contributor

Choose a reason for hiding this comment

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

can be package private?

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, nil
}
}
return nil, nil
}

// 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 (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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

add a logger to the function and log what you find with Info, super useful

if currentConfig == nil {
return false, errors.New("current config is nil")
}

if newConfig == nil {
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
}

if currOK && !bytes.Equal(extendedCurrConfig.GetSignCert(), extendedNewConfig.GetSignCert()) {
return true, nil
}
Comment on lines +85 to +87
Copy link
Contributor

Choose a reason for hiding this comment

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

Leave a comment that we may need to match the public key, so that we can do dynamic instead of restart in some cases when the private key & public key do not change


return false, nil
}
96 changes: 96 additions & 0 deletions config/reconfig_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
/*
Copyright IBM Corp. All Rights Reserved.

SPDX-License-Identifier: Apache-2.0
*/

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"
)

func TestIsPartyEvicted(t *testing.T) {
partyID := types.PartyID(1)
partyConfig := &protos.PartyConfig{
PartyID: 2,
}
conf := &config.Configuration{
SharedConfig: &protos.SharedConfig{
PartiesConfig: []*protos.PartyConfig{partyConfig},
MaxPartyID: 1,
},
}

isPartyEvicted, err := config.IsPartyEvicted(partyID, conf)
require.NoError(t, err)
require.True(t, isPartyEvicted)
partyID = types.PartyID(2)
isPartyEvicted, err = config.IsPartyEvicted(partyID, conf)
require.NoError(t, err)
require.False(t, isPartyEvicted)
}

func TestIsNodeConfigChangeRestartRequired(t *testing.T) {
// Test Router
currRouterConfig := &protos.RouterNodeConfig{
Comment on lines +40 to +42
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

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

This test covers address/TLS/sign-cert changes, but it doesn't cover the error branches of IsNodeConfigChangeRestartRequired (nil args, type mismatch). Add test cases for these conditions to prevent regressions.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

True, do it

Host: "127.0.0.1",
Port: 5060,
TlsCert: []byte("cert"),
}

newRouterConfig := &protos.RouterNodeConfig{
Host: "127.0.0.1",
Port: 5060,
TlsCert: []byte("cert"),
}

isRestartRequired, err := config.IsNodeConfigChangeRestartRequired(currRouterConfig, newRouterConfig)
require.NoError(t, err)
require.False(t, isRestartRequired)

newRouterConfig.Port = 5070

isRestartRequired, err = config.IsNodeConfigChangeRestartRequired(currRouterConfig, newRouterConfig)
require.NoError(t, err)
require.True(t, isRestartRequired)

newRouterConfig.Port = 5060
newRouterConfig.TlsCert = []byte("TLSCert")

isRestartRequired, err = config.IsNodeConfigChangeRestartRequired(currRouterConfig, newRouterConfig)
require.NoError(t, err)
require.True(t, isRestartRequired)

// Test Batcher
currBatcherConfig := &protos.BatcherNodeConfig{
ShardID: 1,
Host: "127.0.0.1",
Port: 5060,
SignCert: []byte("SignCert"),
TlsCert: []byte("TLSCert"),
}

newBatcherConfig := &protos.BatcherNodeConfig{
ShardID: 1,
Host: "127.0.0.1",
Port: 5060,
SignCert: []byte("SignCert"),
TlsCert: []byte("TLSCert"),
}

isRestartRequired, err = config.IsNodeConfigChangeRestartRequired(currBatcherConfig, newBatcherConfig)
require.NoError(t, err)
require.False(t, isRestartRequired)

newBatcherConfig.SignCert = []byte("NewSignCert")
isRestartRequired, err = config.IsNodeConfigChangeRestartRequired(currBatcherConfig, newBatcherConfig)
require.NoError(t, err)
require.True(t, isRestartRequired)
}