From a1177ddc23fad7ffad5f3be1c93db96501c50e76 Mon Sep 17 00:00:00 2001 From: Priya Wadhwa Date: Wed, 6 Jul 2022 16:47:41 +0100 Subject: [PATCH] Fix bug where /retrieve endpoint returns wrong logIndex across shards Signed-off-by: Priya Wadhwa --- pkg/api/entries.go | 78 ++++++++++++++++---------------------- tests/sharding-e2e-test.sh | 4 ++ 2 files changed, 37 insertions(+), 45 deletions(-) diff --git a/pkg/api/entries.go b/pkg/api/entries.go index d7cb832cc..1f0ccc8f7 100644 --- a/pkg/api/entries.go +++ b/pkg/api/entries.go @@ -147,30 +147,13 @@ func logEntryFromLeaf(ctx context.Context, signer signature.Signer, tc TrillianC // GetLogEntryAndProofByIndexHandler returns the entry and inclusion proof for a specified log index func GetLogEntryByIndexHandler(params entries.GetLogEntryByIndexParams) middleware.Responder { ctx := params.HTTPRequest.Context() - tid, resolvedIndex := api.logRanges.ResolveVirtualIndex(int(params.LogIndex)) - tc := NewTrillianClientFromTreeID(ctx, tid) - log.ContextLogger(ctx).Debugf("Retrieving resolved index %v from TreeID %v", resolvedIndex, tid) - - resp := tc.getLeafAndProofByIndex(resolvedIndex) - switch resp.status { - case codes.OK: - case codes.NotFound, codes.OutOfRange, codes.InvalidArgument: - return handleRekorAPIError(params, http.StatusNotFound, fmt.Errorf("grpc error: %w", resp.err), "") - default: - return handleRekorAPIError(params, http.StatusInternalServerError, fmt.Errorf("grpc err: %w", resp.err), trillianCommunicationError) - } - - result := resp.getLeafAndProofResult - leaf := result.Leaf - if leaf == nil { - return handleRekorAPIError(params, http.StatusNotFound, errors.New("grpc returned 0 leaves with success code"), "") - } - - logEntry, err := logEntryFromLeaf(ctx, api.signer, tc, leaf, result.SignedLogRoot, result.Proof, tid, api.logRanges) + logEntry, err := retrieveLogEntryByIndex(ctx, int(params.LogIndex)) if err != nil { + if errors.Is(err, ErrNotFound) { + return handleRekorAPIError(params, http.StatusNotFound, fmt.Errorf("grpc error: %w", err), "") + } return handleRekorAPIError(params, http.StatusInternalServerError, err, err.Error()) } - return entries.NewGetLogEntryByIndexOK().WithPayload(logEntry) } @@ -411,22 +394,14 @@ func SearchLogQueryHandler(params entries.SearchLogQueryParams) middleware.Respo if len(params.Entry.LogIndexes) > 0 { g, _ := errgroup.WithContext(httpReqCtx) - leafResults := make([]*trillian.GetEntryAndProofResponse, len(params.Entry.LogIndexes)) - for i, logIndex := range params.Entry.LogIndexes { - i, logIndex := i, logIndex // https://golang.org/doc/faq#closures_and_goroutines + for _, logIndex := range params.Entry.LogIndexes { + logIndex := logIndex // https://golang.org/doc/faq#closures_and_goroutines g.Go(func() error { - tid, resolvedIndex := api.logRanges.ResolveVirtualIndex(int(swag.Int64Value(logIndex))) - trillianClient := NewTrillianClientFromTreeID(httpReqCtx, tid) - resp := trillianClient.getLeafAndProofByIndex(resolvedIndex) - switch resp.status { - case codes.OK, codes.NotFound: - default: - return resp.err - } - leafResult := resp.getLeafAndProofResult - if leafResult != nil && leafResult.Leaf != nil { - leafResults[i] = leafResult + logEntry, err := retrieveLogEntryByIndex(httpReqCtx, int(swag.Int64Value(logIndex))) + if err != nil { + return err } + resultPayload = append(resultPayload, logEntry) return nil }) } @@ -434,16 +409,6 @@ func SearchLogQueryHandler(params entries.SearchLogQueryParams) middleware.Respo if err := g.Wait(); err != nil { return handleRekorAPIError(params, http.StatusInternalServerError, fmt.Errorf("grpc error: %w", err), trillianUnexpectedResult) } - - for _, result := range leafResults { - if result != nil { - logEntry, err := logEntryFromLeaf(httpReqCtx, api.signer, tc, result.Leaf, result.SignedLogRoot, result.Proof, api.logRanges.ActiveTreeID(), api.logRanges) - if err != nil { - return handleRekorAPIError(params, http.StatusInternalServerError, err, trillianUnexpectedResult) - } - resultPayload = append(resultPayload, logEntry) - } - } } return entries.NewSearchLogQueryOK().WithPayload(resultPayload) @@ -451,6 +416,29 @@ func SearchLogQueryHandler(params entries.SearchLogQueryParams) middleware.Respo var ErrNotFound = errors.New("grpc returned 0 leaves with success code") +func retrieveLogEntryByIndex(ctx context.Context, logIndex int) (models.LogEntry, error) { + tid, resolvedIndex := api.logRanges.ResolveVirtualIndex(logIndex) + tc := NewTrillianClientFromTreeID(ctx, tid) + log.ContextLogger(ctx).Debugf("Retrieving resolved index %v from TreeID %v", resolvedIndex, tid) + + resp := tc.getLeafAndProofByIndex(resolvedIndex) + switch resp.status { + case codes.OK: + case codes.NotFound, codes.OutOfRange, codes.InvalidArgument: + return models.LogEntry{}, ErrNotFound + default: + return models.LogEntry{}, fmt.Errorf("%s: %w", trillianCommunicationError, resp.err) + } + + result := resp.getLeafAndProofResult + leaf := result.Leaf + if leaf == nil { + return models.LogEntry{}, ErrNotFound + } + + return logEntryFromLeaf(ctx, api.signer, tc, leaf, result.SignedLogRoot, result.Proof, tid, api.logRanges) +} + // Retrieve a Log Entry // If a tree ID is specified, look in that tree // Otherwise, look through all inactive and active shards diff --git a/tests/sharding-e2e-test.sh b/tests/sharding-e2e-test.sh index cf24c98c6..ffeb662ff 100755 --- a/tests/sharding-e2e-test.sh +++ b/tests/sharding-e2e-test.sh @@ -265,4 +265,8 @@ stringsMatch $NUM_ELEMENTS "2" NUM_ELEMENTS=$(curl -f -H "Content-Type: application/json" --data '{"logIndexes": [0,3]}' "http://localhost:3000/api/v1/log/entries/retrieve" | jq '. | length') stringsMatch $NUM_ELEMENTS "2" +# Make sure we get the expected LogIndex in the response when calling /retrieve endpoint +RETRIEVE_LOGINDEX1=$(curl -f http://localhost:3000/api/v1/log/entries/retrieve -H "Content-Type: application/json" -H "Accept: application/json" -d "{ \"logIndexes\": [1]}" | jq '.[0]' | jq -r .$UUID1.logIndex) +stringsMatch $RETRIEVE_LOGINDEX1 "1" + echo "Test passed successfully :)"