wallet: make wallet.dat a file you can actually back up - #46
Merged
Conversation
Issue #40: a wallet.dat copied out on its own will not open anywhere else. The file is intact -- same bytes, same keys -- and the node refuses it with "Db::put: Invalid argument", thrown from a path that aborted the process. The reporter believes they lost funds to this, having saved the one file anybody would think to save. The chain, from the bottom: _endthread() is pthread_exit() on POSIX, and glibc implements that by throwing abi::__forced_unwind through the stack. That is not a real exception; catching it without rethrowing is a fatal error by contract. CATCH_PRINT_EXCEPTION ended in a bare catch(...), so every Linux shutdown ended: UNKNOWN EXCEPTION bitflash-node in ThreadSocketHandler() FATAL: exception not rethrown An abort, mid-shutdown, so nothing after StopNode() ran. Which mattered because Berkeley DB stamps every page with a log sequence number belonging to the environment that wrote it. Carried elsewhere, those numbers point at logs that do not exist and BDB refuses to write. DBFlush(true) calls lsn_reset, which is precisely the cure -- and DBFlush has been declared since 2009 and called from nowhere. Even had it been called, the abort stood in front of it. So the wallet was never released from its directory, and the one file users copy was the one file that could not travel. Three fixes, each covering a different case: - Rethrow abi::__forced_unwind, and call DBFlush(true) on both shutdown paths. Shutdown completes, and wallet.dat is left portable by construction. - Adopt orphaned files on open: a freshly created database environment beside pre-existing .dat files means they came from elsewhere, so reset their sequence numbers. This is what recovers wallets already stranded by older builds -- including, if they still hold the file, the reporter's. - BackupWallet(), reachable as /backupwallet=FILE, which checkpoints, copies, and clears the copy's sequence numbers. Bitcoin 0.1.0 had a backup command; this tree lost it in the wxWidgets removal and has shipped with no way to take a backup since. DBFlush's own txn_checkpoint and lsn_reset are wrapped too -- they throw, and throwing on the way out is how this started. Verified on Linux and Windows: shutdown exits 0 rather than 134, DBFlush runs, and a lone wallet.dat opens in an empty directory with no exceptions and byte-identical key records (defaultkey, key, name, setting, version) before and after. Not addressed here, and the larger half of #40: keys are generated as they are needed rather than derived from a seed, so any backup is a snapshot and coins paid to addresses created after it are not in it. /backupwallet says so on every run. The real answer is deterministic derivation, which is a wallet format change and its own piece of work.
This was referenced Jul 30, 2026
The PR description claimed this section existed and it did not. Covers the two things that cost money and are not obvious: a lone wallet.dat is refused elsewhere because Berkeley DB ties it to the database/ directory beside it, and every backup is a snapshot because keys are made on demand rather than derived from a seed.
Owner
Author
|
Pushed one commit after opening this: the README section on backups. Flagging it rather than letting it land quietly, because the description above already said "the README change says so too" — and it did not. I wrote that before writing the section and never checked. The push makes the claim true instead of leaving it false; it is the only change since the PR opened, and it touches no code. |
Merged
Bitflash-sh
added a commit
that referenced
this pull request
Jul 30, 2026
Wallet backup and clean shutdown (#46). Two reasons not to sit on this: every Linux node aborts on the way out instead of releasing wallet.dat, and until a node has shut down cleanly once, the wallet it wrote cannot be moved anywhere else.
6 tasks
Bitflash-sh
added a commit
that referenced
this pull request
Jul 30, 2026
The last door out. Everything else assumes Berkeley DB will cooperate. When it will not -- a build that cannot read the file, a truncation, an environment past recovering -- the keys are usually still perfectly intact and were unreachable anyway. #40 is one person's account of exactly that, and #46 only rescues the cases where BDB still opens the file at all. bitflash -dumpwallet=keys.txt bitflash -importwallet=keys.txt One key per line with its address and label. No database, no environment, no version coupling: anything that can read a line can recover from it. The format is the DER private key in hex, which round-trips exactly through SetPrivKey. Not interchangeable with other wallets -- worth having eventually, but a WIF encoder I got subtly wrong would be worse than no encoder at all, and this file exists for the case where nothing else works. Care taken, because the failure modes here cost money: - Refuses to overwrite an existing file. A mistyped path must not eat the only copy of something irreplaceable. - Written owner-only on POSIX, before any key reaches it. Windows inherits the folder's ACL; the README says so rather than pretending otherwise. - Newlines in labels are flattened, so a label cannot forge an extra entry on read-back. - Import skips keys already present, so running it twice does nothing, and one unreadable line is reported and stepped over instead of costing the reader every other key in the file. Verified on Linux and Windows: a key exported from one wallet imports into a different one and its address then appears there; re-importing adds nothing; a corrupted line is skipped while the rest still load; the dump is 0600 and refuses to overwrite. This does not solve #47. Keys still are not derived from a seed, so a dump is as much a snapshot as any other backup. What it does is make the keys reachable no matter what state the database is in.
Vic-Nas
added a commit
to Vic-Nas/bitflash-mod
that referenced
this pull request
Jul 30, 2026
…itflash-sh#46) Issue Bitflash-sh#40's first half: a wallet.dat copied out on its own doesn't open anywhere else, even though it's byte-identical and holds the same keys. Root cause chain, from the bottom: _endthread() is pthread_exit() on POSIX, which glibc implements by throwing abi::__forced_unwind through the stack -- not a real exception, and catching it without rethrowing is a fatal error by contract. CATCH_PRINT_EXCEPTION's bare catch(...) did exactly that, so every Linux shutdown aborted mid-way rather than completing. That mattered because Berkeley DB stamps every page with a log sequence number tied to the environment that wrote it; DBFlush(true) is what clears that (via lsn_reset) before the file is safe to move, and DBFlush has been declared since 2009 and never actually called on either shutdown path. So the wallet was never released from its directory, and the one file anyone would think to back up was the one file that couldn't travel. Three fixes: - Rethrow abi::__forced_unwind rather than swallowing it (util.h), and call the now-effective DBFlush(true) on both shutdown paths (headless and GUI) in main_gui.cpp. - Adopt orphaned wallet.dat/blkindex.dat on open: a freshly created database/ environment beside pre-existing .dat files means those files came from elsewhere, so their sequence numbers get reset. Recovers wallets already stranded by older builds. - BackupWallet(), reachable as /backupwallet=FILE: checkpoints, copies, clears the copy's sequence numbers. Bitcoin 0.1.0 had a backup command; this tree hasn't had one since the wxWidgets removal. DBFlush's own lsn_reset call is now wrapped too (its txn_checkpoint already was, from this fork's earlier BDB-exception fix) -- letting it throw on the way out is how this whole chain started. Adapted rather than copied: this fork's dead-code sweep earlier this session correctly removed FileExists as unused at the time -- it has a real caller again now (both the orphan-adoption check and BackupWallet), restored scoped to db.cpp rather than re-exposed in util.h, since that's the only file that needs it. DBFlush's txn_checkpoint wrapping already existed here from a prior fix; only lsn_reset needed adding. Updated the README's backup guidance to point at /backupwallet as the correct way to do this, rather than copying the data directory by hand. Still not addressed, and the larger half of Bitflash-sh#40: keys are generated on demand rather than derived from a seed, so a backup is a snapshot and coins paid to addresses created after it aren't in it. That's a wallet-format change and its own piece of work, matching what upstream's own PR description says about scope. Verified by compiling every touched and dependent file for real against the actual headers, with the format-checking flags from the previous commit live -- clean throughout.
Vic-Nas
added a commit
to Vic-Nas/bitflash-mod
that referenced
this pull request
Jul 30, 2026
Went back and actually integrated this instead of leaving it as a
note. It was excluded from the previous pass with a weaker reason
('scope already covered this pass') than Bitflash-sh#46 -- which was pulled in
the same pass and is comparably sized and no less delicate -- really
warranted. Bitflash-sh#47 stays out; that one's a genuinely different kind of
change (wallet format, not a bug fix), which is a real distinction,
not just a smaller version of this one.
/dumpwallet=FILE and /importwallet=FILE: the case Bitflash-sh#46 doesn't cover,
where Berkeley DB itself won't cooperate at all (unreadable version,
truncated file, an unrecoverable environment) rather than just being
tied to the wrong directory. A plain text file, one key per line
(hex private key, address, label), no database, no environment, no
version coupling.
Refuses to overwrite an existing dump (a mistyped path must not eat
the only copy of something irreplaceable), written owner-only on
POSIX before any key reaches it, labels with embedded newlines are
flattened so they can't forge an extra entry on read-back, and
import skips keys already present so running it twice is a no-op --
one unreadable line is reported and stepped over rather than costing
every other key in the file.
FileExists (restored locally in db.cpp for Bitflash-sh#46) and HexStr (already
in util.h) covered both new call sites without needing anything
else. README gets a short mention in our condensed style rather than
upstream's fuller section.
Compiled db.cpp and main_gui.cpp for real against the actual headers
with the format-checking flags live -- clean.
This was referenced Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the first half of #40, where the reporter believes they lost funds after backing up
wallet.daton its own — the one file anybody would think to save.The chain
Working from the bottom up, because the wallet symptom is the last link:
1. Shutdown aborted.
_endthread()ispthread_exit()on POSIX, and glibc implements that by throwingabi::__forced_unwindthrough the stack. It is not a real exception; catching it without rethrowing is a fatal error by contract.CATCH_PRINT_EXCEPTIONended in a barecatch(...), so every Linux shutdown ended:2. So nothing after
StopNode()ran. Which mattered because —3. Berkeley DB welds files to environments. Every page carries a log sequence number belonging to the environment that wrote it. Carried elsewhere those numbers point at logs that do not exist, and BDB refuses to write:
Db::put: Invalid argument.DBFlush(true)callslsn_reset, which is exactly the cure — andDBFlushhas been declared since 2009 and called from nowhere in this tree. Even had it been called, the abort stood in front of it.4. So the wallet was never released from its directory, and the one file users copy was the one file that could not travel.
Three fixes, three different cases
abi::__forced_unwind, callDBFlush(true)on both shutdown paths.datfiles means they came from elsewhere, so reset their sequence numbersBackupWallet(), reachable as/backupwallet=FILE— checkpoint, copy, clear the copy's sequence numbersDBFlush's owntxn_checkpointandlsn_resetare wrapped too — they throw, and throwing on the way out is how this started.Verification
Both platforms:
DBFlush(true)runs — previously 0 times, everwallet.datin an empty directory opens with 0 exceptions, loggingCDB() : adopted wallet.dat from another environmentdefaultkey,key,name,setting,version, same digest/backupwalletoutput reopens standalone in a fresh directory; a bad destination reports an error and writes nothing, no abortBefore, on the same file:
Not addressed here
The larger half of #40. Keys are generated as needed rather than derived from a seed, so any backup is a point-in-time snapshot and coins paid to addresses created after it are not recoverable from it.
/backupwalletsays so on every run, and the README change says so too, but the real answer is deterministic derivation — a wallet format change and its own piece of work, worth its own issue rather than being smuggled in here.One known limitation: the Windows GUI binary is linked
-mwindows, so/backupwalletprints todebug.lograther than the console. Pre-existing, not introduced here.