celestiaorg / celestiaorg/rsmt2d
rsmt2d tests do not use nmt for row/column roots calculation
- Dominant language
- Go
- Stars
- 169
- Forks
- 87
- Avg merge
- 8h 9m
- Merged PRs (30d)
- 7
Description
# Problem
While working on [issue #191](https://github.com/celestiaorg/rsmt2d/issues/191) in the rsmt2d repo, I encountered an unusual situation. All the tests (as far as I checked) in the repository utilize a [default tree](https://github.com/celestiaorg/rsmt2d/blob/e8e042fdc8dcf485510c093a9c831f5b2fac2d3b/tree.go#L33), which is a normal Merkle tree but not an NMT. As a result, the tests do not actually make use of any of the logic related to NMT. This discrepancy means that the test scenarios in this repository do not accurately reflect the true mechanics of an EDS in Celestia. Consequently, important issues may remain unnoticed due to this inconsistency.
One such discrepancy can be observed in the [`Root()` method](https://github.com/celestiaorg/rsmt2d/blob/e8e042fdc8dcf485510c093a9c831f5b2fac2d3b/tree.go#L46). In the default tree implementation, the root can be computed regardless of whether the shares are nil or not. However, in the NMT implementation, it is not possible to even [push](https://github.com/celestiaorg/nmt/blob/3693c9a626b7ddaae996b3c75602f5accfa45739/nmt.go#L454) a nil value to the tree.
# Acceptance Criteria
This issue aims to investigate the problem and address it accordingly:
1. If the choice of a normal Merkle tree for the tests is intentional, it is crucial to document the reasoning behind this decision, at least within the tests themselves. It may be the case that for some of the tests, the usage of a normal Merkle tree is fine and desirable.
2. If the use of a normal Merkle tree is not intentional, it is recommended to consider replacing it with an NMT to ensure consistency and accuracy in the test scenarios.
Contributor guide
Research direction
Start by reviewing tree.go and the repository's tests to identify where the default Merkle tree calculates row and column roots. Compare those test setups with the NMT behavior described in the issue, especially handling nil shares. Done means documenting why a normal tree is appropriate for each relevant test or replacing it with an NMT where appropriate, with the test suite passing.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- testing
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100