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

Add SmartBFT consensus support #3781

Merged
merged 53 commits into from
Feb 20, 2023
Merged

Conversation

Param-S
Copy link
Contributor

@Param-S Param-S commented Nov 14, 2022

Type of change

  • New feature

Description

Add support for BFT consensus to orderer consensus plugin

Additional details

RFC: https://hyperledger.github.io/fabric-rfcs/text/006-bft-based-ordering-service.html

Related issues

EPIC task: #3771

Signed-off-by: Yacov Manevich yacovm@il.ibm.com
Signed-off-by: Yoav Tock tock@il.ibm.com
Signed-off-by: Parameswaran Selvam parselva@in.ibm.com

@Param-S Param-S requested a review from a team as a code owner November 14, 2022 12:17
@Param-S Param-S marked this pull request as draft November 14, 2022 12:17
@Param-S Param-S changed the title add SMaRtBFT consensus support WIP: add SMaRtBFT consensus support Nov 14, 2022
go.sum Outdated Show resolved Hide resolved
common/policies/bft.go Outdated Show resolved Hide resolved
// Scenario V: A valid config envelope is passed
require.NoError(t, cs.VerifyBlockSignature([]*protoutil.SignedData{}, testConfigEnvelope(t)))
}
// func TestLedgerResources_VerifyBlockSignature(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this commented out?

@Param-S Param-S force-pushed the bftchain branch 4 times, most recently from 25cdcfd to 4d5a104 Compare February 7, 2023 04:51
common/crypto/signer.go Outdated Show resolved Hide resolved
common/policies/bft.go Outdated Show resolved Hide resolved
integration/nwo/network.go Outdated Show resolved Hide resolved
@@ -158,19 +158,16 @@ Profiles:{{ range .Profiles }}
PreferredMaxBytes: 512 KB
{{- end}}
Capabilities:
{{- if eq $w.Consensus.Type "BFT" }}
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you deleting this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Why indeed?

yacovm and others added 23 commits February 17, 2023 19:37
Signed-off-by: Yacov Manevich <yacov.manevich@ibm.com>
Signed-off-by: Yacov Manevich <yacov.manevich@ibm.com>
Signed-off-by: Parameswaran Selvam <parselva@in.ibm.com>
Signed-off-by: Parameswaran Selvam <parselva@in.ibm.com>
Signed-off-by: Parameswaran Selvam <parselva@in.ibm.com>
Signed-off-by: Parameswaran Selvam <parselva@in.ibm.com>
Signed-off-by: Parameswaran Selvam <parselva@in.ibm.com>
Signed-off-by: Parameswaran Selvam <parselva@in.ibm.com>
Signed-off-by: Yacov Manevich <yacov.manevich@ibm.com>
Signed-off-by: Yacov Manevich <yacov.manevich@ibm.com>
Signed-off-by: Yacov Manevich <yacov.manevich@ibm.com>
Signed-off-by: Yacov Manevich <yacov.manevich@ibm.com>
Signed-off-by: Yacov Manevich <yacov.manevich@ibm.com>
Signed-off-by: Yacov Manevich <yacov.manevich@ibm.com>
Signed-off-by: Yacov Manevich <yacov.manevich@ibm.com>
Signed-off-by: Yacov Manevich <yacov.manevich@ibm.com>
Signed-off-by: Yacov Manevich <yacov.manevich@ibm.com>
Signed-off-by: Yacov Manevich <yacov.manevich@ibm.com>
Signed-off-by: Yacov Manevich <yacov.manevich@ibm.com>
Signed-off-by: Yacov Manevich <yacov.manevich@ibm.com>
Signed-off-by: Yacov Manevich <yacov.manevich@ibm.com>
Signed-off-by: Yacov Manevich <yacov.manevich@ibm.com>
Signed-off-by: Yacov Manevich <yacov.manevich@ibm.com>
Signed-off-by: Yacov Manevich <yacov.manevich@ibm.com>
@andrew-coleman andrew-coleman merged commit cf9030b into hyperledger:main Feb 20, 2023
Copy link
Contributor

@tock-ibm tock-ibm left a comment

Choose a reason for hiding this comment

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

This is just a quick review of the integration tests.

@@ -41,6 +41,7 @@ const (
PrivateDataPurgeBasePort
RaftBasePort
SBEBasePort
SmartBFTBasePort
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the SmartBFTBasePort different than the RaftBasePort? In general, that could be renamed the "ConsensusBasePort"?

Comment on lines +45 to +50

doBody(client, req)

Eventually(func() ChannelInfo {
return ListOne(n, o, channel)
}, n.EventuallyTimeout).Should(Equal(expectedChannelInfo))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this change here?
It is unrelated to BFT, and I think it is wrong. We check the returned body for the correct response. ListOne is done always from the outside later in the tests.,

Comment on lines +15 to +17
"github.com/hyperledger/fabric/common/channelconfig"
"github.com/hyperledger/fabric/common/policies"

Copy link
Contributor

Choose a reason for hiding this comment

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

goimport

@@ -158,19 +158,16 @@ Profiles:{{ range .Profiles }}
PreferredMaxBytes: 512 KB
{{- end}}
Capabilities:
{{- if eq $w.Consensus.Type "BFT" }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why indeed?

Comment on lines +313 to +315
Status: "active",
ConsensusRelation: "consenter",
Height: 0,
Height: 2,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is wrong, because Join was changed incorrectly.


docker "github.com/fsouza/go-dockerclient"
"github.com/hyperledger/fabric-config/configtx"
"github.com/hyperledger/fabric-config/configtx/orderer"
Copy link
Contributor

Choose a reason for hiding this comment

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

go import


for _, o := range network.Orderers {
By("joining " + o.Name + " to channel as a consenter")
channelparticipation.Join(network, o, "testchannel1", genesisBlock, expectedChannelInfoPT)
Copy link
Contributor

Choose a reason for hiding this comment

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

Join was changed incorrectly. Every usage of Join in the new tests should be revisited.

Comment on lines +238 to +241
By("Waiting for followers to see the leader")
Eventually(ordererRunners[1].Err(), network.EventuallyTimeout, time.Second).Should(gbytes.Say("Message from 1"))
Eventually(ordererRunners[2].Err(), network.EventuallyTimeout, time.Second).Should(gbytes.Say("Message from 1"))
Eventually(ordererRunners[3].Err(), network.EventuallyTimeout, time.Second).Should(gbytes.Say("Message from 1"))
Copy link
Contributor

Choose a reason for hiding this comment

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

how does that prove the existence of a leader?
who guarantees the leader is orderer 0?
see how it is done in Raft IT

Comment on lines +350 to +354
By("Waiting for view change to occur")
Eventually(ordererRunners[1].Err(), network.EventuallyTimeout*2, time.Second).Should(gbytes.Say("Changing to leader role, current view: 1, current leader: 2 channel=testchannel1"))
Eventually(ordererRunners[2].Err(), network.EventuallyTimeout*2, time.Second).Should(gbytes.Say("Changing to follower role, current view: 1, current leader: 2 channel=testchannel1"))
Eventually(ordererRunners[3].Err(), network.EventuallyTimeout*2, time.Second).Should(gbytes.Say("Changing to follower role, current view: 1, current leader: 2 channel=testchannel1"))
Eventually(ordererRunners[4].Err(), network.EventuallyTimeout*2, time.Second).Should(gbytes.Say("Changing to follower role, current view: 1, current leader: 2 channel=testchannel1"))
Copy link
Contributor

Choose a reason for hiding this comment

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

again, how can you be sure who the leader will be?

URL: "/participation/v1/channels/" + channel,
Status: "active",
ConsensusRelation: "consenter",
Height: 1,
Copy link
Contributor

Choose a reason for hiding this comment

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

Join was changed incorrectly. Revisit

}

for _, consenter := range oc.Consenters() {
fmt.Println(base64.StdEncoding.EncodeToString(consenter.ServerTlsCert))
Copy link
Contributor

Choose a reason for hiding this comment

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

leftover from a debug session?

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.

4 participants