do_save flushed the temp wallet and renamed it over the wallet with no
fsync of the temp file, the .bak or the directory, so after a power cut
the rename could be on disk while the data was not, leaving a torn
wallet. The readers then panicked on malformed bytes instead of
returning an error (the gzip Decoder and WalletTx unwraps in
LightWallet::read, the version asserts in data.rs and walletzkey.rs,
from_utf8/diversifier unwraps, and vec![0; len] on a corrupt length
prefix, which can abort the process outright), so read_from_disk's .bak
fallback was never reached. And the fallback itself was destructive: it
copied the .bak over the wallet before reading it, on ANY error, so the
damaged file was lost, a transient I/O error replaced a good newer
wallet with an older .bak, and a wallet written by a newer app ("Don't
know how to read wallet version") was overwritten too, dropping imported
keys that are not in the seed.
- do_save: sync_all the temp file (removed on error), copy the .bak,
tighten and sync it through a write handle (FlushFileBuffers needs
write access on Windows), rename, sync the copy-fallback destination,
then fsync the directory on unix. No MoveFileEx write-through on
Windows.
- read_from_disk: each parse runs in read_wallet_file under
catch_unwind (panic = "unwind" is kept for release builds). The .bak
is used only when the wallet fails with InvalidData, UnexpectedEof or
InvalidInput, and it is read first, so a bad backup leaves both files
as they were. A damaged wallet whose header says encrypted is not
replaced by a plaintext .bak (only an older build leaves one): the
open fails with an explanation and nothing changes. Otherwise the
damaged wallet is moved (or, failing that, copied) to
<wallet>.dat.corrupt-<unix secs>, with a -N suffix while that name is
taken (a dangling symlink counts as taken), and is never deleted. The
recovered wallet is tightened as before.
- read_from_disk_reporting returns the same client plus "recovered from
backup; damaged file kept as <name>" after a recovery, for the FFI;
read_from_disk wraps it. Both stages are also written to stderr,
since the logger is not initialised yet when the wallet is opened.
- Readers return Err: a newer wallet version is ErrorKind::Unsupported
(was InvalidData), and so is a newer WalletTx or key version;
Decoder::new and WalletTx::read propagate with ?; a bad Utxo version,
a negative or non-UTF-8 or non-R Utxo address, non-UTF-8 metadata
addresses, an invalid diversifier and a viewing key with no default
address are InvalidData. Every length-prefixed string and address is
read through read_bytes_bounded (take(len).read_to_end, no cap, std's
own short-read error), so a corrupt length is never allocated up
front.
Flagged behaviour changes:
- fsync order: tmp, then .bak, then rename, then the dir (unix); saves
at exit, encrypt and decrypt still run on the UI thread and get
slower on slow disks;
- the readers return Err instead of panicking;
- the .bak fallback runs only for corrupt files, and the damaged file
is kept as .dat.corrupt-<ts> (every copy is kept; one from a
plaintext era keeps its seed, inside the 0700 dir);
- a wallet with a newer format errors (Unsupported) and is never
replaced;
- a damaged encrypted wallet whose backup predates encryption is not
opened.
Wallet format v9 is unchanged; valid files read and write byte for byte
as before. No ABI or symbol change.
Tests: a wallet whose parse panicked recovers from the .bak, the
garbage is kept as .dat.corrupt-<ts> and the report names it, and the
next open reports nothing; a newer-version wallet is Unsupported and
both files are unchanged (this fails when only the version error kind is
reverted); an unreadable (0000) wallet is a PermissionDenied error that
changes nothing either; a corrupt wallet with
a corrupt or no .bak changes nothing; a damaged encrypted wallet with a
pre-encryption plaintext .bak is refused with both files unchanged and
nothing moved aside; a recovery never overwrites an earlier
.dat.corrupt-<ts> or follows a dangling symlink; every truncation of a
wallet and of a WalletTx is an Err, crafted Utxo, metadata, note and key
records are the expected Err kinds, and a u64::MAX length returns at
once.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
DanSandClaude Opus 5.5
31304a1a63