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

Retrieve the block results that include the event with the exact attributes in query #2870

Merged
merged 11 commits into from
Nov 30, 2022

Conversation

romac
Copy link
Member

@romac romac commented Nov 17, 2022

Closes: #2867
Closes: #2868


PR author checklist:

  • Added changelog entry, using unclog.
  • Added tests: integration (for Hermes) or unit/mock tests (for modules).
  • Linked to GitHub issue.
  • Updated code comments and documentation (e.g., docs/).
  • Tagged one reviewer who will be the one responsible for shepherding this PR.

Reviewer checklist:

  • Reviewed Files changed in the GitHub PR explorer.
  • Manually tested (in case integration/unit/mock tests are absent).

ancazamfir and others added 5 commits November 14, 2022 14:30
There should only be a single match for this query, but due to
the fact that the indexer treat the query as a disjunction over
all events in a block rather than a conjunction over a single event,
we may end up with partial matches and therefore have to account for
that by fetching multiple results and filter it down after the fact.
@ancazamfir
Copy link
Collaborator

I think also fixes #2868

@adizere
Copy link
Member

adizere commented Nov 25, 2022

What do you think about untangling this PR to separate the changes related to issue 2868 apart from issue 2867?

Context:

Summary:

@ancazamfir
Copy link
Collaborator

I'll take a look at the tendermint PR and run a custom hermes version that uses that. But it seems that we still need to temp fix hermes with the PR here until all chains move to the new tendermint version.

@romac romac marked this pull request as ready for review November 30, 2022 06:32
Copy link
Collaborator

@ancazamfir ancazamfir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks @romac!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants