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

[DOCUMENT] Interfaces for the M3 Relay Protocol (issue-#265) #306

Merged
merged 6 commits into from
Nov 3, 2022

Conversation

andrewnguyen22
Copy link
Contributor

Description

Code / document interfaces for the Relay Protocol to enable consensus and understanding around this piece of the M3 milestone.

M3 Milestone

This is the first step into beginning the Relay Protocol development.

  • Clear interface(s) that show the lifecycle of the relay protocol
  • Clear documentation supporting the above

Issue

Fixes #265

Type of change

Please mark the relevant option(s):

  • New feature, functionality or library
  • Bug fix
  • Code health or cleanup
  • Major breaking change
  • Documentation
  • Other

List of changes

  • Added Relay Protocol interfaces and diagrams

Testing

  • make develop_test
  • LocalNet w/ all of the steps outlined in the README

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

@andrewnguyen22 andrewnguyen22 added the documentation Improvements or additions to documentation label Oct 17, 2022
@andrewnguyen22 andrewnguyen22 requested a review from a team as a code owner October 17, 2022 15:34
@andrewnguyen22 andrewnguyen22 self-assigned this Oct 17, 2022
Copy link
Member

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

I haven't reviewed utility/service/service.go yet but left some comments on utility/service/README.md and figured I'd submit a first review.

Most NITS, a couple questions, and just one idea I wanted to get your thoughts on.

utility/service/README.md Outdated Show resolved Hide resolved
utility/service/README.md Outdated Show resolved Hide resolved
utility/service/README.md Outdated Show resolved Hide resolved
utility/service/README.md Outdated Show resolved Hide resolved
utility/service/README.md Outdated Show resolved Hide resolved
utility/service/README.md Outdated Show resolved Hide resolved
utility/service/README.md Outdated Show resolved Hide resolved
utility/service/README.md Outdated Show resolved Hide resolved
utility/service/README.md Outdated Show resolved Hide resolved
utility/service/README.md Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (main@bcadafd). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #306   +/-   ##
=======================================
  Coverage        ?   40.06%           
=======================================
  Files           ?       28           
  Lines           ?     2184           
  Branches        ?        0           
=======================================
  Hits            ?      875           
  Misses          ?     1255           
  Partials        ?       54           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

utility/service/PROTOCOLS.md Outdated Show resolved Hide resolved
utility/service/PROTOCOLS.md Outdated Show resolved Hide resolved
utility/service/PROTOCOLS.md Outdated Show resolved Hide resolved

1) Validate payload, look for empty or 'bad' request data
2) Validate the metadata, look for empty or 'bad' metadata
3) Ensure the `RelayChain` is supported locally (in the service node's configuration files)
Copy link
Member

Choose a reason for hiding this comment

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

Should this be from config files from the on-chain state?

Asking because we store the supported RelayChains on chain

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's both technically, but definitely locally is a validation point

utility/service/PROTOCOLS.md Outdated Show resolved Hide resolved
utility/service/PROTOCOLS.md Outdated Show resolved Hide resolved
utility/service/PROTOCOLS.md Outdated Show resolved Hide resolved
utility/service/service.go Outdated Show resolved Hide resolved
utility/service/PROTOCOLS.md Outdated Show resolved Hide resolved
utility/service/service.go Outdated Show resolved Hide resolved
Andrew Nguyen and others added 2 commits November 3, 2022 11:43
Co-authored-by: Daniel Olshansky <olshansky@pokt.network>
utility/service/service.go Outdated Show resolved Hide resolved
@Olshansk
Copy link
Member

Olshansk commented Nov 3, 2022

Approved with one minor remaining NIT.

Co-authored-by: Daniel Olshansky <olshansky@pokt.network>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[DOCUMENT] Interfaces for the M3 Relay Protocol
3 participants