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

[Session] Add support for GetSession via the CLI + deps #123

Merged
merged 37 commits into from
Nov 8, 2023
Merged

[Session] Add support for GetSession via the CLI + deps #123

merged 37 commits into from
Nov 8, 2023

Conversation

Olshansk
Copy link
Member

@Olshansk Olshansk commented Nov 1, 2023 •
edited by reviewpad bot
Loading

Summary

Human Summary

  • Update the CLI code flow to be able to call GetSession
  • Add make targets to trigger GetSession
  • Replace [] with <> in all CLI functions to differentiate required vs optional params
  • Made a couple changes to SessionHydrator (and added tests) to account additional error cases
Screen.Recording.2023-11-01.at.4.49.33.PM.mov

AI Summary

Summary generated by Reviewpad on 08 Nov 23 19:40 UTC

This pull request modifies various files in the codebase. Here is a summary of the changes:

  1. The command used for unstaking an application has been updated and the corresponding description has been clarified.
  2. The TestGenesis function in the x/supplier/genesis_test.go file has been modified to replace the ServiceId field with the Service field in the SupplierServiceConfig struct.
  3. The MsgServerStakeApplication function in the x/application/keeper/msg_server_stake_application_test.go file has undergone changes related to the ServiceConfigs array, replacing the ServiceId field with the Service field in multiple instances.
  4. The file diff includes changes related to the x/supplier/genesis_test.go file, specifically replacing the ServiceId field with the Service field in the SupplierServiceConfig struct and updating the Id and Endpoints fields.
  5. The application.proto file has been updated with changes to the comment of the service_configs field.
  6. The service.go file includes changes such as importing a new package, adding new functions, and updating existing functions related to service validation.
  7. The errors.go file in the x/supplier/types package has new error constants related to invalid session start height and ID.
  8. The query_gateway.go file has changes to the CmdShowGateway function, including updates to the Use, Short, Args, and RunE fields.
  9. The query_gateway.go file includes changes to the CmdShowGateway function, modifying the Use, Short, Args, and RunE fields.
  10. The server_builder.go file has changes related to the BuildProvidedServices function, such as renaming the serviceId variable to service and creating a new slice called serviceEndpoints.
  11. The msg_server_undelegate_from_gateway_test.go file contains changes to multiple test cases, replacing the ServiceId field with the Service field.
  12. The services.go file includes changes such as importing a new package, adding a new function for service validation, and updating existing functions related to service validation.
  13. The go.mod file has changes related to dependencies, including additions and removals of dependencies.
  14. The tx_submit_proof.go file includes changes such as reordering import statements, renaming variables, and modifying the CmdSubmitProof function.
  15. There are changes to the session hydrator functionality, including imports, field replacements, error handling updates, and validation checks.
  16. Changes to the tx_stake_application.go file involve modifying the usage string and providing instructions for the stake application command.
  17. The service_configs.go file has had its validation code replaced by validation code for the service instead of the serviceId.
  18. The NewMsgStakeApplication function in the message_stake_application.go file has been modified to change the ServiceId field name to Service in the sharedtypes.ApplicationServiceConfig struct.

Please let me know if you would like more information on any specific part of the diff.

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
  • 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

@Olshansk Olshansk added the session Changes related to Session management label Nov 1, 2023
@Olshansk Olshansk added this to the Shannon TestNet milestone Nov 1, 2023
@Olshansk Olshansk self-assigned this Nov 1, 2023
@Olshansk Olshansk removed the request for review from bryanchriswhite November 1, 2023 05:21
@Olshansk Olshansk changed the title [WIP][Session] Retrieve the latest Session via the CLI [Session] Add support for GetSession via the CLI + deps Nov 1, 2023
@Olshansk Olshansk marked this pull request as ready for review November 1, 2023 05:32
Olshansk
Olshansk commented Nov 1, 2023
View reviewed changes
Copy link
Member Author

@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.

Completed self review

bryanchriswhite
bryanchriswhite reviewed Nov 2, 2023
View reviewed changes
Makefile Outdated Show resolved Hide resolved
Olshansk and others added 10 commits November 7, 2023 11:48
@Olshansk @bryanchriswhite
Update x/session/keeper/session_hydrator_test.go
04b9ce0 
Co-authored-by: Bryan White <[email protected]>
@Olshansk @bryanchriswhite
Update x/shared/helpers/service_test.go
dc8a37d 
Co-authored-by: Bryan White <[email protected]>
@bryanchriswhite @Olshansk
[Miner] feat: add TxClient (#94)
2c940f4 
* chore: add `TxClient` interface

* chore: add option support to `ReplayObservable`

* feat: add `txClient` implementation

* test: `txClient`

* test: tx client integration

* chore: s/tx/transaction/g

* chore: update pkg README.md template

* wip: client pkg README

* docs: fix client pkg godoc comment

* fix: flakey test

* chore: dial back godoc comments 😅

* chore: revise (and move to godoc.go) `testblock` & `testeventsquery` pkg godoc comment

* chore: update go.mod

* chore: refactor & condense godoc comments

* chore: fix import paths post-update

* chore: review feedback improvements

* docs: update client README.md

* docs: add `tx query` usage association between `txContext` & `Blockchain`

* docs: add TOC

* chore: review feedback improvements

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

* docs: improve godoc comments & client README.md

---------

Co-authored-by: Daniel Olshansky <[email protected]>
@bryanchriswhite @Olshansk
[Off-chain] refactor: keyring errors & helpers (#131)
9bf5508 
* chore: add `TxClient` interface

* chore: add option support to `ReplayObservable`

* feat: add `txClient` implementation

* test: `txClient`

* test: tx client integration

* chore: s/tx/transaction/g

* chore: update pkg README.md template

* wip: client pkg README

* docs: fix client pkg godoc comment

* refactor: consolidate keyring errors & helpers

* refactor: keyring test helpers

* fix: flakey test

* chore: dial back godoc comments 😅

* chore: revise (and move to godoc.go) `testblock` & `testeventsquery` pkg godoc comment

* chore: update go.mod

* chore: refactor & condense godoc comments

* chore: fix import paths post-update
@bryanchriswhite @Olshansk
[Miner] feat: add supplier client (#42)
6ebb23b 
* chore: add `TxClient` interface

* chore: add option support to `ReplayObservable`

* feat: add `txClient` implementation

* test: `txClient`

* test: tx client integration

* chore: s/tx/transaction/g

* chore: update pkg README.md template

* wip: client pkg README

* docs: fix client pkg godoc comment

* refactor: consolidate keyring errors & helpers

* refactor: keyring test helpers

* fix: flakey test

* chore: dial back godoc comments 😅

* chore: add `SupplierClient` interface

* feat: add supplier client implementation

* test:  supplier test helpers

* test: supplier client tests

* test: supplier client integration test

* chore: update go.mod

* trigger CI

* chore: revise (and move to godoc.go) `testblock` & `testeventsquery` pkg godoc comment

* chore: update go.mod

* chore: refactor & condense godoc comments

* chore: fix import paths post-update

* chore: add godoc comment
@Olshansk
Reply to some review comments
e24489a
@Olshansk
Rename session errors
b312089
@Olshansk
Add back miss &&
bc92251
@Olshansk
Fixed a few failing tests
f4c37da
@Olshansk
Merge with issues/140/rename_serviceid_to_service
9245065
@Olshansk Olshansk changed the base branch from main to issues/140/rename_serviceid_to_service November 7, 2023 22:13
@bryanchriswhite bryanchriswhite mentioned this pull request Nov 8, 2023
Merged
10 tasks
bryanchriswhite
bryanchriswhite reviewed Nov 8, 2023
View reviewed changes
x/session/keeper/session_hydrator_test.go
},
{
desc: "blockHeight > contextHeight",
blockHeight: 9001, // block height over 9000 is too height given that the context height is 100
Copy link
Contributor

Choose a reason for hiding this comment

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

wat 9000?

Copy link
Member Author

Choose a reason for hiding this comment

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

https://twitter.com/olshansky/status/1722339951466217476

bryanchriswhite
bryanchriswhite approved these changes Nov 8, 2023
View reviewed changes
Copy link
Contributor

@bryanchriswhite bryanchriswhite left a comment

Choose a reason for hiding this comment

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

LFG

Let's gooooooo! 🚀

Olshansk and others added 2 commits November 8, 2023 11:25
@Olshansk @bryanchriswhite @okdas
[Service] Rename the ServiceId proto to Service (#150)
823262a 
Renamed the `ServiceId` proto to `Service` so `ServiceId.Id` is more semantic and less confusing.
---

Co-authored-by: Bryan White <[email protected]>
Co-authored-by: Daniel Olshansky <[email protected]>
Co-authored-by: Dima Kniazev <[email protected]>
@Olshansk
Merge with main
a70bbbb
@Olshansk Olshansk mentioned this pull request Nov 8, 2023
Merged
10 tasks
@Olshansk Olshansk force-pushed the issues/15/implement_get_session_cli branch from 3f32a15 to dd17f66 Compare November 8, 2023 19:40
@Olshansk Olshansk merged this pull request into issues/140/rename_serviceid_to_service Nov 8, 2023
3 checks passed
Olshansk added a commit that referenced this pull request Nov 8, 2023
@Olshansk
[Session] Add support for GetSession via the CLI + deps (#123)
0a005b0 
    - Update the CLI code flow to be able to call `GetSession`
    - Add make targets to trigger `GetSession`
    - Replace `[]` with `<>` in all CLI functions to differentiate required vs optional params
    - Made a couple changes to `SessionHydrator` (and added tests) to account additional error cases

    https://github.com/pokt-network/poktroll/assets/1892194/56425906-d06f-4c41-b8a5-d210d8a450cd

    ---

    Co-authored-by: Bryan White <[email protected]>
    Co-authored-by: Daniel Olshansky <[email protected]>
    Co-authored-by: Dima Kniazev <[email protected]>
@Olshansk
Copy link
Member Author

Olshansk commented Nov 8, 2023

I accidently merged this into the wrong branch so attempt #2 it in #163.

@Olshansk Olshansk mentioned this pull request Nov 8, 2023
Merged
10 tasks
@Olshansk Olshansk deleted the issues/15/implement_get_session_cli branch November 8, 2023 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@bryanchriswhite bryanchriswhite bryanchriswhite approved these changes

@h5law h5law Awaiting requested review from h5law

Assignees

@Olshansk Olshansk

Labels
session Changes related to Session management
Projects
Shannon
Status: ✅ Done
Milestone
Shannon Private TestNet
Development

Successfully merging this pull request may close these issues.
3 participants
@Olshansk @bryanchriswhite @h5law