-
Notifications
You must be signed in to change notification settings - Fork 89
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
chore: replace rpc to rate-limited one #3187
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
REACT_APP_INFURA_KEY=586e7e6b7c7e437aa41f5da496a749f5 | ||
REACT_APP_INFURA_KEY=2af29cd5ac554ae3b8d991afe1ba4b7d |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing that I'd like to raise is that, REACT_APP_INFURA_KEY
is required and the app will fail to start when it's not set:
But it's not used at all if the other keys are set, which they are by default:
cowswap/libs/common-const/src/networks.ts
Lines 17 to 19 in 0bf6c4e
[SupportedChainId.MAINNET]: REACT_APP_NETWORK_URL_1 || `https://mainnet.infura.io/v3/${INFURA_KEY}`, | |
[SupportedChainId.GNOSIS_CHAIN]: REACT_APP_NETWORK_URL_100 || `https://rpc.gnosis.gateway.fm`, | |
[SupportedChainId.GOERLI]: REACT_APP_NETWORK_URL_5 || `https://goerli.infura.io/v3/${INFURA_KEY}`, |
So I suggest to:
- Make it a requirement only if one of the other env vars are missing
- Remove it from
.env
files as it's redundant, and leads to confusion
I mention that because locally I changed only REACT_APP_INFURA_KEY
in my .env.local
and couldn't figure out why it was not being picked up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great suggestion.
This PR is about is about replacing the KEY only.
I will iterate in another PR for this
55b207a
to
34d252e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alfetopito merging this, as it works out of the box with all my .env.local
commented out. I will reiterate with your comments
Hopefully it makes it easier to someone to fork and run quickly without worrying about infura.
The key is rate-limited and we can keep an eye if people abuse it, but i think is great to offer this
REACT_APP_INFURA_KEY=586e7e6b7c7e437aa41f5da496a749f5 | ||
REACT_APP_INFURA_KEY=2af29cd5ac554ae3b8d991afe1ba4b7d |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great suggestion.
This PR is about is about replacing the KEY only.
I will iterate in another PR for this
Summary
Replaces an old default infura key we don't control anymore. I think is from Gnosis.
Adds our own, which has a max number of requests, and a ratelimit.
To Test
Just check its using the good one now (see infura key --> search for "public" one)