rpc: Add submit option to generateblock #18933

pull maflcko wants to merge 2 commits into bitcoin:master from maflcko:2004-rpcBlock changing 3 files +29 −14
  1. maflcko commented at 2:18 pm on May 10, 2020: member
    When submit is turned off, a block can be generated and returned as hex, to be used for further tests. For example, it can be submitted on a different node, on a different interface (like p2p), or just never submitted and be used for other testing purposes.
  2. maflcko commented at 2:20 pm on May 10, 2020: member

    Requested by @instagibbs, I believe, in comment #17693 (review)

    This work is based on the generateblock RPC added by @andrewtoth in #17693

  3. maflcko added the label Tests on May 10, 2020
  4. andrewtoth commented at 4:35 pm on May 10, 2020: contributor
    generatetodescriptor will be available for at least 2 releases. Is there any concern that other users will depend on it? I suppose workarounds for it are easy enough.
  5. in doc/release-notes.md:107 in fa872b5f6e outdated 
    103@@ -104,6 +104,9 @@ Low-level changes
104 Tests
105 -----
106 
107+- The `genereratetodescriptor` RPC has been removed and replaced by the
    jonatack commented at 4:46 pm on May 10, 2020:
    s/genereratetodescriptor/generatetodescriptor/
    maflcko commented at 4:55 pm on May 10, 2020:
    Thanks, fixed.
  6. maflcko commented at 4:47 pm on May 10, 2020: member
    If anyone was quick enough to add generatetodescriptor to their test scripts, they are surely quick enough to pull it out again the next time they update their node to a new major version.
  7. jonatack commented at 4:48 pm on May 10, 2020: contributor
    Does removing the generatetodescriptor RPC require a deprecation cycle?
  8. maflcko commented at 4:53 pm on May 10, 2020: member

    Does removing the generatetodescriptor RPC require a deprecation cycle?

    I’d say no, because it is only used for testing. See also #18933 (comment)

  9. maflcko force-pushed on May 10, 2020
  10. andrewtoth commented at 5:50 pm on May 10, 2020: contributor
    I haven’t used generatetodescriptor myself, but I would imagine a use case would be to mine transactions in the mempool with it. With generateblock a user has to manually add all txids to get the same functionality. If we are replacing it, should we also add an option to generateblock to mine the mempool?
  11. maflcko force-pushed on May 10, 2020
  12. maflcko commented at 6:23 pm on May 10, 2020: member
    Ok, that can be done as a follow-up. Dropped the last commit faefc219b63825b3393ccea683645ecbbac0e169 because it was too controversial.
  13. DrahtBot commented at 9:36 pm on May 10, 2020: contributor

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

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK TheCharlatan, instagibbs

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    Conflicts

    No conflicts as of last run.

  14. in src/rpc/mining.cpp:1203 in fa58b4cd18 outdated 
    1199@@ -1187,7 +1200,7 @@ static const CRPCCommand commands[] =
1200 
1201     { "generating",         "generatetoaddress",      &generatetoaddress,      {"nblocks","address","maxtries"} },
1202     { "generating",         "generatetodescriptor",   &generatetodescriptor,   {"num_blocks","descriptor","maxtries"} },
1203-    { "generating",         "generateblock",          &generateblock,          {"output","transactions"} },
1204+    { "generating",         "generateblock",          &generateblock,          {"output","transactions","submit"} },
    instagibbs commented at 2:20 pm on May 11, 2020:
    Also should be added to “src/rpc/client.cpp” to be parsed as non-string on -cli, right?
    maflcko commented at 2:24 pm on May 11, 2020:

    Doh. Can’t wait until RPCMan does that for me

    #18531

    instagibbs commented at 2:25 pm on May 11, 2020:
    oh nice, will review
    maflcko commented at 4:53 pm on December 15, 2021:
    Thanks, fixed.
  15. instagibbs commented at 2:21 pm on May 11, 2020: member
    ACK aside from the json question
  16. andrewtoth commented at 3:25 am on May 12, 2020: contributor
    The comment this is based on (https://github.com/bitcoin/bitcoin/pull/17693#discussion_r393727104) suggests to bypass validity check. However, https://github.com/bitcoin/bitcoin/pull/18933/files#diff-9651347c8e00bed3ddc7631de569406dL364 still does that. Do we want to skip that check as well?
  17. DrahtBot added the label Needs rebase on May 23, 2020
  18. maflcko force-pushed on Dec 15, 2021
  19. DrahtBot removed the label Needs rebase on Dec 15, 2021
  20. maflcko force-pushed on Dec 15, 2021
  21. maflcko force-pushed on Dec 15, 2021
  22. DrahtBot added the label Needs rebase on Feb 21, 2022
  23. maflcko force-pushed on Feb 22, 2022
  24. DrahtBot removed the label Needs rebase on Feb 22, 2022
  25. DrahtBot added the label Needs rebase on Mar 25, 2022
  26. maflcko force-pushed on Mar 30, 2022
  27. DrahtBot removed the label Needs rebase on Mar 30, 2022
  28. maflcko force-pushed on Mar 31, 2022
  29. DrahtBot added the label Needs rebase on Apr 6, 2022
  30. maflcko force-pushed on Apr 6, 2022
  31. DrahtBot removed the label Needs rebase on Apr 6, 2022
  32. DrahtBot added the label Needs rebase on May 13, 2022
  33. maflcko force-pushed on May 13, 2022
  34. DrahtBot removed the label Needs rebase on May 13, 2022
  35. DrahtBot added the label Needs rebase on May 20, 2022
  36. maflcko force-pushed on May 27, 2022
  37. DrahtBot removed the label Needs rebase on May 27, 2022
  38. DrahtBot added the label Needs rebase on Aug 30, 2022
  39. achow101 commented at 6:08 pm on October 12, 2022: member
    Closing this as it has not had any activity in a while. If you are interested in continuing work on this, please leave a comment so that it can be reopened.
  40. achow101 closed this on Oct 12, 2022
  41. maflcko reopened this on Mar 10, 2023
  42. maflcko force-pushed on Mar 10, 2023
  43. refactor: Replace block_hash with block_out fab9a08e14
  44. rpc: Add submit option to generateblock fa18504d57
  45. maflcko force-pushed on Mar 10, 2023
  46. maflcko commented at 10:02 am on March 10, 2023: member

    Do we want to skip that check as well?

    Should be trivial to add in a follow-up with a one-line patch, if and when needed?

  47. DrahtBot removed the label Needs rebase on Mar 10, 2023
  48. TheCharlatan approved
  49. TheCharlatan commented at 10:14 am on March 21, 2023: contributor
    tACK fa18504d5767a40dc9827fb081633219bf251001
  50. fanquake requested review from instagibbs on Mar 23, 2023
  51. fanquake requested review from pinheadmz on Mar 23, 2023
  52. instagibbs commented at 12:21 pm on March 23, 2023: member
    ACK fa18504d5767a40dc9827fb081633219bf251001
  53. DrahtBot removed review request from instagibbs on Mar 23, 2023
  54. fanquake merged this on Mar 23, 2023
  55. fanquake closed this on Mar 23, 2023
  56. maflcko deleted the branch on Mar 23, 2023
  57. sidhujag referenced this in commit caceb360ad on Mar 23, 2023
  58. bitcoin locked this on Mar 22, 2024


maflcko andrewtoth jonatack DrahtBot instagibbs achow101 TheCharlatan


pinheadmz

Labels
Tests

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-09-29 01:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me