aboutsummaryrefslogtreecommitdiff
path: root/src/rpc/server.cpp
AgeCommit message (Collapse)Author
2022-12-13Merge bitcoin/bitcoin#26628: RPC: Reject RPC requests with same named ↵MarcoFalke
parameter specified multiple times 8c3ff7d52ae3314959e1e66da8718a3f0d30abaa test: Suggested cleanups for rpc_namedparams test (Ryan Ofsky) d1ca56382512df3084fce7353bf1e8b66cae61bc bitcoin-cli: Make it an error to specify the "args" parameter two different ways (Ryan Ofsky) 6bd1d20b8cf27aa72ec2907342787e6fc9f94c50 rpc: Make it an error server-side to specify same named parameter multiple times (Ryan Ofsky) e2c3b18e671e347e422d696d1cbdd9f82b2ce468 test: Add RPC tests for same named parameter specified more than once (Ryan Ofsky) Pull request description: Make the JSON-RPC server reject requests with the same named parameter specified multiple times, instead of silently overwriting earlier parameter values with later ones. Generally JSON keys are supposed to unique, and their order isn't supposed to be significant, so having the server silently discard duplicate keys is error-prone. Most likely if an RPC client is sending a request with duplicate keys it means something is wrong with the request and there should be an error. After this change, named parameters are still allowed to specified multiple times on the `bitcoin-cli` command line, since `bitcoin-cli` automatically replaces earlier values with later values before sending the JSON-RPC request. This makes sense, since it's not unusual for the order of command line options to be significant or for later command line options to override earlier ones. ACKs for top commit: MarcoFalke: review ACK 8c3ff7d52ae3314959e1e66da8718a3f0d30abaa 🗂 kristapsk: ACK 8c3ff7d52ae3314959e1e66da8718a3f0d30abaa stickies-v: ACK 8c3ff7d52 Tree-SHA512: 2d1357dcc2c171da287aeefc7b333ba4e67babfb64fc14d7fa0940256e18010a2a65054f3bf7fa1571b144d2de8b82d53076111b5f97ba29320cfe84b6ed986f
2022-12-05Fixup clang-tidy named argument commentsfanquake
Fix comments so they are checked/consistent. Fix incorrect arguments.
2022-12-02rpc: Make it an error server-side to specify same named parameter multiple timesRyan Ofsky
Specifying same named parameter multiple times is still allowed by bitcoin-cli. The client implementation overwrites earlier option values with later ones before sending to server. This is tested by interface_bitcoin_cli.py Rationale for allowing client parameters to be specified multiple times in bitcoin-cli is that this behavior has been supported for a long time, and that when using the command line interactively, it can be convenient to override earlier option values with new values without having to go back and remove the old value. But for the RPC server, there isn't really a good use-case for earlier values to be discarded if multiple values are specified. JSON keys are generally supposed to be unique and if they aren't it's probably an indication of some problem generating the RPC request.
2022-11-05rpc: Allow named and positional arguments to be used togetherRyan Ofsky
It's nice to be able to use named options and positional arguments together. Most shell tools accept both, and python functions combine options and arguments allowing them to be passed with even more flexibility. This change adds support for python's approach so as a motivating example: bitcoin-cli -named createwallet wallet_name=mywallet load_on_startup=1 Can be shortened to: bitcoin-cli -named createwallet mywallet load_on_startup=1 JSON-RPC standard doesn't have a convention for passing named and positional parameters together, so this implementation makes one up and interprets any unused "args" named parameter as a positional parameter array.
2022-09-30Merge bitcoin/bitcoin#26074: refactor: Set RPCArg options with designated ↵MacroFake
initializers fa2c72dda09f9b51332f6c7953ae81e573cc834f rpc: Set RPCArg options with designated initializers (MacroFake) Pull request description: For optional constructor arguments, use a new struct. This comes with two benefits: * Earlier unused optional arguments can be omitted * Designated initializers can be used ACKs for top commit: stickies-v: re-ACK fa2c72dda09f9b51332f6c7953ae81e573cc834f Tree-SHA512: 2a0619548187cc7437fee2466ac4780746490622f202659f53641be01bc2a1fea4416d1a77f3e963bf7c4cce62899b61fab0b9683440cf82f68be44f63826658
2022-09-21Merge bitcoin/bitcoin#25737: rpc: treat univalue type check error as ↵fanquake
RPC_TYPE_ERROR, not RPC_MISC_ERROR e68d380797918e655decb76fc11725197d6d5323 rpc: remove unneeded RPCTypeCheckArgument checks (furszy) 55566630c60d23993a52ed54c95e7891f4588d57 rpc: treat univalue type check error as RPC_TYPE_ERROR, not RPC_MISC_ERROR (furszy) Pull request description: Same rationale as #26039, tackling another angle of the problem. #### Context We have the same univalue type error checking code spread/duplicated few times: `RPCTypeCheckObj`, `RPCTypeCheckArgument`, `UniValue::checkType`. In the first two functions, we are properly returning an `RPC_TYPE_ERROR` while in `UniValue::checkType` we are throwing an `std::runtime_error` which is caught by the RPC server request handler, who invalidly treats it as `RPC_MISC_ERROR` (which is a generic error return code that provides no information to the user). #### Proposed Changes Throw a custom exception from `Univalue::checkType` (instead of a plain `std::runtime_error`) and catch it on the RPC server request handler. So we properly return `RPC_TYPE_ERROR` (-3) on every arg type error and not the general `RPC_MISC_ERROR` (-1). This will allow us to remove all the `RPCTypeCheckArgument` calls. As them are redundant since #25629. Top commit has no ACKs. Tree-SHA512: 4e4c41851fd4e2b01a2d8b94e71513f9831f810768ebd89684caca4901e87d3677980003949bcce441f9ca607a1b38a5894839b6c492f5947b8bab8cd9423ba6
2022-09-15rpc: treat univalue type check error as RPC_TYPE_ERROR, not RPC_MISC_ERRORfurszy
By throwing a custom exception from `Univalue::checkType` (instead of a plain std::runtime_error) and catching it on the RPC server request handler. So we properly return RPC_TYPE_ERROR (-3) on arg type errors and not the general RPC_MISC_ERROR (-1).
2022-09-13rpc: Set RPCArg options with designated initializersMacroFake
2022-07-30Use steady clock for all millis bench loggingMacroFake
2022-06-23rpc: Use steady_clock for getrpcinfo durationsMacroFake
2022-05-21scripted-diff: Convert global Mutexes to GlobalMutexesAnthony Towns
-BEGIN VERIFY SCRIPT- sed -i -E -e '/^([a-z]+ )?Mutex [a-z]/ s/Mutex/GlobalMutex/' $(git grep -lE '^([a-z]+ )?Mutex [a-z]') -END VERIFY SCRIPT-
2022-05-18scripted-diff: Use getInt<T> over get_int/get_int64MacroFake
-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-
2022-04-26Merge bitcoin/bitcoin#24959: Remove not needed clang-format off commentslaanwj
fa870e3d4ccd6dfd0a9a8f2c608721a7251114e2 Remove not needed clang-format off comments (MarcoFalke) Pull request description: It seems odd to disable clang-format and force manual formatting when there is no need for it. So remove the clang-format comments and other unneeded comments. Can be reviewed with `--word-diff-regex=. --ignore-all-space` Looks like this was initially added in commit d9d79576f423cd9c5cef4547c7e3648dbb339460 to accommodate a linter that has since been removed and replaced by a functional test. ACKs for top commit: laanwj: Code review ACK fa870e3d4ccd6dfd0a9a8f2c608721a7251114e2 fanquake: ACK fa870e3d4ccd6dfd0a9a8f2c608721a7251114e2 Tree-SHA512: 0f8f97c12f5dbe517dd96c10b10ce1b8772d8daed33e6b41f73ea1040e89888cf3b8c0ad7b20319e366fe30c71e8b181c89098ae7f6a3deb8647e1b4731db815
2022-04-25Remove not needed clang-format off commentsMarcoFalke
Can be reviewed with --word-diff-regex=. --ignore-all-space
2022-04-11refactor: introduce single-separator split helper `SplitString`Sebastian Falbesoner
This helper uses spanparsing::Split internally and enables to replace all calls to boost::split where only a single separator is passed. Co-authored-by: Martin Ankerl <Martin.Ankerl@gmail.com> Co-authored-by: MarcoFalke <falke.marco@gmail.com>
2022-04-04refactor: fix clang-tidy named args usagefanquake
2021-12-30scripted-diff: Bump copyright headersHennadii Stepanov
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT- Commits of previous years: * 2020: fa0074e2d82928016a43ca408717154a1c70a4db * 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
2021-10-05refactor: Block unsafe fs::path std::string conversion callsRussell Yanofsky
There is no change in behavior. This just helps prepare for the transition from boost::filesystem to std::filesystem by avoiding calls to methods which will be unsafe after the transaction to std::filesystem to due lack of a boost::filesystem::path::imbue equivalent and inability to set a predictable locale. Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Co-authored-by: Kiminuo <kiminuo@protonmail.com> Co-authored-by: MarcoFalke <falke.marco@gmail.com>
2021-09-27scripted-diff: Rename overloaded int GetArg to GetIntArgRussell Yanofsky
Improve readability of code, simplify future scripted diff cleanup PRs, and be more consistent with naming for GetBoolArg. This will also be useful for replacing runtime settings type checking with compile time checking. -BEGIN VERIFY SCRIPT- git grep -l GetArg | xargs sed -i 's/GetArg(\([^)]*\( [0-9]\+\|-1\|port\|BaseParams().RPCPort()\|Params().GetDefaultPort()\|_TIMEOUT\|Height\|_WORKQUEUE\|_THREADS\|_CONNECTIONS\|LIMIT\|SigOp\|Bytes\|_VERSION\|_AGE\|_CHECKS\|Checks() ? 1 : 0\|_BANTIME\|Cache\|BLOCKS\|LEVEL\|Weight\|Version\|BUFFER\|TARGET\|WEIGHT\|TXN\|TRANSACTIONS\|ADJUSTMENT\|i64\|Size\|nDefault\|_EXPIRY\|HEIGHT\|SIZE\|SNDHWM\|_TIME_MS\)\))/GetIntArg(\1)/g' -END VERIFY SCRIPT- Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
2021-04-17rpc: Keep default argument value in correct typeJoão Barbosa
2021-04-01rpc: Fail to return undocumented return valuesMarcoFalke
2021-03-29refactor: replace util::Ref by std::any (C++17)Sebastian Falbesoner
2021-01-29Remove pointer cast in CRPCTable::dumpArgMapRussell Yanofsky
CRPCTable::dumpArgMap currently works by casting RPC command unique_id integer field to a function pointer, and then calling the function. The unique_id field wasn't supposed to be used this way (it's meant to be used to detect RPC aliases), and this code segfaults in the rpc_help.py test in multiprocess PR https://github.com/bitcoin/bitcoin/pull/10102 because wallet RPC functions aren't directly accessible from the node process. Fix this by adding a new GET_ARGS request mode to retrieve argument information similar to the way the GET_HELP mode retrieves help information.
2021-01-29refactor: Add RPC server ExecuteCommands functionRussell Yanofsky
No change in behavior. New function is split from CRPCTable::execute and used in the next commit.
2021-01-29refactor: Replace JSONRPCRequest fHelp field with mode fieldRussell Yanofsky
No change in behavior
2021-01-28rpc: Remove duplicate name and argNames from CRPCCommandMarcoFalke
2021-01-28rpc: Use RPCHelpMan for check-rpc-mappings linterMarcoFalke
2020-08-14rpc: Remove unused return type from appendCommandMarcoFalke
2020-08-02refactor: Use C++11 range based for loops to simplify rpc codeMarcoFalke
2020-07-15Merge #19386: rpc: Assert that RPCArg names are equal to CRPCCommand ones ↵MarcoFalke
(server) fa7592bfa8691eb0289b21da3571709a18391b0f rpc: Update server to use new RPCHelpMan (MarcoFalke) aaaaad562790cd4dce1568ae193f5393aacacedf rpc: Add option to hide RPCArg (MarcoFalke) fa9708f94c01cb8bf2971bdf404af38c38fa341b rpc: Assert that passed arg names are equal to hardcoded ones (MarcoFalke) faaeb2b0b347b40ce456a951eec5e820587e5b02 rpc: Add CRPCCommand constructor which takes RPCHelpMan (MarcoFalke) fa8ec00061567e56333bb69c5623919d45a9a92d rpc: Check that left section is not multiline (MarcoFalke) Pull request description: This is split out from #18531 to just touch the RPC methods in server. Description from the main pr: ### Motivation RPCArg names in the rpc help are currently only used for documentation. However, in the future they could be used to teach the server the named arguments. Named arguments are currently registered by the `CRPCCommand`s and duplicate the RPCArg names from the documentation. This redundancy is fragile, and has lead to errors in the past (despite having linters to catch those kind of errors). See section "bugs found" for a list of bugs that have been found as a result of the changes here. ### Changes The changes here add an assert in the `CRPCCommand` constructor that the RPCArg names are identical to the ones in the `CRPCCommand`. ### Future work > Here or follow up, makes sense to also assert type of returned UniValue? Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including: * Removing the CRPCCommand arguments, now that they are asserted to be equal and thus redundant * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table * Auto-formatting and sanity checking the RPCExamples with RPCMan * Checking passed-in json in self-check. Removing redundant checks * Checking returned json against documentation to avoid regressions or false documentation * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static ### Bugs found * The assert identified issue #18607 * The changes itself fixed bug #19250 ACKs for top commit: laanwj: ACK fa7592bfa8691eb0289b21da3571709a18391b0f ryanofsky: Code review ACK fa7592bfa8691eb0289b21da3571709a18391b0f. Looks great! Just some hidden arg and Check() and comment cleanups since last review Tree-SHA512: e64b6a212f4a3aeedeee47557559bde104d5fd40cdc1746b27eb2f3d4c8885d5e6e4dd287595ea11cdbc6a939654fe103cae765fd505875444d851f0abb11308
2020-07-03rpc: Update server to use new RPCHelpManMarcoFalke
Also, move Check to inside HandleRequest
2020-06-28refactor: Replace RecursiveMutex with Mutex in rpc/server.cppHennadii Stepanov
2020-05-29Limit scope of all global std::once_flagMarcoFalke
2020-05-29Merge #18452: qt: Fix shutdown when waitfor* cmds are called from RPC consoleJonas Schnelli
da73f1513a637a9f347b64de66564d6cdb2541f8 qt: Fix shutdown when waitfor* cmds are called from RPC console (Hennadii Stepanov) Pull request description: On master (7eed413e72a236b6f1475a198f7063fd24929e23), if the GUI has been started with`-server=1`, `bitcoin-qt` hangs on shutdown during calling any of the `waitfor*` commands in the GUI RPC console. This PR suggests minimal changes to fix this bug. Fix #17495 ACKs for top commit: jonasschnelli: utACK da73f1513a637a9f347b64de66564d6cdb2541f8 Tree-SHA512: 469f5332945a5f2c57d19336cda5df79b123ccc494aea6d58a85eb1293be52708b2b9c5bb6bc2c402a90b7b4e9e8d7ab8fe84cf201cf7ce612c9290c57e43681
2020-05-22rpc: factor out RpcInterruptionPoint from dumptxoutsetMarcoFalke
2020-05-13qt: Fix shutdown when waitfor* cmds are called from RPC consoleHennadii Stepanov
2020-04-29rpc: Add mutex to guard deadlineTimersJoão Barbosa
2020-03-13rpc: Document an RPCResult for all calls; Enforce at compile timeMarcoFalke
2020-03-06Merge #16117: util: Replace boost sleep with std sleepfanquake
fae86c38bca5c960462e53975314a0749db5d17d util: Remove unused MilliSleep (MarcoFalke) fa9af06d91e9357e86863781746f0e78a509967e scripted-diff: Replace MilliSleep with UninterruptibleSleep (MarcoFalke) fa4620be782c2bf6b5ffddf4f671194fdd1536f3 util: Add UnintrruptibleSleep (MarcoFalke) Pull request description: We don't use the interruptible feature of boost's sleep anywhere, so replace it with the sleep in `std::thread` ACKs for top commit: ajtowns: ACK fae86c38bca5c960462e53975314a0749db5d17d quick code review practicalswift: ACK fae86c38bca5c960462e53975314a0749db5d17d -- patch looks correct sipa: Concept and code review ACK fae86c38bca5c960462e53975314a0749db5d17d fanquake: ACK fae86c38bca5c960462e53975314a0749db5d17d - note that an instance of `DHAVE_WORKING_BOOST_SLEEP_FOR` was missed in the [linter](https://github.com/bitcoin/bitcoin/blob/master/test/lint/extended-lint-cppcheck.sh#L69), but that can be cleaned up later. Tree-SHA512: 7c0f8eb197664b9f7d9fe6c472c77d384f11c797c913afc31de4b532e3b4fd9ea6dd174f92062ff9d1ec39b25e0900ca7c597435add87f0f2477d9557204848c
2020-02-25rpc: Auto-format RPCResultMarcoFalke
2020-02-21scripted-diff: Replace MilliSleep with UninterruptibleSleepMarcoFalke
This is safe because MilliSleep is never executed in a boost::thread, the only type of thread that is interruptible. * The RPC server uses std::thread * The wallet is either executed in an RPC thread or the main thread * bitcoin-cli, benchmarks and tests are only one thread (the main thread) -BEGIN VERIFY SCRIPT- sed -i --regexp-extended -e 's/MilliSleep\((\S+)\);/UninterruptibleSleep(std::chrono::milliseconds{\1});/g' $(git grep -l MilliSleep) -END VERIFY SCRIPT-
2020-02-09scripted-diff: Add missing spaces in RPCResult, Fix type namesMarcoFalke
This makes the rendered diff smaller when the RPCResult is machine generated later on -BEGIN VERIFY SCRIPT- # Add space after dictionary key and before colon sed -i --regexp-extended -e 's/(^ +" +\\"[a-zA-Z_]+\\"): ?/\1 : /g' $(git grep -l '\\":') # Rename (array) to (json array) sed -i -e 's/ (array) / (json array) /g' $(git grep -l '(array)' ./src) # Rename (object) to (json object) sed -i -e 's/ (object) / (json object) /g' $(git grep -l '(object)' ./src) # Rename (bool) to (boolean) sed -i -e 's/ (bool) / (boolean) /g' $(git grep -l '(bool)' ./src) # Rename (int) to (numeric) sed -i -e 's/ (int) / (numeric) /g' $(git grep -l '(int)' ./src) -END VERIFY SCRIPT-
2020-01-15scripted-diff: Bump copyright of files changed in 2020MarcoFalke
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT-
2020-01-15scripted-diff: Replace CCriticalSection with RecursiveMutexMarcoFalke
-BEGIN VERIFY SCRIPT- # Delete outdated alias for RecursiveMutex sed -i -e '/CCriticalSection/d' ./src/sync.h # Replace use of outdated alias with RecursiveMutex sed -i -e 's/CCriticalSection/RecursiveMutex/g' $(git grep -l CCriticalSection) -END VERIFY SCRIPT-
2019-10-15Remove unused includespracticalswift
2019-08-01log: harmonize bitcoind server loggingJon Atack
Harmonize the user-facing output of the `bitcoind -daemon`, `bitcoin-cli help stop`, `bitcoin-cli stop`, and `bitcoind -version` commands to be consistent with each other as well as with the "Bitcoin Core is probably already running" messages, e.g. `git grep 'probably already running.")'`.
2019-07-09Merge #16240: JSONRPCRequest-aware RPCHelpManMarcoFalke
b6fb617aaaad5f9cdd7f2ad2825b253ca792055d rpc: switch to using RPCHelpMan.Check() (Karl-Johan Alm) c7a9fc234f3ce400ce78b9b434d2d210b2646c50 Make the RPCHelpMan aware of JSONRPCRequest and add Check() helper (Karl-Johan Alm) 5c5e32bbe3dfa790dd8bb421fbd6301ae10b09f5 rpc: migrate JSONRPCRequest functionality into request.cpp (Karl-Johan Alm) 0ab8ba1ac65b70f044a5e323b13d098cef33695a rpc: fix RPC help requirements for getblocktemplate (Karl-Johan Alm) Pull request description: Every single RPC call has a helper-section at the start, which throws a help string if the user asks for help or if the user provided too few/many arguments. ```C++ const RPCHelpMan help{...}; if (request.fHelp || !help.IsValidNumArgs(request.params.size())) { throw std::runtime_error(help.ToString()); } ``` or (older version) ```C++ if (request.fHelp || request.params.size() < min || request.params.size() > max) throw std::runtime_error( RPCHelpMan{...}.ToString() ); ``` It seems like an obvious improvement, and less copy-pasting, to make `RPCHelpMan` aware of `JSONRPCRequest`, and to let it handle the checks instead. Both of the above become ```C++ RPCHelpMan{...}.Check(request); ``` which means we save roughly 3 lines per RPC command, and the `RPCHelpMan` instance is never referenced afterwards, so the approach is a tiny fraction cleaner. This is a complete update, sans a few special case locations that had special rules. 623 lines turn into 284 (which includes the addition to `RPCHelpMan`). ACKs for top commit: laanwj: code rview and lightly tested ACK b6fb617aaaad5f9cdd7f2ad2825b253ca792055d MarcoFalke: ACK b6fb617aaa, looked at the diff, verified move-only where applicable Tree-SHA512: eb73f47f812512905b852e313281d1c8df803db40a6188aa39d5a7586631664db6764491152a8a96769946c796dc56d38c6e3a66ddd06ba3fb9d20050e6274e1
2019-07-08rpc: switch to using RPCHelpMan.Check()Karl-Johan Alm
2019-07-05rpc: migrate JSONRPCRequest functionality into request.cppKarl-Johan Alm
2019-07-03Add logpath description for getrpcinfoGregory Sanders
generated by cgit v1.2.3 (git 2.26.2) at 2024-10-30 15:32:18 +0000