add config request to control event #305

moradna · 2025-10-20T09:39:39Z

HagarMeir · 2025-10-21T07:54:56Z

node/consensus/state/state.go

+	Pending             []types.BatchAttestationFragment
+	Complaints          []Complaint
+	AppContext          []byte
+	LatestConfigRequest *ConfigRequest


as we discussed this is not needed

HagarMeir · 2025-10-21T07:56:47Z

node/consensus/state/state.go

+}
+
+func (c *ConfigRequest) String() string {
+	return "Config Request"


add a TODO comment to add more info to this string

Some hash, the config sequence sequence? better to have more than less, this is a rare event.

HagarMeir · 2025-10-21T08:00:58Z

node/consensus/state/state.go

 		copy(payloadToHash[22:], ce.BAF.Digest())
+	} else if ce.ConfigRequest != nil {
+		ce.ConfigRequest.Envelope.Signature = nil
+		payloadToHash = ce.ConfigRequest.Bytes()


add a TODO comment to maybe use a different ID

Maybe the ID should hash both content and sig? i.e. the marshalled bytes?

maybe we could do something like Fabric does
but again this can be done in another PR
there is a TODO here

HagarMeir · 2025-10-21T08:03:09Z

node/consensus/state/state.go

 		s2.AppContext = make([]byte, 0, len(s.AppContext))
 		s2.AppContext = append(s2.AppContext, s.AppContext...)
 	}
+	if s.LatestConfigRequest != nil {


also remove this

HagarMeir · 2025-10-21T08:03:37Z

node/consensus/state/state.go

 			s.Complaints = append(s.Complaints, *ce.Complaint)
 		}
+		if ce.ConfigRequest != nil {
+			s.LatestConfigRequest = ce.ConfigRequest


remove this

HagarMeir · 2025-10-21T08:04:06Z

node/consensus/state/state_test.go

 	}

-	ce = consensus_state.ControlEvent{nil, &c}
+	ce = consensus_state.ControlEvent{nil, &c, nil}


add a simple test with a non nil config request

HagarMeir · 2025-10-21T09:58:45Z

node/consensus/state/state.go

 	return -1, false
 }
+
+func ExtractConfigRequests(ces []ControlEvent) *ConfigRequest {


request and not requests

also add a TODO comment about what we should do if there are multiple config requests

tock-ibm · 2025-10-26T06:53:51Z

node/consensus/state/state.go

+func ExtractConfigRequest(ces []ControlEvent) *ConfigRequest {
+	// TODO: decide how to handle multiple config requests
+	for _, ce := range ces {
+		if ce.ConfigRequest != nil {
+			return ce.ConfigRequest
+		}
+	}
+	return nil
+}


In the context here, extract all into a slice. The decision happens by the caller of this function.

In general, the desired behavior is that the first valid config request goes through and is proposed, and the next ones are rejected, with feedback to the router.

feedback to the router happens in an earlier stage I believe, when the request is sent by router
as for returning a slice I agree that can be done
if not right now then in the next PR

We should remember there is a pending config change and not propose another before it is committed, correct?

No I am not sure we do

tock-ibm · 2025-10-26T06:58:48Z

node/consensus/state/state.go

 		copy(payloadToHash[22:], ce.BAF.Digest())
+	} else if ce.ConfigRequest != nil {
+		// TODO: maybe use a different ID for ConfigRequest
+		ce.ConfigRequest.Envelope.Signature = nil


Why set nil here? If this is the leader, it would contain its signature by this time.
(the second stage of verification changes the update envelope to a full config, with the leader consenter sig).

A getter should never be destructive...

this is done with other CEs (if you look at the top of this function)
maybe we should change this there as well?

Why is it done on other CEs?

we could fix it to all CEs

tock-ibm · 2025-10-26T07:00:23Z

node/consensus/state/state.go

 		copy(payloadToHash[22:], ce.BAF.Digest())
+	} else if ce.ConfigRequest != nil {
+		ce.ConfigRequest.Envelope.Signature = nil
+		payloadToHash = ce.ConfigRequest.Bytes()


Maybe the ID should hash both content and sig? i.e. the marshalled bytes?

tock-ibm · 2025-10-26T07:03:07Z

node/consensus/state/state.go

 		signerID = fmt.Sprintf("%d", ce.Complaint.Signer)
 	} else if ce.BAF != nil {
 		signerID = fmt.Sprintf("%d", ce.BAF.Signer())
+		// TODO: add ConfigRequest SignerID


The signer will be the leader node

the identity will be in the payload, in the sig header, as in a normal TX

sure
can be added later in another PR
there is a TODO

tock-ibm · 2025-10-26T07:19:11Z

node/consensus/consensus.go


 	c.stateLock.Lock()
-	computedState, attestations := c.Arma.SimulateStateTransition(c.State, batch)
+	computedState, attestations, _ := c.Arma.SimulateStateTransition(c.State, batch)


Extract the config request and do a log.Info on it if it exists.
Log there it is not supported yet.

But I will support it in the next PR

tock-ibm · 2025-10-26T07:19:41Z

node/consensus/consensus.go


 	c.stateLock.Lock()
-	_, bafs := c.Arma.SimulateStateTransition(c.State, requests)
+	_, bafs, _ := c.Arma.SimulateStateTransition(c.State, requests)


Extract the config request and do a log.Info on it if it exists.
Log there it is not supported yet.

But I will support it in the next PR

tock-ibm · 2025-10-26T07:20:05Z

node/consensus/consensus.go

 	c.stateLock.Lock()
-	newState, attestations := c.Arma.SimulateStateTransition(c.State, requests)
+	// TODO: config request not handled yet
+	newState, attestations, _ := c.Arma.SimulateStateTransition(c.State, requests)


Extract the config request and do a log.Info on it if it exists.
Log there it is not supported yet.

But I will support it in the next PR

tock-ibm · 2025-10-26T07:21:00Z

node/consensus/consenter.go

 // TODO revise the recovery from failure or shutdown, specifically the order of Commit and Append.
 func (c *Consenter) Commit(events [][]byte) {
-	state, batchAttestations := c.SimulateStateTransition(c.State, events)
+	state, batchAttestations, _ := c.SimulateStateTransition(c.State, events)


Extract the config request and do a log.Info on it if it exists.
Log there it is not supported yet.

But I will support it in the next PR

tock-ibm · 2025-10-26T07:26:19Z

node/consensus/state/state.go

+}
+
+func (c *ConfigRequest) String() string {
+	return "Config Request"


Some hash, the config sequence sequence? better to have more than less, this is a rare event.

tock-ibm · 2025-10-26T07:29:52Z

node/consensus/state/state.go

 }

-func (s *State) Process(l types.Logger, ces ...ControlEvent) (*State, []types.BatchAttestationFragment) {
+func (s *State) Process(l types.Logger, ces ...ControlEvent) (*State, []types.BatchAttestationFragment, *ConfigRequest) {


Will a config request change the state?
Should a config be also handled by a rule?

Nit - rename s2 into something meaningful... nextState maybe?

Right now it does not change the state
A rule may be added later
We can add here a TODO comment

Signed-off-by: Natalie Morad <natalie.morad@ibm.com>

tock-ibm

Let take all the comments in the next PR

HagarMeir reviewed Oct 21, 2025

View reviewed changes

moradna force-pushed the config-control-event branch from d6c7861 to 2a2f212 Compare October 21, 2025 09:48

HagarMeir reviewed Oct 21, 2025

View reviewed changes

HagarMeir force-pushed the config-control-event branch 3 times, most recently from d2ed4f0 to 5573cf7 Compare October 26, 2025 06:51

tock-ibm reviewed Oct 26, 2025

View reviewed changes

tock-ibm force-pushed the config-control-event branch from 5573cf7 to 2cf981d Compare October 27, 2025 14:20

moradna added 2 commits October 28, 2025 14:23

add config request to control event

6da63f4

Signed-off-by: Natalie Morad <natalie.morad@ibm.com>

fix review

09be6c4

Signed-off-by: Natalie Morad <natalie.morad@ibm.com>

tock-ibm force-pushed the config-control-event branch from 2cf981d to 09be6c4 Compare October 28, 2025 12:23

tock-ibm approved these changes Oct 28, 2025

View reviewed changes

tock-ibm merged commit 0cf9d51 into hyperledger:main Oct 28, 2025
7 checks passed

HagarMeir mentioned this pull request Oct 28, 2025

fix some issues with config reqs control events #320

Open

Uh oh!

add config request to control event #305

add config request to control event #305

Conversation

moradna commented Oct 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

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!

tock-ibm Oct 26, 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!

tock-ibm Oct 27, 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

moradna commented Oct 20, 2025 •

edited

Loading

tock-ibm Oct 26, 2025 •

edited

Loading

tock-ibm Oct 27, 2025 •

edited

Loading

HagarMeir Oct 26, 2025 •

edited

Loading