Skip to content

Commit

Permalink
feat!: removes the Tree method from the NMT wrapper and introduces …
Browse files Browse the repository at this point in the history
…the `ProveRange` API (#1438)

## Overview
While reviewing the NMT wrapper implementation and attempting to write
its specs, came across a contradiction in the wrapper's design. On one
hand, the wrapper is intended to encapsulate an instance of NMT and
re-define/modify some of its APIs, specifically the
[`Push`](https://github.com/celestiaorg/celestia-app/blob/6d27b78aa64a749a808e84ea682352b8b551fbd7/pkg/wrapper/nmt_wrapper.go#L80)
method. On the other hand, the wrapper also exposes the
[`Tree()`](https://github.com/celestiaorg/celestia-app/blob/6d27b78aa64a749a808e84ea682352b8b551fbd7/pkg/wrapper/nmt_wrapper.go#L112)
method, which returns a pointer to the underlying NMT tree and thus
allows direct access to the tree and enables arbitrary manipulation of
the underlying tree outside of the intended control of the wrapper. For
instance, one can bypass the NMT wrapper `Push` method and directly call
the `Push` method of the underlying tree using `.Tree().Push()`. This
not only widens the error surface but also violates the initial
objective of the wrapper.

To address this issue, the `Tree()` API has been removed, and all
necessary methods of the underlying NMT are now exposed through the
wrapper directly.

## Checklist
- [x] New and updated code has appropriate documentation
- [x] New and updated code has new and/or updated testing
- [x] Required CI checks are passing
- [x] Visual proof for any user facing features like CLI or
documentation updates
- [x] Linked issues closed with keywords
  • Loading branch information
staheri14 committed Mar 3, 2023
1 parent 62eae87 commit 4ee939b
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 20 deletions.
2 changes: 1 addition & 1 deletion pkg/proof/proof.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func NewShareInclusionProof(
}

rawShares = append(rawShares, shares.ToBytes(row[startLeafPos:endLeafPos+1])...)
proof, err := tree.Tree().ProveRange(int(startLeafPos), int(endLeafPos+1))
proof, err := tree.ProveRange(int(startLeafPos), int(endLeafPos+1))
if err != nil {
return types.ShareProof{}, err
}
Expand Down
7 changes: 4 additions & 3 deletions pkg/wrapper/nmt_wrapper.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,14 @@ func (w *ErasuredNamespacedMerkleTree) Root() []byte {
return w.tree.Root()
}

// Prove returns a Merkle inclusion proof for the leaf at index `ind`.
func (w *ErasuredNamespacedMerkleTree) Prove(ind int) (nmt.Proof, error) {
return w.tree.Prove(ind)
}

// Tree returns the underlying NamespacedMerkleTree
func (w *ErasuredNamespacedMerkleTree) Tree() *nmt.NamespacedMerkleTree {
return w.tree
// ProveRange returns a Merkle range proof for the leaf range [start, end] where `end` is non-inclusive.
func (w *ErasuredNamespacedMerkleTree) ProveRange(start, end int) (nmt.Proof, error) {
return w.tree.ProveRange(start, end)
}

// incrementShareIndex increments the share index by one.
Expand Down
16 changes: 0 additions & 16 deletions pkg/wrapper/nmt_wrapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,22 +110,6 @@ func TestComputeExtendedDataSquare(t *testing.T) {
assert.NoError(t, err)
}

// TestErasuredNamespacedMerkleTree verifies that Tree() returns the underlying
// NMT tree.
func TestErasuredNamespacedMerkleTree(t *testing.T) {
squareSize := 8
data := generateRandNamespacedRawData(squareSize, appconsts.NamespaceSize, appconsts.ShareSize-appconsts.NamespaceSize)
tree := NewErasuredNamespacedMerkleTree(uint64(squareSize), 0)

for _, d := range data {
tree.Push(d)
}

assert.Equal(t, tree.Tree(), tree.tree)
assert.Equal(t, tree.Tree().Root(), tree.tree.Root())
assert.Equal(t, appconsts.NamespaceSize, int(tree.Tree().NamespaceSize()))
}

// generateErasuredData generates random data and then erasure codes it. It
// returns a slice that is twice as long as numLeaves because it returns the
// original data + erasured data.
Expand Down

0 comments on commit 4ee939b

Please sign in to comment.