Changes in forward sync constants

CHANGELOG.md

@@ -7,7 +7,7 @@
 ## Unreleased Changes
 
 ### Breaking Changes
-`--Xvalidators-builder-registration-default-gas-limit` is removed in favour of `--validators-builder-registration-default-gas-limit`
+- `--Xvalidators-builder-registration-default-gas-limit` is removed in favour of `--validators-builder-registration-default-gas-limit`
 
 ### Additions and Improvements
 - Default the gas limit to 36 million for externally produced blocks

beacon/sync/src/main/java/tech/pegasys/teku/beacon/sync/DefaultSyncServiceFactory.java

@@ -196,6 +196,7 @@ protected ForwardSyncService createForwardSyncService() {
               syncConfig.getForwardSyncBatchSize(),
               syncConfig.getForwardSyncMaxPendingBatches(),
               syncConfig.getForwardSyncMaxBlocksPerMinute(),
+              syncConfig.getForwardSyncMaxBlobSidecarsPerMinute(),
               spec);
     } else {
       LOG.info("Using single peer sync");

beacon/sync/src/main/java/tech/pegasys/teku/beacon/sync/SyncConfig.java

@@ -20,10 +20,11 @@ public class SyncConfig {
   public static final boolean DEFAULT_MULTI_PEER_SYNC_ENABLED = true;
   public static final boolean DEFAULT_RECONSTRUCT_HISTORIC_STATES_ENABLED = false;
   public static final boolean DEFAULT_FETCH_ALL_HISTORIC_BLOCKS = true;
-  public static final int DEFAULT_FORWARD_SYNC_BATCH_SIZE = 50;
-  public static final int DEFAULT_HISTORICAL_SYNC_BATCH_SIZE = 50;
+  public static final int DEFAULT_FORWARD_SYNC_BATCH_SIZE = 25;
+  public static final int DEFAULT_HISTORICAL_SYNC_BATCH_SIZE = 25;
   public static final int DEFAULT_FORWARD_SYNC_MAX_PENDING_BATCHES = 5;
   public static final int DEFAULT_FORWARD_SYNC_MAX_BLOCKS_PER_MINUTE = 500;
+  public static final int DEFAULT_FORWARD_SYNC_MAX_BLOB_SIDECARS_PER_MINUTE = 1250;
 
   private final boolean isEnabled;
   private final boolean isMultiPeerSyncEnabled;
@@ -33,6 +34,7 @@ public class SyncConfig {
   private final int forwardSyncBatchSize;
   private final int forwardSyncMaxPendingBatches;
   private final int forwardSyncMaxBlocksPerMinute;
+  private final int forwardSyncMaxBlobSidecarsPerMinute;
 
   private SyncConfig(
       final boolean isEnabled,
@@ -42,7 +44,8 @@ private SyncConfig(
       final int historicalSyncBatchSize,
       final int forwardSyncBatchSize,
       final int forwardSyncMaxPendingBatches,
-      final int forwardSyncMaxBlocksPerMinute) {
+      final int forwardSyncMaxBlocksPerMinute,
+      final int forwardSyncMaxBlobSidecarsPerMinute) {
     this.isEnabled = isEnabled;
     this.isMultiPeerSyncEnabled = isMultiPeerSyncEnabled;
     this.reconstructHistoricStatesEnabled = reconstructHistoricStatesEnabled;
@@ -51,6 +54,7 @@ private SyncConfig(
     this.forwardSyncBatchSize = forwardSyncBatchSize;
     this.forwardSyncMaxPendingBatches = forwardSyncMaxPendingBatches;
     this.forwardSyncMaxBlocksPerMinute = forwardSyncMaxBlocksPerMinute;
+    this.forwardSyncMaxBlobSidecarsPerMinute = forwardSyncMaxBlobSidecarsPerMinute;
   }
 
   public static Builder builder() {
@@ -89,6 +93,10 @@ public int getForwardSyncMaxBlocksPerMinute() {
     return forwardSyncMaxBlocksPerMinute;
   }
 
+  public int getForwardSyncMaxBlobSidecarsPerMinute() {
+    return forwardSyncMaxBlobSidecarsPerMinute;
+  }
+
   public static class Builder {
     private Boolean isEnabled;
     private Boolean isMultiPeerSyncEnabled = DEFAULT_MULTI_PEER_SYNC_ENABLED;
@@ -98,6 +106,8 @@ public static class Builder {
     private Integer forwardSyncBatchSize = DEFAULT_FORWARD_SYNC_BATCH_SIZE;
     private Integer forwardSyncMaxPendingBatches = DEFAULT_FORWARD_SYNC_MAX_PENDING_BATCHES;
     private Integer forwardSyncMaxBlocksPerMinute = DEFAULT_FORWARD_SYNC_MAX_BLOCKS_PER_MINUTE;
+    private Integer forwardSyncMaxBlobSidecarsPerMinute =
+        DEFAULT_FORWARD_SYNC_MAX_BLOB_SIDECARS_PER_MINUTE;
 
     private Builder() {}
 
@@ -111,7 +121,8 @@ public SyncConfig build() {
           historicalSyncBatchSize,
           forwardSyncBatchSize,
           forwardSyncMaxPendingBatches,
-          forwardSyncMaxBlocksPerMinute);
+          forwardSyncMaxBlocksPerMinute,
+          forwardSyncMaxBlobSidecarsPerMinute);
     }
 
     private void initMissingDefaults() {
@@ -163,6 +174,13 @@ public Builder forwardSyncMaxBlocksPerMinute(final Integer forwardSyncMaxBlocksP
       return this;
     }
 
+    public Builder forwardSyncMaxBlobSidecarsPerMinute(
+        final Integer forwardSyncMaxBlobSidecarsPerMinute) {
+      checkNotNull(forwardSyncMaxBlobSidecarsPerMinute);
+      this.forwardSyncMaxBlobSidecarsPerMinute = forwardSyncMaxBlobSidecarsPerMinute;
+      return this;
+    }
+
     public Builder reconstructHistoricStatesEnabled(
         final Boolean reconstructHistoricStatesEnabled) {
       checkNotNull(reconstructHistoricStatesEnabled);

beacon/sync/src/main/java/tech/pegasys/teku/beacon/sync/forward/multipeer/MultipeerSyncService.java

@@ -76,6 +76,7 @@ public static MultipeerSyncService create(
       final int batchSize,
       final int maxPendingBatches,
       final int maxBlocksPerMinute,
+      final int maxBlobSidecarsPerMinute,
       final Spec spec) {
     final EventThread eventThread = new AsyncRunnerEventThread("sync", asyncRunnerFactory);
     final SettableLabelledGauge targetChainCountGauge =
@@ -117,7 +118,8 @@ eventThread, blobSidecarManager, new PeerScoringConflictResolutionStrategy()),
             recentChainData.getSpec(),
             eventThread,
             p2pNetwork,
-            new SyncSourceFactory(asyncRunner, timeProvider, maxBlocksPerMinute, batchSize),
+            new SyncSourceFactory(
+                asyncRunner, timeProvider, maxBlocksPerMinute, maxBlobSidecarsPerMinute),
             finalizedTargetChains,
             nonfinalizedTargetChains);
     peerChainTracker.subscribeToTargetChainUpdates(syncController::onTargetChainsUpdated);

beacon/sync/src/main/java/tech/pegasys/teku/beacon/sync/forward/multipeer/chains/SyncSourceFactory.java

@@ -21,38 +21,40 @@
 import tech.pegasys.teku.networking.eth2.peers.Eth2Peer;
 import tech.pegasys.teku.networking.eth2.peers.SyncSource;
 import tech.pegasys.teku.spec.Spec;
+import tech.pegasys.teku.spec.SpecMilestone;
 
 public class SyncSourceFactory {
 
   private final AsyncRunner asyncRunner;
   private final TimeProvider timeProvider;
-  private final Map<Eth2Peer, SyncSource> syncSourcesByPeer = new HashMap<>();
   private final int maxBlocksPerMinute;
-  private final int batchSize;
+  private final int maxBlobSidecarsPerMinute;
+
+  private final Map<Eth2Peer, SyncSource> syncSourcesByPeer = new HashMap<>();
 
   public SyncSourceFactory(
       final AsyncRunner asyncRunner,
       final TimeProvider timeProvider,
       final int maxBlocksPerMinute,
-      final int batchSize) {
+      final int maxBlobSidecarsPerMinute) {
     this.asyncRunner = asyncRunner;
     this.timeProvider = timeProvider;
     this.maxBlocksPerMinute = maxBlocksPerMinute;
-    this.batchSize = batchSize;
+    this.maxBlobSidecarsPerMinute = maxBlobSidecarsPerMinute;
   }
 
   public SyncSource getOrCreateSyncSource(final Eth2Peer peer, final Spec spec) {
-    // Limit request rate to just a little under what we'd accept
-    final int maxBlocksPerMinute = this.maxBlocksPerMinute - batchSize - 1;
-    final Optional<Integer> maxBlobSidecarsPerMinute =
-        spec.getMaxBlobsPerBlockForHighestMilestone()
-            .map(maxBlobsPerBlock -> maxBlocksPerMinute * maxBlobsPerBlock);
-
     return syncSourcesByPeer.computeIfAbsent(
         peer,
         source ->
             new ThrottlingSyncSource(
-                asyncRunner, timeProvider, source, maxBlocksPerMinute, maxBlobSidecarsPerMinute));
+                asyncRunner,
+                timeProvider,
+                source,
+                maxBlocksPerMinute,
+                spec.isMilestoneSupported(SpecMilestone.DENEB)
+                    ? Optional.of(maxBlobSidecarsPerMinute)
+                    : Optional.empty()));
   }
 
   public void onPeerDisconnected(final Eth2Peer peer) {

teku/src/main/java/tech/pegasys/teku/cli/options/P2POptions.java

@@ -255,13 +255,25 @@ The network interface(s) on which the node listens for P2P communication.
       SyncConfig.DEFAULT_FORWARD_SYNC_MAX_PENDING_BATCHES;
 
   @Option(
-      names = {"--Xp2p-sync-rate-limit"},
+      names = {"--Xp2p-sync-blocks-rate-limit"},
       paramLabel = "<NUMBER>",
       showDefaultValue = Visibility.ALWAYS,
-      description = "Number of objects being requested per minute to a single peer, while syncing.",
+      description = "Number of blocks being requested per minute to a single peer, while syncing.",
       hidden = true,
       arity = "1")
-  private Integer forwardSyncRateLimit = SyncConfig.DEFAULT_FORWARD_SYNC_MAX_BLOCKS_PER_MINUTE;
+  private Integer forwardSyncBlocksRateLimit =
+      SyncConfig.DEFAULT_FORWARD_SYNC_MAX_BLOCKS_PER_MINUTE;
+
+  @Option(
+      names = {"--Xp2p-sync-blob-sidecars-rate-limit"},
+      paramLabel = "<NUMBER>",
+      showDefaultValue = Visibility.ALWAYS,
+      description =
+          "Number of blob sidecars being requested per minute to a single peer, while syncing.",
+      hidden = true,
+      arity = "1")
+  private Integer forwardSyncBlobSidecarsRateLimit =
+      SyncConfig.DEFAULT_FORWARD_SYNC_MAX_BLOB_SIDECARS_PER_MINUTE;
 
   @Option(
       names = {"--p2p-subscribe-all-subnets-enabled"},
@@ -286,7 +298,8 @@ The network interface(s) on which the node listens for P2P communication.
       names = {"--Xpeer-rate-limit"},
       paramLabel = "<NUMBER>",
       description =
-          "The number of requested objects per peer to allow per minute before disconnecting the peer.",
+          "The number of requested blocks/blobs per peer to allow per minute before disconnecting the peer.\n"
+              + "NOTE: the actual size for the allowed blobs per peer per minute will be `maxBlobsPerBlock` times the value of this parameter.",
       arity = "1",
       hidden = true)
   private Integer peerRateLimit = P2PConfig.DEFAULT_PEER_RATE_LIMIT;
@@ -490,7 +503,8 @@ public void configure(final TekuConfiguration.Builder builder) {
             s ->
                 s.isMultiPeerSyncEnabled(multiPeerSyncEnabled)
                     .historicalSyncBatchSize(historicalSyncBatchSize)
-                    .forwardSyncMaxBlocksPerMinute(forwardSyncRateLimit)
+                    .forwardSyncMaxBlocksPerMinute(forwardSyncBlocksRateLimit)
+                    .forwardSyncMaxBlobSidecarsPerMinute(forwardSyncBlobSidecarsRateLimit)
                     .forwardSyncBatchSize(forwardSyncBatchSize)
                     .forwardSyncMaxPendingBatches(forwardSyncMaxPendingBatches));
 

teku/src/test/java/tech/pegasys/teku/cli/options/P2POptionsTest.java

@@ -67,6 +67,7 @@ public void shouldReadFromConfigurationFile() {
     assertThat(syncConfig.getForwardSyncBatchSize()).isEqualTo(103);
     assertThat(syncConfig.getForwardSyncMaxPendingBatches()).isEqualTo(8);
     assertThat(syncConfig.getForwardSyncMaxBlocksPerMinute()).isEqualTo(80);
+    assertThat(syncConfig.getForwardSyncMaxBlobSidecarsPerMinute()).isEqualTo(120);
   }
 
   @Test
@@ -246,12 +247,19 @@ public void forwardSyncMaxPendingBatches_shouldBeSettable() {
   }
 
   @Test
-  public void forwardSyncRateLimit_shouldBeSettable() {
+  public void forwardSyncBlocksRateLimit_shouldBeSettable() {
     TekuConfiguration tekuConfiguration =
-        getTekuConfigurationFromArguments("--Xp2p-sync-rate-limit", "10");
+        getTekuConfigurationFromArguments("--Xp2p-sync-blocks-rate-limit", "10");
     assertThat(tekuConfiguration.sync().getForwardSyncMaxBlocksPerMinute()).isEqualTo(10);
   }
 
+  @Test
+  public void forwardSyncBlobSidecarsRateLimit_shouldBeSettable() {
+    TekuConfiguration tekuConfiguration =
+        getTekuConfigurationFromArguments("--Xp2p-sync-blob-sidecars-rate-limit", "10");
+    assertThat(tekuConfiguration.sync().getForwardSyncMaxBlobSidecarsPerMinute()).isEqualTo(10);
+  }
+
   @Test
   public void forwardSyncBatchSize_greaterThanMessageSizeShouldThrowException() {
     assertThatThrownBy(() -> createConfigBuilder().sync(s -> s.forwardSyncBatchSize(3000)).build())

teku/src/test/resources/P2POptions_config.yaml

@@ -16,4 +16,5 @@ Xpeer-request-limit: 101
 Xp2p-historical-sync-batch-size: 102
 Xp2p-sync-batch-size: 103
 Xp2p-sync-max-pending-batches: 8
-Xp2p-sync-rate-limit: 80
+Xp2p-sync-blocks-rate-limit: 80
+Xp2p-sync-blob-sidecars-rate-limit: 120