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.

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

0.16.0 bitcoin-qt: "Assertion `copyFrom' failed" during launch #13110

Closed
DeltaLeonis opened this issue Apr 28, 2018 · 7 comments · Fixed by #13265
Closed

0.16.0 bitcoin-qt: "Assertion `copyFrom' failed" during launch #13110

DeltaLeonis opened this issue Apr 28, 2018 · 7 comments · Fixed by #13265
Assignees
Milestone

Comments

@DeltaLeonis
Copy link

During startup of bitcoint-qt version 0.16.0

$ ./bitcoin-qt

the following assertion is triggered:

bitcoin-qt: wallet/wallet.cpp:542: void CWallet::SyncMetaData(std::pair<std::_Rb_tree_iterator<std::pair<const COutPoint, uint256> >, std::_Rb_tree_iterator<std::pair<const COutPoint, uint256> > >): Assertion `copyFrom' failed.

The issue reliably reproduces with 0.16.0 when using the same ~/.bitcoin directory that was used without problems in 0.15.1. Relaunching 0.15.1 instead of 0.16.0 gives no assertion failure and still works without any issue.

A (stripped down) debug.log is attached here. The log really ends at 2018-04-28 17:21:47 init message: Loading wallet... (which seems consistent with the assertion failure).

bitcoin.conf is an empty, zero byte file.

This machine runs (K)Ubuntu 17.04. CPU is an AMD A6-7400K Radeon R5, 6 Compute Cores 2C+4G. Disk is an Intel 512 GB 545s SSD.

Bitcoin Core 0.15.1 was downloaded from https://bitcoin.org/bin/bitcoin-core-0.15.1/bitcoin-0.15.1-x86_64-linux-gnu.tar.gz if I recall correctly.

Bitcoin Core 0.16.0 was downloaded from https://bitcoincore.org/bin/bitcoin-core-0.16.0/bitcoin-0.16.0-x86_64-linux-gnu.tar.gz.

Note that those are not versions from the Ubuntu repositories/the Ubuntu specific downloads.

@laanwj
Copy link
Member

laanwj commented Apr 30, 2018

Interesting: this assertion was added in #11074. This was indeed first released in 0.16.0.

Can you try to run with that commit (6c4042a) reverted?

It might segfault later on, I have no idea what can cause copyFrom to be null but apparently the assumption behind adding it was not correct.

@maflcko maflcko added this to the 0.16.1 milestone Apr 30, 2018
@maflcko
Copy link
Member

maflcko commented Apr 30, 2018

We should probably return early if there is nothing to copy from, i.e. range.first == range.second

@DeltaLeonis
Copy link
Author

Thanks for the response. I will try to (build and) run 0.16.0 with commit 6c4042a reverted but having limited time available at the moment it might take a while before I follow up.

@DeltaLeonis
Copy link
Author

With commit 6c4042a reverted I get:

bitcoin-qt: wallet/wallet.cpp:547: void CWallet::SyncMetaData(std::pair<std::_Rb_tree_iterator<std::pair<const COutPoint, uint256> >, std::_Rb_tree_iterator<std::pair<const COutPoint, uint256> > >): Assertion `copyFrom && "Oldest wallet transaction in range assumed to have been found."' failed.

I could try with commit fdc3293 additionally reverted but it seems obvious at this point that it would dereference a NULL pointer at

if (!copyFrom->IsEquivalentTo(*copyTo)) continue;

@laanwj
Copy link
Member

laanwj commented May 1, 2018

This should work:

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 6e0f49f..844167d 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -548,7 +548,9 @@ void CWallet::SyncMetaData(std::pair<TxSpends::iterator, TxSpends::iterator> ran
         }
     }
 
-    assert(copyFrom);
+    if (!copyFrom) {
+        return;
+    }
 
     // Now copy data from copyFrom to rest:
     for (TxSpends::iterator it = range.first; it != range.second; ++it)

@DeltaLeonis
Copy link
Author

This does indeed seem to work.

However, I guess it only fixes the symptom but does not explain why copyFrom is a nullptr for me, yet isn't for most people? (well... that's what I assume).

I have not tried to understand the code in more detail and/or run with GDB, but can try to do so if we want to understand the cause? Two remarks about this wallet:

  • It only consists of a few old transactions (over 300 000 confirmations) where coins were received
  • No coins were ever spent
  • It's not an HD wallet

By the way, note that the line

assert(copyFrom && "Oldest wallet transaction in range assumed to have been found.");

seems superfluous to me anyway (either the assert(copyFrom) or your suggested patch will prevent it from ever failing).

@laanwj laanwj self-assigned this May 17, 2018
laanwj added a commit to laanwj/bitcoin that referenced this issue May 17, 2018
Instead of crash with an assertion error, simply exit the function
`SyncMetaData` if there is no metadata to sync.

Fixes bitcoin#13110.
@laanwj
Copy link
Member

laanwj commented May 17, 2018

I think the explanation is simply that in your case, there is no metadata to sync, so the range is empty.

laanwj added a commit that referenced this issue May 18, 2018
…to sync

b0d2ca9 wallet: Exit SyncMetaData if there are no transactions to sync (Wladimir J. van der Laan)

Pull request description:

  Instead of crash with an assertion error, simply exit the function `SyncMetaData` if there is no metadata to sync.

  Fixes #13110.

Tree-SHA512: 44c4789497b5b63963bef66d8b695987dde80764199f6ea0f2c974be19d29c2663f32446a663a2ee9029e143e5d1d9e8a591e52e6e7e795b982782626bec25bb
laanwj added a commit that referenced this issue May 18, 2018
Instead of crash with an assertion error, simply exit the function
`SyncMetaData` if there is no metadata to sync.

Fixes #13110.

Github-Pull: #13265
Rebased-From: b0d2ca9
Tree-SHA512: 67e446e9ced901e37003a9661b6abea268e2ea648ac3b076d91c8d996de96bed389839a09d579a6562d930bcf501a091eb67454f474aae1174108a8650502072
stamhe pushed a commit to stamhe/bitcoin that referenced this issue Jun 27, 2018
Instead of crash with an assertion error, simply exit the function
`SyncMetaData` if there is no metadata to sync.

Fixes bitcoin#13110.
HashUnlimited pushed a commit to chaincoin/chaincoin that referenced this issue Jun 29, 2018
Instead of crash with an assertion error, simply exit the function
`SyncMetaData` if there is no metadata to sync.

Fixes bitcoin#13110.

Github-Pull: bitcoin#13265
Rebased-From: b0d2ca9
Tree-SHA512: 67e446e9ced901e37003a9661b6abea268e2ea648ac3b076d91c8d996de96bed389839a09d579a6562d930bcf501a091eb67454f474aae1174108a8650502072
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this issue Mar 5, 2020
…to sync

Summary:
b0d2ca9 wallet: Exit SyncMetaData if there are no transactions to sync (Wladimir J. van der Laan)

Pull request description:

  Instead of crash with an assertion error, simply exit the function `SyncMetaData` if there is no metadata to sync.

  Fixes #13110.

Tree-SHA512: 44c4789497b5b63963bef66d8b695987dde80764199f6ea0f2c974be19d29c2663f32446a663a2ee9029e143e5d1d9e8a591e52e6e7e795b982782626bec25bb

Backport of Core [[bitcoin/bitcoin#13265 | PR13265]]

This skips [[bitcoin/bitcoin#11074 | PR11074]] and should avoid the issue found here: bitcoin/bitcoin#13110

Test Plan:
  ninja check
  ./bitcoin-qt

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D5435
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this issue Mar 14, 2020
…ctions to sync

b0d2ca9 wallet: Exit SyncMetaData if there are no transactions to sync (Wladimir J. van der Laan)

Pull request description:

  Instead of crash with an assertion error, simply exit the function `SyncMetaData` if there is no metadata to sync.

  Fixes bitcoin#13110.

Tree-SHA512: 44c4789497b5b63963bef66d8b695987dde80764199f6ea0f2c974be19d29c2663f32446a663a2ee9029e143e5d1d9e8a591e52e6e7e795b982782626bec25bb
ftrader pushed a commit to bitcoin-cash-node/bitcoin-cash-node that referenced this issue May 19, 2020
…to sync

Summary:
b0d2ca9 wallet: Exit SyncMetaData if there are no transactions to sync (Wladimir J. van der Laan)

Pull request description:

  Instead of crash with an assertion error, simply exit the function `SyncMetaData` if there is no metadata to sync.

  Fixes #13110.

Tree-SHA512: 44c4789497b5b63963bef66d8b695987dde80764199f6ea0f2c974be19d29c2663f32446a663a2ee9029e143e5d1d9e8a591e52e6e7e795b982782626bec25bb

Backport of Core [[bitcoin/bitcoin#13265 | PR13265]]

This skips [[bitcoin/bitcoin#11074 | PR11074]] and should avoid the issue found here: bitcoin/bitcoin#13110

Test Plan:
  ninja check
  ./bitcoin-qt

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D5435
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this issue Jun 17, 2020
…ctions to sync

b0d2ca9 wallet: Exit SyncMetaData if there are no transactions to sync (Wladimir J. van der Laan)

Pull request description:

  Instead of crash with an assertion error, simply exit the function `SyncMetaData` if there is no metadata to sync.

  Fixes bitcoin#13110.

Tree-SHA512: 44c4789497b5b63963bef66d8b695987dde80764199f6ea0f2c974be19d29c2663f32446a663a2ee9029e143e5d1d9e8a591e52e6e7e795b982782626bec25bb
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this issue Jun 27, 2020
…ctions to sync

b0d2ca9 wallet: Exit SyncMetaData if there are no transactions to sync (Wladimir J. van der Laan)

Pull request description:

  Instead of crash with an assertion error, simply exit the function `SyncMetaData` if there is no metadata to sync.

  Fixes bitcoin#13110.

Tree-SHA512: 44c4789497b5b63963bef66d8b695987dde80764199f6ea0f2c974be19d29c2663f32446a663a2ee9029e143e5d1d9e8a591e52e6e7e795b982782626bec25bb
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this issue Jun 28, 2020
…ctions to sync

b0d2ca9 wallet: Exit SyncMetaData if there are no transactions to sync (Wladimir J. van der Laan)

Pull request description:

  Instead of crash with an assertion error, simply exit the function `SyncMetaData` if there is no metadata to sync.

  Fixes bitcoin#13110.

Tree-SHA512: 44c4789497b5b63963bef66d8b695987dde80764199f6ea0f2c974be19d29c2663f32446a663a2ee9029e143e5d1d9e8a591e52e6e7e795b982782626bec25bb
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this issue Jun 29, 2020
…ctions to sync

b0d2ca9 wallet: Exit SyncMetaData if there are no transactions to sync (Wladimir J. van der Laan)

Pull request description:

  Instead of crash with an assertion error, simply exit the function `SyncMetaData` if there is no metadata to sync.

  Fixes bitcoin#13110.

Tree-SHA512: 44c4789497b5b63963bef66d8b695987dde80764199f6ea0f2c974be19d29c2663f32446a663a2ee9029e143e5d1d9e8a591e52e6e7e795b982782626bec25bb
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this issue Jul 1, 2020
…ctions to sync

b0d2ca9 wallet: Exit SyncMetaData if there are no transactions to sync (Wladimir J. van der Laan)

Pull request description:

  Instead of crash with an assertion error, simply exit the function `SyncMetaData` if there is no metadata to sync.

  Fixes bitcoin#13110.

Tree-SHA512: 44c4789497b5b63963bef66d8b695987dde80764199f6ea0f2c974be19d29c2663f32446a663a2ee9029e143e5d1d9e8a591e52e6e7e795b982782626bec25bb
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
gades pushed a commit to cosanta/cosanta-core that referenced this issue Mar 10, 2022
…ctions to sync

b0d2ca9 wallet: Exit SyncMetaData if there are no transactions to sync (Wladimir J. van der Laan)

Pull request description:

  Instead of crash with an assertion error, simply exit the function `SyncMetaData` if there is no metadata to sync.

  Fixes bitcoin#13110.

Tree-SHA512: 44c4789497b5b63963bef66d8b695987dde80764199f6ea0f2c974be19d29c2663f32446a663a2ee9029e143e5d1d9e8a591e52e6e7e795b982782626bec25bb
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants