Currently when a user uses `signrawtransaction` on a transaction that is
already partially signed with SIGHASH_RANGEPROOF signatures, the existing
signatures will be deleted because they are not recognized as valid. This
makes it impossible to collabratively sign transactions using this RPC.
Fix this.
Several conflicts in the C++ code related to the new `flags` parameter
to `CheckSignature` and the corresponding function being renamed upstream
to `CheckSignatureECDSA`.
Several conflicts in the test harness as Steven sorta pulled the new
upstream ECKey module into the Python code, and the actual upstream
code was slightly different. Also needed to update the feature_taproot
code to always use the non-RANGEPROOF sighash since dynafed is not
enabled in the Taproot test.
Also had to pull the `set_wif` method out of `ECKey` and inline it because
otherwise it triggers a "circular inclusion" error between script.py (which
would pull in `base58_to_bytes` from address.py) and address.py (which now
pulls in some taproot EC related stuff from script.py).
Noticed that #960 does not test the "sighash rangeproof flag set but no
witnesses" case.
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 adds a `TxoutType::WITNESS_V1_TAPROOT` for P2TR outputs, and permits spending
them in standardness rules. No corresponding `CTxDestination` is added for it,
as that isn't needed until we want wallet integration. The taproot validation flags
are also enabled for mempool transactions, and standardness rules are added
(stack item size limit, no annexes).
In preparation for adding Schnorr versions of `CheckSig`, `VerifySignature`, and
`ComputeEntry`, give them an ECDSA specific name.
-BEGIN VERIFY SCRIPT-
sed -i 's/CheckSig(/CheckECDSASignature(/g' $(git grep -l CheckSig ./src)
sed -i 's/VerifySignature(/VerifyECDSASignature(/g' $(git grep -l VerifySignature ./src)
sed -i 's/ComputeEntry(/ComputeEntryECDSA(/g' $(git grep -l ComputeEntry ./src)
-END VERIFY SCRIPT-
-BEGIN VERIFY SCRIPT-
# General rename helper: $1 -> $2
rename_global() { sed -i "s/\<$1\>/$2/g" $(git grep -l "$1"); }
# Helper to rename TxoutType $1
rename_value() {
sed -i "s/ TX_$1,/ $1,/g" src/script/standard.h; # First strip the prefix in the definition (header)
rename_global TX_$1 "TxoutType::$1"; # Then replace globally
}
# Change the type globally to bring it in line with the style-guide
# (clsses are UpperCamelCase)
rename_global 'enum txnouttype' 'enum class TxoutType'
rename_global 'txnouttype' 'TxoutType'
# Now rename each enum value
rename_value 'NONSTANDARD'
rename_value 'PUBKEY'
rename_value 'PUBKEYHASH'
rename_value 'SCRIPTHASH'
rename_value 'MULTISIG'
rename_value 'NULL_DATA'
rename_value 'WITNESS_V0_KEYHASH'
rename_value 'WITNESS_V0_SCRIPTHASH'
rename_value 'WITNESS_UNKNOWN'
-END VERIFY SCRIPT-
Make sure that there are no errors set for an input after it is signed.
This is useful for when there are multiple ScriptPubKeyMans. Some may
fail to sign, but one may be able to sign, and after it does, we don't
want there to be any more errors there.
73aaf4ecf8 Make SignatureExtractorChecker private to its own file (Ben Woosley)
Pull request description:
~If we add a CTxIn constructor to SignatureData, then constructing the
SignatureData directly is no more verbose than calling DataFromTransaction,
and grants the caller additional flexibiliy in how to provide the CTxIn.~
A simple change to enhance encapsulation.
ACKs for top commit:
MarcoFalke:
utACK 73aaf4ecf8
laanwj:
ACK 73aaf4ecf8
Tree-SHA512: f7eafbce22b0e9917a8487e88d1f5a1061f2a0959ae1a097cbd9c8ea0d774edfb807da56813cb5fb26f6ca98499a0604a8ff024c198a7c8dc755164de66d972a
Moves all of the various SigningProviders out of sign.{cpp,h} and
keystore.{cpp,h}. As such, keystore.{cpp,h} is also removed.
Includes and the Makefile are updated to reflect this. Includes were largely
changed using:
git grep -l "keystore.h" | xargs sed -i -e 's;keystore.h;script/signingprovider.h;g'
This is not a pure refactor; additional functionality is added in
IsSegWitOutput which lets it recurse into P2SH when a
SigningProvider is provided that knows about the inner script.
When signing an input, figure out what was requested for but was unable
to be found and store it in a SignatureData.
Return this information in SignPSBTInput.
Move non-wallet PSBT code to src/psbt.{h,cpp}, and PSBT wallet code to
src/wallet/psbtwallet.{h,cpp}. This commit contains only code movement (and
adjustments to includes and Makefile.am.)
This commit makes the minimal changes necessary to fix compilation once CTransaction(const CMutableTransaction &tx) is made explicit. In each case an explicit call `CTransaction(...)` was added. Shouldn't affect behaviour or performance.
b81a186056 GetPubKey: make sigdata const (Gregory Sanders)
f7beb95a1f remove redundant KeyOriginInfo access, already done in CreateSig (Gregory Sanders)
Pull request description:
This redundancy is confusing as it looks like pubkeyhashes are special in some way based on where it's called.
Tree-SHA512: a980b7c774c6d69322945227a2b156489fb1991ebf57fe6f26096d5f8047f246a133debc241b05af67810f604b040079add3ab3d30d9e2928095905a2afe17eb
6b8d86ddb8 Require a public key to be retrieved when signing a P2PKH input (Andrew Chow)
Pull request description:
If we do not have the public key for a P2PKH input, we should not continue to attempt to sign for it.
This fixes a problem where a PSBT with a P2PKH output would include invalid BIP 32 derivation paths that are missing the public key.
Tree-SHA512: 850d5e74c06833da937d5bf0348bd134180be7167b6f9b9cecbf09f75e3543fbad60d0abbc0b9afdfa51ce165aa36168849f24a7c5abf1e75f37ce8f9a13d127
