This one took a little bit of work. Basically everywhere that Andy's new code
didn't compile, I looked for a similarly shaped line in the pre-PR diff between
Bitcoin and Elements and adjusted it in the same way. Was pretty typical; all
CAmounts become CAmountMaps, etc.
I then spent about 7 hours chasing down a coin selection failure in the blocksign
functional tests, which ultimately turned out to be a Core issue. I added a
stopgap fix (commented, search for "stopgap" in wallet.cpp), and opened the issue
https://github.com/bitcoin/bitcoin/issues/20347
There was also a bug where we were not estimating change output sizes correctly
in case we had massive blinded outputs, which I had to fix to get the functional
tests to pass.
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.
Also uncommented a bunch of PSBT functional tests (had to add a fee output
to one transaction, update `find_output` to skip CT outputs, and change two
constant checks at the end of the commented-out section).
I'm sorry, this should have been a few commits, but it got away from me
as the scope of this refactoring was not immediately obvious. It essentially
just moves code around so hopefully is not too hard to review. --asp
This PR introduces a `avoid_reuse` flag to the `getnewaddress` `sendtoaddress` `getbalance`
RPCs, which we've also added some asset-specific stuff. Since both changes have default
values they aren't individually breaking, but together they are since some existing
asset-related RPC workflows now require the user to jam a `False` in there to make
the new avoid-reuse stuff go away.