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

pull luke-jr wants to merge 2 commits into bitcoin:master from luke-jr:mempool_dat_extensible changing 2 files +118 −43

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:None 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:None 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:None 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 12: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:None 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

Contributors

Labels

Linked (view graph)

#9103 mempool saving doesn't save mempoolminfee #19488 Refactor mempool.dat to be extensible, and store missing info