From fcd65dbdafb0852872a6b0e1e9b26207ae672f6e Mon Sep 17 00:00:00 2001 From: Rootul P Date: Mon, 17 Oct 2022 18:49:39 -0600 Subject: [PATCH] feat!: increase share size to 512 bytes (#850) Closes https://github.com/celestiaorg/celestia-app/issues/825 This is ready for review but we may not merge it b/c we haven't made a final decision on what the share size should be Co-authored-by: evan-forbes --- app/estimate_square_size_test.go | 4 +- app/test/block_size_test.go | 2 +- pkg/appconsts/appconsts.go | 4 +- pkg/da/data_availability_header_test.go | 27 +++--- pkg/prove/proof_test.go | 14 +-- pkg/shares/reserved_bytes.go | 39 +++++++++ pkg/shares/reserved_bytes_test.go | 84 ++++++++++++++++++ pkg/shares/share_splitting_test.go | 111 ++++++++++++++---------- pkg/shares/split_compact_shares.go | 51 ++++++----- testutil/testnode/full_node.go | 2 +- testutil/testnode/full_node_test.go | 4 +- x/payment/types/payfordata_test.go | 2 +- x/payment/types/square_sizes_test.go | 30 +++---- x/payment/types/wirepayfordata_test.go | 9 -- 14 files changed, 261 insertions(+), 122 deletions(-) create mode 100644 pkg/shares/reserved_bytes.go create mode 100644 pkg/shares/reserved_bytes_test.go diff --git a/app/estimate_square_size_test.go b/app/estimate_square_size_test.go index 5b0203b..b5667a8 100644 --- a/app/estimate_square_size_test.go +++ b/app/estimate_square_size_test.go @@ -26,9 +26,9 @@ func Test_estimateSquareSize(t *testing.T) { tests := []test{ {"empty block minimum square size", 0, 0, 0, appconsts.MinSquareSize}, {"full block with only txs", 10000, 0, 0, appconsts.MaxSquareSize}, - {"random small block square size 4", 0, 1, appconsts.SparseShareContentSize, 4}, + {"random small block square size 2", 0, 1, appconsts.SparseShareContentSize, 2}, {"random small block square size 4", 0, 1, appconsts.SparseShareContentSize * 10, 4}, - {"random small block w/ 10 normal txs square size 4", 10, 1, appconsts.SparseShareContentSize, 8}, + {"random small block w/ 10 normal txs square size 4", 10, 1, appconsts.SparseShareContentSize, 4}, {"random small block square size 16", 0, 4, appconsts.SparseShareContentSize * 8, 16}, {"random medium block square size 32", 0, 50, appconsts.SparseShareContentSize * 4, 32}, {"full block max square size", 0, 8000, appconsts.SparseShareContentSize, appconsts.MaxSquareSize}, diff --git a/app/test/block_size_test.go b/app/test/block_size_test.go index f8286b5..c6141f1 100644 --- a/app/test/block_size_test.go +++ b/app/test/block_size_test.go @@ -31,7 +31,7 @@ func TestIntegrationTestSuite(t *testing.T) { cfg.EnableTMLogging = false cfg.MinGasPrices = "0ucls" cfg.NumValidators = 1 - cfg.TimeoutCommit = time.Millisecond * 200 + cfg.TimeoutCommit = time.Millisecond * 400 suite.Run(t, NewIntegrationTestSuite(cfg)) } diff --git a/pkg/appconsts/appconsts.go b/pkg/appconsts/appconsts.go index 1014c06..cbb4f5a 100644 --- a/pkg/appconsts/appconsts.go +++ b/pkg/appconsts/appconsts.go @@ -13,7 +13,7 @@ import ( // https://github.com/celestiaorg/celestia-specs/blob/master/src/specs/consensus.md#constants const ( // ShareSize is the size of a share in bytes. - ShareSize = 256 + ShareSize = 512 // NamespaceSize is the namespace size in bytes. NamespaceSize = 8 @@ -27,7 +27,7 @@ const ( // CompactShareReservedBytes is the number of bytes reserved for the location of // the first unit (transaction, ISR, evidence) in a compact share. - CompactShareReservedBytes = 1 + CompactShareReservedBytes = 2 // ContinuationCompactShareContentSize is the number of bytes usable for // data in a continuation compact share. A continuation share is any diff --git a/pkg/da/data_availability_header_test.go b/pkg/da/data_availability_header_test.go index b1760b4..1123c07 100644 --- a/pkg/da/data_availability_header_test.go +++ b/pkg/da/data_availability_header_test.go @@ -25,8 +25,8 @@ func TestNilDataAvailabilityHeaderHashDoesntCrash(t *testing.T) { func TestMinDataAvailabilityHeader(t *testing.T) { dah := MinDataAvailabilityHeader() expectedHash := []byte{ - 0x7b, 0x57, 0x8b, 0x35, 0x1b, 0x1b, 0xb, 0xbd, 0x70, 0xbb, 0x35, 0x0, 0x19, 0xeb, 0xc9, 0x64, - 0xc4, 0x4a, 0x14, 0xa, 0x37, 0xef, 0x71, 0x5b, 0x55, 0x2a, 0x7f, 0x8f, 0x31, 0x5a, 0xcd, 0x19, + 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()) @@ -47,8 +47,8 @@ func TestNewDataAvailabilityHeader(t *testing.T) { { name: "typical", expectedHash: []byte{ - 0xfe, 0x9c, 0x6b, 0xd8, 0xe5, 0x7c, 0xd1, 0x5d, 0x1f, 0xd6, 0x55, 0x7e, 0x87, 0x7d, 0xd9, 0x7d, - 0xdb, 0xf2, 0x66, 0xfa, 0x60, 0x24, 0x2d, 0xb3, 0xa0, 0x9c, 0x4f, 0x4e, 0x5b, 0x2a, 0x2c, 0x2a, + 0x57, 0x71, 0xc6, 0x77, 0x2f, 0x32, 0x95, 0x73, 0xaa, 0xb8, 0x20, 0xd1, 0xbe, 0x4c, 0xc2, 0x21, + 0x7d, 0x54, 0xb6, 0x7e, 0xf2, 0x4f, 0xbc, 0xd3, 0x9a, 0x95, 0x15, 0xd0, 0x92, 0x63, 0xc1, 0xf9, }, squareSize: 2, shares: generateShares(4, 1), @@ -56,8 +56,8 @@ func TestNewDataAvailabilityHeader(t *testing.T) { { name: "max square size", expectedHash: []byte{ - 0xe2, 0x87, 0x23, 0xd0, 0x2d, 0x54, 0x25, 0x5f, 0x79, 0x43, 0x8e, 0xfb, 0xb7, 0xe8, 0xfa, 0xf5, - 0xbf, 0x93, 0x50, 0xb3, 0x64, 0xd0, 0x4f, 0xa7, 0x7b, 0xb1, 0x83, 0x3b, 0x8, 0xba, 0xd3, 0xa4, + 0xbf, 0xe5, 0x8f, 0x4b, 0xae, 0x2b, 0x65, 0x8b, 0xa8, 0xcb, 0xf9, 0xee, 0x8c, 0x6a, 0x1f, 0x72, + 0xa9, 0x58, 0xc4, 0xcc, 0xca, 0x41, 0x4c, 0xbf, 0x8b, 0x18, 0xf9, 0x53, 0xe, 0xb1, 0x40, 0x54, }, squareSize: appconsts.MaxSquareSize, shares: generateShares(appconsts.MaxSquareSize*appconsts.MaxSquareSize, 99), @@ -65,13 +65,14 @@ func TestNewDataAvailabilityHeader(t *testing.T) { } for _, tt := range tests { - tt := tt - 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) - require.Equal(t, tt.squareSize*2, uint64(len(resdah.RowsRoots)), tt.name) - require.Equal(t, tt.expectedHash, resdah.hash, tt.name) + t.Run(tt.name, func(t *testing.T) { + 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) + require.Equal(t, tt.squareSize*2, uint64(len(resdah.RowsRoots)), tt.name) + require.Equal(t, tt.expectedHash, resdah.hash, tt.name) + }) } } diff --git a/pkg/prove/proof_test.go b/pkg/prove/proof_test.go index bdcaf06..40ba03a 100644 --- a/pkg/prove/proof_test.go +++ b/pkg/prove/proof_test.go @@ -152,14 +152,14 @@ func TestTxShareIndex(t *testing.T) { {appconsts.FirstCompactShareContentSize + appconsts.ContinuationCompactShareContentSize + 1, 2}, {appconsts.FirstCompactShareContentSize + (appconsts.ContinuationCompactShareContentSize * 2), 2}, {appconsts.FirstCompactShareContentSize + (appconsts.ContinuationCompactShareContentSize * 2) + 1, 3}, - // 81 compact shares + partially filled out last share + // 81 full compact shares then a partially filled out 82nd share (which is index 81 because 0-indexed) {appconsts.FirstCompactShareContentSize + (appconsts.ContinuationCompactShareContentSize * 80) + 160, 81}, - // 81 compact shares + full last share - {appconsts.FirstCompactShareContentSize + (appconsts.ContinuationCompactShareContentSize * 80) + 246, 81}, - // 82 compact shares + one byte in last share - {appconsts.FirstCompactShareContentSize + (appconsts.ContinuationCompactShareContentSize * 80) + 247, 82}, - // 82 compact shares + two bytes in last share - {appconsts.FirstCompactShareContentSize + (appconsts.ContinuationCompactShareContentSize * 80) + 248, 82}, + // 81 full compact shares then a full 82nd share + {appconsts.FirstCompactShareContentSize + (appconsts.ContinuationCompactShareContentSize * 80) + 501, 81}, + // 82 full compact shares then one byte in 83rd share + {appconsts.FirstCompactShareContentSize + (appconsts.ContinuationCompactShareContentSize * 80) + 502, 82}, + // 82 compact shares then two bytes in 83rd share + {appconsts.FirstCompactShareContentSize + (appconsts.ContinuationCompactShareContentSize * 80) + 503, 82}, } for _, tt := range tests { diff --git a/pkg/shares/reserved_bytes.go b/pkg/shares/reserved_bytes.go new file mode 100644 index 0000000..180756b --- /dev/null +++ b/pkg/shares/reserved_bytes.go @@ -0,0 +1,39 @@ +package shares + +import ( + "bytes" + "encoding/binary" + "fmt" + + "github.com/celestiaorg/celestia-app/pkg/appconsts" +) + +// NewReservedBytes returns a byte slice of length +// appconsts.CompactShareReservedBytes that contains a varint of the byteIndex +// of the first unit that starts in a compact share. If no unit starts in the +// compact share, ReservedBytes is [0, 0]. +func NewReservedBytes(byteIndex uint64) ([]byte, error) { + if byteIndex >= appconsts.ShareSize { + return []byte{}, fmt.Errorf("byte index %d must be less than share size %d", byteIndex, appconsts.ShareSize) + } + reservedBytes := make([]byte, appconsts.CompactShareReservedBytes) + binary.PutUvarint(reservedBytes, byteIndex) + return reservedBytes, nil +} + +// ParseReservedBytes parses a byte slice of length +// appconsts.CompactShareReservedBytes into a byteIndex. +func ParseReservedBytes(reservedBytes []byte) (uint64, error) { + if len(reservedBytes) != appconsts.CompactShareReservedBytes { + return 0, fmt.Errorf("reserved bytes must be of length %d", appconsts.CompactShareReservedBytes) + } + reader := bytes.NewReader(reservedBytes) + byteIndex, err := binary.ReadUvarint(reader) + if err != nil { + return 0, err + } + if byteIndex >= appconsts.ShareSize { + return 0, fmt.Errorf("reserved bytes varint %d must be less than share size %d", byteIndex, appconsts.ShareSize) + } + return byteIndex, nil +} diff --git a/pkg/shares/reserved_bytes_test.go b/pkg/shares/reserved_bytes_test.go new file mode 100644 index 0000000..3a68b39 --- /dev/null +++ b/pkg/shares/reserved_bytes_test.go @@ -0,0 +1,84 @@ +package shares + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestParseReservedBytes(t *testing.T) { + type testCase struct { + name string + input []byte + want uint64 + expectErr bool + } + testCases := []testCase{ + {"byte index of 0", []byte{0, 0}, 0, false}, + {"byte index of 2", []byte{2, 0}, 2, false}, + {"byte index of 4", []byte{4, 0}, 4, false}, + {"byte index of 8", []byte{8, 0}, 8, false}, + {"byte index of 16", []byte{16, 0}, 16, false}, + {"byte index of 32", []byte{32, 0}, 32, false}, + {"byte index of 64", []byte{64, 0}, 64, false}, + {"byte index of 128", []byte{128, 1}, 128, false}, + {"byte index of 256", []byte{128, 2}, 256, false}, + {"byte index of 511", []byte{255, 3}, 511, false}, + + // error cases + {"empty", []byte{}, 0, true}, + {"too few reserved bytes", []byte{1}, 0, true}, + {"too many reserved bytes", []byte{3, 3, 3}, 0, true}, + {"byte index of 512 is equal to share size", []byte{128, 4}, 0, true}, + {"byte index of 1000 is greater than share size", []byte{232, 7}, 0, true}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got, err := ParseReservedBytes(tc.input) + if tc.expectErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + +func TestNewReservedBytes(t *testing.T) { + type testCase struct { + name string + input uint64 + want []byte + expectErr bool + } + testCases := []testCase{ + {"byte index of 0", 0, []byte{0, 0}, false}, + {"byte index of 2", 2, []byte{2, 0}, false}, + {"byte index of 4", 4, []byte{4, 0}, false}, + {"byte index of 8", 8, []byte{8, 0}, false}, + {"byte index of 16", 16, []byte{16, 0}, false}, + {"byte index of 32", 32, []byte{32, 0}, false}, + {"byte index of 64", 64, []byte{64, 0}, false}, + {"byte index of 128", 128, []byte{128, 1}, false}, + {"byte index of 256", 256, []byte{128, 2}, false}, + {"byte index of 511", 511, []byte{255, 3}, false}, + + // error cases + {"byte index of 512 is equal to share size", 512, []byte{}, true}, + {"byte index of 1000 is greater than share size", 1000, []byte{}, true}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got, err := NewReservedBytes(tc.input) + if tc.expectErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} diff --git a/pkg/shares/share_splitting_test.go b/pkg/shares/share_splitting_test.go index 699acb0..4c1955e 100644 --- a/pkg/shares/share_splitting_test.go +++ b/pkg/shares/share_splitting_test.go @@ -5,10 +5,15 @@ import ( "reflect" "testing" + "github.com/celestiaorg/celestia-app/pkg/appconsts" coretypes "github.com/tendermint/tendermint/types" ) func TestSplitTxs(t *testing.T) { + smallTransactionA := coretypes.Tx{0xa} + smallTransactionB := coretypes.Tx{0xb} + largeTransaction := bytes.Repeat([]byte{0xc}, 512) + type testCase struct { name string txs coretypes.Txs @@ -22,93 +27,94 @@ func TestSplitTxs(t *testing.T) { }, { name: "one small tx", - txs: coretypes.Txs{coretypes.Tx{0xa}}, + txs: coretypes.Txs{smallTransactionA}, want: []Share{ - append([]uint8{ + padShare([]uint8{ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, // namespace id 0x1, // info byte - 0x2, 0x0, 0x0, 0x0, // 1 byte (unit) + 1 byte (unit length) = 2 bytes message length - 14, // reserved byte + 0x2, 0x0, 0x0, 0x0, // 1 byte (unit) + 1 byte (unit length) = 2 bytes sequence length + 15, 0, // reserved bytes 0x1, // unit length of first transaction 0xa, // data of first transaction - }, bytes.Repeat([]byte{0}, 240)...), // padding + }), }, }, { name: "two small txs", - txs: coretypes.Txs{coretypes.Tx{0xa}, coretypes.Tx{0xb}}, + txs: coretypes.Txs{smallTransactionA, smallTransactionB}, want: []Share{ - append([]uint8{ + padShare([]uint8{ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, // namespace id 0x1, // info byte - 0x4, 0x0, 0x0, 0x0, // 2 bytes (first transaction) + 2 bytes (second transaction) = 4 bytes message length - 14, // reserved byte + 0x4, 0x0, 0x0, 0x0, // 2 bytes (first transaction) + 2 bytes (second transaction) = 4 bytes sequence length + 15, 0, // reserved bytes 0x1, // unit length of first transaction 0xa, // data of first transaction 0x1, // unit length of second transaction 0xb, // data of second transaction - }, bytes.Repeat([]byte{0}, 238)...), // padding + }), }, }, { name: "one large tx that spans two shares", - txs: coretypes.Txs{bytes.Repeat([]byte{0xC}, 241)}, + txs: coretypes.Txs{largeTransaction}, want: []Share{ - append([]uint8{ + fillShare([]uint8{ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, // namespace id 0x1, // info byte - 243, 1, 0, 0, // 241 (unit) + 2 (unit length) = 243 message length - 14, // reserved byte - 241, 1, // unit length of first transaction is 241 - }, bytes.Repeat([]byte{0xc}, 240)...), // data of first transaction - append([]uint8{ + 130, 4, 0, 0, // 512 (unit) + 2 (unit length) = 514 sequence length + 15, 0, // reserved bytes + 128, 4, // unit length of transaction is 512 + }, 0xc), // data of transaction + padShare(append([]uint8{ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, // namespace id - 0x0, // info byte - 0x0, // reserved byte - 0xc, // continuation data of first transaction - }, bytes.Repeat([]byte{0x0}, 245)...), // padding + 0x0, // info byte + 0, 0, // reserved bytes + }, bytes.Repeat([]byte{0xc}, 17)..., // continuation data of transaction + )), }, }, { name: "one small tx then one large tx that spans two shares", - txs: coretypes.Txs{coretypes.Tx{0xd}, bytes.Repeat([]byte{0xe}, 241)}, + txs: coretypes.Txs{smallTransactionA, largeTransaction}, want: []Share{ - append([]uint8{ + fillShare([]uint8{ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, // namespace id 0x1, // info byte - 245, 1, 0, 0, // 2 bytes (first transaction) + 243 bytes (second transaction) = 245 bytes message length - 14, // reserved byte + 132, 4, 0, 0, // 2 bytes (first transaction) + 514 bytes (second transaction) = 516 bytes sequence length + 15, 0, // reserved bytes 1, // unit length of first transaction - 0xd, // data of first transaction - 241, 1, // unit length of second transaction is 241 - }, bytes.Repeat([]byte{0xe}, 238)...), // data of first transaction - append([]uint8{ - 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, // namespace id - 0x0, // info byte - 0x0, // reserved byte - 0xe, 0xe, 0xe, // continuation data of second transaction - }, bytes.Repeat([]byte{0x0}, 243)...), // padding + 0xa, // data of first transaction + 128, 4, // unit length of second transaction is 512 + }, 0xc), // data of second transaction + padShare( + append([]uint8{ + 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, // namespace id + 0x0, // info byte + 0, 0, // reserved bytes + }, bytes.Repeat([]byte{0xc}, 19)...), // continuation data of second transaction + ), }, }, { name: "one large tx that spans two shares then one small tx", - txs: coretypes.Txs{bytes.Repeat([]byte{0xe}, 241), coretypes.Tx{0xd}}, + txs: coretypes.Txs{largeTransaction, smallTransactionA}, want: []Share{ - append([]uint8{ + fillShare([]uint8{ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, // namespace id 0x1, // info byte - 245, 1, 0, 0, // 243 bytes (first transaction) + 2 bytes (second transaction) = 245 bytes message length - 14, // reserved byte - 241, 1, // unit length of first transaction is 241 - }, bytes.Repeat([]byte{0xe}, 240)...), // data of first transaction - append([]uint8{ + 132, 4, 0, 0, // 514 bytes (first transaction) + 2 bytes (second transaction) = 516 bytes sequence length + 15, 0, // reserved bytes + 128, 4, // unit length of first transaction is 512 + }, 0xc), // data of first transaction + padShare([]uint8{ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, // namespace id - 0x0, // info byte - 11, // reserved byte - 0xe, // continuation data of first transaction + 0x0, // info byte + 28, 0, // reserved bytes + 0xc, 0xc, 0xc, 0xc, 0xc, 0xc, 0xc, 0xc, 0xc, 0xc, 0xc, 0xc, 0xc, 0xc, 0xc, 0xc, 0xc, // continuation data of first transaction 1, // unit length of second transaction - 0xd, // data of second transaction - }, bytes.Repeat([]byte{0x0}, 243)...), // padding + 0xa, // data of second transaction + }), }, }, } @@ -120,4 +126,15 @@ func TestSplitTxs(t *testing.T) { } }) } -} // +} + +// padShare returns a share padded with trailing zeros. +func padShare(share []byte) (paddedShare []byte) { + return fillShare(share, 0) +} + +// fillShare returns a share filled with filler so that the share length +// is equal to appconsts.ShareSize. +func fillShare(share []byte, filler byte) (paddedShare []byte) { + return append(share, bytes.Repeat([]byte{filler}, appconsts.ShareSize-len(share))...) +} diff --git a/pkg/shares/split_compact_shares.go b/pkg/shares/split_compact_shares.go index 41b5ff2..4fe15f7 100644 --- a/pkg/shares/split_compact_shares.go +++ b/pkg/shares/split_compact_shares.go @@ -61,7 +61,7 @@ func (css *CompactShareSplitter) WriteEvidence(evd coretypes.Evidence) error { // WriteBytes adds the delimited data to the underlying compact shares. func (css *CompactShareSplitter) WriteBytes(rawData []byte) { - css.maybeWriteReservedByteToPendingShare() + css.maybeWriteReservedBytesToPendingShare() txCursor := len(rawData) for txCursor != 0 { @@ -158,38 +158,45 @@ func (css *CompactShareSplitter) writeDataLengthVarintToFirstShare(dataLengthVar css.shares[0] = firstShare } -// maybeWriteReservedByteToPendingShare will be a no-op if the reserved byte has -// already been populated. If the reserved byte is empty, it will write the -// location of the next unit of data to the reserved byte. -func (css *CompactShareSplitter) maybeWriteReservedByteToPendingShare() { - if !css.isEmptyReservedByte() { +// maybeWriteReservedBytesToPendingShare will be a no-op if the reserved bytes +// have already been populated. If the reserved bytes are empty, it will write +// the location of the next unit of data to the reserved bytes. +func (css *CompactShareSplitter) maybeWriteReservedBytesToPendingShare() { + if !css.isEmptyReservedBytes() { return } - locationOfNextUnit := len(css.pendingShare) - if locationOfNextUnit >= appconsts.ShareSize { - panic(fmt.Sprintf("location of next unit %v is greater than or equal to the share size %v", locationOfNextUnit, appconsts.ShareSize)) + byteIndexOfNextUnit := len(css.pendingShare) + reservedBytes, err := NewReservedBytes(uint64(byteIndexOfNextUnit)) + if err != nil { + panic(err) } - // write the location of next unit to the reserved byte of the pending share - if css.isPendingShareTheFirstShare() { - css.pendingShare[appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareSequenceLengthBytes : appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareContentSize+appconsts.CompactShareReservedBytes][0] = byte(locationOfNextUnit) - } else { - css.pendingShare[appconsts.NamespaceSize+appconsts.ShareInfoBytes : appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.CompactShareReservedBytes][0] = byte(locationOfNextUnit) + indexOfReservedBytes := css.indexOfReservedBytes() + // overwrite the reserved bytes of the pending share + for i := 0; i < appconsts.CompactShareReservedBytes; i++ { + css.pendingShare[indexOfReservedBytes+i] = reservedBytes[i] } } -// isEmptyReservedByte returns true if the reserved byte is empty. -func (css *CompactShareSplitter) isEmptyReservedByte() bool { - var reservedByte byte +// isEmptyReservedBytes returns true if the reserved bytes are empty. +func (css *CompactShareSplitter) isEmptyReservedBytes() bool { + indexOfReservedBytes := css.indexOfReservedBytes() + reservedBytes, err := ParseReservedBytes(css.pendingShare[indexOfReservedBytes : indexOfReservedBytes+appconsts.CompactShareReservedBytes]) + if err != nil { + panic(err) + } + return reservedBytes == 0 +} +// indexOfReservedBytes returns the index of the reserved bytes in the pending share. +func (css *CompactShareSplitter) indexOfReservedBytes() int { if css.isPendingShareTheFirstShare() { - reservedByte = css.pendingShare[appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareSequenceLengthBytes : appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareContentSize+appconsts.CompactShareReservedBytes][0] - } else { - reservedByte = css.pendingShare[appconsts.NamespaceSize+appconsts.ShareInfoBytes : appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.CompactShareReservedBytes][0] + // if the pending share is the first share, the reserved bytes follow the namespace, info byte, and sequence length varint + return appconsts.NamespaceSize + appconsts.ShareInfoBytes + appconsts.FirstCompactShareSequenceLengthBytes } - - return reservedByte == 0 + // if the pending share is not the first share, the reserved bytes follow the namespace and info byte + return appconsts.NamespaceSize + appconsts.ShareInfoBytes } // dataLength returns the total length in bytes of all units (transactions, diff --git a/testutil/testnode/full_node.go b/testutil/testnode/full_node.go index 88199b3..a19189f 100644 --- a/testutil/testnode/full_node.go +++ b/testutil/testnode/full_node.go @@ -131,7 +131,7 @@ func DefaultParams() *tmproto.ConsensusParams { func DefaultTendermintConfig() *config.Config { tmCfg := config.DefaultConfig() - tmCfg.Consensus.TimeoutCommit = time.Millisecond * 100 + tmCfg.Consensus.TimeoutCommit = time.Millisecond * 200 tmCfg.Mempool.MaxTxBytes = 22020096 // 21MB return tmCfg } diff --git a/testutil/testnode/full_node_test.go b/testutil/testnode/full_node_test.go index c646207..5ade22c 100644 --- a/testutil/testnode/full_node_test.go +++ b/testutil/testnode/full_node_test.go @@ -64,9 +64,9 @@ func (s *IntegrationTestSuite) Test_Liveness() { var params *coretypes.ResultConsensusParams // this query can be flaky with fast block times, so we repeat it multiple // times in attempt to increase the probability of it working - for i := 0; i < 5; i++ { + for i := 0; i < 10; i++ { params, err = s.cctx.Client.ConsensusParams(context.TODO(), nil) - if err != nil { + if err != nil || params == nil { continue } break diff --git a/x/payment/types/payfordata_test.go b/x/payment/types/payfordata_test.go index 37daf05..ab45eba 100644 --- a/x/payment/types/payfordata_test.go +++ b/x/payment/types/payfordata_test.go @@ -167,7 +167,7 @@ func TestCreateCommitment(t *testing.T) { squareSize: 4, namespace: bytes.Repeat([]byte{0xFF}, 8), message: bytes.Repeat([]byte{0xFF}, 11*ShareSize), - expected: []byte{0x6a, 0x29, 0xa3, 0xfa, 0x6a, 0xe3, 0x68, 0x9b, 0xce, 0xc8, 0x30, 0x1d, 0x32, 0xe5, 0x25, 0x1c, 0xad, 0x38, 0xa1, 0xde, 0x6b, 0xf5, 0xb9, 0xca, 0xec, 0x4c, 0x17, 0xe3, 0xbf, 0x77, 0x3, 0x2f}, + expected: []byte{0x1e, 0xdc, 0xc4, 0x69, 0x8f, 0x47, 0xf6, 0x8d, 0xfc, 0x11, 0xec, 0xac, 0xaa, 0x37, 0x4a, 0x3d, 0xbd, 0xfc, 0x1a, 0x9b, 0x6e, 0x87, 0x6f, 0xba, 0xd3, 0x6c, 0x6, 0x6c, 0x9f, 0x5b, 0x65, 0x38}, }, { squareSize: 2, diff --git a/x/payment/types/square_sizes_test.go b/x/payment/types/square_sizes_test.go index 5690279..29397e4 100644 --- a/x/payment/types/square_sizes_test.go +++ b/x/payment/types/square_sizes_test.go @@ -17,22 +17,22 @@ func TestAllSquareSizes(t *testing.T) { {2, []uint64{2, 4, 8, 16, 32, 64, 128}}, {4, []uint64{2, 4, 8, 16, 32, 64, 128}}, {8, []uint64{2, 4, 8, 16, 32, 64, 128}}, - // A square size of 2 has 4 shares. 4 shares * 256 bytes per share = - // 1024 bytes. So a square size of 2 is too small to fit a message of - // size 1024 bytes. - {1024, []uint64{4, 8, 16, 32, 64, 128}}, - // A square size of 4 has 16 shares. 16 shares * 256 bytes per share = - // 4096 bytes. So a square size of 4 is too small to fit a message of - // size 4096 bytes. - {4096, []uint64{8, 16, 32, 64, 128}}, - // A square size of 8 has 64 shares. 64 shares * 256 bytes per share = - // 16384 bytes. So a square size of 4 is too small to fit a message of + // A square size of 2 has 4 shares. 4 shares * 512 bytes per share = + // 2048 bytes. So a square size of 2 is too small to fit a message of + // size 2048 bytes. + {2048, []uint64{4, 8, 16, 32, 64, 128}}, + // A square size of 4 has 16 shares. 16 shares * 512 bytes per share = + // 8192 bytes. So a square size of 4 is too small to fit a message of + // size 8192 bytes. + {8192, []uint64{8, 16, 32, 64, 128}}, + // A square size of 8 has 64 shares. 64 shares * 512 bytes per share = + // 32768 bytes. So a square size of 4 is too small to fit a message of + // size 32768 bytes. + {32768, []uint64{16, 32, 64, 128}}, + // A square size of 128 has 16384 shares. 16384 shares * 512 bytes per share = + // 8388608 bytes. So a square size of 128 is too small to fit a message of // size 16384 bytes. - {16384, []uint64{16, 32, 64, 128}}, - // A square size of 128 has 16384 shares. 16384 shares * 256 bytes per share = - // 4194304 bytes. So a square size of 128 is too small to fit a message of - // size 16384 bytes. - {4194304, []uint64{}}, + {8388608, []uint64{}}, } for _, test := range tests { diff --git a/x/payment/types/wirepayfordata_test.go b/x/payment/types/wirepayfordata_test.go index 41e3741..c484ea3 100644 --- a/x/payment/types/wirepayfordata_test.go +++ b/x/payment/types/wirepayfordata_test.go @@ -45,10 +45,6 @@ func TestWirePayForData_ValidateBasic(t *testing.T) { invalidSquareSizeMsg := validWirePayForData(t) invalidSquareSizeMsg.MessageShareCommitment[0].SquareSize = 15 - // pfd that has a different power of 2 square size - badSquareSizeMsg := validWirePayForData(t) - badSquareSizeMsg.MessageShareCommitment[0].SquareSize = 16 - // pfd that signs over all squares but the first one missingCommitmentForOneSquareSize := validWirePayForData(t) missingCommitmentForOneSquareSize.MessageShareCommitment = missingCommitmentForOneSquareSize.MessageShareCommitment[1:] @@ -88,11 +84,6 @@ func TestWirePayForData_ValidateBasic(t *testing.T) { msg: invalidSquareSizeMsg, wantErr: ErrCommittedSquareSizeNotPowOf2, }, - { - name: "wrong but valid square size", - msg: badSquareSizeMsg, - wantErr: ErrInvalidShareCommit, - }, { name: "parity shares namespace id", msg: paritySharesMsg,