feat: Add Arbitrum, Optimism and Polygon support #46
Conversation
eliseygusev
changed the title
ape_infura/__init__.py
Outdated
| "rinkeby", | ||
| "goerli" |
There was a problem hiding this comment.
This is a bug with ape-arbitrum, but we only have support for rinkeby right now (but it's just called testnet which is the bug)
| "rinkeby", | |
| "goerli" | |
| "testnet", |
There was a problem hiding this comment.
is it defined here? https://github.com/ApeWorX/ape-arbitrum/blob/main/ape_arbitrum/ecosystem.py#L9
may be I can just fix it there? or are there other dependencies using testnet instead of rinkeby and goerli?
There was a problem hiding this comment.
ultimately, it would be best to update it there, however that is a larger breaking change because we will have to make people aware of the update in case they were using it, and also update other plugins too
There was a problem hiding this comment.
oh, I see
I think the best is to make it with "testnet" here at first.
And I guess, I would make separate requests to ape-arbitrum and ape-infura fixing the names (we can also add backward-compatilibity, should be easy to do). and leave those requests ready for announcements. does it sound good?
There was a problem hiding this comment.
exactly, make it work with arbitrum:testnet here for now, and we'll make the update when fixing ApeWorX/ape-arbitrum#3 at a later point in this plugin (and the others)
There was a problem hiding this comment.
ok, got you. I've got one more question: as of now tests for all networks are failing because ape-arbitrum, ape-optimism and ape-polygon are not installed. how do you usually proceed with this? I guess, I shouldnt add arbitrum and optimism packages to requirements, but what can be done? Or the tests for those should just be ommited
There was a problem hiding this comment.
I think it's good to add the tests, you can add installing those plugins under this extra and it will get installed during testing:
https://github.com/ApeWorX/ape-infura/blob/main/setup.py#L6
There was a problem hiding this comment.
ok, would follow
just wasnt sure whether you ok with tests being dependent on the other libraries
thanks for the help!
There was a problem hiding this comment.
should be good now. thanks a lot for reviewing!
Head branch was pushed to by a user without write access
|
flake failed because the comment was too long 😅 pushed a fix, sorry |
|
@fubuloubu do you know how to fix CI in that PR? would love it to get merged, but seems like I didnt touch anythin CI related, and the tests are failing. |
No, basically because this is your first time contributing, you don't have access to the secret necessary to run the CI |
ape_infura/providers.py
Outdated
| _ENVIRONMENT_VARIABLE_NAMES = ("WEB3_INFURA_PROJECT_ID", "WEB3_INFURA_API_KEY") | ||
| _ENVIRONMENT_VARIABLE_OPTIONS = { | ||
| "ethereum": ( | ||
| "WEB3_INFURA_PROJECT_ID", | ||
| "WEB3_INFURA_API_KEY", | ||
| ), | ||
| "arbitrum": ( | ||
| "WEB3_ARBITRUM_INFURA_PROJECT_ID", | ||
| "WEB3_ARBITRUM_INFURA_API_KEY", | ||
| ), | ||
| "optimism": ( | ||
| "WEB3_OPTIMISM_INFURA_PROJECT_ID", | ||
| "WEB3_OPTIMISM_INFURA_API_KEY", | ||
| ), | ||
| "polygon": ( | ||
| "WEB3_POLYGON_INFURA_PROJECT_ID", | ||
| "WEB3_POLYGON_INFURA_API_KEY", | ||
| ), | ||
| } |
There was a problem hiding this comment.
Just noticed this. You actually don't need this change because Infura uses just one project ID for all of it's RPC endpoints, unlike Alchemy which uses one per network
There was a problem hiding this comment.
thanks for clarifying.
fixed the issue. tests work locally for me, so shall be good in CI as well
|
@eliseygusev fix that one issue, and I'll run the CI locally and merge this (since the CI isn't going to pass right now for you) |
Head branch was pushed to by a user without write access
|
Tested locally with my api key |
What I did
Added support for Arbitrum and Optimism to Infura Provider so that it doesn't say
NetworkError: 'infura' is not a valid provider for network 'mainnet'anymorefixes: #44 and #45 and #24
How I did it
Similar to how it is done in ape-alchemy ApeWorX/ape-alchemy#17
How to verify it
I wrote test for it
Checklist