Skip to content
This repository has been archived by the owner on Feb 2, 2024. It is now read-only.

chore: fix issue with rpc provider #642

Closed
wants to merge 3 commits into from
Closed

Conversation

anxolin
Copy link
Contributor

@anxolin anxolin commented Oct 9, 2023

Summary

This PR fixes 2 issues:

  • Make sure our infura key is used with the InfuraProvider. Before it was using the config file even if you override with an env
  • For playwrite tests, we shouldn't use the rpcUrl (because that points to mainnet and not rinkeby). Leaving infura as we currently depend on having the ID setup (we have an infura provider in some other part of the app)
  • Pass the ETH_NODE_URL to the app (in webpack)
  • Pass ETH_NODE_URL and INFURA_ID during vercel build
  • Replaces the public infura key with one we control, and has limitations on number of requests and rate limit: see chore: replace rpc to rate-limited one cowswap#3187 (companion PR)

To Test

Hopefully now we use NodeReal for mainnet RPC, and we use the right infura key for the infura provider.

@vercel
Copy link

vercel bot commented Oct 9, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
explorer-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 10, 2023 9:01am

@anxolin
Copy link
Contributor Author

anxolin commented Oct 9, 2023

@alfetopito Whyyyyyyy?

I added this two:

INFURA_ID: ${{ secrets.INFURA_ID }}

I added the secrets in Github and Vercel env (for dev).

I tested locally (it works now thanks to the webpack config).

It doesn't work in Vercel! (shows the infura key of the config file, and it doesn't pick the envs ETH_NODE_URL or INFURA_ID

What am i missing?

@anxolin anxolin marked this pull request as draft October 9, 2023 18:12
Copy link
Collaborator

@alfetopito alfetopito left a comment

Choose a reason for hiding this comment

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

Don't know what's wrong, will try it out locally and play with Vercel to try to figure it out

Comment on lines +43 to +44
ETH_NODE_URL: ${{ secrets.ETH_NODE_URL }}
INFURA_ID: ${{ secrets.INFURA_ID }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

FYI, this is only used in builds executed on GH Actions workflows, which are NOT the PR preview builds.

Copy link
Collaborator

@alfetopito alfetopito left a comment

Choose a reason for hiding this comment

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

After Vercel PREVIEW env var update, this PR is working.

There are some caveats I left as comments.

As for running locally, it no longer works when no env vars are set, which means the dev defaults are not being used.

Comment on lines +22 to 23
ETH_NODE_URL: process.env.ETH_NODE_URL,
INFURA_ID: process.env.INFURA_ID,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Keep in mind that:

  1. INFURA_ID takes precedence. If this is set, ETH_NODE_URL won't be used at all, AFAICT
  2. If ETH_NODE_URL is chain specific, other chains might not work (I have not tested this but I assume that's what will happen)

@anxolin anxolin closed this Jan 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants