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

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.
MarcoFalke added the label Refactoring on Jan 24, 2020
MarcoFalke added the label P2P on Jan 24, 2020
MarcoFalke force-pushed on Jan 24, 2020
DrahtBot commented at 7:36 PM on January 24, 2020: 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:
- #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.
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
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.
MarcoFalke closed this on Jan 25, 2020
MarcoFalke reopened this on Jan 25, 2020
practicalswift commented at 8:39 AM on January 26, 2020: contributor

Concept ACK

Reduced use of globals is very welcome from a fuzzing perspective :)
promag commented at 6:55 PM on January 26, 2020: member

ACK fa6ff6e7346ca87dfcb9331d1b21b131c91a8f58, I think it makes more sense squashed.
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.
DrahtBot added the label Needs rebase on Feb 18, 2020
MarcoFalke force-pushed on Feb 18, 2020
MarcoFalke commented at 4:26 PM on February 18, 2020: member

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

Code review ACK fa2c363cca8cb2224bc90a47362244af9a9c498e.
DrahtBot removed the label Needs rebase on Feb 18, 2020
DrahtBot added the label Needs rebase on Mar 11, 2020
MarcoFalke force-pushed on Mar 11, 2020
MarcoFalke force-pushed on Mar 11, 2020
MarcoFalke commented at 8:13 PM on March 11, 2020: member

Rebased and squashed, as requested by @promag in #17997#pullrequestreview-348398170
DrahtBot removed the label Needs rebase on Mar 11, 2020
MarcoFalke force-pushed on Mar 12, 2020

refactor: Remove mempool global from net

This refactor does two things:
* Pass mempool in to PeerLogicValidation
* Pass m_mempool around where needed

fa7fea3654

MarcoFalke force-pushed on Mar 12, 2020
jnewbery commented at 9:51 PM on March 13, 2020: member

code review ACK fa7fea3654203bf7e7bd504589dd564af7fc749d
jnewbery commented at 9:51 PM on March 13, 2020: member

(looking forward to the mempool -> g_mempool rename)
MarcoFalke merged this on Mar 16, 2020
MarcoFalke closed this on Mar 16, 2020
MarcoFalke deleted the branch on Mar 16, 2020
sidhujag referenced this in commit 0740cffe39 on Mar 16, 2020
sidhujag referenced this in commit 0b546d1d67 on Nov 10, 2020
Fabcien referenced this in commit 46971b8236 on Jan 4, 2021
kittywhiskers referenced this in commit ab71bb599e on Jan 11, 2022
kittywhiskers referenced this in commit 301e416ac8 on Jan 25, 2022
DrahtBot locked this on Feb 15, 2022

Contributors

Labels

refactor: Remove mempool global from net #17997

Conflicts