noise: specify proto2

[marten-seemann]

Proto2 allows us to distinguish between unset and default values, which Proto3 doesn't. Since the protobuf we used for Noise was that simple, the serialization is compatible, so we can change to Proto2, even though some implementations interpreted the Protobuf defined here as Proto3.

I confirmed that a node running v0.22.0 (using a proto3 protobuf) can connect to a node using the proto2 protobuf, and vice versa.

Server

func main() {
    h1, err := libp2p.New(
        libp2p.ListenAddrStrings("/ip4/127.0.0.1/tcp/1234"),
        libp2p.Security("noise", noise.New),
    )
    if err != nil {
        log.Fatal(err)
    }
    fmt.Println("I am:", h1.ID())

    h1.SetStreamHandler("/proto1", func(str network.Stream) {
        data, err := io.ReadAll(str)
        if err != nil {
            log.Fatal(err)
        }
        fmt.Println("received data for proto1:", string(data))
    })
    select {}
}

client (using the ID printed by the server):

func main() {
    id, err := peer.Decode("QmcWzKr7pPn7TXc5rYiW6YtLSHBc1Bjw3ScvUK9wHn4rgW")
    if err != nil {
        log.Fatal(err)
    }
    addr := ma.StringCast("/ip4/127.0.0.1/tcp/1234")

    h2, err := libp2p.New(
        libp2p.ListenAddrStrings("/ip4/127.0.0.1/tcp/1235"),
        libp2p.Security("noise", noise.New),
    )
    if err != nil {
        log.Fatal(err)
    }
    if err := h2.Connect(context.Background(), peer.AddrInfo{
        ID:    id,
        Addrs: []ma.Multiaddr{addr},
    }); err != nil {
        log.Fatal(err)
    }

    str, err := h2.NewStream(context.Background(), id, "/proto1")
    if err != nil {
        log.Fatal(err)
    }
    str.Write([]byte("foobar"))
    str.Close()
    select {}
}

[MarcoPolo]

Can you include some context here too please. Why do we want to use proto2 instead of proto3? Also do you have an example test program you used to verify the bytes are the same over the wire? Thanks!

[marten-seemann]

@MarcoPolo I added the code and a short motivation.

[mxinden]

I think this deserves a revision bump.

Missing this one though, right?

[marten-seemann]

I think this deserves a revision bump.

Sorry, missed that. I bumped it. Not sure if it technically requires one, since this should be a backwards-compatible change, but then it doesn't hurt either.

[mxinden]

I think this deserves a revision bump.

Sorry, missed that. I bumped it. Not sure if it technically requires one, since this should be a backwards-compatible change, but then it doesn't hurt either.

If I am not mistaken, a non-breaking change qualifies as a revision bump:

Revision numbers start with lowercase r followed by an integer, which gets bumped whenever the spec is modified by merging a new PR.

https://github.com/libp2p/specs/blob/master/00-framework-02-document-header.md

[marten-seemann]

If I am not mistaken, a non-breaking change qualifies as a revision bump

Revision numbers start with lowercase r followed by an integer, which gets bumped whenever the spec is modified by merging a new PR.

https://github.com/libp2p/specs/blob/master/00-framework-02-document-header.md

Indeed, that's what the document says. We should change that. It doesn't make sense to bump the version for every change, e.g. in the extreme case fixing a typo.

[achingbrain]

Proto2 allows us to distinguish between unset and default values, which Proto3 doesn't

The protocol buffers v3 spec says of the optional keyword (emphasis mine):

• optional: the same as singular, except that you can check to see if the value was explicitly set. An optional field is in one of two possible states:

  • the field is set, and contains a value that was explicitly set or parsed from the wire. It will be serialized to the wire.
  • the field is unset, and will return the default value. It will not be serialized to the wire.

This seems to contradict the above sentence from the OP? Is it just the generated go code that doesn't let you check to see if a value was explicitly set? If so it sounds more like singular:

• singular: a well-formed message can have zero or one of this field (but not more than one). When using proto3 syntax, this is the default field rule when no other field rules are specified for a given field. You cannot determine whether it was parsed from the wire. It will be serialized to the wire unless it is the default value. For more on this subject, see