Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

change evaluate RequestId for update config #4585

Merged
merged 1 commit into from
Dec 28, 2023

Conversation

pfi79
Copy link
Contributor

@pfi79 pfi79 commented Dec 27, 2023

In the documentation and in the examples, the channel update transaction is send to one and only one orderer. Like in raft. The smartbft features are not used.

What's happening?
When an orderer receives a channel update transaction, it takes the current channel config and applies the update to it and re-signs the transaction. It then sends it to the smartbft consensus. Total 7 different transactions appear in the consensus.

My current solution fixes the error. RequestID will be calculated differently for channel update transactions.

After this change, the channel update can be sent out as a bft.

@pfi79 pfi79 requested a review from a team as a code owner December 27, 2023 05:01
@pfi79 pfi79 force-pushed the 2-part-bft-update-config branch from 5facf86 to 5f5b50a Compare December 27, 2023 05:35
@pfi79 pfi79 force-pushed the 2-part-bft-update-config branch 2 times, most recently from 41726a5 to 9bb8799 Compare December 27, 2023 19:33
return types.RequestInfo{}
}

req, err = ri.unwrapReqFromEnvelop(configEnvelope.LastUpdate)
Copy link
Contributor

Choose a reason for hiding this comment

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

let's log here a warning

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix

@pfi79 pfi79 force-pushed the 2-part-bft-update-config branch from 9bb8799 to 557c936 Compare December 28, 2023 09:54
@pfi79 pfi79 force-pushed the 2-part-bft-update-config branch from 557c936 to 5f8f833 Compare December 28, 2023 11:24
if err != nil {
v.Logger.Errorf("Error verifying config update: %v", err)
return types.RequestInfo{}, err
}

return v.ReqInspector.RequestID(rawRequest), nil
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe check if it returns the zero value (empty request ID) then return error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fix

@pfi79 pfi79 force-pushed the 2-part-bft-update-config branch from 5f8f833 to 5059e96 Compare December 28, 2023 11:47
@pfi79 pfi79 force-pushed the 2-part-bft-update-config branch from 5059e96 to ab65998 Compare December 28, 2023 12:32
@yacovm yacovm enabled auto-merge (squash) December 28, 2023 14:05
@yacovm yacovm merged commit fc588b5 into hyperledger:main Dec 28, 2023
13 checks passed
@pfi79 pfi79 deleted the 2-part-bft-update-config branch December 28, 2023 17:13
pfi79 added a commit to scientificideas/fabric that referenced this pull request Dec 28, 2023
Signed-off-by: Fedor Partanskiy <[email protected]>

(cherry picked from commit fc588b5)
Signed-off-by: Fedor Partanskiy <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants