Race leading to LMDB assert failed on wallet creation

[PlasmaPower]

Description of bug:

Occasional crash when creating a wallet, due to yet another bad side effect of the dual wallet memory and LMDB storage.

Steps to reproduce the issue:

Can't be reliably reproduced, but try creating wallets en masse during bootstrapping or high tps.

Describe the results you received:

Assertion (status == 0) failed
nano::mdb_iterator<T, U>::mdb_iterator(const nano::transaction&, MDB_dbi, const MDB_val&) [with T = nano::public_key; U = nano::wallet_value; MDB_dbi = unsigned int; MDB_val = MDB_val]
/home/lee/programming/cpp/nano-node/nano/node/lmdb/lmdb_iterator.hpp:39

Describe the results you expected:

The node to not crash

Additional information you deem important (e.g. issue happens only occasionally):

I believe I've traced this issue. The generic problem is the following situation:

• Wallet transaction A created
• Wallet transaction B created
• Wallet created with transaction B
• Transaction B committed
• Wallet saved to memory
• Attempt to access wallet with transaction A, probably because of looping over in-memory wallets, but the wallet didn't exist at the time of transaction A's creation

I believe this is how it happened for me, given the situation and the exact assert that failed (copied from my Discord post):

active_transactions::block_cemented_callback creates a wallet transaction then calls node::receive_confirmed with it, which then iterates over the wallets in memory via wallets::get_wallets and calls wallet_store::exists with the transaction and the wallet store including the MDB_dbi, which then calls wallet_store::begin, which then calls mdb_iterator::mdb_iterator, which fails because the wallet db didn't exist when the transaction was created, which leads to the failed assert in the crash

and I should note that this race is made more likely by the mutex which protects both wallets::create and wallets::get_wallets, meaning that the get_wallets call waits for the wallet to finish being created, making this race significantly more likely

Environment:

Local beta net release build of V22.0DB8

logs

Ends with:

[2020-Nov-01 11:26:41.953555]: 0x5602b9060210 {"action":"wallet_create"}
[2020-Nov-01 11:26:42.047889]: RPC request 0x5602b9060210 completed in: 94357 microseconds

[wezrule]

Thanks for the detailed bug report. I also agree with your sequence of events, and have provided a fix for it here: https://github.com/nanocurrency/nano-node/pull/3051

[zhyatt]

Closed by #3051