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

[Miner] feat: add events query client #64

Merged
merged 64 commits into from
Oct 27, 2023
Merged

Conversation

bryanchriswhite
Copy link
Contributor

@bryanchriswhite bryanchriswhite commented Oct 14, 2023

@Reviewer

This PR may be more digestible / reviewable on a commit-by-commit basis. Commits are organized logically and any given line is only modified in a single commit, with few exceptions*.

*(In the interest of preserving the git-time-continuum 👮🚨, this applies in batches of commits between comments or reviews by humans, only once "in review")

Summary

Adds EventsQueryClient interface and implementation.

---
title: Clients Dependency Tree
---
flowchart

sup[SupplierClient]
tx[TxClient]
bl[BlockClient]
evt[EventsQueryClient]

sup --"#SignAndBroadcast()"--> tx
tx --"#CommittedBlocksSequence()"--> bl
tx --"#EventsBytes()"--> evt
bl --"#EventsBytes()"--> evt
Loading

Issue

Relates to:

QueryClient is a nested dependency of Miner.

Type of change

Select one or more:

  • New feature, functionality or library
  • Bug fix
  • Code health or cleanup
  • Documentation
  • Other (specify)

Testing

  • Run all unit tests: make go_test_integration
  • Verify Localnet manually: See the instructions [here](TODO: add link to instructions)

Sanity Checklist

  • I have tested my changes using the available tooling
  • I have performed a self-review of my own code
  • I have commented my code, updated documentation and left TODOs throughout the codebase

@bryanchriswhite bryanchriswhite added the miner Changes related to the Miner label Oct 14, 2023
@bryanchriswhite bryanchriswhite added this to the Shannon TestNet milestone Oct 14, 2023
@bryanchriswhite bryanchriswhite self-assigned this Oct 14, 2023
@bryanchriswhite bryanchriswhite changed the title [Miner] feat: query client [Miner] feat: add query client Oct 14, 2023
@bryanchriswhite bryanchriswhite linked an issue Oct 14, 2023 that may be closed by this pull request
10 tasks
Makefile Outdated Show resolved Hide resolved
@bryanchriswhite bryanchriswhite changed the base branch from wip/merge/observable_x_supplier-msgs to merge/observable_x_supplier-msgs October 18, 2023 11:09
@bryanchriswhite bryanchriswhite force-pushed the feat/query-client branch 2 times, most recently from e9c47f9 to 0d73bd8 Compare October 18, 2023 12:09
@bryanchriswhite bryanchriswhite force-pushed the feat/query-client branch 2 times, most recently from 6bc8e56 to 4f9f3ea Compare October 19, 2023 20:47
@bryanchriswhite bryanchriswhite changed the title [Miner] feat: add query client [Miner] feat: add events query client Oct 20, 2023
@bryanchriswhite bryanchriswhite force-pushed the feat/query-client branch 4 times, most recently from 0439247 to 015e5ef Compare October 20, 2023 11:09
(cherry picked from commit 22371aa550eb0060b528f4573ba6908bbdfa0c1c)
* pokt/main:
  fix: RelayerProxy interface mismatch (#91)
  [RelayerProxy] feat: implement relayerProxy struct (#82)
  [AppGate] Scaffold undelegate-from-gateway message & nothing else (#86)
  [AppGate] Scaffold delegate-to-gateway message and nothing else (#85)
  [Supplier] Implement MsgUnstakeSupplier w/ Tests (#77)
  [Supplier] Implement MsgStakeSupplier & Add Extensive Tests (#76)
  [RelayerProxy] feat: add RelayerProxy interface (#80)
(cherry picked from commit ab21790164ab544ae5f1508d3237a3faab33e71e)
* feat/observable-map:
  feat: add the map channel observable operator
bryanchriswhite and others added 3 commits October 25, 2023 14:57
* merge/map_x_replay:
  fix: race
  chore: review improvements
  fix: comment typo
  fix: interface assertion
  test: improve and add tests
  chore: add warning log and improve comments
  [Miner] feat: add the map channel observable operator (#92)
…-client

* pokt/feat/query-client:
  fix: interface assertions
Copy link
Member

@Olshansk Olshansk left a comment

Choose a reason for hiding this comment

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

@bryanchriswhite

  1. Reviewed everything other than pkg/client/events_query/client_test.go
  2. The base branch needs to be updated
  3. Still wrapping my head around all the different clients/components but it's slowly coming together
  4. I pushed some NITs to the README so please make sure to do a git pull
  5. Great job!

internal/testclient/common.go Show resolved Hide resolved
internal/testclient/common.go Outdated Show resolved Hide resolved
pkg/client/events_query/client.go Outdated Show resolved Hide resolved
pkg/client/events_query/client.go Outdated Show resolved Hide resolved
pkg/client/events_query/client.go Show resolved Hide resolved
docs/pkg/client/events_query.md Outdated Show resolved Hide resolved
docs/pkg/client/events_query.md Outdated Show resolved Hide resolved
docs/pkg/client/events_query.md Outdated Show resolved Hide resolved
@bryanchriswhite

This comment was marked as outdated.

pkg/client/events_query/client.go Outdated Show resolved Hide resolved

import (
"github.com/gorilla/websocket"
gorillaws "github.com/gorilla/websocket"
Copy link
Member

Choose a reason for hiding this comment

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

I wasn't aware gorrilas have laws 🦍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

gorillaws

pkg/client/events_query/client.go Show resolved Hide resolved
Copy link
Member

@Olshansk Olshansk left a comment

Choose a reason for hiding this comment

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

A few minor nits plus a request to #PUC, but otherwise lgtm!

@bryanchriswhite bryanchriswhite dismissed red-0ne’s stale review October 27, 2023 10:27

All comments resolved.

* main:
  [Session] Implement the 1st iteration of the SessionHydrator (#78)
  Update pull_request_template.md
  [Test] Enforce `msg.ValidateBasic` in all the `keepers` and `msgServers` for redundancy (#100)
  test: add itest script and makefile target (#99)
@bryanchriswhite bryanchriswhite changed the base branch from merge/map_x_replay to main October 27, 2023 10:41
@bryanchriswhite bryanchriswhite merged commit 2c78154 into main Oct 27, 2023
bryanchriswhite added a commit that referenced this pull request Oct 27, 2023
* pokt/main:
  [Miner] feat: add events query client (#64)
bryanchriswhite added a commit that referenced this pull request Oct 27, 2023
* pokt/main:
  [Miner] feat: add events query client (#64)
@bryanchriswhite bryanchriswhite deleted the feat/query-client branch November 6, 2023 09:39
okdas pushed a commit that referenced this pull request Nov 14, 2024
* feat: add the map channel observable operator

(cherry picked from commit 22371aa550eb0060b528f4573ba6908bbdfa0c1c)

* feat: add replay observable

(cherry picked from commit ab21790164ab544ae5f1508d3237a3faab33e71e)

* chore: add query client interface

* chore: add query client errors

* test: fix false positive, prevent regression, & add comments

* chore: add godoc comment

* feat: add query client implementation

* chore: add connection & dialer wrapper implementations

* test: query client & add testquery helper pkg

* chore: add go_test_integration make target

* chore: add internal mocks pkg

* test: query client integration test

* docs: add event query client docs

* chore: update go.mod

* chore: re-order `eventsQueryClient` methods to improve readability

* chore: add godoc comments to testclient helpers

* fix: comment formatting

* chore: improve comment & naming in evt query client test

* test: tune events query client parameters

* chore: improve godoc comments

* chore: review improvements

* refactor: `replayObservable` as its own interface type

* refactor: `replayObservable#Next() V`  to `ReplayObservable#Last(ctx, n) []V`

* chore: add constructor func for `ReplayObservable`

* test: reorder to improve readibility

* refactor: rename and add godoc comments

* chore: improve naming & comments

* chore: add warning log and improve comments

* test: improve and add tests

* fix: interface assertion

* fix: comment typo

* chore: review improvements

* fix: race

* fix: race on eventsBytesAndConns map

* fix: interface assertions

Co-authored-by: Redouane Lakrache <[email protected]>

* fix: race

* Small updates to the README

* chore: review improvements

(cherry picked from commit 31555cdc68211964358c43842e0581f565d1afff)

* refactor: eliminate `EventsQueryClient#requestId` field

(cherry picked from commit ccb1d6981f67ab860cb65dde4da15d89bcf57875)

* refactor: eliminate `EventsQueryClient#requestId` field

* refactor: move websocket dialer and connection to own pkg

* chore: add comment

* chore: move `EventsBytesObservable type above interfaces

* chore: review improvements

* fix: bug & improve naming & comments

* chore: review improvements

* chore: review improvements

* chore: add comment

Co-authored-by: Daniel Olshansky <[email protected]>

* revert: replay observable, merged into previous base branch

---------

Co-authored-by: Redouane Lakrache <[email protected]>
Co-authored-by: Daniel Olshansky <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
miner Changes related to the Miner
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

[Miner] Foundation for the Miner submodule
4 participants