celestiaorg / celestiaorg/rsmt2d

rsmt2d tests do not use nmt for row/column roots calculation

Open
#214 0 comments 1 reaction 0 assignees View on GitHub
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

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.