diff options
-rw-r--r-- | doc/developer-notes.md | 4 | ||||
-rwxr-xr-x | qa/rpc-tests/pruning.py | 2 | ||||
-rwxr-xr-x | qa/rpc-tests/wallet.py | 7 | ||||
-rw-r--r-- | src/Makefile.test.include | 1 | ||||
-rw-r--r-- | src/net.cpp | 8 | ||||
-rw-r--r-- | src/net.h | 1 | ||||
-rw-r--r-- | src/test/net_tests.cpp | 136 | ||||
-rw-r--r-- | src/wallet/rpcwallet.cpp | 4 |
8 files changed, 155 insertions, 8 deletions
diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 8affb2158a..add2fb5004 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -5,7 +5,9 @@ Various coding styles have been used during the history of the codebase, and the result is not very consistent. However, we're now trying to converge to a single style, so please use it in new code. Old code will be converted gradually. -- Basic rules specified in src/.clang-format. Use a recent clang-format-3.5 to format automatically. +- Basic rules specified in [src/.clang-format](/src/.clang-format). + Use a recent clang-format to format automatically using one of the [dev scripts] + (/contrib/devtools/README.md#clang-formatpy). - Braces on new lines for namespaces, classes, functions, methods. - Braces on the same line for everything else. - 4 space indentation (no tabs) for every block except namespaces. diff --git a/qa/rpc-tests/pruning.py b/qa/rpc-tests/pruning.py index 92d33bd20e..eac2272db2 100755 --- a/qa/rpc-tests/pruning.py +++ b/qa/rpc-tests/pruning.py @@ -75,7 +75,7 @@ class PruneTest(BitcoinTestFramework): waitstart = time.time() while os.path.isfile(self.prunedir+"blk00000.dat"): time.sleep(0.1) - if time.time() - waitstart > 10: + if time.time() - waitstart > 30: raise AssertionError("blk00000.dat not pruned when it should be") print("Success") diff --git a/qa/rpc-tests/wallet.py b/qa/rpc-tests/wallet.py index 42ce0a7260..f321f5e90b 100755 --- a/qa/rpc-tests/wallet.py +++ b/qa/rpc-tests/wallet.py @@ -306,7 +306,7 @@ class WalletTest (BitcoinTestFramework): # Check that the txid and balance is found by node1 self.nodes[1].gettransaction(cbTxId) - #check if wallet or blochchain maintenance changes the balance + # check if wallet or blockchain maintenance changes the balance self.sync_all() blocks = self.nodes[0].generate(2) self.sync_all() @@ -318,7 +318,8 @@ class WalletTest (BitcoinTestFramework): '-reindex', '-zapwallettxes=1', '-zapwallettxes=2', - '-salvagewallet', + # disabled until issue is fixed: https://github.com/bitcoin/bitcoin/issues/7463 + # '-salvagewallet', ] for m in maintenance: print("check " + m) @@ -338,4 +339,4 @@ class WalletTest (BitcoinTestFramework): assert_equal(len(self.nodes[0].listsinceblock(blocks[1])["transactions"]), 0) if __name__ == '__main__': - WalletTest ().main () + WalletTest().main() diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 08e2f6af4d..897a7dd4a2 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -59,6 +59,7 @@ BITCOIN_TESTS =\ test/merkle_tests.cpp \ test/miner_tests.cpp \ test/multisig_tests.cpp \ + test/net_tests.cpp \ test/netbase_tests.cpp \ test/pmt_tests.cpp \ test/policyestimator_tests.cpp \ diff --git a/src/net.cpp b/src/net.cpp index aa5b47340a..abc492e2b9 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1950,6 +1950,7 @@ void StartNode(boost::thread_group& threadGroup, CScheduler& scheduler) if (adb.Read(addrman)) LogPrintf("Loaded %i addresses from peers.dat %dms\n", addrman.size(), GetTimeMillis() - nStart); else { + addrman.Clear(); // Addrman can be in an inconsistent state after failure, reset it LogPrintf("Invalid or missing peers.dat; recreating\n"); DumpAddresses(); } @@ -2320,6 +2321,11 @@ bool CAddrDB::Read(CAddrMan& addr) if (hashIn != hashTmp) return error("%s: Checksum mismatch, data corrupted", __func__); + return Read(addr, ssPeers); +} + +bool CAddrDB::Read(CAddrMan& addr, CDataStream& ssPeers) +{ unsigned char pchMsgTmp[4]; try { // de-serialize file header (network specific magic number) and .. @@ -2333,6 +2339,8 @@ bool CAddrDB::Read(CAddrMan& addr) ssPeers >> addr; } catch (const std::exception& e) { + // de-serialization has failed, ensure addrman is left in a clean state + addr.Clear(); return error("%s: Deserialize or I/O error - %s", __func__, e.what()); } @@ -794,6 +794,7 @@ public: CAddrDB(); bool Write(const CAddrMan& addr); bool Read(CAddrMan& addr); + bool Read(CAddrMan& addr, CDataStream& ssPeers); }; /** Access to the banlist database (banlist.dat) */ diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp new file mode 100644 index 0000000000..6debf6ac5e --- /dev/null +++ b/src/test/net_tests.cpp @@ -0,0 +1,136 @@ +// Copyright (c) 2012-2016 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include "addrman.h" +#include "test/test_bitcoin.h" +#include <string> +#include <boost/test/unit_test.hpp> +#include "hash.h" +#include "serialize.h" +#include "streams.h" +#include "net.h" +#include "chainparams.h" + +using namespace std; + +class CAddrManSerializationMock : public CAddrMan +{ +public: + virtual void Serialize(CDataStream& s, int nType, int nVersionDummy) const = 0; +}; + +class CAddrManUncorrupted : public CAddrManSerializationMock +{ +public: + void Serialize(CDataStream& s, int nType, int nVersionDummy) const + { + CAddrMan::Serialize(s, nType, nVersionDummy); + } +}; + +class CAddrManCorrupted : public CAddrManSerializationMock +{ +public: + void Serialize(CDataStream& s, int nType, int nVersionDummy) const + { + // Produces corrupt output that claims addrman has 20 addrs when it only has one addr. + unsigned char nVersion = 1; + s << nVersion; + s << ((unsigned char)32); + s << nKey; + s << 10; // nNew + s << 10; // nTried + + int nUBuckets = ADDRMAN_NEW_BUCKET_COUNT ^ (1 << 30); + s << nUBuckets; + + CAddress addr = CAddress(CService("252.1.1.1", 7777)); + CAddrInfo info = CAddrInfo(addr, CNetAddr("252.2.2.2")); + s << info; + } +}; + +CDataStream AddrmanToStream(CAddrManSerializationMock& addrman) +{ + CDataStream ssPeersIn(SER_DISK, CLIENT_VERSION); + ssPeersIn << FLATDATA(Params().MessageStart()); + ssPeersIn << addrman; + std::string str = ssPeersIn.str(); + vector<unsigned char> vchData(str.begin(), str.end()); + return CDataStream(vchData, SER_DISK, CLIENT_VERSION); +} + +BOOST_FIXTURE_TEST_SUITE(net_tests, BasicTestingSetup) + +BOOST_AUTO_TEST_CASE(caddrdb_read) +{ + CAddrManUncorrupted addrmanUncorrupted; + + CService addr1 = CService("250.7.1.1", 8333); + CService addr2 = CService("250.7.2.2", 9999); + CService addr3 = CService("250.7.3.3", 9999); + + // Add three addresses to new table. + addrmanUncorrupted.Add(CAddress(addr1), CService("252.5.1.1", 8333)); + addrmanUncorrupted.Add(CAddress(addr2), CService("252.5.1.1", 8333)); + addrmanUncorrupted.Add(CAddress(addr3), CService("252.5.1.1", 8333)); + + // Test that the de-serialization does not throw an exception. + CDataStream ssPeers1 = AddrmanToStream(addrmanUncorrupted); + bool exceptionThrown = false; + CAddrMan addrman1; + + BOOST_CHECK(addrman1.size() == 0); + try { + unsigned char pchMsgTmp[4]; + ssPeers1 >> FLATDATA(pchMsgTmp); + ssPeers1 >> addrman1; + } catch (const std::exception& e) { + exceptionThrown = true; + } + + BOOST_CHECK(addrman1.size() == 3); + BOOST_CHECK(exceptionThrown == false); + + // Test that CAddrDB::Read creates an addrman with the correct number of addrs. + CDataStream ssPeers2 = AddrmanToStream(addrmanUncorrupted); + + CAddrMan addrman2; + CAddrDB adb; + BOOST_CHECK(addrman2.size() == 0); + adb.Read(addrman2, ssPeers2); + BOOST_CHECK(addrman2.size() == 3); +} + + +BOOST_AUTO_TEST_CASE(caddrdb_read_corrupted) +{ + CAddrManCorrupted addrmanCorrupted; + + // Test that the de-serialization of corrupted addrman throws an exception. + CDataStream ssPeers1 = AddrmanToStream(addrmanCorrupted); + bool exceptionThrown = false; + CAddrMan addrman1; + BOOST_CHECK(addrman1.size() == 0); + try { + unsigned char pchMsgTmp[4]; + ssPeers1 >> FLATDATA(pchMsgTmp); + ssPeers1 >> addrman1; + } catch (const std::exception& e) { + exceptionThrown = true; + } + // Even through de-serialization failed adddrman is not left in a clean state. + BOOST_CHECK(addrman1.size() == 1); + BOOST_CHECK(exceptionThrown); + + // Test that CAddrDB::Read leaves addrman in a clean state if de-serialization fails. + CDataStream ssPeers2 = AddrmanToStream(addrmanCorrupted); + + CAddrMan addrman2; + CAddrDB adb; + BOOST_CHECK(addrman2.size() == 0); + adb.Read(addrman2, ssPeers2); + BOOST_CHECK(addrman2.size() == 0); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 623037e766..27596929f1 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2233,7 +2233,7 @@ UniValue settxfee(const UniValue& params, bool fHelp) "settxfee amount\n" "\nSet the transaction fee per kB. Overwrites the paytxfee parameter.\n" "\nArguments:\n" - "1. amount (numeric or sting, required) The transaction fee in " + CURRENCY_UNIT + "/kB\n" + "1. amount (numeric or string, required) The transaction fee in " + CURRENCY_UNIT + "/kB\n" "\nResult\n" "true|false (boolean) Returns true if successful\n" "\nExamples:\n" @@ -2328,8 +2328,6 @@ UniValue listunspent(const UniValue& params, bool fHelp) "\nReturns array of unspent transaction outputs\n" "with between minconf and maxconf (inclusive) confirmations.\n" "Optionally filter to only include txouts paid to specified addresses.\n" - "Results are an array of Objects, each of which has:\n" - "{txid, vout, scriptPubKey, amount, confirmations}\n" "\nArguments:\n" "1. minconf (numeric, optional, default=1) The minimum confirmations to filter\n" "2. maxconf (numeric, optional, default=9999999) The maximum confirmations to filter\n" |