From 296d26ccaa579a5dfa02498aab5c8ca5ea3f28b7 Mon Sep 17 00:00:00 2001 From: Rahul Ghangas Date: Thu, 15 Dec 2022 12:46:38 +0530 Subject: [PATCH 01/20] chore: remove unused constant --- x/blob/types/payforblob.go | 1 - 1 file changed, 1 deletion(-) diff --git a/x/blob/types/payforblob.go b/x/blob/types/payforblob.go index 966dd5f61d..a8e7fdbb26 100644 --- a/x/blob/types/payforblob.go +++ b/x/blob/types/payforblob.go @@ -20,7 +20,6 @@ const ( URLMsgWirePayForBlob = "/blob.MsgWirePayForBlob" URLMsgPayForBlob = "/blob.MsgPayForBlob" ShareSize = appconsts.ShareSize - SquareSize = appconsts.DefaultMaxSquareSize NamespaceIDSize = appconsts.NamespaceSize ) From 74253c310fd497c61f404c613287dc61d90e993b Mon Sep 17 00:00:00 2001 From: Rahul Ghangas Date: Thu, 15 Dec 2022 12:47:36 +0530 Subject: [PATCH 02/20] feat: remove dependence on constant vals for min/max sqaure size --- pkg/da/data_availability_header.go | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/pkg/da/data_availability_header.go b/pkg/da/data_availability_header.go index 88dea67311..0754e1082f 100644 --- a/pkg/da/data_availability_header.go +++ b/pkg/da/data_availability_header.go @@ -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 @@ -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, ) } @@ -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") } @@ -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 if dah == nil { return errors.New("nil data availability header is not valid") } @@ -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 { 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) } From ef6ecb58573c0c7eab4f7f2e8dd3a6af2884fcab Mon Sep 17 00:00:00 2001 From: Rahul Ghangas Date: Thu, 15 Dec 2022 12:48:03 +0530 Subject: [PATCH 03/20] feat: remove dependence on constant vals for min/max sqaure size --- app/estimate_square_size.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/estimate_square_size.go b/app/estimate_square_size.go index d741ec68b2..49e0e40c07 100644 --- a/app/estimate_square_size.go +++ b/app/estimate_square_size.go @@ -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 @@ -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 @@ -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 From deb903594adc78d409efce67359586504f0a058b Mon Sep 17 00:00:00 2001 From: Rahul Ghangas Date: Thu, 15 Dec 2022 12:48:31 +0530 Subject: [PATCH 04/20] feat: pull params from blob module keeper --- app/prepare_proposal.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/prepare_proposal.go b/app/prepare_proposal.go index 97da9210f8..12abd7f887 100644 --- a/app/prepare_proposal.go +++ b/app/prepare_proposal.go @@ -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()}) + minSquareSize := app.BlobKeeper.MinSquareSize(ctx) + maxSquareSize := app.BlobKeeper.MaxSquareSize(ctx) + 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 @@ -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", From e656e366667f0bd8b8172d6d5969106d71885422 Mon Sep 17 00:00:00 2001 From: Rahul Ghangas Date: Thu, 15 Dec 2022 12:48:45 +0530 Subject: [PATCH 05/20] test: refactor test to use new interface --- pkg/da/data_availability_header_test.go | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/pkg/da/data_availability_header_test.go b/pkg/da/data_availability_header_test.go index 08e027c63b..256dc42fdc 100644 --- a/pkg/da/data_availability_header_test.go +++ b/pkg/da/data_availability_header_test.go @@ -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 @@ -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) @@ -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 @@ -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", @@ -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) @@ -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) @@ -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", @@ -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) From e3d33af08fa7d8248c11efb3bd2b899cc5271d2b Mon Sep 17 00:00:00 2001 From: Rahul Ghangas Date: Mon, 19 Dec 2022 17:24:37 +0530 Subject: [PATCH 06/20] Update pkg/da/data_availability_header.go Co-authored-by: Rootul P --- pkg/da/data_availability_header.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/da/data_availability_header.go b/pkg/da/data_availability_header.go index 0754e1082f..f3b40f6f15 100644 --- a/pkg/da/data_availability_header.go +++ b/pkg/da/data_availability_header.go @@ -124,8 +124,8 @@ func DataAvailabilityHeaderFromProto(dahp *daproto.DataAvailabilityHeader, minSq // ValidateBasic runs stateless checks on the DataAvailabilityHeader. func (dah *DataAvailabilityHeader) ValidateBasic(minSquareSize, maxSquareSize int) error { - maxExtendedSquareWidth := minSquareSize * 2 - minExtendedSquareWidth := maxSquareSize * 2 + minExtendedSquareWidth := minSquareSize * 2 + maxExtendedSquareWidth := maxSquareSize * 2 if dah == nil { return errors.New("nil data availability header is not valid") } From 4e4e4adbe8643503a127ac89c6e65fde3187a7ea Mon Sep 17 00:00:00 2001 From: Rahul Ghangas Date: Mon, 19 Dec 2022 18:22:08 +0530 Subject: [PATCH 07/20] feat: add function for checking if square size is valid --- app/estimate_square_size.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/estimate_square_size.go b/app/estimate_square_size.go index 49e0e40c07..3cc42a7adf 100644 --- a/app/estimate_square_size.go +++ b/app/estimate_square_size.go @@ -202,3 +202,8 @@ func overEstimateMalleatedTxSize(txLen, blobSize int) int { // actually malleating the tx return appconsts.MalleatedTxBytes + appconsts.MalleatedTxEstimateBuffer + malleatedTxLen } + +func isValidSquareSize(squareSize, minSquareSize, maxSquareSize int) bool { + // Check that square size is with range + return squareSize < minSquareSize || squareSize > maxSquareSize +} From 35820d420e492849d8c0e638dcade81a35123b0b Mon Sep 17 00:00:00 2001 From: Rahul Ghangas Date: Mon, 19 Dec 2022 18:23:02 +0530 Subject: [PATCH 08/20] chore: remove validity check for square size while creating extended data square --- pkg/da/data_availability_header.go | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/pkg/da/data_availability_header.go b/pkg/da/data_availability_header.go index f3b40f6f15..818b02241f 100644 --- a/pkg/da/data_availability_header.go +++ b/pkg/da/data_availability_header.go @@ -44,16 +44,8 @@ func NewDataAvailabilityHeader(eds *rsmt2d.ExtendedDataSquare) DataAvailabilityH return dah } -func ExtendShares(squareSize uint64, shares [][]byte, minSquareSize, maxSquareSize uint64) (*rsmt2d.ExtendedDataSquare, error) { - // Check that square size is with range - if squareSize < minSquareSize || squareSize > maxSquareSize { - return nil, fmt.Errorf( - "invalid square size: min %d max %d provided %d", - minSquareSize, - maxSquareSize, - squareSize, - ) - } +// ExtendShares generates an extended data square from given square size and shares +func ExtendShares(squareSize uint64, shares [][]byte) (*rsmt2d.ExtendedDataSquare, error) { // check that valid number of shares have been provided if squareSize*squareSize != uint64(len(shares)) { return nil, fmt.Errorf( @@ -171,9 +163,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(minSqaureSize, maxSqaureSize int) DataAvailabilityHeader { +func MinDataAvailabilityHeader(minSquareSize int) DataAvailabilityHeader { shares := GenerateEmptyShares(appconsts.MinShareCount) - eds, err := ExtendShares(uint64(minSqaureSize), shares, uint64(minSqaureSize), uint64(maxSqaureSize)) + eds, err := ExtendShares(uint64(minSquareSize), shares) if err != nil { panic(err) } From 6d7f9f011719e4ca0e491bcb0339ba5359377944 Mon Sep 17 00:00:00 2001 From: Rahul Ghangas Date: Mon, 19 Dec 2022 18:23:46 +0530 Subject: [PATCH 09/20] chore: remove extra parameters in call to ExtendShares --- app/prepare_proposal.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/prepare_proposal.go b/app/prepare_proposal.go index 12abd7f887..0cd37e9654 100644 --- a/app/prepare_proposal.go +++ b/app/prepare_proposal.go @@ -60,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), uint64(minSquareSize), uint64(maxSquareSize)) + eds, err := da.ExtendShares(squareSize, shares.ToBytes(dataSquare)) if err != nil { app.Logger().Error( "failure to erasure the data square while creating a proposal block", From 4406e4ac552c61cd3795f0e4f7b983b19b9d1106 Mon Sep 17 00:00:00 2001 From: Rahul Ghangas Date: Mon, 19 Dec 2022 18:24:03 +0530 Subject: [PATCH 10/20] test: remove extra parameters in call to ExtendShares --- pkg/da/data_availability_header_test.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/da/data_availability_header_test.go b/pkg/da/data_availability_header_test.go index 256dc42fdc..669882004d 100644 --- a/pkg/da/data_availability_header_test.go +++ b/pkg/da/data_availability_header_test.go @@ -23,7 +23,7 @@ func TestNilDataAvailabilityHeaderHashDoesntCrash(t *testing.T) { } func TestMinDataAvailabilityHeader(t *testing.T) { - dah := MinDataAvailabilityHeader(appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize) + dah := MinDataAvailabilityHeader(appconsts.DefaultMinSquareSize) 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, @@ -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, appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize) + eds, err := ExtendShares(tt.squareSize, tt.shares) require.NoError(t, err) resdah := NewDataAvailabilityHeader(eds) require.Equal(t, tt.squareSize*2, uint64(len(resdah.ColumnRoots)), tt.name) @@ -101,7 +101,7 @@ func TestExtendShares(t *testing.T) { for _, tt := range tests { tt := tt - eds, err := ExtendShares(tt.squareSize, tt.shares, appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize) + eds, err := ExtendShares(tt.squareSize, tt.shares) if tt.expectedErr { require.NotNil(t, err) continue @@ -118,14 +118,14 @@ func TestDataAvailabilityHeaderProtoConversion(t *testing.T) { } shares := generateShares(appconsts.DefaultMaxSquareSize*appconsts.DefaultMaxSquareSize, 1) - eds, err := ExtendShares(appconsts.DefaultMaxSquareSize, shares, appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize) + eds, err := ExtendShares(appconsts.DefaultMaxSquareSize, shares) require.NoError(t, err) bigdah := NewDataAvailabilityHeader(eds) tests := []test{ { name: "min", - dah: MinDataAvailabilityHeader(appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize), + dah: MinDataAvailabilityHeader(appconsts.DefaultMinSquareSize), }, { name: "max", @@ -153,7 +153,7 @@ func Test_DAHValidateBasic(t *testing.T) { } shares := generateShares(appconsts.DefaultMaxSquareSize*appconsts.DefaultMaxSquareSize, 1) - eds, err := ExtendShares(appconsts.DefaultMaxSquareSize, shares, appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize) + eds, err := ExtendShares(appconsts.DefaultMaxSquareSize, shares) require.NoError(t, err) bigdah := NewDataAvailabilityHeader(eds) @@ -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(appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize) + badHashDah := MinDataAvailabilityHeader(appconsts.DefaultMinSquareSize) badHashDah.hash = []byte{1, 2, 3, 4} // dah with not equal number of roots - mismatchDah := MinDataAvailabilityHeader(appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize) + mismatchDah := MinDataAvailabilityHeader(appconsts.DefaultMinSquareSize) mismatchDah.ColumnRoots = append(mismatchDah.ColumnRoots, bytes.Repeat([]byte{2}, 32)) tests := []test{ { name: "min", - dah: MinDataAvailabilityHeader(appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize), + dah: MinDataAvailabilityHeader(appconsts.DefaultMinSquareSize), }, { name: "max", From 20a72aa6048360185709febcb041754dc345c3f6 Mon Sep 17 00:00:00 2001 From: Rahul Ghangas Date: Mon, 19 Dec 2022 18:25:42 +0530 Subject: [PATCH 11/20] feat: add square size check as a block validity rule --- app/process_proposal.go | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/app/process_proposal.go b/app/process_proposal.go index 9b6ab8f76b..b3c57e50f4 100644 --- a/app/process_proposal.go +++ b/app/process_proposal.go @@ -2,6 +2,7 @@ package app import ( "bytes" + "fmt" "sort" "github.com/celestiaorg/celestia-app/pkg/appconsts" @@ -27,6 +28,10 @@ func (app *App) ProcessProposal(req abci.RequestProcessProposal) abci.ResponsePr // - the commitment in each PFB should match the commitment for the shares that contain that blob data // - there should be no unpaid-for data + ctx := app.NewContext(true, tmproto.Header{Height: app.LastBlockHeight()}) + minSquareSize := app.BlobKeeper.MinSquareSize(ctx) + maxSquareSize := app.BlobKeeper.MaxSquareSize(ctx) + data, err := coretypes.DataFromProto(req.BlockData) if err != nil { logInvalidPropBlockError(app.Logger(), req.Header, "failure to unmarshal block data:", err) @@ -35,6 +40,21 @@ func (app *App) ProcessProposal(req abci.RequestProcessProposal) abci.ResponsePr } } + squareSize := data.SquareSize + + // validate proposed square size + if !isValidSquareSize(int(squareSize), int(minSquareSize), int(maxSquareSize)) { + logInvalidPropBlockError(app.Logger(), req.Header, "invalid square size:", fmt.Errorf( + "min %d max %d provided %d", + minSquareSize, + maxSquareSize, + squareSize, + )) + return abci.ResponseProcessProposal{ + Result: abci.ResponseProcessProposal_REJECT, + } + } + if !sort.IsSorted(coretypes.BlobsByNamespace(data.Blobs)) { logInvalidPropBlock(app.Logger(), req.Header, "blobs are unsorted") return abci.ResponseProcessProposal{ @@ -50,7 +70,7 @@ func (app *App) ProcessProposal(req abci.RequestProcessProposal) abci.ResponsePr } } - cacher := inclusion.NewSubtreeCacher(data.SquareSize) + cacher := inclusion.NewSubtreeCacher(squareSize) eds, err := rsmt2d.ComputeExtendedDataSquare(shares.ToBytes(dataSquare), appconsts.DefaultCodec(), cacher.Constructor) if err != nil { logInvalidPropBlockError(app.Logger(), req.Header, "failure to erasure the data square", err) From 2888fbee4cad63059b980f086d2c303670885e51 Mon Sep 17 00:00:00 2001 From: Rahul Ghangas Date: Mon, 19 Dec 2022 20:07:15 +0530 Subject: [PATCH 12/20] test: check rejection on incorrect block size --- app/test/process_proposal_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/test/process_proposal_test.go b/app/test/process_proposal_test.go index f8aab7c078..1a00c054a8 100644 --- a/app/test/process_proposal_test.go +++ b/app/test/process_proposal_test.go @@ -100,6 +100,14 @@ func TestBlobInclusionCheck(t *testing.T) { }, expectedResult: abci.ResponseProcessProposal_REJECT, }, + { + name: "square size greater than max square size", + input: validData(), + mutator: func(d *core.Data) { + d.SquareSize = appconsts.DefaultMaxSquareSize * 2 + }, + expectedResult: abci.ResponseProcessProposal_REJECT, + }, } for _, tt := range tests { From 90b624b95f781e14167ed3c0989b4f6a2db85be7 Mon Sep 17 00:00:00 2001 From: Rahul Ghangas Date: Mon, 19 Dec 2022 21:30:27 +0530 Subject: [PATCH 13/20] fix: incorrect logic for square size validity check --- app/estimate_square_size.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/estimate_square_size.go b/app/estimate_square_size.go index 3cc42a7adf..991cadd3fb 100644 --- a/app/estimate_square_size.go +++ b/app/estimate_square_size.go @@ -205,5 +205,5 @@ func overEstimateMalleatedTxSize(txLen, blobSize int) int { func isValidSquareSize(squareSize, minSquareSize, maxSquareSize int) bool { // Check that square size is with range - return squareSize < minSquareSize || squareSize > maxSquareSize + return squareSize >= minSquareSize && squareSize <= maxSquareSize } From bc82ef6375ca057e5526377e88c3d2f2d9a5f018 Mon Sep 17 00:00:00 2001 From: Rahul Ghangas Date: Mon, 19 Dec 2022 21:31:19 +0530 Subject: [PATCH 14/20] chore: typo --- app/estimate_square_size.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/estimate_square_size.go b/app/estimate_square_size.go index 991cadd3fb..fd9022aa26 100644 --- a/app/estimate_square_size.go +++ b/app/estimate_square_size.go @@ -204,6 +204,6 @@ func overEstimateMalleatedTxSize(txLen, blobSize int) int { } func isValidSquareSize(squareSize, minSquareSize, maxSquareSize int) bool { - // Check that square size is with range + // Check that square size is within range return squareSize >= minSquareSize && squareSize <= maxSquareSize } From 9a06093be61b360401b2896e910f9f1bdcfdbb0e Mon Sep 17 00:00:00 2001 From: Rahul Ghangas Date: Mon, 19 Dec 2022 21:56:03 +0530 Subject: [PATCH 15/20] chore: convert uint32 to uint64 --- app/prepare_proposal.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/prepare_proposal.go b/app/prepare_proposal.go index 1abfad2556..e70eec2897 100644 --- a/app/prepare_proposal.go +++ b/app/prepare_proposal.go @@ -23,7 +23,7 @@ func (app *App) PrepareProposal(req abci.RequestPrepareProposal) abci.ResponsePr ctx := app.NewContext(true, core.Header{Height: app.LastBlockHeight()}) minSquareSize := app.BlobKeeper.MinSquareSize(ctx) maxSquareSize := app.BlobKeeper.MaxSquareSize(ctx) - squareSize, nonreservedStart := estimateSquareSize(parsedTxs, minSquareSize, maxSquareSize) + squareSize, nonreservedStart := estimateSquareSize(parsedTxs, uint64(minSquareSize), uint64(maxSquareSize)) // finalizeLayout wraps any blob transactions with their final share index. // This requires sorting the blobs by namespace and potentially pruning From eaabd6c5e6c8ce1ce455ebe4b20a1f67cb937979 Mon Sep 17 00:00:00 2001 From: Rahul Ghangas Date: Mon, 19 Dec 2022 21:59:01 +0530 Subject: [PATCH 16/20] test: refactor test to use updated function signature --- app/estimate_square_size_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/estimate_square_size_test.go b/app/estimate_square_size_test.go index e2f64190b6..8a02fe4717 100644 --- a/app/estimate_square_size_test.go +++ b/app/estimate_square_size_test.go @@ -35,7 +35,7 @@ func Test_estimateSquareSize(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { ptxs := generateMixedParsedTxs(tt.normalTxs, tt.pfbCount, tt.pfbSize) - res, _ := estimateSquareSize(ptxs) + res, _ := estimateSquareSize(ptxs, appconsts.DefaultMinSquareSize, appconsts.DefaultMaxSquareSize) assert.Equal(t, tt.expectedSquareSize, res) }) } From 10b7e34f715946686d17a7ac7043352868d19451 Mon Sep 17 00:00:00 2001 From: Rahul Ghangas Date: Mon, 19 Dec 2022 23:12:40 +0530 Subject: [PATCH 17/20] test: remove now invalid test --- pkg/da/data_availability_header_test.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pkg/da/data_availability_header_test.go b/pkg/da/data_availability_header_test.go index 669882004d..252991cda6 100644 --- a/pkg/da/data_availability_header_test.go +++ b/pkg/da/data_availability_header_test.go @@ -85,12 +85,6 @@ func TestExtendShares(t *testing.T) { } tests := []test{ - { - name: "too large square size", - expectedErr: true, - squareSize: appconsts.DefaultMaxSquareSize + 1, - shares: generateShares((appconsts.DefaultMaxSquareSize+1)*(appconsts.DefaultMaxSquareSize+1), 1), - }, { name: "invalid number of shares", expectedErr: true, From 076b9ad691ccfe4536695f6466a6a6dc0d54a0e3 Mon Sep 17 00:00:00 2001 From: Rahul Ghangas Date: Tue, 20 Dec 2022 18:56:10 +0530 Subject: [PATCH 18/20] fix: return default values for min/max square size if not block has been committed yet --- x/blob/keeper/params.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/x/blob/keeper/params.go b/x/blob/keeper/params.go index 62ef08be64..4d9a14411d 100644 --- a/x/blob/keeper/params.go +++ b/x/blob/keeper/params.go @@ -1,6 +1,7 @@ package keeper import ( + "github.com/celestiaorg/celestia-app/pkg/appconsts" "github.com/celestiaorg/celestia-app/x/blob/types" sdk "github.com/cosmos/cosmos-sdk/types" ) @@ -21,12 +22,18 @@ func (k Keeper) SetParams(ctx sdk.Context, params types.Params) { // MinSquareSize returns the MinSquareSize param func (k Keeper) MinSquareSize(ctx sdk.Context) (res uint32) { + if ctx.BlockHeader().Height < 1 { + return appconsts.DefaultMinSquareSize + } k.paramStore.Get(ctx, types.KeyMinSquareSize, &res) return } // MaxSquareSize returns the MaxSquareSize param func (k Keeper) MaxSquareSize(ctx sdk.Context) (res uint32) { + if ctx.BlockHeader().Height < 1 { + return appconsts.DefaultMaxSquareSize + } k.paramStore.Get(ctx, types.KeyMaxSquareSize, &res) return } From 286eaaab573bafc465bc8b5f8d0c169cb84caa38 Mon Sep 17 00:00:00 2001 From: Rahul Ghangas Date: Tue, 20 Dec 2022 21:31:43 +0530 Subject: [PATCH 19/20] test: test rejection of process proposal on square size less than minimum square size --- app/test/process_proposal_test.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/test/process_proposal_test.go b/app/test/process_proposal_test.go index 2e18246cfb..b09493473b 100644 --- a/app/test/process_proposal_test.go +++ b/app/test/process_proposal_test.go @@ -104,6 +104,14 @@ func TestBlobInclusionCheck(t *testing.T) { }, expectedResult: abci.ResponseProcessProposal_REJECT, }, + { + name: "square size of zero", + input: validData(), + mutator: func(d *core.Data) { + d.SquareSize = 0 + }, + expectedResult: abci.ResponseProcessProposal_REJECT, + }, } for _, tt := range tests { From 0a296c1d54b7b1a2cdc525f000b43d76ec2931f8 Mon Sep 17 00:00:00 2001 From: Rahul Ghangas Date: Wed, 21 Dec 2022 12:51:25 +0530 Subject: [PATCH 20/20] chore: add documentation for edge case of fetching min/max square size --- x/blob/keeper/params.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/x/blob/keeper/params.go b/x/blob/keeper/params.go index 4d9a14411d..64db2c2fd5 100644 --- a/x/blob/keeper/params.go +++ b/x/blob/keeper/params.go @@ -22,6 +22,8 @@ func (k Keeper) SetParams(ctx sdk.Context, params types.Params) { // MinSquareSize returns the MinSquareSize param func (k Keeper) MinSquareSize(ctx sdk.Context) (res uint32) { + // use the default size for the first block so that we return a value on the + // first block in PrepareProposal if ctx.BlockHeader().Height < 1 { return appconsts.DefaultMinSquareSize } @@ -31,6 +33,8 @@ func (k Keeper) MinSquareSize(ctx sdk.Context) (res uint32) { // MaxSquareSize returns the MaxSquareSize param func (k Keeper) MaxSquareSize(ctx sdk.Context) (res uint32) { + // use the default size for the first block so that we return a value on the + // first block in PrepareProposal if ctx.BlockHeader().Height < 1 { return appconsts.DefaultMaxSquareSize }