spec/fix-rpl-abnf #52
Conversation
|
the spec still isn't fully done @oskarth, but this brings in the first couple of fixes. |
waku.md
Outdated
|
|
||
| ; packet-format needs to be paired with its corresponding packet-format | ||
| packet = "[" required-packet / [ optional-packet ] "]" | ||
| packet-code = 0 / 1 / 2 / 3 / [ x4-127 ] |
There was a problem hiding this comment.
I know I wrote this initially, but this isn't the proper way of doing number ranges in abnf
There was a problem hiding this comment.
IIRC the way ISO8601 datetimes are documented is that they are 1-3DIGITS, and then they have a separate constraints section that indicates '0-127' are reserved. That would work too imo
There was a problem hiding this comment.
So I looked at ISO8601, what it does is add 1*3DIGIT meaning 1-3 Digits but does not actually set a constraint.
There was a problem hiding this comment.
I mean this Restrictions section here: https://tools.ietf.org/html/rfc3339#section-5.7
They also do some stuff in-line if it is very small, e.g. date-month = 2DIGIT ; 01-12
| packet-format = "[" packet-code packet-format "]" | ||
|
|
||
| status = "[" version pow-requirement [ bloom-filter ] [ light-node ] "]" | ||
| status = "[" version pow-requirement [ bloom-filter ] [ light-node ] "]" |
There was a problem hiding this comment.
two optionals seems a bit ambiguous, might want to clarify in a comment
|
|
||
| p2p-request = waku-envelope | ||
| light-node = BIT |
There was a problem hiding this comment.
fwiw this works for me in Panini, it expect 0 or 1
waku.md
Outdated
| nonce = bytes | ||
| nonce = 8*OCTET | ||
|
|
||
| messages = *waku-envelope |
waku.md
Outdated
|
|
||
| p2p-request = waku-envelope | ||
|
|
||
| p2p-message = waku-envelope |
There was a problem hiding this comment.
what about multiple envelopes here?
There was a problem hiding this comment.
do we allow multiple envelopes here?
There was a problem hiding this comment.
you reviewed the mailserver spec...
There was a problem hiding this comment.
Historic messages MUST be sent to a peer as a packet with a P2P Message code (0x7f) followed by an array of Waku envelopes. It is incompatible with the original Whisper spec (EIP-627) because it allows only a single envelope, however, an array of envelopes is much more performant. In order to stay compatible with EIP-627, a peer receiving historic message MUST handle both cases.
you literally merged this
|
Ping (wrong button) |
|
There's also a weird section with whisper envelope double specified that should probably be removed in this PR |
|
|
|
@oskarth I feel this addresses most issues. |
|
|
||
| p2p-request = waku-envelope | ||
|
|
||
| p2p-message = waku-envelope | ||
| p2p-message = 1*waku-envelope |
There was a problem hiding this comment.
Is it encoded differently? IIRC it is either waku-envelope OR an rlp list of waku envelopes. @adambabik ?
| @@ -53,7 +53,7 @@ Using [Augmented Backus-Naur form (ABNF)](https://tools.ietf.org/html/rfc5234) w | |||
|
|
|||
| ``` | |||
| ; Packet codes 0 - 127 are reserved for Waku protocol | |||
| packet-code = 0 / 1 / 2 / 3 / [ x4-127 ] | |||
| packet-code = 1*3DIGIT | |||
There was a problem hiding this comment.
WDYT about section below with additional constraints, where we mention 0-127?
|
|
||
| envelope = flags auxiliary-field payload padding [signature] salt | ||
| envelope = flags auxiliary-field payload padding [signature] [salt] |
There was a problem hiding this comment.
two optionals here may seem fishy, but it makes sense with additional constraints (flag indicated) - perhaps we can pull this out into additional constraints section to keep the abnf more tight?
|
PR been open too long, it blocks/makes other things more complicated. Issues above are still issues but can be addressed in follow up issues/PRs. Lets make sure we get these types of PRs merged faster than a week+ |
|
#43 see issues |
* fixes * indent * rearranged * Update waku.md * commit
No description provided.