simplify MutWallet

[MartinquaXD]

Description

alloy's EthereumWallet has some &mut self methods which make it a bit awkward to work with when multiple parties should have access to the same wallet.
This presented some challenges in terms of making it work in the current_thread runtime which is the default for tokio tests. The implementation chosen at the time spawns a new runtime and blocks on that. However, the block_in_place call shows up very prominently in flamegraphs which suggests it has a ton of overhead.

Changes

Replace the tokio::sync::RwLock with std::sync::RwLock which makes all the tokio runtime shenanigans unnecessary. RwLock should in practice never block because once we set up all the signing keys at the start all methods only need to take a read lock so starving the tokio runtime is not an issue.

How to test

existing tests should still pass

[claude]

Claude finished @MartinquaXD's task in 1m 11s —— View job

---

PR Review: simplify MutWallet

• Read changed file
• Analyze correctness of std::sync::RwLock switch
• Post inline findings
• Post summary

Nice simplification — dropping the block_in_place / spawned-runtime machinery in favor of a std::sync::RwLock reads much cleaner, and the flamegraph motivation is compelling.

Correctness looks good. The one thing that matters when using a sync RwLock under async is holding a guard across an .await. In sign_transaction_from you correctly avoid that: Arc::clone(self.0.read().unwrap().deref()) drops the read guard at the end of the statement (before the .await), so only the cheap inner Arc is carried across. 👍

Minor findings:

• One inline comment: the Arc::make_mut doc comment claims we "never give out clones of the inner Arc", but sign_transaction_from does hand one out — so a concurrent sign-in-flight would trigger a deep clone. Harmless (that's what make_mut is for), just worth softening the wording.

Non-blocking note: std::sync::RwLock introduces lock-poisoning semantics that the tokio one didn't have — a panic while holding the write lock (e.g. inside register_signer) would make every subsequent .unwrap() panic. Given signers are registered once at startup and the lock is otherwise read-only, this is very unlikely to bite; flagging only for awareness.

[claude]

The claim "we never give out clones of the inner Arc" isn't strictly accurate anymore: sign_transaction_from does Arc::clone(self.0.read().unwrap().deref()) (line 89). So if a sign_transaction_from future is in flight while register_signer runs, the strong count is >1 and Arc::make_mut will deep-clone here.

That's harmless for correctness (make_mut deep-clones precisely so the mutation stays isolated), but the comment's absolute wording is misleading. Per the PR description all signers are registered up-front before any signing, so this doesn't happen in practice — worth softening the comment to say the deep clone only happens if a sign is concurrently in flight, which shouldn't occur given the registration-at-startup usage.

[MartinquaXD]

Hmm, true. Because you can't extract the inner wallet from the returned future the worst that could happen is a deep clone in register_signers but we can't run into the scenario where we have multiple MutWallet instances flying around that have different sets of signers. That would be a real headache to debug.

[MartinquaXD]

@claude check that the updated comment is fully accurate.

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	ruint@​1.20.0

View full report