From 6c0ed67a2f6b39b630c5748ecf214f86e9d26a5c Mon Sep 17 00:00:00 2001 From: bitflash-maintainer Date: Sun, 23 Aug 2026 11:22:32 +0000 Subject: payout: leave no temporary file behind, and say so where it will be read Every failure path in WriteFileAtomic returned without removing ".tmp". A disk that filled up once, or a single EIO, left a short file called pending_payouts.json.tmp sitting next to a good ledger for as long as the node ran. That file is worse than no file: it carries the name of a half-finished save, and the operator cleaning up after the incident is precisely the person who will move it into place and publish a ledger missing whatever had not been written yet. The failures also logged under LogPrint("payout", ...). pool_status.json is written through the same function but everything else about it is logged under "pool", so an operator running -debug=pool was told nothing when the status file could not be replaced -- the message existed and was discarded for having the wrong category. They now print unconditionally: a write failure is not a trace of normal operation, it is the file that is about to be read not being there, and gating it on a category chosen before the failure happened is how it went missing. The brief's alternative, a category argument, would still have required the right guess in advance. Properties: no failure of WriteFileAtomic leaves a .tmp behind, and no failure of it is invisible at default log settings. --- src/rpc.cpp | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) (limited to 'src/rpc.cpp') diff --git a/src/rpc.cpp b/src/rpc.cpp index f853e78..be265db 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -506,20 +506,35 @@ static void SyncDirectoryOf(const std::string& path) // write mutex for `path`: the temp name is derived from the target, so two // threads in here at once for one path is precisely the race this exists to // prevent. +// +// Every failure removes the temp file before returning. It used to be left +// where it fell, so a disk that filled up once left a short, valid-looking +// "pending_payouts.json.tmp" next to the good ledger for as long as the node +// ran. That file is a trap: it has the name of a half-finished save, and an +// operator cleaning up after an incident is exactly the person who will move +// it into place. +// +// Failures print unconditionally rather than through LogPrint(). These three +// files are written under two different debug categories ("payout" for the +// ledgers, "pool" for the status file), and a write failure is not a trace of +// normal operation -- it is the file the operator is about to read not being +// there. Gating it behind a category that had to be chosen before the failure +// happened is how "could not replace pool_status.json" went missing. static bool WriteFileAtomic(const std::string& path, const std::string& data, const char* what) { std::string tmp = path + ".tmp"; FILE* f = fopen(tmp.c_str(), "w"); if (!f) { - LogPrint("payout", "[payout] WARNING: could not write %s (%s)\n", - tmp.c_str(), what); + printf("[payout] WARNING: could not write %s (%s)\n", + tmp.c_str(), what); return false; } if (!data.empty() && fwrite(data.c_str(), 1, data.size(), f) != data.size()) { - LogPrint("payout", "[payout] WARNING: short write to %s (%s)\n", - tmp.c_str(), what); + printf("[payout] WARNING: short write to %s (%s)\n", + tmp.c_str(), what); fclose(f); + remove(tmp.c_str()); return false; } // Before the rename, not after: publishing a name that points at nothing @@ -527,9 +542,10 @@ static bool WriteFileAtomic(const std::string& path, const std::string& data, // made durable, do not rename -- the previous file is still intact and // still correct, and the next write will try again. if (!SyncFileHandle(f)) { - LogPrint("payout", "[payout] WARNING: could not flush %s to disk (%s)" - " -- keeping the previous file\n", tmp.c_str(), what); + printf("[payout] WARNING: could not flush %s to disk (%s)" + " -- keeping the previous file\n", tmp.c_str(), what); fclose(f); + remove(tmp.c_str()); return false; } // fclose() is where buffered data actually reaches the OS, so a failure @@ -537,8 +553,9 @@ static bool WriteFileAtomic(const std::string& path, const std::string& data, // destroy the good copy, which is exactly what these files exist to // prevent. if (fclose(f) != 0) { - LogPrint("payout", "[payout] WARNING: could not close %s (%s)\n", - tmp.c_str(), what); + printf("[payout] WARNING: could not close %s (%s)\n", + tmp.c_str(), what); + remove(tmp.c_str()); return false; } #ifdef _WIN32 @@ -546,8 +563,9 @@ static bool WriteFileAtomic(const std::string& path, const std::string& data, remove(path.c_str()); #endif if (rename(tmp.c_str(), path.c_str()) != 0) { - LogPrint("payout", "[payout] WARNING: could not replace %s (%s)\n", - path.c_str(), what); + printf("[payout] WARNING: could not replace %s (%s)\n", + path.c_str(), what); + remove(tmp.c_str()); return false; } SyncDirectoryOf(path); -- cgit v1.2.3