refactor: Broadcast tx insert to p2p

[bvrooman]

• Removes the insert_with_broadcast method
• Replaces call to insert_with_broadcast with sequential insert followed by broadcast for each inserted tx

This approach reduces some code complexity at the cost of second iteration over the transactions. See the discussion in the parent PR here #590 (comment).

[ControlCplusControlV]

I actually would prefer to hold off on this since the txpool is particularly performance sensitive and the double iteration over tx's. It might also be useful later to separate insert from insert_from_broadcast for when more complex p2p tx broadcasting is introduced (such as broadcasting beyond a peer-depth of 1) but I understand if readability is more important at this time

[Voxelot]

I actually would prefer to hold off on this since the txpool is particularly performance sensitive and the double iteration over tx's. It might also be useful later to separate insert from insert_from_broadcast for when more complex p2p tx broadcasting is introduced (such as broadcasting beyond a peer-depth of 1) but I understand if readability is more important at this time

We should generally try to avoid code duplication, as it makes future refactors more difficult - there might not be any immediate errors or tests that fail if the shared behavior changes in one and not the other. If we really want to optimize out an extra for-loop, there are other ways to do that without maintaining duplicate code.

[bvrooman]

You could argue that the path is still O(2N) = O(N). Before, we did N iterations of 2 operations; now, we do 2N iterations of 1 operation each. (This is, of course, a simplification of the actual case). I would bet that the performance hit would be fairly negligible in this context. I think the increased maintainability and simplicity outweigh the potential performance benefit.