refactor: Remove mempool global from net #17997

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:2001-netMempool changing 6 files +49 −44
  1. MarcoFalke commented at 7:14 PM on January 24, 2020: member

    To increase modularisation and simplify testing, remove the mempool global from net in favour of a mempool member.

    This is done in the same way it was done for the connection manager global.

  2. MarcoFalke added the label Refactoring on Jan 24, 2020
  3. MarcoFalke added the label P2P on Jan 24, 2020
  4. MarcoFalke force-pushed on Jan 24, 2020
  5. DrahtBot commented at 7:36 PM on January 24, 2020: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #18044 (Use wtxid for transaction relay by sdaftuar)
    • #17479 (Return BlockValidationState from ProcessNewBlock if CheckBlock/AcceptBlock fails by jnewbery)

    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. Empact commented at 9:31 PM on January 24, 2020: member

    Concept ACK - my only concern is that this increases the number of cases in which the local and the global form of the variable are shadowing one another. How about doing something like this (in whole or in part) to simplify the call patterns overall while making the member references locally explicit: https://github.com/Empact/bitcoin/tree/2020-01-peer-logic-members

  7. MarcoFalke commented at 5:02 PM on January 25, 2020: member

    my only concern is that this increases the number of cases in which the local and the global form of the variable are shadowing one another

    The global will be renamed to g_mempool, so this won't be an issue. See also #17564 (review)

    How about doing something like this (in whole or in part) to simplify the call patterns overall while making the member references locally explicit:

    Looks good. But seems to increase the diff and time needed to review, so I'd rather do it as a follow-up.

  8. MarcoFalke closed this on Jan 25, 2020

  9. MarcoFalke reopened this on Jan 25, 2020

  10. practicalswift commented at 8:39 AM on January 26, 2020: contributor

    Concept ACK

    Reduced use of globals is very welcome from a fuzzing perspective :)

  11. promag commented at 6:55 PM on January 26, 2020: member

    ACK fa6ff6e7346ca87dfcb9331d1b21b131c91a8f58, I think it makes more sense squashed.

  12. Empact commented at 3:34 AM on January 30, 2020: member

    ACK https://github.com/bitcoin/bitcoin/pull/17997/commits/fa6ff6e7346ca87dfcb9331d1b21b131c91a8f58 code review plus I checked each existing mempool reference to ensure there were no remaining references to ::mempool.

  13. DrahtBot cross-referenced this on Feb 11, 2020 from issue Use wtxid for transaction relay by sdaftuar
  14. DrahtBot cross-referenced this on Feb 11, 2020 from issue tests: Add fuzzing harness for ProcessMessage(...). Enables high-level fuzzing of the P2P layer. by practicalswift
  15. DrahtBot cross-referenced this on Feb 11, 2020 from issue Switch to weight units for all feerates computation by darosior
  16. DrahtBot cross-referenced this on Feb 11, 2020 from issue Return BlockValidationState from ProcessNewBlock if CheckBlock/AcceptBlock fails by jnewbery
  17. DrahtBot cross-referenced this on Feb 12, 2020 from issue Util: Allow scheduler to be mocked by amitiuttarwar
  18. DrahtBot added the label Needs rebase on Feb 18, 2020
  19. MarcoFalke force-pushed on Feb 18, 2020
  20. MarcoFalke commented at 4:26 PM on February 18, 2020: member

    Rebased

  21. promag commented at 5:02 PM on February 18, 2020: member

    Code review ACK fa2c363cca8cb2224bc90a47362244af9a9c498e.

  22. DrahtBot removed the label Needs rebase on Feb 18, 2020
  23. DrahtBot cross-referenced this on Mar 2, 2020 from issue Serve BIP 157 compact filters by jimpo
  24. DrahtBot added the label Needs rebase on Mar 11, 2020
  25. MarcoFalke force-pushed on Mar 11, 2020
  26. MarcoFalke force-pushed on Mar 11, 2020
  27. MarcoFalke commented at 8:13 PM on March 11, 2020: member

    Rebased and squashed, as requested by @promag in #17997#pullrequestreview-348398170

  28. DrahtBot removed the label Needs rebase on Mar 11, 2020
  29. MarcoFalke force-pushed on Mar 12, 2020
  30. refactor: Remove mempool global from net
    This refactor does two things:
    * Pass mempool in to PeerLogicValidation
    * Pass m_mempool around where needed
    fa7fea3654
  31. MarcoFalke force-pushed on Mar 12, 2020
  32. DrahtBot cross-referenced this on Mar 12, 2020 from issue refactor: Add params to node context by MarcoFalke
  33. jnewbery commented at 9:51 PM on March 13, 2020: member

    code review ACK fa7fea3654203bf7e7bd504589dd564af7fc749d

  34. jnewbery commented at 9:51 PM on March 13, 2020: member

    (looking forward to the mempool -> g_mempool rename)

  35. MarcoFalke merged this on Mar 16, 2020
  36. MarcoFalke closed this on Mar 16, 2020

  37. MarcoFalke deleted the branch on Mar 16, 2020
  38. sidhujag referenced this in commit 0740cffe39 on Mar 16, 2020
  39. MarkLTZ cross-referenced this on Apr 4, 2020 from issue Bitcoin PR tracking by MarkLTZ
  40. sidhujag referenced this in commit 0b546d1d67 on Nov 10, 2020
  41. Fabcien referenced this in commit 46971b8236 on Jan 4, 2021
  42. kwvg referenced this in commit ab71bb599e on Jan 11, 2022
  43. kwvg referenced this in commit 301e416ac8 on Jan 25, 2022
  44. bitcoin locked this on Feb 15, 2022

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: 2026-05-19 06:54 UTC