imp: cleanup verifcation arg code for 23-commitment (#7493)

* imp: cleanup verifcation arg code for 23-commitment

* Update modules/core/23-commitment/types/merkle.go

* Update modules/core/23-commitment/types/merkle_test.go

* chore: review followups

* Update modules/core/23-commitment/types/merkle.go

---------

Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>
Co-authored-by: Damian Nolan <damiannolan@gmail.com>

modules/core/23-commitment/types/merkle.go

@@ -3,13 +3,10 @@ package types
 import (
 	"bytes"
 
-	"github.com/cosmos/gogoproto/proto"
 	ics23 "github.com/cosmos/ics23/go"
 
 	errorsmod "cosmossdk.io/errors"
 
-	cmtcrypto "github.com/cometbft/cometbft/proto/tendermint/crypto"
-
 	"github.com/cosmos/ibc-go/v9/modules/core/23-commitment/types/v2"
 	"github.com/cosmos/ibc-go/v9/modules/core/exported"
 )
@@ -86,19 +83,16 @@ func ApplyPrefix(prefix exported.Prefix, path v2.MerklePath) (v2.MerklePath, err
 // VerifyMembership verifies the membership of a merkle proof against the given root, path, and value.
 // Note that the path is expected as []string{<store key of module>, <key corresponding to requested value>}.
 func (proof MerkleProof) VerifyMembership(specs []*ics23.ProofSpec, root exported.Root, path exported.Path, value []byte) error {
-	if err := proof.validateVerificationArgs(specs, root); err != nil {
-		return err
-	}
-
-	// VerifyMembership specific argument validation
 	mpath, ok := path.(v2.MerklePath)
 	if !ok {
 		return errorsmod.Wrapf(ErrInvalidProof, "path %v is not of type MerklePath", path)
 	}
-	if len(mpath.KeyPath) != len(specs) {
-		return errorsmod.Wrapf(ErrInvalidProof, "path length %d not same as proof %d",
-			len(mpath.KeyPath), len(specs))
+
+	if err := validateVerificationArgs(proof, mpath, specs, root); err != nil {
+		return err
 	}
+
+	// VerifyMembership specific argument validation
 	if len(value) == 0 {
 		return errorsmod.Wrap(ErrInvalidProof, "empty value in membership proof")
 	}
@@ -112,17 +106,13 @@ func (proof MerkleProof) VerifyMembership(specs []*ics23.ProofSpec, root exporte
 // VerifyNonMembership verifies a chained proof where the absence of a given path is proven
 // at the lowest subtree and then each subtree's inclusion is proved up to the final root.
 func (proof MerkleProof) VerifyNonMembership(specs []*ics23.ProofSpec, root exported.Root, path exported.Path) error {
-	if err := proof.validateVerificationArgs(specs, root); err != nil {
-		return err
-	}
-
-	// VerifyNonMembership specific argument validation
 	mpath, ok := path.(v2.MerklePath)
 	if !ok {
 		return errorsmod.Wrapf(ErrInvalidProof, "path %v is not of type MerkleProof", path)
 	}
-	if len(mpath.KeyPath) != len(specs) {
-		return errorsmod.Wrapf(ErrInvalidProof, "path length %d not same as proof %d", len(mpath.KeyPath), len(specs))
+
+	if err := validateVerificationArgs(proof, mpath, specs, root); err != nil {
+		return err
 	}
 
 	// VerifyNonMembership will verify the absence of key in lowest subtree, and then chain inclusion proofs
@@ -198,40 +188,25 @@ func verifyChainedMembershipProof(root []byte, specs []*ics23.ProofSpec, proofs
 	return nil
 }
 
-// blankMerkleProof and blankProofOps will be used to compare against their zero values,
-// and are declared as globals to avoid having to unnecessarily re-allocate on every comparison.
-var (
-	blankMerkleProof = &MerkleProof{}
-	blankProofOps    = &cmtcrypto.ProofOps{}
-)
-
-// Empty returns true if the root is empty
-func (proof *MerkleProof) Empty() bool {
-	return proof == nil || proto.Equal(proof, blankMerkleProof) || proto.Equal(proof, blankProofOps)
-}
-
-// ValidateBasic checks if the proof is empty.
-func (proof MerkleProof) ValidateBasic() error {
-	if proof.Empty() {
-		return ErrInvalidProof
-	}
-	return nil
-}
-
-// validateVerificationArgs verifies the proof arguments are valid
-func (proof MerkleProof) validateVerificationArgs(specs []*ics23.ProofSpec, root exported.Root) error {
-	if proof.Empty() {
-		return errorsmod.Wrap(ErrInvalidMerkleProof, "proof cannot be empty")
+// validateVerificationArgs verifies the proof arguments are valid.
+// The merkle path and merkle proof contain a list of keys and their proofs
+// which correspond to individual trees. The length of these keys and their proofs
+// must equal the length of the given specs. All arguments must be non-empty.
+func validateVerificationArgs(proof MerkleProof, path v2.MerklePath, specs []*ics23.ProofSpec, root exported.Root) error {
+	if proof.GetProofs() == nil {
+		return errorsmod.Wrap(ErrInvalidMerkleProof, "proof must not be empty")
 	}
 
 	if root == nil || root.Empty() {
 		return errorsmod.Wrap(ErrInvalidMerkleProof, "root cannot be empty")
 	}
 
 	if len(specs) != len(proof.Proofs) {
-		return errorsmod.Wrapf(ErrInvalidMerkleProof,
-			"length of specs: %d not equal to length of proof: %d",
-			len(specs), len(proof.Proofs))
+		return errorsmod.Wrapf(ErrInvalidMerkleProof, "length of specs: %d not equal to length of proof: %d", len(specs), len(proof.Proofs))
+	}
+
+	if len(path.KeyPath) != len(specs) {
+		return errorsmod.Wrapf(ErrInvalidProof, "path length %d not same as proof %d", len(path.KeyPath), len(specs))
 	}
 
 	for i, spec := range specs {

modules/core/23-commitment/types/merkle_test.go

@@ -27,9 +27,6 @@ func (suite *MerkleTestSuite) TestVerifyMembership() {
 	proof, err := types.ConvertProofs(res.ProofOps)
 	require.NoError(suite.T(), err)
 
-	suite.Require().NoError(proof.ValidateBasic())
-	suite.Require().Error(types.MerkleProof{}.ValidateBasic())
-
 	cases := []struct {
 		name       string
 		root       []byte
@@ -93,8 +90,6 @@ func (suite *MerkleTestSuite) TestVerifyNonMembership() {
 	proof, err := types.ConvertProofs(res.ProofOps)
 	require.NoError(suite.T(), err)
 
-	suite.Require().NoError(proof.ValidateBasic())
-
 	cases := []struct {
 		name       string
 		root       []byte