search

celestiaorg / go-square

A library for encoding blobs into a 2D square of evenly sized chunks designed for sampling and reconstruction
Apache License 2.0
13 stars 17 forks source link

refactor: remove compact and spare shares #30

Open rootulp opened 1 year ago

rootulp commented 1 year ago

Context

https://github.com/celestiaorg/celestia-app/pull/1880

Problem

Currently the pkg/shares implementation treats compact and spare shares differently. Instead of describing shares as compact or sparse, one can describe all shares as using the sparse format with the addition that Celestia transaction & PFB transaction shares encode additional metadata in their blobs (in the form of reserved bytes). In this way we could describe a share structure that applies to all shares and a way of encoding blob data (i.e. the current compact share schema) that enables developers to parse shares in the middle of a sequence.

Proposal

Refactor pkg/shares to treat all shares identically. The high level structure may be:

  1. parse(shares) => share sequences
  2. getBlob(shareSequence) => blob
  3. parseCelestiaTransactions(blob) => transactions or PFB transactions
cmwaters commented 1 year ago

I didn't see this issue beforehand but 100% agree with refactoring it to this structure.

My personal preference would be to have another package called aggregate (or combine. I don't really mind the naming) which implements the "compact" part. Thus you would have:

Celestia []Txs -> aggregator -> Blob -> splitter -> []Shares

rootulp commented 3 months ago

I briefly explored this issue and the CompactShareSplitter can't easily be decoupled from share indexes because it is aware of ShareRanges. If we remove the index from PFB txs (CIP-20) then this refactor becomes significantly easier / cleaner.

Separately, we're punting this refactor for now to work on higher priority issues.