-
-
Notifications
You must be signed in to change notification settings - Fork 132
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: add default public RPC endpoints for default configured networks #1866
Conversation
@antazoey thoughts on adding more default ecosystem configurations here? Script currently also includes base, polygon, polygon-zkevm, and gnosis. Could expand/contract that if we want, too. |
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.
cool! I have some ideas but can be discussed or changed later perhaps
CHAIN_CONST_FILE = Path(__file__).parent.parent / "src" / "ape_geth" / "chains.py" | ||
|
||
|
||
class Chain(BaseModel): |
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.
this reminds of me of https://github.com/ApeWorX/ape/issues/254
Specifically this comment: #254 (comment)
If we did that we could use in ape-etherscan to automatically set the URLs
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.
That is like a packaging step away from that comment. Though not sure it's worth the time now.
networkId: int | ||
slip44: Optional[int] = None | ||
ens: Optional[Dict[str, str]] = None | ||
explorers: List[Dict[str, str]] |
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.
thought: I wonder if we should do something similar in ape-etherscan
(As a future)
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.
looks good - a test would be cool though.
this makes forking in tests work out-of-the-gate without having to configure a default provider or set a URI. |
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.
test is going to be too flakey as-is, but it is good.
i added 2 options i could think of.
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.
this is great but I dont want people to open PRs here adding more URIs so before that happens, let's make the chain enum package thing.
What I did
Adds RPC endpoints for default configured networks sourced from official Ethereum Foundation repository.
fixes: #1729
How I did it
Added a script to generate a source file that has constants for the RPC endpoints.
How to verify it
Checklist