From f09539f67356bdc25f31e048f2e58ab9b4956af9 Mon Sep 17 00:00:00 2001 From: Viacheslav Gonkivskyi Date: Wed, 28 Jun 2023 16:00:15 +0300 Subject: [PATCH] share/befp: rework collecting and veryfying the BEFP --- share/eds/byzantine/bad_encoding.go | 37 ++++++++++++++------------- share/eds/byzantine/byzantine.go | 39 ++++++++++++++++------------- share/eds/byzantine/share_proof.go | 34 +++++++++++++++++-------- 3 files changed, 66 insertions(+), 44 deletions(-) diff --git a/share/eds/byzantine/bad_encoding.go b/share/eds/byzantine/bad_encoding.go index 0672096b25..7b1d35607f 100644 --- a/share/eds/byzantine/bad_encoding.go +++ b/share/eds/byzantine/bad_encoding.go @@ -118,37 +118,34 @@ func (p *BadEncodingProof) Validate(hdr libhead.Header) error { if header.Height() != int64(p.BlockHeight) { return errors.New("fraud: incorrect block height") } - merkleRowRoots := header.DAH.RowRoots - merkleColRoots := header.DAH.ColumnRoots - if len(merkleRowRoots) != len(merkleColRoots) { + + if len(header.DAH.RowRoots) != len(header.DAH.ColumnRoots) { // NOTE: This should never happen as callers of this method should not feed it with a // malformed extended header. panic(fmt.Sprintf( "fraud: invalid extended header: length of row and column roots do not match. (rowRoots=%d) (colRoots=%d)", - len(merkleRowRoots), - len(merkleColRoots)), + len(header.DAH.RowRoots), + len(header.DAH.ColumnRoots)), ) } - if 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("fraud: invalid proof: incorrect number of shares %d != %d", len(p.Shares), len(merkleRowRoots)) - } - root := merkleRowRoots[p.Index] - if p.Axis == rsmt2d.Col { - root = merkleColRoots[p.Index] + // change the order of dah roots as we are now collecting nmt proofs against orthogonal axis + roots := [][][]byte{header.DAH.ColumnRoots, header.DAH.RowRoots} + if int(p.Index) >= len(roots[rsmt2d.Row]) { + return fmt.Errorf("fraud: invalid proof: index out of bounds (%d >= %d)", int(p.Index), len(roots[rsmt2d.Row])) + } + if len(roots[rsmt2d.Row]) != len(p.Shares) { + return fmt.Errorf("fraud: invalid proof: incorrect number of shares %d != %d", len(p.Shares), len(roots[rsmt2d.Row])) } // verify that Merkle proofs correspond to particular shares. - shares := make([][]byte, len(merkleRowRoots)) + shares := make([][]byte, len(roots[rsmt2d.Row])) for index, shr := range p.Shares { if shr == nil { continue } // validate inclusion of the share into one of the DAHeader roots - if ok := shr.Validate(ipld.MustCidFromNamespacedSha256(root)); !ok { + if ok := shr.Validate(ipld.MustCidFromNamespacedSha256(roots[p.Axis][index])); !ok { return fmt.Errorf("fraud: invalid proof: incorrect share received at index %d", index) } // NMTree commits the additional namespace while rsmt2d does not know about, so we trim it @@ -156,7 +153,7 @@ func (p *BadEncodingProof) Validate(hdr libhead.Header) error { shares[index] = share.GetData(shr.Share) } - odsWidth := uint64(len(merkleRowRoots) / 2) + odsWidth := uint64(len(roots[rsmt2d.Row]) / 2) codec := share.DefaultRSMT2DCodec() // rebuild a row or col. @@ -183,6 +180,12 @@ func (p *BadEncodingProof) Validate(hdr libhead.Header) error { return err } + // root is a merkle root of the row/col where ErrByzantine occurred + root := header.DAH.RowRoots[p.Index] + if p.Axis == rsmt2d.Col { + root = header.DAH.ColumnRoots[p.Index] + } + // comparing rebuilt Merkle Root of bad row/col with respective Merkle Root of row/col from block. if bytes.Equal(expectedRoot, root) { return errors.New("fraud: invalid proof: recomputed Merkle root matches the DAH's row/column root") diff --git a/share/eds/byzantine/byzantine.go b/share/eds/byzantine/byzantine.go index b9c8ef414f..d495506d84 100644 --- a/share/eds/byzantine/byzantine.go +++ b/share/eds/byzantine/byzantine.go @@ -35,25 +35,30 @@ func NewErrByzantine( dah *da.DataAvailabilityHeader, errByz *rsmt2d.ErrByzantineData, ) *ErrByzantine { - root := [][][]byte{ - dah.RowRoots, + // changing the order to collect proofs against orthogonal axis + roots := [][][]byte{ dah.ColumnRoots, - }[errByz.Axis][errByz.Index] - sharesWithProof, err := GetProofsForShares( - ctx, - bGetter, - ipld.MustCidFromNamespacedSha256(root), - errByz.Shares, - ) - if err != nil { - // Fatal as rsmt2d proved that error is byzantine, - // but we cannot properly collect the proof, - // so verification will fail and thus services won't be stopped - // while we still have to stop them. - // TODO(@Wondertan): Find a better way to handle - log.Fatalw("getting proof for ErrByzantine", "err", err) + dah.RowRoots, + }[errByz.Axis] + sharesWithProof := make([]*ShareWithProof, len(errByz.Shares)) + for index, share := range errByz.Shares { + if share != nil { + share, err := getProofsAt( + ctx, bGetter, + ipld.MustCidFromNamespacedSha256(roots[index]), + int(errByz.Index), len(errByz.Shares), + ) + if err != nil { + // Fatal as rsmt2d proved that error is byzantine, + // but we cannot properly collect the proof, + // so verification will fail and thus services won't be stopped + // while we still have to stop them. + // TODO(@Wondertan): Find a better way to handle + log.Fatalw("getting proof for ErrByzantine", "err", err) + } + sharesWithProof[index] = share + } } - return &ErrByzantine{ Index: uint32(errByz.Index), Shares: sharesWithProof, diff --git a/share/eds/byzantine/share_proof.go b/share/eds/byzantine/share_proof.go index b8e39ee1d3..d6aa9dad51 100644 --- a/share/eds/byzantine/share_proof.go +++ b/share/eds/byzantine/share_proof.go @@ -78,24 +78,38 @@ func GetProofsForShares( proofs := make([]*ShareWithProof, len(shares)) for index, share := range shares { if share != nil { - proof := make([]cid.Cid, 0) - // TODO(@vgonkivs): Combine GetLeafData and GetProof in one function as the are traversing the same - // tree. Add options that will control what data will be fetched. - s, err := ipld.GetLeaf(ctx, bGetter, root, index, len(shares)) + proof, err := getProofsAt(ctx, bGetter, root, index, len(shares)) if err != nil { return nil, err } - proof, err = ipld.GetProof(ctx, bGetter, root, proof, index, len(shares)) - if err != nil { - return nil, err - } - proofs[index] = NewShareWithProof(index, s.RawData(), proof) + proofs[index] = proof } } - return proofs, nil } +func getProofsAt( + ctx context.Context, + bGetter blockservice.BlockGetter, + root cid.Cid, + index, + total int, +) (*ShareWithProof, error) { + proof := make([]cid.Cid, 0) + // TODO(@vgonkivs): Combine GetLeafData and GetProof in one function as the are traversing the same + // tree. Add options that will control what data will be fetched. + node, err := ipld.GetLeaf(ctx, bGetter, root, index, total) + if err != nil { + return nil, err + } + + proof, err = ipld.GetProof(ctx, bGetter, root, proof, index, total) + if err != nil { + return nil, err + } + return NewShareWithProof(index, node.RawData(), proof), nil +} + func ProtoToShare(protoShares []*pb.Share) []*ShareWithProof { shares := make([]*ShareWithProof, len(protoShares)) for i, share := range protoShares {