chore!: Make Cell be a flat sequence of bytes #14159
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Diff is larger than it looks because its built ontop of #14136 I didn't put this change in that PR as that PR was aiming to make no breaking changes and was just applying an abstraction layer, making it easier to review and merge. |
This was referenced Jun 28, 2024
kevaundray
force-pushed
the
kw/make-cell-be-flat-bytes
branch
from
272188d to
ab7c38f
Compare
kevaundray
commented
Jul 3, 2024
Comment on lines
+31
to
+32
| // fieldElementsPerCell is the number of field elements in a single cell. | ||
| const fieldElementsPerCell = ckzg4844.FieldElementsPerCell |
There was a problem hiding this comment.
This is no longer exposed because callers no longer need to flatten/deflatten the cells
kevaundray
commented
Jul 3, 2024
beacon-chain/blockchain/kzg/kzg.go
Outdated
Comment on lines
161
to
177
| // The correct type for Cell is [BytesPerCell]byte | ||
| // c-kzg currently uses [BytesPerFieldElement]Bytes32 | ||
| // so we have these helper methods to convert between the two. | ||
| func cellToChunkedCell(flattened [BytesPerCell]byte) ckzg4844.Cell { | ||
| var cell ckzg4844.Cell | ||
| for i := 0; i < fieldElementsPerCell; i++ { | ||
| copy(cell[i][:], flattened[i*32:(i+1)*32]) | ||
| } | ||
| return cell | ||
| } | ||
| func cellChunkedToCell(cell ckzg4844.Cell) [BytesPerCell]byte { | ||
| var flattened [BytesPerCell]byte | ||
| for i, fieldElement := range cell { | ||
| copy(flattened[i*32:(i+1)*32], fieldElement[:]) | ||
| } | ||
| return flattened | ||
| } |
There was a problem hiding this comment.
Once the libraries are updated, we can remove these methods entirely (newer version of c-kzg has this as a flat sequence of bytes and go-eth-kzg has it as a flat sequence too)
kevaundray
requested review from
prestonvanloon,
saolyn and
potuz
and removed request for
a team
kevaundray
commented
Jul 3, 2024
Comment on lines
-237
to
-245
| cell := column[i] | ||
|
|
||
| cellBytes := make([]byte, 0, kzg.BytesPerCell) | ||
| for _, fieldElement := range cell { | ||
| copiedElem := fieldElement | ||
| cellBytes = append(cellBytes, copiedElem[:]...) | ||
| } | ||
|
|
||
| columnBytes = append(columnBytes, cellBytes) |
There was a problem hiding this comment.
These conversions are now not needed because we get the Cells as a flat sequence of bytes from p2p and we pass it to the kzg package as a flat sequence of bytes, the kzg package also returns it as a flat sequence of bytes
kevaundray
commented
Jul 3, 2024
Comment on lines
-293
to
-298
| for _, ce := range sc.DataColumn { | ||
| var newCell []kzg.Bytes32 | ||
| for i := 0; i < len(ce); i += 32 { | ||
| newCell = append(newCell, kzg.Bytes32(ce[i:i+32])) | ||
| } | ||
| cells = append(cells, kzg.Cell(newCell)) |
There was a problem hiding this comment.
same as https://github.com/prysmaticlabs/prysm/pull/14159/files#r1664747041
We (the caller) no longer needs to convert each field element in the cell into a bytes32 array
kevaundray
commented
Jul 3, 2024
Comment on lines
-60
to
-66
| // Transform the cell as a cKzg cell. | ||
| var ckzgCell kzg.Cell | ||
| for i := 0; i < kzg.FieldElementsPerCell; i++ { | ||
| copy(ckzgCell[i][:], cell[32*i:32*(i+1)]) | ||
| } | ||
|
|
||
| cKzgCells = append(cKzgCells, ckzgCell) |
There was a problem hiding this comment.
same comment as here: https://github.com/prysmaticlabs/prysm/pull/14159/files#r1664747041
nalepae
pushed a commit
that referenced
this pull request
Jul 17, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nalepae
pushed a commit
that referenced
this pull request
Jul 17, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nisdas
pushed a commit
that referenced
this pull request
Jul 18, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nalepae
pushed a commit
that referenced
this pull request
Jul 18, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nalepae
pushed a commit
that referenced
this pull request
Jul 29, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nisdas
pushed a commit
that referenced
this pull request
Aug 14, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nisdas
pushed a commit
that referenced
this pull request
Aug 15, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nisdas
pushed a commit
that referenced
this pull request
Aug 20, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nalepae
pushed a commit
that referenced
this pull request
Aug 27, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nalepae
pushed a commit
that referenced
this pull request
Aug 28, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nalepae
pushed a commit
that referenced
this pull request
Aug 28, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nalepae
pushed a commit
that referenced
this pull request
Oct 7, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nalepae
pushed a commit
that referenced
this pull request
Oct 7, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nalepae
pushed a commit
that referenced
this pull request
Oct 7, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nalepae
pushed a commit
that referenced
this pull request
Oct 8, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nalepae
pushed a commit
that referenced
this pull request
Oct 23, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nalepae
pushed a commit
that referenced
this pull request
Oct 23, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nalepae
pushed a commit
that referenced
this pull request
Oct 24, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nalepae
pushed a commit
that referenced
this pull request
Nov 20, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nalepae
pushed a commit
that referenced
this pull request
Nov 22, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nalepae
pushed a commit
that referenced
this pull request
Nov 22, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nalepae
pushed a commit
that referenced
this pull request
Nov 25, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nalepae
pushed a commit
that referenced
this pull request
Nov 25, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
nalepae
pushed a commit
that referenced
this pull request
Nov 27, 2024
* chore: move all ckzg related functionality into kzg package * refactor code to match * run: bazel run //:gazelle -- fix * chore: add some docs and stop copying large objects when converting between types * fixes * manually add kzg.go dep to Build.Hazel * move kzg methods to kzg.go * chore: add RecoverCellsAndProofs method * bazel run //:gazelle -- fix * make Cells be flattened sequence of bytes * chore: add test for flattening roundtrip * chore: remove code that was doing the flattening outside of the kzg package * fix merge * fix * remove now un-needed conversion * use pointers for Cell parameters * linter * rename cell conversion methods (this only applies to old version of c-kzg)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
Refactor
This PR is dependent on #14136
What does this PR do? Why is it needed?
Cell was previously being defined as an array of Bytes32, which was an abstraction leak from c-kzg. This PR uses the kzg abstraction we defined in #14136 and exposes a Cell type which is a flat sequence of bytes. The flattened and deflattening now happens inside of the kzg package, and other parts of the code now longer needs to handle this case.
Which issues(s) does this PR fix?
Fixes #
Other notes for review