celestiaorg / celestiaorg/rsmt2d
Decode returns an error when failed to reconstruct
- Dominant language
- Go
- Stars
- 169
- Forks
- 87
- Avg merge
- 8h 9m
- Merged PRs (30d)
- 7
Description
## Context
If I understand correctly the [Codec interface](https://github.com/celestiaorg/rsmt2d/blob/25576208f90d3d171e5cbf7407704c1d61ae6bf1/codecs.go#L14-L27) is meant to serve as a boundary between rsmt2d logic and different erasure coding libraries.
## Problem
The leopard codec returns an error when it fails to reconstruct all shards
https://github.com/celestiaorg/rsmt2d/blob/25576208f90d3d171e5cbf7407704c1d61ae6bf1/leopard.go#L52-L53
This leads to janky error handling: https://github.com/celestiaorg/rsmt2d/blob/25576208f90d3d171e5cbf7407704c1d61ae6bf1/extendeddatacrossword.go#L252-L257
because reconstruction is expected to fail while solving the extended data crossword iteratively. Since the error that is propagated is defined in [klauspost/reedsolomon](https://github.com/klauspost/reedsolomon/blob/523164698be98f1603cf1235f5a1de17728b2091/reedsolomon.go#L604-L607), in order to check if the error is of type `ErrTooFewShards`, the [extendeddatacrossword.go](https://github.com/celestiaorg/rsmt2d/blob/master/extendeddatacrossword.go) has to import an error type directly from [klauspost/reedsolomon](https://github.com/klauspost/reedsolomon/blob/523164698be98f1603cf1235f5a1de17728b2091/reedsolomon.go#L604-L607). Such an import would leak implementation details that should ideally be contained to the codec interface in [codecs.go](https://github.com/celestiaorg/rsmt2d/blob/25576208f90d3d171e5cbf7407704c1d61ae6bf1/codecs.go) and the implementation of that interface in [leopard.go](https://github.com/celestiaorg/rsmt2d/blob/25576208f90d3d171e5cbf7407704c1d61ae6bf1/leopard.go)
## Proposal
### Option A
Define a new error in codecs.go like `rsmt2d.ErrTooFewShards`. In leopard.go if a `reedsolomon.ErrTooFewShards` is observed, return a new `rsmt2d.ErrTooFewShards` instead.
```go
// Decode attempts to reconstruct the missing shards in data. The `data`
// parameter should contain all original + parity shards where missing shards
// should be `nil`. If reconstruction is successful, the original + parity
// shards are returned. If reconstruction is unsuccessful, an error is returned.
Decode(data [][]byte) ([][]byte, error)
```
### Option B
In leopard.go if a `reedsolomon.ErrTooFewShards` is observed don't propagate it. Instead, return all the shards with missing shards still set to `nil`.
```go
// Decode attempts to reconstruct the missing shards in data. The `data`
// parameter should contain all original + parity shards where missing shards
// should be `nil`. If reconstruction is successful, the original + parity
// shards are returned. If reconstruction is unsuccessful, the parameter data
// is returned unmodified.
Decode(data [][]byte) ([][]byte, error)
```
Contributor guide
Research direction
Start with the Codec interface in codecs.go, the Decode implementation in leopard.go, and the reconstruction handling in extendeddatacrossword.go. Compare the two proposed error-contract options and check how reconstruction failures flow through the crossword; done means the contract is decided and extendeddatacrossword.go no longer depends on klauspost/reedsolomon error types.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- distributed-systems
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100