Fix test-network to work with BFT consensus. #1047

arkadiPiven · 2023-05-31T17:16:27Z

Added a new option for creating channel:
Running ./network.sh createChannel -bft will initiate a channel running BFT orderers.

arkadiPiven · 2023-05-31T17:18:25Z

@yacovm

test-network/bft-config/configtx.yaml

test-network/scripts/configUpdate.sh

yacovm · 2023-07-27T14:03:54Z

test-network/scripts/createChannel.sh

@@ -3,15 +3,19 @@
 # imports  
 . scripts/envVar.sh
 . scripts/utils.sh
+export PATH="${PWD}"/../../fabric/build/bin:"${ROOTDIR}"/../bin:"${PWD}"/../bin:"$PATH"


are you sure this is needed?

You need this in case someone will want to use old binaries.
Me and Dave had the same approach when constructing nano-bash

Most people that download the samples and try it out will not have a fabric directory. I'd suggest to remove this change.
We added it for the nano bash sample since a target user of that sample was a Fabric developer who compiles their own binaries.

yacovm · 2023-08-03T11:29:01Z

@denyeart let's get this PR in, shall we?

test-network/bft-config/configtx.yaml

test-network/configtx/configtx.yaml

denyeart · 2023-08-07T19:27:31Z

test-network/scripts/createChannel.sh

@@ -3,15 +3,19 @@
 # imports  
 . scripts/envVar.sh
 . scripts/utils.sh
+export PATH="${PWD}"/../../fabric/build/bin:"${ROOTDIR}"/../bin:"${PWD}"/../bin:"$PATH"


Most people that download the samples and try it out will not have a fabric directory. I'd suggest to remove this change.
We added it for the nano bash sample since a target user of that sample was a Fabric developer who compiles their own binaries.

denyeart · 2023-08-07T19:27:42Z

test-network/network.sh

@@ -18,7 +18,7 @@
 # this script is actually in and infer location from there. (putting first)

 ROOTDIR=$(cd "$(dirname "$0")" && pwd)
-export PATH=${ROOTDIR}/../bin:${PWD}/../bin:$PATH
+export PATH="${PWD}"/../../fabric/build/bin:"${ROOTDIR}"/../bin:"${PWD}"/../bin:"$PATH"


Most people that download the samples and try it out will not have a fabric directory. I'd suggest to remove this change.
We added it for the nano bash sample since a target user of that sample was a Fabric developer who compiles their own binaries.

test-network/scripts/orderer.sh

test-network/README.md

denyeart · 2023-08-17T19:20:55Z

test-network/network.sh

@@ -316,7 +319,8 @@ function deployCCAAS() {

 # Tear down running network
 function networkDown() {
-
+  local temp_compose=$COMPOSE_FILE_BASE
+  COMPOSE_FILE_BASE=compose-bft-test-net.yaml


I'm not following this change... won't this always force the use of compose-bft-test-net.yaml instead of the default compose-test-net.yaml?
Why is the temp_compose required here?

You are correct.
The compose-bft-test-net.yaml is a super set of the compose-test-net.yaml. This means that if a user launched a regular network or a bft network, during shutdown all containers will be stopped no matter what initial configuration the user chose.
This actually spares me to pass any flags to the ./network.sh down command, and to refer to different kinds of networks.

yacovm · 2023-08-20T13:44:31Z

test-network/ADD_ORDERER_TUTORIAL.md

@@ -0,0 +1,310 @@
+# Adding Orderer To An Existing Network


This should be part of the official documentation, not in the samples.

Added a new option for creating channel: Running ./network.sh createChannel -bft will initiate a channel running BFT orderers. Using ./network.sh up -bft will initiate dockers for bft environment. Added option for 4 orderers. Add add_new_orderer_to_config.py which is referenced in the fabric official docs. Signed-off-by: Arkadi Piven <[email protected]> Signed-off-by: arkadipiven <[email protected]>

denyeart · 2023-08-21T23:05:40Z

test-network/scripts/add_new_orderer_to_config.py

+    identities = config['channel_group']['groups']['Orderer']['policies']['BlockValidation']['policy']['value']['identities']
+    identities_before_update = copy.deepcopy(identities)
+    new_identity = copy.deepcopy(identities[0])
+    new_identity['principal']['id_bytes'] = identity
+    identities.append(new_identity)
+    _log_update('block validation identities', identities_before_update, identities)
+
+    rule = config['channel_group']['groups']['Orderer']['policies']['BlockValidation']['policy']['value'][
+        'rule']
+    rule_before_update = copy.deepcopy(rule)
+    rule['n_out_of']['n'] = _calculate_bft_quorum(new_orderers_count)
+    rule['n_out_of']['rules'].append({'signed_by': new_orderers_count - 1})
+    _log_update('block validation rules', rule_before_update, rule)


One general question and one specific question.

General question - I didn't see this file originally... is it used in this PR? Or will other instructions use it?

Specific question - I didn't see anything like these BlockValidation policies in the original configtx.yaml, like I did for the Orderer Endpoints (above) and consenter mapping (below). Can you educate me on this... why are they not in configtx.yaml? Is it in the channel config implicitly somehow originally? Is this new for BFT?

I didn't see anything like these BlockValidation policies in the original configtx.yaml, like I did for the Orderer Endpoints (above) and consenter mapping (below). Can you educate me on this... why are they not in configtx.yaml? Is it in the channel config implicitly somehow originally? Is this new for BFT?

I'll answer this one, and will let Arkadi answer the first one.

First of all, remember that the "name" of the policy in configtx.yaml is a mere pointer to a meta policy of "any orderer".
The meta policies we have, or, the policies we have in general in Fabric, are not suitable to define BFT policies, as BFT policies depend on additional input found in the channel config (the consenters...) . To add support for that, it requires invasive changes to the channel config infrastructure.

However, a signature policy is a supported policy that can always be used and it is flexible enough to fit BFT block validation.

In the spirit of making Fabric more generic and pluggable, we don't want to define policy names for every type of consensus we can imagine. Some protocols require more signatures, some require less, some require a single threshold signature, etc.
While in Raft it's easy to define a policy string ("any orderer"), for BFT it's complex to do that in the configtx.yaml, and it can also be error prone.

For BFT, the Fabric tooling (configtxgen) overrides the block validation policy that is defined in the configtx.yaml, both in order to "protect the user from a misconfiguration", and also because it was my belief that a user should not need to understand the underlying BFT protocol to configure the policy.
So, when a channel is created using configtxgen, the genesis block contains the right policy. When a channel is updated, the orderer itself checks that the policy was correctly computed via an ad-hoc verification.

For the general question:
This file is used in the following tutorial:
hyperledger/fabric#4390
Please look at the add_orderer.md committed file,
Under section "Add the fifth orderer to the config" subsection 4.

denyeart

The sample looks great now, let's go ahead and merge while we finish out the doc PR.

arkadiPiven requested a review from a team as a code owner May 31, 2023 17:16

arkadiPiven force-pushed the test-network-bft branch from 1fb3ed2 to c69771c Compare May 31, 2023 17:17

arkadiPiven force-pushed the test-network-bft branch 3 times, most recently from 7519e1d to d7529ea Compare June 28, 2023 20:56

yacovm reviewed Jun 28, 2023

View reviewed changes

test-network/bft-config/configtx.yaml Show resolved Hide resolved

yacovm reviewed Jul 27, 2023

View reviewed changes

test-network/scripts/configUpdate.sh Outdated Show resolved Hide resolved

yacovm reviewed Jul 27, 2023

View reviewed changes

arkadiPiven force-pushed the test-network-bft branch from d7529ea to 63a0ef4 Compare August 1, 2023 18:21

arkadiPiven force-pushed the test-network-bft branch from 63a0ef4 to 38fda52 Compare August 3, 2023 17:21

denyeart reviewed Aug 7, 2023

View reviewed changes

denyeart reviewed Aug 8, 2023

View reviewed changes

test-network/scripts/orderer.sh Outdated Show resolved Hide resolved

denyeart reviewed Aug 9, 2023

View reviewed changes

test-network/README.md Outdated Show resolved Hide resolved

arkadiPiven force-pushed the test-network-bft branch 2 times, most recently from 6c693ad to b8c697b Compare August 17, 2023 17:09

denyeart reviewed Aug 17, 2023

View reviewed changes

arkadiPiven force-pushed the test-network-bft branch from b8c697b to 5436366 Compare August 20, 2023 13:26

arkadiPiven mentioned this pull request Aug 20, 2023

Document addition and removal of BFT nodes from channel config hyperledger/fabric#4332

Open

yacovm reviewed Aug 20, 2023

View reviewed changes

arkadiPiven force-pushed the test-network-bft branch from 5436366 to 14872dc Compare August 21, 2023 09:26

denyeart reviewed Aug 21, 2023

View reviewed changes

denyeart approved these changes Aug 22, 2023

View reviewed changes

denyeart merged commit eb16caf into hyperledger:main Aug 22, 2023
36 of 41 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix test-network to work with BFT consensus. #1047

Fix test-network to work with BFT consensus. #1047

arkadiPiven commented May 31, 2023

arkadiPiven commented May 31, 2023

yacovm Jul 27, 2023

arkadiPiven Aug 1, 2023

denyeart Aug 7, 2023

yacovm commented Aug 3, 2023

denyeart Aug 7, 2023

denyeart Aug 7, 2023

denyeart Aug 17, 2023

arkadiPiven Aug 17, 2023

yacovm Aug 20, 2023

denyeart Aug 21, 2023

yacovm Aug 22, 2023 •

edited

Loading

arkadiPiven Aug 22, 2023 •

edited

Loading

denyeart left a comment

Fix test-network to work with BFT consensus. #1047

Fix test-network to work with BFT consensus. #1047

Conversation

arkadiPiven commented May 31, 2023

arkadiPiven commented May 31, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yacovm commented Aug 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yacovm Aug 22, 2023 • edited Loading

Choose a reason for hiding this comment

arkadiPiven Aug 22, 2023 • edited Loading

Choose a reason for hiding this comment

denyeart left a comment

Choose a reason for hiding this comment

yacovm Aug 22, 2023 •

edited

Loading

arkadiPiven Aug 22, 2023 •

edited

Loading