Audit v0.47 mempool

[amaury1093]

Audit changes mempool added in v0.47

[amaury1093]

My audit is done, have some small nits only. Waiting for 2nd reviewer to share notes.

[amaury1093]

I had audited the sender nonce mempool only, before the priority+nonce mempool got merged. Going to audit the priority one too.

[amaury1093]

I audited the priority+nonce mempool too, it was not straightforward to read the code though, so might have missed some stuff. I did however review the spec and the tests more thoroughly, and those lgtm.

For the second reviewer, here are my notes: https://hackmd.io/_Aj3jciNTRiNVwLxcZ5K2A

[alexanderbez]

I audited the priority+nonce mempool too, it was not straightforward to read the code though, so might have missed some stuff.

This was also my sentiment and hesitation with that implementation -- it's just too difficult to understand. I really hope that implementation is sound and doesn't need to change much in the future, because it's gnarly.

cc @kocubinski

[kocubinski]

This was also my sentiment and hesitation with that implementation -- it's just too difficult to understand. I really hope that implementation is sound and doesn't need to change much in the future, because it's gnarly.

It's not trivial, partial ordering and graph traversal are hard problems. However, this implementation is (as far as I can tell) the simplest one possible satisfying the the spec and ordering scheme described (at a higher level) in ADR-60.

Definitely open to suggestions for improvement. I consider it a win to provide a mempool supporting fee based tx priorities.

[tac0turtle]

since we have a no op mempool by default, im less concerned about the others, but the others have been extensively reviewed