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 16 forks source link

bug: worst case share indexes uses max square size #47

Closed rootulp closed 5 months ago

rootulp commented 6 months ago

Context

https://github.com/celestiaorg/go-square/blob/a84173978ed29049469dc5e84b4a58cfece98401/square/builder.go#L414

Problem

We want worstCaseShareIndexes to always use the upper bound for max square size (128) so that celestia-node doesn't have to be stateful when extending the ODS.

If worstCaseShareIndexes uses the max effective square size (64) then celestia-node would have to poll celestia-app for the gov max square size every block.

Also note that celestia-app v1.x used a worstCaseShareIndexes of 128 so it was a consensus breaking change to drop down to 64.

Proposal

Modify the second argument to this function to use the upper bound of 128

rootulp commented 6 months ago

FWIW I think this issue has existed since go-square was extracted from celestia-app. See https://github.com/celestiaorg/go-square/commit/68863fc876b175343fe68e21467c70652e629280#diff-f37b1db98350d69502a30f6f3dd88ae2f6844b2bb6e0c46f89d3b6a237788b3cR414

rootulp commented 6 months ago

I just thought of a way to fix this bug without introducing an API breaking change, will publish an alternative PR soon.