From abfd30311b9ca40e49bdc95cd837af5bf02485ea Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 4 May 2025 22:46:13 +0000 Subject: [PATCH 01/14] merge bitcoin#24469: Correctly decode UTF-8 literal string paths --- src/test/fs_tests.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/fs_tests.cpp b/src/test/fs_tests.cpp index 523404a73b720..c3f0d9f55e9f0 100644 --- a/src/test/fs_tests.cpp +++ b/src/test/fs_tests.cpp @@ -46,8 +46,8 @@ BOOST_AUTO_TEST_CASE(fsbridge_fstream) { fs::path tmpfolder = m_args.GetDataDirBase(); // tmpfile1 should be the same as tmpfile2 - fs::path tmpfile1 = tmpfolder / "fs_tests_∋_🏃"; - fs::path tmpfile2 = tmpfolder / "fs_tests_∋_🏃"; + fs::path tmpfile1 = tmpfolder / fs::u8path("fs_tests_∋_🏃"); + fs::path tmpfile2 = tmpfolder / fs::u8path("fs_tests_∋_🏃"); { std::ofstream file{tmpfile1}; file << "bitcoin"; @@ -101,7 +101,7 @@ BOOST_AUTO_TEST_CASE(fsbridge_fstream) } { // Join an absolute path and a relative path. - fs::path p = fsbridge::AbsPathJoin(tmpfolder, "fs_tests_∋_🏃"); + fs::path p = fsbridge::AbsPathJoin(tmpfolder, fs::u8path("fs_tests_∋_🏃")); BOOST_CHECK(p.is_absolute()); BOOST_CHECK_EQUAL(tmpfile1, p); } From ee63d4b802b35571716475621002456e1412c62f Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 4 May 2025 20:12:14 +0000 Subject: [PATCH 02/14] merge bitcoin#24470: Disallow more unsafe string->path conversions allowed by path append operators --- src/addrdb.cpp | 2 +- src/flat-database.h | 2 +- src/flatfile.cpp | 2 +- src/fs.h | 23 +++++++++++++-- src/index/blockfilterindex.cpp | 2 +- src/net.cpp | 2 +- src/qt/guiutil.cpp | 4 +-- src/test/util/chainstate.h | 2 +- src/test/util_tests.cpp | 6 ++-- src/util/getuniquepath.cpp | 2 +- src/util/system.cpp | 4 +-- src/util/system.h | 4 +-- src/validation.cpp | 4 +-- src/validation.h | 4 +-- src/wallet/bdb.cpp | 42 +++++++++++++++------------ src/wallet/bdb.h | 14 ++++----- src/wallet/test/db_tests.cpp | 16 +++++----- src/wallet/test/init_test_fixture.cpp | 7 ++--- src/wallet/wallet.cpp | 4 +-- 19 files changed, 85 insertions(+), 61 deletions(-) diff --git a/src/addrdb.cpp b/src/addrdb.cpp index 51636eafaf02e..315987977f147 100644 --- a/src/addrdb.cpp +++ b/src/addrdb.cpp @@ -54,7 +54,7 @@ bool SerializeFileDB(const std::string& prefix, const fs::path& path, const Data std::string tmpfn = strprintf("%s.%04x", prefix, randv); // open temp output file, and associate with CAutoFile - fs::path pathTmp = gArgs.GetDataDirNet() / tmpfn; + fs::path pathTmp = gArgs.GetDataDirNet() / fs::u8path(tmpfn); FILE *file = fsbridge::fopen(pathTmp, "wb"); CAutoFile fileout(file, SER_DISK, version); if (fileout.IsNull()) { diff --git a/src/flat-database.h b/src/flat-database.h index 7e921f8b96772..75fd91225ac98 100644 --- a/src/flat-database.h +++ b/src/flat-database.h @@ -175,7 +175,7 @@ class CFlatDB public: CFlatDB(std::string&& strFilenameIn, std::string&& strMagicMessageIn) : - pathDB{gArgs.GetDataDirNet() / strFilenameIn}, + pathDB{gArgs.GetDataDirNet() / fs::u8path(strFilenameIn)}, strFilename{strFilenameIn}, strMagicMessage{strMagicMessageIn} { diff --git a/src/flatfile.cpp b/src/flatfile.cpp index d6cada0c4652e..0fecf4f504998 100644 --- a/src/flatfile.cpp +++ b/src/flatfile.cpp @@ -27,7 +27,7 @@ std::string FlatFilePos::ToString() const fs::path FlatFileSeq::FileName(const FlatFilePos& pos) const { - return m_dir / strprintf("%s%05u.dat", m_prefix, pos.nFile); + return m_dir / fs::u8path(strprintf("%s%05u.dat", m_prefix, pos.nFile)); } FILE* FlatFileSeq::Open(const FlatFilePos& pos, bool read_only) diff --git a/src/fs.h b/src/fs.h index f28f0269c360c..4d3f775bd61e2 100644 --- a/src/fs.h +++ b/src/fs.h @@ -96,11 +96,30 @@ static inline auto quoted(const std::string& s) } // Allow safe path append operations. -static inline path operator+(path p1, path p2) +static inline path operator/(path p1, path p2) { - p1 += std::move(p2); + p1 /= std::move(p2); return p1; } +static inline path operator/(path p1, const char* p2) +{ + p1 /= p2; + return p1; +} +static inline path operator+(path p1, const char* p2) +{ + p1 += p2; + return p1; +} +static inline path operator+(path p1, path::value_type p2) +{ + p1 += p2; + return p1; +} + +// Disallow unsafe path append operations. +template static inline path operator/(path p1, T p2) = delete; +template static inline path operator+(path p1, T p2) = delete; // Disallow implicit std::string conversion for copy_file // to avoid locale-dependent encoding on Windows. diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp index fd7c80248e760..5af98dae49bd4 100644 --- a/src/index/blockfilterindex.cpp +++ b/src/index/blockfilterindex.cpp @@ -104,7 +104,7 @@ BlockFilterIndex::BlockFilterIndex(BlockFilterType filter_type, const std::string& filter_name = BlockFilterTypeName(filter_type); if (filter_name.empty()) throw std::invalid_argument("unknown filter_type"); - fs::path path = gArgs.GetDataDirNet() / "indexes" / "blockfilter" / filter_name; + fs::path path = gArgs.GetDataDirNet() / "indexes" / "blockfilter" / fs::u8path(filter_name); fs::create_directories(path); m_name = filter_name + " block filter index"; diff --git a/src/net.cpp b/src/net.cpp index 7e14fe7777a2b..4b759ecd3469f 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -5125,7 +5125,7 @@ static void CaptureMessageToFile(const CAddress& addr, std::string clean_addr = addr.ToStringAddrPort(); std::replace(clean_addr.begin(), clean_addr.end(), ':', '_'); - fs::path base_path = gArgs.GetDataDirNet() / "message_capture" / clean_addr; + fs::path base_path = gArgs.GetDataDirNet() / "message_capture" / fs::u8path(clean_addr); fs::create_directories(base_path); fs::path path = base_path / (is_incoming ? "msgs_recv.dat" : "msgs_sent.dat"); diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index e91f97ce13d6c..c2c1c0a23f256 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -733,7 +733,7 @@ fs::path static StartupShortcutPath() return GetSpecialFolderPath(CSIDL_STARTUP) / "Dash Core.lnk"; if (chain == CBaseChainParams::TESTNET) // Remove this special case when CBaseChainParams::TESTNET = "testnet4" return GetSpecialFolderPath(CSIDL_STARTUP) / "Dash Core (testnet).lnk"; - return GetSpecialFolderPath(CSIDL_STARTUP) / strprintf("Dash Core (%s).lnk", chain); + return GetSpecialFolderPath(CSIDL_STARTUP) / fs::u8path(strprintf("Dash Core (%s).lnk", chain)); } bool GetStartOnSystemStartup() @@ -814,7 +814,7 @@ fs::path static GetAutostartFilePath() std::string chain = gArgs.GetChainName(); if (chain == CBaseChainParams::MAIN) return GetAutostartDir() / "dashcore.desktop"; - return GetAutostartDir() / strprintf("dashcore-%s.desktop", chain); + return GetAutostartDir() / fs::u8path(strprintf("dashcore-%s.desktop", chain)); } bool GetStartOnSystemStartup() diff --git a/src/test/util/chainstate.h b/src/test/util/chainstate.h index ed6680534f0bc..3b6b504ef8b9d 100644 --- a/src/test/util/chainstate.h +++ b/src/test/util/chainstate.h @@ -29,7 +29,7 @@ CreateAndActivateUTXOSnapshot(NodeContext& node, const fs::path root, F malleati // int height; WITH_LOCK(::cs_main, height = node.chainman->ActiveHeight()); - fs::path snapshot_path = root / tfm::format("test_snapshot.%d.dat", height); + fs::path snapshot_path = root / fs::u8path(tfm::format("test_snapshot.%d.dat", height)); FILE* outfile{fsbridge::fopen(snapshot_path, "wb")}; CAutoFile auto_outfile{outfile, SER_DISK, CLIENT_VERSION}; diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index b7bfb880da9b1..31227c82bae7c 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1092,7 +1092,7 @@ BOOST_AUTO_TEST_CASE(test_ParseFixedPoint) BOOST_CHECK(!ParseFixedPoint("31.999999999999999999999", 3, &amount)); } -static void TestOtherThread(fs::path dirname, std::string lockname, bool *result) +static void TestOtherThread(fs::path dirname, fs::path lockname, bool *result) { *result = LockDirectory(dirname, lockname); } @@ -1102,7 +1102,7 @@ static constexpr char LockCommand = 'L'; static constexpr char UnlockCommand = 'U'; static constexpr char ExitCommand = 'X'; -[[noreturn]] static void TestOtherProcess(fs::path dirname, std::string lockname, int fd) +[[noreturn]] static void TestOtherProcess(fs::path dirname, fs::path lockname, int fd) { char ch; while (true) { @@ -1133,7 +1133,7 @@ static constexpr char ExitCommand = 'X'; BOOST_AUTO_TEST_CASE(test_LockDirectory) { fs::path dirname = m_args.GetDataDirBase() / "lock_dir"; - const std::string lockname = ".lock"; + const fs::path lockname = ".lock"; #ifndef WIN32 // Revert SIGCHLD to default, otherwise boost.test will catch and fail on // it: there is BOOST_TEST_IGNORE_SIGCHLD but that only works when defined diff --git a/src/util/getuniquepath.cpp b/src/util/getuniquepath.cpp index 6776e7785b272..1d8e511c83503 100644 --- a/src/util/getuniquepath.cpp +++ b/src/util/getuniquepath.cpp @@ -9,6 +9,6 @@ fs::path GetUniquePath(const fs::path& base) { FastRandomContext rnd; - fs::path tmpFile = base / HexStr(rnd.randbytes(8)); + fs::path tmpFile = base / fs::u8path(HexStr(rnd.randbytes(8))); return tmpFile; } \ No newline at end of file diff --git a/src/util/system.cpp b/src/util/system.cpp index 820fbd55c09e3..ae260298099f7 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -107,7 +107,7 @@ static Mutex cs_dir_locks; */ static std::map> dir_locks GUARDED_BY(cs_dir_locks); -bool LockDirectory(const fs::path& directory, const std::string lockfile_name, bool probe_only) +bool LockDirectory(const fs::path& directory, const fs::path& lockfile_name, bool probe_only) { LOCK(cs_dir_locks); fs::path pathLockFile = directory / lockfile_name; @@ -131,7 +131,7 @@ bool LockDirectory(const fs::path& directory, const std::string lockfile_name, b return true; } -void UnlockDirectory(const fs::path& directory, const std::string& lockfile_name) +void UnlockDirectory(const fs::path& directory, const fs::path& lockfile_name) { LOCK(cs_dir_locks); dir_locks.erase(fs::PathToString(directory / lockfile_name)); diff --git a/src/util/system.h b/src/util/system.h index c0e6ba94746e8..7d53c46737682 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -82,8 +82,8 @@ void AllocateFileRange(FILE *file, unsigned int offset, unsigned int length); */ [[nodiscard]] bool RenameOver(fs::path src, fs::path dest); -bool LockDirectory(const fs::path& directory, const std::string lockfile_name, bool probe_only=false); -void UnlockDirectory(const fs::path& directory, const std::string& lockfile_name); +bool LockDirectory(const fs::path& directory, const fs::path& lockfile_name, bool probe_only=false); +void UnlockDirectory(const fs::path& directory, const fs::path& lockfile_name); bool DirIsWritable(const fs::path& directory); bool CheckDiskSpace(const fs::path& dir, uint64_t additional_bytes = 0); diff --git a/src/validation.cpp b/src/validation.cpp index c584f55c5bdc5..a00fc23c1b7c8 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1503,7 +1503,7 @@ CAmount GetMasternodePayment(int nHeight, CAmount blockValue, bool fV20Active) } CoinsViews::CoinsViews( - std::string ldb_name, + fs::path ldb_name, size_t cache_size_bytes, bool in_memory, bool should_wipe) : m_dbview( @@ -1534,7 +1534,7 @@ void CChainState::InitCoinsDB( size_t cache_size_bytes, bool in_memory, bool should_wipe, - std::string leveldb_name) + fs::path leveldb_name) { if (m_from_snapshot_blockhash) { leveldb_name += "_" + m_from_snapshot_blockhash->ToString(); diff --git a/src/validation.h b/src/validation.h index de466aab6ec90..7edf390eccb9b 100644 --- a/src/validation.h +++ b/src/validation.h @@ -415,7 +415,7 @@ class CoinsViews { //! state to disk, which should not be done until the health of the database is verified. //! //! All arguments forwarded onto CCoinsViewDB. - CoinsViews(std::string ldb_name, size_t cache_size_bytes, bool in_memory, bool should_wipe); + CoinsViews(fs::path ldb_name, size_t cache_size_bytes, bool in_memory, bool should_wipe); //! Initialize the CCoinsViewCache member. void InitCache() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); @@ -514,7 +514,7 @@ class CChainState size_t cache_size_bytes, bool in_memory, bool should_wipe, - std::string leveldb_name = "chainstate"); + fs::path leveldb_name = "chainstate"); //! Initialize the in-memory coins cache (to be done after the health of the on-disk database //! is verified). diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 6b35239a5d00b..4c89f05d237c0 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -271,7 +271,7 @@ BerkeleyBatch::SafeDbt::operator Dbt*() bool BerkeleyDatabase::Verify(bilingual_str& errorStr) { fs::path walletDir = env->Directory(); - fs::path file_path = walletDir / strFile; + fs::path file_path = walletDir / m_filename; LogPrintf("Using BerkeleyDB version %s\n", BerkeleyDatabaseVersion()); LogPrintf("Using wallet %s\n", fs::PathToString(file_path)); @@ -285,6 +285,7 @@ bool BerkeleyDatabase::Verify(bilingual_str& errorStr) assert(m_refcount == 0); Db db(env->dbenv.get(), 0); + const std::string strFile = fs::PathToString(m_filename); int result = db.verify(strFile.c_str(), nullptr, nullptr, 0); if (result != 0) { errorStr = strprintf(_("%s corrupt. Try using the wallet tool dash-wallet to salvage or restoring a backup."), fs::quoted(fs::PathToString(file_path))); @@ -307,11 +308,11 @@ BerkeleyDatabase::~BerkeleyDatabase() { if (env) { LOCK(cs_db); - env->CloseDb(strFile); + env->CloseDb(m_filename); assert(!m_db); - size_t erased = env->m_databases.erase(strFile); + size_t erased = env->m_databases.erase(m_filename); assert(erased == 1); - env->m_fileids.erase(strFile); + env->m_fileids.erase(fs::PathToString(m_filename)); } } @@ -323,7 +324,7 @@ BerkeleyBatch::BerkeleyBatch(BerkeleyDatabase& database, const bool read_only, b fFlushOnClose = fFlushOnCloseIn; env = database.env.get(); pdb = database.m_db.get(); - strFile = database.strFile; + strFile = fs::PathToString(database.m_filename); } void BerkeleyDatabase::Open() @@ -339,6 +340,7 @@ void BerkeleyDatabase::Open() if (m_db == nullptr) { int ret; std::unique_ptr pdb_temp = std::make_unique(env->dbenv.get(), 0); + const std::string strFile = fs::PathToString(m_filename); bool fMockDb = env->IsMock(); if (fMockDb) { @@ -411,11 +413,11 @@ void BerkeleyBatch::Close() Flush(); } -void BerkeleyEnvironment::CloseDb(const std::string& strFile) +void BerkeleyEnvironment::CloseDb(const fs::path& filename) { { LOCK(cs_db); - auto it = m_databases.find(strFile); + auto it = m_databases.find(filename); assert(it != m_databases.end()); BerkeleyDatabase& database = it->second.get(); if (database.m_db) { @@ -438,12 +440,12 @@ void BerkeleyEnvironment::ReloadDbEnv() return true; }); - std::vector filenames; + std::vector filenames; for (auto it : m_databases) { filenames.push_back(it.first); } // Close the individual Db's - for (const std::string& filename : filenames) { + for (const fs::path& filename : filenames) { CloseDb(filename); } // Reset the environment @@ -458,9 +460,10 @@ bool BerkeleyDatabase::Rewrite(const char* pszSkip) while (true) { { LOCK(cs_db); + const std::string strFile = fs::PathToString(m_filename); if (m_refcount <= 0) { // Flush log data to the dat file - env->CloseDb(strFile); + env->CloseDb(m_filename); env->CheckpointLSN(strFile); m_refcount = -1; @@ -512,7 +515,7 @@ bool BerkeleyDatabase::Rewrite(const char* pszSkip) } if (fSuccess) { db.Close(); - env->CloseDb(strFile); + env->CloseDb(m_filename); if (pdbCopy->close(0)) fSuccess = false; } else { @@ -548,13 +551,14 @@ void BerkeleyEnvironment::Flush(bool fShutdown) LOCK(cs_db); bool no_dbs_accessed = true; for (auto& db_it : m_databases) { - std::string strFile = db_it.first; + const fs::path& filename = db_it.first; int nRefCount = db_it.second.get().m_refcount; if (nRefCount < 0) continue; + const std::string strFile = fs::PathToString(filename); LogPrint(BCLog::WALLETDB, "BerkeleyEnvironment::Flush: Flushing %s (refcount = %d)...\n", strFile, nRefCount); if (nRefCount == 0) { // Move log data to the dat file - CloseDb(strFile); + CloseDb(filename); LogPrint(BCLog::WALLETDB, "BerkeleyEnvironment::Flush: %s checkpoint\n", strFile); dbenv->txn_checkpoint(0, 0, 0); LogPrint(BCLog::WALLETDB, "BerkeleyEnvironment::Flush: %s detach\n", strFile); @@ -594,11 +598,12 @@ bool BerkeleyDatabase::PeriodicFlush() // Don't flush if there haven't been any batch writes for this database. if (m_refcount < 0) return false; + const std::string strFile = fs::PathToString(m_filename); LogPrint(BCLog::WALLETDB, "Flushing %s\n", strFile); const auto start{SteadyClock::now()}; // Flush wallet file so it's self contained - env->CloseDb(strFile); + env->CloseDb(m_filename); env->CheckpointLSN(strFile); m_refcount = -1; @@ -609,6 +614,7 @@ bool BerkeleyDatabase::PeriodicFlush() bool BerkeleyDatabase::Backup(const std::string& strDest) const { + const std::string strFile = fs::PathToString(m_filename); while (true) { { @@ -616,14 +622,14 @@ bool BerkeleyDatabase::Backup(const std::string& strDest) const if (m_refcount <= 0) { // Flush log data to the dat file - env->CloseDb(strFile); + env->CloseDb(m_filename); env->CheckpointLSN(strFile); // Copy wallet file - fs::path pathSrc = env->Directory() / strFile; + fs::path pathSrc = env->Directory() / m_filename; fs::path pathDest(fs::PathFromString(strDest)); if (fs::is_directory(pathDest)) - pathDest /= fs::PathFromString(strFile); + pathDest /= m_filename; try { if (fs::exists(pathDest) && fs::equivalent(pathSrc, pathDest)) { @@ -836,7 +842,7 @@ std::unique_ptr MakeBerkeleyDatabase(const fs::path& path, con std::unique_ptr db; { LOCK(cs_db); // Lock env.m_databases until insert in BerkeleyDatabase constructor - std::string data_filename = fs::PathToString(data_file.filename()); + fs::path data_filename = data_file.filename(); std::shared_ptr env = GetBerkeleyEnv(data_file.parent_path()); if (env->m_databases.count(data_filename)) { error = Untranslated(strprintf("Refusing to load database. Data file '%s' is already loaded.", fs::PathToString(env->Directory() / data_filename))); diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index b6865407553ea..6eb699452c685 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -51,7 +51,7 @@ class BerkeleyEnvironment public: std::unique_ptr dbenv; - std::map> m_databases; + std::map> m_databases; std::unordered_map m_fileids; std::condition_variable_any m_db_in_use; @@ -69,7 +69,7 @@ class BerkeleyEnvironment void Flush(bool fShutdown); void CheckpointLSN(const std::string& strFile); - void CloseDb(const std::string& strFile); + void CloseDb(const fs::path& filename); void ReloadDbEnv(); DbTxn* TxnBegin(int flags = DB_TXN_WRITE_NOSYNC) @@ -96,10 +96,10 @@ class BerkeleyDatabase : public WalletDatabase BerkeleyDatabase() = delete; /** Create DB handle to real database */ - BerkeleyDatabase(std::shared_ptr env, std::string filename) : - WalletDatabase(), env(std::move(env)), strFile(std::move(filename)) + BerkeleyDatabase(std::shared_ptr env, fs::path filename) : + WalletDatabase(), env(std::move(env)), m_filename(std::move(filename)) { - auto inserted = this->env->m_databases.emplace(strFile, std::ref(*this)); + auto inserted = this->env->m_databases.emplace(m_filename, std::ref(*this)); assert(inserted.second); } @@ -140,7 +140,7 @@ class BerkeleyDatabase : public WalletDatabase bool Verify(bilingual_str& error); /** Return path to main database filename */ - std::string Filename() override { return fs::PathToString(env->Directory() / strFile); } + std::string Filename() override { return fs::PathToString(env->Directory() / m_filename); } std::string Format() override { return "bdb"; } /** @@ -157,7 +157,7 @@ class BerkeleyDatabase : public WalletDatabase /** Database pointer. This is initialized lazily and reset during flushes, so it can be null. */ std::unique_ptr m_db; - std::string strFile; + fs::path m_filename; /** Make a BerkeleyBatch connected to this database */ std::unique_ptr MakeBatch(bool flush_on_close = true) override; diff --git a/src/wallet/test/db_tests.cpp b/src/wallet/test/db_tests.cpp index 515a26a7a41a5..21385dc24a2fd 100644 --- a/src/wallet/test/db_tests.cpp +++ b/src/wallet/test/db_tests.cpp @@ -14,22 +14,22 @@ BOOST_FIXTURE_TEST_SUITE(db_tests, BasicTestingSetup) -static std::shared_ptr GetWalletEnv(const fs::path& path, std::string& database_filename) +static std::shared_ptr GetWalletEnv(const fs::path& path, fs::path& database_filename) { fs::path data_file = BDBDataFile(path); - database_filename = fs::PathToString(data_file.filename()); + database_filename = data_file.filename(); return GetBerkeleyEnv(data_file.parent_path()); } BOOST_AUTO_TEST_CASE(getwalletenv_file) { - std::string test_name = "test_name.dat"; + fs::path test_name = "test_name.dat"; const fs::path datadir = gArgs.GetDataDirNet(); fs::path file_path = datadir / test_name; std::ofstream f{file_path}; f.close(); - std::string filename; + fs::path filename; std::shared_ptr env = GetWalletEnv(file_path, filename); BOOST_CHECK_EQUAL(filename, test_name); BOOST_CHECK_EQUAL(env->Directory(), datadir); @@ -37,10 +37,10 @@ BOOST_AUTO_TEST_CASE(getwalletenv_file) BOOST_AUTO_TEST_CASE(getwalletenv_directory) { - std::string expected_name = "wallet.dat"; + fs::path expected_name = "wallet.dat"; const fs::path datadir = gArgs.GetDataDirNet(); - std::string filename; + fs::path filename; std::shared_ptr env = GetWalletEnv(datadir, filename); BOOST_CHECK_EQUAL(filename, expected_name); BOOST_CHECK_EQUAL(env->Directory(), datadir); @@ -50,7 +50,7 @@ BOOST_AUTO_TEST_CASE(getwalletenv_g_dbenvs_multiple) { fs::path datadir = gArgs.GetDataDirNet() / "1"; fs::path datadir_2 = gArgs.GetDataDirNet() / "2"; - std::string filename; + fs::path filename; std::shared_ptr env_1 = GetWalletEnv(datadir, filename); std::shared_ptr env_2 = GetWalletEnv(datadir, filename); @@ -64,7 +64,7 @@ BOOST_AUTO_TEST_CASE(getwalletenv_g_dbenvs_free_instance) { fs::path datadir = gArgs.GetDataDirNet() / "1"; fs::path datadir_2 = gArgs.GetDataDirNet() / "2"; - std::string filename; + fs::path filename; std::shared_ptr env_1_a = GetWalletEnv(datadir, filename); std::shared_ptr env_2_a = GetWalletEnv(datadir_2, filename); diff --git a/src/wallet/test/init_test_fixture.cpp b/src/wallet/test/init_test_fixture.cpp index 4b585690801ef..1e3300b2ff376 100644 --- a/src/wallet/test/init_test_fixture.cpp +++ b/src/wallet/test/init_test_fixture.cpp @@ -17,8 +17,7 @@ InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainNam m_coinjoin_loader = interfaces::MakeCoinJoinLoader(m_node); m_wallet_loader = MakeWalletLoader(*m_node.chain, *Assert(m_node.args), m_node, *Assert(m_coinjoin_loader)); - std::string sep; - sep += fs::path::preferred_separator; + const auto sep = fs::path::preferred_separator; m_datadir = gArgs.GetDataDirNet(); m_cwd = fs::current_path(); @@ -27,8 +26,8 @@ InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainNam m_walletdir_path_cases["custom"] = m_datadir / "my_wallets"; m_walletdir_path_cases["nonexistent"] = m_datadir / "path_does_not_exist"; m_walletdir_path_cases["file"] = m_datadir / "not_a_directory.dat"; - m_walletdir_path_cases["trailing"] = m_datadir / ("wallets" + sep); - m_walletdir_path_cases["trailing2"] = m_datadir / ("wallets" + sep + sep); + m_walletdir_path_cases["trailing"] = (m_datadir / "wallets") + sep; + m_walletdir_path_cases["trailing2"] = (m_datadir / "wallets") + sep + sep; fs::current_path(m_datadir); m_walletdir_path_cases["relative"] = "wallets"; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 8243fa19923f9..d009af3e3ea79 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3275,7 +3275,7 @@ bool CWallet::AutoBackupWallet(const fs::path& wallet_path, bilingual_str& error if (wallet_path.empty()) { // ... opened wallet LOCK(cs_wallet); - fs::path backupFile = backupsDir / (strWalletName + dateTimeStr); + fs::path backupFile = backupsDir / fs::u8path(strWalletName + dateTimeStr); backupFile.make_preferred(); if (!BackupWallet(fs::PathToString(backupFile))) { warnings.push_back(strprintf(_("Failed to create backup %s!"), fs::PathToString(backupFile))); @@ -3298,7 +3298,7 @@ bool CWallet::AutoBackupWallet(const fs::path& wallet_path, bilingual_str& error fs::path strSourceFile = BDBDataFile(wallet_path); std::shared_ptr env = GetBerkeleyEnv(strSourceFile.parent_path()); fs::path sourceFile = env->Directory() / strSourceFile; - fs::path backupFile = backupsDir / (strWalletName + dateTimeStr); + fs::path backupFile = backupsDir / fs::u8path(strWalletName + dateTimeStr); sourceFile.make_preferred(); backupFile.make_preferred(); if (fs::exists(backupFile)) From f29610b14dab466cf7cd3c3243b38af359bfa9c0 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 17 Jan 2023 12:56:02 +0000 Subject: [PATCH 03/14] partial bitcoin#26707: Fix `performance-*move*` warnings in headers includes: - 0a5dc030b92a78147787f158d6a5de234ffa8ba4 --- src/fs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fs.h b/src/fs.h index 4d3f775bd61e2..0444bd82c5dbe 100644 --- a/src/fs.h +++ b/src/fs.h @@ -34,7 +34,7 @@ class path : public std::filesystem::path // Allow path objects arguments for compatibility. path(std::filesystem::path path) : std::filesystem::path::path(std::move(path)) {} path& operator=(std::filesystem::path path) { std::filesystem::path::operator=(std::move(path)); return *this; } - path& operator/=(std::filesystem::path path) { std::filesystem::path::operator/=(std::move(path)); return *this; } + path& operator/=(std::filesystem::path path) { std::filesystem::path::operator/=(path); return *this; } // Allow literal string arguments, which are safe as long as the literals are ASCII. path(const char* c) : std::filesystem::path(c) {} From e2eb625fb8b00bed317b6e6e8ae1ea21554993e0 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 16 Nov 2023 11:15:08 -0300 Subject: [PATCH 04/14] partial bitcoin#28894: batch all individual spkms setup db writes in a single db txn includes: - bb4554c81e0d819d74996f89cbb9c00476aedf8c --- src/Makefile.bench.include | 1 + src/bench/wallet_create.cpp | 56 +++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 src/bench/wallet_create.cpp diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index 522597710984d..3aca46268b34b 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -83,6 +83,7 @@ endif if ENABLE_WALLET bench_bench_dash_SOURCES += bench/coin_selection.cpp bench_bench_dash_SOURCES += bench/wallet_balance.cpp +bench_bench_dash_SOURCES += bench/wallet_create.cpp endif bench_bench_dash_LDADD += $(BACKTRACE_LIB) $(BDB_LIBS) $(EVENT_PTHREADS_LIBS) $(EVENT_LIBS) $(MINIUPNPC_LIBS) $(NATPMP_LIBS) $(SQLITE_LIBS) $(GMP_LIBS) diff --git a/src/bench/wallet_create.cpp b/src/bench/wallet_create.cpp new file mode 100644 index 0000000000000..4aea631c75979 --- /dev/null +++ b/src/bench/wallet_create.cpp @@ -0,0 +1,56 @@ +// Copyright (c) 2023-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://www.opensource.org/licenses/mit-license.php. + +#include +#include +#include +#include +#include +#include +#include + +namespace wallet { +static void WalletCreate(benchmark::Bench& bench, bool encrypted) +{ + auto test_setup = MakeNoLogFileContext(); + FastRandomContext random; + + WalletContext context; + context.args = &test_setup->m_args; + context.chain = test_setup->m_node.chain.get(); + + DatabaseOptions options; + options.require_format = DatabaseFormat::SQLITE; + options.require_create = true; + options.create_flags = WALLET_FLAG_DESCRIPTORS; + + if (encrypted) { + options.create_passphrase = random.rand256().ToString(); + } + + DatabaseStatus status; + bilingual_str error_string; + std::vector warnings; + + fs::path wallet_path = test_setup->m_path_root / strprintf("test_wallet_%d", random.rand32()).c_str(); + bench.run([&] { + auto wallet = CreateWallet(context, wallet_path.u8string(), /*load_on_start=*/std::nullopt, options, status, error_string, warnings); + assert(status == DatabaseStatus::SUCCESS); + assert(wallet != nullptr); + + // Cleanup + wallet.reset(); + fs::remove_all(wallet_path); + }); +} + +static void WalletCreatePlain(benchmark::Bench& bench) { WalletCreate(bench, /*encrypted=*/false); } +static void WalletCreateEncrypted(benchmark::Bench& bench) { WalletCreate(bench, /*encrypted=*/true); } + +#ifdef USE_SQLITE +BENCHMARK(WalletCreatePlain); +BENCHMARK(WalletCreateEncrypted); +#endif + +} // namespace wallet From 209488d58c5260cfdb1b628cfe43678cdd4a5499 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 5 May 2025 00:12:08 +0000 Subject: [PATCH 05/14] merge bitcoin#29040: Remove pre-C++20 code, fs::path cleanup --- doc/developer-notes.md | 2 +- src/bench/wallet_create.cpp | 2 +- src/fs.cpp | 3 ++- src/fs.h | 33 +++++++++++++++++---------------- src/qt/guiutil.cpp | 5 ++--- src/rpc/blockchain.cpp | 6 +++--- src/rpc/mempool.cpp | 2 +- src/rpc/server.cpp | 2 +- src/test/fs_tests.cpp | 9 ++++++--- src/util/system.cpp | 4 ++-- src/util/system.h | 8 ++++---- src/wallet/rpcdump.cpp | 4 ++-- src/wallet/rpcwallet.cpp | 2 +- 13 files changed, 43 insertions(+), 39 deletions(-) diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 5b252b659f80d..c05c87e9c4896 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -1408,7 +1408,7 @@ A few guidelines for introducing and reviewing new RPC interfaces: - *Rationale*: User-facing consistency. -- Use `fs::path::u8string()` and `fs::u8path()` functions when converting path +- Use `fs::path::u8string()`/`fs::path::utf8string()` and `fs::u8path()` functions when converting path to JSON strings, not `fs::PathToString` and `fs::PathFromString` - *Rationale*: JSON strings are Unicode strings, not byte strings, and diff --git a/src/bench/wallet_create.cpp b/src/bench/wallet_create.cpp index 4aea631c75979..046dc29a804fe 100644 --- a/src/bench/wallet_create.cpp +++ b/src/bench/wallet_create.cpp @@ -35,7 +35,7 @@ static void WalletCreate(benchmark::Bench& bench, bool encrypted) fs::path wallet_path = test_setup->m_path_root / strprintf("test_wallet_%d", random.rand32()).c_str(); bench.run([&] { - auto wallet = CreateWallet(context, wallet_path.u8string(), /*load_on_start=*/std::nullopt, options, status, error_string, warnings); + auto wallet = CreateWallet(context, wallet_path.utf8string(), /*load_on_start=*/std::nullopt, options, status, error_string, warnings); assert(status == DatabaseStatus::SUCCESS); assert(wallet != nullptr); diff --git a/src/fs.cpp b/src/fs.cpp index 45d892c7b4a08..de0ae8533bb62 100644 --- a/src/fs.cpp +++ b/src/fs.cpp @@ -18,6 +18,7 @@ #endif #include +#include #include namespace fsbridge { @@ -135,4 +136,4 @@ std::string get_filesystem_error_message(const fs::filesystem_error& e) #endif } -} // fsbridge +} // namespace fsbridge diff --git a/src/fs.h b/src/fs.h index 0444bd82c5dbe..270ce6fd1f00d 100644 --- a/src/fs.h +++ b/src/fs.h @@ -13,6 +13,8 @@ #include #include #include +#include +#include #include /** Filesystem operations and types */ @@ -34,7 +36,7 @@ class path : public std::filesystem::path // Allow path objects arguments for compatibility. path(std::filesystem::path path) : std::filesystem::path::path(std::move(path)) {} path& operator=(std::filesystem::path path) { std::filesystem::path::operator=(std::move(path)); return *this; } - path& operator/=(std::filesystem::path path) { std::filesystem::path::operator/=(path); return *this; } + path& operator/=(const std::filesystem::path& path) { std::filesystem::path::operator/=(path); return *this; } // Allow literal string arguments, which are safe as long as the literals are ASCII. path(const char* c) : std::filesystem::path(c) {} @@ -51,12 +53,15 @@ class path : public std::filesystem::path // Disallow std::string conversion method to avoid locale-dependent encoding on windows. std::string string() const = delete; - std::string u8string() const + /** + * Return a UTF-8 representation of the path as a std::string, for + * compatibility with code using std::string. For code using the newer + * std::u8string type, it is more efficient to call the inherited + * std::filesystem::path::u8string method instead. + */ + std::string utf8string() const { - const auto& utf8_str{std::filesystem::path::u8string()}; - // utf8_str might either be std::string (C++17) or std::u8string - // (C++20). Convert both to std::string. This method can be removed - // after switching to C++20. + const std::u8string& utf8_str{std::filesystem::path::u8string()}; return std::string{utf8_str.begin(), utf8_str.end()}; } @@ -68,11 +73,7 @@ class path : public std::filesystem::path static inline path u8path(const std::string& utf8_str) { -#if __cplusplus < 202002L - return std::filesystem::u8path(utf8_str); -#else return std::filesystem::path(std::u8string{utf8_str.begin(), utf8_str.end()}); -#endif } // Disallow implicit std::string conversion for absolute to avoid @@ -96,9 +97,9 @@ static inline auto quoted(const std::string& s) } // Allow safe path append operations. -static inline path operator/(path p1, path p2) +static inline path operator/(path p1, const path& p2) { - p1 /= std::move(p2); + p1 /= p2; return p1; } static inline path operator/(path p1, const char* p2) @@ -139,7 +140,7 @@ static inline bool copy_file(const path& from, const path& to, copy_options opti * Because \ref PathToString and \ref PathFromString functions don't specify an * encoding, they are meant to be used internally, not externally. They are not * appropriate to use in applications requiring UTF-8, where - * fs::path::u8string() and fs::u8path() methods should be used instead. Other + * fs::path::u8string() / fs::path::utf8string() and fs::u8path() methods should be used instead. Other * applications could require still different encodings. For example, JSON, XML, * or URI applications might prefer to use higher-level escapes (\uXXXX or * &XXXX; or %XX) instead of multibyte encoding. Rust, Python, Java applications @@ -153,13 +154,13 @@ static inline std::string PathToString(const path& path) // use here, because these methods encode the path using C++'s narrow // multibyte encoding, which on Windows corresponds to the current "code // page", which is unpredictable and typically not able to represent all - // valid paths. So fs::path::u8string() and + // valid paths. So fs::path::utf8string() and // fs::u8path() functions are used instead on Windows. On - // POSIX, u8string/u8path functions are not safe to use because paths are + // POSIX, u8string/utf8string/u8path functions are not safe to use because paths are // not always valid UTF-8, so plain string methods which do not transform // the path there are used. #ifdef WIN32 - return path.u8string(); + return path.utf8string(); #else static_assert(std::is_same::value, "PathToString not implemented on this platform"); return path.std::filesystem::path::string(); diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index c2c1c0a23f256..cb2ea62ab05fd 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -1666,7 +1666,7 @@ fs::path QStringToPath(const QString &path) QString PathToQString(const fs::path &path) { - return QString::fromStdString(path.u8string()); + return QString::fromStdString(path.utf8string()); } QString NetworkToQString(Network net) @@ -1715,8 +1715,7 @@ QString ConnectionTypeToQString(ConnectionType conn_type, bool prepend_direction QString formatDurationStr(std::chrono::seconds dur) { - using days = std::chrono::duration>; // can remove this line after C++20 - const auto d{std::chrono::duration_cast(dur)}; + const auto d{std::chrono::duration_cast(dur)}; const auto h{std::chrono::duration_cast(dur - d)}; const auto m{std::chrono::duration_cast(dur - d - h)}; const auto s{std::chrono::duration_cast(dur - d - h - m)}; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index c7759f0cc9f05..d37c2363e31fa 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -2631,7 +2631,7 @@ static RPCHelpMan dumptxoutset() if (fs::exists(path)) { throw JSONRPCError( RPC_INVALID_PARAMETER, - path.u8string() + " already exists. If you are sure this is what you want, " + path.utf8string() + " already exists. If you are sure this is what you want, " "move it out of the way first"); } @@ -2642,7 +2642,7 @@ static RPCHelpMan dumptxoutset() node, node.chainman->ActiveChainstate(), afile, path, temppath); fs::rename(temppath, path); - result.pushKV("path", path.u8string()); + result.pushKV("path", path.utf8string()); return result; }, }; @@ -2713,7 +2713,7 @@ UniValue CreateUTXOSnapshot( result.pushKV("coins_written", stats.coins_count); result.pushKV("base_hash", tip->GetBlockHash().ToString()); result.pushKV("base_height", tip->nHeight); - result.pushKV("path", path.u8string()); + result.pushKV("path", path.utf8string()); result.pushKV("txoutset_hash", stats.hashSerialized.ToString()); // Cast required because univalue doesn't have serialization specified for // `unsigned int`, nChainTx's type. diff --git a/src/rpc/mempool.cpp b/src/rpc/mempool.cpp index e6baf27176803..cd05f91b12708 100644 --- a/src/rpc/mempool.cpp +++ b/src/rpc/mempool.cpp @@ -459,7 +459,7 @@ RPCHelpMan savemempool() } UniValue ret(UniValue::VOBJ); - ret.pushKV("filename", fs::path((args.GetDataDirNet() / "mempool.dat")).u8string()); + ret.pushKV("filename", fs::path((args.GetDataDirNet() / "mempool.dat")).utf8string()); return ret; }, diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 149db2785be50..79dc6f9e67ca1 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -258,7 +258,7 @@ static RPCHelpMan getrpcinfo() UniValue result(UniValue::VOBJ); result.pushKV("active_commands", active_commands); - const std::string path = LogInstance().m_file_path.u8string(); + const std::string path = LogInstance().m_file_path.utf8string(); UniValue log_path(UniValue::VSTR, path); result.pushKV("logpath", log_path); diff --git a/src/test/fs_tests.cpp b/src/test/fs_tests.cpp index c3f0d9f55e9f0..5110bd967a665 100644 --- a/src/test/fs_tests.cpp +++ b/src/test/fs_tests.cpp @@ -18,9 +18,12 @@ BOOST_FIXTURE_TEST_SUITE(fs_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(fsbridge_pathtostring) { std::string u8_str = "fs_tests_∋_🏃"; + std::u8string str8{u8"fs_tests_∋_🏃"}; BOOST_CHECK_EQUAL(fs::PathToString(fs::PathFromString(u8_str)), u8_str); - BOOST_CHECK_EQUAL(fs::u8path(u8_str).u8string(), u8_str); - BOOST_CHECK_EQUAL(fs::PathFromString(u8_str).u8string(), u8_str); + BOOST_CHECK_EQUAL(fs::u8path(u8_str).utf8string(), u8_str); + BOOST_CHECK_EQUAL(fs::path(str8).utf8string(), u8_str); + BOOST_CHECK(fs::path(str8).u8string() == str8); + BOOST_CHECK_EQUAL(fs::PathFromString(u8_str).utf8string(), u8_str); BOOST_CHECK_EQUAL(fs::PathToString(fs::u8path(u8_str)), u8_str); #ifndef WIN32 // On non-windows systems, verify that arbitrary byte strings containing @@ -47,7 +50,7 @@ BOOST_AUTO_TEST_CASE(fsbridge_fstream) fs::path tmpfolder = m_args.GetDataDirBase(); // tmpfile1 should be the same as tmpfile2 fs::path tmpfile1 = tmpfolder / fs::u8path("fs_tests_∋_🏃"); - fs::path tmpfile2 = tmpfolder / fs::u8path("fs_tests_∋_🏃"); + fs::path tmpfile2 = tmpfolder / fs::path(u8"fs_tests_∋_🏃"); { std::ofstream file{tmpfile1}; file << "bitcoin"; diff --git a/src/util/system.cpp b/src/util/system.cpp index ae260298099f7..e9b77f76c7837 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -418,7 +418,7 @@ fs::path ArgsManager::GetPathArg(std::string arg, const fs::path& default_value) return result.has_filename() ? result : result.parent_path(); } -const fs::path ArgsManager::GetBlocksDirPath() const +fs::path ArgsManager::GetBlocksDirPath() const { LOCK(cs_args); fs::path& path = m_cached_blocks_path; @@ -443,7 +443,7 @@ const fs::path ArgsManager::GetBlocksDirPath() const return path; } -const fs::path ArgsManager::GetDataDir(bool net_specific) const +fs::path ArgsManager::GetDataDir(bool net_specific) const { LOCK(cs_args); fs::path& path = net_specific ? m_cached_network_datadir_path : m_cached_datadir_path; diff --git a/src/util/system.h b/src/util/system.h index 7d53c46737682..7b9a39c3cd2c9 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -296,7 +296,7 @@ class ArgsManager * * @return Blocks path which is network specific */ - const fs::path GetBlocksDirPath() const; + fs::path GetBlocksDirPath() const; /** * Get data directory path @@ -304,7 +304,7 @@ class ArgsManager * @return Absolute path on success, otherwise an empty path when a non-directory path would be returned * @post Returned directory path is created unless it is empty */ - const fs::path GetDataDirBase() const { return GetDataDir(false); } + fs::path GetDataDirBase() const { return GetDataDir(false); } /** * Get data directory path with appended network identifier @@ -312,7 +312,7 @@ class ArgsManager * @return Absolute path on success, otherwise an empty path when a non-directory path would be returned * @post Returned directory path is created unless it is empty */ - const fs::path GetDataDirNet() const { return GetDataDir(true); } + fs::path GetDataDirNet() const { return GetDataDir(true); } fs::path GetBackupsDirPath(); @@ -503,7 +503,7 @@ class ArgsManager * @return Absolute path on success, otherwise an empty path when a non-directory path would be returned * @post Returned directory path is created unless it is empty */ - const fs::path GetDataDir(bool net_specific) const; + fs::path GetDataDir(bool net_specific) const; // Helper function for LogArgs(). void logArgsPrefix( diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 18a9501a8e5b8..3d9ac57988dab 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -958,7 +958,7 @@ RPCHelpMan dumpwallet() * It may also avoid other security issues. */ if (fs::exists(filepath)) { - throw JSONRPCError(RPC_INVALID_PARAMETER, filepath.u8string() + " already exists. If you are sure this is what you want, move it out of the way first"); + throw JSONRPCError(RPC_INVALID_PARAMETER, filepath.utf8string() + " already exists. If you are sure this is what you want, move it out of the way first"); } std::ofstream file; @@ -1082,7 +1082,7 @@ RPCHelpMan dumpwallet() std::string strWarning = strprintf(_("%s file contains all private keys from this wallet. Do not share it with anyone!").translated, request.params[0].get_str()); obj.pushKV("keys", int(vKeyBirth.size())); - obj.pushKV("filename", filepath.u8string()); + obj.pushKV("filename", filepath.utf8string()); obj.pushKV("warning", strWarning); return obj; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 31a331f2e6181..d626ddf1faf68 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2604,7 +2604,7 @@ static RPCHelpMan listwalletdir() UniValue wallets(UniValue::VARR); for (const auto& path : ListDatabases(GetWalletDir())) { UniValue wallet(UniValue::VOBJ); - wallet.pushKV("name", path.u8string()); + wallet.pushKV("name", path.utf8string()); wallets.push_back(wallet); } From 9f2c278cc828b85a409d421dd476b1e7ea3b8479 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Sun, 4 May 2025 23:50:44 +0000 Subject: [PATCH 06/14] merge bitcoin#29181: remove systemtap variadic patch --- depends/packages/systemtap.mk | 3 +-- .../patches/systemtap/fix_variadic_warning.patch | 16 ---------------- 2 files changed, 1 insertion(+), 18 deletions(-) delete mode 100644 depends/patches/systemtap/fix_variadic_warning.patch diff --git a/depends/packages/systemtap.mk b/depends/packages/systemtap.mk index 541ebeee01408..c912e18c31e77 100644 --- a/depends/packages/systemtap.mk +++ b/depends/packages/systemtap.mk @@ -3,11 +3,10 @@ $(package)_version=4.8 $(package)_download_path=https://sourceware.org/ftp/systemtap/releases/ $(package)_file_name=$(package)-$($(package)_version).tar.gz $(package)_sha256_hash=cbd50a4eba5b261394dc454c12448ddec73e55e6742fda7f508f9fbc1331c223 -$(package)_patches=remove_SDT_ASM_SECTION_AUTOGROUP_SUPPORT_check.patch fix_variadic_warning.patch +$(package)_patches=remove_SDT_ASM_SECTION_AUTOGROUP_SUPPORT_check.patch define $(package)_preprocess_cmds patch -p1 < $($(package)_patch_dir)/remove_SDT_ASM_SECTION_AUTOGROUP_SUPPORT_check.patch && \ - patch -p1 < $($(package)_patch_dir)/fix_variadic_warning.patch && \ mkdir -p $($(package)_staging_prefix_dir)/include/sys && \ cp includes/sys/sdt.h $($(package)_staging_prefix_dir)/include/sys/sdt.h endef diff --git a/depends/patches/systemtap/fix_variadic_warning.patch b/depends/patches/systemtap/fix_variadic_warning.patch deleted file mode 100644 index 93cc2d6081d77..0000000000000 --- a/depends/patches/systemtap/fix_variadic_warning.patch +++ /dev/null @@ -1,16 +0,0 @@ -Could be dropped after a migration to C++20. -See: https://github.com/bitcoin/bitcoin/issues/26916. - -diff --git a/includes/sys/sdt.h b/includes/sys/sdt.h -index 4075a5f..7c6138c 100644 ---- a/includes/sys/sdt.h -+++ b/includes/sys/sdt.h -@@ -276,7 +276,7 @@ __extension__ extern unsigned long long __sdt_unsp; - _SDT_ASM_1(.purgem _SDT_TYPE_) \ - _SDT_ASM_1(.purgem _SDT_TYPE) - --#define _SDT_ASM_BODY(provider, name, pack_args, args, ...) \ -+#define _SDT_ASM_BODY(provider, name, pack_args, args) \ - _SDT_DEF_MACROS \ - _SDT_ASM_1(990: _SDT_NOP) \ - _SDT_ASM_3( .pushsection .note.stapsdt,_SDT_ASM_AUTOGROUP,"note") \ From e317a5c537b0bacd19e12057d3f3f967d23e3bea Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Tue, 12 Dec 2023 18:37:48 +0100 Subject: [PATCH 07/14] merge bitcoin#29085: Use std::rotl --- src/crypto/chacha20.cpp | 11 ++++--- src/crypto/sha3.cpp | 65 +++++++++++++++++++---------------------- src/crypto/siphash.cpp | 14 ++++----- src/hash.cpp | 12 +++----- 4 files changed, 46 insertions(+), 56 deletions(-) diff --git a/src/crypto/chacha20.cpp b/src/crypto/chacha20.cpp index 517c1d08f9fb2..f386b7bd705cd 100644 --- a/src/crypto/chacha20.cpp +++ b/src/crypto/chacha20.cpp @@ -11,15 +11,14 @@ #include #include +#include #include -constexpr static inline uint32_t rotl32(uint32_t v, int c) { return (v << c) | (v >> (32 - c)); } - #define QUARTERROUND(a,b,c,d) \ - a += b; d = rotl32(d ^ a, 16); \ - c += d; b = rotl32(b ^ c, 12); \ - a += b; d = rotl32(d ^ a, 8); \ - c += d; b = rotl32(b ^ c, 7); + a += b; d = std::rotl(d ^ a, 16); \ + c += d; b = std::rotl(b ^ c, 12); \ + a += b; d = std::rotl(d ^ a, 8); \ + c += d; b = std::rotl(b ^ c, 7); #define REPEAT10(a) do { {a}; {a}; {a}; {a}; {a}; {a}; {a}; {a}; {a}; {a}; } while(0) diff --git a/src/crypto/sha3.cpp b/src/crypto/sha3.cpp index b3fc7f330c2a5..c1d2f991653f0 100644 --- a/src/crypto/sha3.cpp +++ b/src/crypto/sha3.cpp @@ -10,15 +10,10 @@ #include #include // For std::begin and std::end. +#include #include -// Internal implementation code. -namespace -{ -uint64_t Rotl(uint64_t x, int n) { return (x << n) | (x >> (64 - n)); } -} // namespace - void KeccakF(uint64_t (&st)[25]) { static constexpr uint64_t RNDC[24] = { @@ -40,38 +35,38 @@ void KeccakF(uint64_t (&st)[25]) bc2 = st[2] ^ st[7] ^ st[12] ^ st[17] ^ st[22]; bc3 = st[3] ^ st[8] ^ st[13] ^ st[18] ^ st[23]; bc4 = st[4] ^ st[9] ^ st[14] ^ st[19] ^ st[24]; - t = bc4 ^ Rotl(bc1, 1); st[0] ^= t; st[5] ^= t; st[10] ^= t; st[15] ^= t; st[20] ^= t; - t = bc0 ^ Rotl(bc2, 1); st[1] ^= t; st[6] ^= t; st[11] ^= t; st[16] ^= t; st[21] ^= t; - t = bc1 ^ Rotl(bc3, 1); st[2] ^= t; st[7] ^= t; st[12] ^= t; st[17] ^= t; st[22] ^= t; - t = bc2 ^ Rotl(bc4, 1); st[3] ^= t; st[8] ^= t; st[13] ^= t; st[18] ^= t; st[23] ^= t; - t = bc3 ^ Rotl(bc0, 1); st[4] ^= t; st[9] ^= t; st[14] ^= t; st[19] ^= t; st[24] ^= t; + t = bc4 ^ std::rotl(bc1, 1); st[0] ^= t; st[5] ^= t; st[10] ^= t; st[15] ^= t; st[20] ^= t; + t = bc0 ^ std::rotl(bc2, 1); st[1] ^= t; st[6] ^= t; st[11] ^= t; st[16] ^= t; st[21] ^= t; + t = bc1 ^ std::rotl(bc3, 1); st[2] ^= t; st[7] ^= t; st[12] ^= t; st[17] ^= t; st[22] ^= t; + t = bc2 ^ std::rotl(bc4, 1); st[3] ^= t; st[8] ^= t; st[13] ^= t; st[18] ^= t; st[23] ^= t; + t = bc3 ^ std::rotl(bc0, 1); st[4] ^= t; st[9] ^= t; st[14] ^= t; st[19] ^= t; st[24] ^= t; // Rho Pi t = st[1]; - bc0 = st[10]; st[10] = Rotl(t, 1); t = bc0; - bc0 = st[7]; st[7] = Rotl(t, 3); t = bc0; - bc0 = st[11]; st[11] = Rotl(t, 6); t = bc0; - bc0 = st[17]; st[17] = Rotl(t, 10); t = bc0; - bc0 = st[18]; st[18] = Rotl(t, 15); t = bc0; - bc0 = st[3]; st[3] = Rotl(t, 21); t = bc0; - bc0 = st[5]; st[5] = Rotl(t, 28); t = bc0; - bc0 = st[16]; st[16] = Rotl(t, 36); t = bc0; - bc0 = st[8]; st[8] = Rotl(t, 45); t = bc0; - bc0 = st[21]; st[21] = Rotl(t, 55); t = bc0; - bc0 = st[24]; st[24] = Rotl(t, 2); t = bc0; - bc0 = st[4]; st[4] = Rotl(t, 14); t = bc0; - bc0 = st[15]; st[15] = Rotl(t, 27); t = bc0; - bc0 = st[23]; st[23] = Rotl(t, 41); t = bc0; - bc0 = st[19]; st[19] = Rotl(t, 56); t = bc0; - bc0 = st[13]; st[13] = Rotl(t, 8); t = bc0; - bc0 = st[12]; st[12] = Rotl(t, 25); t = bc0; - bc0 = st[2]; st[2] = Rotl(t, 43); t = bc0; - bc0 = st[20]; st[20] = Rotl(t, 62); t = bc0; - bc0 = st[14]; st[14] = Rotl(t, 18); t = bc0; - bc0 = st[22]; st[22] = Rotl(t, 39); t = bc0; - bc0 = st[9]; st[9] = Rotl(t, 61); t = bc0; - bc0 = st[6]; st[6] = Rotl(t, 20); t = bc0; - st[1] = Rotl(t, 44); + bc0 = st[10]; st[10] = std::rotl(t, 1); t = bc0; + bc0 = st[7]; st[7] = std::rotl(t, 3); t = bc0; + bc0 = st[11]; st[11] = std::rotl(t, 6); t = bc0; + bc0 = st[17]; st[17] = std::rotl(t, 10); t = bc0; + bc0 = st[18]; st[18] = std::rotl(t, 15); t = bc0; + bc0 = st[3]; st[3] = std::rotl(t, 21); t = bc0; + bc0 = st[5]; st[5] = std::rotl(t, 28); t = bc0; + bc0 = st[16]; st[16] = std::rotl(t, 36); t = bc0; + bc0 = st[8]; st[8] = std::rotl(t, 45); t = bc0; + bc0 = st[21]; st[21] = std::rotl(t, 55); t = bc0; + bc0 = st[24]; st[24] = std::rotl(t, 2); t = bc0; + bc0 = st[4]; st[4] = std::rotl(t, 14); t = bc0; + bc0 = st[15]; st[15] = std::rotl(t, 27); t = bc0; + bc0 = st[23]; st[23] = std::rotl(t, 41); t = bc0; + bc0 = st[19]; st[19] = std::rotl(t, 56); t = bc0; + bc0 = st[13]; st[13] = std::rotl(t, 8); t = bc0; + bc0 = st[12]; st[12] = std::rotl(t, 25); t = bc0; + bc0 = st[2]; st[2] = std::rotl(t, 43); t = bc0; + bc0 = st[20]; st[20] = std::rotl(t, 62); t = bc0; + bc0 = st[14]; st[14] = std::rotl(t, 18); t = bc0; + bc0 = st[22]; st[22] = std::rotl(t, 39); t = bc0; + bc0 = st[9]; st[9] = std::rotl(t, 61); t = bc0; + bc0 = st[6]; st[6] = std::rotl(t, 20); t = bc0; + st[1] = std::rotl(t, 44); // Chi Iota bc0 = st[0]; bc1 = st[1]; bc2 = st[2]; bc3 = st[3]; bc4 = st[4]; diff --git a/src/crypto/siphash.cpp b/src/crypto/siphash.cpp index 2e90c393e1693..d73592de8f342 100644 --- a/src/crypto/siphash.cpp +++ b/src/crypto/siphash.cpp @@ -4,15 +4,15 @@ #include -#define ROTL(x, b) (uint64_t)(((x) << (b)) | ((x) >> (64 - (b)))) +#include #define SIPROUND do { \ - v0 += v1; v1 = ROTL(v1, 13); v1 ^= v0; \ - v0 = ROTL(v0, 32); \ - v2 += v3; v3 = ROTL(v3, 16); v3 ^= v2; \ - v0 += v3; v3 = ROTL(v3, 21); v3 ^= v0; \ - v2 += v1; v1 = ROTL(v1, 17); v1 ^= v2; \ - v2 = ROTL(v2, 32); \ + v0 += v1; v1 = std::rotl(v1, 13); v1 ^= v0; \ + v0 = std::rotl(v0, 32); \ + v2 += v3; v3 = std::rotl(v3, 16); v3 ^= v2; \ + v0 += v3; v3 = std::rotl(v3, 21); v3 ^= v0; \ + v2 += v1; v1 = std::rotl(v1, 17); v1 ^= v2; \ + v2 = std::rotl(v2, 32); \ } while (0) CSipHasher::CSipHasher(uint64_t k0, uint64_t k1) diff --git a/src/hash.cpp b/src/hash.cpp index 8bf7040518f2d..87ffc3982a395 100644 --- a/src/hash.cpp +++ b/src/hash.cpp @@ -7,13 +7,9 @@ #include #include +#include #include -inline uint32_t ROTL32(uint32_t x, int8_t r) -{ - return (x << r) | (x >> (32 - r)); -} - unsigned int MurmurHash3(unsigned int nHashSeed, Span vDataToHash) { // The following is MurmurHash3 (x86_32), see https://github.com/aappleby/smhasher/blob/master/src/MurmurHash3.cpp @@ -31,11 +27,11 @@ unsigned int MurmurHash3(unsigned int nHashSeed, Span vData uint32_t k1 = ReadLE32(blocks + i*4); k1 *= c1; - k1 = ROTL32(k1, 15); + k1 = std::rotl(k1, 15); k1 *= c2; h1 ^= k1; - h1 = ROTL32(h1, 13); + h1 = std::rotl(h1, 13); h1 = h1 * 5 + 0xe6546b64; } @@ -55,7 +51,7 @@ unsigned int MurmurHash3(unsigned int nHashSeed, Span vData case 1: k1 ^= tail[0]; k1 *= c1; - k1 = ROTL32(k1, 15); + k1 = std::rotl(k1, 15); k1 *= c2; h1 ^= k1; } From 74bcdf0de3c99980bdbf21f6b1dc413e0f879fef Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 26 Feb 2024 19:52:14 +0000 Subject: [PATCH 08/14] merge bitcoin#29484: replace char-is-int8_t autoconf detection with c++20 concept --- configure.ac | 8 -------- src/serialize.h | 16 ++++++++++------ 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/configure.ac b/configure.ac index a809c41327765..8e9a03256e91b 100644 --- a/configure.ac +++ b/configure.ac @@ -1215,14 +1215,6 @@ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include [ AC_MSG_RESULT([no])] ) -AC_MSG_CHECKING([for if type char equals int8_t]) -AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include - #include ]], - [[ static_assert(std::is_same::value, ""); ]])], - [ AC_MSG_RESULT([yes]); AC_DEFINE([CHAR_EQUALS_INT8], [1], [Define this symbol if type char equals int8_t]) ], - [ AC_MSG_RESULT([no])] -) - dnl ensure backtrace() is found, check -lexecinfo if necessary if test "$TARGET_OS" != "windows"; then if test "$enable_stacktraces" != "no"; then diff --git a/src/serialize.h b/src/serialize.h index 8f49f8a0a7ab7..3c25e8c1753cf 100644 --- a/src/serialize.h +++ b/src/serialize.h @@ -10,6 +10,7 @@ #include #include +#include #include #include #include @@ -193,9 +194,14 @@ template const X& ReadWriteAsHelper(const X& x) { return x; } FORMATTER_METHODS(cls, obj) // clang-format off -#ifndef CHAR_EQUALS_INT8 -template void Serialize(Stream&, char) = delete; // char serialization forbidden. Use uint8_t or int8_t -#endif + +// Typically int8_t and char are distinct types, but some systems may define int8_t +// in terms of char. Forbid serialization of char in the typical case, but allow it if +// it's the only way to describe an int8_t. +template +concept CharNotInt8 = std::same_as && !std::same_as; + +template void Serialize(Stream&, V) = delete; // char serialization forbidden. Use uint8_t or int8_t template void Serialize(Stream& s, std::byte a) { ser_writedata8(s, uint8_t(a)); } template inline void Serialize(Stream& s, int8_t a ) { ser_writedata8(s, a); } template inline void Serialize(Stream& s, uint8_t a ) { ser_writedata8(s, a); } @@ -209,9 +215,7 @@ template inline void Serialize(Stream& s, const char (&a template inline void Serialize(Stream& s, const unsigned char (&a)[N]) { s.write(MakeByteSpan(a)); } template void Serialize(Stream& s, Span span) { (void)/* force byte-type */UCharCast(span.data()); s.write(AsBytes(span)); } -#ifndef CHAR_EQUALS_INT8 -template void Unserialize(Stream&, char) = delete; // char serialization forbidden. Use uint8_t or int8_t -#endif +template void Unserialize(Stream&, V) = delete; // char serialization forbidden. Use uint8_t or int8_t template void Unserialize(Stream& s, std::byte& a) { a = std::byte{ser_readdata8(s)}; } template inline void Unserialize(Stream& s, int8_t& a ) { a = ser_readdata8(s); } template inline void Unserialize(Stream& s, uint8_t& a ) { a = ser_readdata8(s); } From 3a71354ae198b1396337b21012217b421920f5e9 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 1 Feb 2024 11:27:05 +0000 Subject: [PATCH 09/14] merge bitcoin#29577: ignore deprecated-declarations warnings in objc++ macOS code --- configure.ac | 5 ++++- src/Makefile.am | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 8e9a03256e91b..ac738d666810d 100644 --- a/configure.ac +++ b/configure.ac @@ -851,7 +851,10 @@ case $host in fi CORE_CPPFLAGS="$CORE_CPPFLAGS -DMAC_OSX -DOBJC_OLD_DISPATCH_PROTOTYPES=0" - OBJCXXFLAGS="$CXXFLAGS" + + dnl ignore deprecated-declarations warnings coming from objcxx code + dnl "'NSUserNotificationCenter' is deprecated: first deprecated in macOS 11.0". + OBJCXXFLAGS="$CXXFLAGS -Wno-deprecated-declarations" ;; *android*) dnl make sure android stays above linux for hosts like *linux-android* diff --git a/src/Makefile.am b/src/Makefile.am index 3d76eb85286aa..e687553ada601 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -12,6 +12,7 @@ DIST_SUBDIRS = secp256k1 AM_LDFLAGS = $(LIBTOOL_LDFLAGS) $(HARDENED_LDFLAGS) $(GPROF_LDFLAGS) $(SANITIZER_LDFLAGS) $(CORE_LDFLAGS) AM_CFLAGS = $(DEBUG_CFLAGS) AM_CXXFLAGS = $(DEBUG_CXXFLAGS) $(HARDENED_CXXFLAGS) $(WARN_CXXFLAGS) $(NOWARN_CXXFLAGS) $(ERROR_CXXFLAGS) $(GPROF_CXXFLAGS) $(SANITIZER_CXXFLAGS) $(CORE_CXXFLAGS) +AM_OBJCXXFLAGS = $(AM_CXXFLAGS) AM_CPPFLAGS = $(DEBUG_CPPFLAGS) $(HARDENED_CPPFLAGS) $(CORE_CPPFLAGS) AM_LIBTOOLFLAGS = --preserve-dup-deps PTHREAD_FLAGS = $(PTHREAD_CFLAGS) $(PTHREAD_LIBS) From d5179ec67311f9664ca096c62ce78e3cfd76319c Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 5 May 2025 00:50:40 +0000 Subject: [PATCH 10/14] refactor: move away from `gmtime` The next commit moves inherited code away from `gmtime` and removes it altogether. --- src/util/time.cpp | 15 ++++++--------- src/wallet/wallet.cpp | 16 +++++++--------- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/src/util/time.cpp b/src/util/time.cpp index 1882e54c0d170..7132523761666 100644 --- a/src/util/time.cpp +++ b/src/util/time.cpp @@ -135,15 +135,12 @@ std::string FormatISO8601Date(int64_t nTime) { return strprintf("%04i-%02i-%02i", ts.tm_year + 1900, ts.tm_mon + 1, ts.tm_mday); } -std::string FormatISO8601Time(int64_t nTime) { - struct tm ts; - time_t time_val = nTime; -#ifdef HAVE_GMTIME_R - gmtime_r(&time_val, &ts); -#else - gmtime_s(&ts, &time_val); -#endif - return strprintf("%02i:%02i:%02iZ", ts.tm_hour, ts.tm_min, ts.tm_sec); +std::string FormatISO8601Time(int64_t nTime) +{ + const std::chrono::sys_seconds secs{std::chrono::seconds{nTime}}; + const auto days{std::chrono::floor(secs)}; + const std::chrono::hh_mm_ss hms{secs - days}; + return strprintf("%02i:%02i:%02iZ", hms.hours().count(), hms.minutes().count(), hms.seconds().count()); } int64_t ParseISO8601DateTime(const std::string& str) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d009af3e3ea79..07cd4ad32801b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3262,15 +3262,13 @@ bool CWallet::AutoBackupWallet(const fs::path& wallet_path, bilingual_str& error } // Create backup of the ... - struct tm ts; - time_t time_val = GetTime(); -#ifdef HAVE_GMTIME_R - gmtime_r(&time_val, &ts); -#else - gmtime_s(&ts, &time_val); -#endif - std::string dateTimeStr = strprintf(".%04i-%02i-%02i-%02i-%02i", - ts.tm_year + 1900, ts.tm_mon + 1, ts.tm_mday, ts.tm_hour, ts.tm_min); + std::string dateTimeStr = [&]() { + const std::chrono::sys_seconds secs{GetTime()}; + const auto days{std::chrono::floor(secs)}; + const std::chrono::year_month_day ymd{days}; + const std::chrono::hh_mm_ss hms{secs - days}; + return strprintf(".%04i-%02u-%02u-%02i-%02i", signed{ymd.year()}, unsigned{ymd.month()}, unsigned{ymd.day()}, hms.hours().count(), hms.minutes().count()); + }(); if (wallet_path.empty()) { // ... opened wallet From 42aa57a9d380828dd9e1ce3ba3d2f6ed1bed3863 Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Mon, 5 May 2025 00:47:49 +0000 Subject: [PATCH 11/14] merge bitcoin#29081: Remove gmtime* --- configure.ac | 17 -------- src/init/common.cpp | 5 --- src/test/sanity_tests.cpp | 2 - src/test/util_tests.cpp | 5 +++ src/util/time.cpp | 89 +++++++-------------------------------- src/util/time.h | 3 -- 6 files changed, 21 insertions(+), 100 deletions(-) diff --git a/configure.ac b/configure.ac index ac738d666810d..9a3d1d83b8508 100644 --- a/configure.ac +++ b/configure.ac @@ -1161,22 +1161,6 @@ if test "$use_thread_local" = "yes" || test "$use_thread_local" = "auto"; then LDFLAGS="$TEMP_LDFLAGS" fi -dnl check for gmtime_r(), fallback to gmtime_s() if that is unavailable -dnl fail if neither are available. -AC_MSG_CHECKING([for gmtime_r]) -AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include ]], - [[ gmtime_r((const time_t *) nullptr, (struct tm *) nullptr); ]])], - [ AC_MSG_RESULT([yes]); AC_DEFINE([HAVE_GMTIME_R], [1], [Define this symbol if gmtime_r is available]) ], - [ AC_MSG_RESULT([no]); - AC_MSG_CHECKING([for gmtime_s]); - AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include ]], - [[ gmtime_s((struct tm *) nullptr, (const time_t *) nullptr); ]])], - [ AC_MSG_RESULT([yes])], - [ AC_MSG_RESULT([no]); AC_MSG_ERROR([Both gmtime_r and gmtime_s are unavailable]) ] - ) - ] -) - dnl Check for different ways of gathering OS randomness AC_MSG_CHECKING([for Linux getrandom syscall]) AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include @@ -1905,7 +1889,6 @@ AC_SUBST(HAVE_O_CLOEXEC) AC_SUBST(HAVE_BUILTIN_PREFETCH) AC_SUBST(HAVE_MM_PREFETCH) AC_SUBST(HAVE_STRONG_GETAUXVAL) -AC_SUBST(HAVE_GMTIME_R) AC_SUBST(ANDROID_ARCH) AC_SUBST(HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR) AC_CONFIG_FILES([Makefile src/Makefile doc/man/Makefile share/setup.nsi share/qt/Info.plist test/config.ini]) diff --git a/src/init/common.cpp b/src/init/common.cpp index d29925e397351..a484174116929 100644 --- a/src/init/common.cpp +++ b/src/init/common.cpp @@ -17,7 +17,6 @@ #include #include #include -#include #include #include @@ -52,10 +51,6 @@ bool SanityChecks() return InitError(Untranslated("OS cryptographic RNG sanity check failure. Aborting.")); } - if (!ChronoSanityCheck()) { - return InitError(Untranslated("Clock epoch mismatch. Aborting.")); - } - return true; } diff --git a/src/test/sanity_tests.cpp b/src/test/sanity_tests.cpp index 907a3fd15b078..e7de62ec4d426 100644 --- a/src/test/sanity_tests.cpp +++ b/src/test/sanity_tests.cpp @@ -4,7 +4,6 @@ #include #include -#include #include @@ -13,7 +12,6 @@ BOOST_FIXTURE_TEST_SUITE(sanity_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(basic_sanity) { BOOST_CHECK_MESSAGE(ECC_InitSanityCheck() == true, "secp256k1 sanity test"); - BOOST_CHECK_MESSAGE(ChronoSanityCheck() == true, "chrono epoch test"); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 31227c82bae7c..ea1d0ffd7c306 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -268,6 +268,8 @@ BOOST_AUTO_TEST_CASE(util_TrimString) BOOST_AUTO_TEST_CASE(util_FormatParseISO8601DateTime) { + BOOST_CHECK_EQUAL(FormatISO8601DateTime(971890963199), "32767-12-31T23:59:59Z"); + BOOST_CHECK_EQUAL(FormatISO8601DateTime(971890876800), "32767-12-31T00:00:00Z"); BOOST_CHECK_EQUAL(FormatISO8601DateTime(1317425777), "2011-09-30T23:36:17Z"); BOOST_CHECK_EQUAL(FormatISO8601DateTime(0), "1970-01-01T00:00:00Z"); @@ -278,7 +280,10 @@ BOOST_AUTO_TEST_CASE(util_FormatParseISO8601DateTime) BOOST_AUTO_TEST_CASE(util_FormatISO8601Date) { + BOOST_CHECK_EQUAL(FormatISO8601Date(971890963199), "32767-12-31"); + BOOST_CHECK_EQUAL(FormatISO8601Date(971890876800), "32767-12-31"); BOOST_CHECK_EQUAL(FormatISO8601Date(1317425777), "2011-09-30"); + BOOST_CHECK_EQUAL(FormatISO8601Date(0), "1970-01-01"); } BOOST_AUTO_TEST_CASE(util_FormatISO8601Time) diff --git a/src/util/time.cpp b/src/util/time.cpp index 7132523761666..76b69ac9b87e8 100644 --- a/src/util/time.cpp +++ b/src/util/time.cpp @@ -3,69 +3,21 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#if defined(HAVE_CONFIG_H) -#include -#endif - -#include #include +#include +#include #include #include -#include -#include #include -#include +#include void UninterruptibleSleep(const std::chrono::microseconds& n) { std::this_thread::sleep_for(n); } static std::atomic g_mock_time{}; //!< For testing -bool ChronoSanityCheck() -{ - // std::chrono::system_clock.time_since_epoch and time_t(0) are not guaranteed - // to use the Unix epoch timestamp, prior to C++20, but in practice they almost - // certainly will. Any differing behavior will be assumed to be an error, unless - // certain platforms prove to consistently deviate, at which point we'll cope - // with it by adding offsets. - - // Create a new clock from time_t(0) and make sure that it represents 0 - // seconds from the system_clock's time_since_epoch. Then convert that back - // to a time_t and verify that it's the same as before. - const time_t time_t_epoch{}; - auto clock = std::chrono::system_clock::from_time_t(time_t_epoch); - if (std::chrono::duration_cast(clock.time_since_epoch()).count() != 0) { - return false; - } - - time_t time_val = std::chrono::system_clock::to_time_t(clock); - if (time_val != time_t_epoch) { - return false; - } - - // Check that the above zero time is actually equal to the known unix timestamp. - struct tm epoch; -#ifdef HAVE_GMTIME_R - if (gmtime_r(&time_val, &epoch) == nullptr) { -#else - if (gmtime_s(&epoch, &time_val) != 0) { -#endif - return false; - } - - if ((epoch.tm_sec != 0) || - (epoch.tm_min != 0) || - (epoch.tm_hour != 0) || - (epoch.tm_mday != 1) || - (epoch.tm_mon != 0) || - (epoch.tm_year != 70)) { - return false; - } - return true; -} - NodeClock::time_point NodeClock::now() noexcept { const auto mocktime{g_mock_time.load(std::memory_order_relaxed)}; @@ -109,30 +61,21 @@ int64_t GetTimeMicros() int64_t GetTime() { return GetTime().count(); } -std::string FormatISO8601DateTime(int64_t nTime) { - struct tm ts; - time_t time_val = nTime; -#ifdef HAVE_GMTIME_R - if (gmtime_r(&time_val, &ts) == nullptr) { -#else - if (gmtime_s(&ts, &time_val) != 0) { -#endif - return {}; - } - return strprintf("%04i-%02i-%02iT%02i:%02i:%02iZ", ts.tm_year + 1900, ts.tm_mon + 1, ts.tm_mday, ts.tm_hour, ts.tm_min, ts.tm_sec); +std::string FormatISO8601DateTime(int64_t nTime) +{ + const std::chrono::sys_seconds secs{std::chrono::seconds{nTime}}; + const auto days{std::chrono::floor(secs)}; + const std::chrono::year_month_day ymd{days}; + const std::chrono::hh_mm_ss hms{secs - days}; + return strprintf("%04i-%02u-%02uT%02i:%02i:%02iZ", signed{ymd.year()}, unsigned{ymd.month()}, unsigned{ymd.day()}, hms.hours().count(), hms.minutes().count(), hms.seconds().count()); } -std::string FormatISO8601Date(int64_t nTime) { - struct tm ts; - time_t time_val = nTime; -#ifdef HAVE_GMTIME_R - if (gmtime_r(&time_val, &ts) == nullptr) { -#else - if (gmtime_s(&ts, &time_val) != 0) { -#endif - return {}; - } - return strprintf("%04i-%02i-%02i", ts.tm_year + 1900, ts.tm_mon + 1, ts.tm_mday); +std::string FormatISO8601Date(int64_t nTime) +{ + const std::chrono::sys_seconds secs{std::chrono::seconds{nTime}}; + const auto days{std::chrono::floor(secs)}; + const std::chrono::year_month_day ymd{days}; + return strprintf("%04i-%02u-%02u", signed{ymd.year()}, unsigned{ymd.month()}, unsigned{ymd.day()}); } std::string FormatISO8601Time(int64_t nTime) diff --git a/src/util/time.h b/src/util/time.h index 9f870cfa2b0ca..ee7631e10de58 100644 --- a/src/util/time.h +++ b/src/util/time.h @@ -122,7 +122,4 @@ struct timeval MillisToTimeval(int64_t nTimeout); */ struct timeval MillisToTimeval(std::chrono::milliseconds ms); -/** Sanity check epoch match normal Unix epoch */ -bool ChronoSanityCheck(); - #endif // BITCOIN_UTIL_TIME_H From 525bf6fcfea8ad1f3a993290d29f535ab42600db Mon Sep 17 00:00:00 2001 From: Kittywhiskers Van Gogh <63189531+kwvg@users.noreply.github.com> Date: Thu, 4 Apr 2024 19:56:21 +0000 Subject: [PATCH 12/14] merge bitcoin#29815: always use our fallback timingsafe_bcmp rather than libc's --- configure.ac | 2 -- src/crypto/chacha20poly1305.cpp | 9 ++------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/configure.ac b/configure.ac index 9a3d1d83b8508..af9654255d1a9 100644 --- a/configure.ac +++ b/configure.ac @@ -1059,8 +1059,6 @@ AC_CHECK_DECLS([setsid]) AC_CHECK_DECLS([pipe2]) -AC_CHECK_FUNCS([timingsafe_bcmp]) - dnl Check for mallopt(M_ARENA_MAX) (to set glibc arenas) AC_MSG_CHECKING([for mallopt M_ARENA_MAX]) AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[#include ]], diff --git a/src/crypto/chacha20poly1305.cpp b/src/crypto/chacha20poly1305.cpp index 59671d304c449..b969bb1a29903 100644 --- a/src/crypto/chacha20poly1305.cpp +++ b/src/crypto/chacha20poly1305.cpp @@ -26,10 +26,7 @@ void AEADChaCha20Poly1305::SetKey(Span key) noexcept namespace { -#ifndef HAVE_TIMINGSAFE_BCMP -#define HAVE_TIMINGSAFE_BCMP - -int timingsafe_bcmp(const unsigned char* b1, const unsigned char* b2, size_t n) noexcept +int timingsafe_bcmp_internal(const unsigned char* b1, const unsigned char* b2, size_t n) noexcept { const unsigned char *p1 = b1, *p2 = b2; int ret = 0; @@ -38,8 +35,6 @@ int timingsafe_bcmp(const unsigned char* b1, const unsigned char* b2, size_t n) return (ret != 0); } -#endif - /** Compute poly1305 tag. chacha20 must be set to the right nonce, block 0. Will be at block 1 after. */ void ComputeTag(ChaCha20& chacha20, Span aad, Span cipher, Span tag) noexcept { @@ -93,7 +88,7 @@ bool AEADChaCha20Poly1305::Decrypt(Span cipher, Span Date: Thu, 19 Oct 2023 12:46:32 +0100 Subject: [PATCH 13/14] merge bitcoin#28687: std::views::reverse --- contrib/devtools/copyright_header.py | 1 - src/Makefile.am | 1 - src/net_processing.cpp | 6 ++--- src/node/blockstorage.cpp | 4 +-- src/reverse_iterator.h | 39 ---------------------------- src/test/fuzz/prevector.cpp | 14 +++++----- src/test/prevector_tests.cpp | 15 +++++------ src/txmempool.cpp | 4 +-- src/validation.cpp | 4 +-- src/wallet/wallet.cpp | 6 ++--- 10 files changed, 25 insertions(+), 69 deletions(-) delete mode 100644 src/reverse_iterator.h diff --git a/contrib/devtools/copyright_header.py b/contrib/devtools/copyright_header.py index 81d2f5f8c11db..f46f90e52d459 100755 --- a/contrib/devtools/copyright_header.py +++ b/contrib/devtools/copyright_header.py @@ -23,7 +23,6 @@ 'src/bench/nanobench.h', 'src/crypto/*', 'src/ctpl_stl.h', - 'src/reverse_iterator.h', 'src/test/fuzz/FuzzedDataProvider.h', 'src/tinyformat.h', 'src/util/expected.h', diff --git a/src/Makefile.am b/src/Makefile.am index e687553ada601..0a91b742a016a 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -297,7 +297,6 @@ BITCOIN_CORE_H = \ psbt.h \ random.h \ randomenv.h \ - reverse_iterator.h \ rpc/blockchain.h \ rpc/client.h \ rpc/index_util.h \ diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 43feb7c261db9..ac5bce6d8cbb9 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -26,7 +26,6 @@ #include #include #include -#include #include #include #include @@ -47,6 +46,7 @@ #include #include #include +#include #include #include @@ -2124,7 +2124,7 @@ void PeerManagerImpl::UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlock if (peer == nullptr) return; LOCK(peer->m_block_inv_mutex); - for (const uint256& hash : reverse_iterate(vHashes)) { + for (const uint256& hash : vHashes | std::views::reverse) { peer->m_blocks_for_headers_relay.push_back(hash); } }); @@ -3035,7 +3035,7 @@ void PeerManagerImpl::HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, c } else { std::vector vGetData; // Download as much as possible, from earliest to latest. - for (const CBlockIndex *pindex : reverse_iterate(vToFetch)) { + for (const CBlockIndex *pindex : vToFetch | std::views::reverse) { if (nodestate->nBlocksInFlight >= MAX_BLOCKS_IN_TRANSIT_PER_PEER) { // Can't download any more from this peer break; diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 71a6bdd3f8d18..d634af2ae8887 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -15,7 +15,6 @@ #include #include #include -#include #include #include #include @@ -24,6 +23,7 @@ #include #include +#include #include std::atomic_bool fImporting(false); @@ -410,7 +410,7 @@ const CBlockIndex* BlockManager::GetLastCheckpoint(const CCheckpointData& data) { const MapCheckpoints& checkpoints = data.mapCheckpoints; - for (const MapCheckpoints::value_type& i : reverse_iterate(checkpoints)) { + for (const MapCheckpoints::value_type& i : checkpoints | std::views::reverse) { const uint256& hash = i.second; const CBlockIndex* pindex = LookupBlockIndex(hash); if (pindex) { diff --git a/src/reverse_iterator.h b/src/reverse_iterator.h deleted file mode 100644 index 4db001c04b3fb..0000000000000 --- a/src/reverse_iterator.h +++ /dev/null @@ -1,39 +0,0 @@ -// Taken from https://gist.github.com/arvidsson/7231973 - -#ifndef BITCOIN_REVERSE_ITERATOR_H -#define BITCOIN_REVERSE_ITERATOR_H - -/** - * Template used for reverse iteration in range-based for loops. - * - * std::vector v = {1, 2, 3, 4, 5}; - * for (auto x : reverse_iterate(v)) - * std::cout << x << " "; - */ - -template -class reverse_range -{ - T &m_x; - -public: - explicit reverse_range(T &x) : m_x(x) {} - - auto begin() const -> decltype(this->m_x.rbegin()) - { - return m_x.rbegin(); - } - - auto end() const -> decltype(this->m_x.rend()) - { - return m_x.rend(); - } -}; - -template -reverse_range reverse_iterate(T &x) -{ - return reverse_range(x); -} - -#endif // BITCOIN_REVERSE_ITERATOR_H diff --git a/src/test/fuzz/prevector.cpp b/src/test/fuzz/prevector.cpp index e2d65a479633f..a8e9fcd368e92 100644 --- a/src/test/fuzz/prevector.cpp +++ b/src/test/fuzz/prevector.cpp @@ -2,16 +2,14 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include -#include - #include -#include - -#include #include #include +#include +#include +#include +#include namespace { template @@ -47,7 +45,7 @@ class prevector_tester assert(v == real_vector[pos]); ++pos; } - for (const T& v : reverse_iterate(pre_vector)) { + for (const T& v : pre_vector | std::views::reverse) { --pos; assert(v == real_vector[pos]); } @@ -55,7 +53,7 @@ class prevector_tester assert(v == real_vector[pos]); ++pos; } - for (const T& v : reverse_iterate(const_pre_vector)) { + for (const T& v : const_pre_vector | std::views::reverse) { --pos; assert(v == real_vector[pos]); } diff --git a/src/test/prevector_tests.cpp b/src/test/prevector_tests.cpp index 3977a3d548ab1..a089fda5731cc 100644 --- a/src/test/prevector_tests.cpp +++ b/src/test/prevector_tests.cpp @@ -3,16 +3,15 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include -#include - -#include #include #include - #include #include +#include +#include + BOOST_FIXTURE_TEST_SUITE(prevector_tests, TestingSetup) template @@ -57,14 +56,14 @@ class prevector_tester { for (const T& v : pre_vector) { local_check(v == real_vector[pos++]); } - for (const T& v : reverse_iterate(pre_vector)) { - local_check(v == real_vector[--pos]); + for (const T& v : pre_vector | std::views::reverse) { + local_check(v == real_vector[--pos]); } for (const T& v : const_pre_vector) { local_check(v == real_vector[pos++]); } - for (const T& v : reverse_iterate(const_pre_vector)) { - local_check(v == real_vector[--pos]); + for (const T& v : const_pre_vector | std::views::reverse) { + local_check(v == real_vector[--pos]); } CDataStream ss1(SER_DISK, 0); CDataStream ss2(SER_DISK, 0); diff --git a/src/txmempool.cpp b/src/txmempool.cpp index a011ea8afc315..e2b92ba5575eb 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -14,7 +14,6 @@ #include #include #include -#include #include #include #include @@ -30,6 +29,7 @@ #include #include +#include // Helpers for modifying CTxMemPool::mapTx, which is a boost multi_index. struct update_descendant_state @@ -195,7 +195,7 @@ void CTxMemPool::UpdateTransactionsFromBlock(const std::vector &vHashes // This maximizes the benefit of the descendant cache and guarantees that // CTxMemPoolEntry::m_children will be updated, an assumption made in // UpdateForDescendants. - for (const uint256 &hash : reverse_iterate(vHashesToUpdate)) { + for (const uint256& hash : vHashesToUpdate | std::views::reverse) { // calculate children from mapNextTx txiter it = mapTx.find(hash); if (it == mapTx.end()) { diff --git a/src/validation.cpp b/src/validation.cpp index a00fc23c1b7c8..e12b66fb86d2c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -31,7 +31,6 @@ #include #include #include -#include #include