search

libp2p / specs

Technical specifications for the libp2p networking stack
https://libp2p.io
1.56k stars 273 forks source link

[Rendezvous] Change `ttl` and `limit` to `uint64` #338

Closed thomaseizinger closed 3 years ago

thomaseizinger commented 3 years ago

To be meaningful, TTL has to be a positive integer hence the use of an signed integer does not bring any value and only introduces unnecessary error handling.

mxinden commented 3 years ago

I am guessing you meant to write:

- hence the use of an unsigned integer does not bring any value
+ hence the use of a signed integer does not bring any value

I agree, though I don't think we can just alter the type as that would be a breaking protobuf change while this protocol is already used in the wild. @vyzo @vasco-santos can you comment here?

thomaseizinger commented 3 years ago

I am guessing you meant to write:

- hence the use of an unsigned integer does not bring any value
+ hence the use of a signed integer does not bring any value

I agree, though I don't think we can just alter the type as that would be a breaking protobuf change while this protocol is already used in the wild. @vyzo @vasco-santos can you comment here?

Yep will fix that!

Re breaking change: How does the protocol being a Draft affect this? I would have assumed that prior to being finalized, we can make breaking changes. It is not particularly nice for implementations but the way I see it, early adoptions of protocols have to accept that?

vyzo commented 3 years ago

yeah thats fine.

On Mon, Jun 21, 2021, 14:21 Thomas Eizinger @.***> wrote:

I am guessing you meant to write:

  • hence the use of an unsigned integer does not bring any value+ hence the use of a signed integer does not bring any value

I agree, though I don't think we can just alter the type as that would be a breaking protobuf change while this protocol is already used in the wild. @vyzo https://github.com/vyzo @vasco-santos https://github.com/vasco-santos can you comment here?

Yep will fix that!

Re breaking change: How does the protocol being a Draft affect this? I would have assumed that prior to being finalized, we can make breaking changes. It is not particularly nice for implementations but the way I see it, early adoptions of protocols have to accept that?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/libp2p/specs/pull/338#issuecomment-864954301, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAI4SQ2ZGIMUHWAB5GAXYTTT4OENANCNFSM47ATCA7Q .

thomaseizinger commented 3 years ago

@mxinden I've changed the commit message to say "signed integer".

thomaseizinger commented 3 years ago

In the same spirit, I changed limit of Discover to a uint64.

thomaseizinger commented 3 years ago

Would you mind bumping the spec revision?

Done!

mxinden commented 3 years ago

Thanks @thomaseizinger!