Skip to content
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: use square size params #1126

Closed
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
296d26c
chore: remove unused constant
rahulghangas Dec 15, 2022
74253c3
feat: remove dependence on constant vals for min/max sqaure size
rahulghangas Dec 15, 2022
ef6ecb5
feat: remove dependence on constant vals for min/max sqaure size
rahulghangas Dec 15, 2022
deb9035
feat: pull params from blob module keeper
rahulghangas Dec 15, 2022
e656e36
test: refactor test to use new interface
rahulghangas Dec 15, 2022
e3d33af
Update pkg/da/data_availability_header.go
rahulghangas Dec 19, 2022
4e4e4ad
feat: add function for checking if square size is valid
rahulghangas Dec 19, 2022
35820d4
chore: remove validity check for square size while creating extended …
rahulghangas Dec 19, 2022
6d7f9f0
chore: remove extra parameters in call to ExtendShares
rahulghangas Dec 19, 2022
4406e4a
test: remove extra parameters in call to ExtendShares
rahulghangas Dec 19, 2022
20a72aa
feat: add square size check as a block validity rule
rahulghangas Dec 19, 2022
2888fbe
test: check rejection on incorrect block size
rahulghangas Dec 19, 2022
90b624b
fix: incorrect logic for square size validity check
rahulghangas Dec 19, 2022
bc82ef6
chore: typo
rahulghangas Dec 19, 2022
012666a
Merge branch 'main' into feat/use-squaresize-params
rahulghangas Dec 19, 2022
9a06093
chore: convert uint32 to uint64
rahulghangas Dec 19, 2022
eaabd6c
test: refactor test to use updated function signature
rahulghangas Dec 19, 2022
10b7e34
test: remove now invalid test
rahulghangas Dec 19, 2022
076b9ad
fix: return default values for min/max square size if not block has b…
rahulghangas Dec 20, 2022
286eaaa
test: test rejection of process proposal on square size less than min…
rahulghangas Dec 20, 2022
0fca546
Merge branch 'main' into feat/use-squaresize-params
rahulghangas Dec 20, 2022
0a296c1
chore: add documentation for edge case of fetching min/max square size
rahulghangas Dec 21, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions app/estimate_square_size.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func calculateCompactShareCount(txs []*parsedTx, squareSize int) int {
// estimateSquareSize uses the provided block data to estimate the square size
// assuming that all malleated txs follow the non interactive default rules.
// Returns the estimated square size and the number of shares used.
func estimateSquareSize(txs []*parsedTx) (uint64, int) {
func estimateSquareSize(txs []*parsedTx, minSquareSize, maxSquareSize int) (uint64, int) {
// get the raw count of shares taken by each type of block data
txShares, msgLens := rawShareCount(txs)
msgShares := 0
Expand All @@ -99,8 +99,8 @@ func estimateSquareSize(txs []*parsedTx) (uint64, int) {
squareSize := shares.RoundUpPowerOfTwo(int(math.Ceil(math.Sqrt(float64(txShares + msgShares)))))

// the starting square size should at least be the minimum
if squareSize < appconsts.DefaultMinSquareSize {
squareSize = appconsts.DefaultMinSquareSize
if squareSize < minSquareSize {
squareSize = minSquareSize
}

var fits bool
Expand All @@ -113,8 +113,8 @@ func estimateSquareSize(txs []*parsedTx) (uint64, int) {
fits, msgShares = shares.FitsInSquare(txShares, squareSize, msgLens...)
switch {
// stop estimating if we know we can reach the max square size
case squareSize >= appconsts.DefaultMaxSquareSize:
return appconsts.DefaultMaxSquareSize, txShares + msgShares
case squareSize >= maxSquareSize:
return uint64(maxSquareSize), txShares + msgShares
// return if we've found a square size that fits all of the txs
case fits:
return uint64(squareSize), txShares + msgShares
Expand Down
7 changes: 5 additions & 2 deletions app/prepare_proposal.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ func (app *App) PrepareProposal(req abci.RequestPrepareProposal) abci.ResponsePr

// estimate the square size. This estimation errors on the side of larger
// squares but can only return values within the min and max square size.
squareSize, totalSharesUsed := estimateSquareSize(parsedTxs)
ctx := app.NewContext(true, core.Header{Height: app.LastBlockHeight()})
rahulghangas marked this conversation as resolved.
Show resolved Hide resolved
minSquareSize := app.BlobKeeper.MinSquareSize(ctx)
maxSquareSize := app.BlobKeeper.MaxSquareSize(ctx)
rahulghangas marked this conversation as resolved.
Show resolved Hide resolved
squareSize, totalSharesUsed := estimateSquareSize(parsedTxs, int(minSquareSize), int(maxSquareSize))

// the totalSharesUsed can be larger that the max number of shares if we
// reach the max square size. In this case, we must prune the deprioritized
Expand Down Expand Up @@ -57,7 +60,7 @@ func (app *App) PrepareProposal(req abci.RequestPrepareProposal) abci.ResponsePr
}

// erasure the data square which we use to create the data root.
eds, err := da.ExtendShares(squareSize, shares.ToBytes(dataSquare))
eds, err := da.ExtendShares(squareSize, shares.ToBytes(dataSquare), uint64(minSquareSize), uint64(maxSquareSize))
if err != nil {
app.Logger().Error(
"failure to erasure the data square while creating a proposal block",
Expand Down
25 changes: 11 additions & 14 deletions pkg/da/data_availability_header.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,6 @@ import (
"github.com/tendermint/tendermint/types"
)

const (
maxExtendedSquareWidth = appconsts.DefaultMaxSquareSize * 2
minExtendedSquareWidth = appconsts.DefaultMinSquareSize * 2
)

// DataAvailabilityHeader (DAHeader) contains the row and column roots of the
// erasure coded version of the data in Block.Data. The original Block.Data is
// split into shares and arranged in a square of width squareSize. Then, this
Expand Down Expand Up @@ -49,13 +44,13 @@ func NewDataAvailabilityHeader(eds *rsmt2d.ExtendedDataSquare) DataAvailabilityH
return dah
}

func ExtendShares(squareSize uint64, shares [][]byte) (*rsmt2d.ExtendedDataSquare, error) {
func ExtendShares(squareSize uint64, shares [][]byte, minSquareSize, maxSquareSize uint64) (*rsmt2d.ExtendedDataSquare, error) {
// Check that square size is with range
if squareSize < appconsts.DefaultMinSquareSize || squareSize > appconsts.DefaultMaxSquareSize {
if squareSize < minSquareSize || squareSize > maxSquareSize {
return nil, fmt.Errorf(
"invalid square size: min %d max %d provided %d",
appconsts.DefaultMinSquareSize,
appconsts.DefaultMaxSquareSize,
minSquareSize,
maxSquareSize,
squareSize,
)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a thought here, but I feel like verification of the square size and extending the shares should be two separate functions. In PrepareProposal, estimateSquareSize already guarantees that the square size is within the expected range.

Also the comparison of squareSize*squareSize == len(shares) should probably be done in ComputeExtendedDataSquare.

Expand Down Expand Up @@ -115,7 +110,7 @@ func (dah *DataAvailabilityHeader) ToProto() (*daproto.DataAvailabilityHeader, e
return dahp, nil
}

func DataAvailabilityHeaderFromProto(dahp *daproto.DataAvailabilityHeader) (dah *DataAvailabilityHeader, err error) {
func DataAvailabilityHeaderFromProto(dahp *daproto.DataAvailabilityHeader, minSquareSize, maxSquareSize int) (dah *DataAvailabilityHeader, err error) {
if dahp == nil {
return nil, errors.New("nil DataAvailabilityHeader")
}
Expand All @@ -124,11 +119,13 @@ func DataAvailabilityHeaderFromProto(dahp *daproto.DataAvailabilityHeader) (dah
dah.RowsRoots = dahp.RowRoots
dah.ColumnRoots = dahp.ColumnRoots

return dah, dah.ValidateBasic()
return dah, dah.ValidateBasic(minSquareSize, maxSquareSize)
}

// ValidateBasic runs stateless checks on the DataAvailabilityHeader.
func (dah *DataAvailabilityHeader) ValidateBasic() error {
func (dah *DataAvailabilityHeader) ValidateBasic(minSquareSize, maxSquareSize int) error {
maxExtendedSquareWidth := minSquareSize * 2
minExtendedSquareWidth := maxSquareSize * 2
rahulghangas marked this conversation as resolved.
Show resolved Hide resolved
if dah == nil {
return errors.New("nil data availability header is not valid")
}
Expand Down Expand Up @@ -174,9 +171,9 @@ var tailPaddingShare = append(

// MinDataAvailabilityHeader returns the minimum valid data availability header.
// It is equal to the data availability header for an empty block
func MinDataAvailabilityHeader() DataAvailabilityHeader {
func MinDataAvailabilityHeader(minSqaureSize, maxSqaureSize int) DataAvailabilityHeader {
rahulghangas marked this conversation as resolved.
Show resolved Hide resolved
shares := GenerateEmptyShares(appconsts.MinShareCount)
eds, err := ExtendShares(appconsts.DefaultMinSquareSize, shares)
eds, err := ExtendShares(uint64(minSqaureSize), shares, uint64(minSqaureSize), uint64(maxSqaureSize))
if err != nil {
panic(err)
}
Expand Down
24 changes: 12 additions & 12 deletions pkg/da/data_availability_header_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ func TestNilDataAvailabilityHeaderHashDoesntCrash(t *testing.T) {
}

func TestMinDataAvailabilityHeader(t *testing.T) {
dah := MinDataAvailabilityHeader()
dah := MinDataAvailabilityHeader(appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize)
expectedHash := []byte{
0x6f, 0x52, 0xda, 0xc1, 0x65, 0x45, 0xe4, 0x57, 0x25, 0xbe, 0x6e, 0xa3, 0x2a, 0xed, 0x55, 0x26,
0x6e, 0x45, 0x3, 0x48, 0x0, 0xee, 0xe1, 0xd8, 0x7c, 0x94, 0x28, 0xf4, 0x84, 0x4e, 0xa4, 0x7a,
}
require.Equal(t, expectedHash, dah.hash)
require.NoError(t, dah.ValidateBasic())
require.NoError(t, dah.ValidateBasic(appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize))
// important note: also see the types.TestEmptyBlockDataAvailabilityHeader test
// which ensures that empty block data results in the minimum data availability
// header
Expand Down Expand Up @@ -66,7 +66,7 @@ func TestNewDataAvailabilityHeader(t *testing.T) {

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
eds, err := ExtendShares(tt.squareSize, tt.shares)
eds, err := ExtendShares(tt.squareSize, tt.shares, appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize)
require.NoError(t, err)
resdah := NewDataAvailabilityHeader(eds)
require.Equal(t, tt.squareSize*2, uint64(len(resdah.ColumnRoots)), tt.name)
Expand Down Expand Up @@ -101,7 +101,7 @@ func TestExtendShares(t *testing.T) {

for _, tt := range tests {
tt := tt
eds, err := ExtendShares(tt.squareSize, tt.shares)
eds, err := ExtendShares(tt.squareSize, tt.shares, appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize)
if tt.expectedErr {
require.NotNil(t, err)
continue
Expand All @@ -118,14 +118,14 @@ func TestDataAvailabilityHeaderProtoConversion(t *testing.T) {
}

shares := generateShares(appconsts.DefaultMaxSquareSize*appconsts.DefaultMaxSquareSize, 1)
eds, err := ExtendShares(appconsts.DefaultMaxSquareSize, shares)
eds, err := ExtendShares(appconsts.DefaultMaxSquareSize, shares, appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize)
require.NoError(t, err)
bigdah := NewDataAvailabilityHeader(eds)

tests := []test{
{
name: "min",
dah: MinDataAvailabilityHeader(),
dah: MinDataAvailabilityHeader(appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize),
},
{
name: "max",
Expand All @@ -137,7 +137,7 @@ func TestDataAvailabilityHeaderProtoConversion(t *testing.T) {
tt := tt
pdah, err := tt.dah.ToProto()
require.NoError(t, err)
resDah, err := DataAvailabilityHeaderFromProto(pdah)
resDah, err := DataAvailabilityHeaderFromProto(pdah, appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize)
require.NoError(t, err)
resDah.Hash() // calc the hash to make the comparisons fair
require.Equal(t, tt.dah, *resDah, tt.name)
Expand All @@ -153,7 +153,7 @@ func Test_DAHValidateBasic(t *testing.T) {
}

shares := generateShares(appconsts.DefaultMaxSquareSize*appconsts.DefaultMaxSquareSize, 1)
eds, err := ExtendShares(appconsts.DefaultMaxSquareSize, shares)
eds, err := ExtendShares(appconsts.DefaultMaxSquareSize, shares, appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize)
require.NoError(t, err)
bigdah := NewDataAvailabilityHeader(eds)

Expand All @@ -170,16 +170,16 @@ func Test_DAHValidateBasic(t *testing.T) {
tooSmallDah.ColumnRoots = [][]byte{bytes.Repeat([]byte{2}, 32)}
tooSmallDah.RowsRoots = [][]byte{bytes.Repeat([]byte{2}, 32)}
// use a bad hash
badHashDah := MinDataAvailabilityHeader()
badHashDah := MinDataAvailabilityHeader(appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize)
badHashDah.hash = []byte{1, 2, 3, 4}
// dah with not equal number of roots
mismatchDah := MinDataAvailabilityHeader()
mismatchDah := MinDataAvailabilityHeader(appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize)
mismatchDah.ColumnRoots = append(mismatchDah.ColumnRoots, bytes.Repeat([]byte{2}, 32))

tests := []test{
{
name: "min",
dah: MinDataAvailabilityHeader(),
dah: MinDataAvailabilityHeader(appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize),
},
{
name: "max",
Expand Down Expand Up @@ -213,7 +213,7 @@ func Test_DAHValidateBasic(t *testing.T) {

for _, tt := range tests {
tt := tt
err := tt.dah.ValidateBasic()
err := tt.dah.ValidateBasic(appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize)
if tt.expectErr {
require.True(t, strings.Contains(err.Error(), tt.errStr), tt.name)
require.Error(t, err)
Expand Down
1 change: 0 additions & 1 deletion x/blob/types/payforblob.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ const (
URLMsgWirePayForBlob = "/blob.MsgWirePayForBlob"
URLMsgPayForBlob = "/blob.MsgPayForBlob"
ShareSize = appconsts.ShareSize
SquareSize = appconsts.DefaultMaxSquareSize
NamespaceIDSize = appconsts.NamespaceSize
)

Expand Down