Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors lock handling for the ethui_networks::Networks global state to prevent UI hang-ups caused by holding locks during async network operations. The solution introduces helper functions that clone network data or providers after briefly acquiring and releasing the lock, ensuring locks are never held across await points.
Key changes:
- Added public helper functions
get_network()andget_provider()in the networks crate that handle lock acquisition/release - Replaced all instances where locks were held during network requests with calls to these new helpers
- Added
_cloned()variants of getter methods to make cloning explicit and prevent lock retention
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
crates/networks/src/lib.rs |
Added get_network(), get_provider(), and _cloned() helper methods; removed chain_id_from_provider() |
crates/networks/src/commands.rs |
Updated to use get_network_by_dedup_chain_id_cloned() and moved provider connection logic inline to avoid holding lock |
crates/types/src/prelude.rs |
Added Ethereum and RootProvider to prelude exports to support new helper function signatures |
crates/sync/src/utils.rs |
Replaced local provider() helper with calls to ethui_networks::get_provider() |
crates/sync/src/commands.rs |
Replaced lock-holding pattern with get_network() helper; removed unused imports |
crates/rpc/src/utils.rs |
Simplified to use get_provider() helper instead of manually managing lock |
crates/forge/src/utils.rs |
Replaced error-handling and lock management with get_provider() helper; removed unused imports |
crates/ws/src/peers.rs |
Store lock result before conditional check to release lock earlier; reordered imports |
bin/src/commands.rs |
Updated to use get_network() helper; removed unused imports |
bin/src/app.rs |
Added conditional compilation guards for Runtime and TauriPlugin imports only used with aptabase feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I identified a few places where locks on
ethui_networks::Networkswere being held for too long (during network requests) which is the likely cause of some UI hang-ups in recent versionsThe new approach avoids locking into the global networks object by cloning the network itself, or fetching a provider directly in an auxiliary function
in a future update, we'll likely migrate this to kameo as well to ensure this kind of bug can't happen so easily