From 2005566dbee3f67e239d2df5fbd0ee067ea518dd Mon Sep 17 00:00:00 2001 From: sweexordious Date: Wed, 31 Jul 2024 02:05:51 +0400 Subject: [PATCH 01/46] fix: use a proof to the data root of blob --- blob/blob.go | 115 ++++++++++++++++++----- blob/service.go | 219 ++++++++++++++++++++++++++----------------- blob/service_test.go | 42 ++++----- go.mod | 2 +- go.sum | 4 +- 5 files changed, 247 insertions(+), 135 deletions(-) diff --git a/blob/blob.go b/blob/blob.go index 610b0cba96..e7453ecaca 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -5,54 +5,119 @@ import ( "encoding/json" "errors" "fmt" - - tmproto "github.com/tendermint/tendermint/proto/tendermint/types" + "math" "github.com/celestiaorg/celestia-app/pkg/appconsts" "github.com/celestiaorg/celestia-app/x/blob/types" - "github.com/celestiaorg/nmt" - "github.com/celestiaorg/celestia-node/share" + "github.com/celestiaorg/nmt" + "github.com/tendermint/tendermint/pkg/consts" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" + coretypes "github.com/tendermint/tendermint/types" ) var errEmptyShares = errors.New("empty shares") -// The Proof is a set of nmt proofs that can be verified only through -// the included method (due to limitation of the nmt https://github.com/celestiaorg/nmt/issues/218). -// Proof proves the WHOLE namespaced data to the row roots. -// TODO (@vgonkivs): rework `Proof` in order to prove a particular blob. -// https://github.com/celestiaorg/celestia-node/issues/2303 -type Proof []*nmt.Proof - -func (p Proof) Len() int { return len(p) } +// Proof constructs the proof of a blob to the data root. +type Proof struct { + // ShareToRowRootProof the proofs of the shares to the row roots they belong to. + // If the blob spans across multiple rows, then this will contain multiple proofs. + ShareToRowRootProof []*tmproto.NMTProof + // RowProof the proofs of the row roots containing the blob shares + // to the data root. + RowProof coretypes.RowProof +} // equal is a temporary method that compares two proofs. // should be removed in BlobService V1. -func (p Proof) equal(input Proof) error { - if p.Len() != input.Len() { - return ErrInvalidProof - } - - for i, proof := range p { - pNodes := proof.Nodes() - inputNodes := input[i].Nodes() - for i, node := range pNodes { - if !bytes.Equal(node, inputNodes[i]) { +func (p *Proof) equal(input *Proof) error { + // compare the NMT proofs + for i, proof := range p.ShareToRowRootProof { + if proof.Start != input.ShareToRowRootProof[i].Start { + // TODO(@rach-id): should we define specific errors for each case? that's better + // to know which part is not equal. + // Also, this error should be ErrProofNotEqual or similar, and not ErrInvalidProof + return ErrInvalidProof + } + if proof.End != input.ShareToRowRootProof[i].End { + // TODO(@rach-id): should we define specific errors for each case? that's better + // to know which part is not equal. + // Also, this error should be ErrProofNotEqual or similar, and not ErrInvalidProof + return ErrInvalidProof + } + for j, node := range proof.Nodes { + if !bytes.Equal(node, input.ShareToRowRootProof[i].Nodes[j]) { return ErrInvalidProof } } + } - if proof.Start() != input[i].Start() || proof.End() != input[i].End() { + // compare the row proof + if p.RowProof.StartRow != input.RowProof.StartRow { + return ErrInvalidProof + } + if p.RowProof.EndRow != input.RowProof.EndRow { + return ErrInvalidProof + } + for index, rowRoot := range p.RowProof.RowRoots { + if !bytes.Equal(rowRoot, input.RowProof.RowRoots[index]) { return ErrInvalidProof } - - if !bytes.Equal(proof.LeafHash(), input[i].LeafHash()) { + } + for i, proof := range p.RowProof.Proofs { + if proof.Index != input.RowProof.Proofs[i].Index { return ErrInvalidProof } + if proof.Total != input.RowProof.Proofs[i].Total { + return ErrInvalidProof + } + for j, aunt := range proof.Aunts { + if !bytes.Equal(aunt, input.RowProof.Proofs[i].Aunts[j]) { + return ErrInvalidProof + } + } } return nil } +// TODO(@rach-id): rename to Verify +func (p *Proof) VerifyProof(rawShares [][]byte, namespace share.Namespace, dataRoot []byte) (bool, error) { + // verify the row proof + if !p.RowProof.VerifyProof(dataRoot) { + return false, errors.New("invalid row root to data root proof") + } + + // verify the share proof + cursor := int32(0) + for i, proof := range p.ShareToRowRootProof { + nmtProof := nmt.NewInclusionProof( + int(proof.Start), + int(proof.End), + proof.Nodes, + true, + ) + sharesUsed := proof.End - proof.Start + if namespace.Version() > math.MaxUint8 { + return false, errors.New("invalid namespace version") + } + ns := append([]byte{namespace.Version()}, namespace.ID()...) + if len(rawShares) < int(sharesUsed+cursor) { + return false, errors.New("invalid number of shares") + } + valid := nmtProof.VerifyInclusion( + consts.NewBaseHashFunc(), + ns, + rawShares[cursor:sharesUsed+cursor], + p.RowProof.RowRoots[i], + ) + if !valid { + return false, nil + } + cursor += sharesUsed + } + return true, nil +} + // Blob represents any application-specific binary data that anyone can submit to Celestia. type Blob struct { types.Blob `json:"blob"` diff --git a/blob/service.go b/blob/service.go index 70714d53b6..d90ae24b33 100644 --- a/blob/service.go +++ b/blob/service.go @@ -9,6 +9,10 @@ import ( "slices" "sync" + "github.com/tendermint/tendermint/crypto/merkle" + "github.com/tendermint/tendermint/libs/bytes" + core "github.com/tendermint/tendermint/types" + "github.com/cosmos/cosmos-sdk/types" logging "github.com/ipfs/go-log/v2" "go.opentelemetry.io/otel" @@ -221,7 +225,7 @@ func (s *Service) Included( default: return false, err } - return true, resProof.equal(*proof) + return true, resProof.equal(proof) } // retrieve retrieves blobs and their proofs by requesting the whole namespace and @@ -249,123 +253,170 @@ func (s *Service) retrieve( headerGetterSpan.AddEvent("received eds", trace.WithAttributes( attribute.Int64("eds-size", int64(len(header.DAH.RowRoots))))) - rowIndex := -1 + // find the index of the row where the blob could start + startRowIndex := -1 for i, row := range header.DAH.RowRoots { if !namespace.IsOutsideRange(row, row) { - rowIndex = i + startRowIndex = i break } } + if startRowIndex == -1 { + return nil, nil, ErrBlobNotFound + } - getCtx, getSharesSpan := tracer.Start(ctx, "get-shares-by-namespace") + // end exclusive index of the row root containing the namespace + endRowIndex := startRowIndex + for i, row := range header.DAH.RowRoots[startRowIndex:] { + if namespace.IsOutsideRange(row, row) { + endRowIndex = startRowIndex + i + break + } + } - // collect shares for the requested namespace - namespacedShares, err := s.shareGetter.GetSharesByNamespace(getCtx, header, namespace) - if err != nil { - if errors.Is(err, share.ErrNotFound) { - err = ErrBlobNotFound + // calculate the square size + squareSize := len(header.DAH.RowRoots) / 2 + + // get all the shares of the rows containing the namespace + getCtx, getSharesSpan := tracer.Start(ctx, "get-all-shares-in-namespace") + // store the ODS shares of the rows containing the blob + rowsShares := make([]share.Share, 0, (endRowIndex-startRowIndex)*squareSize) + // store the EDS shares of the rows containing the blob + rowsWithParityShares := make([][]shares.Share, endRowIndex-startRowIndex) + for rowIndex := startRowIndex; rowIndex < endRowIndex; rowIndex++ { + for colIndex := 0; colIndex < squareSize*2; colIndex++ { + share, err := s.shareGetter.GetShare(ctx, header, rowIndex, colIndex) + if err != nil { + return nil, nil, err + } + if colIndex < squareSize { + rowsShares = append(rowsShares, share) + } + appShare, err := toAppShares(share) + if err != nil { + return nil, nil, err + } + rowsWithParityShares[rowIndex-startRowIndex] = append(rowsWithParityShares[rowIndex-startRowIndex], appShare[0]) } - getSharesSpan.SetStatus(codes.Error, err.Error()) - return nil, nil, err } getSharesSpan.SetStatus(codes.Ok, "") getSharesSpan.AddEvent("received shares", trace.WithAttributes( - attribute.Int64("eds-size", int64(len(header.DAH.RowRoots))))) - - var ( - appShares = make([]shares.Share, 0) - proofs = make(Proof, 0) - ) + attribute.Int64("eds-size", int64(squareSize*2)))) - for _, row := range namespacedShares { - if len(row.Shares) == 0 { - // the above condition means that we've faced with an Absence Proof. - // This Proof proves that the namespace was not found in the DAH, so - // we can return `ErrBlobNotFound`. - return nil, nil, ErrBlobNotFound + // go over the shares until finding the requested blobs + for currentShareIndex := 0; currentShareIndex < len(rowsShares); { + currentShareApp, err := shares.NewShare(rowsShares[currentShareIndex]) + if err != nil { + return nil, nil, err } - appShares, err = toAppShares(row.Shares...) + // skip if it's a padding share + isPadding, err := currentShareApp.IsPadding() if err != nil { return nil, nil, err } - - proofs = append(proofs, row.Proof) - index := row.Proof.Start() - - for { - var ( - isComplete bool - shrs []shares.Share - wasEmpty = sharesParser.isEmpty() - ) - - if wasEmpty { - // create a parser if it is empty - shrs, err = sharesParser.set(rowIndex*len(header.DAH.RowRoots)+index, appShares) - if err != nil { - if errors.Is(err, errEmptyShares) { - // reset parser as `skipPadding` can update next blob's index - sharesParser.reset() - appShares = nil - break - } - return nil, nil, err - } - - // update index and shares if padding shares were detected. - if len(appShares) != len(shrs) { - index += len(appShares) - len(shrs) - appShares = shrs - } + if isPadding { + currentShareIndex++ + continue + } + isSequenceStart, err := currentShareApp.IsSequenceStart() + if err != nil { + return nil, nil, err + } + if isSequenceStart { + // calculate the blob length + sequenceLength, err := currentShareApp.SequenceLen() + if err != nil { + return nil, nil, err + } + blobLen := shares.SparseSharesNeeded(sequenceLength) + + endShareIndex := currentShareIndex + blobLen + if endShareIndex > len(rowsShares) { + // this blob spans to the next row which has a namespace > requested namespace. + // this means that we can stop. + // TODO(@rach-id): should we return an error here? I don't think so + // since the share parser verifier sometimes returns false while getting the shares. + return nil, nil, ErrBlobNotFound + } + // convert the blob shares to app shares + blobShares := rowsShares[currentShareIndex:endShareIndex] + appBlobShares, err := toAppShares(blobShares...) + if err != nil { + return nil, nil, err } - shrs, isComplete = sharesParser.addShares(appShares) - // move to the next row if the blob is incomplete + // parse the blob + sharesParser.length = blobLen + _, isComplete := sharesParser.addShares(appBlobShares) if !isComplete { - appShares = nil - break + return nil, nil, fmt.Errorf("expected the shares to construct a full blob") } - // otherwise construct blob blob, err := sharesParser.parse() if err != nil { return nil, nil, err } + // setting the index manually since we didn't use the parser.set() method + blob.index = currentShareIndex%squareSize + (startRowIndex+currentShareIndex/squareSize)*squareSize*2 + if sharesParser.verify(blob) { - return blob, &proofs, nil - } + // now that we found the requested blob, we will create + // its inclusion proof. + blobStartRow := startRowIndex + currentShareIndex/squareSize + blobEndRow := startRowIndex + (currentShareIndex+blobLen)/squareSize + + // create the row roots to data root inclusion proof + rowProofs := proveRowRootsToDataRoot( + append(header.DAH.RowRoots, header.DAH.ColumnRoots...), + blobStartRow, + blobEndRow+1, + ) + rowRoots := make([]bytes.HexBytes, blobEndRow-blobStartRow+1) + for index, rowRoot := range header.DAH.RowRoots[blobStartRow : blobEndRow+1] { + rowRoots[index] = rowRoot + } - index += len(appShares) - len(shrs) - appShares = shrs - sharesParser.reset() + // create the share to row root proofs + shareToRowRootProofs, _, err := pkgproof.CreateShareToRowRootProofs( + squareSize, + rowsWithParityShares, + header.DAH.RowRoots[blobStartRow:blobEndRow+1], + currentShareIndex, + (currentShareIndex+blobLen)%squareSize, + ) + if err != nil { + return nil, nil, err + } - if !wasEmpty { - // remove proofs for prev rows if verified blob spans multiple rows - proofs = proofs[len(proofs)-1:] + proof := Proof{ + ShareToRowRootProof: shareToRowRootProofs, + RowProof: core.RowProof{ + RowRoots: rowRoots, + Proofs: rowProofs, + StartRow: uint32(blobStartRow), + EndRow: uint32(blobEndRow), + }, + } + return blob, &proof, nil } - } - - rowIndex++ - if sharesParser.isEmpty() { - proofs = nil + sharesParser.reset() + currentShareIndex += blobLen + } else { + // this is a continuation of a previous blob + // we can skip + currentShareIndex++ } } + return nil, nil, ErrBlobNotFound +} - err = ErrBlobNotFound - for _, sh := range appShares { - ok, err := sh.IsPadding() - if err != nil { - return nil, nil, err - } - if !ok { - err = fmt.Errorf("incomplete blob with the "+ - "namespace: %s detected at %d: %w", namespace.String(), height, err) - log.Error(err) - } - } - return nil, nil, err +// proveRowRootsToDataRoot creates a set of binary merkle proofs for all the +// roots defined by the range [start, end). +func proveRowRootsToDataRoot(roots [][]byte, start int, end int) []*merkle.Proof { + _, proofs := merkle.ProofsFromByteSlices(roots) + return proofs[start:end] } // getBlobs retrieves the DAH and fetches all shares from the requested Namespace and converts diff --git a/blob/service_test.go b/blob/service_test.go index efbc6a65bf..1fee8f47b0 100644 --- a/blob/service_test.go +++ b/blob/service_test.go @@ -102,7 +102,7 @@ func TestBlobService_Get(t *testing.T) { } }, }, - { + { // not working name: "verify indexes", doFn: func() (interface{}, error) { b0, err := service.Get(ctx, 1, @@ -141,9 +141,10 @@ func TestBlobService_Get(t *testing.T) { row, col := calculateIndex(len(h.DAH.RowRoots), blobs[i].index) sh, err := service.shareGetter.GetShare(ctx, h, row, col) require.NoError(t, err) - require.True(t, bytes.Equal(sh, resultShares[shareOffset]), - fmt.Sprintf("issue on %d attempt. ROW:%d, COL: %d, blobIndex:%d", i, row, col, blobs[i].index), - ) + assert.Equal(t, sh, resultShares[shareOffset]) + //require.True(t, bytes.Equal(sh, resultShares[shareOffset]), + // fmt.Sprintf("issue on %d attempt. ROW:%d, COL: %d, blobIndex:%d", i, row, col, blobs[i].index), + //) shareOffset += shares.SparseSharesNeeded(uint32(len(blobs[i].Data))) } }, @@ -209,7 +210,7 @@ func TestBlobService_Get(t *testing.T) { assert.Empty(t, blobs[0]) }, }, - { + { // not working name: "get proof", doFn: func() (interface{}, error) { proof, err := service.GetProof(ctx, 1, @@ -228,18 +229,9 @@ func TestBlobService_Get(t *testing.T) { assert.True(t, ok) verifyFn := func(t *testing.T, rawShares [][]byte, proof *Proof, namespace share.Namespace) { - for _, row := range header.DAH.RowRoots { - to := 0 - for _, p := range *proof { - from := to - to = p.End() - p.Start() + from - eq := p.VerifyInclusion(share.NewSHA256Hasher(), namespace.ToNMT(), rawShares[from:to], row) - if eq == true { - return - } - } - } - t.Fatal("could not prove the shares") + valid, err := proof.VerifyProof(rawShares, namespace, header.DataHash) + require.NoError(t, err) + require.True(t, valid) } rawShares, err := BlobsToShares(blobsWithDiffNamespaces[1]) @@ -342,7 +334,7 @@ func TestBlobService_Get(t *testing.T) { originalDataWidth := len(h.DAH.RowRoots) / 2 sizes := []int{blobSize0, blobSize1} for i, proof := range proofs { - require.True(t, sizes[i]/originalDataWidth+1 == proof.Len()) + require.True(t, sizes[i]/originalDataWidth+1 == len(proof.ShareToRowRootProof)) } }, }, @@ -382,7 +374,7 @@ func TestBlobService_Get(t *testing.T) { blobsWithDiffNamespaces[1].Commitment, ) require.NoError(t, err) - require.NoError(t, proof.equal(*newProof)) + require.NoError(t, proof.equal(newProof)) }, }, { @@ -392,13 +384,17 @@ func TestBlobService_Get(t *testing.T) { shareService := service.shareGetter shareGetterMock := shareMock.NewMockModule(ctrl) shareGetterMock.EXPECT(). - GetSharesByNamespace(gomock.Any(), gomock.Any(), gomock.Any()). + GetShare(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). DoAndReturn( - func(ctx context.Context, h *header.ExtendedHeader, ns share.Namespace) (share.NamespacedShares, error) { - if ns.Equals(blobsWithDiffNamespaces[0].Namespace()) { + func(ctx context.Context, header *header.ExtendedHeader, row, col int) (share.Share, error) { + sh, err := shareService.GetShare(ctx, header, row, col) + if err != nil { + return nil, err + } + if share.GetNamespace(sh).Equals(blobsWithDiffNamespaces[0].Namespace()) { return nil, errors.New("internal error") } - return shareService.GetSharesByNamespace(ctx, h, ns) + return shareService.GetShare(ctx, header, row, col) }).AnyTimes() service.shareGetter = shareGetterMock diff --git a/go.mod b/go.mod index 2143209f86..0feddeeb7b 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/BurntSushi/toml v1.4.0 github.com/alecthomas/jsonschema v0.0.0-20220216202328-9eeeec9d044b github.com/benbjohnson/clock v1.3.5 - github.com/celestiaorg/celestia-app v1.13.0 + github.com/celestiaorg/celestia-app v1.14.0 github.com/celestiaorg/go-fraud v0.2.1 github.com/celestiaorg/go-header v0.6.2 github.com/celestiaorg/go-libp2p-messenger v0.2.0 diff --git a/go.sum b/go.sum index 154d7dc132..7d37d5bfe5 100644 --- a/go.sum +++ b/go.sum @@ -353,8 +353,8 @@ github.com/buger/jsonparser v0.0.0-20181115193947-bf1c66bbce23/go.mod h1:bbYlZJ7 github.com/bwesterb/go-ristretto v1.2.0/go.mod h1:fUIoIZaG73pV5biE2Blr2xEzDoMj7NFEuV9ekS419A0= github.com/c-bata/go-prompt v0.2.2/go.mod h1:VzqtzE2ksDBcdln8G7mk2RX9QyGjH+OVqOCSiVIqS34= github.com/casbin/casbin/v2 v2.1.2/go.mod h1:YcPU1XXisHhLzuxH9coDNf2FbKpjGlbCg3n9yuLkIJQ= -github.com/celestiaorg/celestia-app v1.13.0 h1:7MWEox6lim6WDyiP84Y2/ERfWUJxWPfZlKxzO6OFcig= -github.com/celestiaorg/celestia-app v1.13.0/go.mod h1:CF9VZwWAlTU0Is/BOsmxqkbkYnnmrgl0YRlSBIzr0m0= +github.com/celestiaorg/celestia-app v1.14.0 h1:Av1Q8de41WRABQ+mnwHwj4a6Rd5sqEUZqrTwuA1LbBM= +github.com/celestiaorg/celestia-app v1.14.0/go.mod h1:OPkbzIvBUGwTvfunQ/uh7qEekjkix59kB0CsK8+i6uM= github.com/celestiaorg/celestia-core v1.38.0-tm-v0.34.29 h1:HwbA4OegRvXX0aNchBA7Cmu+oIxnH7xRcOhISuDP0ak= github.com/celestiaorg/celestia-core v1.38.0-tm-v0.34.29/go.mod h1:MyElURdWAOJkOp84WZnfEUJ+OLvTwOOHG2lbK9E8XRI= github.com/celestiaorg/cosmos-sdk v1.23.0-sdk-v0.46.16 h1:N2uETI13szEKnGAdKhtTR0EsrpcW0AwRKYER74WLnuw= From 93f99984b08ba790cb0a28fd270d17afa792616b Mon Sep 17 00:00:00 2001 From: sweexordious Date: Wed, 31 Jul 2024 15:24:10 +0400 Subject: [PATCH 02/46] fix: row indexes --- blob/service.go | 44 ++++++++++++++++++++++---------------------- blob/service_test.go | 9 ++++----- 2 files changed, 26 insertions(+), 27 deletions(-) diff --git a/blob/service.go b/blob/service.go index d90ae24b33..ec64751e8e 100644 --- a/blob/service.go +++ b/blob/service.go @@ -254,22 +254,22 @@ func (s *Service) retrieve( attribute.Int64("eds-size", int64(len(header.DAH.RowRoots))))) // find the index of the row where the blob could start - startRowIndex := -1 + inclusiveNamespaceStartRowIndex := -1 for i, row := range header.DAH.RowRoots { if !namespace.IsOutsideRange(row, row) { - startRowIndex = i + inclusiveNamespaceStartRowIndex = i break } } - if startRowIndex == -1 { + if inclusiveNamespaceStartRowIndex == -1 { return nil, nil, ErrBlobNotFound } // end exclusive index of the row root containing the namespace - endRowIndex := startRowIndex - for i, row := range header.DAH.RowRoots[startRowIndex:] { + exclusiveNamespaceEndRowIndex := inclusiveNamespaceStartRowIndex + for i, row := range header.DAH.RowRoots[inclusiveNamespaceStartRowIndex:] { if namespace.IsOutsideRange(row, row) { - endRowIndex = startRowIndex + i + exclusiveNamespaceEndRowIndex = inclusiveNamespaceStartRowIndex + i break } } @@ -280,10 +280,10 @@ func (s *Service) retrieve( // get all the shares of the rows containing the namespace getCtx, getSharesSpan := tracer.Start(ctx, "get-all-shares-in-namespace") // store the ODS shares of the rows containing the blob - rowsShares := make([]share.Share, 0, (endRowIndex-startRowIndex)*squareSize) + rowsShares := make([]share.Share, 0, (exclusiveNamespaceEndRowIndex-inclusiveNamespaceStartRowIndex)*squareSize) // store the EDS shares of the rows containing the blob - rowsWithParityShares := make([][]shares.Share, endRowIndex-startRowIndex) - for rowIndex := startRowIndex; rowIndex < endRowIndex; rowIndex++ { + rowsWithParityShares := make([][]shares.Share, exclusiveNamespaceEndRowIndex-inclusiveNamespaceStartRowIndex) + for rowIndex := inclusiveNamespaceStartRowIndex; rowIndex < exclusiveNamespaceEndRowIndex; rowIndex++ { for colIndex := 0; colIndex < squareSize*2; colIndex++ { share, err := s.shareGetter.GetShare(ctx, header, rowIndex, colIndex) if err != nil { @@ -296,7 +296,7 @@ func (s *Service) retrieve( if err != nil { return nil, nil, err } - rowsWithParityShares[rowIndex-startRowIndex] = append(rowsWithParityShares[rowIndex-startRowIndex], appShare[0]) + rowsWithParityShares[rowIndex-inclusiveNamespaceStartRowIndex] = append(rowsWithParityShares[rowIndex-inclusiveNamespaceStartRowIndex], appShare[0]) } } @@ -359,31 +359,31 @@ func (s *Service) retrieve( } // setting the index manually since we didn't use the parser.set() method - blob.index = currentShareIndex%squareSize + (startRowIndex+currentShareIndex/squareSize)*squareSize*2 + blob.index = currentShareIndex%squareSize + (inclusiveNamespaceStartRowIndex+currentShareIndex/squareSize)*squareSize*2 if sharesParser.verify(blob) { // now that we found the requested blob, we will create // its inclusion proof. - blobStartRow := startRowIndex + currentShareIndex/squareSize - blobEndRow := startRowIndex + (currentShareIndex+blobLen)/squareSize + inclusiveBlobStartRowIndex := inclusiveNamespaceStartRowIndex + currentShareIndex/squareSize + exclusiveBlobEndRowIndex := inclusiveBlobStartRowIndex + blobLen/squareSize + 1 // create the row roots to data root inclusion proof rowProofs := proveRowRootsToDataRoot( append(header.DAH.RowRoots, header.DAH.ColumnRoots...), - blobStartRow, - blobEndRow+1, + inclusiveBlobStartRowIndex, + exclusiveBlobEndRowIndex, ) - rowRoots := make([]bytes.HexBytes, blobEndRow-blobStartRow+1) - for index, rowRoot := range header.DAH.RowRoots[blobStartRow : blobEndRow+1] { + rowRoots := make([]bytes.HexBytes, exclusiveBlobEndRowIndex-inclusiveBlobStartRowIndex) + for index, rowRoot := range header.DAH.RowRoots[inclusiveBlobStartRowIndex:exclusiveBlobEndRowIndex] { rowRoots[index] = rowRoot } // create the share to row root proofs shareToRowRootProofs, _, err := pkgproof.CreateShareToRowRootProofs( squareSize, - rowsWithParityShares, - header.DAH.RowRoots[blobStartRow:blobEndRow+1], - currentShareIndex, + rowsWithParityShares[inclusiveBlobStartRowIndex-inclusiveNamespaceStartRowIndex:exclusiveBlobEndRowIndex-inclusiveNamespaceStartRowIndex], + header.DAH.RowRoots[inclusiveBlobStartRowIndex:exclusiveBlobEndRowIndex], + currentShareIndex%squareSize, (currentShareIndex+blobLen)%squareSize, ) if err != nil { @@ -395,8 +395,8 @@ func (s *Service) retrieve( RowProof: core.RowProof{ RowRoots: rowRoots, Proofs: rowProofs, - StartRow: uint32(blobStartRow), - EndRow: uint32(blobEndRow), + StartRow: uint32(inclusiveBlobStartRowIndex), + EndRow: uint32(exclusiveBlobEndRowIndex), }, } return blob, &proof, nil diff --git a/blob/service_test.go b/blob/service_test.go index 1fee8f47b0..f0edadf13e 100644 --- a/blob/service_test.go +++ b/blob/service_test.go @@ -102,7 +102,7 @@ func TestBlobService_Get(t *testing.T) { } }, }, - { // not working + { name: "verify indexes", doFn: func() (interface{}, error) { b0, err := service.Get(ctx, 1, @@ -141,10 +141,9 @@ func TestBlobService_Get(t *testing.T) { row, col := calculateIndex(len(h.DAH.RowRoots), blobs[i].index) sh, err := service.shareGetter.GetShare(ctx, h, row, col) require.NoError(t, err) - assert.Equal(t, sh, resultShares[shareOffset]) - //require.True(t, bytes.Equal(sh, resultShares[shareOffset]), - // fmt.Sprintf("issue on %d attempt. ROW:%d, COL: %d, blobIndex:%d", i, row, col, blobs[i].index), - //) + require.True(t, bytes.Equal(sh, resultShares[shareOffset]), + fmt.Sprintf("issue on %d attempt. ROW:%d, COL: %d, blobIndex:%d", i, row, col, blobs[i].index), + ) shareOffset += shares.SparseSharesNeeded(uint32(len(blobs[i].Data))) } }, From cdac209fa7945276f76de7ded88d9e5c8e1ef310 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Wed, 31 Jul 2024 19:21:41 +0400 Subject: [PATCH 03/46] fix: end share index --- blob/blob.go | 10 ++++++---- blob/service.go | 17 +++++++++++------ nodebuilder/blobstream/data_root_tuple_root.go | 3 +-- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/blob/blob.go b/blob/blob.go index e7453ecaca..455b664449 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -7,13 +7,15 @@ import ( "fmt" "math" - "github.com/celestiaorg/celestia-app/pkg/appconsts" - "github.com/celestiaorg/celestia-app/x/blob/types" - "github.com/celestiaorg/celestia-node/share" - "github.com/celestiaorg/nmt" "github.com/tendermint/tendermint/pkg/consts" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" coretypes "github.com/tendermint/tendermint/types" + + "github.com/celestiaorg/celestia-app/pkg/appconsts" + "github.com/celestiaorg/celestia-app/x/blob/types" + "github.com/celestiaorg/nmt" + + "github.com/celestiaorg/celestia-node/share" ) var errEmptyShares = errors.New("empty shares") diff --git a/blob/service.go b/blob/service.go index ec64751e8e..77ec749aa3 100644 --- a/blob/service.go +++ b/blob/service.go @@ -9,12 +9,11 @@ import ( "slices" "sync" + "github.com/cosmos/cosmos-sdk/types" + logging "github.com/ipfs/go-log/v2" "github.com/tendermint/tendermint/crypto/merkle" "github.com/tendermint/tendermint/libs/bytes" core "github.com/tendermint/tendermint/types" - - "github.com/cosmos/cosmos-sdk/types" - logging "github.com/ipfs/go-log/v2" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" @@ -365,7 +364,13 @@ func (s *Service) retrieve( // now that we found the requested blob, we will create // its inclusion proof. inclusiveBlobStartRowIndex := inclusiveNamespaceStartRowIndex + currentShareIndex/squareSize - exclusiveBlobEndRowIndex := inclusiveBlobStartRowIndex + blobLen/squareSize + 1 + exclusiveBlobEndRowIndex := inclusiveNamespaceStartRowIndex + endShareIndex/squareSize + if (currentShareIndex+blobLen)%squareSize != 0 { + // if the row is not complete with the blob shares, + // then we increment the exclusive blob end row index + // so that it's exclusive. + exclusiveBlobEndRowIndex++ + } // create the row roots to data root inclusion proof rowProofs := proveRowRootsToDataRoot( @@ -384,7 +389,7 @@ func (s *Service) retrieve( rowsWithParityShares[inclusiveBlobStartRowIndex-inclusiveNamespaceStartRowIndex:exclusiveBlobEndRowIndex-inclusiveNamespaceStartRowIndex], header.DAH.RowRoots[inclusiveBlobStartRowIndex:exclusiveBlobEndRowIndex], currentShareIndex%squareSize, - (currentShareIndex+blobLen)%squareSize, + (endShareIndex-1)%squareSize, ) if err != nil { return nil, nil, err @@ -414,7 +419,7 @@ func (s *Service) retrieve( // proveRowRootsToDataRoot creates a set of binary merkle proofs for all the // roots defined by the range [start, end). -func proveRowRootsToDataRoot(roots [][]byte, start int, end int) []*merkle.Proof { +func proveRowRootsToDataRoot(roots [][]byte, start, end int) []*merkle.Proof { _, proofs := merkle.ProofsFromByteSlices(roots) return proofs[start:end] } diff --git a/nodebuilder/blobstream/data_root_tuple_root.go b/nodebuilder/blobstream/data_root_tuple_root.go index de2d60c4fa..5596ee2723 100644 --- a/nodebuilder/blobstream/data_root_tuple_root.go +++ b/nodebuilder/blobstream/data_root_tuple_root.go @@ -6,11 +6,10 @@ import ( "fmt" "strconv" - nodeheader "github.com/celestiaorg/celestia-node/header" - "github.com/tendermint/tendermint/crypto/merkle" "github.com/tendermint/tendermint/libs/bytes" + nodeheader "github.com/celestiaorg/celestia-node/header" "github.com/celestiaorg/celestia-node/nodebuilder/header" ) From 5e01836d1c5b75d08e5f7f7a18412a0f12a332b0 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Wed, 31 Jul 2024 19:51:05 +0400 Subject: [PATCH 04/46] docs: verify --- blob/blob.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/blob/blob.go b/blob/blob.go index 1c39060133..7f96da4004 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -83,8 +83,9 @@ func (p *Proof) equal(input *Proof) error { return nil } -// TODO(@rach-id): rename to Verify -func (p *Proof) VerifyProof(rawShares [][]byte, namespace share.Namespace, dataRoot []byte) (bool, error) { +// Verify takes a set of shares, a namespace and a data root, and verifies if the +// provided shares are committed to by the data root. +func (p *Proof) Verify(rawShares [][]byte, namespace share.Namespace, dataRoot []byte) (bool, error) { // verify the row proof if !p.RowProof.VerifyProof(dataRoot) { return false, errors.New("invalid row root to data root proof") From c6db926cc0674b5856052463aa7db3b1e36d1f15 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Wed, 31 Jul 2024 19:52:05 +0400 Subject: [PATCH 05/46] docs: verify --- blob/service.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/blob/service.go b/blob/service.go index ffbbc10b6b..92072f161f 100644 --- a/blob/service.go +++ b/blob/service.go @@ -100,7 +100,8 @@ type SubscriptionResponse struct { // Subscribe returns a channel that will receive SubscriptionResponse objects. // The channel will be closed when the context is canceled or the service is stopped. // Please note that no errors are returned: underlying operations are retried until successful. -// Additionally, not reading from the returned channel will cause the stream to close after 16 messages. +// Additionally, not reading from the returned channel will cause the stream to close after 16 +// messages. func (s *Service) Subscribe(ctx context.Context, ns share.Namespace) (<-chan *SubscriptionResponse, error) { if s.ctx == nil { return nil, fmt.Errorf("service has not been started") From adafd9261e9a06db0b30411eb81a19353aca1754 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Wed, 31 Jul 2024 19:52:10 +0400 Subject: [PATCH 06/46] docs: verify --- blob/service_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/blob/service_test.go b/blob/service_test.go index e8a64e96b9..11f7cf6420 100644 --- a/blob/service_test.go +++ b/blob/service_test.go @@ -209,7 +209,7 @@ func TestBlobService_Get(t *testing.T) { assert.Empty(t, blobs[0]) }, }, - { // not working + { name: "get proof", doFn: func() (interface{}, error) { proof, err := service.GetProof(ctx, 1, @@ -228,7 +228,7 @@ func TestBlobService_Get(t *testing.T) { assert.True(t, ok) verifyFn := func(t *testing.T, rawShares [][]byte, proof *Proof, namespace share.Namespace) { - valid, err := proof.VerifyProof(rawShares, namespace, header.DataHash) + valid, err := proof.Verify(rawShares, namespace, header.DataHash) require.NoError(t, err) require.True(t, valid) } From 1e41c42faf9a08d824df1d3cb21ca7304000216e Mon Sep 17 00:00:00 2001 From: sweexordious Date: Wed, 31 Jul 2024 19:52:10 +0400 Subject: [PATCH 07/46] docs: verify --- blob/service_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/blob/service_test.go b/blob/service_test.go index e8a64e96b9..11f7cf6420 100644 --- a/blob/service_test.go +++ b/blob/service_test.go @@ -209,7 +209,7 @@ func TestBlobService_Get(t *testing.T) { assert.Empty(t, blobs[0]) }, }, - { // not working + { name: "get proof", doFn: func() (interface{}, error) { proof, err := service.GetProof(ctx, 1, @@ -228,7 +228,7 @@ func TestBlobService_Get(t *testing.T) { assert.True(t, ok) verifyFn := func(t *testing.T, rawShares [][]byte, proof *Proof, namespace share.Namespace) { - valid, err := proof.VerifyProof(rawShares, namespace, header.DataHash) + valid, err := proof.Verify(rawShares, namespace, header.DataHash) require.NoError(t, err) require.True(t, valid) } From 45623fa59e072f49ee22ae94e4ecd5edf7e3820a Mon Sep 17 00:00:00 2001 From: sweexordious Date: Wed, 31 Jul 2024 21:18:42 +0400 Subject: [PATCH 08/46] chore: lint --- blob/blob.go | 5 ----- blob/service.go | 10 +++++++--- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/blob/blob.go b/blob/blob.go index 7f96da4004..33685389e0 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -5,8 +5,6 @@ import ( "encoding/json" "errors" "fmt" - "math" - "github.com/tendermint/tendermint/pkg/consts" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" coretypes "github.com/tendermint/tendermint/types" @@ -101,9 +99,6 @@ func (p *Proof) Verify(rawShares [][]byte, namespace share.Namespace, dataRoot [ true, ) sharesUsed := proof.End - proof.Start - if namespace.Version() > math.MaxUint8 { - return false, errors.New("invalid namespace version") - } ns := append([]byte{namespace.Version()}, namespace.ID()...) if len(rawShares) < int(sharesUsed+cursor) { return false, errors.New("invalid number of shares") diff --git a/blob/service.go b/blob/service.go index 92072f161f..ce567ba085 100644 --- a/blob/service.go +++ b/blob/service.go @@ -380,7 +380,7 @@ func (s *Service) retrieve( squareSize := len(header.DAH.RowRoots) / 2 // get all the shares of the rows containing the namespace - getCtx, getSharesSpan := tracer.Start(ctx, "get-all-shares-in-namespace") + _, getSharesSpan := tracer.Start(ctx, "get-all-shares-in-namespace") // store the ODS shares of the rows containing the blob rowsShares := make([]share.Share, 0, (exclusiveNamespaceEndRowIndex-inclusiveNamespaceStartRowIndex)*squareSize) // store the EDS shares of the rows containing the blob @@ -398,7 +398,10 @@ func (s *Service) retrieve( if err != nil { return nil, nil, err } - rowsWithParityShares[rowIndex-inclusiveNamespaceStartRowIndex] = append(rowsWithParityShares[rowIndex-inclusiveNamespaceStartRowIndex], appShare[0]) + rowsWithParityShares[rowIndex-inclusiveNamespaceStartRowIndex] = append( + rowsWithParityShares[rowIndex-inclusiveNamespaceStartRowIndex], + appShare[0], + ) } } @@ -461,7 +464,8 @@ func (s *Service) retrieve( } // setting the index manually since we didn't use the parser.set() method - blob.index = currentShareIndex%squareSize + (inclusiveNamespaceStartRowIndex+currentShareIndex/squareSize)*squareSize*2 + blob.index = currentShareIndex%squareSize + + (inclusiveNamespaceStartRowIndex+currentShareIndex/squareSize)*squareSize*2 if sharesParser.verify(blob) { // now that we found the requested blob, we will create From 9465b5db3900255b2c948f202b8b1c63fd067907 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Wed, 31 Jul 2024 21:26:00 +0400 Subject: [PATCH 09/46] chore: revert unnecessary change --- blob/service.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/blob/service.go b/blob/service.go index ce567ba085..fd4d821c09 100644 --- a/blob/service.go +++ b/blob/service.go @@ -100,8 +100,7 @@ type SubscriptionResponse struct { // Subscribe returns a channel that will receive SubscriptionResponse objects. // The channel will be closed when the context is canceled or the service is stopped. // Please note that no errors are returned: underlying operations are retried until successful. -// Additionally, not reading from the returned channel will cause the stream to close after 16 -// messages. +// Additionally, not reading from the returned channel will cause the stream to close after 16 messages. func (s *Service) Subscribe(ctx context.Context, ns share.Namespace) (<-chan *SubscriptionResponse, error) { if s.ctx == nil { return nil, fmt.Errorf("service has not been started") From 3d071a570c40c7b64cc6f06dae53e07c105cf301 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Wed, 31 Jul 2024 22:52:22 +0400 Subject: [PATCH 10/46] fix: blobsub tests --- blob/blob.go | 1 + blob/service.go | 8 ++++++++ blob/service_test.go | 29 +++++++++++++++++++++++++---- share/eds/edstest/testing.go | 12 +++++++++++- 4 files changed, 45 insertions(+), 5 deletions(-) diff --git a/blob/blob.go b/blob/blob.go index 33685389e0..12b6217374 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "github.com/tendermint/tendermint/pkg/consts" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" coretypes "github.com/tendermint/tendermint/types" diff --git a/blob/service.go b/blob/service.go index fd4d821c09..e3d29a37cf 100644 --- a/blob/service.go +++ b/blob/service.go @@ -424,6 +424,14 @@ func (s *Service) retrieve( currentShareIndex++ continue } + isCompactShare, err := currentShareApp.IsCompactShare() + if err != nil { + return nil, nil, err + } + if isCompactShare { + currentShareIndex++ + continue + } isSequenceStart, err := currentShareApp.IsSequenceStart() if err != nil { return nil, nil, err diff --git a/blob/service_test.go b/blob/service_test.go index 11f7cf6420..0b05189379 100644 --- a/blob/service_test.go +++ b/blob/service_test.go @@ -17,11 +17,14 @@ import ( "github.com/stretchr/testify/require" "github.com/tendermint/tendermint/crypto/merkle" tmrand "github.com/tendermint/tendermint/libs/rand" + coretypes "github.com/tendermint/tendermint/types" "github.com/celestiaorg/celestia-app/pkg/appconsts" appns "github.com/celestiaorg/celestia-app/pkg/namespace" pkgproof "github.com/celestiaorg/celestia-app/pkg/proof" "github.com/celestiaorg/celestia-app/pkg/shares" + "github.com/celestiaorg/celestia-app/pkg/square" + "github.com/celestiaorg/celestia-app/test/util/testfactory" blobtypes "github.com/celestiaorg/celestia-app/x/blob/types" "github.com/celestiaorg/go-header/store" "github.com/celestiaorg/nmt" @@ -854,19 +857,38 @@ func BenchmarkGetByCommitment(b *testing.B) { } } -func createServiceWithSub(ctx context.Context, t testing.TB, blobs []*Blob) *Service { +func createServiceWithSub(ctx context.Context, t *testing.T, blobs []*Blob) *Service { + acc := "test" + kr := testfactory.GenerateKeyring(acc) + signer := blobtypes.NewKeyringSigner(kr, acc, "test") + addr, err := signer.GetSignerInfo().GetAddress() + require.NoError(t, err) + bs := ipld.NewMemBlockservice() batching := ds_sync.MutexWrap(ds.NewMapDatastore()) headerStore, err := store.NewStore[*header.ExtendedHeader](batching) require.NoError(t, err) edsses := make([]*rsmt2d.ExtendedDataSquare, len(blobs)) + for i, blob := range blobs { - rawShares, err := BlobsToShares(blob) + msg, err := blobtypes.NewMsgPayForBlobs( + addr.String(), + &blob.Blob, + ) require.NoError(t, err) - eds, err := ipld.AddShares(ctx, rawShares, bs) + coreTx := edstest.BuildCoreTx(t, signer, msg, &blob.Blob) + dataSquare, err := square.Construct( + coretypes.Txs{coreTx}.ToSliceOfBytes(), + appconsts.LatestVersion, + appconsts.SquareSizeUpperBound(appconsts.LatestVersion), + ) + require.NoError(t, err) + + eds, err := ipld.AddShares(ctx, shares.ToBytes(dataSquare), bs) require.NoError(t, err) edsses[i] = eds } + headers := headertest.ExtendedHeadersFromEdsses(t, edsses) err = headerStore.Init(ctx, headers[0]) @@ -877,7 +899,6 @@ func createServiceWithSub(ctx context.Context, t testing.TB, blobs []*Blob) *Ser fn := func(ctx context.Context, height uint64) (*header.ExtendedHeader, error) { return headers[height-1], nil - // return headerStore.GetByHeight(ctx, height) } fn2 := func(ctx context.Context) (<-chan *header.ExtendedHeader, error) { headerChan := make(chan *header.ExtendedHeader, len(headers)) diff --git a/share/eds/edstest/testing.go b/share/eds/edstest/testing.go index 5f6dcfa6f7..451e4c4e65 100644 --- a/share/eds/edstest/testing.go +++ b/share/eds/edstest/testing.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/require" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" coretypes "github.com/tendermint/tendermint/types" "github.com/celestiaorg/celestia-app/app" @@ -138,7 +139,16 @@ func createTestBlobTransaction( ns := namespace.RandomBlobNamespace() msg, blob := blobfactory.RandMsgPayForBlobsWithNamespaceAndSigner(addr.String(), ns, size) require.NoError(t, err) + cTx := BuildCoreTx(t, signer, msg, blob) + return ns, msg, blob, cTx +} +func BuildCoreTx( + t *testing.T, + signer *types.KeyringSigner, + msg *types.MsgPayForBlobs, + blob *tmproto.Blob, +) coretypes.Tx { builder := signer.NewTxBuilder() stx, err := signer.BuildSignedTx(builder, msg) require.NoError(t, err) @@ -146,5 +156,5 @@ func createTestBlobTransaction( require.NoError(t, err) cTx, err := coretypes.MarshalBlobTx(rawTx, blob) require.NoError(t, err) - return ns, msg, blob, cTx + return cTx } From b97e2dd29d8afd862063d96ff04d555f3e7b8a66 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Wed, 31 Jul 2024 22:56:30 +0400 Subject: [PATCH 11/46] chore: lint --- blob/blob.go | 1 + blob/parser.go | 5 +++++ blob/service.go | 1 + 3 files changed, 7 insertions(+) diff --git a/blob/blob.go b/blob/blob.go index 12b6217374..6fbe0cde6f 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -18,6 +18,7 @@ import ( "github.com/celestiaorg/celestia-node/share" ) +//nolint:unused var errEmptyShares = errors.New("empty shares") // Proof constructs the proof of a blob to the data root. diff --git a/blob/parser.go b/blob/parser.go index 1b5ddc4a7a..c0aec5a5aa 100644 --- a/blob/parser.go +++ b/blob/parser.go @@ -21,6 +21,8 @@ type parser struct { // set tries to find the first blob's share by skipping padding shares and // sets the metadata of the blob(index and length) +// +//nolint:unused func (p *parser) set(index int, shrs []shares.Share) ([]shares.Share, error) { if len(shrs) == 0 { return nil, errEmptyShares @@ -113,6 +115,8 @@ func (p *parser) parse() (*Blob, error) { // skipPadding iterates through the shares until non-padding share will be found. It guarantees that // the returned set of shares will start with non-padding share(or empty set of shares). +// +//nolint:unused func (p *parser) skipPadding(shares []shares.Share) ([]shares.Share, error) { if len(shares) == 0 { return nil, errEmptyShares @@ -144,6 +148,7 @@ func (p *parser) verify(blob *Blob) bool { return p.verifyFn(blob) } +//nolint:unused func (p *parser) isEmpty() bool { return p.index == 0 && p.length == 0 && len(p.shares) == 0 } diff --git a/blob/service.go b/blob/service.go index e3d29a37cf..02eae482a8 100644 --- a/blob/service.go +++ b/blob/service.go @@ -500,6 +500,7 @@ func (s *Service) retrieve( // create the share to row root proofs shareToRowRootProofs, _, err := pkgproof.CreateShareToRowRootProofs( squareSize, + //nolint:lll rowsWithParityShares[inclusiveBlobStartRowIndex-inclusiveNamespaceStartRowIndex:exclusiveBlobEndRowIndex-inclusiveNamespaceStartRowIndex], header.DAH.RowRoots[inclusiveBlobStartRowIndex:exclusiveBlobEndRowIndex], currentShareIndex%squareSize, From 469cc8c66f911b97d0b9a0dc67c4c52d3e951013 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Wed, 31 Jul 2024 23:04:00 +0400 Subject: [PATCH 12/46] chore: remove TODOS --- blob/blob.go | 3 --- blob/service.go | 2 -- 2 files changed, 5 deletions(-) diff --git a/blob/blob.go b/blob/blob.go index 6fbe0cde6f..d7c8612f62 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -43,9 +43,6 @@ func (p *Proof) equal(input *Proof) error { return ErrInvalidProof } if proof.End != input.ShareToRowRootProof[i].End { - // TODO(@rach-id): should we define specific errors for each case? that's better - // to know which part is not equal. - // Also, this error should be ErrProofNotEqual or similar, and not ErrInvalidProof return ErrInvalidProof } for j, node := range proof.Nodes { diff --git a/blob/service.go b/blob/service.go index 02eae482a8..4150d4bd8a 100644 --- a/blob/service.go +++ b/blob/service.go @@ -448,8 +448,6 @@ func (s *Service) retrieve( if endShareIndex > len(rowsShares) { // this blob spans to the next row which has a namespace > requested namespace. // this means that we can stop. - // TODO(@rach-id): should we return an error here? I don't think so - // since the share parser verifier sometimes returns false while getting the shares. return nil, nil, ErrBlobNotFound } // convert the blob shares to app shares From 0ca01c3c0b9f573f499d0c521c57c20a5e14bb71 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Thu, 1 Aug 2024 00:35:43 +0400 Subject: [PATCH 13/46] fix: update Included implementation too --- blob/blob.go | 2 +- blob/service.go | 12 ++++++++++-- blob/service_test.go | 4 +++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/blob/blob.go b/blob/blob.go index d7c8612f62..b6ac41f900 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -109,7 +109,7 @@ func (p *Proof) Verify(rawShares [][]byte, namespace share.Namespace, dataRoot [ p.RowProof.RowRoots[i], ) if !valid { - return false, nil + return false, ErrInvalidProof } cursor += sharesUsed } diff --git a/blob/service.go b/blob/service.go index 4150d4bd8a..7b785a54ce 100644 --- a/blob/service.go +++ b/blob/service.go @@ -318,7 +318,7 @@ func (s *Service) Included( sharesParser := &parser{verifyFn: func(blob *Blob) bool { return blob.compareCommitments(commitment) }} - _, resProof, err := s.retrieve(ctx, height, namespace, sharesParser) + blob, _, err := s.retrieve(ctx, height, namespace, sharesParser) switch { case err == nil: case errors.Is(err, ErrBlobNotFound): @@ -326,7 +326,15 @@ func (s *Service) Included( default: return false, err } - return true, resProof.equal(proof) + appShares, err := BlobsToShares(blob) + if err != nil { + return false, err + } + getter, err := s.headerGetter(ctx, height) + if err != nil { + return false, err + } + return proof.Verify(appShares, namespace, getter.DataHash) } // retrieve retrieves blobs and their proofs by requesting the whole namespace and diff --git a/blob/service_test.go b/blob/service_test.go index 0b05189379..0dd979fe0a 100644 --- a/blob/service_test.go +++ b/blob/service_test.go @@ -281,7 +281,9 @@ func TestBlobService_Get(t *testing.T) { require.ErrorIs(t, err, ErrInvalidProof) included, ok := res.(bool) require.True(t, ok) - require.True(t, included) + // This is supposed to be false not true. + // we're querying blob[1]'s proof and verifying it against blob[0], no? + require.False(t, included) }, }, { From b60fbed5a19c2bfab0304df9fc718d199eb74fd0 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Thu, 1 Aug 2024 15:46:06 +0400 Subject: [PATCH 14/46] chore: update the blob proof example --- api/docgen/exampledata/blobProof.json | 78 +++++++++++++++++---------- 1 file changed, 50 insertions(+), 28 deletions(-) diff --git a/api/docgen/exampledata/blobProof.json b/api/docgen/exampledata/blobProof.json index 1ee87af9e3..a135f19424 100644 --- a/api/docgen/exampledata/blobProof.json +++ b/api/docgen/exampledata/blobProof.json @@ -1,31 +1,53 @@ -[ - { - "end": 8, - "nodes": [ - "/////////////////////////////////////////////////////////////////////////////wuxStDHcZ7+b5byNQMVLJbzBT3wmObsThoQ0sCTjTCP" +{ + "ShareToRowRootProof": [ + { + "start": 3, + "end": 4, + "nodes": [ + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAQ72eTVOUxB9THxFjAEwtTePJQA1b0xcz2f6TJc400Uw", + "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAABBD0CYbGYoGN4q9VfSmeGZeg/h1NDBA/jtXjZrrKRHE6", + "/////////////////////////////////////////////////////////////////////////////8KDE4JDf0N2lZB7DW1Fpasdk/wz4jHOxuBPAk5Vf5ZI" + ] + }, + { + "end": 1, + "nodes": [ + "//////////////////////////////////////7//////////////////////////////////////plEqgR/c4IAVkNdYRWOYOAESD4whneKR54Dz5Dfe4p2", + "//////////////////////////////////////7//////////////////////////////////////lrD0qJ9dspxSO1Yl8NDioZfgOm8Yj63Y+BGDRHlKCRj", + "/////////////////////////////////////////////////////////////////////////////xQyI+g89aM6rhy9rl2eKr0Uc2NPauf3fkLY3Z+gBtuM" + ] + } + ], + "RowProof": { + "row_roots": [ + "00000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000808080808080808BC517066A5A8C81E2A4353DB500EBB3410047A93D2EE8ADF0B6797B9A5519557", + "0000000000000000000000000000000000000000000808080808080808FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFE015AB6AAC6FAF0ABF26F9453AF390FDA3B39EB384F0B71D0170D84CF69CBA2BC" ], - "is_max_namespace_ignored": true - }, - { - "end": 8, - "nodes": [ - "//////////////////////////////////////////////////////////////////////////////n1NeJxPU2bZUAccKZZ+LAu2Wj5ajbVYURV9ojhSKwp" + "proofs": [ + { + "total": 16, + "index": 1, + "leaf_hash": "lJek/BHnKH6PyRB8jlk69F6EY9Tfx2LRanaF74JVciU=", + "aunts": [ + "bLjvftajE6jVsgQQBkV4RUPESRc+v4bhP0Ljf36858Q=", + "QaF9mNskaURxk98S3BExB1PzRAjOqVydrDLvUu0B5/M=", + "K2xW8JJ3Ff4FvtbfZi5ZD/ygnswaNCNIKXsSzbO2Jrc=", + "uySRG/gINLAgGgywJCTiXMlFkfQivF1O1zLg5+RRUP8=" + ] + }, + { + "total": 16, + "index": 2, + "leaf_hash": "h+4ND52kT4qkc9nWW22dIMAK/4YjkC6fBoD01WF0+Uo=", + "aunts": [ + "2x8OISRBMLYJRV8NfTNtVvZUg2F7MtCK5xCZuE9fQwQ=", + "Xvr5IalE2y3pxHjxh5kcHFSRaz4g5MxdOj4NIGwRXY0=", + "K2xW8JJ3Ff4FvtbfZi5ZD/ygnswaNCNIKXsSzbO2Jrc=", + "uySRG/gINLAgGgywJCTiXMlFkfQivF1O1zLg5+RRUP8=" + ] + } ], - "is_max_namespace_ignored": true - }, - { - "end": 8, - "nodes": [ - "/////////////////////////////////////////////////////////////////////////////0xK8BKnzDmwK0HR4ZJvyB4kh3jPPXGxaGPFoga8vPxF" - ], - "is_max_namespace_ignored": true - }, - { - "end": 7, - "nodes": [ - "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAMJ/xGlNMdEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAwn/EaU0x0UTO9HUGKjyjcv5U2gHeSjJ8S1rftqv6k8kxlVWW8e/7", - "/////////////////////////////////////////////////////////////////////////////wexh4khLQ9HQ2X6nh9wU5B+m6r+LWwPTEDTa5/CosDF" - ], - "is_max_namespace_ignored": true + "start_row": 1, + "end_row": 3 } -] +} \ No newline at end of file From 5ca260ab1121c68e33431cd4c259c687026db504 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Thu, 1 Aug 2024 18:52:42 +0400 Subject: [PATCH 15/46] chore: use the blob as a parameter for the proof.Verify --- blob/blob.go | 16 +++++++++++++--- blob/service.go | 6 +----- blob/service_test.go | 8 +++----- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/blob/blob.go b/blob/blob.go index b6ac41f900..aaa5d09ea0 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -80,15 +80,26 @@ func (p *Proof) equal(input *Proof) error { return nil } -// Verify takes a set of shares, a namespace and a data root, and verifies if the +// Verify takes a blob and a data root and verifies if the +// provided blob was committed to by the data root. +func (p *Proof) Verify(blob *Blob, dataRoot []byte) (bool, error) { + rawShares, err := BlobsToShares(blob) + if err != nil { + return false, err + } + return p.VerifyShares(rawShares, blob.namespace, dataRoot) +} + +// VerifyShares takes a set of shares, a namespace and a data root, and verifies if the // provided shares are committed to by the data root. -func (p *Proof) Verify(rawShares [][]byte, namespace share.Namespace, dataRoot []byte) (bool, error) { +func (p *Proof) VerifyShares(rawShares [][]byte, namespace share.Namespace, dataRoot []byte) (bool, error) { // verify the row proof if !p.RowProof.VerifyProof(dataRoot) { return false, errors.New("invalid row root to data root proof") } // verify the share proof + ns := append([]byte{namespace.Version()}, namespace.ID()...) cursor := int32(0) for i, proof := range p.ShareToRowRootProof { nmtProof := nmt.NewInclusionProof( @@ -98,7 +109,6 @@ func (p *Proof) Verify(rawShares [][]byte, namespace share.Namespace, dataRoot [ true, ) sharesUsed := proof.End - proof.Start - ns := append([]byte{namespace.Version()}, namespace.ID()...) if len(rawShares) < int(sharesUsed+cursor) { return false, errors.New("invalid number of shares") } diff --git a/blob/service.go b/blob/service.go index 7b785a54ce..f61ee198d4 100644 --- a/blob/service.go +++ b/blob/service.go @@ -326,15 +326,11 @@ func (s *Service) Included( default: return false, err } - appShares, err := BlobsToShares(blob) - if err != nil { - return false, err - } getter, err := s.headerGetter(ctx, height) if err != nil { return false, err } - return proof.Verify(appShares, namespace, getter.DataHash) + return proof.Verify(blob, getter.DataHash) } // retrieve retrieves blobs and their proofs by requesting the whole namespace and diff --git a/blob/service_test.go b/blob/service_test.go index 0dd979fe0a..e09b143acf 100644 --- a/blob/service_test.go +++ b/blob/service_test.go @@ -230,15 +230,13 @@ func TestBlobService_Get(t *testing.T) { proof, ok := res.(*Proof) assert.True(t, ok) - verifyFn := func(t *testing.T, rawShares [][]byte, proof *Proof, namespace share.Namespace) { - valid, err := proof.Verify(rawShares, namespace, header.DataHash) + verifyFn := func(t *testing.T, blob *Blob, proof *Proof) { + valid, err := proof.Verify(blob, header.DataHash) require.NoError(t, err) require.True(t, valid) } - rawShares, err := BlobsToShares(blobsWithDiffNamespaces[1]) - require.NoError(t, err) - verifyFn(t, rawShares, proof, blobsWithDiffNamespaces[1].Namespace()) + verifyFn(t, blobsWithDiffNamespaces[1], proof) }, }, { From afb2293718e9d4413e7761dfe2c697cb83726a77 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Thu, 1 Aug 2024 18:53:47 +0400 Subject: [PATCH 16/46] chore: remove errEmptyShares --- blob/blob.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/blob/blob.go b/blob/blob.go index aaa5d09ea0..d29c64bdd9 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -18,9 +18,6 @@ import ( "github.com/celestiaorg/celestia-node/share" ) -//nolint:unused -var errEmptyShares = errors.New("empty shares") - // Proof constructs the proof of a blob to the data root. type Proof struct { // ShareToRowRootProof the proofs of the shares to the row roots they belong to. From 052a79751d133df92da76bf51317e9764b4762cd Mon Sep 17 00:00:00 2001 From: sweexordious Date: Thu, 1 Aug 2024 18:54:47 +0400 Subject: [PATCH 17/46] chore: new line --- api/docgen/exampledata/blobProof.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/docgen/exampledata/blobProof.json b/api/docgen/exampledata/blobProof.json index a135f19424..d23f4b6aa0 100644 --- a/api/docgen/exampledata/blobProof.json +++ b/api/docgen/exampledata/blobProof.json @@ -50,4 +50,4 @@ "start_row": 1, "end_row": 3 } -} \ No newline at end of file +} From a006f94a188ece637a3be15cdd8d14be8876fd2a Mon Sep 17 00:00:00 2001 From: sweexordious Date: Thu, 1 Aug 2024 18:55:59 +0400 Subject: [PATCH 18/46] chore: godoc --- share/eds/edstest/testing.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/share/eds/edstest/testing.go b/share/eds/edstest/testing.go index 451e4c4e65..3b122b8d35 100644 --- a/share/eds/edstest/testing.go +++ b/share/eds/edstest/testing.go @@ -143,6 +143,9 @@ func createTestBlobTransaction( return ns, msg, blob, cTx } +// BuildCoreTx takes a signer, a message and a blob and creates a core transaction. +// The core transaction is the final form of a transaction that gets pushed +// into the square builder. func BuildCoreTx( t *testing.T, signer *types.KeyringSigner, From 698d90671df4045c5c4ce1bd59713d4826434206 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Thu, 1 Aug 2024 18:56:36 +0400 Subject: [PATCH 19/46] chore: remove comment --- blob/service_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/blob/service_test.go b/blob/service_test.go index e09b143acf..b624e3e772 100644 --- a/blob/service_test.go +++ b/blob/service_test.go @@ -279,8 +279,6 @@ func TestBlobService_Get(t *testing.T) { require.ErrorIs(t, err, ErrInvalidProof) included, ok := res.(bool) require.True(t, ok) - // This is supposed to be false not true. - // we're querying blob[1]'s proof and verifying it against blob[0], no? require.False(t, included) }, }, From ccb5799c7cd4575d78735f3f6244da44decfa9e2 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Thu, 1 Aug 2024 19:04:37 +0400 Subject: [PATCH 20/46] chore: wrap errors --- blob/blob.go | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/blob/blob.go b/blob/blob.go index d29c64bdd9..77bb34c554 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -34,43 +34,40 @@ func (p *Proof) equal(input *Proof) error { // compare the NMT proofs for i, proof := range p.ShareToRowRootProof { if proof.Start != input.ShareToRowRootProof[i].Start { - // TODO(@rach-id): should we define specific errors for each case? that's better - // to know which part is not equal. - // Also, this error should be ErrProofNotEqual or similar, and not ErrInvalidProof - return ErrInvalidProof + return fmt.Errorf("%w: unnequal share proof %d start", ErrInvalidProof, i) } if proof.End != input.ShareToRowRootProof[i].End { - return ErrInvalidProof + return fmt.Errorf("%w: unnequal share proof %d end", ErrInvalidProof, i) } for j, node := range proof.Nodes { if !bytes.Equal(node, input.ShareToRowRootProof[i].Nodes[j]) { - return ErrInvalidProof + return fmt.Errorf("%w: unnequal share proof %d side nodes", ErrInvalidProof, i) } } } // compare the row proof if p.RowProof.StartRow != input.RowProof.StartRow { - return ErrInvalidProof + return fmt.Errorf("%w: unnequal row proof start", ErrInvalidProof) } if p.RowProof.EndRow != input.RowProof.EndRow { - return ErrInvalidProof + return fmt.Errorf("%w: unnequal row proof end", ErrInvalidProof) } for index, rowRoot := range p.RowProof.RowRoots { if !bytes.Equal(rowRoot, input.RowProof.RowRoots[index]) { - return ErrInvalidProof + return fmt.Errorf("%w: unnequal row proof row root %d", ErrInvalidProof, index) } } for i, proof := range p.RowProof.Proofs { if proof.Index != input.RowProof.Proofs[i].Index { - return ErrInvalidProof + return fmt.Errorf("%w: unnequal row proof row proof %d index", ErrInvalidProof, i) } if proof.Total != input.RowProof.Proofs[i].Total { - return ErrInvalidProof + return fmt.Errorf("%w: unnequal row proof row proof %d total", ErrInvalidProof, i) } for j, aunt := range proof.Aunts { if !bytes.Equal(aunt, input.RowProof.Proofs[i].Aunts[j]) { - return ErrInvalidProof + return fmt.Errorf("%w: unnequal row proof row proof %d aunts", ErrInvalidProof, i) } } } From 8f79b51697160771fd78d4cb30bfc71f821ab078 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Thu, 1 Aug 2024 19:04:49 +0400 Subject: [PATCH 21/46] Revert "chore: remove errEmptyShares" This reverts commit afb2293718e9d4413e7761dfe2c697cb83726a77. --- blob/blob.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/blob/blob.go b/blob/blob.go index 77bb34c554..18e12358e9 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -18,6 +18,9 @@ import ( "github.com/celestiaorg/celestia-node/share" ) +//nolint:unused +var errEmptyShares = errors.New("empty shares") + // Proof constructs the proof of a blob to the data root. type Proof struct { // ShareToRowRootProof the proofs of the shares to the row roots they belong to. From 952b4add6b2560cd2e2c77b4a1b32516816ea445 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Thu, 1 Aug 2024 19:07:32 +0400 Subject: [PATCH 22/46] chore: getter -> header --- blob/service.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/blob/service.go b/blob/service.go index f61ee198d4..7dbe77243c 100644 --- a/blob/service.go +++ b/blob/service.go @@ -326,11 +326,11 @@ func (s *Service) Included( default: return false, err } - getter, err := s.headerGetter(ctx, height) + header, err := s.headerGetter(ctx, height) if err != nil { return false, err } - return proof.Verify(blob, getter.DataHash) + return proof.Verify(blob, header.DataHash) } // retrieve retrieves blobs and their proofs by requesting the whole namespace and From 82d30cc9ef7c297777684c16507fb44900d5a107 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Thu, 1 Aug 2024 19:21:30 +0400 Subject: [PATCH 23/46] chore: remove equal method --- blob/blob.go | 46 -------------------------------------------- blob/service_test.go | 9 ++++----- 2 files changed, 4 insertions(+), 51 deletions(-) diff --git a/blob/blob.go b/blob/blob.go index 18e12358e9..658b0f0658 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -31,52 +31,6 @@ type Proof struct { RowProof coretypes.RowProof } -// equal is a temporary method that compares two proofs. -// should be removed in BlobService V1. -func (p *Proof) equal(input *Proof) error { - // compare the NMT proofs - for i, proof := range p.ShareToRowRootProof { - if proof.Start != input.ShareToRowRootProof[i].Start { - return fmt.Errorf("%w: unnequal share proof %d start", ErrInvalidProof, i) - } - if proof.End != input.ShareToRowRootProof[i].End { - return fmt.Errorf("%w: unnequal share proof %d end", ErrInvalidProof, i) - } - for j, node := range proof.Nodes { - if !bytes.Equal(node, input.ShareToRowRootProof[i].Nodes[j]) { - return fmt.Errorf("%w: unnequal share proof %d side nodes", ErrInvalidProof, i) - } - } - } - - // compare the row proof - if p.RowProof.StartRow != input.RowProof.StartRow { - return fmt.Errorf("%w: unnequal row proof start", ErrInvalidProof) - } - if p.RowProof.EndRow != input.RowProof.EndRow { - return fmt.Errorf("%w: unnequal row proof end", ErrInvalidProof) - } - for index, rowRoot := range p.RowProof.RowRoots { - if !bytes.Equal(rowRoot, input.RowProof.RowRoots[index]) { - return fmt.Errorf("%w: unnequal row proof row root %d", ErrInvalidProof, index) - } - } - for i, proof := range p.RowProof.Proofs { - if proof.Index != input.RowProof.Proofs[i].Index { - return fmt.Errorf("%w: unnequal row proof row proof %d index", ErrInvalidProof, i) - } - if proof.Total != input.RowProof.Proofs[i].Total { - return fmt.Errorf("%w: unnequal row proof row proof %d total", ErrInvalidProof, i) - } - for j, aunt := range proof.Aunts { - if !bytes.Equal(aunt, input.RowProof.Proofs[i].Aunts[j]) { - return fmt.Errorf("%w: unnequal row proof row proof %d aunts", ErrInvalidProof, i) - } - } - } - return nil -} - // Verify takes a blob and a data root and verifies if the // provided blob was committed to by the data root. func (p *Proof) Verify(blob *Blob, dataRoot []byte) (bool, error) { diff --git a/blob/service_test.go b/blob/service_test.go index b624e3e772..3293119167 100644 --- a/blob/service_test.go +++ b/blob/service_test.go @@ -369,12 +369,11 @@ func TestBlobService_Get(t *testing.T) { var proof Proof require.NoError(t, json.Unmarshal(jsonData, &proof)) - newProof, err := service.GetProof(ctx, 1, - blobsWithDiffNamespaces[1].Namespace(), - blobsWithDiffNamespaces[1].Commitment, - ) + header, err := service.headerGetter(ctx, 1) + require.NoError(t, err) + valid, err := proof.Verify(blobsWithDiffNamespaces[1], header.DataHash) require.NoError(t, err) - require.NoError(t, proof.equal(newProof)) + require.True(t, valid) }, }, { From 85e23910e7b305cca1bcd3d391a16746ba305248 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Thu, 1 Aug 2024 22:21:02 +0400 Subject: [PATCH 24/46] fix: check namespace when getting blob --- blob/service.go | 2 +- nodebuilder/tests/blob_test.go | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/blob/service.go b/blob/service.go index 7dbe77243c..3b746a65f9 100644 --- a/blob/service.go +++ b/blob/service.go @@ -476,7 +476,7 @@ func (s *Service) retrieve( blob.index = currentShareIndex%squareSize + (inclusiveNamespaceStartRowIndex+currentShareIndex/squareSize)*squareSize*2 - if sharesParser.verify(blob) { + if blob.Namespace().Equals(namespace) && sharesParser.verify(blob) { // now that we found the requested blob, we will create // its inclusion proof. inclusiveBlobStartRowIndex := inclusiveNamespaceStartRowIndex + currentShareIndex/squareSize diff --git a/nodebuilder/tests/blob_test.go b/nodebuilder/tests/blob_test.go index a1478108a2..165e0ab688 100644 --- a/nodebuilder/tests/blob_test.go +++ b/nodebuilder/tests/blob_test.go @@ -10,7 +10,6 @@ import ( "github.com/libp2p/go-libp2p/core/host" "github.com/libp2p/go-libp2p/core/peer" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/celestiaorg/celestia-node/blob" From 8e686934eb216e7f8375fd69684048afdcf94110 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Thu, 1 Aug 2024 22:21:28 +0400 Subject: [PATCH 25/46] chore: refactor blob index calculation --- blob/service.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/blob/service.go b/blob/service.go index 3b746a65f9..84da9b8910 100644 --- a/blob/service.go +++ b/blob/service.go @@ -472,9 +472,20 @@ func (s *Service) retrieve( return nil, nil, err } + // number of shares per EDS row + numberOfSharesPerEDSRow := squareSize * 2 + // number of shares from square start to namespace start + sharesFromSquareStartToNsStart := inclusiveNamespaceStartRowIndex * numberOfSharesPerEDSRow + // number of rows from namespace start row to current row + rowsFromNsStartToCurrentRow := currentShareIndex / squareSize + // number of shares from namespace row start to current row + sharesFromNsStartToCurrentRow := rowsFromNsStartToCurrentRow * squareSize * 2 + // number of shares from the beginning of current row to current share + sharesFromCurrentRowStart := currentShareIndex % squareSize // setting the index manually since we didn't use the parser.set() method - blob.index = currentShareIndex%squareSize + - (inclusiveNamespaceStartRowIndex+currentShareIndex/squareSize)*squareSize*2 + blob.index = sharesFromSquareStartToNsStart + + sharesFromNsStartToCurrentRow + + sharesFromCurrentRowStart if blob.Namespace().Equals(namespace) && sharesParser.verify(blob) { // now that we found the requested blob, we will create From 2c9e2f56cdd115ab45c55da028a2f51fde0c1073 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Thu, 1 Aug 2024 22:30:34 +0400 Subject: [PATCH 26/46] fix: revert removing import --- nodebuilder/tests/blob_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/nodebuilder/tests/blob_test.go b/nodebuilder/tests/blob_test.go index 165e0ab688..a1478108a2 100644 --- a/nodebuilder/tests/blob_test.go +++ b/nodebuilder/tests/blob_test.go @@ -10,6 +10,7 @@ import ( "github.com/libp2p/go-libp2p/core/host" "github.com/libp2p/go-libp2p/core/peer" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/celestiaorg/celestia-node/blob" From 2d209adef1e5cd662efe2994b60236a95ca76d4e Mon Sep 17 00:00:00 2001 From: sweexordious Date: Fri, 2 Aug 2024 13:53:26 +0400 Subject: [PATCH 27/46] chore: use getEDS to get the shares --- blob/service.go | 10 ++++++---- blob/service_test.go | 19 +++---------------- 2 files changed, 9 insertions(+), 20 deletions(-) diff --git a/blob/service.go b/blob/service.go index 84da9b8910..18ac86ee2b 100644 --- a/blob/service.go +++ b/blob/service.go @@ -354,6 +354,10 @@ func (s *Service) retrieve( return nil, nil, err } + eds, err := s.shareGetter.GetEDS(ctx, header) + if err != nil { + return nil, nil, err + } headerGetterSpan.SetStatus(codes.Ok, "") headerGetterSpan.AddEvent("received eds", trace.WithAttributes( attribute.Int64("eds-size", int64(len(header.DAH.RowRoots))))) @@ -388,12 +392,10 @@ func (s *Service) retrieve( rowsShares := make([]share.Share, 0, (exclusiveNamespaceEndRowIndex-inclusiveNamespaceStartRowIndex)*squareSize) // store the EDS shares of the rows containing the blob rowsWithParityShares := make([][]shares.Share, exclusiveNamespaceEndRowIndex-inclusiveNamespaceStartRowIndex) + for rowIndex := inclusiveNamespaceStartRowIndex; rowIndex < exclusiveNamespaceEndRowIndex; rowIndex++ { for colIndex := 0; colIndex < squareSize*2; colIndex++ { - share, err := s.shareGetter.GetShare(ctx, header, rowIndex, colIndex) - if err != nil { - return nil, nil, err - } + share := eds.GetCell(uint(rowIndex), uint(colIndex)) if colIndex < squareSize { rowsShares = append(rowsShares, share) } diff --git a/blob/service_test.go b/blob/service_test.go index 3293119167..27ea511b45 100644 --- a/blob/service_test.go +++ b/blob/service_test.go @@ -380,20 +380,12 @@ func TestBlobService_Get(t *testing.T) { name: "internal error", doFn: func() (interface{}, error) { ctrl := gomock.NewController(t) - shareService := service.shareGetter shareGetterMock := shareMock.NewMockModule(ctrl) shareGetterMock.EXPECT(). - GetShare(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()). + GetEDS(gomock.Any(), gomock.Any()). DoAndReturn( - func(ctx context.Context, header *header.ExtendedHeader, row, col int) (share.Share, error) { - sh, err := shareService.GetShare(ctx, header, row, col) - if err != nil { - return nil, err - } - if share.GetNamespace(sh).Equals(blobsWithDiffNamespaces[0].Namespace()) { - return nil, errors.New("internal error") - } - return shareService.GetShare(ctx, header, row, col) + func(context.Context, *header.ExtendedHeader) (*rsmt2d.ExtendedDataSquare, error) { + return nil, errors.New("internal error") }).AnyTimes() service.shareGetter = shareGetterMock @@ -405,13 +397,8 @@ func TestBlobService_Get(t *testing.T) { ) }, expectedResult: func(res interface{}, err error) { - blobs, ok := res.([]*Blob) - assert.True(t, ok) assert.Error(t, err) assert.Contains(t, err.Error(), "internal error") - assert.Equal(t, blobs[0].Namespace(), blobsWithSameNamespace[0].Namespace()) - assert.NotEmpty(t, blobs) - assert.Len(t, blobs, len(blobsWithSameNamespace)) }, }, } From ae65e8d6e23f54895e284ae4de34a6268cd3f17e Mon Sep 17 00:00:00 2001 From: sweexordious Date: Fri, 2 Aug 2024 14:49:55 +0400 Subject: [PATCH 28/46] feat: check if the commitment is valid as suggested by @vgonkivs --- blob/blob.go | 11 +++++++++-- blob/helper.go | 2 +- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/blob/blob.go b/blob/blob.go index 658b0f0658..17289c5765 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -34,6 +34,13 @@ type Proof struct { // Verify takes a blob and a data root and verifies if the // provided blob was committed to by the data root. func (p *Proof) Verify(blob *Blob, dataRoot []byte) (bool, error) { + blobCommitment, err := types.CreateCommitment(ToAppBlobs(blob)[0]) + if err != nil { + return false, err + } + if !blob.Commitment.Equal(blobCommitment) { + return false, fmt.Errorf("%w: generated commitment does not match the provided blob commitment", ErrInvalidProof) + } rawShares, err := BlobsToShares(blob) if err != nil { return false, err @@ -46,7 +53,7 @@ func (p *Proof) Verify(blob *Blob, dataRoot []byte) (bool, error) { func (p *Proof) VerifyShares(rawShares [][]byte, namespace share.Namespace, dataRoot []byte) (bool, error) { // verify the row proof if !p.RowProof.VerifyProof(dataRoot) { - return false, errors.New("invalid row root to data root proof") + return false, fmt.Errorf("%w: invalid row root to data root proof", ErrInvalidProof) } // verify the share proof @@ -61,7 +68,7 @@ func (p *Proof) VerifyShares(rawShares [][]byte, namespace share.Namespace, data ) sharesUsed := proof.End - proof.Start if len(rawShares) < int(sharesUsed+cursor) { - return false, errors.New("invalid number of shares") + return false, fmt.Errorf("%w: invalid number of shares", ErrInvalidProof) } valid := nmtProof.VerifyInclusion( consts.NewBaseHashFunc(), diff --git a/blob/helper.go b/blob/helper.go index c1de773c24..615d5fbc08 100644 --- a/blob/helper.go +++ b/blob/helper.go @@ -39,7 +39,7 @@ func BlobsToShares(blobs ...*Blob) ([]share.Share, error) { // ToAppBlobs converts node's blob type to the blob type from celestia-app. func ToAppBlobs(blobs ...*Blob) []*apptypes.Blob { - appBlobs := make([]*apptypes.Blob, 0, len(blobs)) + appBlobs := make([]*apptypes.Blob, len(blobs)) for i := range blobs { appBlobs[i] = &blobs[i].Blob } From e473057d5f0e3a9486dce557a5565c7e7962c809 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Fri, 2 Aug 2024 14:55:58 +0400 Subject: [PATCH 29/46] feat: check the row index of the end of the namespace --- blob/service.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/blob/service.go b/blob/service.go index 18ac86ee2b..9afffcb380 100644 --- a/blob/service.go +++ b/blob/service.go @@ -376,11 +376,14 @@ func (s *Service) retrieve( // end exclusive index of the row root containing the namespace exclusiveNamespaceEndRowIndex := inclusiveNamespaceStartRowIndex - for i, row := range header.DAH.RowRoots[inclusiveNamespaceStartRowIndex:] { + for _, row := range header.DAH.RowRoots[inclusiveNamespaceStartRowIndex:] { if namespace.IsOutsideRange(row, row) { - exclusiveNamespaceEndRowIndex = inclusiveNamespaceStartRowIndex + i break } + exclusiveNamespaceEndRowIndex++ + } + if exclusiveNamespaceEndRowIndex == inclusiveNamespaceStartRowIndex { + return nil, nil, fmt.Errorf("couldn't find the row index of the namespace end") } // calculate the square size From e5677a8d498f140555f7213e050ee972e97de866 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Fri, 2 Aug 2024 20:44:25 +0400 Subject: [PATCH 30/46] chore: refactor getting shares from eds --- blob/service.go | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/blob/service.go b/blob/service.go index 9afffcb380..76f4afe39f 100644 --- a/blob/service.go +++ b/blob/service.go @@ -397,20 +397,13 @@ func (s *Service) retrieve( rowsWithParityShares := make([][]shares.Share, exclusiveNamespaceEndRowIndex-inclusiveNamespaceStartRowIndex) for rowIndex := inclusiveNamespaceStartRowIndex; rowIndex < exclusiveNamespaceEndRowIndex; rowIndex++ { - for colIndex := 0; colIndex < squareSize*2; colIndex++ { - share := eds.GetCell(uint(rowIndex), uint(colIndex)) - if colIndex < squareSize { - rowsShares = append(rowsShares, share) - } - appShare, err := toAppShares(share) - if err != nil { - return nil, nil, err - } - rowsWithParityShares[rowIndex-inclusiveNamespaceStartRowIndex] = append( - rowsWithParityShares[rowIndex-inclusiveNamespaceStartRowIndex], - appShare[0], - ) + rowShares := eds.Row(uint(rowIndex)) + rowsShares = append(rowsShares, rowShares[:squareSize]...) + rowAppShares, err := toAppShares(rowShares...) + if err != nil { + return nil, nil, err } + rowsWithParityShares[rowIndex-inclusiveNamespaceStartRowIndex] = rowAppShares } getSharesSpan.SetStatus(codes.Ok, "") From 2bd0da0e87253d2c3022af6bf8cfdd7409e77925 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Fri, 2 Aug 2024 20:44:40 +0400 Subject: [PATCH 31/46] refactor: use correct variable for calculation --- blob/service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/blob/service.go b/blob/service.go index 76f4afe39f..a48545ef36 100644 --- a/blob/service.go +++ b/blob/service.go @@ -477,7 +477,7 @@ func (s *Service) retrieve( // number of rows from namespace start row to current row rowsFromNsStartToCurrentRow := currentShareIndex / squareSize // number of shares from namespace row start to current row - sharesFromNsStartToCurrentRow := rowsFromNsStartToCurrentRow * squareSize * 2 + sharesFromNsStartToCurrentRow := rowsFromNsStartToCurrentRow * numberOfSharesPerEDSRow // number of shares from the beginning of current row to current share sharesFromCurrentRowStart := currentShareIndex % squareSize // setting the index manually since we didn't use the parser.set() method From d7bb13b4ff89bd00fab59635e8e49e0bb5f20bb7 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Fri, 2 Aug 2024 20:45:40 +0400 Subject: [PATCH 32/46] refactor: to odsShares --- blob/service.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/blob/service.go b/blob/service.go index a48545ef36..e42ca29a8f 100644 --- a/blob/service.go +++ b/blob/service.go @@ -392,13 +392,13 @@ func (s *Service) retrieve( // get all the shares of the rows containing the namespace _, getSharesSpan := tracer.Start(ctx, "get-all-shares-in-namespace") // store the ODS shares of the rows containing the blob - rowsShares := make([]share.Share, 0, (exclusiveNamespaceEndRowIndex-inclusiveNamespaceStartRowIndex)*squareSize) + odsShares := make([]share.Share, 0, (exclusiveNamespaceEndRowIndex-inclusiveNamespaceStartRowIndex)*squareSize) // store the EDS shares of the rows containing the blob rowsWithParityShares := make([][]shares.Share, exclusiveNamespaceEndRowIndex-inclusiveNamespaceStartRowIndex) for rowIndex := inclusiveNamespaceStartRowIndex; rowIndex < exclusiveNamespaceEndRowIndex; rowIndex++ { rowShares := eds.Row(uint(rowIndex)) - rowsShares = append(rowsShares, rowShares[:squareSize]...) + odsShares = append(odsShares, rowShares[:squareSize]...) rowAppShares, err := toAppShares(rowShares...) if err != nil { return nil, nil, err @@ -411,8 +411,8 @@ func (s *Service) retrieve( attribute.Int64("eds-size", int64(squareSize*2)))) // go over the shares until finding the requested blobs - for currentShareIndex := 0; currentShareIndex < len(rowsShares); { - currentShareApp, err := shares.NewShare(rowsShares[currentShareIndex]) + for currentShareIndex := 0; currentShareIndex < len(odsShares); { + currentShareApp, err := shares.NewShare(odsShares[currentShareIndex]) if err != nil { return nil, nil, err } @@ -447,13 +447,13 @@ func (s *Service) retrieve( blobLen := shares.SparseSharesNeeded(sequenceLength) endShareIndex := currentShareIndex + blobLen - if endShareIndex > len(rowsShares) { + if endShareIndex > len(odsShares) { // this blob spans to the next row which has a namespace > requested namespace. // this means that we can stop. return nil, nil, ErrBlobNotFound } // convert the blob shares to app shares - blobShares := rowsShares[currentShareIndex:endShareIndex] + blobShares := odsShares[currentShareIndex:endShareIndex] appBlobShares, err := toAppShares(blobShares...) if err != nil { return nil, nil, err From 11bfc485c48197427e951683d35e400ccea6416f Mon Sep 17 00:00:00 2001 From: sweexordious Date: Fri, 2 Aug 2024 20:46:22 +0400 Subject: [PATCH 33/46] refactor: to edsShares --- blob/service.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/blob/service.go b/blob/service.go index e42ca29a8f..41f6d34acb 100644 --- a/blob/service.go +++ b/blob/service.go @@ -394,7 +394,7 @@ func (s *Service) retrieve( // store the ODS shares of the rows containing the blob odsShares := make([]share.Share, 0, (exclusiveNamespaceEndRowIndex-inclusiveNamespaceStartRowIndex)*squareSize) // store the EDS shares of the rows containing the blob - rowsWithParityShares := make([][]shares.Share, exclusiveNamespaceEndRowIndex-inclusiveNamespaceStartRowIndex) + edsShares := make([][]shares.Share, exclusiveNamespaceEndRowIndex-inclusiveNamespaceStartRowIndex) for rowIndex := inclusiveNamespaceStartRowIndex; rowIndex < exclusiveNamespaceEndRowIndex; rowIndex++ { rowShares := eds.Row(uint(rowIndex)) @@ -403,7 +403,7 @@ func (s *Service) retrieve( if err != nil { return nil, nil, err } - rowsWithParityShares[rowIndex-inclusiveNamespaceStartRowIndex] = rowAppShares + edsShares[rowIndex-inclusiveNamespaceStartRowIndex] = rowAppShares } getSharesSpan.SetStatus(codes.Ok, "") @@ -512,7 +512,7 @@ func (s *Service) retrieve( shareToRowRootProofs, _, err := pkgproof.CreateShareToRowRootProofs( squareSize, //nolint:lll - rowsWithParityShares[inclusiveBlobStartRowIndex-inclusiveNamespaceStartRowIndex:exclusiveBlobEndRowIndex-inclusiveNamespaceStartRowIndex], + edsShares[inclusiveBlobStartRowIndex-inclusiveNamespaceStartRowIndex:exclusiveBlobEndRowIndex-inclusiveNamespaceStartRowIndex], header.DAH.RowRoots[inclusiveBlobStartRowIndex:exclusiveBlobEndRowIndex], currentShareIndex%squareSize, (endShareIndex-1)%squareSize, From 500f0b8c438b0667e2df482f9d6d2fa3a77f2b0d Mon Sep 17 00:00:00 2001 From: sweexordious Date: Fri, 2 Aug 2024 20:48:50 +0400 Subject: [PATCH 34/46] refactor: to exclusiveEndShareIndex --- blob/service.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/blob/service.go b/blob/service.go index 41f6d34acb..55999f87b7 100644 --- a/blob/service.go +++ b/blob/service.go @@ -446,14 +446,14 @@ func (s *Service) retrieve( } blobLen := shares.SparseSharesNeeded(sequenceLength) - endShareIndex := currentShareIndex + blobLen - if endShareIndex > len(odsShares) { + exclusiveEndShareIndex := currentShareIndex + blobLen + if exclusiveEndShareIndex > len(odsShares) { // this blob spans to the next row which has a namespace > requested namespace. // this means that we can stop. return nil, nil, ErrBlobNotFound } // convert the blob shares to app shares - blobShares := odsShares[currentShareIndex:endShareIndex] + blobShares := odsShares[currentShareIndex:exclusiveEndShareIndex] appBlobShares, err := toAppShares(blobShares...) if err != nil { return nil, nil, err @@ -489,7 +489,7 @@ func (s *Service) retrieve( // now that we found the requested blob, we will create // its inclusion proof. inclusiveBlobStartRowIndex := inclusiveNamespaceStartRowIndex + currentShareIndex/squareSize - exclusiveBlobEndRowIndex := inclusiveNamespaceStartRowIndex + endShareIndex/squareSize + exclusiveBlobEndRowIndex := inclusiveNamespaceStartRowIndex + exclusiveEndShareIndex/squareSize if (currentShareIndex+blobLen)%squareSize != 0 { // if the row is not complete with the blob shares, // then we increment the exclusive blob end row index @@ -515,7 +515,7 @@ func (s *Service) retrieve( edsShares[inclusiveBlobStartRowIndex-inclusiveNamespaceStartRowIndex:exclusiveBlobEndRowIndex-inclusiveNamespaceStartRowIndex], header.DAH.RowRoots[inclusiveBlobStartRowIndex:exclusiveBlobEndRowIndex], currentShareIndex%squareSize, - (endShareIndex-1)%squareSize, + (exclusiveEndShareIndex-1)%squareSize, ) if err != nil { return nil, nil, err From 11d425c3fe02293c3229f14bad985d6a5a075f56 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Fri, 2 Aug 2024 20:57:54 +0400 Subject: [PATCH 35/46] refactor: move prove row roots to blob.go --- blob/blob.go | 8 ++++++++ blob/service.go | 10 +--------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/blob/blob.go b/blob/blob.go index 17289c5765..93644c3cfc 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" + "github.com/tendermint/tendermint/crypto/merkle" "github.com/tendermint/tendermint/pkg/consts" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" coretypes "github.com/tendermint/tendermint/types" @@ -202,3 +203,10 @@ func (b *Blob) UnmarshalJSON(data []byte) error { b.index = blob.Index return nil } + +// proveRowRootsToDataRoot creates a set of binary merkle proofs for all the +// roots defined by the range [start, end). +func proveRowRootsToDataRoot(roots [][]byte, start, end int) []*merkle.Proof { + _, proofs := merkle.ProofsFromByteSlices(roots) + return proofs[start:end] +} diff --git a/blob/service.go b/blob/service.go index 55999f87b7..e9090df3e3 100644 --- a/blob/service.go +++ b/blob/service.go @@ -11,7 +11,6 @@ import ( "github.com/cosmos/cosmos-sdk/types" logging "github.com/ipfs/go-log/v2" - "github.com/tendermint/tendermint/crypto/merkle" "github.com/tendermint/tendermint/libs/bytes" core "github.com/tendermint/tendermint/types" "go.opentelemetry.io/otel" @@ -488,7 +487,7 @@ func (s *Service) retrieve( if blob.Namespace().Equals(namespace) && sharesParser.verify(blob) { // now that we found the requested blob, we will create // its inclusion proof. - inclusiveBlobStartRowIndex := inclusiveNamespaceStartRowIndex + currentShareIndex/squareSize + inclusiveBlobStartRowIndex := blob.index / (squareSize * 2) exclusiveBlobEndRowIndex := inclusiveNamespaceStartRowIndex + exclusiveEndShareIndex/squareSize if (currentShareIndex+blobLen)%squareSize != 0 { // if the row is not complete with the blob shares, @@ -543,13 +542,6 @@ func (s *Service) retrieve( return nil, nil, ErrBlobNotFound } -// proveRowRootsToDataRoot creates a set of binary merkle proofs for all the -// roots defined by the range [start, end). -func proveRowRootsToDataRoot(roots [][]byte, start, end int) []*merkle.Proof { - _, proofs := merkle.ProofsFromByteSlices(roots) - return proofs[start:end] -} - // getBlobs retrieves the DAH and fetches all shares from the requested Namespace and converts // them to Blobs. func (s *Service) getBlobs( From 5b054bc56d12437886dbe2a9f681eb0d57c5c853 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Mon, 5 Aug 2024 15:42:03 +0400 Subject: [PATCH 36/46] refactor: rename to RowToDataRootProof --- blob/blob.go | 8 ++++---- blob/service.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/blob/blob.go b/blob/blob.go index 93644c3cfc..fd35e59a8e 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -27,9 +27,9 @@ type Proof struct { // ShareToRowRootProof the proofs of the shares to the row roots they belong to. // If the blob spans across multiple rows, then this will contain multiple proofs. ShareToRowRootProof []*tmproto.NMTProof - // RowProof the proofs of the row roots containing the blob shares + // RowToDataRootProof the proofs of the row roots containing the blob shares // to the data root. - RowProof coretypes.RowProof + RowToDataRootProof coretypes.RowProof } // Verify takes a blob and a data root and verifies if the @@ -53,7 +53,7 @@ func (p *Proof) Verify(blob *Blob, dataRoot []byte) (bool, error) { // provided shares are committed to by the data root. func (p *Proof) VerifyShares(rawShares [][]byte, namespace share.Namespace, dataRoot []byte) (bool, error) { // verify the row proof - if !p.RowProof.VerifyProof(dataRoot) { + if !p.RowToDataRootProof.VerifyProof(dataRoot) { return false, fmt.Errorf("%w: invalid row root to data root proof", ErrInvalidProof) } @@ -75,7 +75,7 @@ func (p *Proof) VerifyShares(rawShares [][]byte, namespace share.Namespace, data consts.NewBaseHashFunc(), ns, rawShares[cursor:sharesUsed+cursor], - p.RowProof.RowRoots[i], + p.RowToDataRootProof.RowRoots[i], ) if !valid { return false, ErrInvalidProof diff --git a/blob/service.go b/blob/service.go index e9090df3e3..437e3e1dcd 100644 --- a/blob/service.go +++ b/blob/service.go @@ -522,7 +522,7 @@ func (s *Service) retrieve( proof := Proof{ ShareToRowRootProof: shareToRowRootProofs, - RowProof: core.RowProof{ + RowToDataRootProof: core.RowProof{ RowRoots: rowRoots, Proofs: rowProofs, StartRow: uint32(inclusiveBlobStartRowIndex), From 1fb124d22f0c1f41fc8aeb23f3f6e278dbb2e43a Mon Sep 17 00:00:00 2001 From: CHAMI Rachid Date: Mon, 5 Aug 2024 13:43:45 +0200 Subject: [PATCH 37/46] Update blob/blob.go Co-authored-by: rene <41963722+renaynay@users.noreply.github.com> --- blob/blob.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/blob/blob.go b/blob/blob.go index fd35e59a8e..ffd74cc239 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -33,7 +33,7 @@ type Proof struct { } // Verify takes a blob and a data root and verifies if the -// provided blob was committed to by the data root. +// provided blob was committed to the given data root. func (p *Proof) Verify(blob *Blob, dataRoot []byte) (bool, error) { blobCommitment, err := types.CreateCommitment(ToAppBlobs(blob)[0]) if err != nil { From 28cb03f036a42f938a2eff43d0886176a9575e78 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Mon, 5 Aug 2024 16:55:39 +0400 Subject: [PATCH 38/46] refactor: move if before --- blob/blob.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/blob/blob.go b/blob/blob.go index fd35e59a8e..ad8c32a6ca 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -61,16 +61,16 @@ func (p *Proof) VerifyShares(rawShares [][]byte, namespace share.Namespace, data ns := append([]byte{namespace.Version()}, namespace.ID()...) cursor := int32(0) for i, proof := range p.ShareToRowRootProof { + sharesUsed := proof.End - proof.Start + if len(rawShares) < int(sharesUsed+cursor) { + return false, fmt.Errorf("%w: invalid number of shares", ErrInvalidProof) + } nmtProof := nmt.NewInclusionProof( int(proof.Start), int(proof.End), proof.Nodes, true, ) - sharesUsed := proof.End - proof.Start - if len(rawShares) < int(sharesUsed+cursor) { - return false, fmt.Errorf("%w: invalid number of shares", ErrInvalidProof) - } valid := nmtProof.VerifyInclusion( consts.NewBaseHashFunc(), ns, From 8a28b3f198e4efaa4312631476ba36178de1224e Mon Sep 17 00:00:00 2001 From: sweexordious Date: Mon, 5 Aug 2024 19:22:46 +0400 Subject: [PATCH 39/46] test: add verification tests --- blob/service_test.go | 121 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/blob/service_test.go b/blob/service_test.go index 27ea511b45..323c116f25 100644 --- a/blob/service_test.go +++ b/blob/service_test.go @@ -1061,3 +1061,124 @@ func generateCommitmentProofFromBlock( return commitmentProof } + +func TestBlobVerify(t *testing.T) { + _, blobs, nss, eds, _, _, dataRoot := edstest.GenerateTestBlock(t, 80, 10) + + // create the blob from the data + blob, err := NewBlob( + uint8(blobs[5].ShareVersion), + nss[5].Bytes(), + blobs[5].Data, + ) + require.NoError(t, err) + + // convert the blob to a number of shares + blobShares, err := BlobsToShares(blob) + require.NoError(t, err) + + // find the first share of the blob in the ODS + startShareIndex := -1 + for i, sh := range eds.FlattenedODS() { + if bytes.Equal(sh, blobShares[0]) { + startShareIndex = i + break + } + } + require.Greater(t, startShareIndex, 0) + + // create an inclusion proof of the blob using the share range instead of the commitment + sharesProof, err := pkgproof.NewShareInclusionProofFromEDS( + eds, + nss[5], + shares.NewRange(startShareIndex, startShareIndex+len(blobShares)), + ) + require.NoError(t, err) + require.NoError(t, sharesProof.Validate(dataRoot)) + + blobProof := Proof{ + ShareToRowRootProof: sharesProof.ShareProofs, + RowToDataRootProof: sharesProof.RowProof, + } + tests := []struct { + name string + blob Blob + proof Proof + dataRoot []byte + expectErr bool + }{ + { + name: "invalid blob commitment", + dataRoot: dataRoot, + proof: blobProof, + blob: func() Blob { + b := *blob + b.Commitment = []byte{0x1} + return b + }(), + expectErr: true, + }, + { + name: "invalid proof", + dataRoot: dataRoot, + proof: func() Proof { + p := blobProof + p.ShareToRowRootProof = p.ShareToRowRootProof[1:] + return p + }(), + blob: *blob, + expectErr: true, + }, + { + name: "malformed blob and proof", + dataRoot: dataRoot, + proof: func() Proof { + p := blobProof + p.ShareToRowRootProof = p.ShareToRowRootProof[1:] + return p + }(), + blob: func() Blob { + b := *blob + b.Commitment = []byte{0x1} + return b + }(), + expectErr: true, + }, + { + name: "mismatched number of share proofs and row proofs", + dataRoot: dataRoot, + proof: func() Proof { + p := blobProof + p.ShareToRowRootProof = p.ShareToRowRootProof[1:] + return p + }(), + blob: *blob, + expectErr: true, + }, + { + name: "invalid data root", + dataRoot: []byte{0x1, 0x2}, + proof: blobProof, + blob: *blob, + expectErr: true, + }, + { + name: "valid proof", + dataRoot: dataRoot, + blob: *blob, + proof: blobProof, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + valid, err := test.proof.Verify(&test.blob, test.dataRoot) + if test.expectErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.True(t, valid) + } + }) + } +} From 175730015d57ced30f962a9daa0a9b4f1044eb9b Mon Sep 17 00:00:00 2001 From: sweexordious Date: Mon, 5 Aug 2024 19:24:43 +0400 Subject: [PATCH 40/46] chore: refactor the eds shares range --- blob/service.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/blob/service.go b/blob/service.go index 437e3e1dcd..b7d6e14c11 100644 --- a/blob/service.go +++ b/blob/service.go @@ -507,11 +507,12 @@ func (s *Service) retrieve( rowRoots[index] = rowRoot } + edsShareStart := inclusiveBlobStartRowIndex - inclusiveNamespaceStartRowIndex + edsShareEnd := exclusiveBlobEndRowIndex - inclusiveNamespaceStartRowIndex // create the share to row root proofs shareToRowRootProofs, _, err := pkgproof.CreateShareToRowRootProofs( squareSize, - //nolint:lll - edsShares[inclusiveBlobStartRowIndex-inclusiveNamespaceStartRowIndex:exclusiveBlobEndRowIndex-inclusiveNamespaceStartRowIndex], + edsShares[edsShareStart:edsShareEnd], header.DAH.RowRoots[inclusiveBlobStartRowIndex:exclusiveBlobEndRowIndex], currentShareIndex%squareSize, (exclusiveEndShareIndex-1)%squareSize, From 857e3ed106e17a43ca88eff68fc2672ca1fddb7c Mon Sep 17 00:00:00 2001 From: sweexordious Date: Mon, 5 Aug 2024 19:29:52 +0400 Subject: [PATCH 41/46] test: invalid row proof test --- blob/service_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/blob/service_test.go b/blob/service_test.go index 323c116f25..9975a3a9d1 100644 --- a/blob/service_test.go +++ b/blob/service_test.go @@ -1119,11 +1119,12 @@ func TestBlobVerify(t *testing.T) { expectErr: true, }, { - name: "invalid proof", + name: "invalid row proof", dataRoot: dataRoot, proof: func() Proof { p := blobProof - p.ShareToRowRootProof = p.ShareToRowRootProof[1:] + p.RowToDataRootProof.StartRow = 10 + p.RowToDataRootProof.EndRow = 15 return p }(), blob: *blob, From 712c2728ea118f571019133bb09e28467a29d3d4 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Mon, 5 Aug 2024 19:48:50 +0400 Subject: [PATCH 42/46] fix: add row proof validation --- blob/blob.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/blob/blob.go b/blob/blob.go index 303d2bc606..2c5da49531 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -53,6 +53,9 @@ func (p *Proof) Verify(blob *Blob, dataRoot []byte) (bool, error) { // provided shares are committed to by the data root. func (p *Proof) VerifyShares(rawShares [][]byte, namespace share.Namespace, dataRoot []byte) (bool, error) { // verify the row proof + if err := p.RowToDataRootProof.Validate(dataRoot); err != nil { + return false, fmt.Errorf("%w: invalid row root to data root proof", err) + } if !p.RowToDataRootProof.VerifyProof(dataRoot) { return false, fmt.Errorf("%w: invalid row root to data root proof", ErrInvalidProof) } From 2718e4a2f5de9955e65c679a8accd7822bc1b467 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Mon, 5 Aug 2024 20:10:08 +0400 Subject: [PATCH 43/46] fix: row proof generation and validation --- blob/blob.go | 3 --- blob/service.go | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/blob/blob.go b/blob/blob.go index 2c5da49531..2952ad648d 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -56,9 +56,6 @@ func (p *Proof) VerifyShares(rawShares [][]byte, namespace share.Namespace, data if err := p.RowToDataRootProof.Validate(dataRoot); err != nil { return false, fmt.Errorf("%w: invalid row root to data root proof", err) } - if !p.RowToDataRootProof.VerifyProof(dataRoot) { - return false, fmt.Errorf("%w: invalid row root to data root proof", ErrInvalidProof) - } // verify the share proof ns := append([]byte{namespace.Version()}, namespace.ID()...) diff --git a/blob/service.go b/blob/service.go index b7d6e14c11..6e91e56c9e 100644 --- a/blob/service.go +++ b/blob/service.go @@ -527,7 +527,7 @@ func (s *Service) retrieve( RowRoots: rowRoots, Proofs: rowProofs, StartRow: uint32(inclusiveBlobStartRowIndex), - EndRow: uint32(exclusiveBlobEndRowIndex), + EndRow: uint32(exclusiveBlobEndRowIndex) - 1, }, } return blob, &proof, nil From 3630f332e858c72ae0445e1abb9871b0fcc1e946 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Tue, 6 Aug 2024 14:48:15 +0400 Subject: [PATCH 44/46] chore: use a different error for mismatch commitment --- blob/blob.go | 2 +- blob/service.go | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/blob/blob.go b/blob/blob.go index 2952ad648d..8f42fef0ae 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -40,7 +40,7 @@ func (p *Proof) Verify(blob *Blob, dataRoot []byte) (bool, error) { return false, err } if !blob.Commitment.Equal(blobCommitment) { - return false, fmt.Errorf("%w: generated commitment does not match the provided blob commitment", ErrInvalidProof) + return false, fmt.Errorf("%w: generated commitment does not match the provided blob commitment", ErrMismatchCommitment) } rawShares, err := BlobsToShares(blob) if err != nil { diff --git a/blob/service.go b/blob/service.go index 6e91e56c9e..3ed4400a4a 100644 --- a/blob/service.go +++ b/blob/service.go @@ -32,8 +32,9 @@ import ( ) var ( - ErrBlobNotFound = errors.New("blob: not found") - ErrInvalidProof = errors.New("blob: invalid proof") + ErrBlobNotFound = errors.New("blob: not found") + ErrInvalidProof = errors.New("blob: invalid proof") + ErrMismatchCommitment = errors.New("blob: mismatched commitment") log = logging.Logger("blob") tracer = otel.Tracer("blob/service") From 26de33ebdeb15452b07655455999a9f27b9e47cb Mon Sep 17 00:00:00 2001 From: sweexordious Date: Tue, 6 Aug 2024 14:48:31 +0400 Subject: [PATCH 45/46] test: increase the blob size for the test case to pass --- blob/service_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/blob/service_test.go b/blob/service_test.go index 9975a3a9d1..a8de4817bc 100644 --- a/blob/service_test.go +++ b/blob/service_test.go @@ -1063,7 +1063,7 @@ func generateCommitmentProofFromBlock( } func TestBlobVerify(t *testing.T) { - _, blobs, nss, eds, _, _, dataRoot := edstest.GenerateTestBlock(t, 80, 10) + _, blobs, nss, eds, _, _, dataRoot := edstest.GenerateTestBlock(t, 200, 10) // create the blob from the data blob, err := NewBlob( From a717e4d9ddc5b4dbfdd7b86fe737b4e339a79a56 Mon Sep 17 00:00:00 2001 From: sweexordious Date: Tue, 6 Aug 2024 14:50:15 +0400 Subject: [PATCH 46/46] chore: lint --- blob/blob.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/blob/blob.go b/blob/blob.go index 8f42fef0ae..a21c53c2f1 100644 --- a/blob/blob.go +++ b/blob/blob.go @@ -40,7 +40,10 @@ func (p *Proof) Verify(blob *Blob, dataRoot []byte) (bool, error) { return false, err } if !blob.Commitment.Equal(blobCommitment) { - return false, fmt.Errorf("%w: generated commitment does not match the provided blob commitment", ErrMismatchCommitment) + return false, fmt.Errorf( + "%w: generated commitment does not match the provided blob commitment", + ErrMismatchCommitment, + ) } rawShares, err := BlobsToShares(blob) if err != nil {