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.

Sign up for GitHub

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

Jump to bottom

[SDK] feat: Have distinct JSON-RPC and gRPC urls #261

Merged
merged 3 commits into from
Dec 13, 2023
Merged

[SDK] feat: Have distinct JSON-RPC and gRPC urls #261

merged 3 commits into from
Dec 13, 2023

Conversation

red-0ne
Copy link
Contributor

@red-0ne red-0ne commented Dec 12, 2023 •
edited by reviewpad bot
Loading

Summary

Human Summary

Make POKTRollSDK have different query node URLs so it can be used outside of cosmosclient.Context and be imported from external codebases that do not depend on CosmosSDK

Have distinct query node urls:

  • JSON-RPC QueryNodeUrl for event subscriptions
  • gRPC QueryNodeGRPCUrl for query client usage

AI Summary

Summary generated by Reviewpad on 13 Dec 23 14:56 UTC

This pull request includes three patches:

  1. The first patch adds distinct JSON-RPC and gRPC URLs in the Tiltfile and updates the port forwards accordingly.

  2. The second patch triggers e2e tests in the deps_builder.go file.

  3. The third patch fixes the import groups in the supplierquerier.go file.

Issue

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
  • Run E2E tests locally: make test_e2e
  • **Run E2E tests on DevNet: Add the devnet-test-e2e` label to the PR. This is VERY expensive, o only do it after all the reviews are complete.

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

@red-0ne red-0ne added the off-chain Off-chain business logic label Dec 12, 2023
@red-0ne red-0ne added this to the Shannon TestNet milestone Dec 12, 2023
@red-0ne red-0ne requested review from Olshansk and h5law December 12, 2023 15:40
@red-0ne red-0ne self-assigned this Dec 12, 2023
@github-actions GitHub Actions
Copy link

The CI will now also run the e2e tests on devnet, which increases the time it takes to complete all CI checks.

@github-actions github-actions bot added devnet push-image CI related - pushes images to ghcr.io labels Dec 12, 2023
@red-0ne red-0ne mentioned this pull request Dec 12, 2023
Closed
4 tasks
h5law
h5law approved these changes Dec 13, 2023
View reviewed changes
Copy link
Contributor

@h5law h5law left a comment

Choose a reason for hiding this comment

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

LGTM just double check imports <3

pkg/client/query/supplierquerier.go Show resolved Hide resolved
pkg/client/query/supplierquerier.go
); err != nil {
return nil, err
}

supq.supplierQuerier = suppliertypes.NewQueryClient(cosmosclient.Context(supq.clientCtx))
supq.supplierQuerier = suppliertypes.NewQueryClient(supq.clientConn)
Copy link
Contributor

Choose a reason for hiding this comment

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

🔥

@red-0ne red-0ne merged commit d621631 into main Dec 13, 2023
10 checks passed
Olshansk
Olshansk reviewed Dec 13, 2023
View reviewed changes
pkg/sdk/deps_builder.go
@@ -84,3 +85,10 @@ func (sdk *poktrollSDK) buildDeps(

return deps, nil
}

// hostToWebsocketURL converts the provided host into a websocket URL that can
Copy link
Member

Choose a reason for hiding this comment

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

Going to update this in my PR. Should be queryNodeToWebsocketUrl

h5law added a commit that referenced this pull request Dec 14, 2023
@h5law
Squashed commit of the following:
7e19624 
commit d621631
Author: Redouane Lakrache <[email protected]>
Date:   Wed Dec 13 16:24:34 2023 +0100

    [SDK] feat: Have distinct JSON-RPC and gRPC urls (#261)

    * feat: Have distinct JSON-RPC and gRPC urls

    * chore: Trigger e2e tests

    * chore: Fix import groups

commit 4e30e27
Author: Bryan White <[email protected]>
Date:   Wed Dec 13 14:28:36 2023 +0100

    fix PR template testing checklist item (#268)
h5law added a commit that referenced this pull request Dec 14, 2023
@h5law
squash: merge with main
79dd438 
commit df73cfa
Author: Bryan White <[email protected]>
Date:   Thu Dec 14 01:06:03 2023 +0100

    refactor: `NewMinedRelay` to shared testutil (#262)

commit 410965a
Author: Bryan White <[email protected]>
Date:   Thu Dec 14 01:05:52 2023 +0100

    fix: PR template typo 2 (#269)

commit dd7df68
Author: Daniel Olshansky <[email protected]>
Date:   Wed Dec 13 13:45:26 2023 -0800

    [Testing] Unit tests, E2E tests, logging and other Fixes / "Touchups" (#253)

    This is aiming to fix multiple issues in E2E tests and just general QoL improvements:

    1. Adding a comment in `relay.feature` about the need to run `make supplier1_stake && make app1_stake` until we fix #180 (known issue)
    2. Replacing the reference to host `sequencer-poktroll-sequencer` with `localhost` (see first screenshot below)
    3. Fixed a bug leading us to assert against a `MsgSubmitProof` event instead of a `MsgCreateClaim` event (see second screenshot below)
    4. Fixed on-chain logging under `x/*` by replacing `logger\.(Info|Error)\("([^"]+)"(?:, (.*))?\)` with `logger.$1(fmt.Sprintf("$2", $3))` so it works as expected
    5. Some quality-of-life comments & TODOs
    6. Minor additions to unit tests from #220

commit d621631
Author: Redouane Lakrache <[email protected]>
Date:   Wed Dec 13 16:24:34 2023 +0100

    [SDK] feat: Have distinct JSON-RPC and gRPC urls (#261)

    * feat: Have distinct JSON-RPC and gRPC urls

    * chore: Trigger e2e tests

    * chore: Fix import groups

commit 4e30e27
Author: Bryan White <[email protected]>
Date:   Wed Dec 13 14:28:36 2023 +0100

    fix PR template testing checklist item (#268)
okdas pushed a commit that referenced this pull request Dec 19, 2023
@red-0ne @okdas
[SDK] feat: Have distinct JSON-RPC and gRPC urls (#261)
b9a9dac 
* feat: Have distinct JSON-RPC and gRPC urls

* chore: Trigger e2e tests

* chore: Fix import groups
bryanchriswhite added a commit that referenced this pull request Dec 22, 2023
@bryanchriswhite
Merge remote-tracking branch 'pokt/main' into issues/141/refactor/cla…
7994ca2 
…im-proof

* pokt/main:
  Fix bug introduced by #252 where genesis file was no longer being copied to the right location
  [Docs] Introduce Docusaurus documentation (#252)
  [Cleanup] Centralzie websocket url -> endpoint changes (#272)
  refactor: `NewMinedRelay` to shared testutil (#262)
  fix: PR template typo 2 (#269)
  [Testing] Unit tests, E2E tests, logging and other Fixes / "Touchups" (#253)
  [SDK] feat: Have distinct JSON-RPC and gRPC urls (#261)
bryanchriswhite added a commit that referenced this pull request Dec 22, 2023
@bryanchriswhite
Merge branch 'issues/141/refactor/claim-proof' into issues/141/refact…
6a5b3e6 
…or/supplier-keys

* issues/141/refactor/claim-proof:
  chore: review feedback improvements
  chore: review feedback improvements
  Fix bug introduced by #252 where genesis file was no longer being copied to the right location
  [Docs] Introduce Docusaurus documentation (#252)
  [Cleanup] Centralzie websocket url -> endpoint changes (#272)
  refactor: `NewMinedRelay` to shared testutil (#262)
  fix: PR template typo 2 (#269)
  [Testing] Unit tests, E2E tests, logging and other Fixes / "Touchups" (#253)
  [SDK] feat: Have distinct JSON-RPC and gRPC urls (#261)
bryanchriswhite added a commit that referenced this pull request Dec 22, 2023
@bryanchriswhite
Merge branch 'issues/141/refactor/supplier-keys' into issues/141/refa…
0351cc7 
…ctor/supplier-errors

* issues/141/refactor/supplier-keys:
  chore: review feedback improvements
  chore: review feedback improvements
  Fix bug introduced by #252 where genesis file was no longer being copied to the right location
  [Docs] Introduce Docusaurus documentation (#252)
  [Cleanup] Centralzie websocket url -> endpoint changes (#272)
  refactor: `NewMinedRelay` to shared testutil (#262)
  fix: PR template typo 2 (#269)
  [Testing] Unit tests, E2E tests, logging and other Fixes / "Touchups" (#253)
  [SDK] feat: Have distinct JSON-RPC and gRPC urls (#261)
@bryanchriswhite bryanchriswhite removed push-image CI related - pushes images to ghcr.io devnet-test-e2e labels May 16, 2024
@github-actions github-actions bot removed the devnet label May 16, 2024
okdas pushed a commit that referenced this pull request Nov 14, 2024
@red-0ne
[SDK] feat: Have distinct JSON-RPC and gRPC urls (#261)
44f2b43 
* feat: Have distinct JSON-RPC and gRPC urls

* chore: Trigger e2e tests

* chore: Fix import groups
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Olshansk Olshansk Olshansk left review comments

@h5law h5law h5law approved these changes

Assignees

@red-0ne red-0ne

Labels
off-chain Off-chain business logic
Projects
Shannon
Status: ✅ Done
Milestone
Shannon Private TestNet
Development

Successfully merging this pull request may close these issues.
4 participants
@red-0ne @Olshansk @h5law @bryanchriswhite