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

[Tooling] RPC specification parity #628

Closed
10 tasks
jessicadaugherty opened this issue Mar 28, 2023 · 11 comments · Fixed by #684
Closed
10 tasks

[Tooling] RPC specification parity #628

jessicadaugherty opened this issue Mar 28, 2023 · 11 comments · Fixed by #684
Assignees
Labels
community Open to or owned by a non-core team member tooling tooling to support development, testing et al

Comments

@jessicadaugherty
Copy link
Contributor

jessicadaugherty commented Mar 28, 2023

Objective

Develop and implement an RPC specification for Pocket Network's v1 protocol with feature parity to the V0 utility RPC specification, ensuring compatibility and seamless integration with the existing system.

Origin Document

The V0 RPC specification can be found in this yaml file:

V0 utility RPC specification

The V1 utility specification can be found here:

V1 Utility Specification

Initial work has been done on the RPC specification when building the CLI MVP and can be reviewed in these PRs:

CLI and RPC MVP pt 1
CLI and RPC MVP pt 2

Goals

  • Achieve parity with V0 RPC specification
  • Identify and implement missing endpoints and requirements for the V1 utility RPC specification

Deliverable

  • Review and analyze the V0 utility RPC specification, identifying key components, functionality, and important endpoints, methods, and data structures.
  • Create endpoints for V1 utility logic in the RPC specification and document
  • Create placeholder endpoints for functionality that is not yet ready in utility, but exists in v0, and will be needed in v1 with a TODO comment

Non-goals / Non-deliverables

  • Implementing new features or enhancements that are not part of the V0 utility RPC specification or unrelated to the M3 (relay) requirements

General issue deliverables

  • Update the appropriate CHANGELOG
  • Update any relevant READMEs (local and/or global)
  • Update any relevant global documentation & references
  • If applicable, update the source code tree explanation
  • If applicable, add or update a state, sequence or flowchart diagram using mermaid

Testing Methodology

  • All tests: make test_all
  • LocalNet: verify a LocalNet is still functioning correctly by following the instructions at docs/development/README.md

Creator: @jessicadaugherty

@jessicadaugherty jessicadaugherty converted this from a draft issue Mar 28, 2023
@jessicadaugherty jessicadaugherty added tooling tooling to support development, testing et al triage It requires some decision-making at team level (it can't be worked on as it stands) labels Mar 28, 2023
@jessicadaugherty jessicadaugherty moved this to Backlog in V1 Dashboard Mar 28, 2023
@jessicadaugherty
Copy link
Contributor Author

@Olshansk ready for review

@jessicadaugherty jessicadaugherty changed the title [WIP][Tooling] RPC Specification [WIP][Tooling] RPC specification parity Mar 31, 2023
@h5law
Copy link
Contributor

h5law commented Apr 4, 2023

@jessicadaugherty Is this link up to date with the V0 spec? https://docs.pokt.network/api-docs/pokt/#

I will start getting the V1 spec up to date but its nice to have the readable swagger UI. I will document any areas (if I find any) that cannot be done just yet, when (of if) i get to them

@jessicadaugherty
Copy link
Contributor Author

Hey @h5law! Those may be behind. I'd use this for the most up to date spec https://github.com/pokt-network/pocket-core/blob/staging/doc/specs/rpc-spec.yaml

@Olshansk
Copy link
Member

Olshansk commented Apr 6, 2023

@h5law
Copy link
Contributor

h5law commented Apr 6, 2023

@Olshansk @jessicadaugherty so this PR includes the CLI endpoints as well 👍🏼 was just gunna ask, am getting through the list now

@jessicadaugherty jessicadaugherty changed the title [WIP][Tooling] RPC specification parity [Tooling] RPC specification parity Apr 10, 2023
@jessicadaugherty jessicadaugherty added community Open to or owned by a non-core team member and removed triage It requires some decision-making at team level (it can't be worked on as it stands) labels Apr 10, 2023
@jessicadaugherty
Copy link
Contributor Author

Thanks for picking up this ticket @h5law! Would you mind providing an effort size so I can $$it up? CC @Olshansk

@h5law
Copy link
Contributor

h5law commented Apr 14, 2023

Thanks for picking up this ticket @h5law! Would you mind providing an effort size so I can $$it up? CC @Olshansk

Currently it looks like a L/XL. I have split out 2 PRs from the work I have already done for it: the TxResult and TxIndexer one. Both are S.

Will work on the RPC savepoints afterwards don't want to make this PR too large ahah

@jessicadaugherty
Copy link
Contributor Author

don't want to make this PR too large

we love to see it

it looks like a L/XL. I have split out 2 PRs from the work I have already done for it: the TxResult and TxIndexer one. Both are S.

thanks for the estimates. will break out my bounty abacus. and, as always, let us know if you encounter any issues or have questions along the way!

@Olshansk
Copy link
Member

Appreciate the update, and totally agree that the scope of this is growing to an XL but +100 to the small PRs.

@h5law
Copy link
Contributor

h5law commented Apr 14, 2023

@Olshansk So currently the RPC spec and CLI endpoints are these - the only faked one is v1/query/upgrade couldnt find anything that did this so I make a placeholder function in the postgres context (potentially a new param pocket_version this when changed would make it easy to query the height of any protocol upgrades but thats not important right now)

Screenshot from 2023-04-14 21-18-08

Screenshot from 2023-04-14 21-18-45

The remaining v1/query endpoints are as follows:
v1/query/nodeclaim
v1/query/nodeclaims
v1/query/signinginfo
v1/query/node
v1/query/nodes
v1/query/supply
v1/query/supportedchains
v1/query/state

I dont believe any of these are currently able to be implemented without significant code changes. So the plan was to create functions in the UtilityContext/PostgresContext (Im thinking utility at the moment) that do nothing but let me write the RPC and CLI endpoints.

This is the same for the following:
v1/client/dispatch
v1/client/relay
v1/client/sim
v1/client/rawtx
v1/client/challenge

Again for these I was thinking utility interface no-op functions

However for the following I really don't know where the faked functions should go?

v1/private/stop
v1/private/chains
v1/private/updatechains
v1/private/nodes

Any feedback is appreciated, if any of these could actually be implemented currently let me know - but I am not sure they can be. 👍

@Olshansk
Copy link
Member

v1/query/nodeclaim

Currently ill-defined in v1, so let's skip it for now.

v1/query/nodeclaims

Currently ill-defined in v1, so let's skip it for now.

v1/query/signinginfo

Can be replaced by /v1/query/validators below.

v1/query/node

Look at shared/modules/persistence_module.go, I think we get leverage GetActor and have endpoints for

/v1/query/fisherman
/v1/query/validator
/v1/query/servicer
/v1/query/application

It can accept the height and address in the data fields

v1/query/nodes

Look at shared/modules/persistence_module.go, I think we get leverage GetAllValidators, GetAllServicers, etc... and have endpoints for:

/v1/query/fishermen
/v1/query/validators
/v1/query/servicers
/v1/query/applications

It can accept the height and pagination in the data fields

v1/query/supply

We can leverage the types and balanced in the pools.

	GetPoolAmount(address []byte, height int64) (amount string, err error)
	GetAllPools(height int64) ([]*coreTypes.Account, error)

v1/query/supportedchains

Can you create a no-op endpoint for this?

v1/query/state

This endpoint is trying to do too much. I don't think we nee dit.

v1/client/dispatch

I'm working on the backend for this RIGHT NOW. Can you create a no-op for it?

v1/client/relay

I'm working on the backend for this RIGHT NOW. Can you create a no-op for it?

v1/client/sim

Don't think we need this.

v1/client/rawtx

We already do this in /v1/client/broadcast_tx_sync

v1/client/challenge

Can you create a no-op for it and add links to this in the code comments:

v1/private/stop

Don't think we need this.

v1/private/chains

Don't think we need this.

v1/private/updatechains

Don't think we need this.

v1/private/nodes

Don't think we need this.

@jessicadaugherty jessicadaugherty moved this from Backlog to In Review in V1 Dashboard Apr 17, 2023
@h5law h5law closed this as completed in #684 May 4, 2023
h5law added a commit that referenced this issue May 4, 2023
## Description

This PR aims to build out the RPC specification to have parity (not
backwards compatability) with the V0 RPC specification.

The OpenAPI YAML spec can be viewed
[here](https://editor.swagger.io/?url=https://raw.githubusercontent.com/pokt-network/pocket/rpc-spec-parity/rpc/v1/openapi.yaml).
This compares to the V0 spec which can be viewed
[here](https://editor.swagger.io/?url=https://raw.githubusercontent.com/pokt-network/pocket-core/staging/doc/specs/rpc-spec.yaml)

The CLI endpoints for the `v1/query` endpoints have also been
implemented


![image](https://user-images.githubusercontent.com/53987565/232327322-953d2f0a-e65f-4a60-bc63-2a88b88b4f75.png)

Not only does this PR build out the specification but it also implements
some new functionalities, laid out in the changes section below. Some
have already been covered in the PRs #676 and #677

## Issue

Fixes #628 

## Type of change

Please mark the relevant option(s):

- [x] New feature, functionality or library
- [ ] Bug fix
- [x] Code health or cleanup
- [ ] Major breaking change
- [x] Documentation
- [ ] Other

## List of changes

- Add `networkId` and `timestamp` fields to the block
- Ability to query the mempool for transactions yet to be commited to a
block
- Move the `utility/types/errors.go` file to
`shared/core/types/errors.go` to be used throughout the codebase
- Expose NO-OP functions for future implementation
- Add Protobuf files for Sessions, Relays and Challenges for future
implementations
- Build out a functional RPC server and specification with CLI endpoints

## Testing

- [x] `make develop_test`; if any code changes were made
- [x] [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
- [x] 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
- [x] 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: Daniel Olshansky <[email protected]>
Co-authored-by: Daniel Olshansky <[email protected]>
@github-project-automation github-project-automation bot moved this from In Review to Done in V1 Dashboard May 4, 2023
bryanchriswhite added a commit that referenced this issue May 8, 2023
* pokt/main:
  [RPC][CLI] RPC Specification Parity (Issue #628) (#684)
  [Persistence] Index Transactions by Transaction Hash (#721)
  [Docs] Add missing README.md links (#722)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Open to or owned by a non-core team member tooling tooling to support development, testing et al
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants