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 block client #65

Merged
merged 97 commits into from
Nov 1, 2023
Merged

[Miner] feat: add block client #65

merged 97 commits into from
Nov 1, 2023

Conversation

bryanchriswhite
Copy link
Contributor

@bryanchriswhite bryanchriswhite commented Oct 14, 2023

Summary

Adds BlockClient 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:

BlockClient 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 linked an issue Oct 14, 2023 that may be closed by this pull request
10 tasks
@bryanchriswhite bryanchriswhite force-pushed the feat/query-client branch 4 times, most recently from e9c47f9 to 0d73bd8 Compare October 18, 2023 12:09
@bryanchriswhite bryanchriswhite force-pushed the feat/query-client branch 6 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
* feat/replay-observable:
  feat: add replay observable
@bryanchriswhite bryanchriswhite changed the base branch from main to feat/replay-observable October 27, 2023 12:45
@bryanchriswhite bryanchriswhite changed the base branch from feat/replay-observable to main October 27, 2023 12:52
@bryanchriswhite bryanchriswhite changed the base branch from main to feat/replay-observable October 27, 2023 12:53
* pokt/main:
  Update main README.md
  [Miner] feat: add replay observable (#93)
  [Proxy] feat: add general proxy logic and server builder (#96)
  [Code Health] Replace `mocks.go` w/ `.gitkeep` (#103)
  [Application] Add `ServiceConfigs` to `AppStaking` (#95)
@bryanchriswhite bryanchriswhite changed the base branch from feat/replay-observable to main October 30, 2023 20:16
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 last minute NITs but no blockers.

I didn't do a deep dive into pkg/retry/retry_test.go since it's really hard to build context into this. When the TODOs are tended to, it'll be easier to compartmentalize on a per-PR basis.

internal/testclient/testblock/client.go Show resolved Hide resolved
pkg/client/block/client.go Outdated Show resolved Hide resolved
pkg/client/block/client.go Outdated Show resolved Hide resolved
pkg/client/block/client.go Outdated Show resolved Hide resolved
pkg/client/block/client.go Outdated Show resolved Hide resolved
pkg/client/block/client.go Outdated Show resolved Hide resolved
pkg/client/block/client.go Outdated Show resolved Hide resolved
pkg/client/block/client_integration_test.go Show resolved Hide resolved
@@ -0,0 +1,337 @@
package retry_test

/* TODO_TECHDEBT: improve this test:
Copy link
Member

Choose a reason for hiding this comment

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

#EZ

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😅

}

/*
TODO_TECHDEBT/TODO_CONSIDERATION(#XXX): this duplicates the unexported block event
Copy link
Member

Choose a reason for hiding this comment

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

Noice

@bryanchriswhite
Copy link
Contributor Author

Waiting to merge until CI is working and green:

@okdas
Copy link
Member

okdas commented Oct 31, 2023

Closing this as ignite.com now works. Will add a comment linking to this PR in case a similar issue arises.

@okdas okdas closed this Oct 31, 2023
@Olshansk
Copy link
Member

Olshansk commented Oct 31, 2023

@okdas Shouldn't we close #116 but keep this one open?

@okdas okdas reopened this Oct 31, 2023
@okdas
Copy link
Member

okdas commented Oct 31, 2023

@okdas Shouldn't we close #116 but keep this one open?

LOL, sorry. I pushed the comment on the wrong tab. :)

@Olshansk
Copy link
Member

@bryanchriswhite Try to merge with main to see if CI passes and please take not of #121 as well

@okdas
Copy link
Member

okdas commented Oct 31, 2023

@bryanchriswhite bryanchriswhite merged commit e85cc8a into main Nov 1, 2023
bryanchriswhite added a commit that referenced this pull request Nov 1, 2023
* pokt/main:
  [Miner] feat: add block client (#65)
bryanchriswhite added a commit that referenced this pull request Nov 1, 2023
* pokt/main:
  [Miner] feat: add block client (#65)
  [Supplier] Add `ServiceConfigs` to `SupplierStaking` (#114)
  [AppGate] Add the MaxDelegatedGateways parameter (#109)
  [Test] Temporarily skip a flaky test `TestEventsQueryClient_Subscribe_Succeeds` (#121)
  [AppGate] Implement DelegateToGateway and add Tests (#90)
  [E2E] Add (Un)Stake Tests (#88)
bryanchriswhite added a commit that referenced this pull request Nov 7, 2023
…plementation

* pokt/main: (28 commits)
  [Miner] feat: add supplier client (#42)
  [Off-chain] refactor: keyring errors & helpers (#131)
  [Miner] feat: add `TxClient` (#94)
  [CI] Build container images (#107)
  fix: flaky block client test (#132)
  [Tooling] add `go_lint` & `go_imports` make targets & CI step (#129)
  Update README.md
  [Code Health] Support `godoc` by replacing the `pocket `module name with `github.com/pokt-network/poktroll` (#128)
  [Miner] feat: add `TxContext` (#118)
  [Testing] fix: flaky tests in observable & client pkgs (#124)
  Added first roadmap change
  [AppGate] Implement UndelegateFromGateway with Extensive Tests (#125)
  [Miner] feat: add block client (#65)
  [Supplier] Add `ServiceConfigs` to `SupplierStaking` (#114)
  [AppGate] Add the MaxDelegatedGateways parameter (#109)
  [Test] Temporarily skip a flaky test `TestEventsQueryClient_Subscribe_Succeeds` (#121)
  [AppGate] Implement DelegateToGateway and add Tests (#90)
  [E2E] Add (Un)Stake Tests (#88)
  refactor: add `either.Bytes` alias (#117)
  feat: add either.AsyncErr type & helpers (#115)
  ...
@Olshansk Olshansk deleted the feat/block-client branch May 29, 2024 16:44
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

* chore: add block client interface

* chore: add `MapReplay` operator

* feat: add block client

* test: block client integration

* test: block client

* docs: fix install instructions

* fix: race on eventsBytesAndConns map

* fix: interface assertions

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

* fix: race

* Small updates to the README

* refactor: add observableInternals interface

(cherry picked from commit 5d149e5297ce7d11dad77983f53be53efd8dae15)

* chore: update last; only block for 1 value min

(cherry picked from commit b24a5e586e9c776a962008043d065a2294fd921c)

* chore: review improvements

* refactor: move add `channelObservableInternals` & migrate its relevant methods & state from channelObservable

* refactor: simplify, cleanup, & improve comments

* chore: review improvements

(cherry picked from commit 31555cdc68211964358c43842e0581f565d1afff)

* refactor: eliminate `EventsQueryClient#requestId` field

(cherry picked from commit ccb1d6981f67ab860cb65dde4da15d89bcf57875)

* refactor: review improvements

* refactor: eliminate `EventsQueryClient#requestId` field

* refactor: move websocket dialer and connection to own pkg

* chore: add comment

* fix: notify `retryOnError()` of async error propagating through `#EventsBytes()` observable

* chore: review improvements

* chore: move `EventsBytesObservable type above interfaces

* chore: review improvements

* fix: bug & improve naming & comments

* chore: review improvements

* fix: bug in `accumulateReplayValues()`

* refactor: promote `retryOnError` to its own pkg: `retry.OnError`

* chore: improve comments

* test: inline wip test helpers

* test: skip retry.OnError tests &  comment

* chore: review feedback improvements

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

* chore: review feedback improvements

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

* fix: format placeholder error

---------

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