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.
I really like this PR, but it wound up being pretty nontrivial to merge.
The crux is that it pulls PSBT signing logic into scriptpubkey manager,
which is where it belongs, but for us this means reasoning about pegins
inside script/sign.cpp.
However, sign.cpp is part of libbitcoin_common, which does not include
anything for reasoning about PoW or RPC (lol) or anything heavy about
that. This means that some pegin validation had to remain split between
the wallet/rpc layer and sign.cpp. I added a new file script/pegins.cpp
which has the (one) method we actually need in sign.cpp.
Aside from that, this diff is very large but is mostly just moving our
code changes to wallet/psbtwallet.* into wallet/wallet.* where those
functions now live. As far as review, it's probably not worthwhile to
spend too much too much time on this since it's going to be change
again in #16528 and others. The test coverage is pretty extensive.
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 PR associates OutputType::BECH32 to PAK online keys, where before we
were able to directly access keys from the keypool. In a future refactoring
we should give PAK keys their own output type (and own scriptpubkey manager)
so that the wallet won't accept payments "to the PAK key".
Also changes `ReserveDestination::SetBlindingPubKey` to use a visitor pattern
to apply a blinding pubkey to a destination directly, rather than using the
old hacky method of regenerating the destination by pulling its key (which
is no longer contained in the class) out and giving it to a new constructor.
This was a long-overdue refactoring and the minimal-diff way to get the code
compiling (and it's not bad, maybe 10LOC to add a new visitor class) but
nonetheless I apologize for sticking this into a merge commit.
This Elements PR includes components of Core PR #17211, which since the
refactors to use effective value landed, no longer provides the right
error message when a user provides an unowned input from a wallet tx.
See https://github.com/bitcoin/bitcoin/pull/17211#pullrequestreview-528389011
This breaks a functional test which was included in this PR, but which
conveniently has been changed in the current version of the Core PR. I
fixed the behavior (commented, in SelectCoins) rather than updating the
test to the most recent version.
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.
Recreated addresses for functional tests using `hal elements address create --script`
with the given script from data/rpc_bip67.json, taking the P2SH output
FIXME: we disable standardness checks for the non-PAK node in the PAK tests.
This is because of a bug in Elements which causes non-PAK nodes to reject
pegouts for standardness reasons. Need to fix it after the rebase.
