Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix: Return all inner transactions are returned for logs endpoint. #915

Merged
merged 6 commits into from
Mar 9, 2022

Conversation

algochoi
Copy link
Contributor

@algochoi algochoi commented Mar 9, 2022

Summary

This PR removes deduplication when returning inner transaction results. Without this check, a query that should return all matched inner txns will return only the first result as inner txns all share the same txID as their root txn. Currently, this only affects the applications/ID/logs endpoint, as inner txns are otherwise "rolled up" and any queries matches on inner txns return its root txn anyway.

Test Plan

Added test case to check that results are returned properly without deduplication based on txID when logs are queried. Tested locally on sandbox as well, and multiple logs with same txIDs show up.

Prior to this change, the added unit test will only return the first inner txn's logs and fail.

@algochoi algochoi self-assigned this Mar 9, 2022
@algochoi algochoi marked this pull request as ready for review March 9, 2022 16:47
@codecov-commenter
Copy link

codecov-commenter commented Mar 9, 2022

Codecov Report

Merging #915 (f38da76) into develop (4eaadeb) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #915   +/-   ##
========================================
  Coverage    58.03%   58.03%           
========================================
  Files           40       40           
  Lines         4637     4637           
========================================
  Hits          2691     2691           
  Misses        1615     1615           
  Partials       331      331           
Impacted Files Coverage Δ
api/handlers.go 69.27% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4eaadeb...f38da76. Read the comment docs.

@algochoi algochoi requested a review from winder March 9, 2022 19:02
@winder winder merged commit 82b9fe3 into develop Mar 9, 2022
@winder winder changed the title Don't do deduplication based on txID when returning inner transactions Bugfix: Return all inner transactions are returned for logs endpoint. Mar 9, 2022
@winder winder deleted the algochoi/fix-dedup-logs branch March 9, 2022 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants