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.

Sign up for GitHub

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

fix: eds mutation #134

Merged
merged 6 commits into from
Jan 22, 2023
Merged

fix: eds mutation #134

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9c6a1a5
fix: avoid mutation of underlying data while avoiding expensive copying
rahulghangas Oct 4, 2022
be186f1
chore: create constant for -1, when no share is to be inserted
rahulghangas Oct 21, 2022
ea8040c
chore: give method argument a more descriptive name
rahulghangas Oct 21, 2022
fea9985
chore: un-expoprt constant for no share insetrtion
rahulghangas Oct 22, 2022
6536df4
Merge branch 'master' into fix/eds-mutation
liamsi Oct 27, 2022
cda0a15
chore: move noShareInsertion to separate const block
rahulghangas Nov 21, 2022
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
68 changes: 50 additions & 18 deletions extendeddatacrossword.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ const (
Col
)

const (
// noShareInsertion indicates that a new share hasn't been inserted in the eds
noShareInsertion = -1
rahulghangas marked this conversation as resolved.
Show resolved Hide resolved
)

func (a Axis) String() string {
switch a {
case Row:
Expand Down Expand Up @@ -117,7 +122,7 @@ func (eds *ExtendedDataSquare) solveCrosswordRow(
rowRoots [][]byte,
colRoots [][]byte,
) (bool, bool, error) {
isComplete := noMissingData(eds.row(uint(r)))
isComplete := noMissingData(eds.row(uint(r)), noShareInsertion)
if isComplete {
return true, false, nil
}
Expand All @@ -140,7 +145,7 @@ func (eds *ExtendedDataSquare) solveCrosswordRow(
}

// Check that rebuilt shares matches appropriate root
err = eds.verifyAgainstRowRoots(rowRoots, uint(r), rebuiltShares)
err = eds.verifyAgainstRowRoots(rowRoots, uint(r), rebuiltShares, noShareInsertion, nil)
liamsi marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
var byzErr *ErrByzantineData
if errors.As(err, &byzErr) {
Expand All @@ -155,9 +160,8 @@ func (eds *ExtendedDataSquare) solveCrosswordRow(
if col[r] != nil {
continue // not newly completed
}
col[r] = rebuiltShares[c]
if noMissingData(col) { // not completed
err := eds.verifyAgainstColRoots(colRoots, uint(c), col)
if noMissingData(col, r) { // not completed
err := eds.verifyAgainstColRoots(colRoots, uint(c), col, r, rebuiltShares[c])
if err != nil {
return false, false, err
}
Expand All @@ -182,7 +186,7 @@ func (eds *ExtendedDataSquare) solveCrosswordCol(
rowRoots [][]byte,
colRoots [][]byte,
) (bool, bool, error) {
isComplete := noMissingData(eds.col(uint(c)))
isComplete := noMissingData(eds.col(uint(c)), noShareInsertion)
if isComplete {
return true, false, nil
}
Expand All @@ -206,7 +210,7 @@ func (eds *ExtendedDataSquare) solveCrosswordCol(
}

// Check that rebuilt shares matches appropriate root
err = eds.verifyAgainstColRoots(colRoots, uint(c), rebuiltShares)
err = eds.verifyAgainstColRoots(colRoots, uint(c), rebuiltShares, noShareInsertion, nil)
if err != nil {
var byzErr *ErrByzantineData
if errors.As(err, &byzErr) {
Expand All @@ -221,9 +225,8 @@ func (eds *ExtendedDataSquare) solveCrosswordCol(
if row[c] != nil {
continue // not newly completed
}
row[c] = rebuiltShares[r]
if noMissingData(row) { // not completed
err := eds.verifyAgainstRowRoots(rowRoots, uint(r), row)
if noMissingData(row, c) { // not completed
err := eds.verifyAgainstRowRoots(rowRoots, uint(r), row, c, rebuiltShares[r])
if err != nil {
return false, false, err
}
Expand Down Expand Up @@ -274,9 +277,16 @@ func (eds *ExtendedDataSquare) rebuildShares(
func (eds *ExtendedDataSquare) verifyAgainstRowRoots(
rowRoots [][]byte,
r uint,
shares [][]byte,
oldShares [][]byte,
rebuiltIndex int,
Copy link
Member

Choose a reason for hiding this comment

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

[code quality, blocking] Honestly I would just make these into new functions, instead of passing in a magic value for branching logic. If rebuiltIndex is noShareInsertion, then rebuiltShare must be nil, but this isn't enforced by the compiler since it's all one function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Separating out leads to a lot of code duplication because noShareInsertion needs to be propagated all the way inside to the last function call. Which is why I included noShareInsertion in the first place

rebuiltShare []byte,
) error {
root := eds.computeSharesRoot(shares, Row, r)
var root []byte
if rebuiltIndex < 0 || rebuiltShare == nil {
liamsi marked this conversation as resolved.
Show resolved Hide resolved
root = eds.computeSharesRoot(oldShares, Row, r)
} else {
root = eds.computeSharesRootWithRebuiltShare(oldShares, Row, r, rebuiltIndex, rebuiltShare)
}

if !bytes.Equal(root, rowRoots[r]) {
return &ErrByzantineData{Row, r, nil}
Expand All @@ -288,9 +298,16 @@ func (eds *ExtendedDataSquare) verifyAgainstRowRoots(
func (eds *ExtendedDataSquare) verifyAgainstColRoots(
colRoots [][]byte,
c uint,
shares [][]byte,
oldShares [][]byte,
rebuiltIndex int,
rebuiltShare []byte,
) error {
root := eds.computeSharesRoot(shares, Col, c)
var root []byte
if rebuiltIndex < 0 || rebuiltShare == nil {
root = eds.computeSharesRoot(oldShares, Col, c)
} else {
root = eds.computeSharesRootWithRebuiltShare(oldShares, Col, c, rebuiltIndex, rebuiltShare)
}

if !bytes.Equal(root, colRoots[c]) {
return &ErrByzantineData{Col, c, nil}
Expand All @@ -307,8 +324,8 @@ func (eds *ExtendedDataSquare) prerepairSanityCheck(

for i := uint(0); i < eds.width; i++ {
i := i
rowIsComplete := noMissingData(eds.row(i))
colIsComplete := noMissingData(eds.col(i))
rowIsComplete := noMissingData(eds.row(i), noShareInsertion)
colIsComplete := noMissingData(eds.col(i), noShareInsertion)

// if there's no missing data in this row
if rowIsComplete {
Expand Down Expand Up @@ -362,8 +379,11 @@ func (eds *ExtendedDataSquare) prerepairSanityCheck(
return errs.Wait()
}

func noMissingData(input [][]byte) bool {
for _, d := range input {
func noMissingData(input [][]byte, rebuiltIndex int) bool {
for index, d := range input {
if index == rebuiltIndex {
continue
}
if d == nil {
return false
}
Expand All @@ -379,6 +399,18 @@ func (eds *ExtendedDataSquare) computeSharesRoot(shares [][]byte, axis Axis, i u
return tree.Root()
}

func (eds *ExtendedDataSquare) computeSharesRootWithRebuiltShare(shares [][]byte, axis Axis, i uint, rebuiltIndex int, rebuiltShare []byte) []byte {
tree := eds.createTreeFn(axis, i)
for _, d := range shares[:rebuiltIndex] {
tree.Push(d)
}
tree.Push(rebuiltShare)
for _, d := range shares[rebuiltIndex+1:] {
tree.Push(d)
}
return tree.Root()
}

func (eds *ExtendedDataSquare) rowRangeNoMissingData(r, start, end uint) bool {
for c := start; c <= end && c < eds.width; c++ {
if eds.squareRow[r][c] == nil {
Expand Down