Neo: getblocks message only uses the first hash and hashstop is never used?

[lock9]

Hello,
First, I'm sorry to bother you with all this 'useless' issues. It's because I'm doing very extensive review of the current implementation, it's documentation to try to create a better documentation, or even some sort of "minimal specification".

"Problem" 1:
The documentation states that the network message _getblocks_ has this content:
32*? | HashStart | uint256[] | Hash of latest block that node requests
32 | HashStop | uint256 | Hash of last block that

Question:
How is it possible to have multiple "hashStart"? If we check the code, it's not even possible to create an array:
public static GetBlocksPayload Create(UInt256 hash_start, UInt256 hash_stop = null)

"Problem" 2:
I don't think HashStop is ever used. Take a look into the code:

C# private void OnGetBlocksMessageReceived(GetBlocksPayload payload) { UInt256 hash = payload.HashStart[0]; if (hash == payload.HashStop) return; BlockState state = Blockchain.Singleton.Store.GetBlocks().TryGet(hash); if (state == null) return; List<UInt256> hashes = new List<UInt256>(); for (uint i = 1; i <= InvPayload.MaxHashesCount; i++) { uint index = state.TrimmedBlock.Index + i; if (index > Blockchain.Singleton.Height) break; hash = Blockchain.Singleton.GetBlockHash(index); if (hash == null) break; hashes.Add(hash); } if (hashes.Count == 0) return; Context.Parent.Tell(Message.Create("inv", InvPayload.Create(InventoryType.Block, hashes.ToArray()))); }

It only breaks the loop if it gets into a null block. Shouldn't it be
if (hash == null || hash == hashStop) break; ?

Because of that, the nodes are always returning 500 blocks.

Suggestion:
Since hashes are non-linear, shouldn't this message be more like getBlocks(UInt256 hashStart, int numberOfBlocks)? How is it possible for the "other side" to know the "hashStop"?

[erikzhang]

You're right about that. This is a temporary implementation, and we actually have plans to improve the Peer-to-peer protocol in version 3.0 to optimize the process of block synchronization.

[lock9]

Ok, thanks Erik!