-
Notifications
You must be signed in to change notification settings - Fork 975
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
chore!: Refactor RecoverBlob
to RecoverCellsAndProofs
#14160
chore!: Refactor RecoverBlob
to RecoverCellsAndProofs
#14160
Conversation
// Compute cells and proofs. | ||
cells := make([][kzg.CellsPerExtBlob]kzg.Cell, 0, blobsCount) | ||
proofs := make([][kzg.CellsPerExtBlob]kzg.Proof, 0, blobsCount) | ||
|
||
for i := range blobs { | ||
blob := &blobs[i] | ||
blobCells, blobProofs, err := kzg.ComputeCellsAndKZGProofs(blob) | ||
if err != nil { | ||
return nil, errors.Wrap(err, "compute cells and KZG proofs") | ||
} | ||
|
||
cells = append(cells, blobCells) | ||
proofs = append(proofs, blobProofs) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code has now been moved to recoverCellsAndProofs, so instead of computing the cells and proofs here, we now pass them to this method
for rowIndex := 0; rowIndex < blobsCount; rowIndex++ { | ||
cell := cells[rowIndex][columnIndex] | ||
cellsForRow := cellsAndProofs[rowIndex].Cells | ||
proofsForRow := cellsAndProofs[rowIndex].Proofs | ||
|
||
cell := cellsForRow[columnIndex] | ||
column = append(column, cell) | ||
|
||
kzgProof := proofs[rowIndex][columnIndex] | ||
kzgProof := proofsForRow[columnIndex] | ||
kzgProofOfColumn = append(kzgProofOfColumn, kzgProof) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for my understanding; this code is iterating over each rowIndex(blobIndex) and getting the corresponding cells and proofs for that Blob. The code is then using the column index to fetch the cell and proof for that blob that corresponds to the column that we want
blobCells, blobProofs, err := kzg.ComputeCellsAndKZGProofs(&recoveredBlob) | ||
if err != nil { | ||
return nil, errors.Wrapf(err, "compute cells and KZG proofs for blob %d", blobIndex) | ||
} | ||
recoveredCellsAndProofs = append(recoveredCellsAndProofs, kzg.CellsAndProofs{ | ||
Cells: blobCells, | ||
Proofs: blobProofs, | ||
}) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is essentially the code we removed from the DataColumnSideCarsForReconstruct method, ie:
// Compute cells and proofs.
cells := make([][kzg.CellsPerExtBlob]kzg.Cell, 0, blobsCount)
proofs := make([][kzg.CellsPerExtBlob]kzg.Proof, 0, blobsCount)
for i := range blobs {
blob := &blobs[i]
blobCells, blobProofs, err := kzg.ComputeCellsAndKZGProofs(blob)
if err != nil {
return nil, errors.Wrap(err, "compute cells and KZG proofs")
}
cells = append(cells, blobCells)
proofs = append(proofs, blobProofs)
}
Just that we are doing it in this method and passing the cells and proofs we computed, instead of passing the blobs and computing the cells and proofs over there
* change recoverBlobs to recoverCellsAndProofs * modify code to take in the cells and proofs for a particular blob instead of the blob itself * add CellsAndProofs structure * modify recoverCellsAndProofs to return `cellsAndProofs` structure * modify `DataColumnSidecarsForReconstruct` to accept the `cellsAndKZGProofs` structure * bazel run //:gazelle -- fix * use kzg abstraction for kzg method * move CellsAndProofs to kzg.go
* change recoverBlobs to recoverCellsAndProofs * modify code to take in the cells and proofs for a particular blob instead of the blob itself * add CellsAndProofs structure * modify recoverCellsAndProofs to return `cellsAndProofs` structure * modify `DataColumnSidecarsForReconstruct` to accept the `cellsAndKZGProofs` structure * bazel run //:gazelle -- fix * use kzg abstraction for kzg method * move CellsAndProofs to kzg.go
* change recoverBlobs to recoverCellsAndProofs * modify code to take in the cells and proofs for a particular blob instead of the blob itself * add CellsAndProofs structure * modify recoverCellsAndProofs to return `cellsAndProofs` structure * modify `DataColumnSidecarsForReconstruct` to accept the `cellsAndKZGProofs` structure * bazel run //:gazelle -- fix * use kzg abstraction for kzg method * move CellsAndProofs to kzg.go
* change recoverBlobs to recoverCellsAndProofs * modify code to take in the cells and proofs for a particular blob instead of the blob itself * add CellsAndProofs structure * modify recoverCellsAndProofs to return `cellsAndProofs` structure * modify `DataColumnSidecarsForReconstruct` to accept the `cellsAndKZGProofs` structure * bazel run //:gazelle -- fix * use kzg abstraction for kzg method * move CellsAndProofs to kzg.go
* change recoverBlobs to recoverCellsAndProofs * modify code to take in the cells and proofs for a particular blob instead of the blob itself * add CellsAndProofs structure * modify recoverCellsAndProofs to return `cellsAndProofs` structure * modify `DataColumnSidecarsForReconstruct` to accept the `cellsAndKZGProofs` structure * bazel run //:gazelle -- fix * use kzg abstraction for kzg method * move CellsAndProofs to kzg.go
What type of PR is this?
Refactor
What does this PR do? Why is it needed?
The consensus-specs has recently changed to no longer expose recoverAllCells. Instead there is a new method named RecoverCellsAndProofs. This method essentially recovers a blob, then it computes the cells and proofs for it.
In this PR, I have modified the recoverBlob method to recover cells and proofs, so that when we switch to the newer method, we only need to replace the following three calls
RecoverAllCells, CellsToBlob, ComputeCellsAndKZGProofs
withRecoverCellsAndProofs
Here is a reference to that method: https://github.com/ethereum/consensus-specs/blob/dev/specs/_features/eip7594/polynomial-commitments-sampling.md#recover_cells_and_kzg_proofs
This was also brought up in #14136 where we defined the kzg abstraction layer -- This PR is not dependent on that one.
Which issues(s) does this PR fix?
Fixes #
Other notes for review