Skip to content

Rethink eth_chainId calls #2266

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

Closed
wants to merge 2 commits into from

Conversation

fselmo
Copy link
Collaborator

@fselmo fselmo commented Dec 17, 2021

What was wrong?

  • We can often end up making multiple calls to eth_chainId to validate the chainId param in a transaction, or to add the default value to each transaction making use of default values (i.e. contract.buildTransaction()).

Related to Issue #2207

How was it fixed?

  • One possible solution is setting an internal chain id value on provider instantiation

  • Another solution would be to cache the call to eth_chainId

    • Downside to this is default caching happens once per instance so each call for w3.eth.chain_id, on its own, would return the cached value and would not be an earnest call.

@fselmo fselmo changed the title Rethink eth chain id calls Rethink eth_chainId calls Dec 17, 2021
@fselmo fselmo force-pushed the rethink-eth_chainId-calls branch from fd3fa9d to 2ee00e6 Compare January 10, 2022 21:57
The `chain_id` value should not change much, if at all. Save this value internally, as `internal_chain_id`, so we are not making extra calls to get the `chain_id` when it is not necessary.
This was preferred over caching the `w3.eth.chain_id` call so that when we explicitly use `w3.eth.chain_id`, we actually make the call in ernest and return the response.
@fselmo fselmo force-pushed the rethink-eth_chainId-calls branch from 2ee00e6 to 020cc42 Compare January 10, 2022 22:23
@fselmo
Copy link
Collaborator Author

fselmo commented Mar 31, 2022

closed since caching seems like the best idea here.

@fselmo fselmo closed this Mar 31, 2022
@fselmo fselmo deleted the rethink-eth_chainId-calls branch April 3, 2024 20:51
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.

1 participant