From 3947b14a79ff983661571bb44f597b769437a06e Mon Sep 17 00:00:00 2001 From: Hein Meling Date: Sat, 15 Nov 2025 11:51:50 +0100 Subject: [PATCH 1/6] fix: partition structure in fhsBugScenario to match new format In a recent change, the partitions now require a full NodeID, and thus the fhsBugScenario failed to parse the old JSON format. --- twins/fhsbug_test.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/twins/fhsbug_test.go b/twins/fhsbug_test.go index 85bc0f06..bf3b31fa 100644 --- a/twins/fhsbug_test.go +++ b/twins/fhsbug_test.go @@ -22,57 +22,57 @@ const fhsBugScenario = ` [ { "leader": 1, - "partitions": [ [1, 2, 3, 4], [] ], + "partitions": [ [{"ReplicaID": 1}, {"ReplicaID": 2}, {"ReplicaID": 3}, {"ReplicaID": 4}], [] ], "comment": "view 1" }, { "leader": 1, - "partitions": [ [1, 2, 3, 4], [] ], + "partitions": [ [{"ReplicaID": 1}, {"ReplicaID": 2}, {"ReplicaID": 3}, {"ReplicaID": 4}], [] ], "comment": "view 2" }, { "leader": 1, - "partitions": [ [1, 2, 3, 4], [] ], + "partitions": [ [{"ReplicaID": 1}, {"ReplicaID": 2}, {"ReplicaID": 3}, {"ReplicaID": 4}], [] ], "comment": "view 3" }, { "leader": 1, - "partitions": [ [1, 2, 3, 4], [] ], + "partitions": [ [{"ReplicaID": 1}, {"ReplicaID": 2}, {"ReplicaID": 3}, {"ReplicaID": 4}], [] ], "comment": "view 4. replicas time out in this view" }, { "leader": 2, - "partitions": [ [1, 3, 4], [2] ], + "partitions": [ [{"ReplicaID": 1}, {"ReplicaID": 3}, {"ReplicaID": 4}], [{"ReplicaID": 2}] ], "comment": "view 5." }, { "leader": 1, - "partitions": [ [1, 3, 4], [2] ], + "partitions": [ [{"ReplicaID": 1}, {"ReplicaID": 3}, {"ReplicaID": 4}], [{"ReplicaID": 2}] ], "comment": "view 6" }, { "leader": 3, - "partitions": [ [1, 2, 4], [3] ], + "partitions": [ [{"ReplicaID": 1}, {"ReplicaID": 2}, {"ReplicaID": 4}], [{"ReplicaID": 3}] ], "comment": "view 7" }, { "leader": 2, - "partitions": [ [1, 2, 4], [3] ], + "partitions": [ [{"ReplicaID": 1}, {"ReplicaID": 2}, {"ReplicaID": 4}], [{"ReplicaID": 3}] ], "comment": "view 8" }, { "leader": 2, - "partitions": [ [1, 3, 4], [2] ], + "partitions": [ [{"ReplicaID": 1}, {"ReplicaID": 3}, {"ReplicaID": 4}], [{"ReplicaID": 2}] ], "comment": "view 9" }, { "leader": 3, - "partitions": [ [1, 3, 4], [2] ], + "partitions": [ [{"ReplicaID": 1}, {"ReplicaID": 3}, {"ReplicaID": 4}], [{"ReplicaID": 2}] ], "comment": "view 10" }, { "leader": 3, - "partitions": [ [1, 3, 4], [2] ], + "partitions": [ [{"ReplicaID": 1}, {"ReplicaID": 3}, {"ReplicaID": 4}], [{"ReplicaID": 2}] ], "comment": "view 11" } ] From e1684ba110fcdfa8923a9933a43379f90e1c151b Mon Sep 17 00:00:00 2001 From: Hein Meling Date: Sat, 15 Nov 2025 15:50:31 +0100 Subject: [PATCH 2/6] refactor: adds newTwinsConsensusRules for consensus rule creation This simplifies creating custom consensus rules for twins. --- twins/node.go | 24 ++++++++---------------- twins/twinsrules.go | 26 ++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 16 deletions(-) create mode 100644 twins/twinsrules.go diff --git a/twins/node.go b/twins/node.go index 41e9d871..6ae8df24 100644 --- a/twins/node.go +++ b/twins/node.go @@ -68,22 +68,14 @@ func newNode(n *Network, nodeID NodeID, consensusName string, pk *ecdsa.PrivateK base, ) node.blockchain = depsSecurity.Blockchain() - var consensusRules consensus.Ruleset - if consensusName == nameVulnerableFHS { - consensusRules = NewVulnFHS( - node.logger, - node.blockchain, - rules.NewFastHotStuff( - node.logger, - node.config, - node.blockchain, - ), - ) - } else { - consensusRules, err = rules.New(node.logger, node.config, node.blockchain, consensusName) - if err != nil { - return nil, err - } + consensusRules, err := newTwinsConsensusRules( + node.logger, + node.config, + node.blockchain, + consensusName, + ) + if err != nil { + return nil, err } node.viewStates, err = protocol.NewViewStates(node.blockchain, depsSecurity.Authority()) if err != nil { diff --git a/twins/twinsrules.go b/twins/twinsrules.go new file mode 100644 index 00000000..df8d86aa --- /dev/null +++ b/twins/twinsrules.go @@ -0,0 +1,26 @@ +package twins + +import ( + "github.com/relab/hotstuff/core" + "github.com/relab/hotstuff/core/logging" + "github.com/relab/hotstuff/protocol/consensus" + "github.com/relab/hotstuff/protocol/rules" + "github.com/relab/hotstuff/security/blockchain" +) + +func newTwinsConsensusRules( + logger logging.Logger, + config *core.RuntimeConfig, + blockchain *blockchain.Blockchain, + name string, +) (ruleset consensus.Ruleset, err error) { + switch name { + case nameVulnerableFHS: + ruleset = NewVulnFHS(logger, blockchain, + rules.NewFastHotStuff(logger, config, blockchain), + ) + default: + ruleset, err = rules.New(logger, config, blockchain, name) + } + return +} From 9d08adeaa9e79695ad983430a1cd280bd9de4c6c Mon Sep 17 00:00:00 2001 From: Hein Meling Date: Sat, 15 Nov 2025 15:55:31 +0100 Subject: [PATCH 3/6] doc(fasthotstuff): minor update --- protocol/rules/fasthotstuff.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/protocol/rules/fasthotstuff.go b/protocol/rules/fasthotstuff.go index 340e210a..c765986a 100644 --- a/protocol/rules/fasthotstuff.go +++ b/protocol/rules/fasthotstuff.go @@ -12,20 +12,21 @@ import ( const NameFastHotStuff = "fasthotstuff" // FastHotStuff is an implementation of the Fast-HotStuff protocol. +// See the paper for details: https://arxiv.org/abs/2010.11454 type FastHotStuff struct { logger logging.Logger config *core.RuntimeConfig blockchain *blockchain.Blockchain } -// NewFastHotStuff returns a new instance of the Fast-HotStuff consensus ruleset. +// NewFastHotStuff returns a new instance of the FastHotStuff consensus ruleset. func NewFastHotStuff( logger logging.Logger, config *core.RuntimeConfig, blockchain *blockchain.Blockchain, ) *FastHotStuff { if !config.HasAggregateQC() { - panic("aggregate qc must be enabled for fasthotstuff") + panic(NameFastHotStuff + " requires aggregated quorum certificates") } return &FastHotStuff{ logger: logger, From c59972c5226cf3d3d5bde0a3dbc8ae085b9ce177 Mon Sep 17 00:00:00 2001 From: Hein Meling Date: Sat, 15 Nov 2025 16:29:26 +0100 Subject: [PATCH 4/6] refactor: streamline timeout ruler initialization in node creation This harmonizes the creation of the TimeoutRuler to be based only on the RuntimeConfig instead of custom if-else logic scattered around the codebase. --- internal/orchestration/worker.go | 8 +------- protocol/synchronizer/timeoutrules.go | 14 +++++++++++++- twins/node.go | 11 +---------- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/internal/orchestration/worker.go b/internal/orchestration/worker.go index f0ba5e39..36f8c89d 100644 --- a/internal/orchestration/worker.go +++ b/internal/orchestration/worker.go @@ -211,12 +211,6 @@ func (w *Worker) createReplica(opts *orchestrationpb.ReplicaOpts) (*replica.Repl base, ) - var timeoutRules synchronizer.TimeoutRuler - if depsCore.RuntimeCfg().HasAggregateQC() { - timeoutRules = synchronizer.NewAggregate(depsCore.RuntimeCfg(), depsSecure.Authority()) - } else { - timeoutRules = synchronizer.NewSimple(depsCore.RuntimeCfg(), depsSecure.Authority()) - } consensusRules, viewStates, leaderRotation, comm, viewDuration, err := initConsensusModules(depsCore, depsSecure, sender, opts) if err != nil { return nil, err @@ -230,7 +224,7 @@ func (w *Worker) createReplica(opts *orchestrationpb.ReplicaOpts) (*replica.Repl leaderRotation, consensusRules, viewDuration, - timeoutRules, + synchronizer.NewTimeoutRuler(depsCore.RuntimeCfg(), depsSecure.Authority()), opts.GetBatchSize(), replicaOpts..., ) diff --git a/protocol/synchronizer/timeoutrules.go b/protocol/synchronizer/timeoutrules.go index 67c93cf0..3c953e30 100644 --- a/protocol/synchronizer/timeoutrules.go +++ b/protocol/synchronizer/timeoutrules.go @@ -1,6 +1,18 @@ package synchronizer -import "github.com/relab/hotstuff" +import ( + "github.com/relab/hotstuff" + "github.com/relab/hotstuff/core" + "github.com/relab/hotstuff/security/cert" +) + +// NewTimeoutRuler returns a TimeoutRuler based on the configuration. +func NewTimeoutRuler(cfg *core.RuntimeConfig, auth *cert.Authority) TimeoutRuler { + if cfg.HasAggregateQC() { + return NewAggregate(cfg, auth) + } + return NewSimple(cfg, auth) +} type TimeoutRuler interface { LocalTimeoutRule(hotstuff.View, hotstuff.SyncInfo) (*hotstuff.TimeoutMsg, error) diff --git a/twins/node.go b/twins/node.go index 6ae8df24..675c7494 100644 --- a/twins/node.go +++ b/twins/node.go @@ -15,7 +15,6 @@ import ( "github.com/relab/hotstuff/protocol/comm" "github.com/relab/hotstuff/protocol/consensus" "github.com/relab/hotstuff/protocol/leaderrotation" - "github.com/relab/hotstuff/protocol/rules" "github.com/relab/hotstuff/protocol/synchronizer" "github.com/relab/hotstuff/protocol/votingmachine" "github.com/relab/hotstuff/security/blockchain" @@ -116,14 +115,6 @@ func newNode(n *Network, nodeID NodeID, consensusName string, pk *ecdsa.PrivateK node.commandCache, committer, ) - var timeoutRules synchronizer.TimeoutRuler - if consensusName == rules.NameFastHotStuff || consensusName == nameVulnerableFHS { - // Use aggregated quorum certificates. - // This must be true for Fast-HotStuff: https://arxiv.org/abs/2010.11454 - timeoutRules = synchronizer.NewAggregate(node.config, depsSecurity.Authority()) - } else { - timeoutRules = synchronizer.NewSimple(node.config, depsSecurity.Authority()) - } node.synchronizer = synchronizer.New( node.eventLoop, node.logger, @@ -131,7 +122,7 @@ func newNode(n *Network, nodeID NodeID, consensusName string, pk *ecdsa.PrivateK depsSecurity.Authority(), node.leaderRotation, synchronizer.NewFixedDuration(500*time.Millisecond), - timeoutRules, + synchronizer.NewTimeoutRuler(node.config, depsSecurity.Authority()), node.proposer, node.voter, node.viewStates, From 9f783838f5cccab829c4652c3ccc6a25e1a48552 Mon Sep 17 00:00:00 2001 From: Hein Meling Date: Sat, 15 Nov 2025 17:16:39 +0100 Subject: [PATCH 5/6] refactor: ExecuteScenario to accept variadic runtime options This makes it possible to pass down runtime options specific for a given scenario, e.g., if we use fasthotstuff, we need to pass the WithAggregateQC() option. --- twins/fhsbug_test.go | 5 +++-- twins/network.go | 7 +++---- twins/node.go | 5 +++-- twins/scenario.go | 6 ++++-- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/twins/fhsbug_test.go b/twins/fhsbug_test.go index bf3b31fa..9c43cf42 100644 --- a/twins/fhsbug_test.go +++ b/twins/fhsbug_test.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + "github.com/relab/hotstuff/core" "github.com/relab/hotstuff/core/logging" "github.com/relab/hotstuff/twins" ) @@ -86,7 +87,7 @@ var ( ) func TestFHSBug(t *testing.T) { - t.Skip("This test is not working as expected; skipping until we have fixed the issue.") + // t.Skip("This test is not working as expected; skipping until we have fixed the issue.") logging.SetLogLevel(*logLevel) src, err := twins.FromJSON(strings.NewReader(fhsBugScenario)) @@ -101,7 +102,7 @@ func TestFHSBug(t *testing.T) { settings := src.Settings() - res, err := twins.ExecuteScenario(scenario, settings.NumNodes, settings.NumTwins, 100, vulnerableModule) + res, err := twins.ExecuteScenario(scenario, settings.NumNodes, settings.NumTwins, 100, vulnerableModule, core.WithAggregateQC()) if err != nil { t.Fatalf("failed to execute scenario: %v", err) } diff --git a/twins/network.go b/twins/network.go index e5ddce66..52719eee 100644 --- a/twins/network.go +++ b/twins/network.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/relab/hotstuff" + "github.com/relab/hotstuff/core" "github.com/relab/hotstuff/core/logging" "github.com/relab/hotstuff/security/crypto/keygen" ) @@ -115,7 +116,7 @@ func NewPartitionedNetwork(views []View, dropTypes ...any) *Network { // createNodesAndTwins creates nodes and their twins in the network. // Twins receive the same private key. -func (n *Network) createNodesAndTwins(nodes []NodeID, consensusName string) error { +func (n *Network) createNodesAndTwins(nodes []NodeID, consensusName string, opts ...core.RuntimeOption) error { for _, nodeID := range nodes { var privKey *ecdsa.PrivateKey var err error @@ -129,7 +130,7 @@ func (n *Network) createNodesAndTwins(nodes []NodeID, consensusName string) erro // reuse the private key of the first replica privKey = twins[0].config.PrivateKey().(*ecdsa.PrivateKey) } - node, err := newNode(n, nodeID, consensusName, privKey) + node, err := newNode(n, nodeID, consensusName, privKey, opts...) if err != nil { return fmt.Errorf("failed to create node %v: %w", nodeID, err) } @@ -200,13 +201,11 @@ func (n *Network) tick() { for node.eventLoop.Tick(context.Background()) { //revive:disable-line:empty-block } } - } // shouldDrop decides if the sender should drop the message, based on the current view of the sender and the // partitions configured for that view. func (n *Network) shouldDrop(sender, receiver NodeID, message any, view hotstuff.View) bool { - // Index into viewPartitions. i := int(view) - 1 diff --git a/twins/node.go b/twins/node.go index 675c7494..5403af84 100644 --- a/twins/node.go +++ b/twins/node.go @@ -42,11 +42,12 @@ type node struct { log strings.Builder } -func newNode(n *Network, nodeID NodeID, consensusName string, pk *ecdsa.PrivateKey) (*node, error) { +func newNode(n *Network, nodeID NodeID, consensusName string, pk *ecdsa.PrivateKey, opts ...core.RuntimeOption) (*node, error) { cryptoName := crypto.NameECDSA + allOpts := append([]core.RuntimeOption{core.WithSyncVerification(), core.WithCache(100)}, opts...) node := &node{ id: nodeID, - config: core.NewRuntimeConfig(nodeID.ReplicaID, pk, core.WithSyncVerification(), core.WithCache(100)), + config: core.NewRuntimeConfig(nodeID.ReplicaID, pk, allOpts...), commandCache: clientpb.NewCommandCache(1), } node.logger = logging.NewWithDest(&n.log, fmt.Sprintf("r%dn%d", nodeID.ReplicaID, nodeID.TwinID)) diff --git a/twins/scenario.go b/twins/scenario.go index ae23646c..d7463549 100644 --- a/twins/scenario.go +++ b/twins/scenario.go @@ -5,6 +5,8 @@ import ( "strconv" "strings" + "github.com/relab/hotstuff/core" + "github.com/relab/hotstuff" "github.com/relab/hotstuff/internal/proto/clientpb" "github.com/relab/hotstuff/protocol/leaderrotation" @@ -46,7 +48,7 @@ type ScenarioResult struct { } // ExecuteScenario executes a twins scenario. -func ExecuteScenario(scenario Scenario, numNodes, numTwins uint8, numTicks int, consensusName string) (result ScenarioResult, err error) { +func ExecuteScenario(scenario Scenario, numNodes, numTwins uint8, numTicks int, consensusName string, opts ...core.RuntimeOption) (result ScenarioResult, err error) { // Network simulator that blocks proposals, votes, and fetch requests between nodes that are in different partitions. // Timeout and NewView messages are permitted. network := NewPartitionedNetwork(scenario, @@ -60,7 +62,7 @@ func ExecuteScenario(scenario Scenario, numNodes, numTwins uint8, numTicks int, nodes, twins := assignNodeIDs(numNodes, numTwins) nodes = append(nodes, twins...) - err = network.createNodesAndTwins(nodes, consensusName) + err = network.createNodesAndTwins(nodes, consensusName, opts...) if err != nil { return ScenarioResult{}, err } From ae6e497c74a206556efdcafba926211e83876fef Mon Sep 17 00:00:00 2001 From: Hein Meling Date: Sun, 16 Nov 2025 01:56:38 +0100 Subject: [PATCH 6/6] refactor: timeout ruler initialization in synchronizer_test.go We now have a global NewTimeoutRuler() constructor function that encapsulate all TimeoutRulers. This replaces existing calls to NewSimple and NewAggregate and unexports these. --- protocol/synchronizer/synchronizer_test.go | 18 ++++++++---------- protocol/synchronizer/timeoutrule_aggregate.go | 4 ++-- protocol/synchronizer/timeoutrule_simple.go | 4 ++-- protocol/synchronizer/timeoutrules.go | 4 ++-- server/server_test.go | 6 +----- twins/fhsbug_test.go | 2 +- 6 files changed, 16 insertions(+), 22 deletions(-) diff --git a/protocol/synchronizer/synchronizer_test.go b/protocol/synchronizer/synchronizer_test.go index a8b6516d..506df213 100644 --- a/protocol/synchronizer/synchronizer_test.go +++ b/protocol/synchronizer/synchronizer_test.go @@ -6,6 +6,7 @@ import ( "cuelang.org/go/pkg/time" "github.com/relab/hotstuff" + "github.com/relab/hotstuff/core" "github.com/relab/hotstuff/internal/proto/clientpb" "github.com/relab/hotstuff/internal/testutil" "github.com/relab/hotstuff/protocol" @@ -63,7 +64,7 @@ func wireUpSynchronizer( essentials.Authority(), leaderRotation, NewFixedDuration(1000*time.Nanosecond), - NewSimple(essentials.RuntimeCfg(), essentials.Authority()), + NewTimeoutRuler(essentials.RuntimeCfg(), essentials.Authority()), depsConsensus.Proposer(), depsConsensus.Voter(), viewStates, @@ -221,15 +222,12 @@ func TestAdvanceView(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - set, viewStates, synchronizer, block := prepareSynchronizer(t) - signers := set.Signers() - essentials := set[0] - // this is a workaround since we want to test both timeout rules in the same test + var opts []core.RuntimeOption if tt.tr == A { - synchronizer.timeoutRules = NewAggregate(essentials.RuntimeCfg(), essentials.Authority()) - } else { - synchronizer.timeoutRules = NewSimple(essentials.RuntimeCfg(), essentials.Authority()) + opts = append(opts, core.WithAggregateQC()) } + set, viewStates, synchronizer, block := prepareSynchronizer(t, opts...) + signers := set.Signers() syncInfo := hotstuff.NewSyncInfo() if tt.qc { @@ -256,8 +254,8 @@ func TestAdvanceView(t *testing.T) { } } -func prepareSynchronizer(t *testing.T) (testutil.EssentialsSet, *protocol.ViewStates, *Synchronizer, *hotstuff.Block) { - set := testutil.NewEssentialsSet(t, 4, crypto.NameECDSA) +func prepareSynchronizer(t *testing.T, opts ...core.RuntimeOption) (testutil.EssentialsSet, *protocol.ViewStates, *Synchronizer, *hotstuff.Block) { + set := testutil.NewEssentialsSet(t, 4, crypto.NameECDSA, opts...) subject := set[0] viewStates, err := protocol.NewViewStates( subject.Blockchain(), diff --git a/protocol/synchronizer/timeoutrule_aggregate.go b/protocol/synchronizer/timeoutrule_aggregate.go index 3d333da1..9e9b1270 100644 --- a/protocol/synchronizer/timeoutrule_aggregate.go +++ b/protocol/synchronizer/timeoutrule_aggregate.go @@ -14,8 +14,8 @@ type Aggregate struct { auth *cert.Authority } -// NewAggregate returns an aggregate timeout rule instance. -func NewAggregate( +// newAggregate returns an aggregate timeout rule instance. +func newAggregate( config *core.RuntimeConfig, auth *cert.Authority, ) *Aggregate { diff --git a/protocol/synchronizer/timeoutrule_simple.go b/protocol/synchronizer/timeoutrule_simple.go index fcc6cb35..01667c51 100644 --- a/protocol/synchronizer/timeoutrule_simple.go +++ b/protocol/synchronizer/timeoutrule_simple.go @@ -14,8 +14,8 @@ type Simple struct { auth *cert.Authority } -// NewSimple returns a simple timeout rule instance. -func NewSimple( +// newSimple returns a simple timeout rule instance. +func newSimple( config *core.RuntimeConfig, auth *cert.Authority, ) *Simple { diff --git a/protocol/synchronizer/timeoutrules.go b/protocol/synchronizer/timeoutrules.go index 3c953e30..c8fa7854 100644 --- a/protocol/synchronizer/timeoutrules.go +++ b/protocol/synchronizer/timeoutrules.go @@ -9,9 +9,9 @@ import ( // NewTimeoutRuler returns a TimeoutRuler based on the configuration. func NewTimeoutRuler(cfg *core.RuntimeConfig, auth *cert.Authority) TimeoutRuler { if cfg.HasAggregateQC() { - return NewAggregate(cfg, auth) + return newAggregate(cfg, auth) } - return NewSimple(cfg, auth) + return newSimple(cfg, auth) } type TimeoutRuler interface { diff --git a/server/server_test.go b/server/server_test.go index c2249053..20b40026 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -310,10 +310,6 @@ func createServers(t *testing.T, td testData) []replicaDeps { sender, ), ) - timeoutRuler := synchronizer.NewSimple( - depsCore.RuntimeCfg(), - depsSecurity.Authority(), - ) synchronizer := synchronizer.New( depsCore.EventLoop(), depsCore.Logger(), @@ -321,7 +317,7 @@ func createServers(t *testing.T, td testData) []replicaDeps { depsSecurity.Authority(), leaderRotation, synchronizer.NewFixedDuration(100*time.Millisecond), - timeoutRuler, + synchronizer.NewTimeoutRuler(depsCore.RuntimeCfg(), depsSecurity.Authority()), depsConsensus.Proposer(), depsConsensus.Voter(), states, diff --git a/twins/fhsbug_test.go b/twins/fhsbug_test.go index 9c43cf42..6e21d663 100644 --- a/twins/fhsbug_test.go +++ b/twins/fhsbug_test.go @@ -87,7 +87,7 @@ var ( ) func TestFHSBug(t *testing.T) { - // t.Skip("This test is not working as expected; skipping until we have fixed the issue.") + t.Skip("This test is not working as expected; skipping until we have fixed the issue.") logging.SetLogLevel(*logLevel) src, err := twins.FromJSON(strings.NewReader(fhsBugScenario))