update insecure transport to plaintext/2.0.0

[yusefnapora]

This changes the behavior of the insecure "security" transport to conform to the new spec. Will close #36.

@bigs I can't seem to figure out how to correctly import the crypto.pb file from the new plaintext.pb file I added.

The way it's set up now, protoc generates an import statement like

import (
  "other/stuff"
  pb "crypto/pb"
)

Which doesn't resolve to anything. I manually changed that to pb "github.com/libp2p/go-libp2p-core/crypto/pb", which is what's committed in here, just so I could get something building.

I also copied the test code over from secio and stripped out the stuff related to actual crypto. It seems like it works...

Another thing I'm not sure about is whether adding a new private key argument to the New constructor will break anything. I think I remember that the reflection magic thing will inject a private key if you take one in as an arg, but I can't remember where that code is, lol.

[yusefnapora]

I'm also wondering if we should log a warning when the transport is instantiated, just as a guard against people accidentally enabling this during production. I don't know anything about go logging though, so idk what's appropriate for a library to log and so on.

[yusefnapora]

About the protobuf import thing, I've discovered that it basically works if you add the go_package option to your protobuf files like so:

crypto.proto:

syntax = "proto2";

package crypto.pb;
option go_package = "github.com/libp2p/go-libp2p-core/crypto/pb";

Then when you import crypto.proto from plaintext.proto, the generated code in plaintext.pb.go is:

import (
	fmt "fmt"
	proto "github.com/gogo/protobuf/proto"
	pb "github.com/libp2p/go-libp2p-core/crypto/pb"
	io "io"
	math "math"
)

Which is great and the compiler loves me. The only problem is that, instead of generating crypto.pb.go in the current directory, protoc now makes a directory tree, e.g.

.
└── github.com
    └── libp2p
        └── go-libp2p-core
            └── sec
                └── insecure
                    └── pb
                        └── plaintext.pb.go

So that's annoying. It's not hard to work around - we can find the generated file in the tree and move back where it's supposed to go.

Maybe there's a way that's less of a hack though, i don't know.

[bigs]

looks great! just some golang tips to make things a little more brief

[yusefnapora]

Sweet, thanks for the feedback @bigs, it's much better now :)

[bigs]

we have a default here https://github.com/libp2p/go-libp2p-core/blob/master/network/network.go#L23

but honestly we can probably place a smaller limit on key exchanges. just linking for info

[bigs]

on second thought, may be worth just using that constant. also this is reminding me, one of msgio's big benefits is buffer pooling, but since this is a test-only upgrader, we're probably fine with a little extra allocation.

[yusefnapora]

awesome, thanks for pointing me at the constant. it seems best to use it for consistency rather than have more magic numbers sprinkled about.

[raulk]

This needs to be rethought. For now, the merge has been reverted.

[raulk]

This causes pretty severe API breakage.

1. in the Transport option of the go-libp2p constructor.
2. elsewhere (e.g. go-tcp-transport) where this constructor is used directly.

[raulk]

The private key is unnecessary here. The public key should do.

[raulk]

We should deprecate the New function and make a WithIdentity() function that the public key, and returns a function of signature func(id peer.ID) *Transport that can then be used with the go-libp2p constructor.

[yusefnapora]

Adding a new constructor function and deprecating the old is definitely the way to go 👍

I added the private key to avoid returning nil from LocalPrivateKey - if we don't care about that then we can just take the public key.

[bigs]

definite lapse here, sorry. i think we should just add a new version of the protocol alongside 1.0.0. that’s what the versioning is for and this protocol is different.

[raulk]

@bigs I thought of that too, but I concluded that v1 does not really work, and there’s no point ossifying something that’s broken.