feature: mempool v1 implementation

[alexanderbez]

Implementation of ADR-067 -- mempool v1.

Note, to aid in review, I've decided to split up this work into two PRs:

1. This PR, which handles the foundation of moving parts around to support versioned mempool reactors. Note, there are no actual protocol or software changes in this PR -- just moving things around.
2. The mempool v1 reactor implementation. This PR (Mempool V1 Implementation #6468) contains the actual changes outlined in ADR-067.

[codecov]

Codecov Report

Merging #6466 (5de3dce) into master (b4307ca) will decrease coverage by 0.39%.
The diff coverage is 55.37%.

@@            Coverage Diff             @@
##           master    #6466      +/-   ##
==========================================
- Coverage   60.93%   60.54%   -0.40%     
==========================================
  Files         292      298       +6     
  Lines       27322    27943     +621     
==========================================
+ Hits        16650    16919     +269     
- Misses       8976     9301     +325     
- Partials     1696     1723      +27     

Impacted Files	Coverage Δ
config/toml.go	59.09% <ø> (ø)
consensus/replay_stubs.go	57.57% <0.00%> (ø)
mempool/errors.go	0.00% <0.00%> (-25.00%)	⬇️
mempool/mempool.go	0.00% <0.00%> (-81.25%)	⬇️
mempool/metrics.go	0.00% <0.00%> (-15.16%)	⬇️
mempool/v1/reactor.go	0.00% <0.00%> (ø)
rpc/core/mempool.go	0.00% <0.00%> (ø)
mempool/ids.go	84.61% <33.33%> (-7.70%)	⬇️
mempool/tx.go	50.00% <50.00%> (ø)
mempool/cache.go	51.42% <51.42%> (ø)
... and 32 more

[tac0turtle]

Utack

Need a change log entry for the changes

[cmwaters]

LGTM 👍

[cmwaters]

Don't we want the default to be MempoolV0 at least until V1 is complete else the node is just going to panic for anything that uses the default config

[alexanderbez]

No, we want v1 to be default. Once I wrap up the 2nd PR, nothing will panic or fail.

[cmwaters]

Ok. So do you plan to merge the PR's together before merging on master?

[alexanderbez]

Yes, exactly :)

[alexanderbez]

Need a change log entry for the changes

That will come in the 2nd PR.

[tychoish]

This seems fine, though I wonder if it's necessary:

• the new mempool, passed with default (zero values) for the new chectx should have the same behavior and semantics as the legacy mempool.
• adding a configuration option may be good as a back door, if there are any unforeseen problems with the new code at scale, but I think long term (certainly, by 1.0) it seems like it would be better to just have the one implementation (testing overhead, maintenance/support burden)

[alexanderbez]

@tychoish,

the new mempool, passed with default (zero values) for the new chectx should have the same behavior and semantics as the legacy mempool.

I'm sorry, but I don't follow this...CheckTx will perform the same logic. Wdym by passing values?

adding a configuration option may be good as a back door, if there are any unforeseen problems with the new code at scale, but I think long term (certainly, by 1.0) it seems like it would be better to just have the one implementation (testing overhead, maintenance/support burden)

It depends. There could be multiple mempools we maintain (as @ValarDragon pointed out) and various operator types may want to use different ones, e.g. validators use X and and sentries/full-nodes use Y, etc...That being said, I see no reason why we'd maintain v0 after 0.35 or after.

[tychoish]

I'm sorry, but I don't follow this...CheckTx will perform the same logic. Wdym by passing values?

My assertion/understanding is that using the new mempool with an old application (e.g. one that passes 0 priority for everything will have the same semantics, and be isomorphic to the legacy mempool. If this is true, is there any reason to let the mempool be configurable?

Similarly, particularly since we're not likely to maintain the legacy implementation in the long term, is providing configuration in the short term going to be valuable, and not just create more operational pain when we remove it/etc. Though it's definitely true that I don't 100% understand the operational case for using the legacy mempool in some cases?

[alexanderbez]

My assertion/understanding is that using the new mempool with an old application (e.g. one that passes 0 priority for everything will have the same semantics, and be isomorphic to the legacy mempool. If this is true, is there any reason to let the mempool be configurable?

Gossiping remains the same, however, block inclusion/construction will not be the same.