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.

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

share merging and splitting #531

Conversation

evan-forbes
Copy link
Member

@evan-forbes evan-forbes commented Sep 17, 2021

Description

still need to figure out when I can merge this portion. I'm also in the process of refactoring/adding #83, and will likely have merge that first. It might also be blocked by #524

part of #528

evan-forbes and others added 12 commits September 16, 2021 14:48
* move ipfs and ipld packages to new pkg/da/

* moved consts to pkg

* clean up
* move Messages field to the end of Block.Data

* Add some constants for share computation and the NMT:

 - also a bunch of todos regarding shares computation

* First (compiling) stab on creating shares

* Test with Evidence and fix bug discovered by test

* remove resolved todos

* introduce split method

* Introduce LenDelimitedMarshaler interface and some reformatting

* Introduce TxLenDelimitedMarshaler

* add some test cases

* fix some comments

* fix some comments & linter

* Add reserved namespaces to params

* Move ll-specific consts into a separate file (consts.go)

* Add MarshalDelimited to HexBytes

* Add tail-padding shares

* Add ComputeShares method on Data to compute all shares

* Fix compute the next square num and not the next power of two

* lints

* Unexport MakeShares function:

- it's likely to change and it doesn't have to be part of the public API

* lints 2

* First stab on computing row/column roots

* fix rebase glitches:
 - move DA related constants out of params.go

* refactor MakeBlock to take in interm. state roots and messages

* refactor state.MakeBlock too

* Add todos LenDelimitedMarshaler and extract appendShares logic

* Simplify shares computation: remove LenDelimitedMarshaler abstraction

* actually use DA header to compute the DataRoot everywhere (will lead to failing tests for sure)

* WIP: Update block related core data structures in protobuf too

* WIP: fix zero shares edge-case and get rid of Block.Data.hash (use dataAvailabilityHeader.Hash() instead)

* Fixed tests, only 3 failing tests to go: TestReapMaxBytesMaxGas, TestTxFilter, TestMempoolFilters

* Fix TestTxFilter:

 - the size of the wrapping Data{} proto message increased a few bytes

* Fix Message proto and `DataFromProto`

* Fix last 2 remaining tests related to the increased block/block.Data size

* Use infectious lib instead of leopard

* proto-lint: snake_case

* some lints and minor changes

* linter

* panic if pushing to tree fails, extend Data.ToProto()

* revert renaming in comment

* add todo about refactoring as soon as the rsmt2d allows the user to choose the merkle tree
 - use the new NMT API where ID and Data are separate fields in the Push method
* Export block data compute shares.
* Refactor to use ShareSize constant directly.
* Change message splitting to prefix namespace ID.
* Implement chunking for contiguous.
* Add termination condition.
* Rename append contiguous to split contiguous.
* Update test for small tx.
* Add test for two contiguous.
* Make tx and msg adjusted share sizes exported constants.
* Panic on hopefully-unreachable condition instead of silently skipping.
* Update hardcoded response for block format.

Co-authored-by: Ismail Khoffi <[email protected]>
* fix overwrite bug and stop splitting shares of size MsgShareSize

* remove ineffectual code

* review feedback: better docs

Co-authored-by: Ismail Khoffi <[email protected]>

* remove uneeded copy and only fix the source of the bug

Co-authored-by: Ismail Khoffi <[email protected]>

* fix overwrite bug while also being consistent with using NamespacedShares

* update to the latest rsmt2d for the nmt wrapper

Co-authored-by: Ismail Khoffi <[email protected]>
* types: erasure namespaces

* use methods instead of fields

* remove unnecessary compteShares call

* test block recovery

* don't mess things up by using power of two

* use better const names

* types: erasure namespaces

* use methods instead of fields

* remove unnecessary compteShares call

* test block recovery

* don't mess things up by using power of two

* use better const names

* remove print statements

* add docs to AdjustedMessageSize

* used the types.AdjustedMessageSize instead of the local

* moving out consts to avoid import cycle

* re applying stashed changes

* return new instances of the wrapper per call of the constructor

* implement tests for recovering data from a square using the new nmt wrapper

* fix sneaky append bug

* remove moved test

* remove uneeded function

* remove comment

* clean up

* fix imports

* linter gods

* more linter fixes

* use appendCopy instead of manually copying

* change name of generateRandomData to generateRandomMsgOnlyData

* use append instead of a utiltiy function
* start spec compliant share merging

* refactor and finish unit testing

* whoops

* linter gods

* fix initial changes and use constants

* use constant

* more polish

* docs fix

Co-authored-by: Ismail Khoffi <[email protected]>

* review feedback: docs and out of range panic protection

* review feedback: add panic protection from empty input

* use constant instead of recalculating `ShareSize`

Co-authored-by: John Adler <[email protected]>

* don't redeclare existing var

Co-authored-by: John Adler <[email protected]>

* be more explicit with returned nil

Co-authored-by: John Adler <[email protected]>

* use constant instead of recalculating `ShareSize`

Co-authored-by: John Adler <[email protected]>

* review feedback: use consistent capitalization

* stop accepting reserved namespaces as normal messages

* use a descriptive var name for message length

* linter and comparison fix

* reorg tests, add test for parse delimiter, DataFromBlock and fix evidence marshal bug

* catch error for linter

* update test MakeShares to include length delimiters for the SHARE_RESERVED_BYTE

* minor iteration change

* refactor share splitting to fix bug

* fix all bugs with third and final refactor

* fix conflict

* revert unnecessary changes

* review feedback: better docs

Co-authored-by: Ismail Khoffi <[email protected]>

* reivew feedback: add comment for safeLen

* review feedback: remove unnecessay comments

* review feedback: split up share merging and splitting into their own files

* review feedback: more descriptive var names

* fix accidental change

* add some constant docs

* spelling error

Co-authored-by: Ismail Khoffi <[email protected]>
Co-authored-by: John Adler <[email protected]>
* unrelated change: update go.mod file

Signed-off-by: Ismail Khoffi <[email protected]>

* Add all non-breaking changes to core types (Block, Header)

- define `DataAvailabilityHeader` and add to `Block`
- comment about `DataHash` (not true yet)
- define `Message` type
- add `IntermediateStateRoots` and `Messages` to Data
- update comments and add a bunch of TODOs

Signed-off-by: Ismail Khoffi <[email protected]>

* Update types/block.go

Co-Authored-By: John Adler <[email protected]>

* consistent json annotation

Signed-off-by: Ismail Khoffi <[email protected]>

Co-authored-by: John Adler <[email protected]>
@evan-forbes evan-forbes self-assigned this Sep 17, 2021
@evan-forbes
Copy link
Member Author

closing in favor of #537

@rootulp rootulp deleted the evan/upgrade-share-merging-and-splitting branch September 22, 2022 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants