using nmt wrapper results in cyclic import in rsmt2d
staheri14 opened this issue · 5 comments
Problem
To effectively test the expected behavior of rsmt2d when the underlying shares are out of order (to address the following issue), we must utilize the NMT wrapper as the TreeConstructorFn. However, this approach leads to a cyclic import issue. Specifically, the github.com/celestiaorg/celestia-app/pkg/wrapper imports github.com/celestiaorg/rsmt2d, and vice versa.
Proposal
The NMT wrapper relies on rsmt2d.TreeConstructorFn and rsmt2d.Tree in these lines of code, as well as rsmt2d.Axis in this part of the code.
To address the circular dependency issue, a possible solution is to create two new packages within rsmt2d. The first package would contain rsmt2d.TreeConstructorFn, rsmt2d.Tree, and their related functions. This package would essentially involve renaming the package name of tree.go to something like tree. The second package would host rsmt2d.Axis and its relevant methods, with a package name like axis.
After looking into it further, I found another way to fix this issue. We can change the package of all the tests in rsmt2d to a new package called rsmt2d_test. However, keep in mind that this change might require us to make some private fields and methods accessible to accommodate the package change.
I am currently working on refactoring the tests into a distinct package, i.e., this approach . Will follow up with a PR.
is there any way that we could just use nmt or another existing merkle tree? I feel like including nmt or another existing tree as a dependency is one thing, but include the wrapper (and thus some portion of the app) should be avoided since that will bubble down to all other projects that use rsmt2d
Actually importing nmt is not sufficient in this case, we need the nmt wrapper since it takes care of adding parity namespaces to the erasure-coded portion of the EDS.
is there any way that we could just use nmt or another existing merkle tree?
I'll explore alternative options to see what could be viable.
Closing this issue as I figured out another way to test rsmt2d with nmt as the underlying tree without causing cyclic imports.