From 40510a70e92d075ab77b2a13b87d1d53743f684b Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Thu, 13 May 2021 23:02:11 -0500 Subject: [PATCH 1/5] make squares have widths that are a power of two --- types/block.go | 35 ++++++++++++++++++++------ types/block_test.go | 60 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 7 deletions(-) diff --git a/types/block.go b/types/block.go index a77a6e58f4..8da309340c 100644 --- a/types/block.go +++ b/types/block.go @@ -1381,9 +1381,8 @@ func (data *Data) ComputeShares() (NamespacedShares, int) { msgShares := data.Messages.splitIntoShares() curLen := len(txShares) + len(intermRootsShares) + len(evidenceShares) + len(msgShares) - // FIXME(ismail): this is not a power of two - // see: https://github.com/lazyledger/lazyledger-specs/issues/80 and - wantLen := getNextSquareNum(curLen) + // find the + wantLen := paddedLen(curLen) // ensure that the min square size is used if wantLen < minSharecount { @@ -1400,10 +1399,32 @@ func (data *Data) ComputeShares() (NamespacedShares, int) { tailShares...), curLen } -func getNextSquareNum(length int) int { - width := int(math.Ceil(math.Sqrt(float64(length)))) - // TODO(ismail): make width a power of two instead - return width * width +// paddedLen calculates the number of shares needed to make a power of 2 square +// given the current number of shares +func paddedLen(length int) int { + width := uint32(math.Ceil(math.Sqrt(float64(length)))) + width = nextPowerOf2(width) + return int(width * width) +} + +// nextPowerOf2 returns the next lowest power of 2 unless the input is a power +// of two, in which case it returns the input +func nextPowerOf2(v uint32) uint32 { + if v == 1 { + return 1 + } + + // find the next highest power using bit mashing + v-- + v |= v >> 1 + v |= v >> 2 + v |= v >> 4 + v |= v >> 8 + v |= v >> 16 + v++ + + // return the next lowest power + return v } type Message struct { diff --git a/types/block_test.go b/types/block_test.go index 7cc9432426..c56c863739 100644 --- a/types/block_test.go +++ b/types/block_test.go @@ -1398,6 +1398,66 @@ func TestPutBlock(t *testing.T) { } } +func TestPaddedLength(t *testing.T) { + type test struct { + input, expected int + } + tests := []test{ + {0, 0}, + {1, 1}, + {2, 4}, + {4, 4}, + {5, 16}, + {11, 16}, + {128, 256}, + } + for _, tt := range tests { + res := paddedLen(tt.input) + assert.Equal(t, tt.expected, res) + } +} + +func TestNextPowerOf2(t *testing.T) { + type test struct { + input uint32 + expected uint32 + } + tests := []test{ + { + input: 2, + expected: 2, + }, + { + input: 11, + expected: 16, + }, + { + input: 511, + expected: 512, + }, + { + input: 1, + expected: 1, + }, + { + input: 0, + expected: 0, + }, + { + input: 5, + expected: 8, + }, + { + input: 6, + expected: 8, + }, + } + for _, tt := range tests { + res := nextPowerOf2(tt.input) + assert.Equal(t, tt.expected, res) + } +} + func generateRandomMsgOnlyData(msgCount int) Data { out := make([]Message, msgCount) for i, msg := range generateRandNamespacedRawData(msgCount, NamespaceSize, MsgShareSize-2) { From e5a7683c44bbf92d759b127226c0652c7b9018e5 Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Thu, 13 May 2021 23:16:36 -0500 Subject: [PATCH 2/5] finish docs --- types/block.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/types/block.go b/types/block.go index 8da309340c..4ca5f4f42c 100644 --- a/types/block.go +++ b/types/block.go @@ -1381,7 +1381,8 @@ func (data *Data) ComputeShares() (NamespacedShares, int) { msgShares := data.Messages.splitIntoShares() curLen := len(txShares) + len(intermRootsShares) + len(evidenceShares) + len(msgShares) - // find the + // find the number of shares needed to create a square that has a power of + // two wantLen := paddedLen(curLen) // ensure that the min square size is used From 4fd8936fef8b63437f8c73d2d760fcb5f0548966 Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Fri, 14 May 2021 10:35:02 -0500 Subject: [PATCH 3/5] switch ipld nextPowerOfTwo to isPowerOf2 --- p2p/ipld/read.go | 33 +++++---------------------------- p2p/ipld/read_test.go | 25 +++++++++++++++---------- 2 files changed, 20 insertions(+), 38 deletions(-) diff --git a/p2p/ipld/read.go b/p2p/ipld/read.go index e08136499a..d08ebdcc64 100644 --- a/p2p/ipld/read.go +++ b/p2p/ipld/read.go @@ -2,7 +2,6 @@ package ipld import ( "context" - "errors" "fmt" "math" "math/rand" @@ -246,8 +245,8 @@ func GetLeafData( func leafPath(index, total uint32) ([]string, error) { // ensure that the total is a power of two - if total != nextPowerOf2(total) { - return nil, errors.New("expected total to be a power of 2") + if isPowerOf2(total) { + return nil, fmt.Errorf("expected total to be a power of 2, got %d", total) } if total == 0 { @@ -269,29 +268,7 @@ func leafPath(index, total uint32) ([]string, error) { return path, nil } -// nextPowerOf2 returns the next lowest power of 2 unless the input is a power -// of two, in which case it returns the input -func nextPowerOf2(v uint32) uint32 { - if v == 1 { - return 1 - } - // keep track of the input - i := v - - // find the next highest power using bit mashing - v-- - v |= v >> 1 - v |= v >> 2 - v |= v >> 4 - v |= v >> 8 - v |= v >> 16 - v++ - - // check if the input was the next highest power - if i == v { - return v - } - - // return the next lowest power - return v / 2 +// isPowerOf2 returns checks if a given number is a power of two +func isPowerOf2(v uint32) bool { + return math.Ceil(math.Log2(float64(v))) == math.Floor(math.Log2(float64(v))) } diff --git a/p2p/ipld/read_test.go b/p2p/ipld/read_test.go index 715597298f..aca4b46780 100644 --- a/p2p/ipld/read_test.go +++ b/p2p/ipld/read_test.go @@ -58,36 +58,41 @@ func TestLeafPath(t *testing.T) { } } -func TestNextPowerOf2(t *testing.T) { +func Test_isPowerOf2(t *testing.T) { type test struct { input uint32 - expected uint32 + expected bool } tests := []test{ { input: 2, - expected: 2, + expected: true, }, { input: 11, - expected: 8, + expected: false, }, { input: 511, - expected: 256, + expected: false, + }, + + { + input: 0, + expected: true, }, { input: 1, - expected: 1, + expected: true, }, { - input: 0, - expected: 0, + input: 16, + expected: true, }, } for _, tt := range tests { - res := nextPowerOf2(tt.input) - assert.Equal(t, tt.expected, res) + res := isPowerOf2(tt.input) + assert.Equal(t, tt.expected, res, fmt.Sprintf("input was %d", tt.input)) } } From 4aa3372fab1efb976da22e362067b77eeacab138 Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Fri, 14 May 2021 10:35:43 -0500 Subject: [PATCH 4/5] better naming and clearer description of zero input --- types/block.go | 14 +++++++------- types/block_test.go | 8 ++++++-- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/types/block.go b/types/block.go index 4ca5f4f42c..481fdc9105 100644 --- a/types/block.go +++ b/types/block.go @@ -1382,7 +1382,7 @@ func (data *Data) ComputeShares() (NamespacedShares, int) { curLen := len(txShares) + len(intermRootsShares) + len(evidenceShares) + len(msgShares) // find the number of shares needed to create a square that has a power of - // two + // two width wantLen := paddedLen(curLen) // ensure that the min square size is used @@ -1404,15 +1404,15 @@ func (data *Data) ComputeShares() (NamespacedShares, int) { // given the current number of shares func paddedLen(length int) int { width := uint32(math.Ceil(math.Sqrt(float64(length)))) - width = nextPowerOf2(width) + width = nextHighestPowerOf2(width) return int(width * width) } -// nextPowerOf2 returns the next lowest power of 2 unless the input is a power +// nextPowerOf2 returns the next highest power of 2 unless the input is a power // of two, in which case it returns the input -func nextPowerOf2(v uint32) uint32 { - if v == 1 { - return 1 +func nextHighestPowerOf2(v uint32) uint32 { + if v == 0 { + return 0 } // find the next highest power using bit mashing @@ -1424,7 +1424,7 @@ func nextPowerOf2(v uint32) uint32 { v |= v >> 16 v++ - // return the next lowest power + // return the next highest power return v } diff --git a/types/block_test.go b/types/block_test.go index c56c863739..667e1c303a 100644 --- a/types/block_test.go +++ b/types/block_test.go @@ -1417,7 +1417,7 @@ func TestPaddedLength(t *testing.T) { } } -func TestNextPowerOf2(t *testing.T) { +func TestNextHighestPowerOf2(t *testing.T) { type test struct { input uint32 expected uint32 @@ -1451,9 +1451,13 @@ func TestNextPowerOf2(t *testing.T) { input: 6, expected: 8, }, + { + input: 16, + expected: 16, + }, } for _, tt := range tests { - res := nextPowerOf2(tt.input) + res := nextHighestPowerOf2(tt.input) assert.Equal(t, tt.expected, res) } } From 98c17919ed2dc86193d626d6ed59df87f859d6bb Mon Sep 17 00:00:00 2001 From: evan-forbes Date: Fri, 14 May 2021 11:16:52 -0500 Subject: [PATCH 5/5] :eyes: stop breaking everything by using correct boolean logic --- p2p/ipld/read.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/p2p/ipld/read.go b/p2p/ipld/read.go index d08ebdcc64..d58255c76b 100644 --- a/p2p/ipld/read.go +++ b/p2p/ipld/read.go @@ -245,7 +245,7 @@ func GetLeafData( func leafPath(index, total uint32) ([]string, error) { // ensure that the total is a power of two - if isPowerOf2(total) { + if !isPowerOf2(total) { return nil, fmt.Errorf("expected total to be a power of 2, got %d", total) }