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

Delay EthQuery instantiation until network provider is available #274

Merged
merged 7 commits into from
Mar 15, 2024

Conversation

pedronfigueiredo
Copy link
Contributor

@pedronfigueiredo pedronfigueiredo commented Feb 21, 2024

In the future, the network provider may no longer be defined when the Smart Transactions Controller is instantiated. As such, this PR initializes EthQuery to undefined, and onNetworkStateChanged can set it as soon as a provider is available.

For more details on the reasoning behind this change, please see the linked PRs and issues.

Copy link
Contributor

@legobeat legobeat left a comment

Choose a reason for hiding this comment

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

Changes here LGTM but I guess we should consider this blocked by #276 , which is already quite sizable?

@legobeat
Copy link
Contributor

@pedronfigueiredo What is the added method mentioned in the description? It seems like onNetworkStateChanged still uses the provider from the constructor, and there is no interface to change that? I may be missing something.

@pedronfigueiredo
Copy link
Contributor Author

Thanks for the heads up @legobeat, I had forgotten to update the PR description. Should be fixed now.

@pedronfigueiredo pedronfigueiredo dismissed legobeat’s stale review March 15, 2024 14:17

Updated PR description

@pedronfigueiredo pedronfigueiredo merged commit 55b22e8 into main Mar 15, 2024
16 checks passed
@pedronfigueiredo pedronfigueiredo deleted the feat/mm-2121 branch March 15, 2024 14:17
legobeat added a commit to legobeat/smart-transactions-controller that referenced this pull request Mar 19, 2024
@legobeat legobeat mentioned this pull request Mar 19, 2024
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