Consensus optimization during transaction request

[shargon]

When reviewing #1040 an issue was discovered and a possible optimization.

The first thing we can see is in the following lines

neo/neo/Consensus/ConsensusService.cs

Lines 465 to 468 in 919cc80

	taskManager.Tell(new TaskManager.RestartTasks
	{
	Payload = InvPayload.Create(InventoryType.TX, hashes)
	});

We are creating an inventory of the pending transaction hashes, but calling the wrong method, we should call

neo/neo/Network/P2P/Payloads/InvPayload.cs

Lines 27 to 35 in 919cc80

	public static IEnumerable<InvPayload> CreateGroup(InventoryType type, UInt256[] hashes)
	{
	for (int i = 0; i < hashes.Length; i += MaxHashesCount)
	yield return new InvPayload
	{
	Type = type,
	Hashes = hashes.Skip(i).Take(MaxHashesCount).ToArray()
	};
	}

Because it control the limits by MaxHashesCount and the other doesn't.

This is the first issue, the next one is an improve.

As you know, in P2P was added a compression mechanism (#710), compression is ver useful for improve the TPS, but is better on bigger messages than smaller. So if a node is asking about 500 TX, why we should send it one by one? We should pack all of this tx, and send in a bulk message. Is more optimal and can save resources.

neo/neo/Network/P2P/ProtocolHandler.cs

Lines 180 to 211 in 919cc80

	UInt256[] hashes = payload.Hashes.Where(p => sentHashes.Add(p)).ToArray();
	foreach (UInt256 hash in hashes)
	{
	switch (payload.Type)
	{
	case InventoryType.TX:
	Transaction tx = Blockchain.Singleton.GetTransaction(hash);
	if (tx != null)
	Context.Parent.Tell(Message.Create(MessageCommand.Transaction, tx));
	break;
	case InventoryType.Block:
	Block block = Blockchain.Singleton.GetBlock(hash);
	if (block != null)
	{
	if (bloom_filter == null)
	{
	Context.Parent.Tell(Message.Create(MessageCommand.Block, block));
	}
	else
	{
	BitArray flags = new BitArray(block.Transactions.Select(p => bloom_filter.Test(p)).ToArray());
	Context.Parent.Tell(Message.Create(MessageCommand.MerkleBlock, MerkleBlockPayload.Create(block, flags)));
	}
	}
	break;
	case InventoryType.Consensus:
	if (Blockchain.Singleton.ConsensusRelayCache.TryGet(hash, out IInventory inventoryConsensus))
	Context.Parent.Tell(Message.Create(MessageCommand.Consensus, inventoryConsensus));
	break;
	}
	}
	}

[shargon]

Please take a look @eryeer and @Tommo-L

[Tommo-L]

Good idea, we have seen

[eryeer]

When reviewing #1040 an issue was discovered and a possible optimization.

The first thing we can see is in the following lines

neo/neo/Consensus/ConsensusService.cs

Lines 465 to 468 in 919cc80

	taskManager.Tell(new TaskManager.RestartTasks
	{
	Payload = InvPayload.Create(InventoryType.TX, hashes)
	});

Thanks for your great idea, but I found the function OnRestartTasks in TaskManager recreate the payload into group, and then broadcast the payload.

neo/neo/Network/P2P/TaskManager.cs

Lines 123 to 124 in 1a39203

	foreach (InvPayload group in InvPayload.CreateGroup(payload.Type, payload.Hashes))
	system.LocalNode.Tell(Message.Create(MessageCommand.GetData, group));

Do we need to create payload into group twice? @shargon

[shargon]

Thanks @eryeer I will focus on the second part then, because as you said, the first one is already good.