-
Notifications
You must be signed in to change notification settings - Fork 386
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
feat: implement non-interactive defaults logic #680
Changes from 2 commits
61e14e7
e80a116
fb3632b
d09b5ab
619b28e
23d8764
9d46124
91c0d72
6777ea5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
package shares | ||
|
||
// FitsInSquare uses the non interactive default rules to see if messages of | ||
// some lengths will fit in a square of size origSquareSize starting at share | ||
// index cursor. See non-interactive default rules | ||
// https://github.com/celestiaorg/celestia-specs/blob/master/src/rationale/message_block_layout.md#non-interactive-default-rules | ||
func FitsInSquare(cursor, origSquareSize int, msgShareLens ...int) (bool, int) { | ||
// if there are 0 messages and the cursor already fits inside the square, | ||
// then we already know that everything fits in the square. | ||
if len(msgShareLens) == 0 && cursor/origSquareSize <= origSquareSize { | ||
return true, 0 | ||
} | ||
firstMsgLen := 1 | ||
if len(msgShareLens) > 0 { | ||
firstMsgLen = msgShareLens[0] | ||
} | ||
// here we account for padding between the contiguous and message shares | ||
cursor, _ = NextAlignedPowerOfTwo(cursor, firstMsgLen, origSquareSize) | ||
sharesUsed, _ := MsgSharesUsedNIDefaults(cursor, origSquareSize, msgShareLens...) | ||
return cursor+sharesUsed <= origSquareSize*origSquareSize, sharesUsed | ||
} | ||
|
||
// MsgSharesUsedNIDefaults calculates the number of shares used by a given set | ||
// of messages share lengths. It follows the non-interactive default rules and | ||
// assumes that each msg length in msgShareLens | ||
func MsgSharesUsedNIDefaults(cursor, origSquareSize int, msgShareLens ...int) (int, []uint32) { | ||
start := cursor | ||
indexes := make([]uint32, len(msgShareLens)) | ||
for i, msgLen := range msgShareLens { | ||
cursor, _ = NextAlignedPowerOfTwo(cursor, msgLen, origSquareSize) | ||
rach-id marked this conversation as resolved.
Show resolved
Hide resolved
|
||
indexes[i] = uint32(cursor) | ||
cursor += msgLen | ||
} | ||
return cursor - start, indexes | ||
} | ||
|
||
// NextAlignedPowerOfTwo calculates the next index in a row that is an aligned | ||
// power of two and returns false if the entire the msg cannot fit on the given | ||
// row at the next aligned power of two. An aligned power of two means that the | ||
// largest power of two that fits entirely in the msg or the square size. pls | ||
// see specs for further details. Assumes that cursor < k, all args are non | ||
// negative, and that k is a power of two. | ||
// https://github.com/celestiaorg/celestia-specs/blob/master/src/rationale/message_block_layout.md#non-interactive-default-rules | ||
evan-forbes marked this conversation as resolved.
Show resolved
Hide resolved
|
||
func NextAlignedPowerOfTwo(cursor, msgLen, k int) (int, bool) { | ||
// if we're starting at the beginning of the row, then return as there are | ||
// no cases where we don't start at 0. | ||
if cursor == 0 || cursor%k == 0 { | ||
return cursor, true | ||
} | ||
|
||
nextLowest := nextLowestPowerOfTwo(msgLen) | ||
endOfCurrentRow := ((cursor / k) + 1) * k | ||
cursor = roundUpBy(cursor, nextLowest) | ||
switch { | ||
// the entire message fits in this row | ||
case cursor+msgLen <= endOfCurrentRow: | ||
return cursor, true | ||
// only a portion of the message fits in this row | ||
case cursor+nextLowest <= endOfCurrentRow: | ||
return cursor, false | ||
// none of the message fits on this row, so return the start of the next row | ||
default: | ||
return endOfCurrentRow, false | ||
} | ||
} | ||
|
||
// roundUpBy rounds cursor up to the next interval of v. If cursor is divisible | ||
evan-forbes marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// by v, then it returns cursor | ||
func roundUpBy(cursor, v int) int { | ||
switch { | ||
case cursor == 0: | ||
return cursor | ||
case cursor%v == 0: | ||
return cursor | ||
default: | ||
return ((cursor / v) + 1) * v | ||
} | ||
} | ||
|
||
func nextPowerOfTwo(v int) int { | ||
k := 1 | ||
for k < v { | ||
k = k << 1 | ||
} | ||
return k | ||
} | ||
|
||
func nextLowestPowerOfTwo(v int) int { | ||
c := nextPowerOfTwo(v) | ||
if c == v { | ||
return c | ||
} | ||
return c / 2 | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,260 @@ | ||
package shares | ||
|
||
import ( | ||
"fmt" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/assert" | ||
"github.com/tendermint/tendermint/pkg/consts" | ||
) | ||
|
||
func TestMsgSharesUsedNIDefaults(t *testing.T) { | ||
type test struct { | ||
cursor, squareSize, expected int | ||
msgLens []int | ||
indexes []uint32 | ||
} | ||
tests := []test{ | ||
{2, 4, 1, []int{1}, []uint32{2}}, | ||
rach-id marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{2, 2, 1, []int{1}, []uint32{2}}, | ||
{3, 4, 8, []int{3, 3}, []uint32{4, 8}}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [no change needed] visual
|
||
{0, 8, 8, []int{8}, nil}, // todo: actually fill out these values instead of nil | ||
{0, 8, 7, []int{7}, nil}, | ||
{0, 8, 7, []int{3, 3}, nil}, | ||
{1, 8, 8, []int{3, 3}, nil}, | ||
{1, 8, 32, []int{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}, nil}, | ||
{3, 8, 16, []int{5, 7}, nil}, | ||
{0, 8, 29, []int{5, 5, 5, 5}, nil}, | ||
{0, 8, 10, []int{10}, nil}, | ||
{0, 8, 26, []int{10, 10}, nil}, | ||
{1, 8, 33, []int{10, 10}, nil}, | ||
{2, 8, 32, []int{10, 10}, nil}, | ||
{0, 8, 55, []int{21, 31}, nil}, | ||
{0, 8, 128, []int{64, 64}, nil}, | ||
{0, consts.MaxSquareSize, 1000, []int{1000}, nil}, | ||
{0, consts.MaxSquareSize, consts.MaxSquareSize + 1, []int{consts.MaxSquareSize + 1}, nil}, | ||
{1, consts.MaxSquareSize, (consts.MaxSquareSize * 4) - 1, []int{consts.MaxSquareSize, consts.MaxSquareSize, consts.MaxSquareSize}, nil}, | ||
{1024, consts.MaxSquareSize, 32, []int{32}, nil}, | ||
evan-forbes marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
for i, tt := range tests { | ||
res, indexes := MsgSharesUsedNIDefaults(tt.cursor, tt.squareSize, tt.msgLens...) | ||
test := fmt.Sprintf("test %d: cursor %d, squareSize %d", i, tt.cursor, tt.squareSize) | ||
assert.Equal(t, tt.expected, res, test) | ||
if tt.indexes != nil { | ||
assert.Equal(t, tt.indexes, indexes, test) | ||
} | ||
evan-forbes marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
|
||
func TestFitsInSquare(t *testing.T) { | ||
type test struct { | ||
name string | ||
msgs []int | ||
start int | ||
size int | ||
fits bool | ||
} | ||
tests := []test{ | ||
{ | ||
name: "1 msgs size 2 shares (2 msg shares, 2 contiguous, size 4)", | ||
msgs: []int{2}, | ||
start: 2, | ||
size: 4, | ||
fits: true, | ||
}, | ||
{ | ||
name: "10 msgs size 10 shares (100 msg shares, 0 contiguous, size 4)", | ||
msgs: []int{10, 10, 10, 10, 10, 10, 10, 10, 10, 10}, | ||
start: 0, | ||
size: 4, | ||
fits: false, | ||
}, | ||
{ | ||
name: "15 msgs size 1 share (15 msg shares, 0 contiguous, size 4)", | ||
msgs: []int{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}, | ||
start: 0, | ||
size: 4, | ||
fits: true, | ||
}, | ||
{ | ||
name: "15 msgs size 1 share starting at share 2 (15 msg shares, 2 contiguous, size 4)", | ||
msgs: []int{1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1}, | ||
start: 2, | ||
size: 4, | ||
fits: false, | ||
}, | ||
{ | ||
name: "8 msgs of various sizes, 7 starting shares (63 msg shares, 1 contigous, size 8)", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [no change needed] I had a tough time visualizing so sharing if helpful for other reviewers
|
||
msgs: []int{3, 9, 3, 7, 8, 3, 7, 8}, | ||
start: 1, | ||
size: 8, | ||
fits: true, | ||
}, | ||
{ | ||
name: "8 msgs of various sizes, 7 starting shares (63 msg shares, 6 contigous, size 8)", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [question] I don't understand what contiguous means in this context. Given the cursor starts at There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 6777ea5 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, I still don't understand what contiguous means in this context or why There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. starting was just a confusing way to say contiguous shares. switched all to compact here 2bc32cf There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay thanks for explaining. I think the test names in 2bc32cf are inaccurate so commented #681 (review) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, sorry, accidentally pushed to the wrong branch. will fix there. |
||
msgs: []int{3, 9, 3, 7, 8, 3, 7, 8}, | ||
start: 6, | ||
size: 8, | ||
fits: false, | ||
}, | ||
} | ||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
res, _ := FitsInSquare(tt.start, tt.size, tt.msgs...) | ||
assert.Equal(t, tt.fits, res) | ||
}) | ||
} | ||
} | ||
|
||
func TestNextAlignedPowerOfTwo(t *testing.T) { | ||
type test struct { | ||
name string | ||
cursor, msgLen, k int | ||
expectedIndex int | ||
fits bool | ||
} | ||
tests := []test{ | ||
{ | ||
name: "whole row msgLen 4", | ||
cursor: 0, | ||
msgLen: 4, | ||
k: 4, | ||
fits: true, | ||
expectedIndex: 0, | ||
}, | ||
{ | ||
name: "half row msgLen 2 cursor 1", | ||
cursor: 1, | ||
msgLen: 2, | ||
k: 4, | ||
fits: true, | ||
expectedIndex: 2, | ||
}, | ||
{ | ||
name: "half row msgLen 2 cursor 2", | ||
cursor: 2, | ||
msgLen: 2, | ||
k: 4, | ||
fits: true, | ||
expectedIndex: 2, | ||
}, | ||
{ | ||
name: "half row msgLen 4 cursor 3", | ||
cursor: 3, | ||
msgLen: 4, | ||
k: 8, | ||
fits: true, | ||
expectedIndex: 4, | ||
}, | ||
{ | ||
name: "msgLen 5 cursor 3 size 8", | ||
cursor: 3, | ||
msgLen: 5, | ||
k: 8, | ||
fits: false, | ||
expectedIndex: 4, | ||
}, | ||
{ | ||
name: "msgLen 2 cursor 3 square size 8", | ||
cursor: 3, | ||
msgLen: 2, | ||
k: 8, | ||
fits: true, | ||
expectedIndex: 4, | ||
}, | ||
{ | ||
name: "cursor 3 msgLen 5 size 8", | ||
cursor: 3, | ||
msgLen: 5, | ||
k: 8, | ||
fits: false, | ||
expectedIndex: 4, | ||
}, | ||
{ | ||
name: "msglen 12 cursor 1 size 16", | ||
cursor: 1, | ||
msgLen: 12, | ||
k: 16, | ||
fits: false, | ||
expectedIndex: 8, | ||
}, | ||
{ | ||
name: "edge case where there are many messages with a single size", | ||
cursor: 10291, | ||
msgLen: 1, | ||
k: 128, | ||
fits: true, | ||
expectedIndex: 10291, | ||
}, | ||
evan-forbes marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
res, fits := NextAlignedPowerOfTwo(tt.cursor, tt.msgLen, tt.k) | ||
assert.Equal(t, tt.fits, fits) | ||
assert.Equal(t, tt.expectedIndex, res) | ||
}) | ||
} | ||
} | ||
|
||
func Test_roundUpBy(t *testing.T) { | ||
type test struct { | ||
cursor, v int | ||
expectedIndex int | ||
} | ||
tests := []test{ | ||
{ | ||
cursor: 1, | ||
v: 2, | ||
expectedIndex: 2, | ||
}, | ||
{ | ||
cursor: 2, | ||
v: 2, | ||
expectedIndex: 2, | ||
}, | ||
{ | ||
cursor: 0, | ||
v: 2, | ||
expectedIndex: 0, | ||
}, | ||
{ | ||
cursor: 5, | ||
v: 2, | ||
expectedIndex: 6, | ||
}, | ||
{ | ||
cursor: 8, | ||
v: 16, | ||
expectedIndex: 16, | ||
}, | ||
{ | ||
cursor: 33, | ||
v: 1, | ||
expectedIndex: 33, | ||
}, | ||
{ | ||
cursor: 32, | ||
v: 16, | ||
expectedIndex: 32, | ||
}, | ||
{ | ||
cursor: 33, | ||
v: 16, | ||
expectedIndex: 48, | ||
}, | ||
} | ||
for i, tt := range tests { | ||
t.Run( | ||
fmt.Sprintf( | ||
"test %d: %d cursor %d v %d expectedIndex", | ||
i, | ||
tt.cursor, | ||
tt.v, | ||
tt.expectedIndex, | ||
), | ||
func(t *testing.T) { | ||
res := roundUpBy(tt.cursor, tt.v) | ||
assert.Equal(t, tt.expectedIndex, res) | ||
}) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the rest of this comment got lost. Perhaps it meant to say
If so, should this function
panic
if this assumption is not met?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function does not rely on that assumption, otherwise it would not be able to handles messges that are larger than a single row. It definitely looks like we have to complete those docs, not exactly sure what I was going for originally 91c0d72