diff options
| -rw-r--r-- | src/addrdb.cpp | 90 | ||||
| -rw-r--r-- | src/addrdb.h | 8 | ||||
| -rw-r--r-- | src/init.cpp | 14 | ||||
| -rw-r--r-- | src/test/addrman_tests.cpp | 4 | ||||
| -rw-r--r-- | src/test/fuzz/data_stream.cpp | 5 | ||||
| -rwxr-xr-x | test/functional/feature_addrman.py | 34 |
6 files changed, 88 insertions, 67 deletions
diff --git a/src/addrdb.cpp b/src/addrdb.cpp index 856f318961..1e73750ce3 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -18,8 +18,15 @@ #include <univalue.h> #include <util/settings.h> #include <util/system.h> +#include <util/translation.h> namespace { + +class DbNotFoundError : public std::exception +{ + using std::exception::exception; +}; + template <typename Stream, typename Data> bool SerializeDB(Stream& stream, const Data& data) { @@ -77,47 +84,40 @@ bool SerializeFileDB(const std::string& prefix, const fs::path& path, const Data } template <typename Stream, typename Data> -bool DeserializeDB(Stream& stream, Data& data, bool fCheckSum = true) +void DeserializeDB(Stream& stream, Data& data, bool fCheckSum = true) { - try { - CHashVerifier<Stream> verifier(&stream); - // de-serialize file header (network specific magic number) and .. - unsigned char pchMsgTmp[4]; - verifier >> pchMsgTmp; - // ... verify the network matches ours - if (memcmp(pchMsgTmp, Params().MessageStart(), sizeof(pchMsgTmp))) - return error("%s: Invalid network magic number", __func__); - - // de-serialize data - verifier >> data; - - // verify checksum - if (fCheckSum) { - uint256 hashTmp; - stream >> hashTmp; - if (hashTmp != verifier.GetHash()) { - return error("%s: Checksum mismatch, data corrupted", __func__); - } - } - } - catch (const std::exception& e) { - return error("%s: Deserialize or I/O error - %s", __func__, e.what()); + CHashVerifier<Stream> verifier(&stream); + // de-serialize file header (network specific magic number) and .. + unsigned char pchMsgTmp[4]; + verifier >> pchMsgTmp; + // ... verify the network matches ours + if (memcmp(pchMsgTmp, Params().MessageStart(), sizeof(pchMsgTmp))) { + throw std::runtime_error{"Invalid network magic number"}; } - return true; + // de-serialize data + verifier >> data; + + // verify checksum + if (fCheckSum) { + uint256 hashTmp; + stream >> hashTmp; + if (hashTmp != verifier.GetHash()) { + throw std::runtime_error{"Checksum mismatch, data corrupted"}; + } + } } template <typename Data> -bool DeserializeFileDB(const fs::path& path, Data& data, int version) +void DeserializeFileDB(const fs::path& path, Data& data, int version) { // open input file, and associate with CAutoFile FILE* file = fsbridge::fopen(path, "rb"); CAutoFile filein(file, SER_DISK, version); if (filein.IsNull()) { - LogPrintf("Missing or invalid file %s\n", path.string()); - return false; + throw DbNotFoundError{}; } - return DeserializeDB(filein, data); + DeserializeDB(filein, data); } } // namespace @@ -176,15 +176,32 @@ bool DumpPeerAddresses(const ArgsManager& args, const CAddrMan& addr) return SerializeFileDB("peers", pathAddr, addr, CLIENT_VERSION); } -bool ReadPeerAddresses(const ArgsManager& args, CAddrMan& addr) +void ReadFromStream(CAddrMan& addr, CDataStream& ssPeers) { - const auto pathAddr = args.GetDataDirNet() / "peers.dat"; - return DeserializeFileDB(pathAddr, addr, CLIENT_VERSION); + DeserializeDB(ssPeers, addr, false); } -bool ReadFromStream(CAddrMan& addr, CDataStream& ssPeers) +std::optional<bilingual_str> LoadAddrman(const std::vector<bool>& asmap, const ArgsManager& args, std::unique_ptr<CAddrMan>& addrman) { - return DeserializeDB(ssPeers, addr, false); + auto check_addrman = std::clamp<int32_t>(args.GetArg("-checkaddrman", DEFAULT_ADDRMAN_CONSISTENCY_CHECKS), 0, 1000000); + addrman = std::make_unique<CAddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman); + + int64_t nStart = GetTimeMillis(); + const auto path_addr{args.GetDataDirNet() / "peers.dat"}; + try { + DeserializeFileDB(path_addr, *addrman, CLIENT_VERSION); + LogPrintf("Loaded %i addresses from peers.dat %dms\n", addrman->size(), GetTimeMillis() - nStart); + } catch (const DbNotFoundError&) { + // Addrman can be in an inconsistent state after failure, reset it + addrman = std::make_unique<CAddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman); + LogPrintf("Creating peers.dat because the file was not found (%s)\n", path_addr); + DumpPeerAddresses(args, *addrman); + } catch (const std::exception& e) { + addrman = nullptr; + return strprintf(_("Invalid or corrupt peers.dat (%s). If you believe this is a bug, please report it to %s. As a workaround, you can move the file (%s) out of the way (rename, move, or delete) to have a new one created on the next start."), + e.what(), PACKAGE_BUGREPORT, path_addr); + } + return std::nullopt; } void DumpAnchors(const fs::path& anchors_db_path, const std::vector<CAddress>& anchors) @@ -196,9 +213,10 @@ void DumpAnchors(const fs::path& anchors_db_path, const std::vector<CAddress>& a std::vector<CAddress> ReadAnchors(const fs::path& anchors_db_path) { std::vector<CAddress> anchors; - if (DeserializeFileDB(anchors_db_path, anchors, CLIENT_VERSION | ADDRV2_FORMAT)) { + try { + DeserializeFileDB(anchors_db_path, anchors, CLIENT_VERSION | ADDRV2_FORMAT); LogPrintf("Loaded %i addresses from %s\n", anchors.size(), anchors_db_path.filename()); - } else { + } catch (const std::exception&) { anchors.clear(); } diff --git a/src/addrdb.h b/src/addrdb.h index c31c126ee3..33cc1f9204 100644 --- a/src/addrdb.h +++ b/src/addrdb.h @@ -10,17 +10,18 @@ #include <net_types.h> // For banmap_t #include <univalue.h> +#include <optional> #include <vector> class ArgsManager; class CAddrMan; class CAddress; class CDataStream; +struct bilingual_str; bool DumpPeerAddresses(const ArgsManager& args, const CAddrMan& addr); -bool ReadPeerAddresses(const ArgsManager& args, CAddrMan& addr); /** Only used by tests. */ -bool ReadFromStream(CAddrMan& addr, CDataStream& ssPeers); +void ReadFromStream(CAddrMan& addr, CDataStream& ssPeers); /** Access to the banlist database (banlist.json) */ class CBanDB @@ -46,6 +47,9 @@ public: bool Read(banmap_t& banSet); }; +/** Returns an error string on failure */ +std::optional<bilingual_str> LoadAddrman(const std::vector<bool>& asmap, const ArgsManager& args, std::unique_ptr<CAddrMan>& addrman); + /** * Dump the anchor IP address database (anchors.dat) * diff --git a/src/init.cpp b/src/init.cpp index 2753745843..d4ba441b0c 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1200,19 +1200,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) LogPrintf("Using /16 prefix for IP bucketing\n"); } - auto check_addrman = std::clamp<int32_t>(args.GetArg("-checkaddrman", DEFAULT_ADDRMAN_CONSISTENCY_CHECKS), 0, 1000000); - node.addrman = std::make_unique<CAddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman); - - // Load addresses from peers.dat uiInterface.InitMessage(_("Loading P2P addresses…").translated); - int64_t nStart = GetTimeMillis(); - if (ReadPeerAddresses(args, *node.addrman)) { - LogPrintf("Loaded %i addresses from peers.dat %dms\n", node.addrman->size(), GetTimeMillis() - nStart); - } else { - // Addrman can be in an inconsistent state after failure, reset it - node.addrman = std::make_unique<CAddrMan>(asmap, /* deterministic */ false, /* consistency_check_ratio */ check_addrman); - LogPrintf("Recreating peers.dat\n"); - DumpPeerAddresses(args, *node.addrman); + if (const auto error{LoadAddrman(asmap, args, node.addrman)}) { + return InitError(*error); } } diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index 835b18d42e..01a492a20b 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -1043,7 +1043,7 @@ BOOST_AUTO_TEST_CASE(load_addrman) CAddrMan addrman2(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 100); BOOST_CHECK(addrman2.size() == 0); - BOOST_CHECK(ReadFromStream(addrman2, ssPeers2)); + ReadFromStream(addrman2, ssPeers2); BOOST_CHECK(addrman2.size() == 3); } @@ -1073,7 +1073,7 @@ BOOST_AUTO_TEST_CASE(load_addrman_corrupted) CAddrMan addrman2(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 100); BOOST_CHECK(addrman2.size() == 0); - BOOST_CHECK(!ReadFromStream(addrman2, ssPeers2)); + BOOST_CHECK_THROW(ReadFromStream(addrman2, ssPeers2), std::ios_base::failure); } diff --git a/src/test/fuzz/data_stream.cpp b/src/test/fuzz/data_stream.cpp index 08e9c91ba3..323090e041 100644 --- a/src/test/fuzz/data_stream.cpp +++ b/src/test/fuzz/data_stream.cpp @@ -23,5 +23,8 @@ FUZZ_TARGET_INIT(data_stream_addr_man, initialize_data_stream_addr_man) FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()}; CDataStream data_stream = ConsumeDataStream(fuzzed_data_provider); CAddrMan addr_man(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 0); - ReadFromStream(addr_man, data_stream); + try { + ReadFromStream(addr_man, data_stream); + } catch (const std::exception&) { + } } diff --git a/test/functional/feature_addrman.py b/test/functional/feature_addrman.py index 8ccff340f0..ee421c89b5 100755 --- a/test/functional/feature_addrman.py +++ b/test/functional/feature_addrman.py @@ -10,6 +10,7 @@ import struct from test_framework.messages import ser_uint256, hash256 from test_framework.p2p import MAGIC_BYTES from test_framework.test_framework import BitcoinTestFramework +from test_framework.test_node import ErrorMatch from test_framework.util import assert_equal @@ -43,6 +44,12 @@ class AddrmanTest(BitcoinTestFramework): def run_test(self): peers_dat = os.path.join(self.nodes[0].datadir, self.chain, "peers.dat") + init_error = lambda reason: ( + f"Error: Invalid or corrupt peers.dat \\({reason}\\). If you believe this " + f"is a bug, please report it to {self.config['environment']['PACKAGE_BUGREPORT']}. " + f'As a workaround, you can move the file \\("{peers_dat}"\\) out of the way \\(rename, ' + "move, or delete\\) to have a new one created on the next start." + ) self.log.info("Check that mocked addrman is valid") self.stop_node(0) @@ -54,30 +61,29 @@ class AddrmanTest(BitcoinTestFramework): self.log.info("Check that addrman from future cannot be read") self.stop_node(0) write_addrman(peers_dat, lowest_compatible=111) - with self.nodes[0].assert_debug_log([ - f'ERROR: DeserializeDB: Deserialize or I/O error - Unsupported format of addrman database: 1. It is compatible with formats >=111, but the maximum supported by this version of {self.config["environment"]["PACKAGE_NAME"]} is 3.', - "Recreating peers.dat", - ]): - self.start_node(0) - assert_equal(self.nodes[0].getnodeaddresses(), []) + self.nodes[0].assert_start_raises_init_error( + expected_msg=init_error( + "Unsupported format of addrman database: 1. It is compatible with " + "formats >=111, but the maximum supported by this version of " + f"{self.config['environment']['PACKAGE_NAME']} is 3.: (.+)" + ), + match=ErrorMatch.FULL_REGEX, + ) self.log.info("Check that corrupt addrman cannot be read") self.stop_node(0) with open(peers_dat, "wb") as f: f.write(serialize_addrman()[:-1]) - with self.nodes[0].assert_debug_log([ - "ERROR: DeserializeDB: Deserialize or I/O error - CAutoFile::read: end of file", - "Recreating peers.dat", - ]): - self.start_node(0) - assert_equal(self.nodes[0].getnodeaddresses(), []) + self.nodes[0].assert_start_raises_init_error( + expected_msg=init_error("CAutoFile::read: end of file.*"), + match=ErrorMatch.FULL_REGEX, + ) self.log.info("Check that missing addrman is recreated") self.stop_node(0) os.remove(peers_dat) with self.nodes[0].assert_debug_log([ - f"Missing or invalid file {peers_dat}", - "Recreating peers.dat", + f'Creating peers.dat because the file was not found ("{peers_dat}")', ]): self.start_node(0) assert_equal(self.nodes[0].getnodeaddresses(), []) |