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

move go-libp2p-core here #1683

Merged
merged 213 commits into from
Aug 18, 2022
Merged

move go-libp2p-core here #1683

merged 213 commits into from
Aug 18, 2022

Conversation

marten-seemann
Copy link
Contributor

@marten-seemann marten-seemann commented Aug 17, 2022

The last remaining part of #1556.

This PR also:

  • moves the mock resource manager from go-libp2p-testing/mocks/network to core/network/mocks
  • stops using the go-libp2p-testing/net helper function for generating peer identities. This is required because currently (until we've forwarded the type, which requires a release), the old and the new peer.ID are not identical, which breaks the build. These functions were not super useful anyway, it's not that hard to generate a new identity.

raulk and others added 30 commits May 22, 2019 18:31
Avoids circular module dependency.
* Use SplitLast to avoid allocating too much.
* Avoid converting to/from strings when working with multiaddrs.
* SplitAddr is a simpler way to split an address into a multiaddr and an ID.
* AddrInfosFromP2pAddrs converts a set of multiaddrs into a set of AddrInfos.
feat(peer): implement AddrInfosFromP2pAddrs and SplitAddr
Add OpenCensus metrics registration functionality to core
License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
* add plaintext/2.0.0 (with ugly protoc hack)

* gofmt

* gofmt (for real this time)

* add `go_package` option to proto files

* Revert "add `go_package` option to proto files"

5a543a79bd89d75c9697f852638b8fe56da862f4

* less hacky protobuf imports

* add doc comment for PublicKeyFromProto

* clean up handshake

* go fmt, lol

* use network.MessageSizeMax for ggio reader
Replace bytes.Equal -> subtle.ConstantTimeCompare
Return error rather than panic in GenerateEKeyPair
License: MIT
Signed-off-by: Jakub Sztandera <kubuxu@protonmail.ch>
avoid duplicate randomly generated keys/peer-ids
* bring back plaintext 2.0.0 with new constructor

* fix deprecation comment

* rm unused context argument

* only check remote id validity if we actually have keys

* bring back msgio & simultaneous read/write
@vyzo
Copy link
Contributor

vyzo commented Aug 17, 2022

As I have outlined in the issue, I disagree with moving core in the main repo.

I have rather strong conviction that applications (and libraries) should not depend on go-libp2p itself except for initialization and tests; everything else only depends on the interface.

This is also going to create a ton of work for upstream users, who currently only depend on the core interfaces for application logic (other than initialization and tests).

@vyzo
Copy link
Contributor

vyzo commented Aug 17, 2022

The last part should be stressed out and very much considered. You are effectively creating a bifurcation point of incompatibility and forcing everyone to change threir code in tandem to accomodate your whims.

You really should put more consideration to the ecosystem as a whole and the myriad of libraries and applications that depend on libp2p.

@marten-seemann
Copy link
Contributor Author

As I have outlined in the issue, I disagree with moving core in the main repo.

I have rather strong conviction that applications (and libraries) should not depend on go-libp2p itself except for initialization and tests; everything else only depends on the interface.

There was a good discussion about this on #1556. I really appreciate your feedback!

Unfortunately, this is a binary decision (either we consolidate, or we don't), and we have to decide one way or the other. I realize we haven't been able to convince each other of our respective opinions.
Based on the discussion on the issue, I've made the decision to move forward with repo consolidation. The feedback from other team members shows that most people think that the pros of consolidating outweigh the cons.

I suggest keeping the discussion on that issue, and use this PR only for conversations about the specifics of the plan.

Copy link
Collaborator

@MarcoPolo MarcoPolo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m really excited for this as a user and a maintainer. We can stop telling people “You need to reference the correct version of go-libp2p-core when you use go-libp2p. Which version? the one that go-libp2p uses itself…”

I won’t rehash our discussion in #1556, but we can all agree that we want go-libp2p to be easier to use and easier to maintain. I think this is a step in the right direction and in reviewing #1556 I believe the main objections have been addressed.

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

Successfully merging this pull request may close these issues.