search

CityOfZion / neo-python

Python Node and SDK for the NEO 2.x blockchain. For NEO 3.x go to our successor project neo-mamba
https://neo-python.readthedocs.io/en/latest/
MIT License
313 stars 189 forks source link

neo-python can't sync with Neo 2.10.3 nodes #1014

Closed hal0x2328 closed 4 years ago

hal0x2328 commented 5 years ago

Current behavior

When connecting to a Neo 2.10.3 singlenet I noticed that the remote peer disconnected immediately after receiving the getheaders message. Since it's a singlenet that means that neo-python stays at 0/0/0 indefinitely even though all configuration files are correct and it works if the Neo node is downgraded to 2.10.2

I traced the behavior to the serialize method in neo/Network/payloads/getblocks.py https://github.com/CityOfZion/neo-python/blob/5bdded2c339219355cf1d31ae58653b0f94c6e51/neo/Network/payloads/getblocks.py#L15

Neo 2.10.3 expects a 32-byte hash_stop value at the end of the getheaders message payload, but it appears that the neo-python serialize method never adds it.

On previous versions of Neo this seems not to have been a problem, but 2.10.3 encounters an exception when trying to deserialize the non-existent value from the payload and immediately terminates the peer connection. Specifically at this line:

https://github.com/neo-project/neo/blob/56174b8fc2e31bc0f300e4dab1f61bd8cf81ca6e/neo/Network/P2P/Payloads/GetBlocksPayload.cs#L25

I'm not quite sure how it was working before, because the GetBlocksPayload.cs file hasn't actually changed recently - it may be related to exception handling in a parent function perhaps but I didn't dig into it any further.

Either way, I think the serialize method in getblocks.py should be modified to include the hash_stop UInt256 value even when it is zero.

Expected behavior

neo-python should sync with Neo 2.10.3

How to reproduce

Connect neo-python to a Neo 2.10.3 singlenet

Your environment

ixje commented 4 years ago

Thanks for the report and analysis 👍 I still don't know how it worked for <= 2.10.2 as I didn't find any changes in the 2.10.3 commits that on first sight make sense with the behaviour. Either way, it should now work in dev.