fix: check for nil before setting nil

[Wondertan]

This change is motivated by the race conditions I observed. The roots are reset every time we do SetCell by setting nil. However, it causes race conditions between multiple SetCell calls. This change fixes this and is tested against celestia-node

[codecov]

Codecov Report

Merging #128 (861ed5d) into master (33a5763) will decrease coverage by 0.73%.
The diff coverage is 0.00%.

❗ Current head 861ed5d differs from pull request most recent head ed5b81e. Consider uploading reports for the commit ed5b81e to get more accurate results

@@            Coverage Diff             @@
##           master     #128      +/-   ##
==========================================
- Coverage   83.23%   82.49%   -0.74%     
==========================================
  Files           8        8              
  Lines         495      497       +2     
==========================================
- Hits          412      410       -2     
- Misses         50       52       +2     
- Partials       33       35       +2     

Impacted Files	Coverage Δ
datasquare.go	90.78% <0.00%> (-2.55%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[liamsi]

I do not fully understand how this would fix a race condition tbh

[adlerjohn]

Also confused at how this resolves race conditions. It's non-obvious (if at all), so should be documented in the code. Otherwise someone will go in and optimize those lines away without context, since there's no test to exercise the behavior in this repo.

[Wondertan]

The race happens when multiple routines calls SetCell. All of them write nil into the same slot. Writing nil is still writing and race detector complains.

The race is fixed here because we check if the slot is nil already and those multiple routines only do reads of the slot now, which is race safe

cc @adlerjohn, cc @liamsi

[Wondertan]

Happy to document this in the code.

[Wondertan]

Done

[rahulghangas]

Ran into the same issue while working on #123, can confirm it fixes the race condition

[adlerjohn]

utACK, but would like @liamsi to confirm

[Wondertan]

@liamsi, kind ping

[liamsi]

https://github.com/celestiaorg/rsmt2d/pull/128/files#r988034202

Right now the EDS does not have any guarantees regarding thread safety for reads and writes.

Can you please open an issue around that?

[Wondertan]

@liamsi, #135