Is it good / wise or just unneded to enable the -checkmempool with a normal node? Do we work towards enabling this as default in the future? Just want to get some insight on it :).

Looks mostly OK. One issue: the locking for the mempool lookup inside ProcessMessage() seems to have changed.

I like the idea of moving this out, but this doesn't really encapsulate it cleanly. A suggestion to obtain that:

• CTxMempool should just be a data structure with associated logic to remain consistent, and not contain part of the validation logic. So, CTxMempool::accept should remain in main (as a function, not a CTxMempool method).
  • That means no forward declaration of CValidationState in txmempool.h.
  • Also no need to move the EraseFromWallets callback from main to txmempool (that certainly doesn't belong there).
  • The mempool object should remain inside main, and not move to txmempool (main just becomes a client that uses one instance of it).
• txmempool.h/.cpp can just include core.h then, which means no circular dependencies at all.

@sipa +1

@sipa : good idea.

@sipa Agreed.

FWIW I wound up implementing a CTxMempool style thing myself when I was looking at doing a child-pays-for-parent mempool, so I think that's the right general direction to go in.

Rebased and updated as per @sipa's suggestions.

@jgarzik: locking has changed, but should be safer than before because there are many fewer cases of "reach inside and LOCK(mempool.cs)". I'd like to make the mempool critical section private, but I think that should be done in a future refactor (we will probably need a "give me a snapshot copy of the memory pool"; but that should wait until after implementing a memory-limited mempool, I think).

ACK design and implementation. Haven't tested, and haven't checked that the moves are really move-only.

An alternative and perhaps cleaner solution to passing the LookupFromTip pointer:

• Move CCoinsView, CCoinsViewBacked and CCoinsViewCache to core
• Pass a CCoinsViewCache object (pcoinsTip) to CTxMempool::check (which the caller knows the corresponding lock is held for).
• CCoinsViewMempool can then move to txmempool as well

ACK

I checked that the moves were all really move-only.

Rebased, and renamed AcceptToMempool to AcceptToMemoryPool for consistency.

@sipa: Moving CCoinsView/etc to core is non-trivial; CBlockIndex (at least) would have to move also. Lets save that for a future even-more-perfect refactor.

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/319b11607f8592d7ef67ec82fa73545ad7430974 for binaries and test log.
This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
Contact BlueMatt on freenode if something looks broken.