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

feat(providers): proxy request for a certain provider #487

Merged
merged 4 commits into from
Feb 1, 2024

Conversation

geekbrother
Copy link
Contributor

@geekbrother geekbrother commented Jan 23, 2024

Description

This PR adds an ability to request a certain provider in the proxy handler.
At the moment when we are querying the proxy for the certain chain_id the weighted provider is used and we can't test the request to the certain provider.

Our integration tests uses the proxy requests and when we are requesting the certain chain_id the server requests a weighted provider and not the provider in tests itself. This makes tests flaky and unstable as we can't make sure what provider exactly was requested for test.

This PR making the following changes:

  • Adds an optional providerId request parameter for the proxy handler.
  • Adds get_provider_by_provider_id helper function to get the exact provider.
  • Making changes to all provider's integration tests to use the certain provider instead of weighted.
  • Allow exact provider requests only for the allowed project ID in RPC_PROXY_TESTING_PROJECT_ID.

ProviderId should be the same as in the ProviderKind.

How Has This Been Tested?

Manual:

  • Start the server locally by RPC_PROXY_TESTING_PROJECT_ID=projectXXX cargo run
  • Run the curl query with the additional providerId parameter:
curl -v 'http://localhost:3000/v1/?chainId=eip155:999&projectId=projectXXX&providerId=Zora' --data '{"jsonrpc":"2.0","method":"eth_chainId","params":[],"id":1}'

The expected result is the server log where the Zora exact provider was used and successful response:

{"jsonrpc":"2.0","result":"0x3e7","id":1}

CI/CD:

Integration tests are included in this PR.

Merging requirements 🚧

  • Sync GitHub Actions PROJECT_ID with the Terraform Cloud testing_project_id to allow CI/CD tests suite run the exact provider requests on staging and prod.

Due Diligence

  • Breaking change
  • Requires a documentation update
  • Requires a e2e/integration test update

@geekbrother geekbrother self-assigned this Jan 23, 2024
@@ -302,6 +309,7 @@ impl Display for ProviderKind {
}
}

#[allow(clippy::should_implement_trait)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cippy started to warn on this trait implementation, adding a temporary allow to fix this later.

@geekbrother geekbrother force-pushed the feat/provider_id_for_tests branch from 028b61a to d9f7377 Compare January 23, 2024 10:36
@geekbrother geekbrother marked this pull request as ready for review January 23, 2024 10:37
@geekbrother geekbrother requested a review from xav as a code owner January 23, 2024 10:37
Copy link
Contributor

@arein arein left a comment

Choose a reason for hiding this comment

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

Can you enable this in staging only?

@geekbrother
Copy link
Contributor Author

Can you enable this in staging only?

I've updated it and now this request is only allowed by a certain project ID to allow only our testing suite to run it or manually when knowing the project ID that's allowed by the RPC_PROXY_TESTING_PROJECT_ID configuration variable.
In this case, we can test it in both staging, prod, and manually if needed.

@geekbrother geekbrother marked this pull request as ready for review January 29, 2024 15:27
@geekbrother geekbrother requested a review from arein January 29, 2024 15:27
src/handlers/proxy.rs Outdated Show resolved Hide resolved
Copy link
Member

@chris13524 chris13524 left a comment

Choose a reason for hiding this comment

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

LGTM

@geekbrother
Copy link
Contributor Author

An integration test is added to test the exact provider request with allowed/not allowed projectId.

@geekbrother geekbrother merged commit fcbb608 into master Feb 1, 2024
16 checks passed
@geekbrother geekbrother deleted the feat/provider_id_for_tests branch February 1, 2024 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants