Add getdeploymentinfo RPC #23508

pull ajtowns wants to merge 6 commits into bitcoin:master from ajtowns:202111-getforkinfo changing 11 files +250 −91
  1. ajtowns commented at 1:29 am on November 14, 2021: member
    The aim of this PR is to improve the ability to monitor soft fork status. It first moves the softfork section from getblockchaininfo into a new RPC named getdeploymentinfo, which is then also able to query the status of forks at an arbitrary block rather than only at the tip. In addition, bip9 status is changed to indicate the status of the given block, rather than just for the next block, and an additional field is included to indicate whether each block in the signalling period signaled.
  2. fanquake added the label RPC/REST/ZMQ on Nov 14, 2021
  3. ajtowns commented at 1:39 am on November 14, 2021: member

    Example output:

     0$ ./bitcoin-cli getdeploymentinfo 000000000000000000013712fc242ee6dd28476d0e9c931c75f83e6974c6bccc | jq .hash,.height,.deployments.taproot
     1"000000000000000000013712fc242ee6dd28476d0e9c931c75f83e6974c6bccc"
     2709631
     3{
     4  "type": "bip9",
     5  "height": 709632,
     6  "active": true,
     7  "bip9": {
     8    "bit": 2,
     9    "start_time": 1619222400,
    10    "timeout": 1628640000,
    11    "since": 687456,
    12    "min_activation_height": 709632,
    13    "status": "locked_in",
    14    "status-next": "active",
    15    "statistics": {
    16      "period": 2016,
    17      "elapsed": 2016,
    18      "count": 1891
    19    },
    20    "signalling": "#########-######-######################################-####################-###################################-##################################################-##################-############################################-#######-#####################-#########-#######################################-########-#######-##################-######-#################-####################-###########-#########################-##########-######################################-#####-###########-####-###############################-##-###-#############-###-########-###########-#######-#-##################--########################-############-#####-###########-###-################-####################################-###-#####-###############-#######################-######################-###########################################################-#-#-#######################-######-########################-##########-####################-####################-################--##-#######################-#########-############-#####-#######################-#############-#####################-##################-##########-####-####-############-#######-#########-##################################################-#############################-###############-######################-#####-###########################-##-##-######-########################-#########-##-##################-######################-########################################-####-########################-#########################################################################-####-########-#####-#######################-##########-##########-########################-#-######################################-#############-####-#####-##############-#-############-#############-###-######-##################-############################-########-#########-########-#######-############################################-#-#######################################-#-###########-########-##########-############-##########-#########"
    21  }
    22}
    23$ ./bitcoin-cli getdeploymentinfo $(./bitcoin-cli getblockhash 683423) | jq '.deployments.taproot'
    24{
    25  "type": "bip9",
    26  "active": false,
    27  "bip9": {
    28    "bit": 2,
    29    "start_time": 1619222400,
    30    "timeout": 1628640000,
    31    "since": 681408,
    32    "min_activation_height": 709632,
    33    "status": "started",
    34    "status-next": "started",
    35    "statistics": {
    36      "period": 2016,
    37      "elapsed": 2016,
    38      "count": 823,
    39      "threshold": 1815,
    40      "possible": false
    41    },
    42    "signalling": 
    43        "--------------------------------------------------#------------------------------------------------------#------------------#----------#------##
    44         ----#-----#---#--#-----#-------#----#------#---###----##----#---#--------#---#-#--------#----##-#----#------------------###--####-----------#---
    45         #-#----##-------#----#-----#-#-#--##---#-#---##-#-#-##---#---------#-----#---------------##---#---#---##--#----#--#---#--##-#--#----#------#-#--
    46         ##-##-----#---#--#-##--#-#####---#-#--##---#-#-#----#-----#----#---#-#-#-##--###----#-#----####--###--#-----#-####--####--##-#-##----#----#----#
    47         #-#--#####---#-#-#--##-#--###-###-##--#-#---###-#----#-###-#----####--#---############---#--#-####-----####------#-#--###--##-#-#---########-#--
    48         #-#-#--###--###-----#--#--##-----#-###-###-####----#-#--#-##-#----#---#-######----#####-#---#---###-##--###-##----##-#####-#--#-###-#----###-##-
    49         ##--##---#-#-#-##-##-#-#--#--#-##--#-##-#-#####--#-#---#########----##-###-#-#-#-----###--#-##-#---##--#-###-----##-----#-##--###--#-##-###---#-
    50         ###-###-----##-#---#------#-###-##-#-##---#--#---#--#-##--###-##-##---#-#-#----#--#--#-#-##--#--#-###--###--#--#---#####----#--#---#####---#-##-
    51         ----###--##---#--##-#---##-#-------#---#--#-#---##-##---###----#-#-#---#-#---##---#-#-##--#-###--##--#------#-###--#-#---##----#####-#-----#-###
    52         #-#------###-##-###-------###----#----##-#--#--##----####-##--#-#-#-#-##--##----#--##-#-##--#--#-#-###-###---------####-#####--#--#--##-------##
    53         ---##--#--#-##-##---##-###-#----#----##-#-#---###-#--##--#---#-----#-#-#-----###-##-------#-#-####--##-####--####-#---#-#--##-#---#-##------#-#-
    54         -#--#-####-#------#-#-###-----##--##-##-#####-##-####--#----#---#---#-##-###-###-######------###--##---#----####-#------###-###-#------#####-##-
    55         ###-#-###-#--#--###--#####---##-#-#-#-#---#######-#-#---#-#----###-##----##-#-#-#-#-##-#-#--###-#-#-#####---#--#--##-----#----#-#-#----#-##-----
    56         -##---##--###---###--#-##--#--###-#-##-##--##-----#-#-##-#-#--#-#-#--#-##-##-##-#--######--#--##-#-#-#---##-##---#---###-#--#----##--#-#-#-#-##-"
    57  }
    58}
    

    (signalling results split into sections of 144 blocks by hand so it’s easier to see on github; 683423 is the last block in taproot’s first signalling period)

  4. ajtowns force-pushed on Nov 14, 2021
  5. DrahtBot commented at 3:05 am on November 14, 2021: member

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #24032 (Refactor vDeployments setup to avoid uninitialized variables by ajtowns)
    • #22016 (rpc: add period_start to version bits statistics by Sjors)
    • #21702 (Implement BIP-119 Validation (CheckTemplateVerify) by JeremyRubin)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  6. ajtowns force-pushed on Nov 14, 2021
  7. ajtowns force-pushed on Nov 14, 2021
  8. MarcoFalke commented at 9:14 am on November 14, 2021: member
    My first thought was that getchaintips already reports all chain “forks”, so this won’t be needed. Maybe this could be named deploymentinfo or so to avoid confusion with cutlery?
  9. ajtowns force-pushed on Nov 14, 2021
  10. ajtowns renamed this:
    Add getforkinfo RPC
    Add getdeploymentinfo RPC
    on Nov 14, 2021
  11. ajtowns commented at 1:11 pm on November 14, 2021: member
    Changed from getforkinfo to getdeploymentinfo, since it seemed like a knife idea.
  12. benthecarman commented at 11:06 pm on November 14, 2021: contributor

    Concept ACK

    this is really cool!

  13. brunoerg approved
  14. brunoerg commented at 11:04 am on November 15, 2021: member

    tACK 684821c52b5894e0407bcbd853fa3e8323049e83 (MacOS 11.3)

     0➜  bitcoin git:(ajtowns) ✗ ./src/bitcoin-cli getdeploymentinfo
     1{
     2  "*": {
     3    "hash": "00000000000001f0c73d20c4843c1eac8658b5f49e2d80032ff1feaeb543fe35",
     4    "height": 207120
     5  },
     6  "bip34": {
     7    "type": "buried",
     8    "active": false,
     9    "height": 227931
    10  },
    11  "bip66": {
    12    "type": "buried",
    13    "active": false,
    14    "height": 363725
    15  },
    16  "bip65": {
    17    "type": "buried",
    18    "active": false,
    19    "height": 388381
    20  },
    21  "csv": {
    22    "type": "buried",
    23    "active": false,
    24    "height": 419328
    25  },
    26  "segwit": {
    27    "type": "buried",
    28    "active": false,
    29    "height": 481824
    30  },
    31  "taproot": {
    32    "type": "bip9",
    33    "active": false,
    34    "bip9": {
    35      "start_time": 1619222400,
    36      "timeout": 1628640000,
    37      "since": 0,
    38      "min_activation_height": 709632,
    39      "status": "defined",
    40      "status-next": "defined"
    41    }
    42  }
    43}
    44➜  bitcoin git:(ajtowns) ✗ ./src/bitcoin-cli getdeploymentinfo | jq '."*", .taproot'
    45{
    46  "hash": "0000000000000228e53941db17bc6eb44079f4e1677f39eb8afc73335842da68",
    47  "height": 210280
    48}
    49{
    50  "type": "bip9",
    51  "active": false,
    52  "bip9": {
    53    "start_time": 1619222400,
    54    "timeout": 1628640000,
    55    "since": 0,
    56    "min_activation_height": 709632,
    57    "status": "defined",
    58    "status-next": "defined"
    59  }
    60}
    

    Functional tests:

  15. w0xlt approved
  16. w0xlt commented at 7:02 pm on November 18, 2021: contributor
    tACK 684821c on Ubuntu 21.10.
  17. luke-jr changes_requested
  18. luke-jr commented at 7:45 pm on November 30, 2021: member
    Should remain in getblockchaininfo, at least as deprecated, for now
  19. ajtowns force-pushed on Dec 2, 2021
  20. ajtowns commented at 5:31 am on December 2, 2021: member
    Updated. -deprecatedrpc=softforks will retain the softforks info in getblockchaininfo. Also tweaked the getdeploymentinfo output.
  21. laanwj commented at 5:44 pm on December 8, 2021: member
    Concept ACK. I was first skeptical about moving this to a new RPC, but being able to pass in a block hash to evaluate the status as of an arbitrary block is a great idea.
  22. in src/rpc/blockchain.cpp:1592 in 797b8533f7 outdated
    1597+                                {RPCResult::Type::NUM, "height", "block height"},
    1598+                            }},
    1599+                            {RPCResult::Type::OBJ, "xxxx", "name of the softfork",
    1600+                            {
    1601+                                {RPCResult::Type::STR, "type", "one of \"buried\", \"bip9\""},
    1602+                                {RPCResult::Type::NUM, "height", /* optional */ true, "height of the first block which the rules are or will be enforced (only for \"buried\" type, or \"bip9\" type with \"active\" status)"},
    


    fanquake commented at 9:06 am on December 9, 2021:

    Please use this syntax so arguments can be automatically checked by clang-tidy. This is what we are migrating towards. i.e #23545.

    0                                {RPCResult::Type::NUM, "height", /*optional=*/true, "height of the first block which the rules are or will be enforced (only for \"buried\" type, or \"bip9\" type with \"active\" status)"},
    

    ajtowns commented at 12:33 pm on December 9, 2021:
    Thanks, updated
  23. DrahtBot added the label Needs rebase on Dec 9, 2021
  24. ajtowns force-pushed on Dec 9, 2021
  25. ajtowns commented at 12:33 pm on December 9, 2021: member

    Rebased past #23703 and tweaked the commit descriptions slightly.

    For testing on mainnet, the following little shell snippet shows some interesting block heights:

    0EARLY="0 1"
    1BURIED="227930 363724 388380 419327 481823"
    2TAPROOT="681406 681407 681408 681409 683423 685439 687455 689471 691487 693503 695519 697535 699551 701567 703583 705599 707615 709631 711647"
    3for a in $EARLY $BURIED $TAPROOT; do ./bitcoin-cli getdeploymentinfo $(./bitcoin-cli getblockhash $a); done | less
    

    Note that "active": true indicates a soft fork is being applied as of the next block, hence the blocks above are 1 below the actual activation heights.

  26. DrahtBot removed the label Needs rebase on Dec 9, 2021
  27. laanwj added the label Needs release note on Dec 9, 2021
  28. laanwj commented at 5:25 pm on December 9, 2021: member
    Needs release note wrt deprecation.
  29. in src/versionbits.h:70 in 4662cf3a81 outdated
    63@@ -64,8 +64,10 @@ class AbstractThresholdConditionChecker {
    64     virtual int Threshold(const Consensus::Params& params) const =0;
    65 
    66 public:
    67-    /** Returns the numerical statistics of an in-progress BIP9 softfork in the current period */
    68-    BIP9Stats GetStateStatisticsFor(const CBlockIndex* pindex, const Consensus::Params& params) const;
    69+    /** Returns the numerical statistics of an in-progress BIP9 softfork in the period including pindex
    70+     * If provided, signals is set to true/false based on whether each block in the period signalled
    71+     */
    72+    BIP9Stats GetStateStatisticsFor(const CBlockIndex* pindex, const Consensus::Params& params, std::vector<bool>* signals = nullptr) const;
    


    laanwj commented at 5:34 pm on December 9, 2021:
    signals is not a particularly good name for this parameter, imo, i had no idea what was meant (or what the index is) at first, maybe signallingBlocks?

    ajtowns commented at 8:29 am on December 13, 2021:
    Changed to signalling_blocks
  30. ajtowns force-pushed on Dec 13, 2021
  31. ajtowns commented at 8:29 am on December 13, 2021: member
    Added release note
  32. in src/rpc/blockchain.cpp:1621 in a4749f9154 outdated
    1588+static RPCHelpMan getdeploymentinfo()
    1589+{
    1590+    return RPCHelpMan{"getdeploymentinfo",
    1591+        "Returns an object containing various state info regarding soft-forks.\n",
    1592+        {},
    1593+        RPCResult{
    


    Sjors commented at 6:15 am on December 16, 2021:

    a4749f9154a579c8da9d6353587fe83c72a8fc31: you could avoid the duplication here with a helper function, see e.g. bumpfee_helper in wallet/rpc/spend.cpp

    Otherwise, maybe reduce indentation.

  33. in src/rpc/blockchain.cpp:1654 in a4749f9154 outdated
    1624+            const CBlockIndex* tip = active_chainstate.m_chain.Tip();
    1625+            CHECK_NONFATAL(tip);
    1626+            const Consensus::Params& consensusParams = Params().GetConsensus();
    1627+
    1628+            UniValue deploymentinfo(UniValue::VOBJ);
    1629+            deploymentinfo.pushKV("deployments", DeploymentInfo(tip, consensusParams));
    


    Sjors commented at 6:18 am on December 16, 2021:
    Nesting everything under deployments seems useless.

    ajtowns commented at 9:02 am on December 17, 2021:
    It separates out height and hash which are introduced in the following commit.
  34. Sjors commented at 6:51 am on December 16, 2021: member

    Concept ACK

    8d2d1b29cd6cd5799c47c0debfe63e2eddc6a347 is a breaking change. For my own use case in ForkMonitor it’s fine, but if someone relies on the status field before putting something in the mempool right in time for the first block of a new softfork, they’d miss it. But that’s not a problem anytime soon… It would be good to add a test case for a block right before activation (where active should be true, state should be locked_in and state_next should be active).

  35. ajtowns commented at 10:45 am on December 17, 2021: member

    8d2d1b2 is a breaking change. For my own use case in ForkMonitor it’s fine, but if someone relies on the status field before putting something in the mempool right in time for the first block of a new softfork, they’d miss it.

    Using active is far better than using status for that case, since it is valid for non-bip9 deployments as well. Also there are no pending deployments for which anyone will be mining such a block (so you would need to either be using regtest with non-default config, or trying to do a massive reorg of mainnet or testnet).

  36. ajtowns commented at 4:13 am on January 13, 2022: member
    @Sjors @laanwj @luke-jr are your concerns adequately addressed?
  37. Sjors commented at 12:24 pm on January 13, 2022: member

    It would be good to add a test case for a block right before activation

    This test would be nice. None of my comments are blocking.

  38. ajtowns commented at 3:18 pm on January 13, 2022: member
    @Sjors added such a test
  39. Sjors commented at 5:31 pm on January 13, 2022: member

    I improved the RPC help a bit in https://github.com/Sjors/bitcoin/commit/af1070dced9a39658799e93c9842d7aab0eb40c4:

    • less excessive indentation
    • get rid of "*" : { structure (hash and height are same level as deployments)
    • nest forks inside deployments
    • clarify that hash and height use the tip by default

    Before:

    0Result:
    1{                                     (json object)
    2  "*" : {                             (json object)
    3    "hash" : "str",                   (string) block hash
    4    "height" : n                      (numeric) block height
    5  },
    6  "xxxx" : {                          (json object) name of the softfork
    7    "type" : "str",                   (string) one of "buried", "bip9"
    

    After:

    0Result:
    1{                                       (json object)
    2  "hash" : "str",                       (string) requested block hash (or tip)
    3  "height" : n,                         (numeric) requested block height (or tip)
    4  "deployments" : {                     (json object) status of softforks
    5    "xxxx" : {                          (json object) name of the softfork
    6      "type" : "str",                   (string) one of "buried", "bip9"
    
  40. in doc/release-notes-23508.md:8 in 517356dafd outdated
    0@@ -0,0 +1,8 @@
    1+Updated RPCs
    2+------------
    3+
    4+Information on soft fork status has been moved from `getblockchaininfo` to
    5+`getdeploymentinfo` which allows querying soft fork status at any block,
    6+rather than just at the chain tip. Inclusion of soft fork status in
    7+`getblockchaininfo` can currently be restored using the configuration
    8+`-deprecatedrpc=softforks`, but this will be removed in a future release.
    


    MarcoFalke commented at 5:35 pm on January 13, 2022:
    0`-deprecatedrpc=softforks`, but this will be removed in a future release. (#23508)
    

    nit

  41. in doc/release-notes-23508.md:4 in 517356dafd outdated
    0@@ -0,0 +1,8 @@
    1+Updated RPCs
    2+------------
    3+
    4+Information on soft fork status has been moved from `getblockchaininfo` to
    


    MarcoFalke commented at 5:35 pm on January 13, 2022:
    0- Information on soft fork status has been moved from `getblockchaininfo` to
    

    nit: items in the notes are lists

  42. in src/rpc/blockchain.cpp:1604 in 517356dafd outdated
    1609 }
    1610 
    1611+static RPCHelpMan getdeploymentinfo()
    1612+{
    1613+    return RPCHelpMan{"getdeploymentinfo",
    1614+        "Returns an object containing various state info regarding soft-forks.\n",
    


    MarcoFalke commented at 5:36 pm on January 13, 2022:
    0        "Returns an object containing various state info regarding soft-forks.",
    

    nit: not needed

  43. MarcoFalke commented at 5:37 pm on January 13, 2022: member
    left some stupid nits (feel free to ignore)
  44. laanwj added this to the "Blockers" column in a project

  45. ajtowns force-pushed on Jan 13, 2022
  46. ajtowns commented at 11:39 pm on January 13, 2022: member
    @Sjors Thanks, included (combined with the nit from @MarcoFalke). @MarcoFalke, de-nitted.
  47. Sjors commented at 12:30 pm on January 14, 2022: member
    @ajtowns thanks. Unless you’re waiting for additional feedback, I suggest squashing the last two commits in to earlier ones. That makes reviewing the individual commits more straight-forward.
  48. ajtowns force-pushed on Jan 14, 2022
  49. rpc: move softfork info from getblockchaininfo to getdeploymentinfo fd826130a0
  50. rpc: getdeploymentinfo: allow specifying a blockhash other than tip 7f15c1841b
  51. rpc: getdeploymentinfo: change stats to always refer to current period
    On a period boundary, getdeploymentinfo (and previously getblockchaininfo)
    would report the status and statistics for the next block rather than
    the current block. Change this to always report the status/statistics
    of the current block, but add status-next to report the status for the
    next block.
    a7469bcd35
  52. rpc: getdeploymentinfo: include block hash/height 376c0c6dae
  53. rpc: getdeploymentinfo: include signalling info 240cad09ba
  54. Release notes for getdeploymentinfo rpc a380922891
  55. ajtowns force-pushed on Jan 14, 2022
  56. ajtowns commented at 6:46 pm on January 14, 2022: member
    @Sjors okay, done that, and also deduplicated the help text between getblockchaininfo and getdeploymentinfo.
  57. in src/rpc/blockchain.cpp:1610 in 7f15c1841b outdated
    1606@@ -1605,8 +1607,18 @@ static RPCHelpMan getdeploymentinfo()
    1607             LOCK(cs_main);
    1608             CChainState& active_chainstate = chainman.ActiveChainstate();
    1609 
    1610-            const CBlockIndex* tip = active_chainstate.m_chain.Tip();
    1611-            CHECK_NONFATAL(tip);
    1612+            const CBlockIndex* tip;
    


    Sjors commented at 8:41 pm on January 14, 2022:
    7f15c1841b98de6931a7ac68e16635a05d3e96cf: this variable should probably be called block. I suggest also renaming the tip parameter of DeploymentInfo( and the active_chain_tip parameter of SoftForkDescPushBack to block

    ajtowns commented at 6:52 pm on January 20, 2022:
    Agree, but leaving this for later
  58. Sjors approved
  59. Sjors commented at 8:53 pm on January 14, 2022: member

    tACK a3809228917b8f750090c8bfec8e283391dbb524

    Small steps to making this stuff less confusing… (see also https://github.com/btcsuite/btcd/pull/1700#issuecomment-1013128081)

  60. in src/versionbits.cpp:110 in 240cad09ba outdated
    107@@ -108,18 +108,26 @@ BIP9Stats AbstractThresholdConditionChecker::GetStateStatisticsFor(const CBlockI
    108     if (pindex == nullptr) return stats;
    109 
    110     // Find beginning of period
    


    fjahr commented at 9:56 pm on January 15, 2022:
    This comment doesn’t seem to fit the code anymore

    ajtowns commented at 7:33 am on January 28, 2022:
    Addressed in #24187
  61. in src/rpc/blockchain.cpp:1459 in fd826130a0 outdated
    1454@@ -1455,8 +1455,16 @@ static void SoftForkDescPushBack(const CBlockIndex* active_chain_tip, UniValue&
    1455     softforks.pushKV(DeploymentName(id), rv);
    1456 }
    1457 
    1458+namespace {
    1459+/* TODO: when -dprecatedrpc=softforks is removed, drop these */
    


    fjahr commented at 9:59 pm on January 15, 2022:
    nit: typo in deprecated

    ajtowns commented at 7:33 am on January 28, 2022:
    Fixed in #24187
  62. MarcoFalke removed the label Needs release note on Jan 20, 2022
  63. in src/rpc/blockchain.cpp:1444 in a7469bcd35 outdated
    1450-    int64_t since_height = g_versionbitscache.StateSinceHeight(active_chain_tip, consensusParams, id);
    1451-    bip9.pushKV("since", since_height);
    1452+    bip9.pushKV("min_activation_height", consensusParams.vDeployments[id].min_activation_height);
    1453+
    1454+    // BIP9 status
    1455+    bip9.pushKV("status", get_state_name(current_state));
    


    MarcoFalke commented at 9:01 pm on January 20, 2022:
    Looks like a behavior change, so might be worth to mention in the release notes if you retouch.

    MarcoFalke commented at 5:49 pm on January 27, 2022:
    If you don’t retouch, I think this should be fixed up in a follow-up

    ajtowns commented at 7:33 am on January 28, 2022:
    Followed up in #24187
  64. fjahr commented at 0:32 am on January 21, 2022: member

    tACK a3809228917b8f750090c8bfec8e283391dbb524

    Reviewed code and tested many historical statistics around the Taproot deployment.

  65. in src/rpc/blockchain.cpp:2810 in a380922891
    2806@@ -2716,6 +2807,7 @@ static const CRPCCommand commands[] =
    2807     { "blockchain",         &getblockheader,                     },
    2808     { "blockchain",         &getchaintips,                       },
    2809     { "blockchain",         &getdifficulty,                      },
    2810+    { "blockchain",         &getdeploymentinfo,                  },
    


    jonatack commented at 4:43 pm on January 27, 2022:
    fd82613 this line should perhaps be before getdifficulty, not sure, feel free to ignore

    ajtowns commented at 7:04 am on January 28, 2022:
    These aren’t in alphabetical order, and I don’t think it quite fits in with any of the other categories of functions (info about the tip, mempool stuff, stats, queries about a particular block; and the functions aren’t really grouped by category anyway), so not seeing much benefit to moving it.
  66. in src/rpc/blockchain.cpp:1635 in a380922891
    1634+        RPCExamples{ HelpExampleCli("getdeploymentinfo", "") + HelpExampleRpc("getdeploymentinfo", "") },
    1635+        [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue
    1636+        {
    1637+            ChainstateManager& chainman = EnsureAnyChainman(request.context);
    1638+            LOCK(cs_main);
    1639+            CChainState& active_chainstate = chainman.ActiveChainstate();
    


    jonatack commented at 5:05 pm on January 27, 2022:

    fd826130a0a4e67fdc26f8064f4ecb4ff79b3333

    0-            ChainstateManager& chainman = EnsureAnyChainman(request.context);
    1-            LOCK(cs_main);
    2-            CChainState& active_chainstate = chainman.ActiveChainstate();
    3+            const ChainstateManager& chainman = EnsureAnyChainman(request.context);
    4+            LOCK(::cs_main);
    5+            const CChainState& active_chainstate = chainman.ActiveChainstate();
    

    ajtowns commented at 7:34 am on January 28, 2022:
    Added in #24187
  67. in src/rpc/blockchain.cpp:1642 in a380922891
    1641+            const CBlockIndex* tip;
    1642+            if (request.params[0].isNull()) {
    1643+                tip = active_chainstate.m_chain.Tip();
    1644+                CHECK_NONFATAL(tip);
    1645+            } else {
    1646+                uint256 hash(ParseHashV(request.params[0], "blockhash"));
    


    jonatack commented at 5:09 pm on January 27, 2022:

    7f15c184

    0                const uint256& hash{ParseHashV(request.params[0], "blockhash")};
    

    ajtowns commented at 7:34 am on January 28, 2022:
    Added in #24187
  68. in src/rpc/blockchain.cpp:1619 in a380922891
    1618+static RPCHelpMan getdeploymentinfo()
    1619+{
    1620+    return RPCHelpMan{"getdeploymentinfo",
    1621+        "Returns an object containing various state info regarding soft-forks.",
    1622+        {
    1623+            {"blockhash", RPCArg::Type::STR_HEX, RPCArg::Default{"chain tip"}, "The block hash at which to query fork state"},
    


    jonatack commented at 5:12 pm on January 27, 2022:

    7f15c1841b98de6931a7ac68e16635a05d3e96cf current naming style for new rpc arguments, and a default description that (maybe) looks a little less like “chain tip” could be a value

    0Arguments:
    11. blockhash    (string, optional, default="chain tip") The block hash at which to query fork state
    
    0            {"block_hash", RPCArg::Type::STR_HEX, RPCArg::Default{"value of chain tip"}, "The block hash at which to query fork state"},
    

    It would be good to have tests for invoking getdeploymentinfo with a valid and an invalid block hash.


    Sjors commented at 5:53 pm on January 27, 2022:
    This code is reused in the deprecated getblockchaininfo part, so let’s not rename these things. Except for new items.

    jonatack commented at 6:27 pm on January 27, 2022:
    Oh that’s true; the functionality is nicely reused. Correcting myself, the argument is only in the new RPC, so it could follow current naming style.

    ajtowns commented at 6:42 am on January 28, 2022:
    Ugh, why are we introducing new naming styles that are incompatible with the existing names, especially where they’re part of an API? :vomiting_face:
  69. in doc/release-notes-23508.md:5 in a380922891
    0@@ -0,0 +1,9 @@
    1+Updated RPCs
    2+------------
    3+
    4+- Information on soft fork status has been moved from `getblockchaininfo`
    5+  to `getdeploymentinfo` which allows querying soft fork status at any
    


    jonatack commented at 5:24 pm on January 27, 2022:

    a3809228

    0  to a new `getdeploymentinfo` RPC, which allows querying soft fork status at any
    

    ajtowns commented at 7:34 am on January 28, 2022:
    Added in #24187
  70. in src/rpc/blockchain.cpp:1617 in a380922891
    1616-    return obj;
    1617-},
    1618+static RPCHelpMan getdeploymentinfo()
    1619+{
    1620+    return RPCHelpMan{"getdeploymentinfo",
    1621+        "Returns an object containing various state info regarding soft-forks.",
    


    jonatack commented at 5:26 pm on January 27, 2022:

    fd82613 It looks like you are writing “softfork” in the RPC documentation, and “soft fork” in the release note, so maybe standardize (on soft fork) throughout.

    0        "Returns an object containing various state info regarding soft forks.",
    

    MarcoFalke commented at 6:01 pm on January 27, 2022:
    buried deployments aren’t soft forks, so it might be better to call this “deployments”

    ajtowns commented at 7:35 am on January 28, 2022:
    Switched from soft[ -]*forks in #24187. Note that rpc/miner.cpp calls them “softforks” in its RPC docs…

    MarcoFalke commented at 7:44 am on January 28, 2022:
    Yeah, I am not suggesting to change existing fields, but for new ones we can pick a good name from the beginning.
  71. laanwj commented at 5:39 pm on January 27, 2022: member
    Code review and lightly tested ACK a3809228917b8f750090c8bfec8e283391dbb524
  72. in src/rpc/blockchain.cpp:1587 in a380922891
    1583+        {RPCResult::Type::NUM_TIME, "start_time", "the minimum median time past of a block at which the bit gains its meaning"},
    1584+        {RPCResult::Type::NUM_TIME, "timeout", "the median time past of a block at which the deployment is considered failed if not yet locked in"},
    1585+        {RPCResult::Type::NUM, "min_activation_height", "minimum height of blocks for which the rules may be enforced"},
    1586+        {RPCResult::Type::STR, "status", "bip9 status of specified block (one of \"defined\", \"started\", \"locked_in\", \"active\", \"failed\")"},
    1587+        {RPCResult::Type::NUM, "since", "height of the first block to which the status applies"},
    1588+        {RPCResult::Type::STR, "status-next", "bip9 status of next block"},
    


    jonatack commented at 5:40 pm on January 27, 2022:

    Throughout the RPC documentation, when referring to the BIP itself and not a field name or input/output value

    0        {RPCResult::Type::STR, "status-next", "BIP 9 status of the next block"},
    

    ajtowns commented at 7:35 am on January 28, 2022:
    Changed in #24187
  73. jonatack commented at 5:49 pm on January 27, 2022: member
    WIP, reviewed the first 3 commits and the release note. A few minor comments to pick/choose.
  74. ajtowns commented at 7:36 am on January 28, 2022: member
    Opened #24187 for non-code-changing followups.
  75. MarcoFalke merged this on Jan 28, 2022
  76. MarcoFalke closed this on Jan 28, 2022

  77. MarcoFalke removed this from the "Blockers" column in a project

  78. sidhujag referenced this in commit bedd40fa70 on Jan 28, 2022
  79. MarcoFalke referenced this in commit af0b578041 on Feb 14, 2022
  80. sidhujag referenced this in commit 2d957724f7 on Feb 14, 2022
  81. MarcoFalke referenced this in commit 8270740bef on May 17, 2022
  82. sidhujag referenced this in commit c64b45dc82 on May 28, 2022
  83. Roasbeef referenced this in commit fabf989e5e on Aug 5, 2022
  84. Roasbeef referenced this in commit 592af654d0 on Aug 5, 2022
  85. Roasbeef referenced this in commit 3a37ef3cb8 on Aug 5, 2022
  86. Roasbeef referenced this in commit 0718e5d6ee on Aug 6, 2022
  87. Roasbeef referenced this in commit 492f8b6999 on Aug 6, 2022
  88. Roasbeef referenced this in commit deeaa0c9c7 on Aug 13, 2022
  89. achow101 referenced this in commit 92be831847 on Oct 13, 2022
  90. sidhujag referenced this in commit 151c348370 on Oct 23, 2022
  91. DrahtBot locked this on Jan 28, 2023

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-30 00:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me