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: support parsing out of context compact shares #1770

Merged
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
18 changes: 14 additions & 4 deletions pkg/shares/compact_shares_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,20 @@ func Test_processCompactShares(t *testing.T) {
}
}

func TestParseOutOfContextShares(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

[optional] in addition to this test, should we consider adding a test case for https://github.com/rootulp/celestia-app/blob/01282c42a8b1d7ea2cfd325c2ed10aed4c317966/pkg/shares/parse.go#L12 that tests out of context shares?

Motivation: parseCompactShares is private and ParseTxs is public.

Copy link
Member Author

Choose a reason for hiding this comment

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

SinceParseTxs uses parseCompactShares under the hood so instead of adding another test, modified this test using ParseTxsto make it simpler.

txs := testfactory.GenerateRandomlySizedTxs(1000, 150)
txShares, _, _, err := SplitTxs(txs)
require.NoError(t, err)

for i := 0; i < 1000; i++ {
start, length := testfactory.GetRandomSlice(len(txShares))

resTxs, err := ParseTxs(txShares[start : start+length])
require.NoError(t, err)
assert.True(t, testfactory.CheckSubArray(txs, resTxs))
evan-forbes marked this conversation as resolved.
Show resolved Hide resolved
}
}

func TestCompactShareContainsInfoByte(t *testing.T) {
css := NewCompactShareSplitter(appns.TxNamespace, appconsts.ShareVersionZero)
txs := testfactory.GenerateRandomTxs(1, appconsts.ContinuationCompactShareContentSize/4)
Expand Down Expand Up @@ -178,10 +192,6 @@ func Test_parseCompactSharesErrors(t *testing.T) {
}

testCases := []testCase{
{
"share with start indicator false",
txShares[1:], // set the first share to the second share which has the start indicator set to false
},
{
"share with unsupported share version",
[]Share{*shareWithUnsupportedShareVersion},
Expand Down
19 changes: 7 additions & 12 deletions pkg/shares/parse_compact_shares.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package shares

import "errors"

// parseCompactShares returns data (transactions or intermediate state roots
// based on the contents of rawShares and supportedShareVersions. If rawShares
// contains a share with a version that isn't present in supportedShareVersions,
Expand All @@ -13,14 +11,6 @@ func parseCompactShares(shares []Share, supportedShareVersions []uint8) (data []
return nil, nil
}

seqStart, err := shares[0].IsSequenceStart()
if err != nil {
return nil, err
}
if !seqStart {
return nil, errors.New("first share is not the start of a sequence")
}

err = validateShareVersions(shares, supportedShareVersions)
if err != nil {
return nil, err
Expand Down Expand Up @@ -61,7 +51,7 @@ func parseRawData(rawData []byte) (units [][]byte, err error) {
if err != nil {
return nil, err
}
if unitLen == 0 {
if unitLen == 0 || unitLen > uint64(len(actualData)) {
rootulp marked this conversation as resolved.
Show resolved Hide resolved
return units, nil
}
rawData = actualData[unitLen:]
Expand All @@ -73,7 +63,12 @@ func parseRawData(rawData []byte) (units [][]byte, err error) {
// not contain the namespace ID, info byte, sequence length, or reserved bytes.
func extractRawData(shares []Share) (rawData []byte, err error) {
for i := 0; i < len(shares); i++ {
raw, err := shares[i].RawData()
var raw []byte
if i == 0 {
raw, err = shares[i].RawDataUsingReserved()
rootulp marked this conversation as resolved.
Show resolved Hide resolved
} else {
raw, err = shares[i].RawData()
}
if err != nil {
return nil, err
}
Expand Down
47 changes: 47 additions & 0 deletions pkg/shares/shares.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,53 @@ func (s *Share) rawDataStartIndex() int {
return index
}

// RawDataWithReserved returns the raw share data while taking reserved bytes into account.
func (s *Share) RawDataUsingReserved() (rawData []byte, err error) {
rawDataStartIndexUsingReserved, err := s.rawDataStartIndexUsingReserved()
if err != nil {
return nil, err
}

// This means share is the last share and does not have any transaction beginning in it
if rawDataStartIndexUsingReserved == 0 {
return []byte{}, nil
}
evan-forbes marked this conversation as resolved.
Show resolved Hide resolved
rootulp marked this conversation as resolved.
Show resolved Hide resolved
if len(s.data) < rawDataStartIndexUsingReserved {
return rawData, fmt.Errorf("share %s is too short to contain raw data", s)
evan-forbes marked this conversation as resolved.
Show resolved Hide resolved
}

return s.data[rawDataStartIndexUsingReserved:], nil
}

func (s *Share) rawDataStartIndexUsingReserved() (int, error) {
evan-forbes marked this conversation as resolved.
Show resolved Hide resolved
isStart, err := s.IsSequenceStart()
if err != nil {
panic(err)
}
isCompact, err := s.IsCompactShare()
if err != nil {
panic(err)
}
evan-forbes marked this conversation as resolved.
Show resolved Hide resolved

index := appconsts.NamespaceSize + appconsts.ShareInfoBytes
if isStart {
index += appconsts.SequenceLenBytes
}

if !isStart && isCompact {
reservedBytes, err := ParseReservedBytes(s.data[index : index+appconsts.CompactShareReservedBytes])
if err != nil {
return 0, err
}
return int(reservedBytes), nil
}

if isCompact {
index += appconsts.CompactShareReservedBytes
}
return index, nil
}

func ToBytes(shares []Share) (bytes [][]byte) {
bytes = make([][]byte, len(shares))
for i, share := range shares {
Expand Down
25 changes: 25 additions & 0 deletions test/util/testfactory/txs.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package testfactory

import (
"bytes"
crand "crypto/rand"
"math/rand"

Expand Down Expand Up @@ -31,3 +32,27 @@ func GenerateRandomTxs(count, size int) types.Txs {
}
return txs
}

func GetRandomSlice(size int) (start int, length int) {
length = rand.Intn(size + 1)
start = rand.Intn(size - length + 1)
return start, length
}
evan-forbes marked this conversation as resolved.
Show resolved Hide resolved

// CheckSubArray returns whether subTxList is a subarray of txList
func CheckSubArray(txList []types.Tx, subTxList []types.Tx) bool {
for i := 0; i <= len(txList)-len(subTxList); i++ {
j := 0
for j = 0; j < len(subTxList); j++ {
tx := txList[i+j]
subTx := subTxList[j]
if !bytes.Equal([]byte(tx), []byte(subTx)) {
break
}
}
if j == len(subTxList) {
return true
}
}
return false
}