Skip to content

Commit

Permalink
Fix TestSnowballFilterBinaryChildren + add rough graphs
Browse files Browse the repository at this point in the history
  • Loading branch information
@aaronbuchwald
aaronbuchwald committed Apr 17, 2024
1 parent 0c4c318 commit 565c5b1
Show file tree
Hide file tree
Showing 2 changed files with 139 additions and 94 deletions.
168 changes: 84 additions & 84 deletions snow/consensus/snowball/tree.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,6 @@ func (u *unaryNode) DecidedPrefix() int {
return u.decidedPrefix
}

//nolint:gci,gofmt,gofumpt // this comment is formatted as intended
//
// This is by far the most complicated function in this algorithm.
// The intuition is that this instance represents a series of consecutive unary
// snowball instances, and this function's purpose is convert one of these unary
Expand All @@ -188,23 +186,23 @@ func (u *unaryNode) DecidedPrefix() int {
//
// For example, attempting to insert the value "00001" in this node:
//
// +-------------------+ <-- This node will not be split
// | |
// | 0 0 0 |
// | |
// +-------------------+ <-- Pass the add to the child
// ^
// |
// +-------------------+ <-- This node will not be split
// | |
// | 0 0 0 |
// | |
// +-------------------+ <-- Pass the add to the child
// ^
// |
//
// Results in:
//
// +-------------------+
// | |
// | 0 0 0 |
// | |
// +-------------------+ <-- With the modified child
// ^
// |
// +-------------------+
// | |
// | 0 0 0 |
// | |
// +-------------------+ <-- With the modified child
// ^
// |
//
// 2. This instance represents a series of only one unary instance and it must
// be split.
Expand All @@ -215,19 +213,19 @@ func (u *unaryNode) DecidedPrefix() int {
//
// For example, attempting to insert the value "1" in this tree:
//
// +-------------------+
// | |
// | 0 |
// | |
// +-------------------+
// +-------------------+
// | |
// | 0 |
// | |
// +-------------------+
//
// Results in:
//
// +-------------------+
// | | |
// | 0 | 1 |
// | | |
// +-------------------+
// +-------------------+
// | | |
// | 0 | 1 |
// | | |
// +-------------------+
//
// 3. This instance must be split on the first bit
//
Expand All @@ -237,26 +235,26 @@ func (u *unaryNode) DecidedPrefix() int {
//
// For example, attempting to insert the value "10" in this tree:
//
// +-------------------+
// | |
// | 0 0 |
// | |
// +-------------------+
// +-------------------+
// | |
// | 0 0 |
// | |
// +-------------------+
//
// Results in:
//
// +-------------------+
// | | |
// | 0 | 1 |
// | | |
// +-------------------+
// ^ ^
// / \
// +-------------------+ +-------------------+
// | | | |
// | 0 | | 0 |
// | | | |
// +-------------------+ +-------------------+
// +-------------------+
// | | |
// | 0 | 1 |
// | | |
// +-------------------+
// ^ ^
// / \
// +-------------------+ +-------------------+
// | | | |
// | 0 | | 0 |
// | | | |
// +-------------------+ +-------------------+
//
// 4. This instance must be split on the last bit
//
Expand All @@ -267,26 +265,26 @@ func (u *unaryNode) DecidedPrefix() int {
//
// For example, attempting to insert the value "01" in this tree:
//
// +-------------------+
// | |
// | 0 0 |
// | |
// +-------------------+
// +-------------------+
// | |
// | 0 0 |
// | |
// +-------------------+
//
// Results in:
//
// +-------------------+
// | |
// | 0 |
// | |
// +-------------------+
// ^
// |
// +-------------------+
// | | |
// | 0 | 1 |
// | | |
// +-------------------+
// +-------------------+
// | |
// | 0 |
// | |
// +-------------------+
// ^
// |
// +-------------------+
// | | |
// | 0 | 1 |
// | | |
// +-------------------+
//
// 5. This instance must be split on an interior bit
//
Expand All @@ -298,33 +296,35 @@ func (u *unaryNode) DecidedPrefix() int {
//
// For example, attempting to insert the value "010" in this tree:
//
// +-------------------+
// | |
// | 0 0 0 |
// | |
// +-------------------+
// +-------------------+
// | |
// | 0 0 0 |
// | |
// +-------------------+
//
// Results in:
//
// +-------------------+
// | |
// | 0 |
// | |
// +-------------------+
// ^
// |
// +-------------------+
// | | |
// | 0 | 1 |
// | | |
// +-------------------+
// ^ ^
// / \
// +-------------------+ +-------------------+
// | | | |
// | 0 | | 0 |
// | | | |
// +-------------------+ +-------------------+
// +-------------------+
// | |
// | 0 |
// | |
// +-------------------+
// ^
// |
// +-------------------+
// | | |
// | 0 | 1 |
// | | |
// +-------------------+
// ^ ^
// / \
// +-------------------+ +-------------------+
// | | | |
// | 0 | | 0 |
// | | | |
// +-------------------+ +-------------------+
//
//nolint:gci,gofmt,gofumpt // this comment is formatted as intended
func (u *unaryNode) Add(newChoice ids.ID) node {
if u.Finalized() {
return u // Only happens if the tree is finalized, or it's a leaf node
Expand Down
65 changes: 55 additions & 10 deletions snow/consensus/snowball/tree_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,6 @@ func TestSnowballConsistent(t *testing.T) {
}

func TestSnowballFilterBinaryChildren(t *testing.T) {
t.Skip()
require := require.New(t)

c0000 := ids.ID{0b00000000}
Expand Down Expand Up @@ -814,11 +813,21 @@ func TestSnowballFilterBinaryChildren(t *testing.T) {
c0000Bag := bag.Of(c0000)
require.True(tree.RecordPoll(c0000Bag))

// *
// / \
// 0 (1) 1 (0) Bit = 0
// | \
// 0 (1) Bits[1, 2) 000 (0) Bits [1, 256)
// / \
// 0 (1) 1 (0) Bit = 2
// | |
// 0 (1) 0 (0) Bits [3, 256)
{
expected := `SB(Preference = 0, PreferenceStrength[0] = 1, PreferenceStrength[1] = 0, SF(Confidence = 1, Finalized = false, SL(Preference = 0))) Bit = 0
SB(Preference = 0, PreferenceStrength[0] = 1, PreferenceStrength[1] = 0, SF(Confidence = 1, Finalized = false, SL(Preference = 0))) Bit = 2
SB(PreferenceStrength = 1, SF(Confidence = 1, Finalized = true)) Bits = [3, 256)
SB(PreferenceStrength = 0, SF(Confidence = 0, Finalized = false)) Bits = [3, 256)
SB(PreferenceStrength = 1, SF(Confidence = 1, Finalized = false)) Bits = [1, 2)
SB(Preference = 0, PreferenceStrength[0] = 1, PreferenceStrength[1] = 0, SF(Confidence = 1, Finalized = false, SL(Preference = 0))) Bit = 2
SB(PreferenceStrength = 1, SF(Confidence = 1, Finalized = false)) Bits = [3, 256)
SB(PreferenceStrength = 0, SF(Confidence = 0, Finalized = false)) Bits = [3, 256)
SB(PreferenceStrength = 0, SF(Confidence = 0, Finalized = false)) Bits = [1, 256)`
require.Equal(expected, tree.String())
require.Equal(c0000, tree.Preference())
Expand All @@ -827,11 +836,29 @@ func TestSnowballFilterBinaryChildren(t *testing.T) {

tree.Add(c0100)

// Options:
// 1000
// 0000
// 0010
// 0100

// Results:
// *
// / \
// 0 (1) 1 (0)
// / \ \
// 0 (1) 1 (0) 000 (0)
// / \ \
// 0 (1) 1 (0) 00 (0)
// | |
// 0 (1) 0 (0)
{
expected := `SB(Preference = 0, PreferenceStrength[0] = 1, PreferenceStrength[1] = 0, SF(Confidence = 1, Finalized = false, SL(Preference = 0))) Bit = 0
SB(Preference = 0, PreferenceStrength[0] = 1, PreferenceStrength[1] = 0, SF(Confidence = 1, Finalized = false, SL(Preference = 0))) Bit = 2
SB(PreferenceStrength = 1, SF(Confidence = 1, Finalized = true)) Bits = [3, 256)
SB(PreferenceStrength = 0, SF(Confidence = 0, Finalized = false)) Bits = [3, 256)
SB(Preference = 0, PreferenceStrength[0] = 1, PreferenceStrength[1] = 0, SF(Confidence = 1, Finalized = false, SL(Preference = 0))) Bit = 1
SB(Preference = 0, PreferenceStrength[0] = 1, PreferenceStrength[1] = 0, SF(Confidence = 1, Finalized = false, SL(Preference = 0))) Bit = 2
SB(PreferenceStrength = 1, SF(Confidence = 1, Finalized = false)) Bits = [3, 256)
SB(PreferenceStrength = 0, SF(Confidence = 0, Finalized = false)) Bits = [3, 256)
SB(PreferenceStrength = 0, SF(Confidence = 0, Finalized = false)) Bits = [2, 256)
SB(PreferenceStrength = 0, SF(Confidence = 0, Finalized = false)) Bits = [1, 256)`
require.Equal(expected, tree.String())
require.Equal(c0000, tree.Preference())
Expand All @@ -841,10 +868,28 @@ func TestSnowballFilterBinaryChildren(t *testing.T) {
c0100Bag := bag.Of(c0100)
require.True(tree.RecordPoll(c0100Bag))

// Options:
// 1000
// 0000
// 0010
// 0100

// Results:
// *
// / \
// 0 (2) 1 (0) (eliminated)
// / \ \
// 0 (1r) 1 (1) 000 (0) (eliminated)
// / \ \
// 0 (1) 1 (0) 00 (1)
// | |
// 0 (1) 0 (0)
{
expected := `SB(Preference = 0, PreferenceStrength[0] = 1, PreferenceStrength[1] = 0, SF(Confidence = 0, Finalized = false, SL(Preference = 0))) Bit = 2
SB(PreferenceStrength = 1, SF(Confidence = 1, Finalized = true)) Bits = [3, 256)
SB(PreferenceStrength = 0, SF(Confidence = 0, Finalized = false)) Bits = [3, 256)`
expected := `SB(Preference = 0, PreferenceStrength[0] = 1, PreferenceStrength[1] = 1, SF(Confidence = 1, Finalized = false, SL(Preference = 1))) Bit = 1
SB(Preference = 0, PreferenceStrength[0] = 1, PreferenceStrength[1] = 0, SF(Confidence = 1, Finalized = false, SL(Preference = 0))) Bit = 2
SB(PreferenceStrength = 1, SF(Confidence = 1, Finalized = false)) Bits = [3, 256)
SB(PreferenceStrength = 0, SF(Confidence = 0, Finalized = false)) Bits = [3, 256)
SB(PreferenceStrength = 1, SF(Confidence = 1, Finalized = false)) Bits = [2, 256)`
require.Equal(expected, tree.String())
require.Equal(c0000, tree.Preference())
require.False(tree.Finalized())
Expand Down

0 comments on commit 565c5b1

Please sign in to comment.