Decode returns an error when failed to reconstruct

[rootulp]

Context

If I understand correctly the Codec interface 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

rsmt2d/leopard.go

Lines 52 to 53 in 2557620

	err = enc.Reconstruct(data)
	return data, err

This leads to janky error handling:

rsmt2d/extendeddatacrossword.go

Lines 252 to 257 in 2557620

	rebuiltShares, err := eds.codec.Decode(shares)
	if err != nil {
	// Decode was unsuccessful but don't propagate the error because that
	// would halt the progress of solveCrosswordRow or solveCrosswordCol.
	return nil, false, nil
	}

because reconstruction is expected to fail while solving the extended data crossword iteratively. Since the error that is propagated is defined in klauspost/reedsolomon, in order to check if the error is of type ErrTooFewShards, the extendeddatacrossword.go has to import an error type directly from klauspost/reedsolomon. Such an import would leak implementation details that should ideally be contained to the codec interface in codecs.go and the implementation of that interface in 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.

	// 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.

	// 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)

[rootulp]

Leaving this issue open b/c I think the error returned by Decode leaks implementation details from klauspost/reedsolomon.