Upstream new protocols (Yamux, TLS, Quic)

[ianopolous]

Hi we're hoping to upstream a bunch of new protocols we've implemented based on this. Specifically Yamux, TLS, multiaddr fixes and soon Quic. We also have Kademlia and bitswap in Java on top of this here: https://github.com/Peergos/nabu These are all tested against kubo for interop.

The only potential difficulties we see are

• the ~1 commit after we forked where this repo was refactored and a lot of stuff was moved around.
• our removal of a bunch of dependencies like guava, log4j, apache commons
• our minimum Java 17 requirement (required for Ed25519 TLS handling)

Is there interest in up-streaming our work? What are your thoughts?

The fork is https://github.com/peergos/jvm-libp2p

[mxinden]

Friendly ping @Nashatyrev.

[Nashatyrev]

Hey, that sounds exciting!
However I believe that should be discussed more closely with the Teku team, since the changes could potentially impose the risk of regression bugs in the Teku networking layer.

In the meanwhile let me ask you some initial questions on the items you listed:

our removal of a bunch of dependencies like guava, log4j, apache commons

What's the reasons behind removing those dependencies?
Are you using other logging API? (I think I've seen Java Logging?) Don't you have plans to switch?

our minimum Java 17 requirement (required for Ed25519 TLS handling)

Teku is still on Java 11, so I'm not sure how we could handle this at the moment. As an option: postpone TLS module until Teku upgrade?

the ~1 commit after we forked where this repo was refactored and a lot of stuff was moved around.

Are you referring to #267 ? I believe it shouldn't be too problematic to adopt, since there were mostly structural changes and the code was not affected mostly

[ianopolous]

Thanks @Nashatyrev , yep that makes sense to discuss with Teku.

We try to minimise dependencies for security and maintainability reasons mainly, but also compiled jar size. For logging in particular we've never needed log4j in various companies over the years and always manage with the built-in logging.

I'd love to remove the Java 17 requirement too. It might be possible with some vendoring of bouncy castle components. Quic also depends on TLS though, so that would also have to be post-poned.

Good to hear #267 shouldn't be difficult to rebase off.

[Nashatyrev]

our removal of a bunch of dependencies like guava, log4j, apache commons

• commons and guava: there are quite few classes used, so it probably makes sense to get rid of those deps
• log4j: we should think a bit on this. I believe that the good practice for libs is to use slf4j thin logging API layer. There are a number of adapters end users may setup for their favorite logger. It includes Log4J and Java Logging

[ianopolous]

Yep we're happy to include slf4j if that helps.

[ajsutton]

For the record, log4j-api works just like slf4j these days and can work with multiple backends, not just log4j (including funnily enough slf4j from memory). It's more common to use slf4j for that kind of thing though so not unreasonable to switch to that. Teku already has some dependencies that use slf4j so needs to ensure it has a mapping to log4j in place anyway.

[StefanBratanov]

Hi @ianopolous , we (Teku) will migrate to Java 17 in a couple of releases, so that shouldn't be a problem. Wonder if it is an idea to implement the changes on a branch (v1.x for example) and potentially release 1.0.0 and then we can start testing with it and when happy can merge these changes to develop and move from there. It is unlikely we would do any significant changes in the meantime to the library. What do you think @Nashatyrev ?

[Nashatyrev]

Wonder if it is an idea to implement the changes on a branch (v1.x for example)

@StefanBratanov , sounds like a good plan 👍 And it would be indeed a good candidate for version 1.0.0

Just a note regarding Java 17. I believe we would also like to keep some compatibility with Android. jvm-libp2p have android-chatter example (which is not yet included to the build #270) which is presumably functional. I'll try to sort it out.
CCing @raulk who was the primary person interested in Android Libp2p

[Nashatyrev]

Related PR #265 which was closed
Also the previous discussion is here: #262

[ianopolous]

Cool, so it sounds like we've got the green light to rebase, add slf4j and submit a PR, and then we can discuss the details there?

[Nashatyrev]

Ok, I've created new branch as discussed : https://github.com/libp2p/jvm-libp2p/tree/v1.0.0
The first PR here is including Android sample as promised earlier: #275

I'm looking forward to integrate all the changes step-by-step with minimal atomic PRs.
I would suggest to start with the following sequence:

• migrating to slf4j
• removing Apache commons dependency
• removing Guava dependency
• multiaddr fixes + additional unit tests on changed classes
• prepare SecureChannel for 'early muxer negotiation' (TLS #262 (comment)) + additional unit tests on changed classes

@ianopolous @StefanBratanov, what do you guys think on this?

[Nashatyrev]

@ianopolous just found out that you have excluded guava dependency but copied some of its classes to the project.
What's the point of this?

[ianopolous]

We care about binary size. So this is ~3MiB saving. They are mostly random util classes.

[Nashatyrev]

Got it.
Didn't you consider using kind of 'minifiers' for that? As far as know they are able to safely exclude unused classes from the distribution.
I would then suggest to postpone that decision because I'm not really sure this is the solution we would like to stick with.

[Nashatyrev]

BTW I've managed to remove Apache commons and use Base32 from bouncycastle: #277

[ianopolous]

Nope but we can probably use one of those.

[ianopolous]

I think we used the Base32 from java-multibase which is itself a vendored version of apache commons.

[ianopolous]

Ok, how does this sound:

PR 1: switch to slf4j
PR 2: multiaddr fixes - #280
PR 3: yamux - #281
PR 4: TLS + early muxer negotiation - #283
PR 5: WIP quic transport (and security and muxer)

We can do the jar minifier downstream.

[Nashatyrev]

@ianopolous Sounds good!
I've already prepared Slf4j changes as well :)
There is a queue of PRs to v1.0.0 at the moment. Calling @StefanBratanov for review help

[StefanBratanov]

@ianopolous Sounds good! I've already prepared Slf4j changes as well :) There is a queue of PRs to v1.0.0 at the moment. Calling @StefanBratanov for review help

Hi @Nashatyrev approved your PRs

[Nashatyrev]

Ok, preparatory PRs are all merged.
Sorry again for Slf4j mess

[ianopolous]

TLS PR is ready for review: #283

[Nashatyrev]

@ianopolous could you please review #285 . Yamux cases in particular.

[ianopolous]

All that's left now that TLS is merged is:

• PR 5: Expose EventLoop on Stream - Expose eventloop on Stream #290 NOT NEEDED
• PR 6: WIP quic transport (and security and muxer) Quic secure transport implementation #294

[Nashatyrev]

@ianopolous I will be on vacation for the next week, but then I would like to sort out the EventLoop need.
More details on that are highly appreciated.

If you think it makes sense we may connect on Telegram: https://t.me/nashatyrev_a

[ianopolous]

Let's upgrade to email (I don't have telegram), and we can upgrade from there. I'm ian at peergos dot org.

[ianopolous]

There are two more PRs to fix a yamux and a TLS regression:

• Fix yamux remote open #292
• Fix ecdsa cert usage in TLS which bouncycastle broke #293

[Nashatyrev]

@ianopolous I'm updating README for v1.0.0 here #301
Should we add your project in the section 'Notable users'?

[ianopolous]

Sure, that'd be great, you can add https://github.com/peergos/nabu and https:///github.com/peergos/peergos if you want.

[ianopolous]

We also have implementations of kademlia, bitswap, autonat, circuit-relay-v2 (WIP) in Nabu.