We'll need to add mempool.dat to doc/files.md

Why not behind a command line flag?

Concept ACK.

Why not behind a command line flag?

Agreed. It can be enabled by default.
But people may want to disable this functionality, the per-10 minute dump could interfere with benchmarks and such for example.

Concept ACK

000000e0  7a 8e 6e 84 83 ab c4 ca  20 fb 9b 88 ac 00 00 00  |z.n..... .......|
000000f0  00 00 00 00 00 0f 6a 0d  48 61 6c 6c 6f 55 62 69  |......j.HalloUbi|
00000100  72 63 68 74 21 00 00 00  00 0d 9a a1 57 00 00 00  |rcht!.......W...|

As we dump raw txes from the mempool, this will conflict with current status of "antivirus-compliant" storage and dumping mempool.dat can stop the bitcoind from running, ie. new DoS vector.

2016-08-03 07:29:00 Dumped mempool: 1.9e-05s to copy, 0.005925s to dump

As we dump raw txes from the mempool, this will conflict with current status of "antivirus-compliant" storage and dumping mempool.dat can stop the bitcoind from running, ie. new DoS vector.

A fair point. Though a missing, or corrupted mempool.dat should never cause bitcoind to stop running. At most this will lose the mempool contents (at next start) due to the file being quarantined.

More XOR?

Indeed this probably should be XOR'd with a random key.

At most this will lose the mempool contents due to the file being quarantined.

AV can be configured to stop bitcoind with a dialog: “This app is writing a virus to your disk. OK | Error”. This is outside of bitcoind…

Concept ACK and the code size is surprisingly small!

The only thing that I miss (not directly related to this PR) is an opportunity to keep a history of some major mempool data (feerate, size, tx rate, etc.). Another option would be keeping ~24*6 dumps per day that would could be stored in file-diff-deltas to allow generating any types of statistics.

I don't really see a huge benefit in doing this. It seems like unless you are immediately restarting, you'll be wasting a lot of time trying to load already confirmed transactions or low fee spam. That said I don't object to making it a command line argument, but I would suggest it default to off.

What's the rationale for running it every 10 minutes? You can copy it from one running node to start another or to handle crashes?

I think it should be off by default. And when it is, we can even skip XORing it, I think as it is expert level feature anyway.

Or the other way: create a RPC to dump mempool and that's all. But this way we loose the save-on-restart feature which is nice (which can be worked around by dumpmempool and restart).

@morcos The reason for dumping every 10 minutes is so that prioritization data is not lost in case of a crash.

But perhaps we should have separate files for that. prioritization.dat which is continuously dumped (maybe only after mapDeltas changed?), and mempool.dat which is only saved at shutdown?

wasting a lot of time trying to load already confirmed transactions or low fee spam

Could it help to save the current block height to the mempool.dat and then discard mempool.dat on start when current height > 3 + height when mempool was saved?

This has split into two threads, really:

1. mempool persistence across restarts
2. persisting prio deltas across restarts & crashes
   2.1) side note - other state such as wallet locked coins (lockunspent) should be persisted also

Persisting bits of state like prio deltas is useful, and seems applicable to a continuous-log or database solution, that can be restored at startup.

As for mempool restore...

Standard rhetorical questions: Who will really use mempool restore, and how often?

• Historically, users really do not like features that pause shutdown for a long time. We spent time optimizing bitcoind shutdown in the past.
• Dump on shutdown does not provide crash protection (as @sipa noted)
• Alternatives to dump-on-shutdown involve a lot of disk traffic (logging or db soln) at runtime, and introduce additional I/O stress points for remote attackers to exploit.
• Therefore, the only use case for dump-on-shutdown is a rapid node restart -- if restart is not rapid, much of the data will be stale, only reloading slow-confirming transactions uninteresting to miners.
• Someone restarting their node tends to be an expert-only or dev-only operation; normal field operation - unattended operation or standard user session - will never see this feature.

mempool restore seems likely to be a little used, expert-only feature.

mempool restore seems likely to be a little used, expert-only feature.

Somewhat agreed, and as a regular 'power' user of bitcoind, I personally already have my own RPC calls equivalent to dumpmempool for this.

@jgarzik This is functionality directly requested by miners, the absence of it creates an operational burden because nodes then end up making small blocks after being restarted.

persisting prio deltas across restarts & crashes

This isn't useful without the transactions themselves-- peers will not re-relay transactions they previously sent. So you could save priorities, but all of them would be almost completely useless.

users really do not like features that pause shutdown for a long time.

Indeed, if it takes a long time that is an issue. But this should be writing no more than about 150 MB of data, which should take under a second. If it's taking a long time then that needs to be addressed.

One solution if that is an issue is to simply not write it at shutdown, but to only use the regular snapshots.

and introduce additional I/O stress points for remote attackers to exploit

I don't follow how remote attackers interact here; it wouldn't make sense to have any of this behavior be remotely triggerable.

if restart is not rapid, much of the data will be stale,

It takes several hours for a full mempool to lose its utility.

I suppose it would be reasonable to not even bother trying to load it if it's more than three days old.

Mempool sync is also a bandwidth concern for future mempool sync efforts-- if it's lost on restart, than every quick restart would waste bandwidth.

Similarly for block transmission-- latency and bandwidth usage will be adversely effected if mempool is lost on restart. It takes about 12 hours in my tests for a node to recover its compact block reconstruction performance after restart, and it can take several seconds to accept a block due to the empty signature cache.

These pressures against restart are an operational burden, because then you're pressured to try to find ways to avoid restarting-- e.g. running extra nodes, or avoiding changing settings.

Separately, the cold mempool means that users and developers spend time in a non-representative state after they try changing settings; which makes it much harder to iterate around settings changes.

( Sorry if some of these points are redundant with #8433 )

Concept ACK, and a very useful feature!

Though I'd suggest making it off by default for now so that nodes that don't need this don't have the risk of a bug creating a restart loop - less risk if we start in a clean state by default.

@petertodd With the exception of the priority deltas the transactions are going through normal mempool accept logic.

I can't see this causing a restart loop unless there's a bug in the priority handling logic or a remotely exploitable DoS issue anyways.

I can't see this causing a restart loop unless there's a bug in the priority handling logic or a remotely exploitable DoS issue anyways.

Which may only be trigger-able under particular circumstance, of which may be captured in the current mempool state?

@pstratem Hmm, that's a good point. Also, as long as we're only saving the mempool in a normal, user-triggered, shutdown all transactions in the saved mempool should be fine.

I'll take back my objection.

It seems like unless you are immediately restarting, you'll be wasting a lot of time trying to load already confirmed transactions or low fee spam.

Eloipool does a save/restore state when it restarts, and one thing it does is write a timestamp early on in the dump. This way, when loading from the dump, it can see if the data is irrelevant and ignore the file. Perhaps that would make sense here too?

Any reason to write out the entire thing every interval, rather than just having another LevelDB database?

@gmaxwell Those concerns seem to be met by remote mempool download also, which @morcos suggested elsewhere in the thread -- and indeed was one original purpose of BIP 35 mempool message -- maintaining miner's mempools and not losing lucrative miner fees due to restart.

Remote download over LAN is also potentially faster, something Google noticed when seeing increased performance through remote page caches: http://static.googleusercontent.com/media/research.google.com/en//pubs/archive/43886.pdf

Remote download also gets you fresh transactions which the db/log will not have - fuller blocks, more miners fees.

and introduce additional I/O stress points for remote attackers to exploit

I don't follow how remote attackers interact here; it wouldn't make sense to have any of this
behavior be remotely triggerable.

The relevant portion that answers your confusion was elided: "Alternatives to dump-on-shutdown [...]" This refers to, e.g. periodic dumps, streaming transactions to a leveldb or WAL log, which occur at runtime not shutdown.

Those concerns seem to be met by remote mempool download also, which @morcos suggested elsewhere in the thread -- and indeed was one original purpose of BIP 35 mempool message -- maintaining miner's mempools and not losing lucrative miner fees due to restart.

Preservation of the mempool is a prerequisite for mempool sync. Otherwise you'd have 150 MB of transfer (burning resources of remote nodes) each time someone restarts to twiddle some configure. I think the mempool message is unsuitable for synchronization generally, due to overhead.

[and while the latency concerns related to compact blocks would be addressed by a bulk download, the bandwidth savings would be lost]

Remote download over LAN is also potentially faster, something Google

In Google datacenters not on our P2P network; in any case I believe the load is bottlenecked on any ordinary host. (and were it faster, it wouldn't matter-- resource usage needs to be heavily weighed).

and introduce additional I/O stress points for remote attackers to exploit

I don't follow how remote attackers interact here; it wouldn't make sense to have any of this behavior be remotely triggerable.

The relevant portion that answers your confusion was elided: "Alternatives to dump-on-shutdown [...]" This refers to, e.g. periodic dumps, streaming transactions to a leveldb or WAL log, which occur at runtime not shutdown.

There isn't any leveldb involved here; but regardless-- I still don't see where the "remote attackers" part that I was asking about comes in.

Notable how remote mempool load on startup could be increase the effects of a DoS attack due to all the extra bandwidth wasted as nodes restart after crashing; it'd be especially harmful if a remote crash exploit could be found if people ever setup nodes to restart automatically.

On 3 August 2016 21:56:34 GMT-07:00, Gregory Maxwell notifications@github.com wrote:

Those concerns seem to be met by remote mempool download also, which
@morcos suggested elsewhere in the thread -- and indeed was one
original purpose of BIP 35 mempool message -- maintaining miner's
mempools and not losing lucrative miner fees due to restart.

Preservation of the mempool is a prerequisite for mempool sync.
Otherwise you'd have 150 MB of transfer (burning resources of remote
nodes) each time someone restarts to twiddle some configure. I think
the mempool message is unsuitable for synchronization generally, due to
overhead.

Remote download over LAN is also potentially faster, something Google

In Google datacenters not on our P2P network; in any case I believe the
load is bottlenecked on any ordinary host. (and were it faster, it
wouldn't matter-- resource usage needs to be heavily weighed).

and introduce additional I/O stress points for remote attackers to
exploit

I don't follow how remote attackers interact here; it wouldn't make
sense to have any of this behavior be remotely triggerable.

The relevant portion that answers your confusion was elided:
"Alternatives to dump-on-shutdown [...]" This refers to, e.g. periodic
dumps, streaming transactions to a leveldb or WAL log, which occur at
runtime not shutdown.

There isn't any leveldb involved here; but regardless-- I still don't
see where the "remote attackers" part that I was asking about comes in.

---

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#8448 (comment)

• Make it off by default: seems reasonable; not every node has a requirement to have a well-calibrated mempool immediately at startup. It does help for BIP152 block relay, but many nodes also likely won't restart sufficiently fast to benefit.
• XORing a key: sigh, ok
• Checking how old the mempool is: height won't work, as at the time you restart, you won't have learned about blocks that have been found in the mean time. Using time would work, but meh... loading is fast and in the background
• Using BIP35 instead: inefficient and unpredictable for resources (you don't know the size of a remote mempool, and can't tell them to not send 20 gigabytes worth of txids), and won't work for prioritized transactions. I do think we should explore network-based mempool syncing independently, though.
• Separating dumping priority data and mempool data: the problem is that for prioritized transactions, we need both, and there is no guaranteed way to load these from anywhere else. What about periodically dumping but only for prioritized transactions, and dumping the full mempool at shutdown (when configured).
• Keeping historical record: much more complicated and can better be done in other ways (separate statistics module that persists to disk).

Rebased with the following changes:

• Several nits addressed (missing lock, add version constant, no big screamy errors on failure).
• Periodic dump is gone; it only happens at shutdown now.
• Expired entries are skipped on load (making a load of a week-old mempool.dat instantaneous).

XORed dump is not implemented, this can be done later. As the regular dump seemed controversial, I've removed it now (it can always be added back later).

ACK 582068a

XOR dumping nit can be ignored now, because we are dumping mempool on shutdown only.

@sipa sorry for commenting on this late, but could we have made it a bit more efficient by storing mapDeltas before the transactions, so then we don't have to do 2 loops of prioritiseTransaction? not sure if its worth it to change now....

@morcos This is not in released version, so I think if there is a benefit, we can still change it. And of course, we can safely ignore this file every time...