afc06fc868 rpc: Fix help text for signtransactionwithXXX (Torkel Rogstad)
Pull request description:
This PR fixes the help text for the `signrawtransactionwithwallet` and `signrawtransactionwithkey` RPC calls. They both marked the `amount` field in the UTXO dependencies as required. This field is omitted in the [`rpc_rawtransaction.py` test](8a8b03ecd2/test/functional/rpc_rawtransaction.py (L155)) and [`successful_signing_test`](8a8b03ecd2/test/functional/rpc_signrawtransaction.py (L42)) in `rpc_signrawtransaction.py`.
ACKs for commit afc06f:
promag:
utACK afc06fc.
MarcoFalke:
utACK afc06fc868
Tree-SHA512: 7847844ca38d8033fef2f5255700d21487d78a63ecee8b80362fa28fadfafc80ba29a89f38d7ebb3a1be4c7d47ff6c338f67afec9ee22bf065fb352bb3d03d3a
fa926ec24f rpc: Mention all output types in decodescript doc (MarcoFalke)
fa3caa1666 rpc: decodescript use IsValidNumArgs over hardcoded check (MarcoFalke)
faad33ff15 rpc: Clarify decodescript RPCResult doc (MarcoFalke)
Pull request description:
* Remove `"hex"` from the decodescript RPCResult doc
* Add `"segwit`" to the doc
Follow up to a6099ef319 and 4f933b3d23
ACKs for commit fa926e:
ryanofsky:
utACK fa926ec24f. Only change since last review is listing possible output types in the help string using a new `GetAllOutputTypes` function
Tree-SHA512: e6ecc563d04769942567118d50188467bf64ceb276ba6268928d469e8f06621f2ca1ae1e555d3daa6ec22a615ee259bb31c4141c19818d0f53fb6c529b18381b
fa1ad200d3 doc: Add release notes for 15620 (MarcoFalke)
fa96d76421 rpc: Uncouple rpcs from maxTxFee global (MarcoFalke)
fa965e03c7 rpc: Use IsValidNumArgs over hardcoded size checks (MarcoFalke)
Pull request description:
This makes the rpcs a bit more stateless by falling back to their own default max fee instead of the global maxTxFee.
A follow up pull request will move `-maxtxfee` to the wallet.
See also related discussions:
* `-maxtxfee` should not be used by both node and wallet #15355
* [RFC] Long term plan for wallet command-line args #13044
ACKs for commit fa1ad2:
jnewbery:
utACK fa1ad200d3
Empact:
utACK fa1ad200d3
jnewbery:
utACK fa1ad200d3
promag:
utACK fa1ad20.
Tree-SHA512: c9cf0b54cd30ff3ab0d090b072cc38fcbb2840bc6ad9a9711995333bc927d2500aece6b5a60e061666eca5ed72b70aa318d21e51eb15ee0106b41f5b6e4e1adf
signrawtransaction was deprecated in 0.17 and removed in 0.18. A warning
was left in place to tell users to migrate to using
signrawtransactionswithwallet or signrawtransactionwithkey. Remove the
warning now that it's been two releases since the method was removed.
d358466de Remove remaining wallet accesses to node globals (Russell Yanofsky)
b1b2b2389 Remove use of CCoinsViewMemPool::GetCoin in wallet code (Russell Yanofsky)
4e4d9e9f8 Remove use of CRPCTable::appendCommand in wallet code (Russell Yanofsky)
91868e628 Remove use CValidationInterface in wallet code (Russell Yanofsky)
Pull request description:
This PR is the last in a chain of PRs (#14437, #14711, and #15288) that make the wallet code access node state through an abstract [`Chain`](https://github.com/ryanofsky/bitcoin/blob/pr/wipc-sep/src/interfaces/chain.h) class in [`src/interfaces/`](https://github.com/ryanofsky/bitcoin/tree/pr/wipc-sep/src/interfaces) instead of using global variables like `cs_main`, `chainActive`, and `g_connman`. After this PR, wallet code no longer accesses global variables declared outside the wallet directory, and no longer calls functions accessing those globals (as verified by the `hide-globals` script in #10244).
This PR and the previous PRs have been refactoring changes that do not affect behavior. Previous PRs have consisted of lots of mechanical changes like:
```diff
- wtx.nTimeReceived = GetAdjustedTime();
+ wtx.nTimeReceived = m_chain->getAdjustedTime();
```
This PR is smaller, but less mechanical. It replaces last few bits of wallet code that access node state directly (through `CValidationInterface`, `CRPCTable`, and `CCoinsViewMemPool` interfaces) with code that uses the `Chain` interface.
These changes allow followup PR #10102 (multiprocess gui & wallet PR) to work without any significant updates to wallet code. Additionally they:
* Provide a single place to describe the interface between wallet and node code.
* Can make better wallet testing possible, because the `Chain` object consists of virtual methods that can be overloaded for mocking. (This could be used to test edge cases in the rescan code, for example).
Tree-SHA512: e6291d8a3c50bdff18a9c8ad11e729beb30b5b7040d7aaf31ba678800b4a97b2dd2be76340b1e5c01fe2827d67d37ed1bb4c8380cf8ed653aadfea003e9b22e7
7abd2e697c wallet/rpc: add maxfeerate parameter to testmempoolaccept (Karl-Johan Alm)
6c0a6f73e3 wallet/rpc: add maxfeerate parameter to sendrawtransaction (Karl-Johan Alm)
e5efacb941 test: Refactor vout fetches in rpc_rawtransaction (Karl-Johan Alm)
Pull request description:
This adds a new `maxfeerate` parameter to `sendrawtransaction` which forces the node to reject a transaction whose feerate is above the given fee rate.
This is a safety harness from shooting yourself in the foot and accidentally overpaying fees.
See also #12911.
Tree-SHA512: efa50134a7c17c9330cfdfd48ba400e095c0a419cc45e630618d8b44929c25d780d1bb2710c1fbbb6e687eca373505b0338cdaa7f2ff4ca22636d84c31557a2e
335931df4a rpc: return a number for estimated_feerate in analyzepsbt (fanquake)
a4d0fd026b doc: correct analysepsbt rpc doc (fanquake)
Pull request description:
Even though `missing` is optional, all its field are also all optional.
`estimated_vsize` is optional (calculate alongside `estimated_feerate`).
Make `estimated_feerate` output numeric.
Tree-SHA512: 5e063bcfbca73d3d08d29c5d1f59bc1791757f69248da4a9c70fe4b2fe82807ec4ed9fca48d439101d66ba924916fa5da9232167d74ea2b967fc04f0a36f190e
fa9b60c842 Remove unused TransactionError constants (MarcoFalke)
Pull request description:
Fixup to #14978, which introduced a bunch of unused enum values, such as `UNKNOWN_ERROR`, `ERROR_COUNT` and `TRANSACTION_ERR_LAST`. None of those have a meaning in the context of an `enum class`, where the compiler can infer if all cases have been covered in a switch-case.
Also, move the global `::maxTxFee` back to the rpc caller, so it can be set on a per call basis (in the future).
Tree-SHA512: 7f1e2d795f1c1278ecd54ddab2b92c2a862f3c637b482d1d008208925befa1c9dd4b3c4bb1bfcbc5ca4b66a41004aaf01ea96ea95236f944250b8a6cf99ff173
540729ef4b Implement analyzepsbt RPC and tests (Andrew Chow)
77542cf2a5 Move PSBT UTXO fetching to a separate method (Andrew Chow)
cb40b3abd4 Figure out what is missing during signing (Andrew Chow)
08f749c914 Implement joinpsbts RPC and tests (Andrew Chow)
7344a7b998 Implement utxoupdatepsbt RPC and tests (Andrew Chow)
Pull request description:
This PR adds 3 new utility RPCs for interacting with PSBTs.
`utxoupdatepsbt` updates a PSBT with UTXO information from the node. It only works with witness UTXOs because full transactions (as would be needed for non-witness UTXOs) are not available unless txindex is enabled.
`joinpsbts` joins the inputs from multiple distinct PSBTs into one PSBT. e.g. if PSBT 1 has inputs 1 and 2, and PSBT 2 has inputs 3 and 4, `joinpsbts` would create a new PSBT with inputs 1, 2, 3, and 4.
`analyzepsbt` analyzes a PSBT and determines the current state of it and all of its inputs, and the next step that needs to be done.
Tree-SHA512: 3c1fa302201abca76a8901d0c2be7b4ccbce334d989533c215f8b3e50e22f2f018ce6209544b26789f58f5980a253c0655111e1e20d47d5656e0414c64891a5c
cb3511b9d Add release notes for importing key origin info change (Andrew Chow)
4c75a69f3 Test importing descriptors with key origin information (Andrew Chow)
02d6586d7 Import KeyOriginData when importing descriptors (Andrew Chow)
3d235dff5 Implement a function to add KeyOriginInfo to a wallet (Andrew Chow)
eab63bc26 Store key origin info in key metadata (Andrew Chow)
345bff601 Remove hdmasterkeyid (Andrew Chow)
bac8c676a Add a method to CWallet to write just CKeyMetadata (Andrew Chow)
e7652d3f6 Add WriteHDKeypath function and move *HDKeypath to util/bip32.{h,cpp} (Andrew Chow)
c45415f73 Refactor keymetadata writing to a separate method (Andrew Chow)
Pull request description:
This PR allows for key origin data as defined by the descriptors document to be imported to the wallet when importing a descriptor using `importmulti`. This allows the `walletprocesspsbt` to include the BIP 32 derivation paths for keys that it is watching that are from a different HD wallet.
In order to make this easier to use, a new field `hdmasterkeyfingerprint` has been added to `getaddressinfo`. Additionally I have removed `hdmasterkeyid` as was planned. I think that this API change is fine since it was going to be removed in 0.18 anyways. `CKeyMetadata` has also been extended to store key origin info to facilitate this.
Tree-SHA512: 9c7794f3c793da57e23c5abbdc3d58779ee9dea3d53168bb86c0643a4ad5a11a446264961e2f772f35eea645048cb60954ed58050002caee4e43cd9f51215097
6ca836ab3a Add release note for listunspent P2WSH change (MeshCollider)
928beae007 Add test for P2SH-P2WSH in signrawtransactionwithkey and listunspent (MeshCollider)
314784a60f Make listunspent and signrawtransaction RPCs support witnessScript (MeshCollider)
Pull request description:
This is a reworked version of #11708 after #12427 and the `signrawtransaction` split.
For a P2WSH address, listunspent should return the witness script, and for a P2SH-P2WSH address, it should also return the inner witness script (because SignTransaction will automatically wrap it in P2SH if required).
Includes a test which also tests the behaviour of #12427, and release note.
Tree-SHA512: a8e72cf16930312bf48ec47e44a68f8d7e26664043c1b4cc0983eb25aec4087e511188ff9a0f181cd7b8a0c068c60d7f1e7e3f226b79e8c48890039dcf57f7b7
Creates new files util/bip32.h and util/bip32.cpp for containing
BIP 32 stuff.
Moves FormatKeyPath from descriptor.cpp to util/bip32.
Adds a wrapper around it to prepent the 'm' for when just the
BIP 32 style keypath is needed.
102faad81 Factor out combine / finalize / extract PSBT helpers (Glenn Willen)
78b9893d0 Remove op== on PSBTs; check compatibility in Merge (Glenn Willen)
bd0dbe876 Switch away from exceptions in refactored tx code (Glenn Willen)
c6c3d42a7 Move PSBT definitions and code to separate files (Glenn Willen)
81cd95884 Factor BroadcastTransaction out of sendrawtransaction (Glenn Willen)
c734aaa15 Split DecodePSBT into Base64 and Raw versions (Glenn Willen)
162ffefd2 Add pf_invalid arg to std::string DecodeBase{32,64} (Glenn Willen)
Pull request description:
* Move most PSBT definitions into psbt.h.
* Move most PSBT RPC utilities into psbt.{h,cpp}.
* Move wallet-touching PSBT RPC utilities (FillPSBT) into
wallet/psbtwallet.{h,cpp}.
* Switch exceptions from JSONRPCError() to new PSBTException class.
* Split DecodePSBT into DecodeBase64PSBT (old behavior) and DecodeRawPSBT.
* Add one new version of DecodeBase64 utility in strencodings.h (and
corresponding DecodeBase32 for completeness).
* Factor BroadcastTransaction utility function out of sendrawtransaction RPC
handler in rpc/rawtransaction.cpp
Note: For those keeping score at home wondering why refactor, this is in anticipation of (and developed in parallel with) a change to actually introduce GUI use of all this stuff, which is already under development and working-ish.
Tree-SHA512: 2197c448e657421f430943025357597e7b06c4c377d5d4b2622b9edea52a7193c48843dd731abb3a88ac4023a9c88d211991e0a9b740c22f2e1cbe72adefe390
Refactor the new CombinePSBT, FinalizePSBT, and FinalizeAndExtractPSBT
general-purpose functions out of the combinepsbt and finalizepsbt RPCs,
for use in the GUI code.
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.
