Skip to content

Add async chain_id setter #2376

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

Merged
merged 2 commits into from
Mar 14, 2022
Merged

Conversation

kclowes
Copy link
Collaborator

@kclowes kclowes commented Mar 3, 2022

What was wrong?

The new chain_id flow wasn't available on async eth.

Related to Issue #2207

How was it fixed?

Added it!

Todo:

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@kclowes kclowes force-pushed the put-back-async-chain-id branch from 952c1d4 to bec39bf Compare March 3, 2022 23:05
@kclowes kclowes requested review from pacrob and fselmo March 3, 2022 23:14
Copy link
Contributor

@pacrob pacrob left a comment

Choose a reason for hiding this comment

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

lgtm!

Copy link
Collaborator

@fselmo fselmo left a comment

Choose a reason for hiding this comment

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

lgtm!

@fselmo
Copy link
Collaborator

fselmo commented Mar 9, 2022

I wonder if we should have a validation that it can only be set to the eth_chainId value. But we can have this be a separate discussion I think. This completes the current flow 👌

edit: Or, alternatively, not even accept a user value, just call the setter to internally set the value to static. Almost like caching but not quite since you can turn it off by setting it back to None.

@kclowes
Copy link
Collaborator Author

kclowes commented Mar 14, 2022

I am going to merge this, but like we talked about this morning, I'll check on the original issue to see if there is any reason to allow a different chain_id to be set.

@kclowes kclowes merged commit ca662c8 into ethereum:master Mar 14, 2022
@kclowes kclowes deleted the put-back-async-chain-id branch March 14, 2022 16:56
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.

3 participants