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.
This check has been moved to the wallet logic GetMinimumFee. The rpc call to
estimatesmartfee will now no longer return a result maxed with the mempool min
fee, but automated fee calculations from the wallet will produce the same result
as before and coincontrol and sendcoins dialogs in the GUI will correctly
display the right prospective fee.
changes to policy/fees.cpp include a big whitespace indentation change.
Some people keep thinking that MAX_BLOCK_BASE_SIZE is a separate
size limit from the weight limit when it fact it is superfluous,
and used in early tests before the witness data has been
validated or just to compute worst case sizes. The size checks
that use it would not behave any differently consensus wise
if they were eliminated completely.
Its correct value is not independently settable but is a function
of the weight limit and weight formula.
This patch just eliminates it and uses the scale factor as
required to compute the worse case constants.
It also moves the weight factor out of primitives into consensus,
which is a more logical place for it.
This makes it possible to mine to any wallet when multi-wallet mode is added.
Solves the same problem as #10649, but IMO in a cleaner way.
It also gets rid of the circuitous `ScriptForMining` method on
`CValidationInterface`, which really doesn't belong there.
After this change it's still possible to mine without wallet through
`generatetoaddress`.
40796e1 Remove references to priority that snuck back in in 870824e9. (Matt Corallo)
Tree-SHA512: fd6f772a9fdf14b3b125e84a79059d7ab34b3571b35dc48f8d4b9f22ea71c6cdd4ae88c2e135ae317a16744c28dd23cf7f7dd88ea9d8b2d408e57845ef87d03b
For the per confirmation number tracking of data, introduce a scale factor so that in the longer horizones confirmations are bucketed together at a resolution of the scale. (instead of 1008 individual data points for each fee bucket, have 42 data points each covering 24 different confirmation values.. (1-24), (25-48), etc.. )
Change the logic of estimateSmartFee to check a 60% threshold at half the target, a 85% threshold at the target and a 95% threshold at double the target. Always check the shortest time horizon possible and ensure that estimates are monotonically decreasing. Add a conservative mode, which makes sure that the 95% threshold is also met at longer time horizons as well.
Track information the ranges of fee rates that were used to calculate the fee estimates (the last range of fee rates in which the data points met the threshold and the first to fail) and provide an RPC call to return this information.
68af651 MOVEONLY: move TxConfirmStats to cpp (Alex Morcos)
2332f19 Initialize TxConfirmStats in constructor (Alex Morcos)
5ba81e5 Read and Write fee estimate file directly from CBlockPolicyEstimator (Alex Morcos)
14e10aa Call estimate(Smart)Fee directly from CBlockPolicyEstimator (Alex Morcos)
dbb9e36 Give CBlockPolicyEstimator it's own lock (Alex Morcos)
f6187d6 Make processBlockTx private. (Alex Morcos)
ae7327b Make feeEstimator its own global instance of CBlockPolicyEstimator (Alex Morcos)
Tree-SHA512: dbf3bd2b30822e609a35f3da519b62d23f8a50e564750695ddebd08553b4c01874ae3e07d792c6cc78cc377d2db33b951ffedc46ac7edaf5793f9ebb931713af
There is no point in even hashing a submitted block which doesn't have
a coinbase transaction.
This also results in more useful error reporting on corrupted input.
Thanks to rawodb for the bug report.
