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

fix: provider settings RPC was not being used when it was the default #1919

Merged
merged 3 commits into from
Feb 7, 2024

Conversation

antazoey
Copy link
Member

@antazoey antazoey commented Feb 7, 2024

What I did

In ape-geth, if was given a provdider-settings URI, use that no matter what
Also, when generating config defaults, if something is running on the local, still use that as the default just in case.
Then, finally, if the default is available, use that instead of again checking for random public

fixes: #1917

How I did it

How to verify it

Checklist

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

@antazoey antazoey changed the title fix: issue when running on default fix: default public RPC clashing with localhost when actually running Feb 7, 2024
fubuloubu
fubuloubu previously approved these changes Feb 7, 2024
Copy link
Contributor

@mikeshultz mikeshultz left a comment

Choose a reason for hiding this comment

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

I get that the public RPC change was "breaking" in that it changed behavior so it may be a bug. But I'm wondering if this is any more correct.

Say that I happen to be running a local anvil/hardhat node that I'm using for unrelated dev purposes. Then I run ape console --network ethereum:mainnet to do otherwork. I wouldn't really want it to connect to my local node just because it was responds to a HEAD request.

src/ape_geth/provider.py Outdated Show resolved Hide resolved
@antazoey antazoey force-pushed the fix/public-rpc-local-interfere branch from 60ff402 to 17c6120 Compare February 7, 2024 21:12
@antazoey antazoey force-pushed the fix/public-rpc-local-interfere branch from 17c6120 to 9fe6f24 Compare February 7, 2024 21:13
Copy link
Contributor

@mikeshultz mikeshultz left a comment

Choose a reason for hiding this comment

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

LGTM

@antazoey antazoey changed the title fix: default public RPC clashing with localhost when actually running fix: provider settings RPC was not being used when it was the default Feb 7, 2024
@antazoey antazoey enabled auto-merge (squash) February 7, 2024 21:35
@antazoey antazoey merged commit 49b9fb1 into ApeWorX:main Feb 7, 2024
16 checks passed
@antazoey antazoey deleted the fix/public-rpc-local-interfere branch February 7, 2024 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

crash on long event query after refusing to connect to localhost
3 participants