ids -> expectedOrder to make it clear that this is being checked against

This block should loop over some number of orderings that include already-in-order and out-of-order txs for vtx.

agreed. no need to include it until it's needed.

CheckDisconnectPoolOrder would be more apt.

I guess the first check should be that both size do match as you risk out of bound access here and this can pass if the disconnectPool just drops everything.

Please define things at point of use.

Just use int when bit manipulation are not needed and range is sufficient.

Making this const is probably a good idea. It'll avoid mistakes. Defining named constants for 5 and 6 would make the code easier to read and follow and also easier to update in the future if one want to change them.

dito

I'm pretty sure you can instantiate the vector in place so the vector gets destroyed and memory freed immediately after use.

You probably want to do the check within CheckDisconnectPoolSort . When the codes forces you to repeat two things together, and if you forget one it's incorrect, then it's a hint that they are actually not separate things. Keeping them separate will inevitably lead to complexity and errors.

I understand the id here correspond to the if of the transaction ? If so, then it's be clearer to just get it from there. If not, then maybe creating some struct that describe the elements of a test case would make the test cases more readable/easily followed.

Create the disconnectPool in the loop body so you don't need to do this kind of stuff.

Declare at point of use. This makes things easier to follow. More generally, this is a wall of code. Adding a few empty lines that separate the different actions would make it more readable without changing much. for instance:

• Copy the mempool content into the vector.
• Clear the mempool
• Create and set the ordered_mempool (bad name BTW, as it is not a mempool).
• Use ordered_mempool to splice everything where it needs to be (BTW i had no idea one could do this on multimap, this is brilliant).
• Clear the vector ?
• Free memory in the pool if necessary.

You have all the components, it just a matter of shuffling bits around in logical blocks.

I there a way to use std::move here ? We'd avoid incrementd', all the ref count to then decrement them all on clear.

You can keep it if you want, but you don't need a local here

Put that with userland includes. This is not a system library.

I'm not sure why you need this. You just checked it before. If you want to make sure the program stops, you can use BOOST_REQUIRE_EQUAL

constant are CAPITAL_SNAKE_CASE . I'm also pretty sure this is not about how long the test is.

There is essentially no way for me to look at these test cases and ensure they are correct in any reliable way.

Test cases usually comes in the form of inputs => expect results. This is just a bunch of numbers.

I don't think you need to call the constructor explicitly.

Remove

Remove

Put the comment on one line and let clang-format handle them. Put dot at the end of sentence.

This is clearly another paragraph from what comes before. Paragraph are separated by an empty line.

So this is returning by value anyways, so std::move does nothing.

It doesn't looks like the pool is used after this, so holding the lock on the pool do not seems necessary.

This is using the pool and you moved the lock just after.