[Refactor] Replace embedded Service with ServiceId #802

red-0ne · 2024-09-07T03:10:50Z

Summary

This PR replaces all embedded Service entries with ServiceId.

Most of the changes are Service -> ServiceId replacements but special attention has to be put on:

x/session/types/session_header.go
x/shared/helpers/service_configs.go
x/shared/types/service.go
x/service/types/message_add_service.go
proto/poktroll/shared/service.proto
proto/poktroll/session/types.proto
proto/poktroll/session/query.proto

A follow-up to update Shannon-SDK is expected after approval.

E2E tests pass locally.

Issue

Type of change

Select one or more from the following:

New feature, functionality or library
Consensus breaking; add the consensus-breaking label if so. See [Infra] Automatically add the consensus-breaking label #791 for details
Bug fix
Code health or cleanup
Documentation
Other (specify)

Testing

Documentation: make docusaurus_start; only needed if you make doc changes
Unit Tests: make go_develop_and_test
LocalNet E2E Tests: make test_e2e
DevNet E2E Tests: Add the devnet-test-e2e label to the PR.

Sanity Checklist

I have tested my changes using the available tooling
I have commented my code
I have performed a self-review of my own code; both comments & source code
I create and reference any new tickets, if applicable
I have left TODOs throughout the codebase, if applicable

…dation' into HEAD

…ervice-to-id

Olshansk

That was much easier than expected, eh?

x/shared/types/service.go

bryanchriswhite

👌 Nice one @red-0ne! 😎

It's interesting to see the effect this change has on the imports. I wonder if it has any effect on the module dependency tree. 🤔

x/proof/types/message_submit_proof_test.go

x/session/keeper/session_hydrator.go

x/session/types/errors.go

x/shared/types/service.go

github-actions · 2024-09-10T12:58:14Z

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

You may need to run make trigger_ci to submit an empty commit that'll trigger the tests.

GCP workloads (requires changing the namespace to 802)
Grafana network dashboard for devnet-issue-802

bryanchriswhite

Preemptively approving. 👍

Just a few more suggestions regarding error messages; otherwise, LGTM. 🚢

x/session/types/query_get_session_request.go

x/session/types/session_header.go

x/shared/helpers/service_configs.go

x/shared/types/service.go

Olshansk · 2024-09-11T02:51:34Z

Let's merge this in!

…ke-transfer * pokt/main: [Tooling, Code Health] refactor: split Makefile into multiple files (#816) [Tooling] fix and add makefile targets (#814) [Docs] Add a debugging tip: using transaction hashes (#786) [Refactor] Replace embedded Service with ServiceId (#802) [Session,Service] Ensure SessionHeader and Service basic validation. (#782) [Tokenomics] Preparation for Global Mint Reimbursement Request (#755) [Quick PR][LocalNet] Turn off minimum-gas-prices (#805) [GATEWAY POC] A couple helpers to help deploy a PATH gateway (#801) [Documentation] Cosmovisor (#768) [CI] Add cosmovisor to container images (#796) Use a descriptive error if service ID is not found (#787) [SessionManager] Implement off-chain proof params usage (#765) [SDK] Update shannon sdk dependency (#800) [E2E Test] Remove redundant logging (#798)

…actor/transfer-msg_period-param * issues/657/feat/app-stake-transfer: [Tooling, Code Health] refactor: split Makefile into multiple files (#816) [Tooling] fix and add makefile targets (#814) [Docs] Add a debugging tip: using transaction hashes (#786) [Refactor] Replace embedded Service with ServiceId (#802) [Session,Service] Ensure SessionHeader and Service basic validation. (#782) [Tokenomics] Preparation for Global Mint Reimbursement Request (#755) [Quick PR][LocalNet] Turn off minimum-gas-prices (#805) [GATEWAY POC] A couple helpers to help deploy a PATH gateway (#801) [Documentation] Cosmovisor (#768) [CI] Add cosmovisor to container images (#796) Use a descriptive error if service ID is not found (#787) [SessionManager] Implement off-chain proof params usage (#765) [SDK] Update shannon sdk dependency (#800) [E2E Test] Remove redundant logging (#798)

…ues/657/chore/app-transfer-period * issues/657/refactor/transfer-msg_period-param: [Tooling, Code Health] refactor: split Makefile into multiple files (#816) [Tooling] fix and add makefile targets (#814) [Docs] Add a debugging tip: using transaction hashes (#786) [Refactor] Replace embedded Service with ServiceId (#802) [Session,Service] Ensure SessionHeader and Service basic validation. (#782) [Tokenomics] Preparation for Global Mint Reimbursement Request (#755) [Quick PR][LocalNet] Turn off minimum-gas-prices (#805) [GATEWAY POC] A couple helpers to help deploy a PATH gateway (#801) [Documentation] Cosmovisor (#768) [CI] Add cosmovisor to container images (#796) Use a descriptive error if service ID is not found (#787) [SessionManager] Implement off-chain proof params usage (#765) [SDK] Update shannon sdk dependency (#800) [E2E Test] Remove redundant logging (#798)

## Summary This PR replaces all embedded `Service` entries with `ServiceId`. Most of the changes are `Service` -> `ServiceId` replacements but special attention has to be put on: * x/session/types/session_header.go * x/shared/helpers/service_configs.go * x/shared/types/service.go * x/service/types/message_add_service.go * proto/poktroll/shared/service.proto * proto/poktroll/session/types.proto * proto/poktroll/session/query.proto A follow-up to update `Shannon-SDK` is expected after approval. E2E tests pass locally. ## Issue ![image](https://github.com/user-attachments/assets/c1feb50b-89c1-4b4f-b4cb-94cfb4ec5ade) ## Type of change Select one or more from the following: - [ ] New feature, functionality or library - [ ] Consensus breaking; add the `consensus-breaking` label if so. See #791 for details - [ ] Bug fix - [x] Code health or cleanup - [ ] Documentation - [ ] Other (specify) ## Testing - [ ] **Documentation**: `make docusaurus_start`; only needed if you make doc changes - [x] **Unit Tests**: `make go_develop_and_test` - [x] **LocalNet E2E Tests**: `make test_e2e` - [ ] **DevNet E2E Tests**: Add the `devnet-test-e2e` label to the PR. ## Sanity Checklist - [x] I have tested my changes using the available tooling - [ ] I have commented my code - [x] I have performed a self-review of my own code; both comments & source code - [ ] I create and reference any new tickets, if applicable - [ ] I have left TODOs throughout the codebase, if applicable --------- Co-authored-by: Daniel Olshansky <[email protected]>

red-0ne and others added 12 commits September 3, 2024 09:47

chore: Session header and service basic validation

3cba149

fix: Adapt test after refactor

124b660

chore: Fix godoc comment

2994415

Merge branch 'main' into chore/session-service-basic-validation

8028d01

WIP: ValidteBasic

9bc4442

refactor: Service to ID

53e953f

chore: Update go.mode

5e10031

Merge remote-tracking branch 'origin/main' into HEAD

1fbc638

fix: Revert service validate basic

281edd5

Merge remote-tracking branch 'origin/chore/session-service-basic-vali…

a6c6643

…dation' into HEAD

Merge branch 'chore/session-service-basic-validation' into refactor/s…

853407e

…ervice-to-id

fix: Tests failing due to merge

982af1e

red-0ne added service Anything related to general-purpose RPC service support code health Cleans up some code labels Sep 7, 2024

red-0ne requested review from bryanchriswhite and Olshansk September 7, 2024 03:10

red-0ne self-assigned this Sep 7, 2024

Olshansk added this to the Shannon Beta TestNet Launch milestone Sep 9, 2024

Olshansk approved these changes Sep 9, 2024

View reviewed changes

x/shared/types/service.go Show resolved Hide resolved

bryanchriswhite reviewed Sep 10, 2024

View reviewed changes

bryanchriswhite requested changes Sep 10, 2024

View reviewed changes

x/proof/types/message_submit_proof_test.go Outdated Show resolved Hide resolved

x/session/keeper/session_hydrator.go Show resolved Hide resolved

x/session/types/errors.go Outdated Show resolved Hide resolved

x/shared/types/service.go Outdated Show resolved Hide resolved

Merge remote-tracking branch 'origin/main' into refactor/service-to-id

293af98

red-0ne added the devnet-test-e2e label Sep 10, 2024

github-actions bot added devnet push-image CI related - pushes images to ghcr.io labels Sep 10, 2024

red-0ne added 2 commits September 10, 2024 15:00

chore: Address review change requests

e2baa0a

Empty commit

265bc78

red-0ne changed the base branch from chore/session-service-basic-validation to main September 10, 2024 13:06

red-0ne requested a review from bryanchriswhite September 10, 2024 13:09

bryanchriswhite approved these changes Sep 10, 2024

View reviewed changes

chore: Address review change requests

f58d681

red-0ne added the consensus-breaking IMPORTANT! If the PR with this tag is merged, next release WILL HAVE TO BE an upgrade. label Sep 11, 2024

red-0ne merged commit 88a9197 into main Sep 11, 2024
11 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Refactor] Replace embedded Service with ServiceId #802

[Refactor] Replace embedded Service with ServiceId #802

red-0ne commented Sep 7, 2024 •

edited

Loading

Olshansk left a comment

bryanchriswhite left a comment •

edited

Loading

github-actions bot commented Sep 10, 2024

bryanchriswhite left a comment

Olshansk commented Sep 11, 2024

[Refactor] Replace embedded Service with ServiceId #802

[Refactor] Replace embedded Service with ServiceId #802

Conversation

red-0ne commented Sep 7, 2024 • edited Loading

Summary

Issue

Type of change

Testing

Sanity Checklist

Olshansk left a comment

Choose a reason for hiding this comment

bryanchriswhite left a comment • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Sep 10, 2024

bryanchriswhite left a comment

Choose a reason for hiding this comment

Olshansk commented Sep 11, 2024

red-0ne commented Sep 7, 2024 •

edited

Loading

bryanchriswhite left a comment •

edited

Loading