Refactor mempool.dat to be extensible, and store missing info

luke-jr commented at 6:07 am on December 25, 2016: member

Should fix #9103

fanquake added the label Mempool on Dec 26, 2016

in src/validation.cpp: in 6272d9add1 outdated

4026@@ -4027,7 +4027,7 @@ int VersionBitsTipStateSinceHeight(const Consensus::Params& params, Consensus::D
4027     return VersionBitsStateSinceHeight(chainActive.Tip(), params, pos, versionbitscache);
4028 }
4029 
4030-static const uint64_t MEMPOOL_DUMP_VERSION = 1;
4031+static const uint64_t MEMPOOL_DUMP_VERSION = 2;

sipa commented at 5:46 pm on December 27, 2016:

We never released a version with version 1, I think?

luke-jr commented at 8:41 pm on December 27, 2016:

I’m planning to for Knots 0.13.2

sipa commented at 10:04 pm on January 9, 2017:

Ok. There is hardly any loss…

paveljanik commented at 10:52 am on January 8, 2017: contributor

Needs rebase.

dcousens approved

in src/validation.cpp: in 6272d9add1 outdated

4112@@ -4092,24 +4113,43 @@ bool LoadMempool(void)
4113     return true;
4114 }
4115 
4116+template <class T>
4117+std::vector<unsigned char> SerializeToVector(T o) {
4118+    CDataStream ss(SER_NETWORK, PROTOCOL_VERSION);

sipa commented at 10:13 pm on January 9, 2017:

SER_DISK / CLIENT_VERSION? I doubt it matters for anything, but better be consistent for now.

in src/validation.cpp: in 6272d9add1 outdated

4066+        std::map<std::string, std::vector<unsigned char>> mapData;
4067+        file >> mapData;
4068+
4069+        it = mapData.find("deltas");
4070+        if (it == mapData.end()) {
4071+            try {

sipa commented at 0:04 am on January 10, 2017:

Can’t this try block be made to enclose all deserialization operations, so that the catch and error reporting can be written only once?

luke-jr commented at 5:09 pm on January 12, 2017: member

Fixed serialisation params. There are conflicts now - may I rebase?

luke-jr force-pushed on Jan 12, 2017

luke-jr commented at 8:12 pm on January 12, 2017: member

Rebased.

luke-jr force-pushed on Feb 2, 2017

luke-jr force-pushed on Feb 6, 2017

in src/validation.cpp: in 7ef2e0d8a8 outdated

4191@@ -4192,42 +4192,63 @@ bool LoadMempool(void)
4192         if (version != MEMPOOL_DUMP_VERSION) {

kallewoof commented at 2:49 am on February 10, 2017:

The implications seem to be minor, but this means all bitcoin nodes prior to this PR being merged will drop all their mempools on startup. Is that okay? Would it be possible / worth it to load version=1 mempools too? Above comment by @sipa seems to indicate this was never used, in which case I think we should simply say MEMPOOL_DUMP_VERSION = 1 above.

kallewoof commented at 2:57 am on February 10, 2017: member

utACK b14300f

luke-jr force-pushed on Aug 23, 2017

luke-jr force-pushed on Sep 3, 2017

TheBlueMatt commented at 10:10 pm on November 10, 2017: member

I’m not actually sure we want to save mempoolminfee - if you just restarted the practical mempoolminfee on the network may be much lower….it puts you in a state of downloading lots of transactions only pretty briefly (assuming any of your peers aren’t limiting their mempool, which they almost always are, so mempoolminfee on most nodes never actually leaves 0 anyway). Probably not worth a incompatible version just for this, IMO, though if we had other things we wanted to store we could revisit.

Refactor {Dump,Load}Mempool to be more extensible 80ded01e65

Store mempool min fee state in mempool.dat 1d34442931

sipa commented at 6:15 pm on March 6, 2018: member

I’m not sure this is worth it right now; we can revisit if there is critical information to add to the mempool file?

luke-jr force-pushed on Mar 6, 2018

laanwj commented at 12:35 pm on May 14, 2018: member

Thee seems to be no agreement to do this right now. I’m also going to close #9103, we shouldn’t have that issue open if we don’t agree that this should be done.

laanwj closed this on May 14, 2018

MarcoFalke locked this on Sep 8, 2021

Refactor mempool.dat to be extensible, and store missing info #9422