blob: CommitmentProof.Verify should firstly invoke .RowProof.Validate before invoking .RowProof.VerifyProof

[odeke-em]

Celestia Node version

1a1286f454c1dd670af856153a8fc8674cd91192

OS

darwin

package blob_test 

import (                
        "testing"

        "github.com/celestiaorg/celestia-app/v2/pkg/proof"
        "github.com/celestiaorg/celestia-node/blob"
)       

func TestCommitmentProofRowProofVerifyWithEmptyRoot(t *testing.T) {
        cp := &blob.CommitmentProof{
                RowProof: proof.RowProof{
                        Proofs: []*proof.Proof{{}},
                },
        }
        root := []byte{0xd3, 0x4d, 0x34}
        _, _ = cp.Verify(root, 1)
} 

Expected result

An error and no panic.

Actual result

--- FAIL: TestCommitmentProofRowProofVerifyWithEmptyRoot (0.00s)
panic: runtime error: index out of range [0] with length 0 [recovered]
    panic: runtime error: index out of range [0] with length 0

goroutine 12 [running]:
testing.tRunner.func1.2({0x10e37e820, 0xc000901740})
    /Users/emmanuelodeke/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.0.darwin-amd64/src/testing/testing.go:1632 +0x230
testing.tRunner.func1()
    /Users/emmanuelodeke/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.0.darwin-amd64/src/testing/testing.go:1635 +0x35e
panic({0x10e37e820?, 0xc000901740?})
    /Users/emmanuelodeke/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.0.darwin-amd64/src/runtime/panic.go:785 +0x132
github.com/celestiaorg/celestia-app/v2/pkg/proof.RowProof.VerifyProof({{0x0, 0x0, 0x0}, {0xc0006bcf58, 0x1, 0x1}, {0x0, 0x0, 0x0}, 0x0, ...}, ...)
    /Users/emmanuelodeke/go/pkg/mod/github.com/celestiaorg/celestia-app/v2@v2.1.2/pkg/proof/row_proof.go:32 +0x172
github.com/celestiaorg/celestia-node/blob.(*CommitmentProof).Verify(0xc0006bceb8, {0xc0006b0ab8, 0x3, 0x3}, 0x1)
    /Users/emmanuelodeke/go/src/github.com/celestiaorg/celestia-node/blob/commitment_proof.go:151 +0x850
github.com/celestiaorg/celestia-node/blob_test.TestCommitmentProofRowProofVerifyWithEmptyRoot(0xc0005c36c0?)
    /Users/emmanuelodeke/go/src/github.com/celestiaorg/celestia-node/blob/repro_test.go:19 +0xad
testing.tRunner(0xc0005c36c0, 0x10e4c30e8)
    /Users/emmanuelodeke/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.0.darwin-amd64/src/testing/testing.go:1690 +0xf4
created by testing.(*T).Run in goroutine 1
    /Users/emmanuelodeke/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.23.0.darwin-amd64/src/testing/testing.go:1743 +0x390
exit status 2
FAIL    github.com/celestiaorg/celestia-node/blob   1.565s

Suggested fix

Firstly ensure that we verify that there won't be an unexpected panic by the following

diff --git a/blob/commitment_proof.go b/blob/commitment_proof.go
index 8fa74671..82fed368 100644
--- a/blob/commitment_proof.go
+++ b/blob/commitment_proof.go
@@ -2,6 +2,7 @@ package blob

 import (
    "bytes"
+   "errors"
    "fmt"

    "github.com/celestiaorg/celestia-app/v2/pkg/appconsts"
@@ -85,6 +86,15 @@ func (commitmentProof *CommitmentProof) Validate() error {
 // Expects the commitment proof to be properly formulated and validated
 // using the Validate() function.
 func (commitmentProof *CommitmentProof) Verify(root []byte, subtreeRootThreshold int) (bool, error) {
+   if len(root) == 0 {
+       return false, errors.New("root must be non-empty")
+   }
+
+   rp := commitmentProof.RowProof
+   if err := rp.Validate(root); err != nil {
+       return false, err
+   }
+
    nmtHasher := nmt.NewNmtHasher(appconsts.NewBaseHashFunc(), share.NamespaceSize, true)

    // computes the total number of shares proven.
@@ -93,6 +103,10 @@ func (commitmentProof *CommitmentProof) Verify(root []byte, subtreeRootThreshold
        numberOfShares += proof.End() - proof.Start()
    }

/cc @liamsi @musalbas @rootulp

[Wondertan]

Cc @rach-id

[odeke-em]

I have mailed out a PR with the fixes along with the fuzzers per https://github.com/celestiaorg/celestia-node/pull/3732.