fraud: fix findings in fraud package (#826)

* fraud: fix all non-functional findings

Co-authored-by: Hlib Kanunnikov <[email protected]>

Co-authored-by: Hlib Kanunnikov <[email protected]>

Author: vgonkivs

Diff for: docs/adr/adr-006-fraud-service.md

@@ -77,7 +77,7 @@ type BadEncodingProof struct {
 }
 ```
 
-2. Full node broadcasts BEFP to all light nodes via separate sub-service via proto message:
+2. Full node broadcasts BEFP to all light and full nodes via separate sub-service via proto message:
 
 ```proto3
 
@@ -125,8 +125,9 @@ type Proof interface {
    encoding.BinaryMarshaller
 }
 ```
+*Note*: Full node, that detected a malicious block and created a Fraud Proof, will also receive it by subscription to stop respective services.
 
-2a. From the other side, light nodes will, by default, subscribe to the BEFP topic and verify messages received on the topic:
+2a. From the other side, nodes will, by default, subscribe to the BEFP topic and verify messages received on the topic:
 
 ```go
 type ProofUnmarshaller func([]byte) (Proof,error)

Diff for: fraud/bad_encoding.go

@@ -25,13 +25,13 @@ type BadEncodingProof struct {
 	// Shares that did not pass verification in rmst2d will be nil.
 	// For non-nil shares MerkleProofs are computed.
 	Shares []*ipld.ShareWithProof
-	// Index represents the row/col index where ErrByzantineRow/ErrByzantineColl occurred
-	Index uint8
-	// isRow shows that verification failed on row
+	// Index represents the row/col index where ErrByzantineRow/ErrByzantineColl occurred.
+	Index uint32
+	// isRow shows that verification failed on row.
 	isRow bool
 }
 
-// CreateBadEncodingProof creates a new Bad Encoding Fraud Proof that should be propagated through network
+// CreateBadEncodingProof creates a new Bad Encoding Fraud Proof that should be propagated through network.
 // The fraud proof will contain shares that did not pass verification and their relevant Merkle proofs.
 func CreateBadEncodingProof(
 	hash []byte,
@@ -48,7 +48,7 @@ func CreateBadEncodingProof(
 	}
 }
 
-// Type returns type of fraud proof
+// Type returns type of fraud proof.
 func (p *BadEncodingProof) Type() ProofType {
 	return BadEncoding
 }
@@ -58,12 +58,12 @@ func (p *BadEncodingProof) HeaderHash() []byte {
 	return p.headerHash
 }
 
-// Height returns block height
+// Height returns block height.
 func (p *BadEncodingProof) Height() uint64 {
 	return p.BlockHeight
 }
 
-// MarshalBinary converts BadEncodingProof to binary
+// MarshalBinary converts BadEncodingProof to binary.
 func (p *BadEncodingProof) MarshalBinary() ([]byte, error) {
 	shares := make([]*ipld_pb.Share, 0, len(p.Shares))
 	for _, share := range p.Shares {
@@ -74,7 +74,7 @@ func (p *BadEncodingProof) MarshalBinary() ([]byte, error) {
 		HeaderHash: p.headerHash,
 		Height:     p.BlockHeight,
 		Shares:     shares,
-		Index:      uint32(p.Index),
+		Index:      p.Index,
 		IsRow:      p.isRow,
 	}
 	return badEncodingFraudProof.Marshal()
@@ -89,7 +89,7 @@ func UnmarshalBEFP(data []byte) (Proof, error) {
 	return befp, nil
 }
 
-// UnmarshalBinary converts binary to BadEncodingProof
+// UnmarshalBinary converts binary to BadEncodingProof.
 func (p *BadEncodingProof) UnmarshalBinary(data []byte) error {
 	in := pb.BadEncoding{}
 	if err := in.Unmarshal(data); err != nil {
@@ -99,7 +99,7 @@ func (p *BadEncodingProof) UnmarshalBinary(data []byte) error {
 		headerHash:  in.HeaderHash,
 		BlockHeight: in.Height,
 		Shares:      ipld.ProtoToShare(in.Shares),
-		Index:       uint8(in.Index),
+		Index:       in.Index,
 		isRow:       in.IsRow,
 	}
 
@@ -114,23 +114,24 @@ func (p *BadEncodingProof) UnmarshalBinary(data []byte) error {
 // and compares it with block's Merkle Root.
 func (p *BadEncodingProof) Validate(header *header.ExtendedHeader) error {
 	if header.Height != int64(p.BlockHeight) {
-		return errors.New("invalid fraud proof: incorrect block height")
+		return errors.New("fraud: incorrect block height")
 	}
 	merkleRowRoots := header.DAH.RowsRoots
 	merkleColRoots := header.DAH.ColumnRoots
 	if len(merkleRowRoots) != len(merkleColRoots) {
 		// NOTE: This should never happen as callers of this method should not feed it with a
 		// malformed extended header.
-		panic(fmt.Sprintf("invalid extended header: length of row and column roots do not match. (rowRoots=%d) (colRoots=%d)",
+		panic(fmt.Sprintf(
+			"fraud: invalid extended header: length of row and column roots do not match. (rowRoots=%d) (colRoots=%d)",
 			len(merkleRowRoots),
 			len(merkleColRoots)),
 		)
 	}
 	if int(p.Index) >= len(merkleRowRoots) {
-		return fmt.Errorf("invalid fraud proof: index out of bounds (%d >= %d)", int(p.Index), len(merkleRowRoots))
+		return fmt.Errorf("fraud: invalid proof: index out of bounds (%d >= %d)", int(p.Index), len(merkleRowRoots))
 	}
 	if len(merkleRowRoots) != len(p.Shares) {
-		return fmt.Errorf("invalid fraud proof: incorrect number of shares %d != %d", len(p.Shares), len(merkleRowRoots))
+		return fmt.Errorf("fraud: invalid proof: incorrect number of shares %d != %d", len(p.Shares), len(merkleRowRoots))
 	}
 
 	root := merkleRowRoots[p.Index]
@@ -140,19 +141,19 @@ func (p *BadEncodingProof) Validate(header *header.ExtendedHeader) error {
 
 	shares := make([][]byte, len(merkleRowRoots))
 
-	// verify that Merkle proofs correspond to particular shares
+	// verify that Merkle proofs correspond to particular shares.
 	for index, share := range p.Shares {
 		if share == nil {
 			continue
 		}
 		shares[index] = share.Share
 		if ok := share.Validate(plugin.MustCidFromNamespacedSha256(root)); !ok {
-			return fmt.Errorf("invalid fraud proof: incorrect share received at Index %d", index)
+			return fmt.Errorf("fraud: invalid proof: incorrect share received at index %d", index)
 		}
 	}
 
 	codec := consts.DefaultCodec()
-	// rebuild a row or col
+	// rebuild a row or col.
 	rebuiltShares, err := codec.Decode(shares)
 	if err != nil {
 		return err
@@ -168,9 +169,9 @@ func (p *BadEncodingProof) Validate(header *header.ExtendedHeader) error {
 		tree.Push(share, rsmt2d.SquareIndex{Axis: uint(p.Index), Cell: uint(i)})
 	}
 
-	// comparing rebuilt Merkle Root of bad row/col with respective Merkle Root of row/col from block
+	// comparing rebuilt Merkle Root of bad row/col with respective Merkle Root of row/col from block.
 	if bytes.Equal(tree.Root(), root) {
-		return errors.New("invalid fraud proof: recomputed Merkle root matches the header's row/column root")
+		return errors.New("fraud: invalid proof: recomputed Merkle root matches the DAH's row/column root")
 	}
 
 	return nil

Diff for: fraud/bad_encoding_test.go

@@ -4,32 +4,26 @@ import (
 	"context"
 	"errors"
 	"testing"
+	"time"
 
 	mdutils "github.com/ipfs/go-merkledag/test"
 	"github.com/stretchr/testify/require"
-	"github.com/tendermint/tendermint/pkg/da"
 
-	"github.com/celestiaorg/celestia-node/header"
 	"github.com/celestiaorg/celestia-node/ipld"
 )
 
 func TestFraudProofValidation(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second*15)
+	defer t.Cleanup(cancel)
 	bServ := mdutils.Bserv()
-	eds := ipld.RandEDS(t, 2)
-
-	shares := ipld.ExtractEDS(eds)
-	copy(shares[3][8:], shares[4][8:])
-	eds, err := ipld.ImportShares(context.Background(), shares, bServ)
+	_, store := createService(t)
+	h, err := store.GetByHeight(ctx, 1)
 	require.NoError(t, err)
-	da := da.NewDataAvailabilityHeader(eds)
-	r := ipld.NewRetriever(bServ)
-	_, err = r.Retrieve(context.Background(), &da)
+
+	faultDAH, err := generateByzantineError(ctx, t, h, bServ)
 	var errByz *ipld.ErrByzantine
 	require.True(t, errors.As(err, &errByz))
-
-	dah := &header.ExtendedHeader{DAH: &da}
-
-	p := CreateBadEncodingProof([]byte("hash"), uint64(dah.Height), errByz)
-	err = p.Validate(dah)
+	p := CreateBadEncodingProof([]byte("hash"), uint64(faultDAH.Height), errByz)
+	err = p.Validate(faultDAH)
 	require.NoError(t, err)
 }

Diff for: fraud/proof.go

@@ -18,21 +18,21 @@ func (p ProofType) String() string {
 	case BadEncoding:
 		return "badencoding"
 	default:
-		panic(fmt.Sprintf("invalid proof type: %d", p))
+		panic(fmt.Sprintf("fraud: invalid proof type: %d", p))
 	}
 }
 
 // Proof is a generic interface that will be used for all types of fraud proofs in the network.
 type Proof interface {
-	// Type returns the exact type of fraud proof
+	// Type returns the exact type of fraud proof.
 	Type() ProofType
 	// HeaderHash returns the block hash.
 	HeaderHash() []byte
-	// Height returns the block height corresponding to the Proof
+	// Height returns the block height corresponding to the Proof.
 	Height() uint64
 	// Validate check the validity of fraud proof.
-	// Validate throws an error if some conditions don't pass and thus fraud proof is not valid
-	// NOTE: header.ExtendedHeader should pass basic validation otherwise it will panic if it's malformed
+	// Validate throws an error if some conditions don't pass and thus fraud proof is not valid.
+	// NOTE: header.ExtendedHeader should pass basic validation otherwise it will panic if it's malformed.
 	Validate(*header.ExtendedHeader) error
 
 	encoding.BinaryMarshaler

Diff for: ipld/retriever_byzantine.go

@@ -17,7 +17,7 @@ import (
 // It is converted from rsmt2d.ByzantineRow/Col +
 // Merkle Proof for each share.
 type ErrByzantine struct {
-	Index  uint8
+	Index  uint32
 	Shares []*ShareWithProof
 	// TODO(@vgokivs): Change to enum type and rename to Axis after
 	// updating rsmt2d
@@ -57,7 +57,7 @@ func NewErrByzantine(
 	}
 
 	return &ErrByzantine{
-		Index:  uint8(errByz.Index),
+		Index:  uint32(errByz.Index),
 		Shares: sharesWithProof,
 		IsRow:  errByz.Axis == rsmt2d.Row,
 	}