Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

migratewallet crashes on an freshly created datadir ( wallet/wallet.h:959: int wallet::CWallet::GetLastBlockHeight() const: Assertion `m_last_block_processed_height >= 0' failed.) #28510

Closed
1 task done
maflcko opened this issue Sep 20, 2023 · 7 comments · Fixed by #28542
Closed
1 task done

migratewallet crashes on an freshly created datadir ( wallet/wallet.h:959: int wallet::CWallet::GetLastBlockHeight() const: Assertion `m_last_block_processed_height >= 0' failed.) #28510

maflcko opened this issue Sep 20, 2023 · 7 comments · Fixed by #28542
Labels
Wallet

Comments

@maflcko
Copy link
Member

maflcko commented Sep 20, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Current behaviour

crash

Expected behaviour

no crash

Steps to reproduce

./src/qt/bitcoin-qt -datadir=/tmp/ -regtest -printtoconsole=1 -nolisten  # Create fresh datadir (empty), stop
mv /tmp/reg_scratch/.bitcoin/regtest/wallet.1491854176.bak /tmp/regtest/wallets/  # Move over a wallet file
(Start qt with the same args as above)
(Call migratewallet)

Relevant log output

#5  0x0000555555bd2bda in wallet::CWallet::GetTxDepthInMainChain(wallet::CWalletTx const&) const (this=<optimized out>, wtx=<optimized out>) at /usr/bin/../lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/std_function.h:290
#6  wallet::CWallet::MarkConflicted(uint256 const&, int, uint256 const&)::$_0::operator()(wallet::CWalletTx&) const (this=<optimized out>, wtx=<optimized out>) at wallet/wallet.cpp:1351
#7  std::__invoke_impl<wallet::CWallet::TxUpdate, wallet::CWallet::MarkConflicted(uint256 const&, int, uint256 const&)::$_0&, wallet::CWalletTx&>(std::__invoke_other, wallet::CWallet::MarkConflicted(uint256 const&, int, uint256 const&)::$_0&, wallet::CWalletTx&) (__f=<optimized out>, __args=<optimized out>) at /usr/bin/../lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/invoke.h:61
#8  std::__invoke_r<wallet::CWallet::TxUpdate, wallet::CWallet::MarkConflicted(uint256 const&, int, uint256 const&)::$_0&, wallet::CWalletTx&>(wallet::CWallet::MarkConflicted(uint256 const&, int, uint256 const&)::$_0&, wallet::CWalletTx&) (__fn=<optimized out>, __args=<optimized out>) at /usr/bin/../lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/invoke.h:114
#9  std::_Function_handler<wallet::CWallet::TxUpdate (wallet::CWalletTx&), wallet::CWallet::MarkConflicted(uint256 const&, int, uint256 const&)::$_0>::_M_invoke(std::_Any_data const&, wallet::CWalletTx&)
    (__functor=<optimized out>, __args=<optimized out>) at /usr/bin/../lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/std_function.h:290
#10 0x0000555555bbb73b in std::function<wallet::CWallet::TxUpdate (wallet::CWalletTx&)>::operator()(wallet::CWalletTx&) const (this=0x7fff417f6bc0, __args=...)
    at /usr/bin/../lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/std_function.h:591
#11 wallet::CWallet::RecursiveUpdateTxState(uint256 const&, std::function<wallet::CWallet::TxUpdate (wallet::CWalletTx&)> const&) (this=0x7fff2db509f0, tx_hash=..., try_updating_state=...) at wallet/wallet.cpp:1382
#12 0x0000555555bb9c4b in wallet::CWallet::MarkConflicted(uint256 const&, int, uint256 const&) (this=0x7fff2db509f0, hashBlock=..., conflicting_height=-1, hashTx=...) at wallet/wallet.cpp:1361
#13 0x0000555555bb9b55 in wallet::CWallet::LoadToWallet(uint256 const&, std::function<bool (wallet::CWalletTx&, bool)> const&) (this=0x7fff2db509f0, hash=<optimized out>, fill_wtx=<optimized out>) at wallet/wallet.cpp:1213
#14 0x0000555555bfc8c6 in wallet::LoadTxRecords(wallet::CWallet*, wallet::DatabaseBatch&, std::vector<uint256, std::allocator<uint256> >&, bool&)::$_0::operator()(wallet::CWallet*, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) const (this=0x7fff417f6ed0, pwallet=0x7fff2db509f0, key=<optimized out>, value=..., err="") at wallet/walletdb.cpp:1065
#15 std::__invoke_impl<wallet::DBErrors, wallet::LoadTxRecords(wallet::CWallet*, wallet::DatabaseBatch&, std::vector<uint256, std::allocator<uint256> >&, bool&)::$_0&, wallet::CWallet*, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&>(std::__invoke_other, wallet::LoadTxRecords(wallet::CWallet*, wallet::DatabaseBatch&, std::vector<uint256, std::allocator<uint256> >&, bool&)::$_0&, wallet::CWallet*&&, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) (__f=..., __args=<optimized out>, __args=<optimized out>, __args=..., __args="")
    at /usr/bin/../lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/invoke.h:61
#16 std::__invoke_r<wallet::DBErrors, wallet::LoadTxRecords(wallet::CWallet*, wallet::DatabaseBatch&, std::vector<uint256, std::allocator<uint256> >&, bool&)::$_0&, wallet::CWallet*, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&>(wallet::LoadTxRecords(wallet::CWallet*, wallet::DatabaseBatch&, std::vector<uint256, std::allocator<uint256> >&, bool&)::$_0&, wallet::CWallet*&&, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) (__fn=..., __args=<optimized out>, __args=<optimized out>, __args=..., __args="")
    at /usr/bin/../lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/invoke.h:114
#17 std::_Function_handler<wallet::DBErrors (wallet::CWallet*, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&), wallet::LoadTxRecords(wallet::CWallet*, wallet::DatabaseBatch&, std::vector<uint256, std::allocator<uint256> >&, bool&)::$_0>::_M_invoke(std::_Any_data const&, wallet::CWallet*&&, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&)
    (__functor=..., __args=<optimized out>, __args=<optimized out>, __args=..., __args="") at /usr/bin/../lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/std_function.h:290
#18 0x0000555555bf87dc in std::function<wallet::DBErrors (wallet::CWallet*, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&)>::operator()(wallet::CWallet*, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&) const (this=0x7fff417f6ed0, __args=0x7fff2db509f0, __args=..., __args=..., __args="")
    at /usr/bin/../lib/gcc/x86_64-redhat-linux/13/../../../../include/c++/13/bits/std_function.h:591
#19 wallet::LoadRecords(wallet::CWallet*, wallet::DatabaseBatch&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, DataStream&, std::function<wallet::DBErrors (wallet::CWallet*, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&)>) (pwallet=pwallet@entry=0x7fff2db509f0, batch=..., key="tx", prefix=..., load_func=...) at wallet/walletdb.cpp:509
#20 0x0000555555bf8542 in wallet::LoadRecords(wallet::CWallet*, wallet::DatabaseBatch&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::function<wallet::DBErrors (wallet::CWallet*, DataStream&, CDataStream&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&)>) (pwallet=pwallet@entry=0x7fff2db509f0, batch=..., key="tx", load_func=...) at wallet/walletdb.cpp:523
#21 0x0000555555bf40a2 in wallet::LoadTxRecords(wallet::CWallet*, wallet::DatabaseBatch&, std::vector<uint256, std::allocator<uint256> >&, bool&)
    (pwallet=0x7fff2db509f0, batch=..., upgraded_txs=std::vector of length 0, capacity 0, any_unordered=@0x7fff417f6f9f: false) at wallet/walletdb.cpp:1021
#22 wallet::WalletBatch::LoadWallet(wallet::CWallet*) (this=0x7fff417f7208, pwallet=0x7fff2db509f0) at wallet/walletdb.cpp:1187
#23 0x0000555555bc29fd in wallet::CWallet::LoadWallet() (this=0x7fff2db509f0) at wallet/wallet.cpp:2304
#24 0x0000555555bad386 in wallet::CWallet::Create(wallet::WalletContext&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::unique_ptr<wallet::WalletDatabase, std::default_delete<wallet::WalletDatabase> >, unsigned long, bilingual_str&, std::vector<bilingual_str, std::allocator<bilingual_str> >&)
    (context=<optimized out>, name="wallet.1491854156.bak", database=std::unique_ptr<wallet::WalletDatabase> = {...}, wallet_creation_flags=0, error=..., warnings=std::vector of length 0, capacity 0) at wallet/wallet.cpp:2868
#25 0x0000555555bcf830 in wallet::MigrateLegacyToDescriptor(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, secure_allocator<char> > const&, wallet::WalletContext&) (wallet_name="wallet.1491854156.bak", passphrase="", context=...) at wallet/wallet.cpp:4169

How did you obtain Bitcoin Core

Compiled from source

What version of Bitcoin Core are you using?

current master

Operating system and version

Linux

Machine specifications

yes
@maflcko
Copy link
Member Author

maflcko commented Sep 20, 2023

wallet.1491854156.bak.gpg.not.txt

@achow101

@maflcko maflcko added the Wallet label Sep 20, 2023
@maflcko
Copy link
Member Author

maflcko commented Sep 20, 2023

The datadir is not corrupt, because it is empty. However, the wallet may be corrupt, as it is just a random file I found on my storage.

@maflcko
Copy link
Member Author

maflcko commented Sep 20, 2023

For reference, this is my normal regtest wallet, which may or may not be corrupt as well:

wallet.dat.gpg.not.txt (encrypted to DB88DB0BD2EDFBFC)

@achow101
Copy link
Member

However, the wallet may be corrupt, as it is just a random file I found on my storage.

That's not a supported use case.

@maflcko
Copy link
Member Author

maflcko commented Sep 20, 2023

Yeah, I wasn't sure how to check for wallet corruption. getwalletinfo passes on the file.

@achow101
Copy link
Member

The issue appears to be due to having txs that are conflicted.

@achow101
Copy link
Member

This is a more general issue with MarkConflicted and how it behaves when there is no chain. It's possible to hit the same assertion using the wallettool.

Specifically the issue is caused by MarkConflicted marking child transactions as conflicted during loading. It requires having a chain of transactions that are conflicted.

@achow101 achow101 mentioned this issue Sep 27, 2023
Merged
Frank-GER pushed a commit to syscoin/syscoin that referenced this issue Oct 5, 2023
@fanquake
Merge bitcoin#28542: wallet: Check for uninitialized last processed a…
3ddbf89 
…nd conflicting heights in MarkConflicted

782701c test: Test loading wallets with conflicts without a chain (Andrew Chow)
4660fc8 wallet: Check last block and conflict height are valid in MarkConflicted (Andrew Chow)

Pull request description:

  `MarkConflicted` assumes that `m_last_block_processed_height` is always valid. However it may not be valid when a chain is not attached, as happens in the wallet tool and during migration. In such situations, when the conflicting height is also negative (which occurs on loading when no chain is available), the calculation of the number of conflict confirms results in a non-negative value which passes the existing check for valid values. This will subsequently hit an assertion in `GetTxDepthInMainChain`.

  Furthermore, `MarkConflicted` is also only called on loading a transaction whose parent has a stored state of `TxStateConflicted` and was loaded before the child transaction. This depends on the loading order, which for both sqlite and bdb depends on the txids.

  We can avoid this by explicitly checking that both `m_last_block_processed_height` and `conflicting_height` are non-negative. Both `tool_wallet.py` and `wallet_migration.py` are updated to create wallets with a state that triggers the assertion.

  Fixes bitcoin#28510

ACKs for top commit:
  ryanofsky:
    Code review ACK 782701c. Nice catch, and clever test (grinding the txid)
  furszy:
    ACK 782701c

Tree-SHA512: 1344e0279ec5413a43a2819d101fb571fbf4821de2d13958a0fdffc99f57082ef3243ec454c8343f97dc02ed1fce8c8b0fd89388420ab2e55618af42ad5630a9
@maflcko maflcko mentioned this issue Feb 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Wallet
Projects
None yet
Milestone
No milestone
2 participants
@achow101 @maflcko