Age | Commit message (Collapse) | Author |
|
43de4d3630274e1287179c86896ed4c2d8b9eff4 doc: fix typos (Sjors Provoost)
Pull request description:
This PR fixes typos found by lint-spelling.py using codespell 2.2.6.
Our CI linter job uses codespell 2.2.5 and found fewer typos that I did locally. In any case it's happy now.
ACKs for top commit:
pablomartin4btc:
re ACK 43de4d3630274e1287179c86896ed4c2d8b9eff4
Tree-SHA512: c032fe86cb49c924a468385653b31f309a9db68c478d70335bba3e65a1ff3826abe80284fe00a090ab5a509e1edbf17e476f6922fb15d055e50f1103dad2ccb0
|
|
rpc/util.{h,cpp} to rpc/net.cpp
bbb68ffdbdafb6717dcadac074f6098750b8aa77 refactor: drop protocol.h include header in rpc/util.h (Jon Atack)
1dd62c5295ca319c94f7233bcb2e11f9d37a33f1 refactor: move GetServicesNames from rpc/util.{h,cpp} to rpc/net.cpp (Jon Atack)
Pull request description:
Move `GetServicesNames()` from `rpc/util` to `rpc/net.cpp`, as it is only called from that compilation unit and there is no reason for other ones to need it.
Remove the `protocol.h` include in `rpc/util.h`, as it was only needed for `GetServicesNames()`, drop an unneeded forward declaration (the other IWYU suggestions would require more extensive changes in other files), and add 3 already-missing include headers in other translation units that are needed to compile without `protocol.h` in `rpc/util.h`, as `protocol.h` includes `netaddress.h`, which in turn includes `util/strencodings.h`.
ACKs for top commit:
kevkevinpal:
lgtm ACK [bbb68ff](https://github.com/bitcoin/bitcoin/pull/28136/commits/bbb68ffdbdafb6717dcadac074f6098750b8aa77)
ns-xvrn:
ACK bbb68ff
achow101:
ACK bbb68ffdbdafb6717dcadac074f6098750b8aa77
Tree-SHA512: fcbe195874dd4aa9e86548685b6b28595a2c46f9869b79b6e2b3835f76b49cab4bef6a59c8ad6428063a41b7bb6f687229b06ea614fbd103e0531104af7de55d
|
|
As found by lint-spelling.py using codespell 2.2.6.
|
|
literals to Parse{Hash,Hex}
bb91131d545d986aab81c4bb13676c4520169259 doc: remove out-of-date external link in src/util/strencodings.h (Jon Atack)
7d494a48ddf4248ef3b1753b6e7f2eeab3a8ecb7 refactor: use string_view to pass string literals to Parse{Hash,Hex} (Jon Atack)
Pull request description:
as `string_view` is optimized to be trivially copiable, whereas the current code creates a `std::string` copy at each call.
These utility methods are called by quite a few RPCs and tests, as well as by each other.
```
$ git grep "ParseHashV\|ParseHashO\|ParseHexV\|ParseHexO" | wc -l
61
```
Also remove an out-of-date external link.
ACKs for top commit:
jonatack:
Rebased per `git range-diff c9273f6 b94581a bb91131` for an include header from the merge of https://github.com/bitcoin/bitcoin/pull/28230. Should be trivial to re-ACK.
maflcko:
lgtm ACK bb91131d545d986aab81c4bb13676c4520169259
ns-xvrn:
ACK https://github.com/bitcoin/bitcoin/commit/bb91131d545d986aab81c4bb13676c4520169259
achow101:
ACK bb91131d545d986aab81c4bb13676c4520169259
brunoerg:
crACK bb91131d545d986aab81c4bb13676c4520169259
Tree-SHA512: 9734fe022c9e43fd93c23a917770d332dbbd3132c80a234059714c32faa6469391e59349954749fc86c4ef0b18d5fd99bf8f4b7b82d9f799943799c1253272ae
|
|
Co-authored-by: Pieter Wuille <bitcoin-dev@wuille.net>
|
|
as it is only called from that compilation unit.
This avoids needlessly compiling GetServicesNames() in the 35 other files that
include rpc/util.h.
|
|
P2PK scripts are not PKHash destinations, they should have their own
type.
This also results in no longer showing a p2pkh address for p2pk outputs.
However for backwards compatibility, ListCoinst will still do this
conversion.
|
|
Make sure that nothing else can change WitnessUnknown's data members by
making them private. Also change the program to use a vector rather than
C-style array.
|
|
delimitation
6e8f6468cbf1320b70cf01333002a31b44cb7c33 removed StrFormatInternalBug quote delimitation (Reese Russell)
Pull request description:
This PR rectifies an unnecessary set of quotes delimiting the contents of ```StrFormatInternalBug```. This is a follow up to MarcoFalke https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1297191493. The method of action was to remove the escaped quotes that were a part of strprintf. A single functional test case was modified to reflect the new output format.
```STR_INTERNAL_BUG``` was applied to https://github.com/bitcoin/bitcoin/pull/28123 in ```std::string RPCArg::ToString(const bool oneline)``` in ```rpc/util.cpp```
The results can be seen below.
Previously
![image](https://github.com/bitcoin/bitcoin/assets/3104223/53f9ea59-317f-4c62-9fc1-04255eeb4641)
This PR
![image](https://github.com/bitcoin/bitcoin/assets/3104223/5c6a3110-f1f3-4b3c-8e8a-9c8f1c3176e7)
Additional context can be found here.
https://github.com/bitcoin/bitcoin/pull/28123#discussion_r1271871716
Thank you.
ACKs for top commit:
MarcoFalke:
review ACK 6e8f6468cbf1320b70cf01333002a31b44cb7c33
stickies-v:
ACK 6e8f6468cbf1320b70cf01333002a31b44cb7c33
Tree-SHA512: 35317e31a527630495b566407e37db9941dab7f81cfaeb1ea3309683c48e4273284645ad615f73e646a137b4f2ae35933603e9182a7dbdd22cac98d038c491dc
|
|
as string_view is optimized to be trivially copiable, and in these use cases we
only perform read operations on the passed object.
These utility methods are called by quite a few RPCs and tests, as well as by each other.
$ git grep "ParseHashV\|ParseHashO\|ParseHexV\|ParseHexO" | wc -l
61
|
|
|
|
|
|
oneline descriptions
5e3e83b005518659a69916c373b808da27e51791 RPC/Mining: Document template_request better for getblocktemplate (Luke Dashjr)
de319c61759952318364fbcb28c47f0959d89d0e RPC/rpcdoccheck: Error if a oneline_description has a quote for a non-string (Luke Dashjr)
7c61e9df90579ed42a30016e52355e437733b128 Bugfix: RPC: Remove quotes from non-string oneline descriptions (Luke Dashjr)
Pull request description:
Various JSON Object parameters had a `oneline_description` with quote characters. Fix those, and extend `rpcdoccheck` to detect them.
Also, slightly improve GBT's oneline description for template_request.
ACKs for top commit:
MarcoFalke:
review ACK 5e3e83b005518659a69916c373b808da27e51791
Tree-SHA512: 363d1669a661d0acfc19fddb57e777d781c7246f330cf62160e77dde10a6adcb0249db748127067da1afe1b7d17c71cf611d9fdc3664d6bf5b3f30105637769a
|
|
Since script/standard only contains things that are used by the Solver
and its callers, rename the files to script/solver.
|
|
Remove standard.h from files that don't use anything in it, and include
it in files that do.
|
|
This check is already done by the rpc parser. Re-doing it is adding dead
code. Instead, throwing an exception when the assumption does not hold
is the already correct behavior.
To make the fuzz test more accurate and not swallow all runtime errors,
add a check that the passed in UniValue sighash argument is either a
string or null.
Co-authored-by: stickies-v <stickies-v@protonmail.com>
|
|
This split is done in preparation for the next commit where the
dependency on UniValue in the kernel library is removed.
|
|
|
|
2cd28e9fef5dd743bcd70025196ee311fcfdcae4 rpc: Add check for unintended option/parameter name clashes (Ryan Ofsky)
95d7de0964620a3f7386a4adc5707559868abf84 test: Update python tests to use named parameters instead of options objects (Ryan Ofsky)
96233146dd31c1d99fd1619be4449944623ef750 RPC: Allow RPC methods accepting options to take named parameters (Ryan Ofsky)
702b56d2a8ce48bc3b66a2867d09fa11dcf12fc5 RPC: Add add OBJ_NAMED_PARAMS type (Ryan Ofsky)
Pull request description:
Allow RPC methods which take an `options` parameter (`importmulti`, `listunspent`, `fundrawtransaction`, `bumpfee`, `send`, `sendall`, `walletcreatefundedpsbt`, `simulaterawtransaction`), to accept the options as named parameters, without the need for nested JSON objects.
This makes it possible to make calls like:
```sh
src/bitcoin-cli -named bumpfee txid fee_rate=10
```
instead of
```sh
src/bitcoin-cli -named bumpfee txid options='{"fee_rate": 10}'
```
RPC help is also updated to show options as top level named arguments instead of as nested objects.
<details><summary>diff</summary>
<p>
```diff
@@ -15,16 +15,17 @@
Arguments:
1. txid (string, required) The txid to be bumped
-2. options (json object, optional)
+2. options (json object, optional) Options object that can be used to pass named arguments, listed below.
+
+Named Arguments:
- {
- "conf_target": n, (numeric, optional, default=wallet -txconfirmtarget) Confirmation target in blocks
+conf_target (numeric, optional, default=wallet -txconfirmtarget) Confirmation target in blocks
- "fee_rate": amount, (numeric or string, optional, default=not set, fall back to wallet fee estimation)
+fee_rate (numeric or string, optional, default=not set, fall back to wallet fee estimation)
Specify a fee rate in sat/vB instead of relying on the built-in fee estimator.
Must be at least 1.000 sat/vB higher than the current transaction fee rate.
WARNING: before version 0.21, fee_rate was in BTC/kvB. As of 0.21, fee_rate is in sat/vB.
- "replaceable": bool, (boolean, optional, default=true) Whether the new transaction should still be
+replaceable (boolean, optional, default=true) Whether the new transaction should still be
marked bip-125 replaceable. If true, the sequence numbers in the transaction will
be left unchanged from the original. If false, any input sequence numbers in the
original transaction that were less than 0xfffffffe will be increased to 0xfffffffe
@@ -32,11 +33,10 @@
still be replaceable in practice, for example if it has unconfirmed ancestors which
are replaceable).
- "estimate_mode": "str", (string, optional, default="unset") The fee estimate mode, must be one of (case insensitive):
+estimate_mode (string, optional, default="unset") The fee estimate mode, must be one of (case insensitive):
"unset"
"economical"
"conservative"
- }
Result:
{ (json object)
```
</p>
</details>
**Review suggestion:** To understand this PR, it is probably easiest to review the commits in reverse order because the last commit shows the external API changes, the middle commit shows the internal API changes, and the first commit contains the low-level implementation.
ACKs for top commit:
achow101:
ACK 2cd28e9fef5dd743bcd70025196ee311fcfdcae4
Tree-SHA512: 50f6e78fa622826dab3f810400d8c1a03a98a090b1f2fea79729c58ad8cff955554bd44c2a5975f62a526b900dda68981862fd7d7d05c17f94f5b5d847317436
|
|
1bce12acd3e271a7c88d9400b4e3a5645bc8a911 test: add test for `descriptorprocesspsbt` RPC (ishaanam)
fb2a3a70e860aa87fb7a21f6554ed9f3ce901e2d rpc: add descriptorprocesspsbt rpc (ishaanam)
Pull request description:
This PR implements an RPC called `descriptorprocesspsbt`. This RPC is based off of `walletprocesspsbt`, but instead of interacting with the wallet to update, sign and finalize a psbt, it instead accepts an array of output descriptors and uses that information along with information from the mempool, txindex, and the utxo set to do so. `utxoupdatepsbt` also updates a psbt in this manner, but doesn't sign or finalize it. Because of this overlap, a helper function that is added in this PR is called by both `utxoupdatepsbt` and `descriptorprocesspsbt`. Whether or not the helper function signs a psbt is dictated by if the HidingSigningProvider passed to it contains any private information. There is also a test added in this PR for this new RPC that uses p2wsh, p2wpkh, and legacy outputs.
Edit: see https://github.com/bitcoin/bitcoin/pull/25796#issuecomment-1228830963
ACKs for top commit:
achow101:
re-ACK 1bce12acd3e271a7c88d9400b4e3a5645bc8a911
instagibbs:
reACK https://github.com/bitcoin/bitcoin/pull/25796/commits/1bce12acd3e271a7c88d9400b4e3a5645bc8a911
Tree-SHA512: e1d0334739943e71f2ee68b4db7637ebe725da62e7aa4be071f71c7196d2a5970a31ece96d91e372d34454cde8509e95ab0eebd2c8edb94f7d5a781a84f8fc5d
|
|
|
|
-BEGIN VERIFY SCRIPT-
sed --regexp-extended -i 's/find_value\(([^ ,]+), /\1.find_value(/g' $(git grep -l find_value)
-END VERIFY SCRIPT-
|
|
This RPC can be the Updater, Signer, and optionally the Input
Finalizer for a psbt, and has no interaction with the Bitcoin
Core wallet.
|
|
Also add flag to allow RPC methods that intendionally accept options and
parameters with the same name bypass the check.
Check and flag were suggested by ajtowns
https://github.com/bitcoin/bitcoin/pull/26485#issuecomment-1507916357
Co-authored-by: Anthony Towns <aj@erisian.com.au>
|
|
OBJ_NAMED_PARAMS type works the same as OBJ type except it registers the object
keys to be accepted as top-level named-only RPC parameters. Generated
documentation also lists the object keys seperately in a new "Named arguments"
section of help text.
Named-only RPC parameters have the same semantics as python keyword-only
arguments (https://peps.python.org/pep-3102/). They are always required to be
passed by name, so they don't affect interpretation of positional arguments,
and aren't affected when positional arguments are added or removed.
The new OBJ_NAMED_PARAMS type is used in the next commit to make it easier to
pass options values to various RPC methods.
Co-authored-by: Andrew Chow <github@achow101.com>
|
|
be55f545d53d44fdcf2d8ae802e9eae551d120c6 move-only: Extract common/args and common/config.cpp from util/system (TheCharlatan)
Pull request description:
This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". It is part of a series of patches splitting up the `util/system` files. Its preceding pull request is https://github.com/bitcoin/bitcoin/pull/27254.
The pull request contains an extraction of ArgsManager related functions from util/system into their own common/ file.
The background of this commit is an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager. The ArgsManager belongs into the common library, since the kernel library should not depend on it. See [doc/design/libraries.md](https://github.com/bitcoin/bitcoin/blob/master/doc/design/libraries.md) for more information on this rationale.
ACKs for top commit:
MarcoFalke:
re-ACK be55f545d53d44fdcf2d8ae802e9eae551d120c6 🚲
ryanofsky:
Code review ACK be55f545d53d44fdcf2d8ae802e9eae551d120c6. Just small cleanups since the last review.
hebasto:
ACK be55f545d53d44fdcf2d8ae802e9eae551d120c6, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 90eb03334af0155b823030b4f2ecf286d35058d700ee2ddbbaa445be19e31eb0fe982656f35bd14ecee3ad2c3d0db3746855cb8f3777eff7253713e42873e111
|
|
This is an extraction of ArgsManager related functions from util/system
into their own common file.
Config file related functions are moved to common/config.cpp.
The background of this commit is an ongoing effort to decouple the
libbitcoinkernel library from the ArgsManager. The ArgsManager belongs
into the common library, since the kernel library should not depend on
it. See doc/design/libraries.md for more information on this rationale.
|
|
This new "warnings" field is a JSON array of strings intended to replace the
"warning" string field in these four RPCs, to better handle returning multiple
warning messages and for consistency with other wallet RPCs.
|
|
|
|
https://clang.llvm.org/extra/clang-tidy/checks/performance/inefficient-vector-operation.html
|
|
https://clang.llvm.org/extra/clang-tidy/checks/performance/faster-string-find.html
|
|
error (1.5/2)
fafeddfe0e6748e9769ad3dd526a6c0eaf6f4aac rpc: Throw more user friendly arg type check error (MarcoFalke)
Pull request description:
The arg type check error doesn't list which arg (position or name) failed. Fix that.
ACKs for top commit:
stickies-v:
ACK fafeddfe0e6748e9769ad3dd526a6c0eaf6f4aac - although I think the functional test isn't in a logical place (but not blocking)
Tree-SHA512: 17425aa145aab5045940ec74fff28f0e3b2b17ae55f91c4bb5cbcdff0ef13732f8e31621d85964dc2c04333ea37dbe528296ac61be27541384b44e37957555c8
|
|
-BEGIN VERIFY SCRIPT-
sed -i -e "/Deprecated alias for OMITTED, can be removed/d" src/rpc/util.h src/rpc/util.cpp
sed -i -e "s/OMITTED_NAMED_ARG/OMITTED/g" $(git grep -l "OMITTED_NAMED_ARG" src/)
-END VERIFY SCRIPT-
|
|
|
|
errors
3d1a4d8a45cd91bdfe0ef107c2e9c5e882b34155 RPC: make RPCResult::MatchesType return useful errors (Anthony Towns)
Pull request description:
Currently if you don't correctly update the description of the return value for an RPC call, you essentially just get an assertion failure with no useful information; this generates a description of the problems instead.
ACKs for top commit:
MarcoFalke:
re-ACK 3d1a4d8a45cd91bdfe0ef107c2e9c5e882b34155 🌷
Tree-SHA512: cf0580b7046faab0128672a74f8cc5a1655dfdca6646a2e38b51f0fb5f672c98aad6cd4c5769454a2d644a67da639ccb1c8ff5d24d3d6b4446a082398a643722
|
|
|
|
|
|
|
|
|
|
https://clang.llvm.org/extra/clang-tidy/checks/performance/no-automatic-move.html
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
Commits of previous years:
- 2021: f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7
- 2020: fa0074e2d82928016a43ca408717154a1c70a4db
- 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
|
|
|
|
|
|
We were throwing two different errors for the same problematic:
* "Expected type {expected], got {type}" --> RPCTypeCheckArgument()
* "JSON value of type {type} is not of expected type {expected}" --> UniValue::checkType()
|
|
This avoids initializing variables with the copy-constructor of a
non-trivially copyable type.
|
|
|
|
fa9af218780b7960d756db80c57222e5bf2137b1 scripted-diff: Use getInt<T> over get_int/get_int64 (MacroFake)
Pull request description:
Seems better to see the return type directly and be able to modify it easier, as the return type is used for exceptions (in-range checking and parsing feedback).
ACKs for top commit:
fanquake:
ACK fa9af218780b7960d756db80c57222e5bf2137b1
Tree-SHA512: 284aa2527d0f663ca01550115025c9c64c787531d595f866c718f6ad09b9b0cac1e683a7d77f8009b75de990fd37166b44063ffa83fba8a04e9a31600b4c2725
|
|
a runtime flag
b953ea6cc691ba61bf08eb186e76e7e8b7ba8120 rpc: Put undocumented JSON failure mode behind a runtime flag (Suhail Saqan)
Pull request description:
Fixes #24695 (Put undocumented JSON failure mode behind a runtime flag)
ACKs for top commit:
luke-jr:
utACK b953ea6cc691ba61bf08eb186e76e7e8b7ba8120
vincenzopalazzo:
ACK https://github.com/bitcoin/bitcoin/pull/25161/commits/b953ea6cc691ba61bf08eb186e76e7e8b7ba8120
Tree-SHA512: 2005ee1b1f3b637918390b2ecd4166f2fd8c86e3c59fba3da8a0cbd5b1dffd03190c92f6dca3c489ecce4276eaf3108b2edcf9cd6224b713adb52f5bb848163b
|
|
rpc: Put undocumented JSON failure mode behind a runtime flag
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's|\<get_int64\>|getInt<int64_t>|g' $(git grep -l get_int ':(exclude)src/univalue')
sed -i 's|\<get_int\>|getInt<int>|g' $(git grep -l get_int ':(exclude)src/univalue')
-END VERIFY SCRIPT-
|