Test Details Overview

Encapsulate the open wallets map and test wallet RPC thread safety (#5127)

* Encapsulate the open wallets map behind synchronized accessors

RPC handlers, the CLI and the Qt wallet accessed wallets::items without
holding the mutex, racing with reload() on the receivable scan thread.
The map is now private; callers go through open() or the new snapshot
queries: all_wallets(), wallet_ids() and wallet_count().

* Test wallet RPC thread safety with concurrent requests

Adds a wait_responses test primitive that keeps a whole batch of RPC
requests in flight at once, together with an rpc_options override for
the RPC → IPC connection count, which defaults to 1 on the dev network
and would otherwise process requests one at a time.

wallets::destroy now returns whether a wallet was destroyed instead of
asserting on a missing wallet, so concurrent wallet_destroy requests
for the same wallet are safe and exactly one of them wins.

* Encapsulate `wallet_store::mutex` to fix lock order inversion

TSAN reported a lock order inversion between wallets::mutex and
wallet_store::mutex. wallets::create holds the former while constructing
a wallet_store that takes the latter, but node.unlock_search evaluated
system::wallet, which now takes wallets::mutex, while already holding a
store mutex.

That guard protected nothing: nano::fan is internally synchronized, so
the value_set it wrapped was already safe. The test, node.send_unkeyed
and the Qt lock button were all open-coding wallet::lock, which does the
same thing and also refreshes the rep keys cache; they now call it.

wallet_store::mutex is private, leaving only the compound operations
inside wallet_store that genuinely need it. refresh_rep_keys_cache drops
its store lock as well, since fetch revalidates the password itself and
the lock only kept the diagnostic log line in step with the result.

* Derive the locked state in `refresh_rep_keys_cache` from the fetch

The cache checked valid_password and then fetched the private key as two
separate operations. Holding the store mutex across the pair was the only
thing keeping them in agreement, and it did not even cover wallet::lock,
which writes the password fan without taking that mutex.

fetch already reports the locked state through wallet_locked, so a single
call yields both the key and the reason it was unavailable, and cannot
disagree with itself under a concurrent rekey. Watch-only representatives
still fall out as bad_private_key and are left uncached without warning.

__________

Type: commit

Hash: 901fdac25909df7d885e27d92362e52bcc57a68d

Created: yesterday

__________

gr0v1ty/nano-node:901fdac25909df7d885e27d92362e52bcc57a68d

Test Results

Test Case Status Duration Median Deviation Log Graph
5n4pr_conf_10k_bintree PASS 116 s 2.0 s - Show
5n4pr_conf_10k_change PASS 201 s 41.0 s - Show
5n4pr_conf_change_dependant PASS 111 s -3.0 s - -
5n4pr_conf_change_independant PASS 109 s -5.0 s - -
5n4pr_conf_send_dependant PASS 137 s 21.0 s - -
5n4pr_conf_send_independant PASS 106 s -7.0 s - -
5n4pr_rocks_10k_bintree PASS 107 s -13.0 s - -
5n4pr_rocks_10k_change PASS 157 s -18.0 s - -