dynamic reconfig utils by MayRosenbaum · Pull Request #647 · hyperledger/fabric-x-orderer

MayRosenbaum · 2026-02-25T13:23:35Z

This PR adds utility functions to determine whether an admin restart is required.
Relevant issues #442 #504 #487 #445

Copilot

Pull request overview

This PR introduces helper utilities in the config package to support dynamic reconfiguration flows by deciding whether a node can apply a config update without an admin restart (and by detecting party eviction), plus a small helper to build a new Configuration from a config block.

Changes:

Added IsPartyEvicted/FindParty helpers for membership/eviction checks.
Added IsNodeConfigChangeRestartRequired to detect restart-required config changes (endpoint/TLS/sign cert).
Added Configuration.BuildNewConfiguration and a new unit test file for the reconfig utilities.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 9 comments.

File	Description
config/reconfig.go	New exported utility functions/interfaces for eviction detection and restart-required decisions.
config/reconfig_test.go	New tests for the reconfig utilities.
config/config.go	Adds `BuildNewConfiguration` helper to construct a new config from a config block.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

config/reconfig.go

config/reconfig_test.go

Copilot · 2026-02-25T15:34:25Z

config/reconfig_test.go

+func TestIsNodeConfigChangeRestartRequired(t *testing.T) {
+	// Test Router
+	currRouterConfig := &protos.RouterNodeConfig{


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.

True, do it

config/reconfig.go

Copilot · 2026-02-25T15:34:26Z

config/config.go

+	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)
+	}


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.

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

config/config.go

Copilot · 2026-02-25T15:34:27Z

config/config.go

+// 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")
+	}


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

True, test it!

config/reconfig.go

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

config/config.go

tock-ibm · 2026-02-26T13:35:24Z

config/config.go

+	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)
+	}


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

tock-ibm · 2026-02-26T13:35:35Z

config/config.go

+// 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")
+	}


True, test it!

tock-ibm · 2026-02-26T13:42:17Z

config/reconfig.go

+// 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) {


can be package private?

tock-ibm · 2026-02-26T13:57:27Z

config/reconfig.go

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


document how this will be used, reference the protos

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

tock-ibm · 2026-02-26T14:03:06Z

config/config.go

+	newConfig := &Configuration{
+		LocalConfig:  config.LocalConfig,
+		SharedConfig: &protos.SharedConfig{},
+	}


move that next to where you use it, below

tock-ibm · 2026-02-26T14:04:43Z

config/config.go

+		SharedConfig: &protos.SharedConfig{},
+	}
+
+	consensusMetadata, err := ReadConsensusMetadataFromConfigBlock(block)


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

tock-ibm · 2026-02-26T14:07:09Z

config/reconfig.go

+	if currOK && !bytes.Equal(extendedCurrConfig.GetSignCert(), extendedNewConfig.GetSignCert()) {
+		return true, nil
+	}


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

tock-ibm · 2026-02-26T14:08:01Z

config/reconfig.go

+//   - 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) {


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

tock-ibm · 2026-02-26T14:08:27Z

config/reconfig_test.go

+func TestIsNodeConfigChangeRestartRequired(t *testing.T) {
+	// Test Router
+	currRouterConfig := &protos.RouterNodeConfig{


True, do it

MayRosenbaum force-pushed the dynamic_reconfig_utils branch 2 times, most recently from 66f6113 to 6311754 Compare February 25, 2026 13:41

DorKatzelnick requested a review from Copilot February 25, 2026 15:30

Copilot started reviewing on behalf of DorKatzelnick February 25, 2026 15:31 View session

Copilot AI reviewed Feb 25, 2026

View reviewed changes

HagarMeir reviewed Feb 25, 2026

View reviewed changes

config/reconfig.go Outdated Show resolved Hide resolved

HagarMeir reviewed Feb 25, 2026

View reviewed changes

config/reconfig.go Outdated Show resolved Hide resolved

HagarMeir reviewed Feb 25, 2026

View reviewed changes

config/reconfig.go Show resolved Hide resolved

HagarMeir reviewed Feb 25, 2026

View reviewed changes

config/reconfig.go Outdated Show resolved Hide resolved

MayRosenbaum force-pushed the dynamic_reconfig_utils branch from 0f26370 to 0c5aaea Compare February 26, 2026 08:45

MayRosenbaum added 2 commits February 26, 2026 15:23

dynamic reconfig utils

94bb07e

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

address review comments

cc803de

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

tock-ibm force-pushed the dynamic_reconfig_utils branch from 0c5aaea to cc803de Compare February 26, 2026 13:23

tock-ibm reviewed Feb 26, 2026

View reviewed changes

Conversation

MayRosenbaum commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MayRosenbaum commented Feb 25, 2026 •

edited

Loading