Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nethermind hello message reports wrong version for witness protocol #4443

Closed
pinges opened this issue Aug 19, 2022 · 5 comments · Fixed by #4447
Closed

Nethermind hello message reports wrong version for witness protocol #4443

pinges opened this issue Aug 19, 2022 · 5 comments · Fixed by #4447

Comments

@pinges
Copy link

pinges commented Aug 19, 2022

Describe the bug
The hello message sent by Nethermind contains the capability for the witness protocol. It is RLP encoded as:
c5: list with length 5
83: String of length 3
776974: “wit”
80: String of length 0 (https://ethereum.org/en/developers/docs/data-structures-and-encoding/rlp/)

This can lead to other ethereum clients to fail to decode the hello message, which means that they are not connecting to Nethermind nodes.

The 80 here represents the version number of that capability. The current version of the witness protocol is 0 (https://github.com/ethereum/devp2p/blob/master/caps/wit.md)

The hello message sent by Nethermind indicates that the version is
Nethermind/v1.13.4-0-3e5972c24-20220811/X64-Linux/6.0.6

Full hello message sent by Nethermind
0xf8
A3
05
B7
4e65746865726d696e642f76312e31332e342d302d3365353937326332342d32303232303831312f5836342d4c696e75782f362e302e36

e4
c5
83
657468
3e
c5
83
657468
3f
C5
83
657468
40
C5
83
657468
41
C5
83
657468
42
C5
83
776974
80

82
765d

B8
40
05c95b2618ba1ca53f0f019d1750d12769267705e46b4dbfb77f73998b21d30973161542a2090bcaa5876e6aed99436009f3f646029bb723b8dff75feec27374

To Reproduce
Print out he hello message sent by Nethermind

Expected behavior
The version number for the witness protocol should be 0 (0x00) instead of 0x80

@tkstanczak
Copy link
Member

Massive thanks for reporting rhis @macfarla

@macfarla
Copy link

macfarla commented Aug 19, 2022

credit goes to @pinges ! We have an issue in Besu to accept this format in the short term (our guess is that other clients are accepting this) - on further investigation it may be that Nethermind is actually doing the right thing here

... making zero a zero length string trimmed to an empty string (spelled out explicitly here https://ethereum.org/en/developers/docs/data-structures-and-encoding/rlp) so c58377697480 is wit/0, rather than c58377697400 - which the latter is incorrect.

@LukaszRozmej
Copy link
Member

LukaszRozmej commented Aug 19, 2022

As a workaround maybe we should retroactively just bump the version number to 1?
You can support wit0 and wit1 for backward compatibility while we will support only wit1.

@pinges
Copy link
Author

pinges commented Aug 22, 2022

I have done some more research and I think that encoding an unsigned integer 0 as 0x80 (empty string) is OK, or maybe even common. I tried to find an official document, but failed to find one.
One example I found is that the boolean value "false" is encoded as 0x80, "true" is encoded as 0x01.

@pinges
Copy link
Author

pinges commented Aug 24, 2022

This problem has been fixed in Besu with hyperledger/besu#4283
The fix will be available in the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants