Self-note: does pwalletMain->IsMine() require a lock? I'm thinking yes.

Why not use the term gc (garbage collection)?

Nice, thanks.

Good idea, ACK on concept.

What has changed regarding bitcoin that is making this feature more necessary now where it wasn't so necessary in the past?

@rebroad : we want to lower the relay fee, to give miners more opportunity to mine lower-fee transactions. But we don't want attackers to be able to gum up memory with lots of spammy, will-never-get-mined transactions...

(and this is where Mike Hearn jumps in and points out, again, that we're doing it wrong and we should relay all transactions until we hit per-peer memory or bandwidth limits... and he is correct, but that is a lot more work).

@Diapolo The name garbage collection is way, way too general. This cleans up the transactions in the pool, so pool manager is a good name.
ACK on concept

Pool boy.

I believe the current implementation will remove unconfirmed dependencies of wallet transactions, transitively removing wallet transactions too.

I'd prefer a flag bit in the mempool to mark transactions as precious, and when applied, it automatically propagates up to its dependencies.

I also don't really like mempool (management) code needing to know about all connected wallets, and acquire locks on them. The mempool should function independently.

Can't believe I registered for this. I'm with gmaxwell, "pool boy" is more appropriate.

Why a separate thread? It could just run after receiving a new tx, I think? The more threads there are, the easier it is to slip up and slice your thumb off ...

This operation is too slow and expensive to run after every new TX.

The first step is to check if it's time to run, i.e. if now - lastTime > 24hrs. So I don't see what the issue is.

@mikehearn Ah, I thought you were suggesting to run the operation after receipt of every TX.

Yes, you could do it that way too.

Rebased

Please explain the merge conflict.

Rebased. The following feedback from @laanwj @sipa must be addressed before merge,
"I'd rather have that the wallets let the mempool know that a transaction is precious (with a flag), than having the mempool manager poll the main wallet.

If you want to do it this way (poolman -> wallets) please use the RegisterWallet / UnregisterWallet machinery from main.cpp, and add a CWalletInterface method for this."

@jgarzik Thanks for your work on this!

I just want to leave this as a comment rather than forking, pull requesting or any such-ness atm. I wanted to suggest that when this goes through to final at such point when rebase please consider the original remark I had some while back which was, I think, to rename from poolman.h to poolbeing.h, (see line 99 at src/Makefile.am for example).
Similarly poolman.cpp would become poolbeing.cpp and so forth. Not clear if there was some other consensus for "manager" as per prior discussions. Tired.
Thank you if possible. Please e-mail me with any questions.

What is a Poolbeing? Politically-correct nonsense which doesn't belong in bitcoin core is what.

I vote Poolbot or Poolcleaner if you're so upset by the ending '-man' in the English language.

Hi, my request for poolbeing was alluding for something beyond the standard anthropomorphic reference, and yes, that which ended in man. Not that I was upset by it, because I'm not, but I think that in light of the progress of technology today (consider for example DAOs) it makes more sense to have poolbeing. In light of your question, "what is a poolbeing," I would ask to you in return, "What is a poolman?" And there is nothing, because there is no poolman. The poolman is not there. We knock at the door, and he is not home.

'In Search of The Eternal Poolbeing'

"Because of the one-pointed Time awareness in which the conventional mind remains immersed, humans tend to think of everything in a sequential, word-oriented framework. This mental trap produces very short-term concepts of effectiveness and consequences, a condition of constant, unplanned response to crises."

• Liet-Kynes
  The Arrakis Workbook
  (as quoted from Children of Dune, by Frank Herbert)

The command "invalidateblock" is handy for testing with one node, but I was looking for mempool manipulation, and this was the closest thing I've found. So for what it's worth, since it's slightly related, I added a command to clear the whole mempool via the RPC interface:

• dexX7/bitcoin@e687023...dexX7:oc-0.10-rpc-clearmempool

@ABISprotocol poolman is short for pool manager, manager is from the latin manus for hand, and has nothing to do with gender

How to move forward here? (and I don't mean in regard to silly word games)

Is expiration of mempool transactions on a fixed known schedule really the best solution?

@laanwj The merge blocker with this pull request is a @sipa comment from long ago: The janitor must not sweep transactions which are "precious" to the local node. Generally this means wallet transactions.

This is not a blocker for disabled-wallet configurations; thus two paths forward are,

1. Disable poolman for wallet nodes.
2. Do The Right Thing, and create an interface whereby the wallet module tells mempool (or poolman) which transactions are precious and should not be automatically culled. (cc @jonasschnelli)

@pstratem No. It is however a rough approximation of a solution that can be deployed today, modulo comments just made.

The consensus on solution is a "superblock" type model, where

• Mempool is modelled after a X megabyte sized block (X = 24 hours worth of data, according to proposal)
• Transactions entering system are judged based on fee/priority, and insertion-sorted accordingly
• One or more transactions, perhaps include the latest transaction itself, if total mempool size exceeds X

This real-time priority based culling is preferable to periodic garbage collection; however the "ideal solution" does not yet have any code or testing under its belt.

poolman is an imperfect stopgap solution until The Ideal Solution arrives.

For the wallet enabled mode, i think, if there would be a signal over CValidationInterface (something like CheckJanitor(const CTransaction& tx, bool& clean) the wallet could allow/disallow "precious" transactions to be preserved/not affected by the janitor.
I will rebase this PR and add the additional wallet/validation-interface signaling (during the next couple of days).
IMO this PR (or lets say something that clean the mempool) is long overdue.

@jgarzik well clearly this not a huge priority for merging (it's been sitting around for over a year now)

modeling the mempool as a superblock seems reasonable, but is maybe too expensive to actually run everytime a new transaction is included.

what do you think about simply removing the transaction (and it's dependents) with the lowest priority?

@pstratem removing transaction(s) and dependents with lowest priority is not a good idea because low-priority dust (and/or transactions which exceed dust threshold but which are of lower priorities) should not be rejected due to the importance of inclusion of types of transactions to support a wide range of participants' needs, income ranges, and re-envisioning of dust as microgiving capacity or opportunity ( see my comments in #6402 - #1536 - #6201 ).

Closing out older pull requests.

This is not a judgement on this pull request or time-based expiration. Time-based mempool expiration continues to be the best way to sample what miners are not confirming.

There is potential for resubmitting or reopening this in the future.