Skip to content

Commit 924baf6

Browse files
authored
tweaks (#10961)
1 parent 79914d9 commit 924baf6

8 files changed

Lines changed: 419 additions & 15 deletions

File tree

ethereum/statetransition/src/main/java/tech/pegasys/teku/statetransition/validation/AttestationValidator.java

Lines changed: 67 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,10 @@ public SafeFuture<InternalValidationResult> validateSingleOrAggregateAttestation
8787
return SafeFuture.completedFuture(InternalValidationResult.ACCEPT);
8888
}
8989
return singleOrAggregateAttestationChecks(
90-
signatureVerifier, validatableAttestation, validatableAttestation.getReceivedSubnetId())
90+
signatureVerifier,
91+
validatableAttestation,
92+
validatableAttestation.getReceivedSubnetId(),
93+
true)
9194
.thenApply(InternalValidationResultWithState::getResult)
9295
.thenPeek(
9396
result -> {
@@ -116,6 +119,19 @@ SafeFuture<InternalValidationResultWithState> singleOrAggregateAttestationChecks
116119
final AsyncBLSSignatureVerifier signatureVerifier,
117120
final ValidatableAttestation validatableAttestation,
118121
final OptionalInt receivedOnSubnetId) {
122+
// Aggregate validation path: preserve legacy behaviour where a future-slot attestation is
123+
// deferred without running signature verification here. The aggregate wrapper uses a batch
124+
// verifier that is flushed separately, so verifying (and optimistically caching) the signature
125+
// here would leave an unverified signature cached if the deferral short-circuits the flush.
126+
return singleOrAggregateAttestationChecks(
127+
signatureVerifier, validatableAttestation, receivedOnSubnetId, false);
128+
}
129+
130+
SafeFuture<InternalValidationResultWithState> singleOrAggregateAttestationChecks(
131+
final AsyncBLSSignatureVerifier signatureVerifier,
132+
final ValidatableAttestation validatableAttestation,
133+
final OptionalInt receivedOnSubnetId,
134+
final boolean verifyFutureSlotAttestationSignature) {
119135

120136
Attestation attestation = validatableAttestation.getAttestation();
121137
final AttestationData data = attestation.getData();
@@ -165,10 +181,20 @@ SafeFuture<InternalValidationResultWithState> singleOrAggregateAttestationChecks
165181
gossipValidationHelper.getCurrentTimeMillis());
166182

167183
if (slotInclusionGossipValidationResult.isPresent()) {
168-
return switch (slotInclusionGossipValidationResult.get()) {
169-
case IGNORE -> completedFuture(InternalValidationResultWithState.ignore());
170-
case SAVE_FOR_FUTURE -> completedFuture(InternalValidationResultWithState.saveForFuture());
171-
};
184+
if (slotInclusionGossipValidationResult.get() == SlotInclusionGossipValidationResult.IGNORE) {
185+
return completedFuture(InternalValidationResultWithState.ignore());
186+
}
187+
// SAVE_FOR_FUTURE: a future-slot attestation is deferred rather than accepted for gossip.
188+
// When requested, we still verify its signature so that a bogus signature is rejected and its
189+
// sender penalised, rather than being deferred with a penalty-free Ignore verdict and later
190+
// forcing a BLS verification in the fork choice path for free. Only an invalid signature
191+
// rejects here; every other gossip check simply defers (fork choice re-validates the rest
192+
// once the attestation's slot arrives). A verified signature is cached so fork choice does
193+
// not re-verify it.
194+
if (!verifyFutureSlotAttestationSignature) {
195+
return completedFuture(InternalValidationResultWithState.saveForFuture());
196+
}
197+
return checkFutureSlotAttestationSignature(validatableAttestation, signatureVerifier);
172198
}
173199

174200
// [REJECT] The block being voted for (attestation.data.beacon_block_root) passes validation.
@@ -276,4 +302,40 @@ SafeFuture<InternalValidationResultWithState> singleOrAggregateAttestationChecks
276302
});
277303
});
278304
}
305+
306+
/**
307+
* Verifies the signature of a future-slot attestation and always defers it (SAVE_FOR_FUTURE),
308+
* rejecting only when the signature is invalid. The remaining gossip checks are intentionally
309+
* skipped here: they either only apply to attestations eligible for immediate propagation, or are
310+
* re-validated by fork choice when the attestation's slot arrives. Rejecting solely on an invalid
311+
* signature keeps the peer penalty limited to provably-invalid messages, matching how other
312+
* clients score future-slot attestations, while still avoiding a penalty-free BLS verification in
313+
* the fork choice path.
314+
*/
315+
private SafeFuture<InternalValidationResultWithState> checkFutureSlotAttestationSignature(
316+
final ValidatableAttestation validatableAttestation,
317+
final AsyncBLSSignatureVerifier signatureVerifier) {
318+
final AttestationData data = validatableAttestation.getAttestation().getData();
319+
// The signature can't be verified without the block being voted for and its state; defer.
320+
if (!gossipValidationHelper.isBlockAvailable(data.getBeaconBlockRoot())) {
321+
return completedFuture(InternalValidationResultWithState.saveForFuture());
322+
}
323+
return gossipValidationHelper
324+
.getStateForAttestationValidation(data)
325+
.thenCompose(
326+
maybeState -> {
327+
if (maybeState.isEmpty()) {
328+
return completedFuture(InternalValidationResultWithState.saveForFuture());
329+
}
330+
return spec.isValidIndexedAttestation(
331+
maybeState.get(), validatableAttestation, signatureVerifier)
332+
.thenApply(
333+
signatureResult ->
334+
signatureResult.isSuccessful()
335+
? InternalValidationResultWithState.saveForFuture()
336+
: InternalValidationResultWithState.reject(
337+
"Attestation is not a valid indexed attestation: %s",
338+
signatureResult.getInvalidReason()));
339+
});
340+
}
279341
}

ethereum/statetransition/src/test/java/tech/pegasys/teku/statetransition/validation/Phase0AttestationValidatorTest.java

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static org.mockito.Mockito.doReturn;
2020
import static org.mockito.Mockito.mock;
2121
import static org.mockito.Mockito.spy;
22+
import static org.mockito.Mockito.verify;
2223
import static org.mockito.Mockito.when;
2324
import static tech.pegasys.teku.infrastructure.unsigned.UInt64.ONE;
2425
import static tech.pegasys.teku.infrastructure.unsigned.UInt64.ZERO;
@@ -364,4 +365,72 @@ public void shouldRejectAttestationsThatHaveLMDVotesInconsistentWithTargetRoot()
364365
.matches(
365366
rejected("descend from target block"), "Rejected does not descend from target block");
366367
}
368+
369+
@Test
370+
public void shouldRejectFutureSlotAttestationWithInvalidSignature() {
371+
// A future-slot attestation with a bogus signature must be rejected (so the sender is
372+
// penalised) rather than deferred with a penalty-free Ignore verdict, which would force a
373+
// signature verification in the fork choice path for free.
374+
final StateAndBlockSummary blockAndState = storageSystem.getChainHead();
375+
final Attestation attestation = attestationGenerator.validAttestation(blockAndState, ONE);
376+
assertThat(attestation.getData().getSlot()).isEqualTo(ONE);
377+
378+
final AsyncBLSSignatureVerifier signatureVerifier = mock(AsyncBLSSignatureVerifier.class);
379+
when(signatureVerifier.verify(anyList(), any(Bytes.class), any()))
380+
.thenReturn(SafeFuture.completedFuture(false));
381+
final AttestationValidator validator =
382+
new AttestationValidator(
383+
spec, signatureVerifier, gossipValidationHelper, invalidBlockRoots);
384+
385+
final int subnetId = spec.computeSubnetForAttestation(blockAndState.getState(), attestation);
386+
chainUpdater.setCurrentSlot(ZERO);
387+
388+
assertThat(
389+
validator
390+
.validate(ValidatableAttestation.fromNetwork(spec, attestation, subnetId))
391+
.join()
392+
.code())
393+
.isEqualTo(REJECT);
394+
// The signature was actually verified at gossip time, giving a REJECT verdict that penalises
395+
// the sender.
396+
verify(signatureVerifier).verify(anyList(), any(Bytes.class), any());
397+
}
398+
399+
@Test
400+
public void shouldVerifyAndCacheSignatureForValidFutureSlotAttestation() {
401+
// A valid future-slot attestation is still only deferred (SAVE_FOR_FUTURE), but its signature
402+
// is verified and cached here so the fork choice path does not re-verify it.
403+
final StateAndBlockSummary blockAndState = storageSystem.getChainHead();
404+
final Attestation attestation = attestationGenerator.validAttestation(blockAndState, ONE);
405+
assertThat(attestation.getData().getSlot()).isEqualTo(ONE);
406+
chainUpdater.setCurrentSlot(ZERO);
407+
408+
final ValidatableAttestation validatableAttestation =
409+
ValidatableAttestation.fromNetwork(
410+
spec,
411+
attestation,
412+
spec.computeSubnetForAttestation(blockAndState.getState(), attestation));
413+
414+
assertThat(validator.validate(validatableAttestation).join().code()).isEqualTo(SAVE_FOR_FUTURE);
415+
assertThat(validatableAttestation.isValidIndexedAttestation()).isTrue();
416+
}
417+
418+
@Test
419+
public void shouldDeferFutureSlotAttestationOnWrongSubnetWhenSignatureIsValid() {
420+
// A future-slot attestation is only rejected for an invalid signature; other gossip failures
421+
// (here, the wrong subnet) defer rather than penalise the sender. Contrast with
422+
// shouldRejectAttestationsSentOnTheWrongSubnet, which rejects an in-window attestation.
423+
final StateAndBlockSummary blockAndState = storageSystem.getChainHead();
424+
final Attestation attestation = attestationGenerator.validAttestation(blockAndState, ONE);
425+
assertThat(attestation.getData().getSlot()).isEqualTo(ONE);
426+
final int subnetId = spec.computeSubnetForAttestation(blockAndState.getState(), attestation);
427+
chainUpdater.setCurrentSlot(ZERO);
428+
429+
assertThat(
430+
validator
431+
.validate(ValidatableAttestation.fromNetwork(spec, attestation, subnetId + 1))
432+
.join()
433+
.code())
434+
.isEqualTo(SAVE_FOR_FUTURE);
435+
}
367436
}

networking/p2p/src/main/java/tech/pegasys/teku/networking/p2p/libp2p/LibP2PNetwork.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ public class LibP2PNetwork implements P2PNetwork<Peer> {
4848
private static final Logger LOG = LogManager.getLogger();
4949
static final int REMOTE_OPEN_STREAMS_RATE_LIMIT = 256;
5050
static final int REMOTE_PARALLEL_OPEN_STREAMS_COUNT_LIMIT = 256;
51+
static final int MAXIMUM_ACTIVE_INBOUND_RPC_STREAMS = 128;
5152

5253
private final PrivKey privKey;
5354
private final NodeId nodeId;

networking/p2p/src/main/java/tech/pegasys/teku/networking/p2p/libp2p/LibP2PNetworkBuilder.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
package tech.pegasys.teku.networking.p2p.libp2p;
1515

16+
import static tech.pegasys.teku.networking.p2p.libp2p.LibP2PNetwork.MAXIMUM_ACTIVE_INBOUND_RPC_STREAMS;
1617
import static tech.pegasys.teku.networking.p2p.libp2p.LibP2PNetwork.REMOTE_OPEN_STREAMS_RATE_LIMIT;
1718
import static tech.pegasys.teku.networking.p2p.libp2p.LibP2PNetwork.REMOTE_PARALLEL_OPEN_STREAMS_COUNT_LIMIT;
1819

@@ -49,6 +50,7 @@
4950
import tech.pegasys.teku.networking.p2p.libp2p.gossip.GossipTopicFilter;
5051
import tech.pegasys.teku.networking.p2p.libp2p.gossip.LibP2PGossipNetwork;
5152
import tech.pegasys.teku.networking.p2p.libp2p.gossip.LibP2PGossipNetworkBuilder;
53+
import tech.pegasys.teku.networking.p2p.libp2p.rpc.InboundRpcStreamLimiter;
5254
import tech.pegasys.teku.networking.p2p.libp2p.rpc.RpcHandler;
5355
import tech.pegasys.teku.networking.p2p.network.P2PNetwork;
5456
import tech.pegasys.teku.networking.p2p.network.PeerHandler;
@@ -289,7 +291,11 @@ private static boolean shouldListenOnQuic(
289291
}
290292

291293
protected List<? extends RpcHandler<?, ?, ?>> createRpcHandlers() {
292-
return rpcMethods.stream().map(m -> new RpcHandler<>(asyncRunner, m, metricsSystem)).toList();
294+
final InboundRpcStreamLimiter inboundRpcStreamLimiter =
295+
new InboundRpcStreamLimiter(MAXIMUM_ACTIVE_INBOUND_RPC_STREAMS);
296+
return rpcMethods.stream()
297+
.map(m -> new RpcHandler<>(asyncRunner, m, metricsSystem, inboundRpcStreamLimiter))
298+
.toList();
293299
}
294300

295301
protected LibP2PGossipNetwork createGossipNetwork() {
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/*
2+
* Copyright Consensys Software Inc., 2026
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
5+
* the License. You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
10+
* an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
11+
* specific language governing permissions and limitations under the License.
12+
*/
13+
14+
package tech.pegasys.teku.networking.p2p.libp2p.rpc;
15+
16+
import java.util.concurrent.atomic.AtomicInteger;
17+
18+
/** Limits the number of active inbound RPC streams across every transport and connection. */
19+
public class InboundRpcStreamLimiter {
20+
21+
private final int maximumActiveStreams;
22+
private final AtomicInteger activeStreams = new AtomicInteger();
23+
24+
public InboundRpcStreamLimiter(final int maximumActiveStreams) {
25+
if (maximumActiveStreams < 1) {
26+
throw new IllegalArgumentException("Maximum active inbound RPC streams must be positive");
27+
}
28+
this.maximumActiveStreams = maximumActiveStreams;
29+
}
30+
31+
public boolean tryAcquire() {
32+
int currentActiveStreams;
33+
do {
34+
currentActiveStreams = activeStreams.get();
35+
if (currentActiveStreams >= maximumActiveStreams) {
36+
return false;
37+
}
38+
} while (!activeStreams.compareAndSet(currentActiveStreams, currentActiveStreams + 1));
39+
return true;
40+
}
41+
42+
public void release() {
43+
final int remainingStreams = activeStreams.decrementAndGet();
44+
if (remainingStreams < 0) {
45+
activeStreams.incrementAndGet();
46+
throw new IllegalStateException("Released an inbound RPC stream that was not acquired");
47+
}
48+
}
49+
50+
public int getMaximumActiveStreams() {
51+
return maximumActiveStreams;
52+
}
53+
54+
int getActiveStreams() {
55+
return activeStreams.get();
56+
}
57+
}

networking/p2p/src/main/java/tech/pegasys/teku/networking/p2p/libp2p/rpc/RpcHandler.java

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ public class RpcHandler<
7070

7171
private final AsyncRunner asyncRunner;
7272
private final RpcMethod<TOutgoingHandler, TRequest, TRespHandler> rpcMethod;
73+
private final InboundRpcStreamLimiter inboundRpcStreamLimiter;
7374

7475
final Counter rpcRequestsTotalCounter;
7576
final Counter rpcRequestsFailedCounter;
@@ -78,9 +79,11 @@ public class RpcHandler<
7879
public RpcHandler(
7980
final AsyncRunner asyncRunner,
8081
final RpcMethod<TOutgoingHandler, TRequest, TRespHandler> rpcMethod,
81-
final MetricsSystem metricsSystem) {
82+
final MetricsSystem metricsSystem,
83+
final InboundRpcStreamLimiter inboundRpcStreamLimiter) {
8284
this.asyncRunner = asyncRunner;
8385
this.rpcMethod = rpcMethod;
86+
this.inboundRpcStreamLimiter = inboundRpcStreamLimiter;
8487

8588
rpcRequestsTotalCounter =
8689
metricsSystem.createCounter(
@@ -206,13 +209,37 @@ public SafeFuture<Controller<TOutgoingHandler>> initChannel(
206209
final Connection connection = streamChannel.getConnection();
207210
final NodeId nodeId = new LibP2PNodeId(connection.secureSession().getRemoteId());
208211

209-
final Controller<TOutgoingHandler> controller = new Controller<>(nodeId, streamChannel);
210-
if (!channel.isInitiator()) {
211-
controller.setIncomingRequestHandler(
212-
rpcMethod.createIncomingRequestHandler(selectedProtocol));
212+
final boolean isIncomingStream = !channel.isInitiator();
213+
if (isIncomingStream && !inboundRpcStreamLimiter.tryAcquire()) {
214+
LOG.debug(
215+
"Rejecting inbound RPC stream for protocol {} from {} because the global active stream limit of {} has been reached",
216+
selectedProtocol,
217+
nodeId,
218+
inboundRpcStreamLimiter.getMaximumActiveStreams());
219+
ignoreFuture(streamChannel.close());
220+
return SafeFuture.failedFuture(
221+
new IllegalStateException("Maximum active inbound RPC streams reached"));
222+
}
223+
224+
try {
225+
final Controller<TOutgoingHandler> controller =
226+
new Controller<>(
227+
nodeId,
228+
streamChannel,
229+
isIncomingStream ? inboundRpcStreamLimiter::release : () -> {});
230+
if (isIncomingStream) {
231+
controller.setIncomingRequestHandler(
232+
rpcMethod.createIncomingRequestHandler(selectedProtocol));
233+
}
234+
channel.pushHandler(controller);
235+
return controller.activeFuture;
236+
} catch (final Throwable t) {
237+
if (isIncomingStream) {
238+
inboundRpcStreamLimiter.release();
239+
}
240+
ignoreFuture(streamChannel.close());
241+
return SafeFuture.failedFuture(t);
213242
}
214-
channel.pushHandler(controller);
215-
return controller.activeFuture;
216243
}
217244

218245
static class Controller<TOutgoingHandler extends RpcRequestHandler>
@@ -221,16 +248,19 @@ static class Controller<TOutgoingHandler extends RpcRequestHandler>
221248

222249
private final NodeId nodeId;
223250
private final Stream p2pStream;
251+
private final Runnable releaseStreamPermit;
224252
private Optional<TOutgoingHandler> outgoingRequestHandler = Optional.empty();
225253
private Optional<RpcRequestHandler> rpcRequestHandler = Optional.empty();
226254
private RpcStream rpcStream;
227255
private boolean readCompleted = false;
228256

229257
protected final SafeFuture<Controller<TOutgoingHandler>> activeFuture = new SafeFuture<>();
230258

231-
private Controller(final NodeId nodeId, final Stream p2pStream) {
259+
private Controller(
260+
final NodeId nodeId, final Stream p2pStream, final Runnable releaseStreamPermit) {
232261
this.nodeId = nodeId;
233262
this.p2pStream = p2pStream;
263+
this.releaseStreamPermit = releaseStreamPermit;
234264
}
235265

236266
@Override
@@ -321,6 +351,7 @@ private void onChannelClosed() {
321351
runHandler(h -> h.closed(nodeId, rpcStream));
322352
} finally {
323353
rpcRequestHandler = Optional.empty();
354+
releaseStreamPermit.run();
324355
}
325356
}
326357

0 commit comments

Comments
 (0)