doc: add release note for #27460 (new importmempool RPC) #28637

pull theStack wants to merge 1 commits into bitcoin:master from theStack:202310-doc-add_importmempool_release_note changing 1 files +7 −0
  1. theStack commented at 5:53 pm on October 11, 2023: contributor
    This PR adds a missing release note for #27460.
  2. DrahtBot commented at 5:53 pm on October 11, 2023: contributor

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

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK glozow
    Stale ACK ismaelsadeeq

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

  3. DrahtBot added the label Docs on Oct 11, 2023
  4. fanquake added this to the milestone 26.0 on Oct 11, 2023
  5. in doc/release-notes-27460.md:1 in 19e99a0c53 outdated
    0@@ -0,0 +1,4 @@
    1+- A new `importmempool` RPC has been added. It loads a `mempool.dat` file (previously created via
    


    ismaelsadeeq commented at 6:21 pm on October 11, 2023:

    Is it only mempool.dat file saved from the savemempool RPC? I think a valid mempool.dat that was dumped after a node shutdown can also be imported?

    0A new `importmempool` RPC has been added. It loads a valid `mempool.dat` file and attempts to add its contents to the mempool. This can be useful to
    1  import mempool data from another node without having to modify the datadir contents and restart the node.
    

    theStack commented at 8:16 pm on October 11, 2023:
    You’re right, I dropped the savemempool mention as suggested. Thanks!
  6. theStack force-pushed on Oct 11, 2023
  7. theStack force-pushed on Oct 11, 2023
  8. ismaelsadeeq approved
  9. ismaelsadeeq commented at 8:50 pm on October 11, 2023: member
    ACK 356a7529e8048623429ed4d08c41622d1276c69b
  10. ajtowns commented at 2:17 am on October 12, 2023: contributor
    If this is going to be advertised in the release notes, shouldn’t it include more warnings? eg, importing someone else’s mempool.dat could result in pinning vectors so you miss out on txs being relayed across the regular network. If you enable any of the options when importing (fee deltas or unbroadcast set or time info) that could also cause problems.
  11. glozow commented at 1:56 pm on October 12, 2023: member

    If this is going to be advertised in the release notes, shouldn’t it include more warnings? eg, importing someone else’s mempool.dat could result in pinning vectors so you miss out on txs being relayed across the regular network. If you enable any of the options when importing (fee deltas or unbroadcast set or time info) that could also cause problems.

    +1 Could also mention that, If you want to apply fee deltas, it is recommended to use the getprioritisedtransactions and prioritisetransaction RPCs instead of the apply_fee_delta_priority option to avoid duplicates.

  12. theStack force-pushed on Oct 12, 2023
  13. theStack force-pushed on Oct 12, 2023
  14. theStack commented at 3:07 pm on October 12, 2023: contributor
    @ajtowns: Thanks for the feedback. I added one generic warning about import untrusted files (taken from the RPC help) and @glozow’s recommendation for applying fee deltas. Happy to take more suggestions.
  15. glozow commented at 9:15 am on October 13, 2023: member
    LGTM, @ajtowns ?
  16. willcl-ark commented at 10:07 am on October 13, 2023: contributor

    Could also mention that, If you want to apply fee deltas, it is recommended to use the getprioritisedtransactions and prioritisetransaction RPCs instead of the apply_fee_delta_priority option to avoid duplicates.

    What exactly does “to avoid duplicates” mean here? That we can end up with two versions of the same tx in the mempool (with different deltas), or something else?

  17. glozow commented at 10:19 am on October 13, 2023: member

    Could also mention that, If you want to apply fee deltas, it is recommended to use the getprioritisedtransactions and prioritisetransaction RPCs instead of the apply_fee_delta_priority option to avoid duplicates.

    What exactly does “to avoid duplicates” mean here? That we can end up with two versions of the same tx in the mempool (with different deltas), or something else?

    No you’ll never have duplicates in your mempool. I mean it just blindly applies the deltas, so you might prioritize a transaction twice. Though I guess using the other RPC doesn’t inherently prevent this; it’s just a way for you to sanitize what the deltas are + check what you already have before applying them. Sentence could be dropped as it seems like it could be confusing.

  18. ajtowns commented at 10:45 am on October 13, 2023: contributor

    LGTM, @ajtowns ?

    Seems okay. Perhaps “over-prioritising” or “double counting” would work better than “duplicates”.

  19. willcl-ark commented at 10:50 am on October 13, 2023: contributor

    LGTM, @ajtowns ?

    Seems okay. Perhaps “over-prioritising” or “double counting” would work better than “duplicates”.

    Agree. I was thinking something even more explicit though, like this:

    “If you want to apply fee deltas, it is recommended to use the getprioritisedtransactions and prioritisetransaction RPCs instead of the apply_fee_delta_priority option to avoid double-prioritising any already-prioritised transactions in the mempool.

  20. doc: add release note for #27460 (new `importmempool` RPC)
    Co-authored-by: glozow <gloriajzhao@gmail.com>
    1b672eb766
  21. theStack force-pushed on Oct 15, 2023
  22. DrahtBot added the label CI failed on Oct 15, 2023
  23. DrahtBot removed the label CI failed on Oct 16, 2023
  24. glozow commented at 2:56 pm on October 17, 2023: member
    ACK 1b672eb7665cc032f2c285b1cad331dc92685265
  25. DrahtBot requested review from ismaelsadeeq on Oct 17, 2023
  26. glozow merged this on Oct 18, 2023
  27. glozow closed this on Oct 18, 2023

  28. theStack deleted the branch on Oct 18, 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-07-03 10:13 UTC

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