Skip to content

Commit 6853849

Browse files
laanwjPastaPastaPasta
authored andcommitted
Merge bitcoin#15710: wallet: Catch ios_base::failure specifically
7486e27 Tests: Unit test related to WalletDB ReadKeyValue (Bushstar) 32def8d Catch ios_base::failure specifically (Peter Bushnell) Pull request description: In bitcoin#2950 a hash of the pubkey and private was added to speed up key import, this was made backwards compatible by reading the hash in a try block with an ellipses catch all in case the hash was not present. CDataStream::read() specifically throws std::ios_base::failure, backwards compatibility expects only that error to be thrown, if something else gets thrown we should not be catching it. The change in this commit is to catch that exception only. If any other exception is thrown other than std::ios_base::failure it will be caught by the wider try block and an error written to the log and/or console. CDataStream::read() throwing std::ios_base::failure. https://github.com/bitcoin/bitcoin/blob/2c364fde423e74b4e03ebcff4582a9db7a6c4e4b/src/streams.h#L191 Wider catch statements that pick up all others exceptions other than ios_base::failure. https://github.com/bitcoin/bitcoin/blob/2c364fde423e74b4e03ebcff4582a9db7a6c4e4b/src/wallet/walletdb.cpp#L425 https://github.com/bitcoin/bitcoin/blob/2c364fde423e74b4e03ebcff4582a9db7a6c4e4b/src/wallet/walletdb.cpp#L430 ACKs for top commit: laanwj: Code review ACK 7486e27 Tree-SHA512: 5364bf935af8ec603bf5b8fef8c23b5cdaa4fe3506090cff988413221f2eaa99f7a91929afb42a35f8881ce2328744a0d32052da51ca0a5b2e65b6809e97f604
1 parent 7fbc0f3 commit 6853849

File tree

3 files changed

+31
-1
lines changed

3 files changed

+31
-1
lines changed

src/Makefile.test.include

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ BITCOIN_TESTS += \
182182
wallet/test/coinjoin_tests.cpp \
183183
wallet/test/psbt_wallet_tests.cpp \
184184
wallet/test/wallet_tests.cpp \
185+
wallet/test/walletdb_tests.cpp \
185186
wallet/test/wallet_crypto_tests.cpp \
186187
wallet/test/coinselector_tests.cpp \
187188
wallet/test/init_tests.cpp \

src/wallet/test/walletdb_tests.cpp

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Copyright (c) 2012-2019 The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#include <test/util/setup_common.h>
6+
#include <clientversion.h>
7+
#include <streams.h>
8+
#include <uint256.h>
9+
10+
#include <boost/test/unit_test.hpp>
11+
12+
BOOST_FIXTURE_TEST_SUITE(walletdb_tests, BasicTestingSetup)
13+
14+
BOOST_AUTO_TEST_CASE(walletdb_readkeyvalue)
15+
{
16+
/**
17+
* When ReadKeyValue() reads from either a "key" or "wkey" it first reads the CDataStream steam into a
18+
* CPrivKey or CWalletKey respectively and then reads a hash of the pubkey and privkey into a uint256.
19+
* Wallets from 0.8 or before do not store the pubkey/privkey hash, trying to read the hash from old
20+
* wallets throws an exception, for backwards compatibility this read is wrapped in a try block to
21+
* silently fail. The test here makes sure the type of exception thrown from CDataStream::read()
22+
* matches the type we expect, otherwise we need to update the "key"/"wkey" exception type caught.
23+
*/
24+
CDataStream ssValue(SER_DISK, CLIENT_VERSION);
25+
uint256 dummy;
26+
BOOST_CHECK_THROW(ssValue >> dummy, std::ios_base::failure);
27+
}
28+
29+
BOOST_AUTO_TEST_SUITE_END()

src/wallet/walletdb.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
327327
{
328328
ssValue >> hash;
329329
}
330-
catch (...) {}
330+
catch (const std::ios_base::failure&) {}
331331

332332
bool fSkipCheck = false;
333333

0 commit comments

Comments
 (0)