e78aaf41f4 [docs] Add release notes for burying bip 9 soft fork deployments (John Newbery)
8319e738f9 [tests] Add coverage for the content of getblockchaininfo.softforks (James O'Beirne)
0328dcdcfc [Consensus] Bury segwit deployment (John Newbery)
1c93b9b31c [Consensus] Bury CSV deployment height (John Newbery)
3862e473f0 [rpc] Tidy up reporting of buried and ongoing softforks (John Newbery)
Pull request description:
This hardcodes CSV and segwit activation heights, similar to the BIP 90 buried deployments for BIPs 34, 65 and 66.
CSV and segwit have been active for over 18 months. Hardcoding the activation height is a code simplification, makes it easier to understand segwit activation status, and reduces technical debt.
This was originally attempted by jl2012 in #11398 and again by me in #12360.
ACKs for top commit:
ajtowns:
ACK e78aaf41f4 ; checked diff to previous acked commit, checked tests still work
ariard:
ACK e78aaf4, check diff, run the tests again and successfully activated csv/segwit heights on mainnet as expected.
MarcoFalke:
ACK e78aaf41f4 (still didn't check if the mainnet block heights are correct, but the code looks good now)
Tree-SHA512: 7e951829106e21a81725f7d3e236eddbb59349189740907bb47e33f5dbf95c43753ac1231f47ae7bee85c8c81b2146afcdfdc11deb1503947f23093a9c399912
b6fb617aaa rpc: switch to using RPCHelpMan.Check() (Karl-Johan Alm)
c7a9fc234f Make the RPCHelpMan aware of JSONRPCRequest and add Check() helper (Karl-Johan Alm)
5c5e32bbe3 rpc: migrate JSONRPCRequest functionality into request.cpp (Karl-Johan Alm)
0ab8ba1ac6 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 b6fb617aaa
MarcoFalke:
ACK b6fb617aaa, looked at the diff, verified move-only where applicable
Tree-SHA512: eb73f47f812512905b852e313281d1c8df803db40a6188aa39d5a7586631664db6764491152a8a96769946c796dc56d38c6e3a66ddd06ba3fb9d20050e6274e1
3b9bf0eb0e rpc: Allow shutdown while in generateblocks (Patrick Strateman)
Pull request description:
By checking the shutdown flag every loop we can use the entire 32 bit nonce space instead of breaking every 16 bits to check the flag.
This is possible now because the shutdown flag is an atomic where before it was controlled by a condition variable and lock.
ACKs for top commit:
kallewoof:
Re-ACK 3b9bf0e
Tree-SHA512: d0664201a55215130c2e9199a31fb81361daf4102a65cb3418984fd61cb98bfb9136d9ee8d23a85d57e50051f9bb0059bd71fe0488a17f63c38ea5caa6004504
By checking the shutdown flag every loop we can use the entire nonce space
instead of breaking every 16 bits to check the shutdown flag.
This has been possible since the shutdown flag was switched to an atomic,
before that change it was controlled by a condition variable and lock.
Though at the moment ChainActive() simply references `g_chainstate.m_chain`,
doing this change now clears the way for multiple chainstate usage and allows
us to script the diff.
-BEGIN VERIFY SCRIPT-
git grep -l "chainActive" | grep -E '(h|cpp)$' | xargs sed -i '/chainActive =/b; /extern CChain& chainActive/b; s/\(::\)\{0,1\}chainActive/::ChainActive()/g'
-END VERIFY SCRIPT-
Adds the following util units and adds them to libbitcoin_util:
- `util/url.cpp` takes `urlDecode` from `httpserver.cpp`
- `util/error.cpp` takes `TransactionErrorString` from
`node/transaction.cpp` and `AmountHighWarn` and `AmountErrMsg` from
`ui_interface.cpp`
- `util/fees.cpp` takes `StringForFeeReason` and `FeeModeFromString` from `policy/fees.cpp`
- `util/rbf.cpp` takes `SignalsOptInRBF` from `policy/rbf.cpp`
- 'util/validation.cpp` takes `FormatStateMessage` and `strMessageMagic` from 'validation.cpp`
fa178a6385 [rpc] mining: Omit uninitialized currentblockweight, currentblocktx (MarcoFalke)
Pull request description:
Previously we'd report "0", which could be mistaken for a valid number. E.g. the number of transactions is 0 or the block weight is 0, whatever that means.
Tree-SHA512: ee94ab203a329e272211b726f4c23edec4b09c650ec363b77fd59ad9264165d73064f78ebb9e11b5c2c543b73c157752410a307655560531c7d5444d203aa0ea
fa0ad4e7ce RPCHelpMan: Check default values are given at compile-time (MarcoFalke)
Pull request description:
Remove the run time assertions on the default values and ensure that the correct default type and value is provided at compile time.
Tree-SHA512: 80df2f3fab4379b500c773c27da63f22786c58be5963fe99744746320e43627a5d433eedf8b32209158df7805ebdce65ed4d242c829c4fe6e5d13deb4799ed42
d2ce315fbf [docs] add release note for change to GBT (John Newbery)
0025c9eae4 [mining] segwit option must be set in GBT (John Newbery)
Pull request description:
Calling getblocktemplate without the segwit rule specified is most
likely a client error, since it results in lower fees for the miner.
Prevent this client error by failing getblocktemplate if called without
the segwit rule specified.
Of the previous 1000 blocks (measured at block [551591 (hash 0x...173c811)](https://blockstream.info/block/000000000000000000173c811e79858808abc3216af607035973f002bef60a7a)), 991 included segwit transactions.
Tree-SHA512: 7933b073d72683c9ab9318db46a085ec19a56a14937945c73f783ac7656887619a86b74db0bdfcb8121df44f63a1d6a6fb19e98505b2a26a6a8a6e768e442fee
Calling getblocktemplate without the segwit rule specified is most
likely a client error, since it results in lower fees for the miner.
Prevent this client error by failing getblocktemplate if called without
the segwit rule specified.
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.
'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'
6fa901fb47 Don't edit Chainparams after initialization (Jorge Timón)
980b38f8a1 MOVEONLY: Move versionbits info out of versionbits.o (Jorge Timón)
Pull request description:
This encapsulates the "-vbparams" option, which is only meant for regtest, directly on CRegTestParams.
This is a refactor and doesn't change functionality.
Related to https://github.com/bitcoin/bitcoin/pull/8994
Tree-SHA512: 79771d729a63a720e743a9c77d5e2d80369f072d66202a43c1304e83a7d0ef7c6103d4968a03aea9666cc89a7203c618da972124a677b38cfe62ddaeb28f9f5d
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
The RPC was removed in a previous version, but a warning was
left for users to use the estimatesmartfee RPC. Remove that warning now
that estimatefee has been gone for over one version.
fa6ab8ada1 rpc: Return more specific reject reason for submitblock (MarcoFalke)
Pull request description:
The second commit in #13439 made the `TODO` in the first commit impossible to solve.
The meaning of `fNewBlock` changed from "This is the first time we process this block" to "We are about to write the new *valid* block".
So whenever `fNewBlock` is true, the block was valid. And whenever the `fNewBlock` is false, the block is either valid or invalid. If it was valid and not new, we know it is a `"duplicate"`. In all other cases, the `BIP22ValidationResult()` will return the reason why it is invalid.
Tree-SHA512: 4b6edf7a912339c3acb0fccfabbdd6d812a0321fb1639c244c2714e58dc119aa2b8c6bf8f7d61ea609a1b861bbc23f920370fcf989c48452721e259a8ce93d24
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
f34c8c466a Make objects in range declarations immutable by default. Avoid unnecessary copying of objects in range declarations. (practicalswift)
Pull request description:
Make objects in range declarations immutable by default.
Rationale:
* Immutable objects are easier to reason about.
* Prevents accidental or hard-to-notice change of value.
Tree-SHA512: cad69d35f0cf8a938b848e65dd537c621d96fe3369be306b65ef0cd1baf6cc0a9f28bc230e1e383d810c555a6743d08cb6b2b0bd51856d4611f537a12e5abb8b
9c4dc597dd Use LOCK macros for non-recursive locks (Russell Yanofsky)
1382913e61 Make LOCK, LOCK2, TRY_LOCK work with CWaitableCriticalSection (Russell Yanofsky)
ba1f095aad MOVEONLY Move AnnotatedMixin declaration (Russell Yanofsky)
41b88e9337 Add unit test for DEBUG_LOCKORDER code (Russell Yanofsky)
Pull request description:
Make LOCK macros work with non-recursive mutexes, and use wherever possible for better deadlock detection.
Also add unit test for DEBUG_LOCKORDER code.
Tree-SHA512: 64ef209307f28ecd0813a283f15c6406138c6ffe7f6cbbd084161044db60e2c099a7d0d2edcd1c5e7770a115e9b931b486e86c9a777bdc96d2e8a9f4dc192942
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.
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)"
ac8a1d092e [RPC] Remove field in getblocktemplate help that has never been used (Conor Scott)
Pull request description:
[BIP 22 - getblocktemplate](https://github.com/bitcoin/bips/blob/master/bip-0022.mediawiki#Transactions%20Object%20Format) specifies an optional flag, `required` if the transaction must be in the block.
Luke's implementation #936 did not include this flag, and it was later added to the help description in #3246 (more than a year later) but the field was still never actually implemented. As far as I can tell, bitcoin core would have never actually included this in a `getblocktemplate` call, so it seems logical to remove it from the help description.
If I am missing something or this is considered harmless - I can close the PR.
Tree-SHA512: f25dda51cc4e1512aff69309be04e3053bdccc1cf03c8d58e8866aa1fdf9d86cc57df872e85528351fc8a8d6d64a8f46a36c513680834762d854f368fbeb0f44
Most includers just wanted to react to pending shutdown.
This isolates access to `fRequestShutdown` and limits access to the shutdown
api functions, including the new `AbortShutdown` for setting it to `false`.
Note I originally called `AbortShutdown` `CancelShutdown` but that name was
already taken by winuser.h
https://travis-ci.org/bitcoin/bitcoin/jobs/386913329
This change also triggered a build error in bench. Fixing it required moving LIBBITCOIN_SERVER after LIBBITCOIN_WALLET in bench_bench_bitcoin_LDADD To make
server definitions in src/net.cpp available to wallet methods in
src/wallet/wallet.cpp. Specifically, solving:
libbitcoin_wallet.a(libbitcoin_wallet_a-wallet.o): In function `CWalletTx::RelayWalletTransaction(CConnman*)':
wallet.cpp:(.text+0x3f0e): undefined reference to `CConnman::NodeFullyConnected(CNode const*)'
collect2: error: ld returned 1 exit status
https://travis-ci.org/bitcoin/bitcoin/jobs/392133581
Need for remaining init.h includes confirmed via a thorough search with a more
specific regex:
\bInterrupt\(\)|\bShutdown\(\)|\bInitLogging\(\)|\bInitParameterInteraction\(\)|\bAppInitBasicSetup\(\)|\bAppInitParameterInteraction\(\)|\bAppInitSanityChecks\(\)|\bAppInitLockDataDirectory\(\)|\bAppInitMain\(\)|\bSetupServerArgs\(\)|\bLicenseInfo\(\)|g_wallet_init_interface|init.h
f74894480 Only set fNewBlock to true in AcceptBlock when we write to disk (Matt Corallo)
fa6e49731 rpc: Avoid "duplicate" return value for invalid submitblock (MarcoFalke)
Pull request description:
This is #13395 with one more commit tacked on. MarcoFalke got tired of dealing with the stupidity of fixing a return code with too many rounds of review (not that I blame him). Honestly we should probably have no return whatsoever, but for now, this fixes it (as well as nLastBlockTime for eviction purposes).
Original description:
When `submitblock` of an invalid block, the return value should not be `"duplicate"`.
This is only seen when the header was previously found (denoted by the incorrectly named boolean `fBlockPresent`). Fix this bug by removing `fBlockPresent`.
Tree-SHA512: 0ce3092655d5d904b4c8c5ff7479f73ce387144a738f20472b8af132564005c6db5594ae366e589508f6258506ee7a28b1c7995a83a8328b334f99316006bf2d
This removes the need to include rpc/blockchain.cpp in order to put
GetDifficulty under test. GetDifficulty was called in two ways:
* with a guaranteed non-null blockindex
* with no argument
Change the latter case to be provided chainActive.Tip() explicitly.
159c32d1f1 Add assertion to guide static analyzers. Clang Static Analyzer needs this guidance. (practicalswift)
fd447a6efe Fix dead stores. Values were stored but never read. Limit scope. (practicalswift)
Pull request description:
Fix Clang Static Analyzer warnings reported by @kallewoof in #12961:
* Fix dead stores. Values were stored but never read.
* Add assertion to guide static analyzers. See #12961 for details.
Tree-SHA512: 83dbec821f45217637316bee978e7543f2d2caeb7f7b0b3aec107fede0fff8baa756da8f6b761ae0d38537740839ac9752f6689109c38a4b05c0c041aaa3a1fb
0851a75 rpc: Interrupt block generation on shutdown request (João Barbosa)
Pull request description:
With this simple change, after running `bitcoin-cli -regtest generate 100000`, it is possible to interrupt `bitcoind` cleanly without waiting for the generation to complete.
Tree-SHA512: f0f7cdde242e595cfdaea31ae8bddbc25933621b63f639e813d272c2b00ce2ef52f0c14ae44954ba8c49f0fc846bcc3bfd5419e52b3347a68bb0341ce6b02d26
4a6c0e3dcf Modernize best block mutex/cv/hash variable naming (Pieter Wuille)
45dd135039 Fix csBestBlock/cvBlockChange waiting in rpc/mining (Pieter Wuille)
Pull request description:
This is an alternative to #11694.
It reintroduces a uint256 variable with the best block hash, protected by csBestBlock, and only updated while holding it.
Also rename the involved variable to modern guidelines, as there are very few uses.
Tree-SHA512: 826a86c7d3cee7fe49f99f4398ae99e81cb0563197eaeba77306a3ca6072b67cdb932bc35720fc0f99c2a57b218efa029d0b8bdfb240591a629b2e90efa3199d
c198dc00e1 [Doc] Clarify the meaning of fee delta not being a fee rate in prioritisetransaction RPC (Jan Čapek)
Pull request description:
Hi,
I have faced some confusion among our developers considering this being a fee rate. Would you consider including this tiny doc update?
Best regards,
Jan Capek
Tree-SHA512: cd0560540418e53c5c19ceab2d5aca229f4ef6b788b9543695742522e1c63a7f2cce2574b47fead098a106da2f77e297f0c728474565f6259b50d62369bbe7da
1f45e21 scripted-diff: Convert 11 enums into scoped enums (C++11) (practicalswift)
Pull request description:
Rationale (from Bjarne Stroustrup's ["C++11 FAQ"](http://www.stroustrup.com/C++11FAQ.html#enum)):
>
> The enum classes ("new enums", "strong enums") address three problems with traditional C++ enumerations:
>
> * conventional enums implicitly convert to int, causing errors when someone does not want an enumeration to act as an integer.
> * conventional enums export their enumerators to the surrounding scope, causing name clashes.
> * the underlying type of an enum cannot be specified, causing confusion, compatibility problems, and makes forward declaration impossible.
>
> The new enums are "enum class" because they combine aspects of traditional enumerations (names values) with aspects of classes (scoped members and absence of conversions).
Tree-SHA512: 9656e1cf4c3cabd4378c7a38d0c2eaf79e4a54d204a3c5762330840e55ee7e141e188a3efb2b4daf0ef3110bbaff80d8b9253abf2a9b015cdc4d60b49ac2b914
92fabcd44 Add LookupBlockIndex function (João Barbosa)
43a32b739 Add missing cs_lock in CreateWalletFromFile (João Barbosa)
f814a3e8f Fix cs_main lock in LoadExternalBlockFile (João Barbosa)
c651df8b3 Lock cs_main while loading block index in AppInitMain (João Barbosa)
02de6a6bc Assert cs_main is held when accessing mapBlockIndex (João Barbosa)
Pull request description:
Replace all `mapBlockIndex` lookups with the new `LookupBlockIndex()`. In some cases it avoids a second lookup.
Tree-SHA512: ca31118f028a19721f2191d86f2dd398144d04df345694575a64aeb293be2f85785201480c3c578a0ec99690516205708558c0fd4168b09313378fd4e60a8412
bb00c95 Consistently use FormatStateMessage in RPC error output (Ben Woosley)
8b8a1c4 Add test for 'mempool min fee not met' rpc error (Ben Woosley)
c04e0f6 Fix 'mempool min fee not met' debug output (Ben Woosley)
Pull request description:
Output the value that is tested, rather than the unmodified fee value.
Prompted by looking into: #11955
Tree-SHA512: fc0bad47d4af375d208f657a6ccbad6ef7f4e2989ae2ce1171226c22fa92847494a2c55cca687bd5a1548663ed3313569bcc31c00d53c0c193a1b865dd8a7657
Commit 1.
This code was written by @TheBlueMatt in the following branch:
* https://github.com/TheBlueMatt/bitcoin/commits/2017-08-test-10923
This commit message was written by me (@practicalswift) who also squashed
@TheBlueMatt's commits into one and tried to summarize the changes made.
Commit 2.
Remove boost include. Remove boost mentions in comments.
Changes the errors field to warnings. To maintain compatibility,
the errors field is deprecated and enabled by starting bitcoind with
-deprecatedrpc=getmininginfo
Deprecate estimatefee in v0.16, for final removal in v0.17.
This commit introduces a phased removal of RPC methods. RPC method is
disabled by default in version x, but can be enabled by using the
`-deprecatedrpc=<method>` argument. RPC method is removed entirely in
version (x+1).
5acd82de9 rpc: make estimatesmartfee argument naming consistent with documentation (Wladimir J. van der Laan)
24697c40e rpc: update cli for estimatefee argument rename (Wladimir J. van der Laan)
Pull request description:
The first argument of `estimaterawfee` was renamed from `nblocks` to `conf_target` in 06bcdb8da6. Update the client-side table as well.
This makes #10753 pass again.
Tree-SHA512: 107c0072a45e0f4e083dc803d534973e6bd4c005e62337a867815d7c98ab1c21d97b7a495c32763883975cbbb001b80003001a6709b7d9bdd81ce4d441b667be
* This removes block-size-limiting code in favor of GBT clients
doing the limiting themselves (if at all).
* -blockmaxsize is deprecated and only used to calculate an implied
blockmaxweight, addressing confusion from multiple users.
* getmininginfo's currentblocksize return value was returning
garbage values, and has been removed, also removing a
GetSerializeSize call in some block generation inner loops and
potentially addressing some performance edge cases.
This patch removes the need for the intermediary Base58 type
CBitcoinAddress, by providing {Encode,Decode,IsValid}Destination
function that directly operate on the conversion between strings
and CTxDestination.
47ba2c312 Fix currency/fee-rate unit string in the help text (Akio Nakamura)
Pull request description:
1. The RPC help text should use the constant `CURRENCY_UNIT` defined in `policy/feerate.cpp` instead of the literal `'BTC'`.
In the following 2 RPC commands, `'BTC'` is written directly in the help text.
This commit changes them to use that constant.
1) `estimatesmartfee`
2) `estimaterawfee`
2. Some RPC command use `'satoshis'` as the unit.
It should be written as `'satoshis'` instead of `'Satoshis'` in the RPC help text.
So, this commit fixes this typo in `getblocktemplate`.
Tree-SHA512: d0bd1cd90560e59bf456b076b958a2a1c998f85a7e65aeb6b2abcaba18919a3ae62f7c3909210461084c1a3275a35b6ba3ea3ec8f5cce33702ffe383c9e84bce
1. The RPC help text should use the constant CURRENCY_UNIT defined in
policy/feerate.cpp instead of the literal 'BTC'. In the following
2 RPC commands, 'BTC' is written directly in the help text.
1) estimatesmartfee
2) estimaterawfee
And also, for these help strings, the notation
'fee-per-kilobyte (in BTC)' is somewhat ambiguous.
To write more precisely, this commit changes to 'fee rate in BTC/kB'
with using the constant CURRENCY_UNIT.
2. Some RPC command use 'satoshis' as the unit. It should be written
as 'satoshis' instead of 'Satoshis' in the RPC help text.
So, this commit fixes this typo in getblocktemplate.
3. The phrase that '... feerate (BTC per KB) ...' is used to explain
the fee rate in the help text of following 2 RPC commands.
1) getmempoolinfo
2) fundrawtransaction
But they are different from other similar help text of the RPCs.
And also, 'KB' implies Kibibyte (2^10 byte).
To unify and to clarify, this commit changes these phrase to
'... fee rate in BTC/kB ...'.
(BTC references the constant 'CURRENCY_UNIT')
This contains most of the changes of 10563 "remove safe mode", but doesn't
remove the safe mode yet, but put an `ObserveSafeMode()` check in
individual calls with okSafeMode=false.
This cleans up the ugly "okSafeMode" flag from the dispatch tables,
which is not a concern for the RPC server.
Extra-author: Wladimir J. van der Laan <laanwj@gmail.com>
745d2e3 Clean up getbalance RPC parameter handling (Russell Yanofsky)
fd5d71e Update developer notes after params.size() cleanup (Russell Yanofsky)
e067673 Avoid treating null RPC arguments different from missing arguments (Russell Yanofsky)
e666efc Get rid of redundant RPC params.size() checks (Russell Yanofsky)
Pull request description:
This is a followup to #10783.
- The first commit doesn't change behavior at all, just simplifies code.
- The second commit just changes RPC methods to treat null arguments the same as missing arguments instead of throwing type errors.
- The third commit updates developer notes after the cleanup.
- The forth commit does some additional code cleanup in `getbalance`.
Followup changes that should happen in future PRs:
- [ ] Replace uses of `.isTrue()` with calls to `.get_bool()` so numbers, objects, and strings cause type errors instead of being interpreted as false. https://github.com/bitcoin/bitcoin/pull/11050#discussion_r133850525
- [ ] Add braces around if statements. https://github.com/bitcoin/bitcoin/pull/11050#discussion_r133851133
- [ ] Maybe improve UniValue type error exceptions and eliminate RPCTypeCheck and RPCTypeCheckArgument functions. https://github.com/bitcoin/bitcoin/pull/11050#discussion_r133829303
Tree-SHA512: e72f696011d20acc0778e996659e41f9426bffce387b29ff63bf59ad1163d5146761e4445b2b9b9e069a80596a57c7f4402b75a15d5d20f69f775ae558cf67e9
4dc1915 check for null values in rpc args and handle appropriately (Gregory Sanders)
999ef20 importmulti options are optional (Gregory Sanders)
a70d025 fixup some rpc param counting for rpc help (Gregory Sanders)
Pull request description:
Audited where named args will fail to use correct default values or may fail when additional optional arguments are added.
Previously for these parameters, it was fine to omit them as positional arguments, but it would trigger UniValue runtime errors to set them to null, or to omit them while passing named parameters with greater positions (which would internally set earlier missing arguments to null). Now null values are treated the same as missing values so these errors do not occur.
Included a few other small fixes while working on it.
I didn't bother fixing account-based rpc calls.
Tree-SHA512: 8baf781a35bd48de7878d4726850a580dab80323d3416c1c146b4fa9062f8a233c03f37e8ae3f3159e9d04a8f39c326627ca64c14e1cb7ce72538f934ab2ae1e
Change parameter for conservative estimates to be an estimate_mode string.
Change to never return a -1 for failure but to instead omit the feerate and
return an error string. Throw JSONRPC error on invalid nblocks parameter.
