config pull delivery client in router #334

DorKatzelnick · 2025-11-04T09:35:19Z

add config-puller delivery client to router, and pull config blocks for consensus

add test

issue: #199

HagarMeir · 2025-11-04T09:39:00Z

node/delivery/consensus_config_deliver_client.go

+			ccp.logger.Panicf("Failed extracting header from decision: %s", err)
+		}
+
+		// if num := len(header.AvailableCommonBlocks); num > 0 {


remove comments

HagarMeir · 2025-11-04T09:43:57Z

node/delivery/consensus_config_deliver_client.go

+			common.HeaderType_DELIVER_SEEK_INFO,
+			"consensus",
+			nil,
+			NewestSeekInfo(),


This might need to change
At least put a TODO here so we remember to give this a second thought

will change it with the last decision number in config store

HagarMeir · 2025-11-04T09:45:32Z

node/delivery/consensus_config_deliver_client.go

+	res := make(chan *common.Block, configBlocksChanSize)
+
+	blockHandlerFunc := func(block *common.Block) {
+		ccp.logger.Infof("Received decision block %d from consensus", block.GetHeader().GetNumber())


maybe debugf?
otherwise we will see a log for each and every decision

i will just remove it, since we log it in pullAndProcessConfigBlocks method

HagarMeir · 2025-11-04T09:46:15Z

node/delivery/consensus_config_deliver_client.go

+
+	blockHandlerFunc := func(block *common.Block) {
+		ccp.logger.Infof("Received decision block %d from consensus", block.GetHeader().GetNumber())
+		// check that the decision contains a config block. then extract the config block and send it on the res channel


HagarMeir · 2025-11-04T09:49:56Z

node/router/consensus_config_puller.go

+	Stop()
+}
+
+//go:generate counterfeiter -o mocks/consensus_state_replicator_creator.go . ConsensusStateReplicatorCreator


this is used for mocking for testing
If not used then you can remove this

and just have the struct without and interface

or just call new without a struct
remove this file

HagarMeir · 2025-11-04T09:59:00Z

node/router/router_test.go

+	err = sc.MakeDecisionFromHeader(&state.Header{Num: 1, DecisionNumOfLastConfigBlock: 1, AvailableCommonBlocks: acb})
+	require.NoError(t, err)
+
+	// check for config block is stored in router's config store


HagarMeir · 2025-11-04T09:59:46Z

node/router/router_test.go

+
+	// check for config block is stored in router's config store
+	require.Eventually(t, func() bool {
+		return testSetup.router.GetConfigStoreSize() == 1


what was the size before?

will also check it is 0 before.

HagarMeir · 2025-11-04T10:01:37Z

node/router/stub_consenter_test.go

+	return fmt.Errorf("not implemented")
+}
+
+func (sc *StubConsenter) MakeDecisionFromHeader(header *state.Header) error {


Deliver Decision From Header?

HagarMeir · 2025-11-04T10:03:20Z

node/router/stub_consenter_test.go

 		server:      server,
 		partyID:     partyID,
+		decisions:   make(chan *common.Block, 100),
+		logger:      testutil.CreateLoggerForModule(t, "stucbconsenter", zapcore.DebugLevel),


why create logger here?

HagarMeir · 2025-11-04T10:05:15Z

node/router/stub_consenter_test.go

+	txs            uint32           // Number of txs received from router
+	partyID        types.PartyID
+	decisions      chan *common.Block
+	decisionSentCh chan struct{} // decision sent signal


what is this for?

HagarMeir · 2025-11-05T09:06:50Z

node/delivery/consensus_config_deliver_client.go

+	return configPuller
+}
+
+func (ccp *ConsensusConfigPuller) PullConfigBlocks() <-chan *common.Block {


We should keep in mind somewhere that we should pull config blocks from all consenters
Maybe add a TODO

HagarMeir · 2025-11-05T13:28:08Z

node/router/router.go

+func NextSeekInfoFromConfigStore(configStore *configstore.Store, logger types.Logger) *orderer.SeekInfo {
+	lastBlock, err := configStore.Last()
+	if err != nil {
+		// TODO - replace this with panic if genesis block is placed in config store during router initialization


please avoid this by changing the tests to start with a full config store (with genesis block)

tock-ibm · 2025-11-09T09:31:10Z

node/router/router.go

+				r.logger.Errorf("Failed adding config block to config store: %s", err)
+				continue


This is a fatal error... better panic

tock-ibm · 2025-11-09T09:34:44Z

node/router/router_test.go

 	stubConsenterInfo := config.ConsenterInfo{PartyID: partyID, Endpoint: consenter.GetConsenterEndpoint(), TLSCACerts: []config.RawBytes{ca.CertBytes()}}

+	configStorePath := t.TempDir()
+	cs, _ := configstore.NewStore(configStorePath)


require.NoError

tock-ibm · 2025-11-09T09:36:01Z

node/router/router_test.go


+	configStorePath := t.TempDir()
+	cs, _ := configstore.NewStore(configStorePath)
+	cs.Add(&common.Block{Header: &common.BlockHeader{Number: 0}, Data: &common.BlockData{}}) // add dummy genesis block


Add a block that at least passes protoutils.IsConfigBlock() as true

tock-ibm · 2025-11-09T09:36:31Z

node/router/router_test.go

+	initialConfigStoreSize := testSetup.router.GetConfigStoreSize()
+
+	// create a decision with a config block
+	configBlock := common.Block{Header: &common.BlockHeader{Number: 999}, Data: &common.BlockData{}}


Add a block that at least passes protoutils.IsConfigBlock() as true

tock-ibm · 2025-11-09T09:37:30Z

node/router/router_test.go

+	sc := testSetup.consenter
+	defer testSetup.Close()
+
+	initialConfigStoreSize := testSetup.router.GetConfigStoreSize()


require something here, I assume ==1

tock-ibm · 2025-11-09T09:39:39Z

test/utils_test.go

 		bundle.ConfigtxValidatorReturns(configtxValidator)

+		configStorePath := t.TempDir()
+		cs, _ := configstore.NewStore(configStorePath)


require NoError

tock-ibm · 2025-11-09T09:40:13Z

test/utils_test.go


+		configStorePath := t.TempDir()
+		cs, _ := configstore.NewStore(configStorePath)
+		cs.Add(&common.Block{Header: &common.BlockHeader{Number: 0}, Data: &common.BlockData{}}) // add dummy genesis block


again should pass IsConfigBlock

tock-ibm · 2025-11-09T09:42:22Z

node/delivery/consensus_config_deliver_client.go

+	return configPuller
+}
+
+func (ccp *ConsensusConfigPuller) PullConfigBlocks() <-chan *common.Block {


Document it. Doe it return only config blocks or is filtering required?

I also added the verification for the config block with protoutil.IsConfigBlock()

tock-ibm · 2025-11-09T09:43:01Z

node/delivery/consensus_config_deliver_client.go

+		requestEnvelope, err := protoutil.CreateSignedEnvelopeWithTLSBinding(
+			common.HeaderType_DELIVER_SEEK_INFO,
+			"consensus",
+			nil,


Add a TODO to use a signer here

tock-ibm · 2025-11-09T09:46:35Z

node/delivery/consensus_config_deliver_client.go

+		return ccp.endpoint
+	}
+
+	requestEnvelopeFactoryFunc := func() *common.Envelope {


There is no point using a factory if the cpp.seekInfo is fixed...

The idea was to make the puller self update when new config blocks are met, i.e. update the endpoint and credentials on the fly. This is not the case here.

I replaced the seekInfo with a function that returns the latest seekInfo when called. I am not sure if this is necessary. Because I don't see when it will be called again unless the router restarts and then the configPuller starts again (like it would before)

Did you mean to add an update method that the router can call and change the params? I can place it in TODO now and do it in next PR's (like in the config-submitter)

You can simply calculate once the seek info in advance and pass the value when you create the ConsensusConfigPuller
It doesn't need to be a function at all since as you put it the seek info does not change

added an Update method with TODO comment

…or consensus add new config blocks to the config-store add test Signed-off-by: Dor.Katzelnick <Dor.Katzelnick@ibm.com>

DorKatzelnick requested review from HagarMeir and tock-ibm November 4, 2025 09:37

HagarMeir reviewed Nov 4, 2025

View reviewed changes

HagarMeir force-pushed the reconfigC2R branch from 2fcfac7 to b8403c1 Compare November 5, 2025 08:19

HagarMeir reviewed Nov 5, 2025

View reviewed changes

DorKatzelnick force-pushed the reconfigC2R branch from b8403c1 to 6ac8e80 Compare November 5, 2025 12:05

DorKatzelnick requested a review from HagarMeir November 5, 2025 12:05

HagarMeir force-pushed the reconfigC2R branch from 6ac8e80 to 1b378ab Compare November 5, 2025 13:17

HagarMeir reviewed Nov 5, 2025

View reviewed changes

DorKatzelnick force-pushed the reconfigC2R branch 2 times, most recently from f8e5ac8 to 4764c16 Compare November 6, 2025 15:12

HagarMeir force-pushed the reconfigC2R branch from 4764c16 to b889a4f Compare November 6, 2025 15:53

tock-ibm reviewed Nov 9, 2025

View reviewed changes

DorKatzelnick force-pushed the reconfigC2R branch 2 times, most recently from 1ee689a to fdc1043 Compare November 10, 2025 14:09

add config-puller delivery client to router, and pull config blocks f…

4888b66

…or consensus add new config blocks to the config-store add test Signed-off-by: Dor.Katzelnick <Dor.Katzelnick@ibm.com>

DorKatzelnick force-pushed the reconfigC2R branch from fdc1043 to 4888b66 Compare November 10, 2025 15:17

		r.logger.Errorf("Failed adding config block to config store: %s", err)
		continue

config pull delivery client in router #334

Are you sure you want to change the base?

config pull delivery client in router #334

Uh oh!

Conversation

DorKatzelnick commented Nov 4, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DorKatzelnick Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

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!

DorKatzelnick Nov 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DorKatzelnick Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

DorKatzelnick Nov 4, 2025 •

edited

Loading

DorKatzelnick Nov 9, 2025 •

edited

Loading

DorKatzelnick Nov 10, 2025 •

edited

Loading