Remove the op== on PartiallySignedTransaction, which only checks that the
CTransactions are equal. Instead, check this directly in Merge, and return
false if the CTransactions are not equal (so the PSBTs cannot be merged.)
After refactoring general-purpose PSBT and transaction code out of RPC code,
for use in the GUI, it's no longer appropriate to throw exceptions. Instead we
now return bools for success, and take an output parameter for an error object.
We still use JSONRPCError() for the error objects, since only RPC callers
actually care about the error codes.
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.)
Factor out a new BroadcastTransaction function, performing the core work of the
sendrawtransaction rpc, so that it can be used from the GUI code. Move it from
src/rpc/ to src/node/.
Split up DecodePSBT, which both decodes base64 and then deserializes a
PartiallySignedTransaction, into two functions: DecodeBase64PSBT, which retains
the old behavior, and DecodeRawPSBT, which only performs the deserialization.
Add a test for base64 decoding failure.
30d0f7be6e rpc: Fix for segfault if combinepsbt called with empty inputs (benthecarman)
Pull request description:
Fixes#15300
Tree-SHA512: 25e7b4e6e48d8b0d197f0ab96df308fff33e2110f8929cb48914877fa7f4c4a84f173b1378fdb2dec5d03fe7d6d1aced4b577e55f9fe180d8147d9106ebf543f
04da9f4834 [RPC] Update getrawtransaction interface (Amiti Uttarwar)
Pull request description:
- stop checking unspent UTXOs for a transaction when txindex is not enabled, as per conversation here: https://github.com/bitcoin/bitcoin/issues/3220#issuecomment-377458383
- code contributed by sipa
Tree-SHA512: aa07353bccc14b81b7803992a25d076d6bc06d15ec7c1b85828dc10aea7e0498d9b49f71783e352ab8a14b0bb2010cfb7835de3dfd1bc6f2323f460449348e66
b301950df3 Made expicit constructor CTransaction(const CMutableTransaction &tx). (lucash-dev)
faf29dd019 Minimal changes to comply with explicit CMutableTransaction -> CTranaction conversion. (lucash-dev)
Pull request description:
This PR is re-submission of #14156, which was automatically closed by github (glitch?)
Original description:
This PR makes explicit the now implicit conversion constructor `CTransaction(const CMutableTransaction&)` in `transaction.h`.
Minimal changes were made elsewhere to make the code compilable. I'll follow up with other PRs to address individually refactoring functions that should have a `CMutableTransaction` version, or where a `CTransaction` should be reused.
The rationale for this change is:
- Conversion constructors should not be explicit unless there's a strong reason for it (in the opinion of, for example, https://google.github.io/styleguide/cppguide.html, and https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Ro-conversion. Let me know your take on this).
- This particular conversion is very costly -- it implies a serialization plus hash of the transaction.
- Even though `CTransaction` and `CMutableTransaction` represent the same data, they have very different use cases and performance properties.
- Making it explicit allows for easier reasoning of performance trade-offs.
- There has been previous performance issues caused by unneeded use of this implicit conversion.
- This PR creates a map for places to look for possible refactoring and performance gains (this benefit still holds if the PR is not merged).
Tree-SHA512: 2427462e7211b5ffc7299dae17339d27f8c43266e0895690fda49a83c72751bd2489d4471b3993075a18f3fef25d741243e5010b2f49aeef4a9688b30b6d0631
b74a52192b fix testmempoolaccept CLI syntax (1Il1)
Pull request description:
`testmempoolaccept "hexstring"` will give a "JSON parse error". The correct syntax is `testmempoolaccept \[\"hexstring\"\]` (but seems escaping is not displayed in other areas so leaving backspaces out).
Tree-SHA512: ad755147d6db0bd3f2d8481517dab29df755a32b28a3bdb4553b1fddd1940850450d1e9a6c3bd04e4e3faa7bc09aadfd3412b4cd65e61d61ea34452831597967
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.
`testmempoolaccept "hexstring"` will give a "JSON parse error". The correct syntax is `testmempoolaccept \[\"hexstring\"\]` (but seems escaping is not displayed in other areas so leaving backspaces out).
fa483e13b3 rpc: Add RPCHelpMan for machine-generated help (MarcoFalke)
fa0d36f712 rpc: Include rpc/util.h where needed for RPCHelpMan (MarcoFalke)
Pull request description:
This introduces a manager for the RPC help generation and demonstrates its use of it in some RPCs.
It is the first non-exhaustive step toward #14378 and I will create pull requests for the next steps after this one is merged.
Tree-SHA512: 86f68322443ff01cd964aaf0ebe186be63fbebe4c47676cf7a622cc2b5305fd176bd57badfd1bbf788a036812253eb0dead74ecc3b30664c3e0d9392b2248054
88a79cb436 fix converttopsbt permitsigdata arg, add basic test (Gregory Sanders)
Pull request description:
The final check for extraneous sigdata has a flipped boolean, resulting in incorrect behavior.
Resolves https://github.com/bitcoin/bitcoin/issues/14355
Tree-SHA512: 5157a74b8ddebd7d836fba96765c4d7ed15a73d4289817353d3566a0f6803bd4bbc3f936735c517c7a83a6cbdb4052b9c61d23f6cc4ad00a6077278cd51adbd4
e13fea975d Add regression test for PSBT signing bug #14473 (Glenn Willen)
565500508a Refactor PSBTInput signing to enforce invariant (Glenn Willen)
0f5bda2bd9 Simplify arguments to SignPSBTInput (Glenn Willen)
53e6fffb8f Add bool PSBTInputSigned (Glenn Willen)
65166d4cf8 New PartiallySignedTransaction constructor from CTransction (Glenn Willen)
4f3f5cb4b1 Remove redundant txConst parameter to FillPSBT (Glenn Willen)
fe5d22bc67 More concise conversion of CDataStream to string (Glenn Willen)
Pull request description:
As discussed in the comments on #14473, I think that bug was caused primarily by failure to adhere to the invariant that a PSBTInput always has exactly one of the two utxo fields present -- an invariant that is already enforced by PSBTInput::IsSane, but which we were temporarily suspending during signing.
This refactor repairs the invariant, also fixing the bug. It also simplifies some other code, and removes redundant parameters from some related functions.
fixes#14473
Tree-SHA512: cbad3428175e30f9b7bac3f600668dd1a8f9acde16b915d27a940a2fa6d5149d4fbe236d5808fd590fb20a032274c99e8cac34bef17f79a53fdf69a5948c0fd0
Just a preparatory commit to add the header to the includes and run
clang-format to sort the include lists.
Splitting this up into a separate commit makes future scripted-diffs
easier.
081accb875 Pass chain locked variables where needed (Russell Yanofsky)
79d579f4e1 Remove uses of cs_main in wallet code (Russell Yanofsky)
ea961c3d72 Remove direct node->wallet calls in init.cpp (Russell Yanofsky)
8db11dd0b1 Pass chain and client variables where needed (Russell Yanofsky)
7e2e62cf7c Add skeleton chain and client classes (Russell Yanofsky)
Pull request description:
This creates an incomplete [`Chain`](https://github.com/ryanofsky/bitcoin/blob/pr/wipc-sep/src/interfaces/chain.h) interface in [`src/interfaces/`](https://github.com/ryanofsky/bitcoin/tree/pr/wipc-sep/src/interfaces) and begins to update wallet code to use it.
#10973 builds on this, changing the wallet to use the new interface to access chain state, instead of using CBlockIndex pointers and global variables like `chainActive`.
Tree-SHA512: 6ef05a4d8ebf57f2ad71835e4d970c9c59e34057e39e48cee76b887492c2fee907e3f6a74a9861e5a9f97cdc6823f4865ebc41ec556ab371ebca1b664c20dbea
535203075e Avoid using numeric_limits for sequence numbers and lock times (Russell Yanofsky)
bafb921507 Remove duplicated code (Hennadii Stepanov)
e4dc39b3bc Replace platform dependent type with proper const (Hennadii Stepanov)
Pull request description:
Switches to named constants, because numeric_limits calls can be harder to read and less portable.
Change was suggested by jamesob in https://github.com/bitcoin/bitcoin/pull/10973#discussion_r213473620
There are no changes in behavior except on some platforms we don't support (ILP64, IP16L32, I16LP32), where `SignalsOptInRBF` and `MutateTxAddInput` functions would now work correctly.
Tree-SHA512: 3f5c6393c260551f65a0edfba55ef7eb3625232eec8d85b1457f26e144aa0b90c7ef5f44b2fd2f7d9be3c3bcb301030a9f5473c21b3bac566cc59b8c8780737c
This commit does not change behavior. All it does is pass new function
parameters.
It is easiest to review this change with:
git log -p -n1 -U0 --word-diff-regex=.
Remove redundant arguments to SignPSBTInput -- since it needs several
bits of the PartiallySignedTransaction, pass in a reference instead of
doing it piecemeal. This saves us having to pass in both a PSBTInput and
its index, as well as having to pass in the CTransaction. Also avoid
redundantly passing the sighash_type, which is contained in the
PSBTInput already.
Switches to named constants, because numeric_limits calls can be harder to read
and less portable.
Change was suggested by James O'Beirne <james.obeirne@gmail.com> in
https://github.com/bitcoin/bitcoin/pull/10973#discussion_r213473620
There are no changes in behavior except on some platforms we don't support
(ILP64, IP16L32, I16LP32), where SignalsOptInRBF() and MutateTxAddInput()
functions would now work correctly.
'Must be one of' should always end in a ':'
'hex encoded' is now always 'hex-encoded'
Remove redundant '(defaults to CONSERVATIVE)' text from estimatesmartfee
Consistent spacing for options '( verbose )' and '( verbosity )'
'BIP125 replaceable' is now always 'BIP125-replaceable'
JSON-RPC example is now always 'As a JSON-RPC call'
5eb20f81d9 Consistently use ParseHashV to validate hash inputs in rpc (Ben Woosley)
Pull request description:
ParseHashV validates the length and encoding of the string and throws
an informative RPC error on failure, which is as good or better than
these alternative calls.
Note I switched ParseHashV to check string length first, because
IsHex tests that the length is even, and an error like:
"must be of length 64 (not 63, for X)" is much more informative than
"must be hexadecimal string (not X)" in that case.
Split from #13420
Tree-SHA512: f0786b41c0d7793ff76e4b2bb35547873070bbf7561d510029e8edb93f59176277efcd4d183b3185532ea69fc0bbbf3dbe9e19362e8017007ae9d51266cd78ae
d9d79576f4 Preserve a format of RPC command definitions (Kostiantyn Stepaniuk)
Pull request description:
Currently, RPC commands are formatted in a way that it's easy to read
and that `test/lint/check-rpc-mappings.py` can parse it.
To void breaking `test/lint/check-rpc-mappings.py` script by running
`clang-format`, RPC command definitions should be disabled for clang-format.
Tree-SHA512: e17d20ec0e6c4e19410198b55687ebbe6fa01654d214d4578cd16c00b872bf8b0b306594a45523685cd2e9d9280702e00471d9366e87954428e8bbeacd8cad60
f7e9e70468 [rpc] Remove deprecated sigrawtransaction rpc method. (John Newbery)
90c834089a [RPC] Remove warning about wallet addresses in createmultisig() (John Newbery)
df905e390e [rpc] Remove deprecated validateaddress usage. (John Newbery)
Pull request description:
The following rpc features were deprecated in V0.17:
- `validateaddress` returning wallet information about an address
- `signrawtransaction`
This PR fully removes those features. It can be merged once V0.17 has been branched from master.
Tree-SHA512: 28293d218cf7e348632081e362f8775f243d091f49aed54c354f017d4a12ae92b87b99f81ee592a1bbf4aebd5d8cd5119278141edde7a0399ff82917ed68b9f6
917353c8b0 Make SignPSBTInput operate on a private SignatureData object (Pieter Wuille)
cad5dd2368 Pass HD path data through SignatureData (Pieter Wuille)
03a99586a3 Implement key origin lookup in CWallet (Pieter Wuille)
3b01efa0d1 [MOVEONLY] Move ParseHDKeypath to utilstrencodings (Pieter Wuille)
81e1dd5ce1 Generalize PublicOnlySigningProvider into HidingSigningProvider (Pieter Wuille)
84f1f1bfdf Make SigningProvider expose key origin information (Pieter Wuille)
611ab307fb Introduce KeyOriginInfo for fingerprint + path (Pieter Wuille)
Pull request description:
This PR adds "key origin" (master fingeprint + key path) information to what is exposed from `SigningProvider`s, allowing this information to be used by the generic PSBT code instead of having the RPC pull it directly from the wallet.
This is also a preparation to having PSBT interact with output descriptors, which can then directly expose key origin information for the scripts they generate.
Tree-SHA512: c718382ba8ba2d6fc9a32c062bd4cff08b6f39b133838aa03115c39aeca0f654c7cc3ec72d87005bf8306e550824cd8eb9d60f0bd41784a3e22e17b2afcfe833
984d72ec65 Return the script type from Solver (Ben Woosley)
Pull request description:
Because false is synonymous with TX_NONSTANDARD, this conveys the same
information and makes the handling explicitly based on script type,
simplifying each call site.
Prior to this change it was common for the return value to be ignored, or for the
return value and TX_NONSTANDARD to be redundantly handled.
Tree-SHA512: 31864f856b8cb75f4b782d12678070e8b1cfe9665c6f57cfb25e7ac8bcea8a22f9a78d7c8cf0101c841f2a612400666fb91798bffe88de856e98b873703b0965
Currently RPC commands are formatted in a way that it's easy to read
and that test/lint/check-rpc-mappings.py can parse it.
To void breaking test/lint/check-rpc-mappings.py script by running
clang-format, RPC command definitions should be disabled for clang-format.
