Difference between Validate and Verify in ShareProof and RowProof

[rach-id]

Currently, in the implementation of ShareProof and RowProof, we have Validate() and Verify() methods. Now I am looking at the implementation, both of them seem to be doing the same thing.

celestia-core/types/row_proof.go

Lines 24 to 52 in b67d79e

	// Validate performs checks on the fields of this RowProof. Returns an error if
	// the proof fails validation. If the proof passes validation, this function
	// attempts to verify the proof. It returns nil if the proof is valid.
	func (rp RowProof) Validate(root []byte) error {
	// HACKHACK performing subtraction with unsigned integers is unsafe.
	if int(rp.EndRow-rp.StartRow+1) != len(rp.RowRoots) {
	return fmt.Errorf("the number of rows %d must equal the number of row roots %d", int(rp.EndRow-rp.StartRow+1), len(rp.RowRoots))
	}
	if len(rp.Proofs) != len(rp.RowRoots) {
	return fmt.Errorf("the number of proofs %d must equal the number of row roots %d", len(rp.Proofs), len(rp.RowRoots))
	}
	if !rp.VerifyProof(root) {
	return errors.New("row proof failed to verify")
	}
	return nil
	}
	// VerifyProof verifies that all the row roots in this RowProof exist in a
	// Merkle tree with the given root. Returns true if all proofs are valid.
	func (rp RowProof) VerifyProof(root []byte) bool {
	for i, proof := range rp.Proofs {
	err := proof.Verify(root, rp.RowRoots[i])
	if err != nil {
	return false
	}
	}
	return true
	}

and,

celestia-core/types/share_proof.go

Lines 66 to 133 in b67d79e

	// Validate runs basic validations on the proof then verifies if it is consistent.
	// It returns nil if the proof is valid. Otherwise, it returns a sensible error.
	// The `root` is the block data root that the shares to be proven belong to.
	// Note: these proofs are tested on the app side.
	func (sp ShareProof) Validate(root []byte) error {
	numberOfSharesInProofs := int32(0)
	for _, proof := range sp.ShareProofs {
	// the range is not inclusive from the left.
	numberOfSharesInProofs += proof.End - proof.Start
	}
	if len(sp.ShareProofs) != len(sp.RowProof.RowRoots) {
	return fmt.Errorf("the number of share proofs %d must equal the number of row roots %d", len(sp.ShareProofs), len(sp.RowProof.RowRoots))
	}
	if len(sp.Data) != int(numberOfSharesInProofs) {
	return fmt.Errorf("the number of shares %d must equal the number of shares in share proofs %d", len(sp.Data), numberOfSharesInProofs)
	}
	for _, proof := range sp.ShareProofs {
	if proof.Start < 0 {
	return errors.New("proof index cannot be negative")
	}
	if (proof.End - proof.Start) <= 0 {
	return errors.New("proof total must be positive")
	}
	}
	if err := sp.RowProof.Validate(root); err != nil {
	return err
	}
	if ok := sp.VerifyProof(); !ok {
	return errors.New("share proof failed to verify")
	}
	return nil
	}
	func (sp ShareProof) VerifyProof() bool {
	cursor := int32(0)
	for i, proof := range sp.ShareProofs {
	nmtProof := nmt.NewInclusionProof(
	int(proof.Start),
	int(proof.End),
	proof.Nodes,
	true,
	)
	sharesUsed := proof.End - proof.Start
	if sp.NamespaceVersion > math.MaxUint8 {
	return false
	}
	// Consider extracting celestia-app's namespace package. We can't use it
	// here because that would introduce a circulcar import.
	namespace := append([]byte{uint8(sp.NamespaceVersion)}, sp.NamespaceID...)
	valid := nmtProof.VerifyInclusion(
	consts.NewBaseHashFunc(),
	namespace,
	sp.Data[cursor:sharesUsed+cursor],
	sp.RowProof.RowRoots[i],
	)
	if !valid {
	return false
	}
	cursor += sharesUsed
	}
	return true
	}

• Validate: runs some basic checks on the proof, then attempts to verify the proof against the root
• Verify: goes straight to verifying the proof against a root

I am thinking that if a proof.Verify returns true, then it's a valid proof. Similarly, if a proof.Validate returns true, there is no need to call Verify. So they're basically the same methods.

So I am wondering if it makes sense to remove the proof verification from the Validate method, and keep it only doing basic checks. And putting the responsibility of verifying the proof on Verify.

What do you think?

[rootulp]

So I am wondering if it makes sense to remove the proof verification from the Validate method, and keep it only doing basic checks. And putting the responsibility of verifying the proof on Verify.

That makes sense to me and aligns with how I would interpret Validate vs. Verify.

[rach-id]

The issue is that it will be a breaking change... Should we just create a PR for it in main and leave it there until someday?

[rootulp]

Hmm yea I think it's still worth fixing. Separately we also need to figure out how we're going to release celestia-core breaking changes (cc: @evan-forbes because figuring that out seems like it falls into the maintenance workstream).