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: add default public RPC endpoints for default configured networks #1866

Merged
merged 14 commits into from
Jan 22, 2024

Conversation

mikeshultz
Copy link
Member

@mikeshultz mikeshultz commented Jan 16, 2024

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

ape console --network ethereum:mainnet

Checklist

  • All changes are completed
  • New test cases have been added
  • Documentation has been updated

@mikeshultz mikeshultz added the category: feature New feature or request label Jan 16, 2024
@mikeshultz mikeshultz self-assigned this Jan 16, 2024
@mikeshultz
Copy link
Member Author

https://github.com/mikeshultz/ape/blob/027305b71bcb99b2cebb8b049e9e66e2ae0f3a8d/src/ape_geth/provider.py#L209-L210

@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.

Copy link
Member

@antazoey antazoey left a 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):
Copy link
Member

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

Copy link
Member Author

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]]
Copy link
Member

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)

scripts/update_rpc.py Outdated Show resolved Hide resolved
src/ape_geth/provider.py Outdated Show resolved Hide resolved
@mikeshultz mikeshultz marked this pull request as ready for review January 18, 2024 00:24
antazoey
antazoey previously approved these changes Jan 18, 2024
Copy link
Member

@antazoey antazoey left a 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.

src/ape_geth/provider.py Show resolved Hide resolved
src/ape_geth/provider.py Show resolved Hide resolved
@antazoey
Copy link
Member

antazoey commented Jan 18, 2024

this makes forking in tests work out-of-the-gate without having to configure a default provider or set a URI.
<3

Copy link
Member

@antazoey antazoey left a 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.

scripts/update_rpc.py Show resolved Hide resolved
src/ape_geth/provider.py Show resolved Hide resolved
Copy link
Member

@antazoey antazoey left a 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.

@mikeshultz mikeshultz enabled auto-merge (squash) January 22, 2024 20:25
@mikeshultz mikeshultz merged commit 7e0bf62 into ApeWorX:main Jan 22, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add public RPCs as default geth provider links [APE-1514]
2 participants