fix: populate shares in ErrByzantineData

datasquare.go

@@ -307,16 +307,13 @@ func (ds *dataSquare) SetCell(x uint, y uint, newChunk []byte) error {
 	return nil
 }
 
-// setCell sets a specific cell. setCell will overwrite any existing value.
-// Returns an error if the newChunk is not the correct size.
-func (ds *dataSquare) setCell(x uint, y uint, newChunk []byte) error {
-	if len(newChunk) != int(ds.chunkSize) {
-		return fmt.Errorf("cannot set cell with chunk size %d because dataSquare chunk size is %d", len(newChunk), ds.chunkSize)
-	}
+// setCell overwrites the contents of a specific cell. setCell does not perform
+// any input validation so most usecases should use `SetCell` instead of
+// `setCell`. This method exists strictly for testing.
+func (ds *dataSquare) setCell(x uint, y uint, newChunk []byte) {
 	ds.squareRow[x][y] = newChunk
 	ds.squareCol[y][x] = newChunk
 	ds.resetRoots()
-	return nil
 }
 
 // Flattened returns the concatenated rows of the data square.

datasquare_test.go

@@ -95,47 +95,40 @@ func TestSetCell(t *testing.T) {
 	}
 }
 
+// Test_setCell verifies that setCell can overwrite cells without performing any
+// input validation.
 func Test_setCell(t *testing.T) {
 	type testCase struct {
-		name         string
-		originalCell []byte
-		newCell      []byte
-		wantErr      bool
+		name     string
+		original []byte
+		new      []byte
 	}
 
 	testCases := []testCase{
 		{
-			name:         "can set cell if originally nil",
-			originalCell: nil,
-			newCell:      []byte{42},
-			wantErr:      false,
+			name:     "can set cell if originally nil",
+			original: nil,
+			new:      []byte{42},
 		},
 		{
-			name:         "can set cell if originally some value",
-			originalCell: []byte{1},
-			newCell:      []byte{42},
-			wantErr:      false,
+			name:     "can set cell if originally some value",
+			original: []byte{1},
+			new:      []byte{42},
 		},
 		{
-			name:         "expect error if new cell is not the correct chunk size",
-			originalCell: nil,
-			newCell:      []byte{1, 2}, // incorrect chunk size
-			wantErr:      true,
+			name:     "can set cell to nil",
+			original: []byte{1},
+			new:      nil,
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			ds, err := newDataSquare([][]byte{tc.originalCell, {2}, {3}, {4}}, NewDefaultTree)
+			ds, err := newDataSquare([][]byte{tc.original, {2}, {3}, {4}}, NewDefaultTree)
 			assert.NoError(t, err)
 
-			err = ds.setCell(0, 0, tc.newCell)
-			if tc.wantErr {
-				assert.Error(t, err)
-			} else {
-				assert.NoError(t, err)
-				assert.Equal(t, tc.newCell, ds.GetCell(0, 0))
-			}
+			ds.setCell(0, 0, tc.new)
+			assert.Equal(t, tc.new, ds.GetCell(0, 0))
 		})
 	}
 }

extendeddatacrossword.go

@@ -164,6 +164,10 @@ func (eds *ExtendedDataSquare) solveCrosswordRow(
 		if noMissingData(col, r) { // not completed
 			err := eds.verifyAgainstColRoots(colRoots, uint(c), col, r, rebuiltShares[c])
 			if err != nil {
+				var byzErr *ErrByzantineData
+				if errors.As(err, &byzErr) {
+					byzErr.Shares = shares
+				}
 				return false, false, err
 			}
 		}
@@ -228,6 +232,10 @@ func (eds *ExtendedDataSquare) solveCrosswordCol(
 		if noMissingData(row, c) { // not completed
 			err := eds.verifyAgainstRowRoots(rowRoots, uint(r), row, c, rebuiltShares[r])
 			if err != nil {
+				var byzErr *ErrByzantineData
+				if errors.As(err, &byzErr) {
+					byzErr.Shares = shares
+				}
 				return false, false, err
 			}
 		}

extendeddatacrossword_test.go

@@ -196,6 +196,22 @@ func TestCorruptedEdsReturnsErrByzantineData(t *testing.T) {
 			coords: [][]uint{{3, 0}, {0, 1}, {0, 2}, {0, 3}},
 			values: [][]byte{corruptChunk, nil, nil, nil},
 		},
+		{
+			// This test case sets all shares along the diagonal to nil so that
+			// the prerepairSanityCheck does not return an error and it can
+			// verify that solveCrossword returns an ErrByzantineData with
+			// shares populated.
+			name: "set all shares along the diagonal to nil and then corrupt the cell at (0, 1)",
+			// In the ASCII diagram below, _ represents a nil share and C
+			// represents a corrupted share.
+			//
+			// _ C O O
+			// O _ O O
+			// O O _ O
+			// O O O _
+			coords: [][]uint{{0, 0}, {1, 1}, {2, 2}, {3, 3}, {0, 1}},
+			values: [][]byte{nil, nil, nil, nil, corruptChunk},
+		},
 	}
 
 	for codecName, codec := range codecs {
@@ -214,6 +230,8 @@ func TestCorruptedEdsReturnsErrByzantineData(t *testing.T) {
 					// due to parallelisation, the ErrByzantineData axis may be either row or col
 					var byzData *ErrByzantineData
 					assert.ErrorAs(t, err, &byzData, "did not return a ErrByzantineData for a bad col or row")
+					assert.NotEmpty(t, byzData.Shares)
+					assert.Contains(t, byzData.Shares, corruptChunk)
 				})
 			}
 		})