From 2be4e3e13c03ee073ea3ddc44141e4646bf44fa9 Mon Sep 17 00:00:00 2001 From: Dan Laine Date: Wed, 27 Sep 2023 14:03:16 -0400 Subject: [PATCH] Remove `snowball.Initialize` and `snowball.Factory` (#2104) --- snow/consensus/snowball/consensus.go | 3 -- .../snowball/consensus_performance_test.go | 4 +- .../snowball/consensus_reversibility_test.go | 6 +-- snow/consensus/snowball/consensus_test.go | 10 ++-- snow/consensus/snowball/factory.go | 9 ---- snow/consensus/snowball/flat.go | 21 +++----- snow/consensus/snowball/flat_test.go | 3 +- snow/consensus/snowball/network_test.go | 42 +++++++++------ snow/consensus/snowball/tree.go | 33 +++++------- snow/consensus/snowball/tree_test.go | 53 +++++++------------ snow/consensus/snowman/snowman_block.go | 3 +- 11 files changed, 79 insertions(+), 108 deletions(-) delete mode 100644 snow/consensus/snowball/factory.go diff --git a/snow/consensus/snowball/consensus.go b/snow/consensus/snowball/consensus.go index 1eb54694633a..7d532fac4dad 100644 --- a/snow/consensus/snowball/consensus.go +++ b/snow/consensus/snowball/consensus.go @@ -15,9 +15,6 @@ import ( type Consensus interface { fmt.Stringer - // Takes in alpha, beta1, beta2, and the initial choice - Initialize(params Parameters, initialPreference ids.ID) - // Adds a new choice to vote on Add(newChoice ids.ID) diff --git a/snow/consensus/snowball/consensus_performance_test.go b/snow/consensus/snowball/consensus_performance_test.go index 59ae6de287f8..b4aa2c53722d 100644 --- a/snow/consensus/snowball/consensus_performance_test.go +++ b/snow/consensus/snowball/consensus_performance_test.go @@ -28,12 +28,12 @@ func TestSnowballOptimized(t *testing.T) { sampler.Seed(seed) for i := 0; i < numNodes; i++ { - nBitwise.AddNode(&Tree{}) + nBitwise.AddNode(NewTree) } sampler.Seed(seed) for i := 0; i < numNodes; i++ { - nNaive.AddNode(&Flat{}) + nNaive.AddNode(NewFlat) } numRounds := 0 diff --git a/snow/consensus/snowball/consensus_reversibility_test.go b/snow/consensus/snowball/consensus_reversibility_test.go index 96c4315c5d9a..d3549edc839a 100644 --- a/snow/consensus/snowball/consensus_reversibility_test.go +++ b/snow/consensus/snowball/consensus_reversibility_test.go @@ -28,7 +28,7 @@ func TestSnowballGovernance(t *testing.T) { sampler.Seed(seed) for i := 0; i < numRed; i++ { - nBitwise.AddNodeSpecificColor(&Tree{}, 0, []int{1}) + nBitwise.AddNodeSpecificColor(NewTree, 0, []int{1}) } for _, node := range nBitwise.nodes { @@ -36,11 +36,11 @@ func TestSnowballGovernance(t *testing.T) { } for i := 0; i < numNodes-numByzantine-numRed; i++ { - nBitwise.AddNodeSpecificColor(&Tree{}, 1, []int{0}) + nBitwise.AddNodeSpecificColor(NewTree, 1, []int{0}) } for i := 0; i < numByzantine; i++ { - nBitwise.AddNodeSpecificColor(&Byzantine{}, 1, []int{0}) + nBitwise.AddNodeSpecificColor(NewByzantine, 1, []int{0}) } for !nBitwise.Finalized() { diff --git a/snow/consensus/snowball/consensus_test.go b/snow/consensus/snowball/consensus_test.go index 0944c68d555d..484c5c9fbc87 100644 --- a/snow/consensus/snowball/consensus_test.go +++ b/snow/consensus/snowball/consensus_test.go @@ -16,16 +16,18 @@ var ( _ Consensus = (*Byzantine)(nil) ) +func NewByzantine(_ Parameters, choice ids.ID) Consensus { + return &Byzantine{ + preference: choice, + } +} + // Byzantine is a naive implementation of a multi-choice snowball instance type Byzantine struct { // Hardcode the preference preference ids.ID } -func (b *Byzantine) Initialize(_ Parameters, choice ids.ID) { - b.preference = choice -} - func (*Byzantine) Add(ids.ID) {} func (b *Byzantine) Preference() ids.ID { diff --git a/snow/consensus/snowball/factory.go b/snow/consensus/snowball/factory.go deleted file mode 100644 index 716f8f6ad3c7..000000000000 --- a/snow/consensus/snowball/factory.go +++ /dev/null @@ -1,9 +0,0 @@ -// Copyright (C) 2019-2023, Ava Labs, Inc. All rights reserved. -// See the file LICENSE for licensing terms. - -package snowball - -// Factory returns new instances of Consensus -type Factory interface { - New() Consensus -} diff --git a/snow/consensus/snowball/flat.go b/snow/consensus/snowball/flat.go index cf08c9bd6196..5ae0aa69692d 100644 --- a/snow/consensus/snowball/flat.go +++ b/snow/consensus/snowball/flat.go @@ -8,16 +8,14 @@ import ( "github.com/ava-labs/avalanchego/utils/bag" ) -var ( - _ Factory = (*FlatFactory)(nil) - _ Consensus = (*Flat)(nil) -) - -// FlatFactory implements Factory by returning a flat struct -type FlatFactory struct{} +var _ Consensus = (*Flat)(nil) -func (FlatFactory) New() Consensus { - return &Flat{} +func NewFlat(params Parameters, choice ids.ID) Consensus { + f := &Flat{ + params: params, + } + f.nnarySnowball.Initialize(params.BetaVirtuous, params.BetaRogue, choice) + return f } // Flat is a naive implementation of a multi-choice snowball instance @@ -29,11 +27,6 @@ type Flat struct { params Parameters } -func (f *Flat) Initialize(params Parameters, choice ids.ID) { - f.nnarySnowball.Initialize(params.BetaVirtuous, params.BetaRogue, choice) - f.params = params -} - func (f *Flat) RecordPoll(votes bag.Bag[ids.ID]) bool { if pollMode, numVotes := votes.Mode(); numVotes >= f.params.Alpha { f.RecordSuccessfulPoll(pollMode) diff --git a/snow/consensus/snowball/flat_test.go b/snow/consensus/snowball/flat_test.go index 0291c03be1aa..22665e610785 100644 --- a/snow/consensus/snowball/flat_test.go +++ b/snow/consensus/snowball/flat_test.go @@ -17,8 +17,7 @@ func TestFlat(t *testing.T) { params := Parameters{ K: 2, Alpha: 2, BetaVirtuous: 1, BetaRogue: 2, } - f := Flat{} - f.Initialize(params, Red) + f := NewFlat(params, Red) f.Add(Green) f.Add(Blue) diff --git a/snow/consensus/snowball/network_test.go b/snow/consensus/snowball/network_test.go index 35b91eaafe3e..711bbf89010e 100644 --- a/snow/consensus/snowball/network_test.go +++ b/snow/consensus/snowball/network_test.go @@ -11,6 +11,8 @@ import ( "github.com/ava-labs/avalanchego/utils/sampler" ) +type newConsensusFunc func(params Parameters, choice ids.ID) Consensus + type Network struct { params Parameters colors []ids.ID @@ -26,34 +28,44 @@ func (n *Network) Initialize(params Parameters, numColors int) { } } -func (n *Network) AddNode(sb Consensus) { +func (n *Network) AddNode(newConsensusFunc newConsensusFunc) Consensus { s := sampler.NewUniform() s.Initialize(uint64(len(n.colors))) indices, _ := s.Sample(len(n.colors)) - sb.Initialize(n.params, n.colors[int(indices[0])]) + + consensus := newConsensusFunc(n.params, n.colors[int(indices[0])]) for _, index := range indices[1:] { - sb.Add(n.colors[int(index)]) + consensus.Add(n.colors[int(index)]) } - n.nodes = append(n.nodes, sb) - if !sb.Finalized() { - n.running = append(n.running, sb) + n.nodes = append(n.nodes, consensus) + if !consensus.Finalized() { + n.running = append(n.running, consensus) } + + return consensus } -// AddNodeSpecificColor adds [sb] to the network which will initially prefer -// [initialPreference] and additionally adds each of the specified [options] to -// consensus. -func (n *Network) AddNodeSpecificColor(sb Consensus, initialPreference int, options []int) { - sb.Initialize(n.params, n.colors[initialPreference]) +// AddNodeSpecificColor adds a new consensus instance to the network which will +// initially prefer [initialPreference] and additionally adds each of the +// specified [options] to consensus. +func (n *Network) AddNodeSpecificColor( + newConsensusFunc newConsensusFunc, + initialPreference int, + options []int, +) Consensus { + consensus := newConsensusFunc(n.params, n.colors[initialPreference]) + for _, i := range options { - sb.Add(n.colors[i]) + consensus.Add(n.colors[i]) } - n.nodes = append(n.nodes, sb) - if !sb.Finalized() { - n.running = append(n.running, sb) + n.nodes = append(n.nodes, consensus) + if !consensus.Finalized() { + n.running = append(n.running, consensus) } + + return consensus } // Finalized returns true iff every node added to the network has finished diff --git a/snow/consensus/snowball/tree.go b/snow/consensus/snowball/tree.go index 367573c76095..a0e8668c9e5c 100644 --- a/snow/consensus/snowball/tree.go +++ b/snow/consensus/snowball/tree.go @@ -12,17 +12,26 @@ import ( ) var ( - _ Factory = (*TreeFactory)(nil) _ Consensus = (*Tree)(nil) _ node = (*unaryNode)(nil) _ node = (*binaryNode)(nil) ) -// TreeFactory implements Factory by returning a tree struct -type TreeFactory struct{} +func NewTree(params Parameters, choice ids.ID) Consensus { + snowball := &unarySnowball{} + snowball.Initialize(params.BetaVirtuous) -func (TreeFactory) New() Consensus { - return &Tree{} + t := &Tree{ + params: params, + } + t.node = &unaryNode{ + tree: t, + preference: choice, + commonPrefix: ids.NumBits, // The initial state has no conflicts + snowball: snowball, + } + + return t } // Tree implements the snowball interface by using a modified patricia tree. @@ -46,20 +55,6 @@ type Tree struct { shouldReset bool } -func (t *Tree) Initialize(params Parameters, choice ids.ID) { - t.params = params - - snowball := &unarySnowball{} - snowball.Initialize(params.BetaVirtuous) - - t.node = &unaryNode{ - tree: t, - preference: choice, - commonPrefix: ids.NumBits, // The initial state has no conflicts - snowball: snowball, - } -} - func (t *Tree) Add(choice ids.ID) { prefix := t.node.DecidedPrefix() // Make sure that we haven't already decided against this new id diff --git a/snow/consensus/snowball/tree_test.go b/snow/consensus/snowball/tree_test.go index 26406e428023..1879b65e64bf 100644 --- a/snow/consensus/snowball/tree_test.go +++ b/snow/consensus/snowball/tree_test.go @@ -24,8 +24,7 @@ func TestSnowballSingleton(t *testing.T) { params := Parameters{ K: 1, Alpha: 1, BetaVirtuous: 2, BetaRogue: 5, } - tree := Tree{} - tree.Initialize(params, Red) + tree := NewTree(params, Red) require.False(tree.Finalized()) @@ -62,8 +61,7 @@ func TestSnowballRecordUnsuccessfulPoll(t *testing.T) { params := Parameters{ K: 1, Alpha: 1, BetaVirtuous: 3, BetaRogue: 5, } - tree := Tree{} - tree.Initialize(params, Red) + tree := NewTree(params, Red) require.False(tree.Finalized()) @@ -89,8 +87,7 @@ func TestSnowballBinary(t *testing.T) { params := Parameters{ K: 1, Alpha: 1, BetaVirtuous: 1, BetaRogue: 2, } - tree := Tree{} - tree.Initialize(params, Red) + tree := NewTree(params, Red) tree.Add(Blue) require.Equal(Red, tree.Preference()) @@ -129,8 +126,7 @@ func TestSnowballLastBinary(t *testing.T) { params := Parameters{ K: 1, Alpha: 1, BetaVirtuous: 2, BetaRogue: 2, } - tree := Tree{} - tree.Initialize(params, zero) + tree := NewTree(params, zero) tree.Add(one) // Should do nothing @@ -170,8 +166,7 @@ func TestSnowballAddPreviouslyRejected(t *testing.T) { params := Parameters{ K: 1, Alpha: 1, BetaVirtuous: 1, BetaRogue: 2, } - tree := Tree{} - tree.Initialize(params, zero) + tree := NewTree(params, zero) tree.Add(one) tree.Add(four) @@ -224,8 +219,7 @@ func TestSnowballNewUnary(t *testing.T) { params := Parameters{ K: 1, Alpha: 1, BetaVirtuous: 2, BetaRogue: 3, } - tree := Tree{} - tree.Initialize(params, zero) + tree := NewTree(params, zero) tree.Add(one) { @@ -271,8 +265,7 @@ func TestSnowballTransitiveReset(t *testing.T) { params := Parameters{ K: 1, Alpha: 1, BetaVirtuous: 2, BetaRogue: 2, } - tree := Tree{} - tree.Initialize(params, zero) + tree := NewTree(params, zero) tree.Add(two) tree.Add(eight) @@ -352,8 +345,7 @@ func TestSnowballTrinary(t *testing.T) { params := Parameters{ K: 1, Alpha: 1, BetaVirtuous: 1, BetaRogue: 2, } - tree := Tree{} - tree.Initialize(params, Green) + tree := NewTree(params, Green) tree.Add(Red) tree.Add(Blue) @@ -403,8 +395,7 @@ func TestSnowballCloseTrinary(t *testing.T) { params := Parameters{ K: 1, Alpha: 1, BetaVirtuous: 1, BetaRogue: 2, } - tree := Tree{} - tree.Initialize(params, yellow) + tree := NewTree(params, yellow) tree.Add(cyan) tree.Add(magenta) @@ -449,8 +440,7 @@ func TestSnowballAddRejected(t *testing.T) { params := Parameters{ K: 1, Alpha: 1, BetaVirtuous: 1, BetaRogue: 2, } - tree := Tree{} - tree.Initialize(params, c0000) + tree := NewTree(params, c0000) tree.Add(c1000) tree.Add(c0010) @@ -495,8 +485,7 @@ func TestSnowballResetChild(t *testing.T) { params := Parameters{ K: 1, Alpha: 1, BetaVirtuous: 1, BetaRogue: 2, } - tree := Tree{} - tree.Initialize(params, c0000) + tree := NewTree(params, c0000) tree.Add(c0100) tree.Add(c1000) @@ -555,8 +544,7 @@ func TestSnowballResetSibling(t *testing.T) { params := Parameters{ K: 1, Alpha: 1, BetaVirtuous: 1, BetaRogue: 2, } - tree := Tree{} - tree.Initialize(params, c0000) + tree := NewTree(params, c0000) tree.Add(c0100) tree.Add(c1000) @@ -618,16 +606,14 @@ func TestSnowball5Colors(t *testing.T) { colors = append(colors, ids.Empty.Prefix(uint64(i))) } - tree0 := Tree{} - tree0.Initialize(params, colors[4]) + tree0 := NewTree(params, colors[4]) tree0.Add(colors[0]) tree0.Add(colors[1]) tree0.Add(colors[2]) tree0.Add(colors[3]) - tree1 := Tree{} - tree1.Initialize(params, colors[3]) + tree1 := NewTree(params, colors[3]) tree1.Add(colors[0]) tree1.Add(colors[1]) @@ -650,8 +636,7 @@ func TestSnowballFineGrained(t *testing.T) { params := Parameters{ K: 1, Alpha: 1, BetaVirtuous: 1, BetaRogue: 2, } - tree := Tree{} - tree.Initialize(params, c0000) + tree := NewTree(params, c0000) require.Equal(initialUnaryDescription, tree.String()) require.Equal(c0000, tree.Preference()) @@ -741,8 +726,7 @@ func TestSnowballDoubleAdd(t *testing.T) { params := Parameters{ K: 1, Alpha: 1, BetaVirtuous: 3, BetaRogue: 5, } - tree := Tree{} - tree.Initialize(params, Red) + tree := NewTree(params, Red) tree.Add(Red) require.Equal(initialUnaryDescription, tree.String()) @@ -766,7 +750,7 @@ func TestSnowballConsistent(t *testing.T) { n.Initialize(params, numColors) for i := 0; i < numNodes; i++ { - n.AddNode(&Tree{}) + n.AddNode(NewTree) } for !n.Finalized() && !n.Disagreement() { @@ -787,8 +771,7 @@ func TestSnowballFilterBinaryChildren(t *testing.T) { params := Parameters{ K: 1, Alpha: 1, BetaVirtuous: 1, BetaRogue: 2, } - tree := Tree{} - tree.Initialize(params, c0000) + tree := NewTree(params, c0000) require.Equal(initialUnaryDescription, tree.String()) require.Equal(c0000, tree.Preference()) diff --git a/snow/consensus/snowman/snowman_block.go b/snow/consensus/snowman/snowman_block.go index ddb3ae3045f8..782c77d8e415 100644 --- a/snow/consensus/snowman/snowman_block.go +++ b/snow/consensus/snowman/snowman_block.go @@ -38,8 +38,7 @@ func (n *snowmanBlock) AddChild(child Block) { // if the snowball instance is nil, this is the first child. So the instance // should be initialized. if n.sb == nil { - n.sb = &snowball.Tree{} - n.sb.Initialize(n.params, childID) + n.sb = snowball.NewTree(n.params, childID) n.children = make(map[ids.ID]Block) } else { n.sb.Add(childID)