doc: Use FeeModes doc helper in estimatesmartfee #20568

MarcoFalke commented at 8:44 AM on December 4, 2020: member

Not sure why this doesn't use the doc helper, probably an oversight?

MarcoFalke added the label Docs on Dec 4, 2020

MarcoFalke removed the label Docs on Dec 4, 2020

MarcoFalke renamed this:
~~rpc: Use FeeModes doc helper in estimatesmartfee~~
doc: Use FeeModes doc helper in estimatesmartfee
on Dec 4, 2020

fanquake added the label RPC/REST/ZMQ on Dec 4, 2020

MarcoFalke added the label Docs on Dec 4, 2020

MarcoFalke commented at 8:49 AM on December 4, 2020: member

Rendered diff:

diff --git a/estimatesmartfee b/estimatesmartfee
index b424d58..3a3193a 100644
--- a/estimatesmartfee
+++ b/estimatesmartfee
@@ -1,35 +1,35 @@
 estimatesmartfee conf_target ( "estimate_mode" )
 
 Estimates the approximate fee per kilobyte needed for a transaction to begin
 confirmation within conf_target blocks if possible and return the number of blocks
 for which the estimate is valid. Uses virtual transaction size as defined
 in BIP 141 (witness data is discounted).
 
 Arguments:
 1. conf_target      (numeric, required) Confirmation target in blocks (1 - 1008)
-2. estimate_mode    (string, optional, default=CONSERVATIVE) The fee estimate mode.
+2. estimate_mode    (string, optional, default=conservative) The fee estimate mode.
                     Whether to return a more conservative estimate which also satisfies
                     a longer history. A conservative estimate potentially returns a
                     higher feerate and is more likely to be sufficient for the desired
                     target, but is not as responsive to short term drops in the
                     prevailing fee market.  Must be one of:
-                    "UNSET"
-                    "ECONOMICAL"
-                    "CONSERVATIVE"
+                    "unset"
+                    "economical"
+                    "conservative"
 
 Result:
 {                   (json object)
   "feerate" : n,    (numeric, optional) estimate fee rate in BTC/kB (only present if no errors were encountered)
   "errors" : [      (json array, optional) Errors encountered during processing (if there are any)
     "str",          (string) error
     ...
   ],
   "blocks" : n      (numeric) block number where estimate was found
                     The request target will be clamped between 2 and the highest target
                     fee estimation is able to return based on how long it has been running.
                     An error is returned if not enough transactions and blocks
                     have been observed to make an estimate for any number of blocks.
 }
 
 Examples:
 > bitcoin-cli estimatesmartfee 6

in src/rpc/mining.cpp:1037 in fa8bdb8763 outdated

1033 | @@ -1034,9 +1034,7 @@ static RPCHelpMan estimatesmartfee()
1034 |              "                   higher feerate and is more likely to be sufficient for the desired\n"
1035 |              "                   target, but is not as responsive to short term drops in the\n"
1036 |              "                   prevailing fee market.  Must be one of:\n"
1037 | -            "       \"UNSET\"\n"
1038 | -            "       \"ECONOMICAL\"\n"
1039 | -            "       \"CONSERVATIVE\""},
1040 | +             "\"" + FeeModes("\"\n\"") + "\""},

laanwj commented at 9:09 AM on December 4, 2020:

This changes the strings to lowercase. I'm not sure that affects anything, but in any case you'd likely also want to change the CONSERVATIVE above to that.

MarcoFalke commented at 9:18 AM on December 4, 2020:

Thanks, done

MarcoFalke force-pushed on Dec 4, 2020

luke-jr approved

luke-jr commented at 5:04 PM on December 6, 2020: member

utACK

in src/rpc/mining.cpp:1037 in fa09513fb6 outdated

1042 |              "                   target, but is not as responsive to short term drops in the\n"
1043 |              "                   prevailing fee market.  Must be one of:\n"
1044 | -            "       \"UNSET\"\n"
1045 | -            "       \"ECONOMICAL\"\n"
1046 | -            "       \"CONSERVATIVE\""},
1047 | +             "\"" + FeeModes("\"\n\"") + "\""},

jonatack commented at 8:17 PM on December 6, 2020:

As the values will be displayed in lowercase, perhaps update the functional test:

--- a/test/functional/rpc_estimatefee.py
+++ b/test/functional/rpc_estimatefee.py
@@ -41,6 +41,8 @@ class EstimateFeeTest(BitcoinTestFramework):
         self.nodes[0].estimatesmartfee(1)
         # self.nodes[0].estimatesmartfee(1, None)
         self.nodes[0].estimatesmartfee(1, 'ECONOMICAL')
+        self.nodes[0].estimatesmartfee(1, 'unset')
+        self.nodes[0].estimatesmartfee(1, 'conservative')

MarcoFalke commented at 8:29 AM on December 7, 2020:

thanks, fixed

jonatack commented at 8:44 PM on December 6, 2020: member

Tested ACK fa09513fb609dbb0d

in src/rpc/mining.cpp:1036 in fa09513fb6 outdated

1038 | +            {"estimate_mode", RPCArg::Type::STR, /* default */ "conservative", "The fee estimate mode.\n"
1039 |              "                   Whether to return a more conservative estimate which also satisfies\n"
1040 |              "                   a longer history. A conservative estimate potentially returns a\n"
1041 |              "                   higher feerate and is more likely to be sufficient for the desired\n"
1042 |              "                   target, but is not as responsive to short term drops in the\n"
1043 |              "                   prevailing fee market.  Must be one of:\n"

jonatack commented at 8:46 PM on December 6, 2020:

for info, the other RPC estimate_mode helps state

            "                   prevailing fee market.  Must be one of (case insensitive):\n"

perhaps do the same for user-facing consistency

kristapsk approved

kristapsk commented at 9:49 PM on December 6, 2020: contributor

ACK fa09513fb609dbb0d7381289505de72257902edc

rpc: Use FeeModes doc helper in estimatesmartfee

Can be reviewed with --ignore-all-space

fa8abdc995

MarcoFalke force-pushed on Dec 7, 2020

laanwj commented at 12:54 PM on December 7, 2020: member

Code review ACK fa8abdc9953e381715493b259908e246914793b0

laanwj merged this on Dec 7, 2020

laanwj closed this on Dec 7, 2020

MarcoFalke deleted the branch on Dec 7, 2020

sidhujag referenced this in commit 8d434285be on Dec 7, 2020

DrahtBot locked this on Feb 15, 2022