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 TxContext #118

Merged
merged 15 commits into from
Nov 2, 2023
Merged

[Miner] feat: add TxContext #118

merged 15 commits into from
Nov 2, 2023

Conversation

bryanchriswhite
Copy link
Contributor

@bryanchriswhite bryanchriswhite commented Oct 30, 2023

Summary

Human Summary

Adds TxContext interface & cosmosTxContext implementation which abstracts cosmos-specific API into discrete concerns.

AI Summary

Summary generated by Reviewpad on 02 Nov 23 11:30 UTC

This pull request includes various changes across multiple files.

In the internal/testclient package:

  • The file common.go has been deleted, which contained the CometLocalWebsocketURL constant set to "ws://localhost:36657/websocket".
  • A new file keyring.go has been added, which defines the NewKey function for generating a new key and mnemonic for testing purposes.

In the package testclient:

  • The file localnet.go has been added, which includes various imports, functions, and constants related to localnet testing. Some notable additions include the CometLocalWebsocketURL constant and the EncodingConfig variable.

In the either package:

  • The types.go file has been modified to include changes to the AsyncError type and the introduction of the Bytes type.

In the pkg/client/tx package:

  • The context.go file has been added, which defines the cosmosTxContext struct implementing the client.TxContext interface. This file also includes various methods related to transaction context within the Cosmos SDK.

In the interface.go file:

  • Changes have been made to include go:generate directives, import packages, and introduce new interfaces for the client, keyring, and types.

These changes add functionality related to localnet testing, key generation, transaction context, and types across multiple packages.

Issue

Relates to:

TxContext

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_develop_and_test
  • 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 miner Changes related to the Miner off-chain Off-chain business logic labels Oct 30, 2023
@bryanchriswhite bryanchriswhite added this to the Shannon TestNet milestone Oct 30, 2023
@bryanchriswhite bryanchriswhite self-assigned this Oct 30, 2023
pkg/client/tx/context.go Outdated Show resolved Hide resolved
return txCtxMock
}

func NewAnyTimesTxTxContext(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@h5law PTAL re: mocking cosmos' client context object...

@bryanchriswhite bryanchriswhite changed the title Feat/tx context [Miner] feat: add TxContext Oct 30, 2023
@bryanchriswhite bryanchriswhite linked an issue Oct 31, 2023 that may be closed by this pull request
10 tasks
@bryanchriswhite bryanchriswhite marked this pull request as ready for review October 31, 2023 14:38
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.

No blockers. Just a couple NITs & questions, but otherwise g2g!

internal/testclient/common.go Outdated Show resolved Hide resolved
internal/testclient/keyring.go Outdated Show resolved Hide resolved
internal/testclient/common.go Outdated Show resolved Hide resolved
internal/testclient/common.go Outdated Show resolved Hide resolved
pkg/client/tx/context.go Show resolved Hide resolved
internal/testclient/testtx/context.go Show resolved Hide resolved
internal/testclient/testtx/context.go Outdated Show resolved Hide resolved
internal/testclient/testtx/context.go Outdated Show resolved Hide resolved
internal/testclient/testtx/context.go Outdated Show resolved Hide resolved
internal/testclient/testtx/context.go Show resolved Hide resolved
(cherry picked from commit 78184e523e62450fe5fc7fe1d23906213e8c81bc)
* 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)
(cherry picked from commit e2303a5b776830815457f12f5e371e916e3bee93)
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 couple NITs but LGTM otherwise!

@@ -1,4 +1,4 @@
//go:generate mockgen -destination=../../internal/mocks/mockclient/events_query_client_mock.go -package=mockclient . Dialer,Connection
//go:generate mockgen -destination=../../internal/mocks/mockclient/events_query_client_mock.go -package=mockclient . Dialer,Connection,EventsQueryClient
Copy link
Member

Choose a reason for hiding this comment

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

Seeing we have different packages but keep them defined in interface.go, it'll be really nice to get a sense of all the different interfaces we have available.

In fact, this is how I went down the #128 🐇 hole

pkg/client/interface.go Show resolved Hide resolved
pkg/client/interface.go Show resolved Hide resolved
* pokt/main:
  [Testing] fix: flaky tests in observable & client pkgs (#124)
  Added first roadmap change
@bryanchriswhite bryanchriswhite merged commit b8b7572 into main Nov 2, 2023
@bryanchriswhite bryanchriswhite deleted the feat/tx-context branch November 2, 2023 12:45
bryanchriswhite added a commit that referenced this pull request Nov 2, 2023
* pokt/godoc_support:
  fix: goimports
  Update path to main package in config.yml
  Import updates
  Update Makefile
  Merge with main. Remove .gitkeep. Add back mocks.go. Tests are passing
  [Miner] feat: add `TxContext` (#118)
  [Testing] fix: flaky tests in observable & client pkgs (#124)
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)
  ...
okdas pushed a commit that referenced this pull request Nov 14, 2024
* chore: add `TxContext` interface

* feat: add `cosmosTxContext` implementation

* test: add `TxContext` test helpers

* chore: move godoc comment above `AsyncError`

* fixup: goimports

* chore: add godoc comments to tx context test helpers

* chore: cleanup

* chore: add godoc comments to common testclient helpers

* chore: review feedback improvements

(cherry picked from commit 78184e523e62450fe5fc7fe1d23906213e8c81bc)

* chore: update comments

* refactor: simplify tx context teset helpers

(cherry picked from commit e2303a5b776830815457f12f5e371e916e3bee93)

* chore: add godoc comments
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 off-chain Off-chain business logic
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

[Miner] Foundation for the Miner submodule
2 participants