Don't resend tx hashes that were sent to us

[LukaszRozmej]

Changes

• Move TxPoolPeer into base peer functionality
• When we get sent transaction or hash don't re-send it to the peer that send it in the first place
• don't allocate unnecessary arrays on block & receipts sync

Types of changes

What types of changes does your code introduce?

• Bugfix (a non-breaking change that fixes an issue)
• New feature (a non-breaking change that adds functionality)
• Breaking change (a change that causes existing functionality not to work as expected)
• Optimization
• Refactoring
• Documentation update
• Build-related changes
• Other: Description

Testing

Requires testing

• Yes
• No

If yes, did you write tests?

• Yes
• No

[LukaszRozmej]

Managed to get Sent hashes down fro average 104k->62k.

Post-sync darkpool looks fine:

[marcindsobczak]

In PR description:

When we get sent transaction or hash don't re-send it to the peer that send it in the first place

1. full transactions are sent anyway: https://github.com/NethermindEth/nethermind/blob/perf/tx-pool-peer/src/Nethermind/Nethermind.Network/P2P/ProtocolHandlers/SyncPeerProtocolHandlerBase.cs#L198
   and it is fine - we want to broadcast local txs as fast and as wide as possible
2. PR name Don't resend tx hashes that were sent to us - we already have protection from sending hashes to the peer which sent this tx to us:
   https://github.com/NethermindEth/nethermind/blob/master/src/Nethermind/Nethermind.TxPool/TxBroadcaster.cs#L224
   https://github.com/NethermindEth/nethermind/blob/master/src/Nethermind/Nethermind.TxPool/TxBroadcaster.cs#L239-L242

I'm not sure why you achieved such reduction in number of hashes sent, for me it looks like we are doing the same as before, just adding to the cache tx hashes which will not be sent anyway - we have logic in TxBroadcaster which is preparing messages exclusively for every peer to protect from sending back hash to the original sender of tx.

[LukaszRozmej]

Good question about DeliveredBy, still I think this abstraction in Nethermind.TxPool/PeerInfo.cs was not useful and this is better.

[marcindsobczak]

Ah I got it, we were not sending back hashes of txs added to the pool, now we are not sending back hashes received and not pooled too. At this point I think we can consider simplifying TxBroadcaster - remove logic of skipping tx by sender and prepare only one HashesMessage for all peers, and then not-resending will be handled by peer cache

[marcindsobczak]

I mean we can drop this https://github.com/NethermindEth/nethermind/blob/master/src/Nethermind/Nethermind.TxPool/TxBroadcaster.cs#L234-L244
and just pass all _txsToSend to all peers instead of iterating it for every peer
https://github.com/NethermindEth/nethermind/blob/master/src/Nethermind/Nethermind.TxPool/TxBroadcaster.cs#L224
Notify(peer, _txsToSend, false);

Also I think we should add

foreach (Keccak hash in msg.Hashes)
{
    NotifiedTransactions.Set(hash);
}

to Eth68ProtocolHandler as well, same as to Eth65ProtocolHandler
https://github.com/NethermindEth/nethermind/blob/master/src/Nethermind/Nethermind.Network/P2P/Subprotocols/Eth/V68/Eth68ProtocolHandler.cs#L67-L93

[marcindsobczak]

Ah I got it, we were not sending back hashes of txs added to the pool, now we are not sending back hashes received and not pooled too

Actually it is wrong, we were not sending not pooled hashes anyway, hashes are added to broadcaster only after adding txs to the pool. But we could received tx hash from many peers and full tx from only one. Before we were announcing that hash to every peer except of one, now we are announcing only to the ones which didn't sent that hash to us. It explains why number of sent hashes dropped.

Refactoring TxBroadcaster to not iterate _txsToSend 100x every second and caching hashes just after receiving them seems good. We can only consider increasing cache - it is only 2048 and one single NewPooledTransactionsHashes message might be bigger. But it will increase memory usage significantly, as we have dedicated cache per every peer

[marcindsobczak]

• HIVE eth protocol tests 🟢 https://github.com/NethermindEth/nethermind/actions/runs/4477015254/jobs/7868150899 (failing same as on master)
• POSDAO pre-merge 🟢 https://github.com/NethermindEth/nethermind/actions/runs/4477027838/jobs/7868048444
• POSDAO post-merge 🟢 https://github.com/NethermindEth/nethermind/actions/runs/4477026140/jobs/7868044380