celestiaorg / celestia-app

PoS application for the consensus portion of the Celestia network. Built using celestia-core (fork of CometBFT) and the cosmos-sdk
https://celestia.org
Apache License 2.0
328 stars 261 forks source link

Decide on Celestia-app specific code in NMT #3615

Open rach-id opened 4 days ago

rach-id commented 4 days ago

The NMT currently contains some of Celestia-app specific code. We need to decide whether we're fine having it there or we want to move it here, or to go-square.

The reason we have that code there is because the inner node verification requires either we export a lot of internal code, or we copy and paste. From one perspective, we can see it as a temporary solution until a full-fledged algorithm that can prove the inner nodes for any tree is implemented. It is certainly possible to implement, and I had an initial PR that does that here: https://github.com/celestiaorg/nmt/pull/258/files

However, this means bigger changes, harder to review, requires more audits, would require more time to complete, and more importantly either going to be a breaking change or will create a new InnerProof type.

So, once we found the celestia specific way of doing it with minimal changes that works for our case, we did it that way.

In the future, we might want to move that work here or go for the generalised solution.

_Originally posted by @liamsi in https://github.com/celestiaorg/nmt/pull/260#discussion_r1650846148_