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

Assert that CWallet::SyncMetaData finds oldest transaction. #11074

Merged
merged 1 commit into from Nov 9, 2017

Conversation

BitonicEelis
Copy link
Contributor

Without this assert, the Clang static analyzer warns about subsequent dereferencing of copyFrom, because it can't be sure that it's not nullptr. See #9573.

This fixes one of the Clang static analyzer warnings mentioned in bitcoin#9573.
@laanwj laanwj added the Wallet label Aug 17, 2017
@@ -569,6 +569,9 @@ void CWallet::SyncMetaData(std::pair<TxSpends::iterator, TxSpends::iterator> ran
copyFrom = &mapWallet[hash];
}
}

assert(copyFrom);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I've seen there is at least one element in the range:

TxSpends::iterator it = range.first;
const CWalletTx* copyFrom = mapWallet[it->second];
for (; it != range.second; ++i) {
    ...

Maybe this assert should come first:

assert(range.first != range.second);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would not suffice to silence the analyzer, because copyFrom is also nullptr if the range is nonempty but n == nMinOrderPos in every iteration.

@laanwj
Copy link
Member

laanwj commented Nov 9, 2017

utACK 6c4042a

@laanwj laanwj merged commit 6c4042a into bitcoin:master Nov 9, 2017
laanwj added a commit that referenced this pull request Nov 9, 2017
…ion.

6c4042a Assert that CWallet::SyncMetaData finds oldest transaction. (Eelis)

Pull request description:

  Without this assert, the Clang static analyzer warns about subsequent dereferencing of copyFrom, because it can't be sure that it's not nullptr. See #9573.

Tree-SHA512: 83cbcb32c52c94fcfefbc90ec7de2011dacd6bdb0da35adc401b8d8dda6a86de2fa0403e2158592268c2cf15eef4f3d887d98c90f1031d4735d5f4bf9dbc1d23
@BitonicEelis BitonicEelis deleted the syncassert branch November 9, 2017 14:23
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Dec 22, 2019
…ransaction.

6c4042a Assert that CWallet::SyncMetaData finds oldest transaction. (Eelis)

Pull request description:

  Without this assert, the Clang static analyzer warns about subsequent dereferencing of copyFrom, because it can't be sure that it's not nullptr. See bitcoin#9573.

Tree-SHA512: 83cbcb32c52c94fcfefbc90ec7de2011dacd6bdb0da35adc401b8d8dda6a86de2fa0403e2158592268c2cf15eef4f3d887d98c90f1031d4735d5f4bf9dbc1d23
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 2, 2020
…ransaction.

6c4042a Assert that CWallet::SyncMetaData finds oldest transaction. (Eelis)

Pull request description:

  Without this assert, the Clang static analyzer warns about subsequent dereferencing of copyFrom, because it can't be sure that it's not nullptr. See bitcoin#9573.

Tree-SHA512: 83cbcb32c52c94fcfefbc90ec7de2011dacd6bdb0da35adc401b8d8dda6a86de2fa0403e2158592268c2cf15eef4f3d887d98c90f1031d4735d5f4bf9dbc1d23
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 4, 2020
…ransaction.

6c4042a Assert that CWallet::SyncMetaData finds oldest transaction. (Eelis)

Pull request description:

  Without this assert, the Clang static analyzer warns about subsequent dereferencing of copyFrom, because it can't be sure that it's not nullptr. See bitcoin#9573.

Tree-SHA512: 83cbcb32c52c94fcfefbc90ec7de2011dacd6bdb0da35adc401b8d8dda6a86de2fa0403e2158592268c2cf15eef4f3d887d98c90f1031d4735d5f4bf9dbc1d23
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 12, 2020
…ransaction.

6c4042a Assert that CWallet::SyncMetaData finds oldest transaction. (Eelis)

Pull request description:

  Without this assert, the Clang static analyzer warns about subsequent dereferencing of copyFrom, because it can't be sure that it's not nullptr. See bitcoin#9573.

Tree-SHA512: 83cbcb32c52c94fcfefbc90ec7de2011dacd6bdb0da35adc401b8d8dda6a86de2fa0403e2158592268c2cf15eef4f3d887d98c90f1031d4735d5f4bf9dbc1d23
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 12, 2020
…ransaction.

6c4042a Assert that CWallet::SyncMetaData finds oldest transaction. (Eelis)

Pull request description:

  Without this assert, the Clang static analyzer warns about subsequent dereferencing of copyFrom, because it can't be sure that it's not nullptr. See bitcoin#9573.

Tree-SHA512: 83cbcb32c52c94fcfefbc90ec7de2011dacd6bdb0da35adc401b8d8dda6a86de2fa0403e2158592268c2cf15eef4f3d887d98c90f1031d4735d5f4bf9dbc1d23
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 12, 2020
…ransaction.

6c4042a Assert that CWallet::SyncMetaData finds oldest transaction. (Eelis)

Pull request description:

  Without this assert, the Clang static analyzer warns about subsequent dereferencing of copyFrom, because it can't be sure that it's not nullptr. See bitcoin#9573.

Tree-SHA512: 83cbcb32c52c94fcfefbc90ec7de2011dacd6bdb0da35adc401b8d8dda6a86de2fa0403e2158592268c2cf15eef4f3d887d98c90f1031d4735d5f4bf9dbc1d23
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 12, 2020
…ransaction.

6c4042a Assert that CWallet::SyncMetaData finds oldest transaction. (Eelis)

Pull request description:

  Without this assert, the Clang static analyzer warns about subsequent dereferencing of copyFrom, because it can't be sure that it's not nullptr. See bitcoin#9573.

Tree-SHA512: 83cbcb32c52c94fcfefbc90ec7de2011dacd6bdb0da35adc401b8d8dda6a86de2fa0403e2158592268c2cf15eef4f3d887d98c90f1031d4735d5f4bf9dbc1d23
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 12, 2020
…ransaction.

6c4042a Assert that CWallet::SyncMetaData finds oldest transaction. (Eelis)

Pull request description:

  Without this assert, the Clang static analyzer warns about subsequent dereferencing of copyFrom, because it can't be sure that it's not nullptr. See bitcoin#9573.

Tree-SHA512: 83cbcb32c52c94fcfefbc90ec7de2011dacd6bdb0da35adc401b8d8dda6a86de2fa0403e2158592268c2cf15eef4f3d887d98c90f1031d4735d5f4bf9dbc1d23
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 12, 2020
…ransaction.

6c4042a Assert that CWallet::SyncMetaData finds oldest transaction. (Eelis)

Pull request description:

  Without this assert, the Clang static analyzer warns about subsequent dereferencing of copyFrom, because it can't be sure that it's not nullptr. See bitcoin#9573.

Tree-SHA512: 83cbcb32c52c94fcfefbc90ec7de2011dacd6bdb0da35adc401b8d8dda6a86de2fa0403e2158592268c2cf15eef4f3d887d98c90f1031d4735d5f4bf9dbc1d23
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 16, 2020
…ransaction.

6c4042a Assert that CWallet::SyncMetaData finds oldest transaction. (Eelis)

Pull request description:

  Without this assert, the Clang static analyzer warns about subsequent dereferencing of copyFrom, because it can't be sure that it's not nullptr. See bitcoin#9573.

Tree-SHA512: 83cbcb32c52c94fcfefbc90ec7de2011dacd6bdb0da35adc401b8d8dda6a86de2fa0403e2158592268c2cf15eef4f3d887d98c90f1031d4735d5f4bf9dbc1d23
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 22, 2020
…ransaction.

6c4042a Assert that CWallet::SyncMetaData finds oldest transaction. (Eelis)

Pull request description:

  Without this assert, the Clang static analyzer warns about subsequent dereferencing of copyFrom, because it can't be sure that it's not nullptr. See bitcoin#9573.

Tree-SHA512: 83cbcb32c52c94fcfefbc90ec7de2011dacd6bdb0da35adc401b8d8dda6a86de2fa0403e2158592268c2cf15eef4f3d887d98c90f1031d4735d5f4bf9dbc1d23
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 29, 2020
…ransaction.

6c4042a Assert that CWallet::SyncMetaData finds oldest transaction. (Eelis)

Pull request description:

  Without this assert, the Clang static analyzer warns about subsequent dereferencing of copyFrom, because it can't be sure that it's not nullptr. See bitcoin#9573.

Tree-SHA512: 83cbcb32c52c94fcfefbc90ec7de2011dacd6bdb0da35adc401b8d8dda6a86de2fa0403e2158592268c2cf15eef4f3d887d98c90f1031d4735d5f4bf9dbc1d23
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 29, 2020
…ransaction.

6c4042a Assert that CWallet::SyncMetaData finds oldest transaction. (Eelis)

Pull request description:

  Without this assert, the Clang static analyzer warns about subsequent dereferencing of copyFrom, because it can't be sure that it's not nullptr. See bitcoin#9573.

Tree-SHA512: 83cbcb32c52c94fcfefbc90ec7de2011dacd6bdb0da35adc401b8d8dda6a86de2fa0403e2158592268c2cf15eef4f3d887d98c90f1031d4735d5f4bf9dbc1d23
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request 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
ftrader pushed a commit to bitcoin-cash-node/bitcoin-cash-node that referenced this pull request 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
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants