Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Wallet create causing crash when confirming blocks #3051

Merged
merged 3 commits into from
Jan 29, 2021

Conversation

wezrule
Copy link
Contributor

@wezrule wezrule commented Nov 17, 2020

This bug was found by @PlasmaPower and nicely documented here.

I think this can be resolved by checking that the handle is valid inside wallet_store::begin. There doesn't seem to be a standard way to test that so I'm just checking that the mdb_dbi_flags doesn't return an error (that function calls TXN_DBI_EXIST which checks that the transaction and dbi arguments are correct and returns EINVAL if they aren't), a few others did that too but this one seemed to make the most sense.

The new slow test added seems to more often than not hit the bug on a single iteration, but doing 5 in a loop just to increase the probability.

Couple unrelated changes I made: Added [[maybe_unused] in receive_confirmed & removed an unnecessary std::move in wallets::queue_wallet_action

@wezrule wezrule added the bug label Nov 17, 2020
@wezrule wezrule added this to the V22.0 milestone Nov 17, 2020
@wezrule wezrule self-assigned this Nov 17, 2020
SergiySW
SergiySW previously approved these changes Nov 18, 2020
@wezrule wezrule force-pushed the wallet_create_block_confirm_bug branch from 34ff27b to 9d3b2ed Compare November 18, 2020 15:49
@PlasmaPower
Copy link
Contributor

Given the latest commit, I should note that I'm not sure I found the only possible scenario where a wallet transaction is created and then get_wallets is called.

@wezrule
Copy link
Contributor Author

wezrule commented Nov 18, 2020

The only other place is search_pending which was already creating the transaction after calling get_wallets (). However they both didn't account for deletes so now getting the read snaphot under the wallets mutex as well to make sure they are in a valid state.

@wezrule wezrule merged commit 27b206c into nanocurrency:develop Jan 29, 2021
@wezrule wezrule deleted the wallet_create_block_confirm_bug branch January 29, 2021 09:56
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants