celestiaorg / celestiaorg/rsmt2d

Decode returns an error when failed to reconstruct

Open
#200 1 comment 1 reaction 0 assignees View on GitHub
enhancement
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.