fix: address review comments - change more proof types to values

Co-authored-by: tzdybal <[email protected]>

Author: Copilot

consensus/propagation/commitment.go

@@ -136,16 +136,16 @@ func extractHashes(blocks ...*types.PartSet) [][]byte {
 	return partHashes
 }
 
-func extractProofs(blocks ...*types.PartSet) []*merkle.Proof {
+func extractProofs(blocks ...*types.PartSet) []merkle.Proof {
 	total := uint32(0)
 	for _, block := range blocks {
 		total += block.Total()
 	}
 
-	proofs := make([]*merkle.Proof, 0, total) // Preallocate capacity
+	proofs := make([]merkle.Proof, 0, total) // Preallocate capacity
 	for _, block := range blocks {
 		for i := uint32(0); i < block.Total(); i++ {
-			proofs = append(proofs, &block.GetPart(int(i)).Proof)
+			proofs = append(proofs, block.GetPart(int(i)).Proof)
 		}
 	}
 	return proofs
@@ -275,7 +275,7 @@ func (blockProp *Reactor) recoverPartsFromMempool(cb *proptypes.CompactBlock) {
 		if partSet.HasPart(int(p.Index)) {
 			continue
 		}
-		p.Proof = *proofs[p.Index]
+		p.Proof = proofs[p.Index]
 
 		added, err := partSet.AddOriginalPart(p)
 		if err != nil {

consensus/propagation/have_wants.go

@@ -467,14 +467,14 @@ func (blockProp *Reactor) handleRecoveryPart(peer p2p.ID, part *proptypes.Recove
 	// sent during catchup.
 	proof := cb.GetProof(part.Index)
 	if proof == nil {
-		if part.Proof == nil {
+		if len(part.Proof.LeafHash) == 0 {
 			blockProp.Logger.Error("proof not found", "peer", peer, "height", part.Height, "round", part.Round, "part", part.Index)
 			return
 		}
 		if len(part.Proof.LeafHash) != tmhash.Size {
 			return
 		}
-		proof = part.Proof
+		proof = &part.Proof
 	}
 
 	added, err := parts.AddPart(part, *proof)
@@ -588,7 +588,7 @@ func (blockProp *Reactor) handleRecoveryPart(peer p2p.ID, part *proptypes.Recove
 		return
 	}
 
-	go blockProp.clearWants(part, *proof)
+	go blockProp.clearWants(part, part.Proof)
 }
 
 // clearWants checks the wantState to see if any peers want the given part, if

consensus/propagation/reactor_test.go

@@ -643,7 +643,7 @@ func TestConcurrentRequestLimit(t *testing.T) {
 
 // testCompactBlock returns a test compact block with the corresponding orignal part set,
 // parity partset, and proofs.
-func testCompactBlock(t *testing.T, height int64, round int32) (*proptypes.CompactBlock, *types.PartSet, *types.PartSet, []*merkle.Proof) {
+func testCompactBlock(t *testing.T, height int64, round int32) (*proptypes.CompactBlock, *types.PartSet, *types.PartSet, []merkle.Proof) {
 	ps, err := types.NewPartSetFromData(cmtrand.Bytes(1000), types.BlockPartSizeBytes)
 	require.NoError(t, err)
 	pse, lastLen, err := types.Encode(ps, types.BlockPartSizeBytes)

consensus/propagation/types/types.go

@@ -75,7 +75,7 @@ type CompactBlock struct {
 	mtx sync.Mutex
 	// proofsCache is local storage from generated proofs from the PartsHashes.
 	// It must not be included in any serialization.
-	proofsCache []*merkle.Proof
+	proofsCache []merkle.Proof
 }
 
 // SignBytes returns the compact block commitment data that
@@ -192,7 +192,7 @@ func (c *CompactBlock) ToProto() *protoprop.CompactBlock {
 // thrown if the proofs are generated and the resulting hashes don't match those
 // in the compact block. This method should be called upon first receiving a
 // compact block.
-func (c *CompactBlock) Proofs() ([]*merkle.Proof, error) {
+func (c *CompactBlock) Proofs() ([]merkle.Proof, error) {
 	c.mtx.Lock()
 	defer c.mtx.Unlock()
 
@@ -206,11 +206,11 @@ func (c *CompactBlock) Proofs() ([]*merkle.Proof, error) {
 		return nil, errors.New("invalid number of partset hashes")
 	}
 
-	c.proofsCache = make([]*merkle.Proof, 0, len(c.PartsHashes))
+	c.proofsCache = make([]merkle.Proof, 0, len(c.PartsHashes))
 
 	root, proofs := merkle.ProofsFromLeafHashes(c.PartsHashes[:total])
 	for i := range proofs {
-		c.proofsCache = append(c.proofsCache, &proofs[i])
+		c.proofsCache = append(c.proofsCache, proofs[i])
 	}
 
 	if !bytes.Equal(root, c.Proposal.BlockID.PartSetHeader.Hash) {
@@ -219,7 +219,7 @@ func (c *CompactBlock) Proofs() ([]*merkle.Proof, error) {
 
 	parityRoot, eproofs := merkle.ProofsFromLeafHashes(c.PartsHashes[total:])
 	for i := range eproofs {
-		c.proofsCache = append(c.proofsCache, &eproofs[i])
+		c.proofsCache = append(c.proofsCache, eproofs[i])
 	}
 
 	if !bytes.Equal(c.BpHash, parityRoot) {
@@ -233,12 +233,12 @@ func (c *CompactBlock) GetProof(i uint32) *merkle.Proof {
 	c.mtx.Lock()
 	defer c.mtx.Unlock()
 	if i < uint32(len(c.proofsCache)) {
-		return c.proofsCache[i]
+		return &c.proofsCache[i]
 	}
 	return nil
 }
 
-func (c *CompactBlock) SetProofCache(proofs []*merkle.Proof) {
+func (c *CompactBlock) SetProofCache(proofs []merkle.Proof) {
 	c.mtx.Lock()
 	defer c.mtx.Unlock()
 	c.proofsCache = proofs
@@ -449,7 +449,7 @@ type RecoveryPart struct {
 	Round  int32
 	Index  uint32
 	Data   []byte
-	Proof  *merkle.Proof
+	Proof  merkle.Proof
 }
 
 func (p *RecoveryPart) ValidateBasic() error {
@@ -462,14 +462,12 @@ func (p *RecoveryPart) ValidateBasic() error {
 	if len(p.Data) == 0 {
 		return errors.New("RecoveryPart: Data cannot be nil or empty")
 	}
-	if p.Proof != nil {
-		if err := p.Proof.ValidateBasic(); err != nil {
-			return fmt.Errorf("RecoveryPart: invalid proof: %w", err)
-		}
-		hash := merkle.LeafHash(p.Data)
-		if !bytes.Equal(hash, p.Proof.LeafHash) {
-			return errors.New("RecoveryPart: invalid proof leaf hash")
-		}
+	if err := p.Proof.ValidateBasic(); err != nil {
+		return fmt.Errorf("RecoveryPart: invalid proof: %w", err)
+	}
+	hash := merkle.LeafHash(p.Data)
+	if !bytes.Equal(hash, p.Proof.LeafHash) {
+		return errors.New("RecoveryPart: invalid proof leaf hash")
 	}
 	return nil
 }
@@ -487,7 +485,7 @@ func RecoveryPartFromProto(r *protoprop.RecoveryPart) (*RecoveryPart, error) {
 		Round:  r.Round,
 		Index:  r.Index,
 		Data:   r.Data,
-		Proof:  &proof,
+		Proof:  proof,
 	}
 	return rp, rp.ValidateBasic()
 }

consensus/propagation/types/types_test.go

@@ -791,7 +791,7 @@ func TestRecoveryPart_ValidateBasic(t *testing.T) {
 				Round:  1,
 				Index:  2,
 				Data:   []byte("valid-data"),
-				Proof: &merkle.Proof{
+				Proof: merkle.Proof{
 					LeafHash: merkle.LeafHash([]byte("valid-data")),
 				},
 			},
@@ -844,7 +844,7 @@ func TestRecoveryPart_ValidateBasic(t *testing.T) {
 				Round:  1,
 				Index:  0,
 				Data:   []byte("data"),
-				Proof:  &merkle.Proof{},
+				Proof:  merkle.Proof{},
 			},
 			expectError: "RecoveryPart: invalid proof",
 		},
@@ -855,7 +855,7 @@ func TestRecoveryPart_ValidateBasic(t *testing.T) {
 				Round:  1,
 				Index:  0,
 				Data:   []byte("data"),
-				Proof: &merkle.Proof{
+				Proof: merkle.Proof{
 					LeafHash: merkle.LeafHash([]byte("invalid")),
 				},
 			},

crypto/merkle/proof_test.go

@@ -189,8 +189,14 @@ func TestVoteProtobuf(t *testing.T) {
 		{"success", &proofs[0], true},
 	}
 	for _, tc := range testCases {
+		if tc.v1 == nil {
+			// Test nil case
+			_, err := ProofFromProto(nil, false)
+			require.Error(t, err)
+			continue
+		}
+		
 		pb := tc.v1.ToProto()
-
 		v, err := ProofFromProto(pb, false)
 		if tc.expPass {
 			require.NoError(t, err)

rpc/core/blocks.go

@@ -476,21 +476,21 @@ func (env *Environment) validateDataRootInclusionProofRequest(height uint64, sta
 }
 
 // proveDataRootTuples returns the merkle inclusion proof for a height.
-func (env *Environment) proveDataRootTuples(tuples []DataRootTuple, height int64) (*merkle.Proof, error) {
+func (env *Environment) proveDataRootTuples(tuples []DataRootTuple, height int64) (merkle.Proof, error) {
 	dataRootEncodedTuples := make([][]byte, 0, len(tuples))
 	for _, tuple := range tuples {
 		encodedTuple, err := EncodeDataRootTuple(
 			tuple.height,
 			tuple.dataRoot,
 		)
 		if err != nil {
-			return nil, err
+			return merkle.Proof{}, err
 		}
 		dataRootEncodedTuples = append(dataRootEncodedTuples, encodedTuple)
 	}
 	_, proofs := merkle.ProofsFromByteSlices(dataRootEncodedTuples)
 	//nolint:gosec
-	return &proofs[height-int64(tuples[0].height)], nil
+	return proofs[height-int64(tuples[0].height)], nil
 }
 
 // fetchDataRootTuples takes an end exclusive range of heights and fetches its
@@ -544,5 +544,5 @@ func (env *Environment) GenerateDataRootInclusionProof(height int64, start, end
 	if err != nil {
 		return nil, err
 	}
-	return proof, nil
+	return &proof, nil
 }