[Utility] trustless relay e2e happy case (POC PR) #869

adshmh · 2023-07-03T14:28:51Z

Description

[POC] This is a POC PR to add E2E happy case for trsutless relays, described in #754. It is not meant to be merged, but used to:

Review the overall approach
Define all the required small PRs for making the E2E test pass.

Summary generated by Reviewpad on 30 Jul 23 21:42 UTC

This pull request includes changes across multiple files.

Here is a summary of the changes:

In the PostV1ClientRelay function, a new field ApplicationAddress is added to the relayMeta struct.
In the buildJsonRPCRelayPayload function, the field JsonRpc is changed to Jsonrpc for the payload struct. Additionally, the field Id is changed to use a pointer and include the coreTypes.JSONRPCId struct.
The field Parameters in the body.Payload struct is changed to Params.
Added import statements: crypto/tls, encoding/json.
Updated function validateRelayChainSupport: Added a new variable servicerAddrBz and decoded the s.config.Address as a byte array before passing it to readCtx.GetServicer.
Updated function executeJsonRPCRelay: Changed the way payload is marshalled to JSON using json.Marshal instead of codec.GetCodec().Marshal.
Updated function executeHTTPRelay: Added a new import statement crypto/tls. Added a new variable tr of type http.Transport with TLSClientConfig set to tls.Config{InsecureSkipVerify: true}. Modified the http.Client initialization to include the new transport configuration.
In the file shared/core/types/relay.go: In the Validate method of the JSONRPCPayload struct, the p.JsonRpc field has been changed to p.Jsonrpc in the condition and error message. Added a new MarshalJSON method to the JSONRPCId struct to customize the JSON marshalling process.
In the servicer.go file in the app/client/cli directory: Line 107 - Added applicationAddr as an argument to the buildRelay function call. Line 145 - Added a check for unmarshalling the relay payload using JSON. Line 267 - Added ApplicationAddress to the relayMeta struct.
In the file rpc/v1/openapi.yaml: Added a comment indicating the need for support for string, number, and NULL values as per the JSONRPC spec. Removed the format: byte property from the id field. Added an items property to the params field, specifying that it should be an array of strings. Added the application_address field to the Payload object, which is of type string.
In the new file "trustless_relays.feature": Added two scenarios to test the successful response of an application requesting the account balance and the timeout scenario. Also included some comments and TODOs for future improvements and test cases.
In the Makefile: Added a new target test_e2e_relay, modified the test_all_with_json_coverage target, and updated the TODO_KEYWORDS variable.
In the relay_test.go file in the shared/core/types package: Added an import statement for the encoding/json package. Changed the field JsonRpc of the JSONRPCPayload struct to Jsonrpc in two test functions. Added a new test function to check the MarshalJSON method of the JSONRPCPayload struct.
In the file "e2e/tests/steps_init_test.go": Added import statements for "net/http" and "time". Added constant variables and populated maps. Added methods for initialization, sending relays, and checking relay responses. Added a setup function for causing a timeout.
In the relay.proto file: Added a new message JSONRPCId, changed types, renamed fields, and added comments.
In the "configs.yaml" file: Modified the "chains" field and the "service_url" field for one entry.
Added a new file "pocket-servicer-overrides.yaml" with configuration overrides for the Pocket servicer.

Please review these changes carefully and let me know if you need any further assistance.

Issue

Part of work on #754

Type of change

Please mark the relevant option(s):

List of changes

happy e2e test case for trustless relays
all proto/openapi changes needed to make the test pass
all bug fixes needed to make the happy test case pass

Testing

make develop_test; if any code changes were made
make test_e2e on k8s LocalNet; if any code changes were made
e2e-devnet-test passes tests on DevNet; if any code was changed
Docker Compose LocalNet; if any major functionality was changed or introduced
k8s LocalNet; if any infrastructure or configuration changes were made

Required Checklist

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have added, or updated, godoc format comments on touched members (see: tip.golang.org/doc/comment)
I have tested my changes using the available tooling

If Applicable Checklist

I have updated the corresponding README(s); local and/or global
I have added tests that prove my fix is effective or that my feature works
I have added, or updated, mermaid.js diagrams in the corresponding README(s)
I have added, or updated, documentation and mermaid.js diagrams in shared/docs/* if I updated shared/*README(s)

adshmh · 2023-07-03T14:40:42Z

PRs to be opened to cover all the changes here:

adshmh · 2023-07-03T14:43:28Z

@dylanlott and @Olshansk please note this is a POC only PR and not intended to be merged. I have added the list of PRs that will be opened to make the necessary code changes. The review request here is to make sure the overall approach is reasonable.

e2e/tests/steps_init_test.go

charts/pocket/pocket-servicer-overrides.yaml

Olshansk

Did a quick initial (partial) review and will do a deeper one on Wednesday

Olshansk · 2023-07-04T02:07:37Z

build/localnet/manifests/configs.yaml

@@ -1699,7 +1699,7 @@ data:
          "address": "001022b138896c4c5466ac86b24a9bbe249905c2",
          "public_key": "56915c1270bc8d9280a633e0be51647f62388a851318381614877ef2ed84a495",
          "chains": ["0001"],
-          "service_url": "servicer-001-pocket:42069",
+          "service_url": "http://servicer-001-pocket:50832",


why did we update the port?

Port 50832 is the configured RPC port, so 42069 seems incorrect (encountered this while running the E2E test introduced in this PR)

I think we should update all the RPC ports then to avoid a discrepancy:

If you want to do it in a small separate PR (while this one is being reviewed), I support that!

Is the intention to do it in this PR (given the comment)?

I still think a different small "micro PR" could be simpler & faster

I still think a different small "micro PR" could be simpler & faster

Yes, I will open a separate, small PR for this shortly.

charts/pocket/pocket-servicer-overrides.yaml

e2e/tests/steps_init_test.go

shared/k8s/debug.go

rpc/v1/openapi.yaml

utility/servicer/module.go

e2e/tests/trustless_relays.feature

adshmh · 2023-07-04T13:49:56Z

Did a quick initial (partial) review and will do a deeper one on Wednesday

Thank you for the review. I have updated the PR with to address the review comments marked as resolved.

e2e/tests/steps_init_test.go

Olshansk · 2023-07-05T23:07:50Z

build/localnet/manifests/configs.yaml

@@ -1699,7 +1699,7 @@ data:
          "address": "001022b138896c4c5466ac86b24a9bbe249905c2",
          "public_key": "56915c1270bc8d9280a633e0be51647f62388a851318381614877ef2ed84a495",
          "chains": ["0001"],
-          "service_url": "servicer-001-pocket:42069",
+          "service_url": "http://servicer-001-pocket:50832",


I think we should update all the RPC ports then to avoid a discrepancy:

If you want to do it in a small separate PR (while this one is being reviewed), I support that!

charts/pocket/pocket-servicer-overrides.yaml

shared/k8s/debug.go

rpc/handlers.go

e2e/tests/trustless_relays.feature

utility/servicer/module.go

adshmh · 2023-07-06T12:20:07Z

I think we should update all the RPC ports then to avoid a discrepancy:

Thank you for the review. Will open a separate PR for this shortly.

Olshansk

Replied to a few previous comments.
Seems like there are related blocking tests
No further action from my end until the IN_THIS_PR comments are addressed

Olshansk · 2023-07-06T18:52:24Z

build/localnet/manifests/configs.yaml

@@ -1699,7 +1699,7 @@ data:
          "address": "001022b138896c4c5466ac86b24a9bbe249905c2",
          "public_key": "56915c1270bc8d9280a633e0be51647f62388a851318381614877ef2ed84a495",
          "chains": ["0001"],
-          "service_url": "servicer-001-pocket:42069",
+          "service_url": "http://servicer-001-pocket:50832",


Is the intention to do it in this PR (given the comment)?

I still think a different small "micro PR" could be simpler & faster

rpc/handlers.go

Olshansk

@adshmh Seems like there are some broken tests in shared/core/types/relay_test.go that need to be updated as well

## Description  ### Summary generated by Reviewpad on 14 Jul 23 23:17 UTC This pull request includes a series of patches that make changes to the file `debug.go` in the `shared/k8s` directory. Patch 1/5: The patch retrieves keys for all actors from k8s secrets. It adds support for retrieving private keys for validators, servicers, fisherman, and applications. Patch 2/5: This patch fixes a linter error in the `fetchPrivateKeys` function by updating the function signature to use a single string parameter for `resourceName` and `actor`. Patch 3/5: This patch further fixes linter errors in the same function and shortens the code by removing unnecessary switch cases. Patch 4/5: This patch updates the constant for the secret resource name from `privateKeysSecretResourceNameFisherman` to `privateKeysSecretResourceNameFishermen`. Patch 5/5: This patch fixes a typo in the function name `FetchFishermanPrivateKeys` by updating the privateKeysSecretResourceName to `privateKeysSecretResourceNameFishermen`. Please review these changes and ensure they are appropriate.  ## Issue Added as a TODO in #869: <img width="1548" alt="Screenshot 2023-07-12 at 5 10 03 PM" src="https://github.com/pokt-network/pocket/assets/1892194/b1409bdd-4240-4993-b7dd-6197beae5500"> ## Type of change Please mark the relevant option(s): - [ ] New feature, functionality or library - [ ] Bug fix - [x] Code health or cleanup - [ ] Major breaking change - [ ] Documentation - [ ] Other  ## List of changes - Fetch keys for all actors from k8s instead of just validators so they can be used in our debug libraries ## Testing - [x] `make develop_test`; if any code changes were made - [x] `make test_e2e` on [k8s LocalNet](https://github.com/pokt-network/pocket/blob/main/build/localnet/README.md); if any code changes were made - [ ] `e2e-devnet-test` passes tests on [DevNet](https://pocketnetwork.notion.site/How-to-DevNet-ff1598f27efe44c09f34e2aa0051f0dd); if any code was changed - [ ] [Docker Compose LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md); if any major functionality was changed or introduced - [ ] [k8s LocalNet](https://github.com/pokt-network/pocket/blob/main/build/localnet/README.md); if any infrastructure or configuration changes were made ## Required Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have added, or updated, [`godoc` format comments](https://go.dev/blog/godoc) on touched members (see: [tip.golang.org/doc/comment](https://tip.golang.org/doc/comment)) - [ ] I have tested my changes using the available tooling - [ ] I have updated the corresponding CHANGELOG ### If Applicable Checklist - [ ] I have updated the corresponding README(s); local and/or global - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] I have added, or updated, [mermaid.js](https://mermaid-js.github.io) diagrams in the corresponding README(s) - [ ] I have added, or updated, documentation and [mermaid.js](https://mermaid-js.github.io) diagrams in `shared/docs/*` if I updated `shared/*`README(s) Co-authored-by: Dima Kniazev <okdas@users.noreply.github.com>

## Description Update servicers' URLs in LocalNet configuration. ## Issue Part of work on #869 ## Type of change Please mark the relevant option(s): - [ ] New feature, functionality or library - [x] Bug fix - [ ] Code health or cleanup - [ ] Major breaking change - [ ] Documentation - [ ] Other  ## List of changes - Updated LocalNet's config in manifests - Updated Helm charts template ## Testing - [ ] `make develop_test`; if any code changes were made - [ ] `make test_e2e` on [k8s LocalNet](https://github.com/pokt-network/pocket/blob/main/build/localnet/README.md); if any code changes were made - [ ] `e2e-devnet-test` passes tests on [DevNet](https://pocketnetwork.notion.site/How-to-DevNet-ff1598f27efe44c09f34e2aa0051f0dd); if any code was changed - [ ] [Docker Compose LocalNet](https://github.com/pokt-network/pocket/blob/main/docs/development/README.md); if any major functionality was changed or introduced - [x] [k8s LocalNet](https://github.com/pokt-network/pocket/blob/main/build/localnet/README.md); if any infrastructure or configuration changes were made ## Required Checklist - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added, or updated, [`godoc` format comments](https://go.dev/blog/godoc) on touched members (see: [tip.golang.org/doc/comment](https://tip.golang.org/doc/comment)) - [x] I have tested my changes using the available tooling ### If Applicable Checklist - [ ] I have updated the corresponding README(s); local and/or global - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] I have added, or updated, [mermaid.js](https://mermaid-js.github.io) diagrams in the corresponding README(s) - [ ] I have added, or updated, documentation and [mermaid.js](https://mermaid-js.github.io) diagrams in `shared/docs/*` if I updated `shared/*`README(s)

Olshansk · 2023-07-27T22:56:19Z

@adshmh When do you think we'll pick up this work again?

adshmh · 2023-07-27T23:38:13Z

@adshmh When do you think we'll pick up this work again?

Working on it: I will address any remaining ADD_IN_THIS_PR items in a couple of days.

adshmh added 3 commits July 3, 2023 09:48

e2e v0 for trustless relay

4a0530e

e2e test passing for ETH relaychain account balance

9bbc842

Parameterized e2e happy case for trustless relays

8704b69

adshmh added utility Utility specific changes do not merge Prevent merging even with sufficient approvals labels Jul 3, 2023

adshmh self-assigned this Jul 3, 2023

reviewpad bot added medium Pull request is medium waiting-for-review labels Jul 3, 2023

adshmh added this to the M3: Pocket RoS (Relay or Slash) milestone Jul 3, 2023

adshmh requested review from dylanlott and Olshansk July 3, 2023 14:29

Add a servicer config to LocalNet

ef8eda4

dylanlott reviewed Jul 3, 2023

View reviewed changes

e2e/tests/steps_init_test.go Show resolved Hide resolved

dylanlott reviewed Jul 3, 2023

View reviewed changes

charts/pocket/pocket-servicer-overrides.yaml Show resolved Hide resolved

Address review comment

1f2e143

Olshansk requested changes Jul 4, 2023

View reviewed changes

adshmh added 7 commits July 4, 2023 08:48

Address review comment

aee6009

Address review comment

40d9964

Address review comment

6df0ef2

Address review comment

e88fb2e

Address review comment

027a69c

Address review comment

00990a6

Address review comment

14fd3f2

Olshansk requested changes Jul 5, 2023

View reviewed changes

Address review comments

6d6a383

adshmh requested a review from Olshansk July 6, 2023 12:26

Address review comment

bfe7ac5

Olshansk requested changes Jul 6, 2023

View reviewed changes

Address review comments

f41039b

Olshansk requested changes Jul 10, 2023

View reviewed changes

Olshansk mentioned this pull request Jul 13, 2023

[Tooling] Retrieve keys for all actors from k8s secrets #903

Merged

20 tasks

Fix syntax error

ed740db

adshmh mentioned this pull request Jul 20, 2023

[Infra] update LocalNet servicers' URLs #921

Merged

19 tasks

adshmh added 3 commits July 25, 2023 17:40

remove ADD_IN_THIS_PR for updating all RPC ports

fbb4c8f

Merge main

dea5b4d

Address IN_THIS_PR

a48dc0d

adshmh added 2 commits July 29, 2023 08:07

Verify id and jsonrpc fields in e2e response

3c46849

Add sad e2e test case: relay timeout

0822a08

reviewpad bot added large Pull request is large and removed medium Pull request is medium labels Jul 30, 2023

Olshansk mentioned this pull request Jul 31, 2023

[Utility][Consensus] PoS Rewards & Slashing Onboarding & Knowledge Transfer #949

Open

6 tasks

This was referenced Aug 2, 2023

[Utility][Documentation] Add a README section on how to use the portal as a servicer #962

Open

[Utility] Deploy a Ganache node to support E2E testing #963

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Utility] trustless relay e2e happy case (POC PR) #869

[Utility] trustless relay e2e happy case (POC PR) #869

adshmh commented Jul 3, 2023 •

edited by reviewpad bot

Loading

adshmh commented Jul 3, 2023 •

edited

Loading

adshmh commented Jul 3, 2023

Olshansk left a comment

Olshansk Jul 4, 2023

adshmh Jul 4, 2023

Olshansk Jul 5, 2023

Olshansk Jul 6, 2023

adshmh Jul 7, 2023

adshmh commented Jul 4, 2023

Olshansk Jul 5, 2023

adshmh commented Jul 6, 2023

Olshansk left a comment

Olshansk Jul 6, 2023

Olshansk left a comment

Olshansk commented Jul 27, 2023

adshmh commented Jul 27, 2023

[Utility] trustless relay e2e happy case (POC PR) #869

Are you sure you want to change the base?

[Utility] trustless relay e2e happy case (POC PR) #869

Conversation

adshmh commented Jul 3, 2023 • edited by reviewpad bot Loading

Description

Summary generated by Reviewpad on 30 Jul 23 21:42 UTC

Issue

Type of change

List of changes

Testing

Required Checklist

If Applicable Checklist

adshmh commented Jul 3, 2023 • edited Loading

adshmh commented Jul 3, 2023

Olshansk left a comment

Choose a reason for hiding this comment

Olshansk Jul 4, 2023

Choose a reason for hiding this comment

adshmh Jul 4, 2023

Choose a reason for hiding this comment

Olshansk Jul 5, 2023

Choose a reason for hiding this comment

Olshansk Jul 6, 2023

Choose a reason for hiding this comment

adshmh Jul 7, 2023

Choose a reason for hiding this comment

adshmh commented Jul 4, 2023

Olshansk Jul 5, 2023

Choose a reason for hiding this comment

adshmh commented Jul 6, 2023

Olshansk left a comment

Choose a reason for hiding this comment

Olshansk Jul 6, 2023

Choose a reason for hiding this comment

Olshansk left a comment

Choose a reason for hiding this comment

Olshansk commented Jul 27, 2023

adshmh commented Jul 27, 2023

adshmh commented Jul 3, 2023 •

edited by reviewpad bot

Loading

adshmh commented Jul 3, 2023 •

edited

Loading