Skip to content

Commit

Permalink
fix!: use squareSizeUpperBound for worstCaseShareIndexes
Browse files Browse the repository at this point in the history
  • Loading branch information
@rootulp
rootulp committed Mar 5, 2024
1 parent 4135f35 commit a8e4223
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 42 deletions.
26 changes: 17 additions & 9 deletions square/builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import (
type Builder struct {
// maxSquareSize is the maximum number of rows (or columns) in the original data square
maxSquareSize int
// squareSize imposes an upper bound on the max effective square size.
squareSizeUpperBound int
// currentSize is an overestimate for the number of shares used by this builder.
currentSize int

Expand All @@ -32,15 +34,22 @@ type Builder struct {
subtreeRootThreshold int
}

func NewBuilder(maxSquareSize int, subtreeRootThreshold int, txs ...[]byte) (*Builder, error) {
func NewBuilder(maxSquareSize int, squareSizeUpperBound int, subtreeRootThreshold int, txs ...[]byte) (*Builder, error) {
if maxSquareSize <= 0 {
return nil, errors.New("max square size must be strictly positive")
}
if !shares.IsPowerOfTwo(maxSquareSize) {
return nil, errors.New("max square size must be a power of two")
}
if squareSizeUpperBound <= 0 {
return nil, errors.New("square size upper bound must be strictly positive")
}
if !shares.IsPowerOfTwo(squareSizeUpperBound) {
return nil, errors.New("square size upper bound must be a power of two")
}
builder := &Builder{
maxSquareSize: maxSquareSize,
squareSizeUpperBound: squareSizeUpperBound,
subtreeRootThreshold: subtreeRootThreshold,
Blobs: make([]*Element, 0),
Pfbs: make([]*blob.IndexWrapper, 0),
Expand Down Expand Up @@ -88,7 +97,7 @@ func (b *Builder) AppendBlobTx(blobTx *blob.BlobTx) bool {
iw := &blob.IndexWrapper{
Tx: blobTx.Tx,
TypeId: blob.ProtoIndexWrapperTypeID,
ShareIndexes: worstCaseShareIndexes(len(blobTx.Blobs), b.maxSquareSize),
ShareIndexes: worstCaseShareIndexes(len(blobTx.Blobs), b.squareSizeUpperBound),
}
size := proto.Size(iw)
pfbShareDiff := b.PfbCounter.Add(size)
Expand Down Expand Up @@ -407,14 +416,13 @@ func (e Element) maxShareOffset() int {
return e.NumShares + e.MaxPadding
}

// worstCaseShareIndexes returns the largest possible share indexes for a set
// of blobs at a given appversion. Largest possible is "worst" in that protobuf
// uses varints to encode integers, so larger integers can require more bytes to
// encode.
func worstCaseShareIndexes(blobs, maxSquareSize int) []uint32 {
shareIndexes := make([]uint32, blobs)
// worstCaseShareIndexes returns the largest possible share indexes for a set of
// blobs. Largest possible is "worst" in that protobuf uses varints to encode
// integers, so larger integers can require more bytes to encode.
func worstCaseShareIndexes(blobCount int, squareSizeUpperBound int) []uint32 {
shareIndexes := make([]uint32, blobCount)
for i := range shareIndexes {
shareIndexes[i] = uint32(maxSquareSize * maxSquareSize)
shareIndexes[i] = uint32(squareSizeUpperBound * squareSizeUpperBound)
}
return shareIndexes
}
16 changes: 8 additions & 8 deletions square/builder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,15 @@ func TestBuilderSquareSizeEstimation(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
txs := generateMixedTxs(tt.normalTxs, tt.pfbCount, 1, tt.pfbSize)
square, _, err := square.Build(txs, 64, defaultSubtreeRootThreshold)
square, _, err := square.Build(txs, 64, defaultSquareSizeUpperBound, defaultSubtreeRootThreshold)
require.NoError(t, err)
require.EqualValues(t, tt.expectedSquareSize, square.Size())
})
}
}

func TestBuilderRejectsTransactions(t *testing.T) {
builder, err := square.NewBuilder(2, 64) // 2 x 2 square
builder, err := square.NewBuilder(2, defaultSquareSizeUpperBound, 64) // 2 x 2 square
require.NoError(t, err)
require.False(t, builder.AppendTx(newTx(shares.AvailableBytesFromCompactShares(4)+1)))
require.True(t, builder.AppendTx(newTx(shares.AvailableBytesFromCompactShares(4))))
Expand Down Expand Up @@ -81,7 +81,7 @@ func TestBuilderRejectsBlobTransactions(t *testing.T) {

for idx, tc := range testCases {
t.Run(fmt.Sprintf("case%d", idx), func(t *testing.T) {
builder, err := square.NewBuilder(2, 64)
builder, err := square.NewBuilder(2, defaultSquareSizeUpperBound, 64)
require.NoError(t, err)
txs := generateBlobTxsWithNamespaces(ns1.Repeat(len(tc.blobSize)), [][]int{tc.blobSize})
require.Len(t, txs, 1)
Expand All @@ -93,11 +93,11 @@ func TestBuilderRejectsBlobTransactions(t *testing.T) {
}

func TestBuilderInvalidConstructor(t *testing.T) {
_, err := square.NewBuilder(-4, 64)
_, err := square.NewBuilder(-4, defaultSquareSizeUpperBound, 64)
require.Error(t, err)
_, err = square.NewBuilder(0, 64)
_, err = square.NewBuilder(0, defaultSquareSizeUpperBound, 64)
require.Error(t, err)
_, err = square.NewBuilder(13, 64)
_, err = square.NewBuilder(13, defaultSquareSizeUpperBound, 64)
require.Error(t, err)
}

Expand All @@ -109,7 +109,7 @@ func TestBuilderFindTxShareRange(t *testing.T) {
blockTxs := generateOrderedTxs(5, 5, 1000, 10)
require.Len(t, blockTxs, 10)

builder, err := square.NewBuilder(128, 64, blockTxs...)
builder, err := square.NewBuilder(128, defaultSquareSizeUpperBound, 64, blockTxs...)
require.NoError(t, err)

dataSquare, err := builder.Export()
Expand Down Expand Up @@ -293,7 +293,7 @@ func TestSquareBlobPostions(t *testing.T) {
}
for i, tt := range tests {
t.Run(fmt.Sprintf("case%d", i), func(t *testing.T) {
builder, err := square.NewBuilder(tt.squareSize, defaultSubtreeRootThreshold)
builder, err := square.NewBuilder(tt.squareSize, defaultSquareSizeUpperBound, defaultSubtreeRootThreshold)
require.NoError(t, err)
for _, tx := range tt.blobTxs {
blobTx, isBlobTx := blob.UnmarshalBlobTx(tx)
Expand Down
16 changes: 8 additions & 8 deletions square/square.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import (
// in the square and which have all PFBs trailing regular transactions. Note, this function does
// not check the underlying validity of the transactions.
// Errors should not occur and would reflect a violation in an invariant.
func Build(txs [][]byte, maxSquareSize, subtreeRootThreshold int) (Square, [][]byte, error) {
builder, err := NewBuilder(maxSquareSize, subtreeRootThreshold)
func Build(txs [][]byte, maxSquareSize, squareSizeUpperBound, subtreeRootThreshold int) (Square, [][]byte, error) {
builder, err := NewBuilder(maxSquareSize, squareSizeUpperBound, subtreeRootThreshold)
if err != nil {
return nil, nil, err
}
Expand Down Expand Up @@ -44,8 +44,8 @@ func Build(txs [][]byte, maxSquareSize, subtreeRootThreshold int) (Square, [][]b
//
// Note that this function does not check the underlying validity of
// the transactions.
func Construct(txs [][]byte, maxSquareSize, subtreeRootThreshold int) (Square, error) {
builder, err := NewBuilder(maxSquareSize, subtreeRootThreshold, txs...)
func Construct(txs [][]byte, maxSquareSize, squareSizeUpperBound, subtreeRootThreshold int) (Square, error) {
builder, err := NewBuilder(maxSquareSize, squareSizeUpperBound, subtreeRootThreshold, txs...)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -144,8 +144,8 @@ func Deconstruct(s Square, decoder PFBDecoder) ([][]byte, error) {

// TxShareRange returns the range of share indexes that the tx, specified by txIndex, occupies.
// The range is end exclusive.
func TxShareRange(txs [][]byte, txIndex, maxSquareSize, subtreeRootThreshold int) (shares.Range, error) {
builder, err := NewBuilder(maxSquareSize, subtreeRootThreshold, txs...)
func TxShareRange(txs [][]byte, txIndex, maxSquareSize, squareSizeUpperBound, subtreeRootThreshold int) (shares.Range, error) {
builder, err := NewBuilder(maxSquareSize, squareSizeUpperBound, subtreeRootThreshold, txs...)
if err != nil {
return shares.Range{}, err
}
Expand All @@ -155,8 +155,8 @@ func TxShareRange(txs [][]byte, txIndex, maxSquareSize, subtreeRootThreshold int

// BlobShareRange returns the range of share indexes that the blob, identified by txIndex and blobIndex, occupies.
// The range is end exclusive.
func BlobShareRange(txs [][]byte, txIndex, blobIndex, maxSquareSize, subtreeRootThreshold int) (shares.Range, error) {
builder, err := NewBuilder(maxSquareSize, subtreeRootThreshold, txs...)
func BlobShareRange(txs [][]byte, txIndex, blobIndex, maxSquareSize, squareSizeUpperBound, subtreeRootThreshold int) (shares.Range, error) {
builder, err := NewBuilder(maxSquareSize, squareSizeUpperBound, subtreeRootThreshold, txs...)
if err != nil {
return shares.Range{}, err
}
Expand Down
6 changes: 3 additions & 3 deletions square/square_benchmark_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ func BenchmarkSquareConstruct(b *testing.B) {
txs := generateOrderedTxs(txCount/2, txCount/2, 1, 1024)
b.ResetTimer()
for i := 0; i < b.N; i++ {
_, err := square.Construct(txs, defaultMaxSquareSize, defaultSubtreeRootThreshold)
_, err := square.Construct(txs, defaultMaxSquareSize, defaultSquareSizeUpperBound, defaultSubtreeRootThreshold)
require.NoError(b, err)
}
})
Expand All @@ -27,7 +27,7 @@ func BenchmarkSquareBuild(b *testing.B) {
txs := generateMixedTxs(txCount/2, txCount/2, 1, 1024)
b.ResetTimer()
for i := 0; i < b.N; i++ {
_, _, err := square.Build(txs, defaultMaxSquareSize, defaultSubtreeRootThreshold)
_, _, err := square.Build(txs, defaultMaxSquareSize, defaultSquareSizeUpperBound, defaultSubtreeRootThreshold)
require.NoError(b, err)
}
})
Expand All @@ -38,7 +38,7 @@ func BenchmarkSquareBuild(b *testing.B) {
txs := generateMixedTxs(0, txCount, 1, blobSize)
b.ResetTimer()
for i := 0; i < b.N; i++ {
_, _, err := square.Build(txs, defaultMaxSquareSize, defaultSubtreeRootThreshold)
_, _, err := square.Build(txs, defaultMaxSquareSize, defaultSquareSizeUpperBound, defaultSubtreeRootThreshold)
require.NoError(b, err)
}
})
Expand Down
29 changes: 15 additions & 14 deletions square/square_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
const (
mebibyte = 1_048_576 // one mebibyte in bytes
defaultMaxSquareSize = 128
defaultSquareSizeUpperBound = 128
defaultSubtreeRootThreshold = 64
)

Expand All @@ -25,18 +26,18 @@ func TestSquareConstruction(t *testing.T) {
t.Run("normal transactions after PFB transactions", func(t *testing.T) {
txs := sendTxs[:5]
txs = append(txs, append(pfbTxs, txs...)...)
_, err := square.Construct(txs, defaultMaxSquareSize, defaultSubtreeRootThreshold)
_, err := square.Construct(txs, defaultMaxSquareSize, defaultSquareSizeUpperBound, defaultSubtreeRootThreshold)
require.Error(t, err)
})
t.Run("not enough space to append transactions", func(t *testing.T) {
_, err := square.Construct(sendTxs, 2, defaultSubtreeRootThreshold)
_, err := square.Construct(sendTxs, 2, defaultSquareSizeUpperBound, defaultSubtreeRootThreshold)
require.Error(t, err)
_, err = square.Construct(pfbTxs, 2, defaultSubtreeRootThreshold)
_, err = square.Construct(pfbTxs, 2, defaultSquareSizeUpperBound, defaultSubtreeRootThreshold)
require.Error(t, err)
})
t.Run("construction should fail if a single PFB tx contains a blob that is too large to fit in the square", func(t *testing.T) {
pfbTxs := test.GenerateBlobTxs(1, 1, 2*mebibyte)
_, err := square.Construct(pfbTxs, 64, defaultSubtreeRootThreshold)
_, err := square.Construct(pfbTxs, 64, defaultSquareSizeUpperBound, defaultSubtreeRootThreshold)
require.Error(t, err)
})
}
Expand Down Expand Up @@ -100,7 +101,7 @@ func TestSquareTxShareRange(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
shareRange, err := square.TxShareRange(tc.txs, tc.index, 128, 64)
shareRange, err := square.TxShareRange(tc.txs, tc.index, 128, defaultSquareSizeUpperBound, 64)
if tc.expectErr {
require.Error(t, err)
} else {
Expand All @@ -115,7 +116,7 @@ func TestSquareTxShareRange(t *testing.T) {
func TestSquareBlobShareRange(t *testing.T) {
txs := test.GenerateBlobTxs(10, 1, 1024)

builder, err := square.NewBuilder(defaultMaxSquareSize, defaultSubtreeRootThreshold, txs...)
builder, err := square.NewBuilder(defaultMaxSquareSize, defaultSquareSizeUpperBound, defaultSubtreeRootThreshold, txs...)
require.NoError(t, err)

dataSquare, err := builder.Export()
Expand All @@ -125,7 +126,7 @@ func TestSquareBlobShareRange(t *testing.T) {
blobTx, isBlobTx := blob.UnmarshalBlobTx(tx)
require.True(t, isBlobTx)
for blobIdx := range blobTx.Blobs {
shareRange, err := square.BlobShareRange(txs, pfbIdx, blobIdx, defaultMaxSquareSize, defaultSubtreeRootThreshold)
shareRange, err := square.BlobShareRange(txs, pfbIdx, blobIdx, defaultMaxSquareSize, defaultSquareSizeUpperBound, defaultSubtreeRootThreshold)
require.NoError(t, err)
require.LessOrEqual(t, shareRange.End, len(dataSquare))
blobShares := dataSquare[shareRange.Start:shareRange.End]
Expand All @@ -136,16 +137,16 @@ func TestSquareBlobShareRange(t *testing.T) {
}

// error on out of bounds cases
_, err = square.BlobShareRange(txs, -1, 0, defaultMaxSquareSize, defaultSubtreeRootThreshold)
_, err = square.BlobShareRange(txs, -1, 0, defaultMaxSquareSize, defaultSquareSizeUpperBound, defaultSubtreeRootThreshold)
require.Error(t, err)

_, err = square.BlobShareRange(txs, 0, -1, defaultMaxSquareSize, defaultSubtreeRootThreshold)
_, err = square.BlobShareRange(txs, 0, -1, defaultMaxSquareSize, defaultSquareSizeUpperBound, defaultSubtreeRootThreshold)
require.Error(t, err)

_, err = square.BlobShareRange(txs, 10, 0, defaultMaxSquareSize, defaultSubtreeRootThreshold)
_, err = square.BlobShareRange(txs, 10, 0, defaultMaxSquareSize, defaultSquareSizeUpperBound, defaultSubtreeRootThreshold)
require.Error(t, err)

_, err = square.BlobShareRange(txs, 0, 10, defaultMaxSquareSize, defaultSubtreeRootThreshold)
_, err = square.BlobShareRange(txs, 0, 10, defaultMaxSquareSize, defaultSquareSizeUpperBound, defaultSubtreeRootThreshold)
require.Error(t, err)
}

Expand All @@ -155,7 +156,7 @@ func TestSquareDeconstruct(t *testing.T) {
for _, numTxs := range []int{2, 128, 1024, 8192} {
t.Run(fmt.Sprintf("%d", numTxs), func(t *testing.T) {
txs := generateOrderedTxs(numTxs/2, numTxs/2, 1, 800)
dataSquare, err := square.Construct(txs, defaultMaxSquareSize, defaultSubtreeRootThreshold)
dataSquare, err := square.Construct(txs, defaultMaxSquareSize, defaultSquareSizeUpperBound, defaultSubtreeRootThreshold)
require.NoError(t, err)
recomputedTxs, err := square.Deconstruct(dataSquare, test.DecodeMockPFB)
require.NoError(t, err)
Expand All @@ -166,15 +167,15 @@ func TestSquareDeconstruct(t *testing.T) {
t.Run("NoPFBs", func(t *testing.T) {
const numTxs = 10
txs := test.GenerateTxs(250, 250, numTxs)
dataSquare, err := square.Construct(txs, defaultMaxSquareSize, defaultSubtreeRootThreshold)
dataSquare, err := square.Construct(txs, defaultMaxSquareSize, defaultSquareSizeUpperBound, defaultSubtreeRootThreshold)
require.NoError(t, err)
recomputedTxs, err := square.Deconstruct(dataSquare, test.DecodeMockPFB)
require.NoError(t, err)
require.Equal(t, txs, recomputedTxs)
})
t.Run("PFBsOnly", func(t *testing.T) {
txs := test.GenerateBlobTxs(100, 1, 1024)
dataSquare, err := square.Construct(txs, defaultMaxSquareSize, defaultSubtreeRootThreshold)
dataSquare, err := square.Construct(txs, defaultMaxSquareSize, defaultSquareSizeUpperBound, defaultSubtreeRootThreshold)
require.NoError(t, err)
recomputedTxs, err := square.Deconstruct(dataSquare, test.DecodeMockPFB)
require.NoError(t, err)
Expand Down

0 comments on commit a8e4223

Please sign in to comment.