Bitcoin Core

test: wallet: Constructing a DSPKM that can't TopUp() throws.

refactor: wallet: Don't reuse WALLET_BLANK flag for born-encrypted wallets.

With the split between LoadWallet and CreateNew, it's no longer
necessary to utilize the blank flag to prevent the wallet from having
descriptors automatically being generated. Instead, CreateNew can take a
separate parameter to indicate whether the wallet is to be born
encrypted and therefore should not have any keys generated.

wallet migration, fuzz: Migrate hd seed once

If a wallet has multiple HD chains that have the same seed, we should
only migrate that seed a single time.

This fixes a fuzz crash that occurs once the return value of
AddDescriptorKeyWithDB is checked during descriptor construction.

fuzz: Skip adding descriptor to wallet if it cannot be expanded

lint: use requirements.txt

wallet: Disallow . and .. from wallet names

Wallet names that are also paths that contain . and .. are unintuitive
and can result in unexpected behavior, particularly in migration.
Therefore we should disallow users from specifying wallet names that
contain . and .. as path elements.

refactor: use SignOptions for SignTransaction

dbwrapper: reuse iterator scratch stream

`CDBIterator::Seek()` and `CDBIterator::GetValue()` only need a temporary owning buffer for immediate use.

Keep one preallocated `DataStream` scratch member on the non-copyable iterator and guard each use with `ScopedDataStreamUsage`.
`SeekImpl()` consumes the serialized key during the immediate LevelDB seek, and `GetValue()` still copies LevelDB value bytes into the owning scratch stream before in-place deobfuscation.

A single stream is enough because these methods cannot overlap on the same iterator without re-entering or concurrently using it, which the guard asserts against.
The preceding test covers repeated seeks and failed value deserialization followed by a successful read from the same entry.

Co-authored-by: Andrew Toth <andrewstoth@gmail.com>

refactor: disable default std::hash for CTransactionRef

The default std::hash for shared_ptr compares by pointer.

CTransactionRefHash or a custom hasher should be used instead.

Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>

rpc: combinerawtransaction now rejects unmergeable transactions

Previously, combinerawtransaction would silently return the first tx when
asked to combine unrelated txs. Now, it will check tx mergeability and
throws a descriptive error if tx cannot be merged.

test: add coverage for loading a wallet in a non-writable directory

Previously, wallets in non-writable directories were loaded,
leading to crashes on any subsequent write.

test: add coverage for wallet creation in non-writable directory

wallet: handle non-writable db directories

1) For wallet load, this fixes a crash.

We currently allow loading wallets located on non-writable directories.
This is problematic because the node crashes on any subsequent write.
E.g. generating a block is enough to trigger it.

2) For wallet creation, this improves the returned error msg.

Before: creating a wallet would return a generic error:
"SQLiteDatabase: Failed to open database: unable to open database file"

After: creating a wallet returns:
"SQLiteDatabase: Failed to open database in directory <dir_path>: directory
is not writable"

bench: drop duplicate balance benchmark

`WalletBalanceMine` duplicated `WalletBalanceClean` exactly.
Remove the duplicate registration so the balance benchmark list stays distinct.

util/stdmutex: Drop StdLockGuard

ArgsManager: automate checking for correct command options

bitcoin-wallet: use command-specific options

rpc: Add exportasmap RPC

refactor: use `SpanReader` in `PrevectorDeserialize`

`PrevectorDeserialize` only needs a reusable read-only view over fixed serialized bytes.
Keeping a mutable `DataStream` around just to call `Rewind()` is unnecessary.

Rebuild a fresh `SpanReader` for each benchmark run and remove `DataStream::Rewind()`, whose remaining use was this benchmark-only reset path.
The benchmark can now serialize exactly the 1000 entries it deserializes, so drop the stale extra element that used to avoid full consumption.

Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>

refactor: replace `DataStream` with `SpanReader` in block deserialization tests

These benchmark inputs are immutable fixture bytes, so `DataStream` adds an unnecessary owned buffer and the setup needed to recreate or preserve its state.

Use `SpanReader` for block deserialization in `checkblock` instead.
This keeps `DeserializeBlockTest` focused on deserialization work, while `CheckBlockTest` still uses untimed setup only to rebuild a fresh uncached `CBlock` for the timed `CheckBlock()` call.

Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>

refactor: use `SpanReader` in `TestBlockAndIndex`

`TestBlockAndIndex` still deserialized its fixed block fixture through `DataStream` and appended a dummy byte to avoid compaction after full consumption.

Use `SpanReader` for that fixture instead.
This removes the leftover dummy-byte workaround and reads the immutable fixture through a read-only view.

dbwrapper: use `SpanReader` for iterator keys

`CDBIterator::GetKey()` only deserializes the current LevelDB key once.
`GetKeyImpl()` already exposes the current key as a contiguous borrowed byte span, and `GetKey()` creates a fresh local reader and only performs immediate forward reads before returning.

Switch this path to `SpanReader` so the key bytes are read in place instead of being copied into a temporary `DataStream`.
This keeps the same exception swallowing and `bool` return semantics while avoiding the extra allocation and copy.

The preceding test locks down the subtle safety property that matters here: a failed decode must not consume the current iterator entry.
Note that the same simplification does not apply to `GetValue()`, because that path deobfuscates the value bytes in place first and still needs an owning mutable buffer.

test: cover failed `CDBIterator::GetKey()` deserialization

The upcoming change will replace the temporary owning `DataStream` inside `CDBIterator::GetKey()` with a borrowed reader over the current LevelDB key bytes.
The copied `DataStream` currently insulates the iterator entry from a failed decode, so the optimization is only safe if a deserialization failure still returns `false` and leaves the same key/value readable afterward.

Extend `dbwrapper_iterator` to read a one-byte key as a `uint16_t`.
The read must fail, return `false`, and still allow the same key and value to be read afterward.
This would fail if `GetKey()` stopped swallowing deserialization exceptions, or if a failed decode started consuming shared iterator state instead of only temporary reader state.

Drop the dead `const_cast` in the test while here, since `dbw` is already non-const.

Locking down that contract first makes the following `SpanReader` switch a behavior-preserving optimization.

wallet: remove fUpdate argument from AddToWalletIfInvolvingMe

Its only usage passes true.

wallet: remove update_tx argument from SyncTransaction

Its only usage passes true.