CHIA-3856 Use an adapted version of deficit round robin algorithm in TransactionQueue's pop

[AmineKhaldi]

Purpose:

Converts TransactionQueue's pop from a simple round robin across peers to a deficit round robin.

Current Behavior:

TransactionQueue's pop implements a simple round robin across peers.

New Behavior:

TransactionQueue's pop implements an adapted version of the deficit round robin algorithm.

Testing Notes:

[coveralls-official]

Pull Request Test Coverage Report for Build 20440722378

Details

• 109 of 111 (98.2%) changed or added relevant lines in 3 files are covered.
• 22 unchanged lines in 8 files lost coverage.
• Overall coverage decreased (-0.008%) to 90.817%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
chia/full_node/tx_processing_queue.py	53	55	96.36%

Files with Coverage Reduction	New Missed Lines	%
chia/full_node/full_node_api.py	1	86.75%
chia/wallet/util/wallet_sync_utils.py	1	85.57%
chia/farmer/farmer_api.py	2	95.04%
chia/server/node_discovery.py	2	81.4%
chia/timelord/timelord_launcher.py	2	71.43%
chia/full_node/full_node.py	3	87.08%
chia/server/server.py	4	84.73%
chia/wallet/wallet_node.py	7	86.32%

Totals
Change from base Build 20439288586:	-0.008%
Covered Lines:	102922
Relevant Lines:	113157

---

💛 - Coveralls

[arvidn]

My understanding of your implementation is that it:

1. scans for a transaction whose cost >= the peer's deficit counter
   1.2 If found, ingest transaction and decrement the deficit counter by the cost
2. If not found, increment the deficit counter of all peers by the lowest cost transaction, and goto 1

I don't think you need the cursor for fairness anymore, you'll get fairness anyway, since you track the deficit counters.

I think it could be made a bit more efficient by reducing the linear scan into a heap pop. It may introduce some more complexity though. This is not a complete thought. But imagine if every peer had a sort "key" which was its deficit_counter - cost_of_top_tx. You could have a priority queue (or really, a heap would suffice) of those peers. Every time a deficit counter is adjusted, the peer is resorted. You always pop from the top (which is O(1)). You'd need to figure out when to increment the deficit counters and by how much. So maybe this wouldn't be so much simpler.

[arvidn]

isn't _queue_dict also a map of the same peer IDs?
It would seem cheaper and simpler to stick this int in that dict instead. Am I missing something?

[arvidn]

I think this deserves a comment explaining the idea behind this behavior. that we want to service transactions fairly between peers, based on cost.

[arvidn]

I don't think you need the _list_cursor anymore, now that you look at every peer every time you pop. There's a risk it might not be very efficient though.
It might be OK for a first version. A heap might be a more efficient choice

[arvidn]

I don't think you need _index_to_peer_map anymore, just like you don''t need _list_cursor anymore. You visit every peer every time you pop(), so there's no need to keep any cursor position in between calls to pop().

[arvidn]

I think this case warrants a comment. This is where we don't know the cost (or fee) for a transaction, so we want to assume a very high cost. I don't think we should search peer_with_tx, we should just assume it has a really high cost. This is just for backwards compatibility, right?

Suggested change

	if tx_info is None:
	top_tx_advertised_cost = max(
	(t.advertised_cost for t in entry.peers_with_tx.values()), default=self._max_tx_clvm_cost
	)
	else:
	top_tx_advertised_cost = tx_info.advertised_cost
	top_tx_advertised_cost = self._max_tx_clvm_cost if tx_info is None else tx_info.advertised_cost