Skip to content

Commit

Permalink
fix: reserved byte for existing test cases (#821)
Browse files Browse the repository at this point in the history 
Closes #817
  • Loading branch information
@rootulp
rootulp authored Sep 30, 2022
1 parent abb9829 commit 56e96cf
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 32 deletions.
37 changes: 29 additions & 8 deletions pkg/shares/share_splitting_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func TestSplitTxs(t *testing.T) {
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
0x0, // BUG: reserved byte should be non-zero
14, // reserved byte
0x1, // unit length of first transaction
0xa, // data of first transaction
}, bytes.Repeat([]byte{0}, 240)...), // padding
Expand All @@ -42,7 +42,7 @@ func TestSplitTxs(t *testing.T) {
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
0x0, // BUG: reserved byte should be non-zero
14, // reserved byte
0x1, // unit length of first transaction
0xa, // data of first transaction
0x1, // unit length of second transaction
Expand All @@ -58,13 +58,13 @@ func TestSplitTxs(t *testing.T) {
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
0x0, // BUG: reserved byte should be non-zero
14, // reserved byte
241, 1, // unit length of first transaction is 241
}, bytes.Repeat([]byte{0xc}, 240)...), // data of first transaction
append([]uint8{
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, // namespace id
0x0, // info byte
0xb, // BUG: reserved byte should be zero
0x0, // reserved byte
0xc, // continuation data of first transaction
}, bytes.Repeat([]byte{0x0}, 245)...), // padding
},
Expand All @@ -77,26 +77,47 @@ func TestSplitTxs(t *testing.T) {
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
0x0, // BUG: reserved byte should be non-zero
14, // reserved byte
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
0xd, // BUG: reserved byte should be zero
0x0, // reserved byte
0xe, 0xe, 0xe, // continuation data of second transaction
}, bytes.Repeat([]byte{0x0}, 243)...), // padding
},
},
{
name: "one large tx that spans two shares then one small tx",
txs: coretypes.Txs{bytes.Repeat([]byte{0xe}, 241), coretypes.Tx{0xd}},
want: [][]uint8{
append([]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{
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, // namespace id
0x0, // info byte
11, // reserved byte
0xe, // continuation data of first transaction
1, // unit length of second transaction
0xd, // data of second transaction
}, bytes.Repeat([]byte{0x0}, 243)...), // padding
},
},
}
for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) {
got := SplitTxs(tt.txs)
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("SplitTxs(%#v) got %#v, want %#v", tt.txs, got, tt.want)
t.Errorf("SplitTxs()\n got %#v\n want %#v", got, tt.want)
}
})
}
}
} //
66 changes: 42 additions & 24 deletions pkg/shares/split_compact_shares.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,12 @@ func NewCompactShareSplitter(ns namespace.ID, version uint8) *CompactShareSplitt
panic(err)
}
placeholderDataLength := make([]byte, appconsts.FirstCompactShareDataLengthBytes)
placeholderReservedBytes := make([]byte, appconsts.CompactShareReservedBytes)

pendingShare.Share = append(pendingShare.Share, ns...)
pendingShare.Share = append(pendingShare.Share, byte(infoByte))
pendingShare.Share = append(pendingShare.Share, placeholderDataLength...)
pendingShare.Share = append(pendingShare.Share, placeholderReservedBytes...)
return &CompactShareSplitter{pendingShare: pendingShare, namespace: ns}
}

Expand All @@ -59,12 +62,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) {
// if this is the first time writing to a pending share, we must add the
// reserved bytes
if css.isEmptyPendingShare() {
reservedBytes := make([]byte, appconsts.CompactShareReservedBytes)
css.pendingShare.Share = append(css.pendingShare.Share, reservedBytes...)
}
css.maybeWriteReservedByteToPendingShare()

txCursor := len(rawData)
for txCursor != 0 {
Expand All @@ -87,22 +85,6 @@ func (css *CompactShareSplitter) WriteBytes(rawData []byte) {
// update the cursor
rawData = rawData[pendingLeft:]
txCursor = len(rawData)

// add the share reserved bytes to the new pending share
pendingCursor := len(rawData) + appconsts.NamespaceSize + appconsts.ShareInfoBytes + appconsts.CompactShareReservedBytes
reservedBytes := make([]byte, appconsts.CompactShareReservedBytes)
if pendingCursor >= appconsts.ShareSize {
// the share reserve byte is zero when some compactly written
// data takes up the entire share
for i := range reservedBytes {
reservedBytes[i] = byte(0)
}
} else {
// TODO this must be changed when share size is increased to 512
reservedBytes[0] = byte(pendingCursor)
}

css.pendingShare.Share = append(css.pendingShare.Share, reservedBytes...)
}

// if the share is exactly the correct size, then append to shares
Expand All @@ -123,7 +105,9 @@ func (css *CompactShareSplitter) stackPending() {
if err != nil {
panic(err)
}
placeholderReservedBytes := make([]byte, appconsts.CompactShareReservedBytes)
newPendingShare = append(newPendingShare, byte(infoByte))
newPendingShare = append(newPendingShare, placeholderReservedBytes...)
css.pendingShare = NamespacedShare{
Share: newPendingShare,
ID: css.namespace,
Expand Down Expand Up @@ -183,6 +167,40 @@ func (css *CompactShareSplitter) writeDataLengthVarintToFirstShare(dataLengthVar
css.shares[0] = newFirstShare
}

// 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() {
return
}

locationOfNextUnit := len(css.pendingShare.Share)
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))
}

// write the location of next unit to the reserved byte of the pending share
if css.isPendingShareTheFirstShare() {
css.pendingShare.Share[appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareDataLengthBytes : appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareContentSize+appconsts.CompactShareReservedBytes][0] = byte(locationOfNextUnit)
} else {
css.pendingShare.Share[appconsts.NamespaceSize+appconsts.ShareInfoBytes : appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.CompactShareReservedBytes][0] = byte(locationOfNextUnit)
}
}

// isEmptyReservedByte returns true if the reserved byte is empty.
func (css *CompactShareSplitter) isEmptyReservedByte() bool {
var reservedByte byte

if css.isPendingShareTheFirstShare() {
reservedByte = css.pendingShare.Share[appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareDataLengthBytes : appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareContentSize+appconsts.CompactShareReservedBytes][0]
} else {
reservedByte = css.pendingShare.Share[appconsts.NamespaceSize+appconsts.ShareInfoBytes : appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.CompactShareReservedBytes][0]
}

return reservedByte == 0
}

// dataLength returns the total length in bytes of all units (transactions,
// intermediate state roots, or evidence) written to this splitter.
// dataLength does not include the # of bytes occupied by the namespace ID or
Expand All @@ -204,9 +222,9 @@ func (css *CompactShareSplitter) dataLength(bytesOfPadding int) uint64 {
// isEmptyPendingShare returns true if the pending share is empty, false otherwise.
func (css *CompactShareSplitter) isEmptyPendingShare() bool {
if css.isPendingShareTheFirstShare() {
return len(css.pendingShare.Share) == appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareDataLengthBytes
return len(css.pendingShare.Share) == appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.FirstCompactShareDataLengthBytes+appconsts.CompactShareReservedBytes
}
return len(css.pendingShare.Share) == appconsts.NamespaceSize+appconsts.ShareInfoBytes
return len(css.pendingShare.Share) == appconsts.NamespaceSize+appconsts.ShareInfoBytes+appconsts.CompactShareReservedBytes
}

// isPendingShareTheFirstShare returns true if the pending share is the first
Expand Down

0 comments on commit 56e96cf

Please sign in to comment.