@maflcko @ryanofsky
There's a subtler issue here that's worth calling out explicitly. Snippet from WriteSettings:
std::ofstream file;
file.open(path.std_path());
if (file.fail()) {
errors.emplace_back(strprintf("Error: Unable to open settings file %s for writing", fs::PathToString(path)));
return false;
}
The real problem is that file.open() truncates the existing file (effectively O_TRUNC semantics) before any write happens, and this truncation succeeds even when the disk is full — truncating to zero length doesn't require free space, so the OS has no reason to error out at open() time. The subsequent write will fail as expected, but by then the previously-good settings file has already been clobbered.
So returning false on write failure isn't enough on its own: it correctly reports the error, but the destructive truncation has already happened regardless.
A more robust fix is to write to a temp file first, verify it opens and the write succeeds, and only then atomically rename it over the original. That way, a failed write never destroys the existing settings file — it just fails to update it.
This is becoming more likely to bite people in practice: as chain state pushes toward 1TB, nodes running on 1TB drives will increasingly hit disk-full conditions. Fixing this means the existing error-handling path actually does its job, instead of being preempted by a silent truncation that masquerades as settings corruption.
Suggested fix here
Should a new issue be opened to address this?
