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

[Proxy] chore: Use depinject for relayerProxy #173

Merged
merged 4 commits into from
Nov 10, 2023
Merged

[Proxy] chore: Use depinject for relayerProxy #173

merged 4 commits into from
Nov 10, 2023

Conversation

red-0ne
Copy link
Contributor

@red-0ne red-0ne commented Nov 9, 2023 •
edited by reviewpad bot
Loading

Summary

Human Summary

Use depinject and options functions to construct the RelayerProxy

AI Summary

Summary generated by Reviewpad on 09 Nov 23 23:53 UTC

This pull request introduces changes to the relayer package.

The diff includes the following modifications:

  • Added a new RelayerProxyOption type to configure the relayer proxy.
  • Changed the error codespace from "relayer/proxy" to "relayer_proxy".
  • Renamed error variables according to the new error codespace.
  • Added a new file options.go that defines options for the relayer proxy.
  • Added a NewRelayerProxy function to create a new relayer proxy.
  • Refactored the relayerProxy struct to use the new options and validate the configuration.
  • Modified the SignRelayResponse and VerifyRelayRequest methods to use the signing key name.
  • Updated the error messages in the VerifyRelayRequest method.
  • Added a function to validate the relayer proxy's configuration options.
  • Modified the BuildProvidedServices method to use the signing key name.

Please review these changes and provide feedback.

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

@red-0ne red-0ne added proxy Changes related to the Proxy off-chain Off-chain business logic labels Nov 9, 2023
@red-0ne red-0ne added this to the Shannon TestNet milestone Nov 9, 2023
@red-0ne red-0ne self-assigned this Nov 9, 2023
@red-0ne red-0ne changed the title [Proxy] Chore/proxy depinject [Proxy] chore: Use depinject for relayerProxy Nov 9, 2023
Olshansk
Olshansk requested changes Nov 9, 2023
View reviewed changes
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.

Leaving a single comment for now

pkg/relayer/proxy/errors.go Outdated Show resolved Hide resolved
pkg/relayer/proxy/proxy.go Show resolved Hide resolved
bryanchriswhite
bryanchriswhite approved these changes Nov 9, 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.

Had a couple of questions regarding conventions but otherwise LGTM! 🙌

Preemptively approving.

@red-0ne red-0ne merged commit e49434e into main Nov 10, 2023
bryanchriswhite added a commit that referenced this pull request Nov 10, 2023
@bryanchriswhite
Merge remote-tracking branch 'pokt/main' into issues/13/feat/miner
286e1b2 
* pokt/main:
  [Proxy] chore: Use depinject for relayerProxy (#173)
  [Sessions] chore: Use depinject for sessions mgr construction (#175)
okdas pushed a commit that referenced this pull request Nov 14, 2024
@red-0ne
[Proxy] chore: Use depinject for relayerProxy (#173)
22aa52a 
* chore: Use depinject for relayerProxy

* chore: rename option validation function

* chore: Add godoc comments

* chore: Improve errors readability
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Olshansk Olshansk Olshansk approved these changes

@bryanchriswhite bryanchriswhite Awaiting requested review from bryanchriswhite

Assignees

@red-0ne red-0ne

Labels
off-chain Off-chain business logic proxy Changes related to the Proxy
Projects
Shannon
Status: ✅ Done
Milestone
Shannon Private TestNet
Development

Successfully merging this pull request may close these issues.
3 participants
@red-0ne @bryanchriswhite @Olshansk