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

Setting RLPx subprotocol version to 0 crashes #119

Closed
oskarth opened this issue Nov 15, 2019 · 0 comments
Closed

Setting RLPx subprotocol version to 0 crashes #119

oskarth opened this issue Nov 15, 2019 · 0 comments

Comments

@oskarth
Copy link
Contributor

oskarth commented Nov 15, 2019

Problem

If I set wakuVersion (or whisperVersion) to 0 like this: https://github.com/oskarth/nim-eth/blob/waku-zero/eth/p2p/rlpx_protocols/waku_protocol.nim#L56-L57

I get the following crash:

[Suite] Waku connections
INF 2019-11-15 17:01:52+08:00 RLPx listener up                           tid=10758 self=enode://23cb2baaab4231bc85271e540c6161549199b78c7f66c2997ca7f0722650a8fcf5e005ff8b34043959bd5e8e0c27a1b868c8a870282a0f8bc18d5c1617084225@127.0.0.1:30304
Traceback (most recent call last)
/home/oskarth/git/nim-eth/tests/p2p/test_waku_connect.nim(26) test_waku_connect
/home/oskarth/.nimble/pkgs/chronos-2.3.2/chronos/asyncloop.nim(933) waitFor
/home/oskarth/.nimble/pkgs/chronos-2.3.2/chronos/asyncloop.nim(274) poll
/home/oskarth/git/nim-eth/eth/p2p/rlpx.nim(1029) rlpxConnect
/home/oskarth/git/nim-eth/eth/p2p/rlpx.nim(899) postHelloSteps
/home/oskarth/git/nim-eth/eth/p2p/p2p_protocol_dsl.nim(305) WakuPeerConnected
/home/oskarth/.choosenim/toolchains/nim-0.20.0/lib/system/gc.nim(238) asgnRef
/home/oskarth/.choosenim/toolchains/nim-0.20.0/lib/system/gc.nim(183) incRef
SIGSEGV: Illegal storage access. (Attempt to read from nil?)
Error: execution of an external program failed: '/home/oskarth/git/nim-eth/tests/p2p/test_waku_connect -vvv'

Solution

I can't find in docs/p2p.md or https://github.com/ethereum/devp2p/blob/master/rlpx.md that suggest that this should be invalid. Either:

  1. Make version 0 valid, or:

  2. Give a better validation/compile error, and add to docs that this is a requirement

Notes

I didn't debug or try to fix this further, but seems like a bug in the P2P DSL.

Additionally, and separately, I noticed that p2p/docs.md says subprotocols should be 3 letters. In devp2p/rlpx.md it just says "short ASCII name" as far as I can tell. I can't tell if this a spec constraint and I'm looking in the wrong place, or what's going on there. Didn't look into it too much. Some validation there might be useful too.

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

No branches or pull requests

1 participant