Skip to content

Conversation

@fselmo
Copy link
Collaborator

@fselmo fselmo commented May 29, 2025

What was wrong?

Closes #3711

How was it fixed?

  • We can't set the provider, we must let AutoProvider decide when it makes proxy calls using the "active" provider.

  • For the reason above, we need to implement a proxy batch request so that it can instead pull the active provider and make the batch request using it.

  • Make sure current tests fail with the old implementation and pass with the new one.

Todo:

Cute Animal Picture

Screenshot 2025-05-29 at 09 30 31



def load_provider_from_environment() -> BaseProvider:
def load_provider_from_environment() -> Optional[JSONBaseProvider]:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This narrows scope by a small margin, but I'd argue it's a bugfix. Thoughts here? Batching requires JSONBaseProvider. All of web3 does at the moment, for that matter.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that's fine

@fselmo fselmo self-assigned this May 29, 2025
@fselmo fselmo requested review from kclowes, pacrob and reedsa May 29, 2025 15:36
@fselmo
Copy link
Collaborator Author

fselmo commented May 29, 2025

cc: @antazoey

- We can't set the provider, we must let `AutoProvider` decide when it
  makes proxy calls using the "active" provider.

- For the reason above, we need to implement a proxy batch request
  so that it can instead pull the active provider and make the
  batch request using it.

- Make sure current tests fail with the old implementation and pass
  with the new one.

closes ethereum#3711
@fselmo fselmo force-pushed the bugfix-auto-provider branch from ae360ca to 4e7bbe7 Compare May 29, 2025 15:40
@pacrob pacrob removed their request for review June 2, 2025 18:55
@fselmo fselmo removed the request for review from reedsa June 3, 2025 15:48
Copy link
Collaborator

@kclowes kclowes left a comment

Choose a reason for hiding this comment

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

LGTM! Nice test updates!



def load_provider_from_environment() -> BaseProvider:
def load_provider_from_environment() -> Optional[JSONBaseProvider]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that's fine

@fselmo fselmo merged commit d7295c6 into ethereum:main Jun 5, 2025
85 checks passed
@fselmo fselmo deleted the bugfix-auto-provider branch June 5, 2025 17:25
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.

AutoProvider has no attribute _batching_context

2 participants