Update Netty 4.1→4.2 and companion dependency upgrades#237
Conversation
Dependency version bumps: - Netty 4.1.115.Final → 4.2.15.Final - Guava 33.2.1-jre → 33.6.0-jre - Log4j 2.23.1 → 2.26.0 - BouncyCastle 1.78.1 → 1.84 - Vert.x 4.5.9 → 4.5.28 - Tuweni 2.7.0 → 2.7.2 - Add org.jspecify:jspecify:1.0.0 (compileOnly) Fix deprecations introduced by these upgrades: Netty 4.2: - Replace NioEventLoopGroup with MultiThreadIoEventLoopGroup + NioIoHandler - Replace InternetProtocolFamily with java.net.StandardProtocolFamily Reactor 3.5+: - Migrate ReplayProcessor/FluxProcessor/FluxSink to Sinks.many().replay().latest() - PipelineImpl.push() uses emitNext with FAIL_NON_SERIALIZED retry to handle concurrent emissions from IPv4 and IPv6 Netty event-loop threads - Suppress deprecated Scheduler.start() override in DelegatingReactorScheduler JSpecify (replacing javax.annotation removed by Netty 4.2 transitive dep): - Replace @nonnull with @nonnull from org.jspecify.annotations Guava: - Replace CacheBuilder.expireAfterWrite(long, TimeUnit) with expireAfterWrite(Duration)
There was a problem hiding this comment.
Pull request overview
This PR upgrades core networking and supporting dependencies (notably Netty 4.1→4.2) and updates the discovery networking/reactive plumbing to match new APIs and remove deprecations, including the migration from Reactor ReplayProcessor/FluxSink to Sinks.Many and adoption of JSpecify annotations.
Changes:
- Upgrade Netty (4.1→4.2) and companion libraries (Guava, Log4j, BouncyCastle, Vert.x, Tuweni), plus add
org.jspecify:jspecifyascompileOnly. - Replace deprecated Reactor processors/sinks with
Sinks.many().replay().latest()across pipeline, networking, and scheduler utilities. - Replace Netty
InternetProtocolFamilyusage withjava.net.StandardProtocolFamilymapping helpers.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/org/ethereum/beacon/discovery/pipeline/handler/OutgoingParcelHandlerTest.java | Updates test to mock Sinks.Many instead of FluxSink and verify emission via Reactor sinks API. |
| src/main/java/org/ethereum/beacon/discovery/scheduler/SimpleProcessor.java | Migrates from ReplayProcessor/FluxSink to Sinks.Many for a lightweight processor abstraction. |
| src/main/java/org/ethereum/beacon/discovery/scheduler/ErrorHandlingScheduler.java | Switches @Nonnull to JSpecify @NonNull on scheduler APIs. |
| src/main/java/org/ethereum/beacon/discovery/scheduler/DelegatingReactorScheduler.java | Switches @Nonnull to JSpecify @NonNull and suppresses deprecated start() override. |
| src/main/java/org/ethereum/beacon/discovery/pipeline/PipelineImpl.java | Migrates pipeline source from ReplayProcessor to Sinks.Many with non-serialized emission retry. |
| src/main/java/org/ethereum/beacon/discovery/pipeline/handler/OutgoingParcelHandler.java | Migrates outgoing parcel emission from FluxSink.next to Sinks.Many.tryEmitNext. |
| src/main/java/org/ethereum/beacon/discovery/network/NettyDiscoveryServerImpl.java | Updates Netty event loop initialization for Netty 4.2 and replaces inbound processor with Sinks.Many. |
| src/main/java/org/ethereum/beacon/discovery/network/NettyDiscoveryClientImpl.java | Replaces InternetProtocolFamily channel selection with StandardProtocolFamily helper. |
| src/main/java/org/ethereum/beacon/discovery/network/IncomingMessageSink.java | Updates inbound Netty handler to emit into Sinks.Many instead of FluxSink. |
| src/main/java/org/ethereum/beacon/discovery/DiscoverySystemBuilder.java | Updates listen-address validation to use StandardProtocolFamily instead of Netty’s InternetProtocolFamily. |
| src/main/java/org/ethereum/beacon/discovery/DiscoveryManagerImpl.java | Migrates outgoing message stream to Sinks.Many, updates client wiring, and uses StandardProtocolFamily for channel map keys. |
| src/main/java/org/ethereum/beacon/discovery/database/ExpirationSet.java | Migrates Guava cache expiration API to Duration overload. |
| gradle/versions.gradle | Bumps Netty/Guava/Log4j/BouncyCastle/Vert.x/Tuweni versions and adds managed JSpecify dependency. |
| build.gradle | Adds compileOnly org.jspecify:jspecify to support JSpecify annotations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| outgoingSink.tryEmitNext(parcel); | ||
| envelope.remove(Field.INCOMING); |
There was a problem hiding this comment.
Fixed in def500b: OutgoingParcelHandler now retries on FAIL_NON_SERIALIZED (bounded, via a new Utils.emitNextWithRetry helper) and only removes Field.INCOMING after a confirmed successful emit; any other failure is logged at ERROR instead of silently dropping the packet.
| protected void channelRead0(ChannelHandlerContext ctx, Envelope msg) { | ||
| LOG.trace(() -> String.format("Incoming packet %s in session %s", msg, ctx)); | ||
| messageSink.next(msg); | ||
| messageSink.tryEmitNext(msg); |
There was a problem hiding this comment.
Fixed in def500b: IncomingMessageSink now checks the EmitResult and logs a WARN if the emit fails, so a dropped incoming packet is no longer silent.
| @Override | ||
| public void onNext(T t) { | ||
| sink.next(t); | ||
| sinks.tryEmitNext(t); | ||
| } |
There was a problem hiding this comment.
Fixed in def500b: SimpleProcessor.onNext now checks the returned EmitResult and logs a WARN on failure instead of discarding it silently.
| public void onError(Throwable throwable) { | ||
| sink.error(throwable); | ||
| sinks.tryEmitError(throwable); | ||
| } |
There was a problem hiding this comment.
Fixed in def500b: SimpleProcessor.onError now checks the returned EmitResult and logs a WARN on failure instead of discarding it silently.
| @Override | ||
| public void onComplete() { | ||
| sink.complete(); | ||
| sinks.tryEmitComplete(); | ||
| } |
There was a problem hiding this comment.
Fixed in def500b: SimpleProcessor.onComplete now checks the returned EmitResult and logs a WARN on failure instead of discarding it silently.
| handler.handle(envelope); | ||
|
|
||
| verify(outgoingSink).next(parcel); | ||
| verify(outgoingSink).tryEmitNext(parcel); |
There was a problem hiding this comment.
Fixed in def500b: the test now stubs outgoingSink.tryEmitNext(any()) to return Sinks.EmitResult.OK before exercising shouldSendPacketsToAllowedHosts, so it stays representative now that OutgoingParcelHandler branches on the emit result.
tryEmitNext/tryEmitError/tryEmitComplete return an EmitResult that the Netty 4.2/Reactor migration was ignoring, silently dropping packets on failure. Add Utils.emitNextWithRetry(), a bounded retry on FAIL_NON_SERIALIZED (contended concurrent producers), and use it in OutgoingParcelHandler and PipelineImpl.push() instead of retrying forever; only remove Field.INCOMING after a confirmed successful emit. IncomingMessageSink and SimpleProcessor log a warning on failure since they have no concurrent-producer path requiring a retry. Addresses Copilot review comments on PR Consensys#237.
Summary
org.jspecify:jspecify:1.0.0(compileOnly) to replacejavax.annotation.Nonnullwhich was previously provided transitively by Netty 4.1 but dropped in 4.2 (aligns with Besu's migration to JSpecify)ReplayProcessor→Sinks, NettyNioEventLoopGroup→MultiThreadIoEventLoopGroup,InternetProtocolFamily→java.net.StandardProtocolFamily, GuavaCacheBuilderduration overload)Details
Netty 4.2 deprecations
NioEventLoopGroup→MultiThreadIoEventLoopGroup(1, NioIoHandler.newFactory())inNettyDiscoveryServerImplInternetProtocolFamily→java.net.StandardProtocolFamilyinNettyDiscoveryClientImpl,DiscoveryManagerImpl,DiscoverySystemBuilderReactor Sinks migration (
ReplayProcessor/FluxProcessor/FluxSinkdeprecated since Reactor 3.5)ReplayProcessor.cacheLast()+FluxSink→Sinks.many().replay().latest()acrossNettyDiscoveryServerImpl,DiscoveryManagerImpl,PipelineImpl,SimpleProcessor,IncomingMessageSink,OutgoingParcelHandlerPipelineImpl.push()usesemitNextwithFAIL_NON_SERIALIZEDretry rather thantryEmitNext— necessary becauseincomingPipelinecan receive concurrent pushes from IPv4 and IPv6 Netty event-loop threads; the oldReplayProcessor.FluxSinkserialized these internally, the new safeSinks.Manydoes notDelegatingReactorScheduler.start(): suppress deprecated override (preserves delegation behaviour for downstream schedulers)Other
CacheBuilder.expireAfterWrite(long, TimeUnit)→expireAfterWrite(Duration)inExpirationSet@Nonnull→@NonNull(JSpecify) inDelegatingReactorScheduler,ErrorHandlingSchedulerTest plan
./gradlew buildpasses (all 16 integration tests green)./gradlew compileJava --rerun-taskswith-Xlint:deprecationproduces zero deprecation warningsNote
Medium Risk
Touches core UDP discovery I/O and message pipelines with a major Netty bump and changed concurrent emit semantics, though failures are logged and retries are bounded.
Overview
Upgrades Netty (4.1→4.2) and several companion libraries (Guava, Log4j, BouncyCastle, Vert.x, Tuweni), and adds
org.jspecifyascompileOnlyso@NonNullreplacesjavax.annotation.Nonnullafter Netty dropped that transitively.Netty 4.2 API updates: discovery servers use
MultiThreadIoEventLoopGroup+NioIoHandlerinstead ofNioEventLoopGroup; IPv4/IPv6 channel maps and listen-address validation usejava.net.StandardProtocolFamilyinstead of Netty’sInternetProtocolFamily.Reactor deprecation cleanup:
ReplayProcessor/FluxSinkpaths becomeSinks.many().replay().latest()(discovery server, manager, pipelines,SimpleProcessor, incoming/outgoing handlers). Emits usetryEmitNextwith logging on failure;PipelineImpl.pushandOutgoingParcelHandlercall newUtils.emitNextWithRetryto handleFAIL_NON_SERIALIZEDwhen multiple event-loop threads push concurrently (behavior the old sink serialized internally). GuavaExpirationSetswitches toexpireAfterWrite(Duration).Reviewed by Cursor Bugbot for commit def500b. Bugbot is set up for automated code reviews on this repo. Configure here.