From b98f9ab74f6d21e2bf2565d9e53d32958a5cfe0c Mon Sep 17 00:00:00 2001 From: Rootul P Date: Fri, 16 Sep 2022 14:40:45 -0600 Subject: [PATCH] Add `InfoReservedByte` to compact and sparse shares (#691) * Add ParseInfoReservedByte * Update split contiguous shares to account for info byte * Fix TestContigShareWriter test * Improve names in parseDataLength * Remove unused function parseDataLength * revert local celestia-core replace * docs: improve docs * docs: fix doc comments * feat: move share consts from celestia-core * feat: new consts for universal share encoding * remove universal share constants * fix: pass Test_parseSparseShares * Rename appconsts to use `Compact` and `Sparse` terminology (#710) * feat: move share consts from celestia-core * feat: new consts for universal share encoding * remove universal share constants * address @SweeXordious feedback * Revert "Rename appconsts to use `Compact` and `Sparse` terminology (#710)" This reverts commit 75e2b9d6cd071bce0520fba302fab5b5b3b32960. * revert change to exactMsgShareSize * fix: add info byte to namespace padded shares Makes `TestParsePaddedMsg` pass * pass first two test cases of TestMerge * pass: TestMerge * pass: TestTxInclusion by updating proof to use share pkg * pass: TestTxSharePosition * pass: Test_genRowShares * pass: Test_genOrigRowShares * use share package over types * pass: TestPrepareMessagesWithReservedNamespaces * pass: TestCreateCommitment by updating expected bytes * extract doc comments to separate PR * extract separate PR for specifying square size in shares_test.go * test: add TestMsgShareContainsInfoByte * test: add TestContiguousMsgShareContainsInfoByte * test: add compact share tests * throw error if isMessageStart != isNewMessage * throw error if compact share does not start with a share with isMessageStart=true --- app/split_shares.go | 2 +- pkg/appconsts/appconsts.go | 12 ++++++-- pkg/prove/proof_test.go | 2 +- pkg/shares/compact_shares_test.go | 49 +++++++++++++++++++++++++++++- pkg/shares/parse_compact_shares.go | 13 ++++++-- pkg/shares/parse_sparse_shares.go | 18 ++++++++--- pkg/shares/share_splitting.go | 7 ++--- pkg/shares/sparse_shares_test.go | 34 +++++++++++++++++++++ pkg/shares/split_compact_shares.go | 40 ++++++++++++++++++------ pkg/shares/split_sparse_shares.go | 23 +++++++++++--- x/payment/types/payfordata_test.go | 2 +- 11 files changed, 170 insertions(+), 32 deletions(-) diff --git a/app/split_shares.go b/app/split_shares.go index 5db1b322e7..a4a112f80a 100644 --- a/app/split_shares.go +++ b/app/split_shares.go @@ -143,7 +143,7 @@ func newShareSplitter(txConf client.TxConfig, squareSize uint64, data *core.Data panic(err) } - sqwr.txWriter = shares.NewCompactShareSplitter(appconsts.TxNamespaceID) + sqwr.txWriter = shares.NewCompactShareSplitter(appconsts.TxNamespaceID, appconsts.ShareVersion) sqwr.msgWriter = shares.NewSparseShareSplitter() return &sqwr diff --git a/pkg/appconsts/appconsts.go b/pkg/appconsts/appconsts.go index b1e412df85..b17c04e444 100644 --- a/pkg/appconsts/appconsts.go +++ b/pkg/appconsts/appconsts.go @@ -17,6 +17,13 @@ const ( // NamespaceSize is the namespace size in bytes. NamespaceSize = 8 + // ShareInfoBytes is the number of bytes reserved for information. The info + // byte contains the share version and a start idicator. + ShareInfoBytes = 1 + + // ShareVersion is the current version of the share format + ShareVersion = uint8(0) + // See https://github.com/celestiaorg/celestia-app/pull/660#discussion_r958603307 // for the motivation behind `CompactShare` and `SparseShare` terminology. @@ -26,10 +33,11 @@ const ( // CompactShareContentSize is the number of bytes usable for data in a compact // (i.e. transactions, ISRs, evidence) share. - CompactShareContentSize = ShareSize - NamespaceSize - CompactShareReservedBytes + CompactShareContentSize = ShareSize - NamespaceSize - ShareInfoBytes - CompactShareReservedBytes + // SparseShareContentSize is the number of bytes usable for data in a sparse (i.e. // message) share. - SparseShareContentSize = ShareSize - NamespaceSize + SparseShareContentSize = ShareSize - NamespaceSize - ShareInfoBytes // MaxSquareSize is the maximum number of // rows/columns of the original data shares in square layout. diff --git a/pkg/prove/proof_test.go b/pkg/prove/proof_test.go index 92f611094f..495358fe33 100644 --- a/pkg/prove/proof_test.go +++ b/pkg/prove/proof_test.go @@ -192,7 +192,7 @@ func joinByteSlices(s ...[]byte) string { out := make([]string, len(s)) for i, sl := range s { sl, _, _ := shares.ParseDelimiter(sl) - out[i] = string(sl[appconsts.NamespaceSize:]) + out[i] = string(sl[appconsts.NamespaceSize+appconsts.ShareInfoBytes:]) } return strings.Join(out, "") } diff --git a/pkg/shares/compact_shares_test.go b/pkg/shares/compact_shares_test.go index 14c2005570..659d78298a 100644 --- a/pkg/shares/compact_shares_test.go +++ b/pkg/shares/compact_shares_test.go @@ -15,7 +15,7 @@ import ( func TestCompactShareWriter(t *testing.T) { // note that this test is mainly for debugging purposes, the main round trip // tests occur in TestMerge and Test_processCompactShares - w := NewCompactShareSplitter(appconsts.TxNamespaceID) + w := NewCompactShareSplitter(appconsts.TxNamespaceID, appconsts.ShareVersion) txs := generateRandomCompactShares(33, 200) for _, tx := range txs { rawTx, _ := MarshalDelimitedTx(tx) @@ -122,3 +122,50 @@ func Test_processCompactShares(t *testing.T) { }) } } + +func TestCompactShareContainsInfoByte(t *testing.T) { + css := NewCompactShareSplitter(appconsts.TxNamespaceID, appconsts.ShareVersion) + txs := generateRandomCompactShares(1, 100) + + for _, tx := range txs { + css.WriteTx(tx) + } + + shares := css.Export().RawShares() + assert.Condition(t, func() bool { return len(shares) == 1 }) + + infoByte := shares[0][appconsts.NamespaceSize : appconsts.NamespaceSize+appconsts.ShareInfoBytes][0] + + isMessageStart := true + want, err := NewInfoReservedByte(appconsts.ShareVersion, isMessageStart) + + require.NoError(t, err) + assert.Equal(t, byte(want), infoByte) +} + +func TestContiguousCompactShareContainsInfoByte(t *testing.T) { + css := NewCompactShareSplitter(appconsts.TxNamespaceID, appconsts.ShareVersion) + txs := generateRandomCompactShares(1, 1000) + + for _, tx := range txs { + css.WriteTx(tx) + } + + shares := css.Export().RawShares() + assert.Condition(t, func() bool { return len(shares) > 1 }) + + infoByte := shares[1][appconsts.NamespaceSize : appconsts.NamespaceSize+appconsts.ShareInfoBytes][0] + + isMessageStart := false + want, err := NewInfoReservedByte(appconsts.ShareVersion, isMessageStart) + + require.NoError(t, err) + assert.Equal(t, byte(want), infoByte) +} + +func Test_parseCompactSharesReturnsErrForShareWithStartIndicatorFalse(t *testing.T) { + txs := generateRandomCompactShares(2, 1000) + shares := SplitTxs(txs) + _, err := parseCompactShares(shares[1:]) // the second share has the message start indicator set to false + assert.Error(t, err) +} diff --git a/pkg/shares/parse_compact_shares.go b/pkg/shares/parse_compact_shares.go index 980b6781cb..674758e611 100644 --- a/pkg/shares/parse_compact_shares.go +++ b/pkg/shares/parse_compact_shares.go @@ -9,7 +9,7 @@ import ( // parseCompactShares takes raw shares and extracts out transactions, // intermediate state roots, or evidence. The returned [][]byte do not have -// namespaces or length delimiters and are ready to be unmarshalled +// namespaces, info bytes, or length delimiters and are ready to be unmarshalled func parseCompactShares(shares [][]byte) (data [][]byte, err error) { if len(shares) == 0 { return nil, nil @@ -37,7 +37,14 @@ func (ss *shareStack) resolve() ([][]byte, error) { if len(ss.shares) == 0 { return nil, nil } - err := ss.peel(ss.shares[0][appconsts.NamespaceSize+appconsts.CompactShareReservedBytes:], true) + infoByte, err := ParseInfoReservedByte(ss.shares[0][appconsts.NamespaceSize : appconsts.NamespaceSize+appconsts.ShareInfoBytes][0]) + if err != nil { + panic(err) + } + if !infoByte.IsMessageStart() { + return nil, errors.New("first share is not a message start") + } + err = ss.peel(ss.shares[0][appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.CompactShareReservedBytes:], true) return ss.data, err } @@ -70,7 +77,7 @@ func (ss *shareStack) peel(share []byte, delimited bool) (err error) { // add the next share to the current share to continue merging if possible if len(ss.shares) > ss.cursor+1 { ss.cursor++ - share := append(share, ss.shares[ss.cursor][appconsts.NamespaceSize+appconsts.CompactShareReservedBytes:]...) + share := append(share, ss.shares[ss.cursor][appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.CompactShareReservedBytes:]...) return ss.peel(share, false) } // collect any remaining data diff --git a/pkg/shares/parse_sparse_shares.go b/pkg/shares/parse_sparse_shares.go index d24672450e..d0133d2ae2 100644 --- a/pkg/shares/parse_sparse_shares.go +++ b/pkg/shares/parse_sparse_shares.go @@ -3,6 +3,7 @@ package shares import ( "bytes" "encoding/binary" + "fmt" coretypes "github.com/tendermint/tendermint/types" @@ -34,16 +35,23 @@ func parseSparseShares(shares [][]byte) ([]coretypes.Message, error) { dataLen = len(currentMsg.Data) + appconsts.SparseShareContentSize switch { case isNewMessage: - nextMsgChunk, nextMsgLen, err := ParseDelimiter(shares[i][appconsts.NamespaceSize:]) + nextMsgChunk, nextMsgLen, err := ParseDelimiter(shares[i][appconsts.NamespaceSize+appconsts.ShareInfoBytes:]) if err != nil { return nil, err } // the current share is namespaced padding so we ignore it - if bytes.Equal(shares[i][appconsts.NamespaceSize:], appconsts.NameSpacedPaddedShareBytes) { + if bytes.Equal(shares[i][appconsts.NamespaceSize+appconsts.ShareInfoBytes:], appconsts.NameSpacedPaddedShareBytes) { continue } currentMsgLen = int(nextMsgLen) nid := shares[i][:appconsts.NamespaceSize] + infoByte, err := ParseInfoReservedByte(shares[i][appconsts.NamespaceSize : appconsts.NamespaceSize+appconsts.ShareInfoBytes][0]) + if err != nil { + panic(err) + } + if infoByte.IsMessageStart() != isNewMessage { + return nil, fmt.Errorf("expected message start indicator to be %t but got %t", isNewMessage, infoByte.IsMessageStart()) + } currentMsg = coretypes.Message{ NamespaceID: nid, Data: nextMsgChunk, @@ -58,12 +66,12 @@ func parseSparseShares(shares [][]byte) ([]coretypes.Message, error) { isNewMessage = false // this entire share contains a chunk of message that we need to save case currentMsgLen > dataLen: - currentMsg.Data = append(currentMsg.Data, shares[i][appconsts.NamespaceSize:]...) + currentMsg.Data = append(currentMsg.Data, shares[i][appconsts.NamespaceSize+appconsts.ShareInfoBytes:]...) // this share contains the last chunk of data needed to complete the // message case currentMsgLen <= dataLen: - remaining := currentMsgLen - len(currentMsg.Data) + appconsts.NamespaceSize - currentMsg.Data = append(currentMsg.Data, shares[i][appconsts.NamespaceSize:remaining]...) + remaining := currentMsgLen - len(currentMsg.Data) + appconsts.NamespaceSize + appconsts.ShareInfoBytes + currentMsg.Data = append(currentMsg.Data, shares[i][appconsts.NamespaceSize+appconsts.ShareInfoBytes:remaining]...) saveMessage() } } diff --git a/pkg/shares/share_splitting.go b/pkg/shares/share_splitting.go index eb48467619..9881445c04 100644 --- a/pkg/shares/share_splitting.go +++ b/pkg/shares/share_splitting.go @@ -86,7 +86,7 @@ func ExtractShareIndexes(txs coretypes.Txs) []uint32 { } func SplitTxs(txs coretypes.Txs) [][]byte { - writer := NewCompactShareSplitter(appconsts.TxNamespaceID) + writer := NewCompactShareSplitter(appconsts.TxNamespaceID, appconsts.ShareVersion) for _, tx := range txs { writer.WriteTx(tx) } @@ -94,10 +94,9 @@ func SplitTxs(txs coretypes.Txs) [][]byte { } func SplitEvidence(evd coretypes.EvidenceList) ([][]byte, error) { - writer := NewCompactShareSplitter(appconsts.EvidenceNamespaceID) - var err error + writer := NewCompactShareSplitter(appconsts.EvidenceNamespaceID, appconsts.ShareVersion) for _, ev := range evd { - err = writer.WriteEvidence(ev) + err := writer.WriteEvidence(ev) if err != nil { return nil, err } diff --git a/pkg/shares/sparse_shares_test.go b/pkg/shares/sparse_shares_test.go index 8f02f25cdd..a1c652f057 100644 --- a/pkg/shares/sparse_shares_test.go +++ b/pkg/shares/sparse_shares_test.go @@ -98,3 +98,37 @@ func TestParsePaddedMsg(t *testing.T) { require.NoError(t, err) require.Equal(t, msgs.MessagesList, pmsgs) } + +func TestMsgShareContainsInfoByte(t *testing.T) { + sss := NewSparseShareSplitter() + smallMsg := generateRandomMessage(100) + sss.Write(smallMsg) + + shares := sss.Export().RawShares() + + got := shares[0][appconsts.NamespaceSize : appconsts.NamespaceSize+appconsts.ShareInfoBytes][0] + + isMessageStart := true + want, err := NewInfoReservedByte(appconsts.ShareVersion, isMessageStart) + + require.NoError(t, err) + assert.Equal(t, byte(want), got) +} + +func TestContiguousMsgShareContainsInfoByte(t *testing.T) { + sss := NewSparseShareSplitter() + longMsg := generateRandomMessage(1000) + sss.Write(longMsg) + + shares := sss.Export().RawShares() + + // we expect longMsg to occupy more than one share + assert.Condition(t, func() bool { return len(shares) > 1 }) + got := shares[1][appconsts.NamespaceSize : appconsts.NamespaceSize+appconsts.ShareInfoBytes][0] + + isMessageStart := false + want, err := NewInfoReservedByte(appconsts.ShareVersion, isMessageStart) + + require.NoError(t, err) + assert.Equal(t, byte(want), got) +} diff --git a/pkg/shares/split_compact_shares.go b/pkg/shares/split_compact_shares.go index a4c7421d12..8ea8a9f3a2 100644 --- a/pkg/shares/split_compact_shares.go +++ b/pkg/shares/split_compact_shares.go @@ -17,13 +17,19 @@ type CompactShareSplitter struct { shares []NamespacedShare pendingShare NamespacedShare namespace namespace.ID + version uint8 } // NewCompactShareSplitter returns a CompactShareSplitter using the provided // namespace. -func NewCompactShareSplitter(ns namespace.ID) *CompactShareSplitter { +func NewCompactShareSplitter(ns namespace.ID, version uint8) *CompactShareSplitter { pendingShare := NamespacedShare{ID: ns, Share: make([]byte, 0, appconsts.ShareSize)} + infoByte, err := NewInfoReservedByte(version, true) + if err != nil { + panic(err) + } pendingShare.Share = append(pendingShare.Share, ns...) + pendingShare.Share = append(pendingShare.Share, byte(infoByte)) return &CompactShareSplitter{pendingShare: pendingShare, namespace: ns} } @@ -52,7 +58,7 @@ func (css *CompactShareSplitter) WriteEvidence(evd coretypes.Evidence) error { func (css *CompactShareSplitter) WriteBytes(rawData []byte) { // if this is the first time writing to a pending share, we must add the // reserved bytes - if len(css.pendingShare.Share) == appconsts.NamespaceSize { + if len(css.pendingShare.Share) == appconsts.NamespaceSize+appconsts.ShareInfoBytes { css.pendingShare.Share = append(css.pendingShare.Share, 0) } @@ -79,7 +85,7 @@ func (css *CompactShareSplitter) WriteBytes(rawData []byte) { txCursor = len(rawData) // add the share reserved bytes to the new pending share - pendingCursor := len(rawData) + appconsts.NamespaceSize + appconsts.CompactShareReservedBytes + pendingCursor := len(rawData) + appconsts.NamespaceSize + appconsts.ShareInfoBytes + appconsts.CompactShareReservedBytes var reservedByte byte if pendingCursor >= appconsts.ShareSize { // the share reserve byte is zero when some compactly written @@ -106,6 +112,11 @@ func (css *CompactShareSplitter) stackPending() { css.shares = append(css.shares, css.pendingShare) newPendingShare := make([]byte, 0, appconsts.ShareSize) newPendingShare = append(newPendingShare, css.namespace...) + infoByte, err := NewInfoReservedByte(css.version, false) + if err != nil { + panic(err) + } + newPendingShare = append(newPendingShare, byte(infoByte)) css.pendingShare = NamespacedShare{ Share: newPendingShare, ID: css.namespace, @@ -115,7 +126,7 @@ func (css *CompactShareSplitter) stackPending() { // Export finalizes and returns the underlying compact shares. func (css *CompactShareSplitter) Export() NamespacedShares { // add the pending share to the current shares before returning - if len(css.pendingShare.Share) > appconsts.NamespaceSize { + if len(css.pendingShare.Share) > appconsts.NamespaceSize+appconsts.ShareInfoBytes { css.pendingShare.Share = zeroPadIfNecessary(css.pendingShare.Share, appconsts.ShareSize) css.shares = append(css.shares, css.pendingShare) } @@ -131,7 +142,7 @@ func (css *CompactShareSplitter) Export() NamespacedShares { // confusion for light clients parsing these shares, as the rest of the // data after transaction is padding. See // https://github.com/celestiaorg/celestia-specs/blob/master/src/specs/data_structures.md#share - rawLastShare[appconsts.NamespaceSize+i] = byte(0) + rawLastShare[appconsts.NamespaceSize+appconsts.ShareInfoBytes+i] = byte(0) } newLastShare := NamespacedShare{ @@ -145,18 +156,22 @@ func (css *CompactShareSplitter) Export() NamespacedShares { // Count returns the current number of shares that will be made if exporting and // the number of availableBytes in the last pending share. func (css *CompactShareSplitter) Count() (count, availableBytes int) { - if len(css.pendingShare.Share) > appconsts.NamespaceSize { + if len(css.pendingShare.Share) > appconsts.NamespaceSize+appconsts.ShareInfoBytes { return len(css.shares), 0 } - availableBytes = appconsts.CompactShareContentSize - (len(css.pendingShare.Share) - appconsts.NamespaceSize) + // this doesn't account for the size of the reserved byte + availableBytes = appconsts.CompactShareContentSize - (len(css.pendingShare.Share) - appconsts.NamespaceSize - appconsts.ShareInfoBytes) return len(css.shares), availableBytes } +var tailPaddingInfo, _ = NewInfoReservedByte(appconsts.ShareVersion, false) + // tail is filler for all tail padded shares // it is allocated once and used everywhere -var tailPaddingShare = append( +var tailPaddingShare = append(append( append(make([]byte, 0, appconsts.ShareSize), appconsts.TailPaddingNamespaceID...), - bytes.Repeat([]byte{0}, appconsts.ShareSize-appconsts.NamespaceSize)..., + byte(tailPaddingInfo)), + bytes.Repeat([]byte{0}, appconsts.ShareSize-appconsts.NamespaceSize-appconsts.ShareInfoBytes)..., ) // TailPaddingShares creates n tail padding shares. @@ -172,11 +187,16 @@ func TailPaddingShares(n int) NamespacedShares { } func namespacedPaddedShares(ns []byte, count int) []NamespacedShare { + infoByte, err := NewInfoReservedByte(appconsts.ShareVersion, true) + if err != nil { + panic(err) + } shares := make([]NamespacedShare, count) for i := 0; i < count; i++ { shares[i] = NamespacedShare{ - Share: append(append( + Share: append(append(append( make([]byte, 0, appconsts.ShareSize), ns...), + byte(infoByte)), make([]byte, appconsts.SparseShareContentSize)...), ID: ns, } diff --git a/pkg/shares/split_sparse_shares.go b/pkg/shares/split_sparse_shares.go index fc83f38c8d..29a2425d4b 100644 --- a/pkg/shares/split_sparse_shares.go +++ b/pkg/shares/split_sparse_shares.go @@ -93,9 +93,14 @@ func (sss *SparseShareSplitter) Count() int { // Used for messages. func AppendToShares(shares []NamespacedShare, nid namespace.ID, rawData []byte) []NamespacedShare { if len(rawData) <= appconsts.SparseShareContentSize { - rawShare := append(append( - make([]byte, 0, len(nid)+len(rawData)), + infoByte, err := NewInfoReservedByte(appconsts.ShareVersion, true) + if err != nil { + panic(err) + } + rawShare := append(append(append( + make([]byte, 0, appconsts.ShareSize), nid...), + byte(infoByte)), rawData..., ) paddedShare := zeroPadIfNecessary(rawShare, appconsts.ShareSize) @@ -111,18 +116,28 @@ func AppendToShares(shares []NamespacedShare, nid namespace.ID, rawData []byte) // namespaced shares func splitMessage(rawData []byte, nid namespace.ID) NamespacedShares { shares := make([]NamespacedShare, 0) - firstRawShare := append(append( + infoByte, err := NewInfoReservedByte(appconsts.ShareVersion, true) + if err != nil { + panic(err) + } + firstRawShare := append(append(append( make([]byte, 0, appconsts.ShareSize), nid...), + byte(infoByte)), rawData[:appconsts.SparseShareContentSize]..., ) shares = append(shares, NamespacedShare{firstRawShare, nid}) rawData = rawData[appconsts.SparseShareContentSize:] for len(rawData) > 0 { shareSizeOrLen := min(appconsts.SparseShareContentSize, len(rawData)) - rawShare := append(append( + infoByte, err := NewInfoReservedByte(appconsts.ShareVersion, false) + if err != nil { + panic(err) + } + rawShare := append(append(append( make([]byte, 0, appconsts.ShareSize), nid...), + byte(infoByte)), rawData[:shareSizeOrLen]..., ) paddedShare := zeroPadIfNecessary(rawShare, appconsts.ShareSize) diff --git a/x/payment/types/payfordata_test.go b/x/payment/types/payfordata_test.go index 9aaa35db0d..e350ea2ce9 100644 --- a/x/payment/types/payfordata_test.go +++ b/x/payment/types/payfordata_test.go @@ -166,7 +166,7 @@ func TestCreateCommitment(t *testing.T) { squareSize: 4, namespace: bytes.Repeat([]byte{0xFF}, 8), message: bytes.Repeat([]byte{0xFF}, 11*ShareSize), - expected: []byte{0xe3, 0xaa, 0xdb, 0x1c, 0x2e, 0xc3, 0x77, 0x4a, 0x46, 0xb6, 0xb2, 0xf0, 0x4f, 0x6b, 0x3d, 0x4f, 0x17, 0x99, 0x82, 0x6b, 0xd6, 0xd4, 0x40, 0x5e, 0xbe, 0x1b, 0x1f, 0x29, 0x5a, 0x53, 0x3f, 0xbb}, + expected: []byte{0x44, 0x7e, 0xa2, 0xf4, 0xee, 0xb, 0xad, 0x9d, 0x7f, 0xfb, 0x5e, 0x9e, 0xc6, 0xd4, 0xf6, 0x70, 0x4f, 0x36, 0x83, 0x1a, 0x58, 0xe, 0x13, 0xd8, 0x5a, 0x9d, 0x43, 0x11, 0x6a, 0x5f, 0xdd, 0xe1}, }, { squareSize: 2,