Also re-`static`'d a method in src/script/standard.cpp which we had made globally
visible for some historical reason, but which I noticed diffing that file against
upstream.
This is the 43-commit descriptor wallet PR. It was remarkably easy to merge, given
its magnitude. With this commit Elements supports importing Bitcoin descriptors
and deriving (Bitcoin) addresses, though of course it does not support blinding
yet. That is a post-rebase project.
The material changes were:
1. Changing constants in the tests (super annoying but nothing surprising)
2. Adding a missing "skip if this coin is not ours" check in src/script/sign.cpp
which was causing us to erroneously remove existing witnesses from transactions.
This wasn't a problem before this commit since we would only ask specific
scriptpubkeymans to sign, and we'd never ask any to sign inputs we didn't
own. Andy simplified the logic here to always try every scriptpubkeyman,
which means they have to play a bit more nicely with each other.
Other than that, this was a big diff with many conflicts but literally all of
them were "we both added code" and the resolution was to take both sides.
7486e2771e Tests: Unit test related to WalletDB ReadKeyValue (Bushstar)
32def8d1c2 Catch ios_base::failure specifically (Peter Bushnell)
Pull request description:
In https://github.com/bitcoin/bitcoin/pull/2950 a hash of the pubkey and private was added to speed up key import, this was made backwards compatible by reading the hash in a try block with an ellipses catch all in case the hash was not present.
CDataStream::read() specifically throws std::ios_base::failure, backwards compatibility expects only that error to be thrown, if something else gets thrown we should not be catching it. The change in this commit is to catch that exception only. If any other exception is thrown other than std::ios_base::failure it will be caught by the wider try block and an error written to the log and/or console.
CDataStream::read() throwing std::ios_base::failure.
2c364fde42/src/streams.h (L191)
Wider catch statements that pick up all others exceptions other than ios_base::failure.
2c364fde42/src/wallet/walletdb.cpp (L425)2c364fde42/src/wallet/walletdb.cpp (L430)
ACKs for top commit:
laanwj:
Code review ACK 7486e2771e
Tree-SHA512: 5364bf935af8ec603bf5b8fef8c23b5cdaa4fe3506090cff988413221f2eaa99f7a91929afb42a35f8881ce2328744a0d32052da51ca0a5b2e65b6809e97f604
Moved GetOnlinePakKey to the scriptpubkey manager; the other blinding
stuff remains in the CWallet. At some point after the rebase we should
consider whether there's a better separation to be had between the PAK
related stuff now that we have wallet boxes.
This is the start of some nontrivial wallet refactoring by achow. See
https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Class-Structure-Changes
for a high-level design.
This PR moves some stuff out of CWallet into a dummy "box" LegacyScriptPubKeyMan
which is (currently) very tightly coulped to CWallet. Because of the coupling there
are currently null-checks that cannot fail, things which assume non-nullness which
will eventually be wrong, and a plethora of currently-equivalent ways to get from
a CWallet to a provider or back.
Our approach is basically to ignore the refactoring; leave the blinding key stuff
in CWallet and have it call into the wallet's provider when we are obtaining the
underlying keys.
Later, probably in a post-rebase PR, we should rethink how we manage blinding keys
to more closely match Core's "all keys go into providers" model.
Checks are now consolidated in MakeBerkeleyDatabase function instead of
happening in higher level code.
This commit does not change behavior except for error messages which now
include more complete information.
New function is not currently called but will be called in upcoming commits. It
moves database path checking, and existence checking, and already-loaded
checking, and verification into a single function so this logic does not need
to be repeated all over higher level wallet code, and so higher level code does
not need to change when SQLite support is added in
https://github.com/bitcoin/bitcoin/pull/19077. This also lets higher level
wallet code make fewer assumptions about the contents of wallet directories.
This commit just adds the new function and does not change behavior in any way.
0bbe26a1af wallet: filter for keys only before record deser in salvage (Andrew Chow)
544e12a4e8 walletdb: Add KeyFilterFn to ReadKeyValue (Andrew Chow)
Pull request description:
When salvaging a wallet, the only things that matter are the private keys. It is not necessary to attempt to deserialize any other records, especially if those records are corrupted too.
This PR adds a `KeyFilterFn` function callback to `ReadKeyValue` that salvage uses to filter for only the records that it wants. Of course doing it this way also lets us do other filters in the future from other places should we so desire.
ACKs for top commit:
ryanofsky:
Code review ACK 0bbe26a1af. Looks great! This should make the recovery code more robust. Normally it'd be good to have a test case for the problem this fixes, but Marco already wrote one in #19078, so I think we're covered
laanwj:
Code review ACK 0bbe26a1af
Tree-SHA512: 8e3ee283a22a79273915711c4fb751f3c9b02ce94e6bf08dc468f1cfdf9fac35c693bbfd2435ce43c3a06c601b9b0a67e209621f6814bedfe3bc7a7ccc37bb01
-zapwallettxes is made a hidden option to inform users that it is
removed and they should be using abandontransaction to do the stuck
transaction thing.
Add a KeyFilterFn callback to ReadKeyValue which allows the caller to
specify which types to actually deserialize. A KeyFilterFn takes the
type as the parameter and returns a bool indicating whether
deserialization should continue.
