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

Deprecate FastBlock Flag #6792

Merged
merged 6 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ public interface ISyncConfig : IConfig
[ConfigItem(Description = "In Fast sync mode, the min height threshold limit up to which the Full sync, if already on, stays on when the chain is behind the network head. If the limit is exceeded, it switches back to Fast sync. For regular usage scenarios, setting this value lower than 32 is not recommended as this can cause issues with chain reorgs. Note that the last 2 blocks are always processed in Full sync, so setting it lower than 2 has no effect.", DefaultValue = "8192")]
long? FastSyncCatchUpHeightDelta { get; set; }

[ConfigItem(Description = "Whether to first download blocks from the provided pivot number downwards in the Fast sync mode. This allows for parallelization of requests with many sync peers and with no need to worry about syncing a valid branch (syncing downwards to 0). You need to provide the pivot block number, hash, and total difficulty from a trusted source (e.g., Etherscan) and confirm with other sources if you want to change it.", DefaultValue = "false")]
[Obsolete]
[ConfigItem(Description = "Deprecated.", DefaultValue = "false", HiddenFromDocs = true)]
bool FastBlocks { get; set; }

[ConfigItem(Description = "Whether to make smaller requests, in Fast Blocks mode, to avoid Geth from disconnecting. On the Geth-heavy networks (e.g., Mainnet), it's a desired behavior while on Nethermind- or OpenEthereum-heavy networks (Goerli, Aura), it slows down the sync by a factor of ~4.", DefaultValue = "true")]
Expand Down
15 changes: 5 additions & 10 deletions src/Nethermind/Nethermind.Blockchain/Synchronization/SyncConfig.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// SPDX-FileCopyrightText: 2022 Demerzel Solutions Limited
// SPDX-License-Identifier: LGPL-3.0-only
using System;
using Nethermind.Config;
using Nethermind.Db;

Expand All @@ -13,10 +12,10 @@ public class SyncConfig : ISyncConfig
private bool _fastSync;

public static ISyncConfig Default { get; } = new SyncConfig();
public static ISyncConfig WithFullSyncOnly { get; } = new SyncConfig { FastSync = false, FastBlocks = false };
public static ISyncConfig WithFullSyncOnly { get; } = new SyncConfig { FastSync = false };
public static ISyncConfig WithFastSync { get; } = new SyncConfig { FastSync = true };
public static ISyncConfig WithFastBlocks { get; } = new SyncConfig { FastSync = true, FastBlocks = true };
public static ISyncConfig WithEth2Merge { get; } = new SyncConfig { FastSync = false, FastBlocks = false, BlockGossipEnabled = false };
public static ISyncConfig WithFastBlocks { get; } = new SyncConfig { FastSync = true };
Copy link
Contributor

Choose a reason for hiding this comment

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

Just delete this whole line

public static ISyncConfig WithEth2Merge { get; } = new SyncConfig { FastSync = false, BlockGossipEnabled = false };

public bool NetworkingEnabled { get; set; } = true;

Expand All @@ -27,13 +26,9 @@ public bool SynchronizationEnabled
}

public long? FastSyncCatchUpHeightDelta { get; set; } = 8192;

// maybe log warnign on set? Remove all references within this file.
// using a class method marked obsolete in reflections might not be possible.
// [ObsoleteAttribute("Fast blocks mode is dependent on and auto enabled in fast sync mode, please use FastSync instead.", false)]
public bool FastBlocks { get; set; }
bool ISyncConfig.FastBlocks { get; set; }
public bool UseGethLimitsInFastBlocks { get; set; } = true;
public bool FastSync { get => _fastSync || SnapSync; set => _fastSync = value; }
public bool FastSync { get => _fastSync || SnapSync; set => _fastSync = value; }
public bool DownloadHeadersInFastSync { get; set; } = true;
public bool DownloadBodiesInFastSync { get; set; } = true;
public bool DownloadReceiptsInFastSync { get; set; } = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,7 @@ private ReceiptsSyncFeed CreateFeed()
LimboLogs.Instance);
}

// maybe remove test completely(obsolete) (previous throws when fast block not enabled?
[Test]
[Obsolete("Fast Blocks mode is active when fast sync mode is active")]
public void Should_throw_when_fast_block_not_enabled()
{
_syncConfig = new SyncConfig { FastSync = false };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,8 @@ public void Best_state_is_head_if_there_is_suggested_block_without_state()
Assert.That(syncProgressResolver.FindBestFullState(), Is.EqualTo(head.Number));
}

// maybe delete(mark obsolete) this test, since fastblock is always used?
[Test]
public void Is_fast_block_finished_returns_true_when_no_fast_block_sync_is_used()
public void Is_fast_block_finished_returns_true_when_no_fast_sync_is_used()
{
IBlockTree blockTree = Substitute.For<IBlockTree>();
IStateReader stateReader = Substitute.For<IStateReader>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,9 @@ public BodiesSyncFeed(
_blocksDb = blocksDb ?? throw new ArgumentNullException(nameof(blocksDb));
_flushDbInterval = flushDbInterval;

// maybe remove fastblock(now replaced with fastsync) condition completely?
if (!_syncConfig.FastSync)
{
throw new InvalidOperationException(
"Entered fast sync with fast blocks mode without fast sync enabled in configuration.");
throw new InvalidOperationException("Entered fast bodies mode without fast sync enabled in configuration.");
}

_pivotNumber = -1; // First reset in `InitializeFeed`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,10 +179,9 @@ public HeadersSyncFeed(
_headersRequestSize = NethermindSyncLimits.MaxHeaderFetch;
}

// maybe remove fastblock(now replaced with fastsync) condition completely?
if (!_syncConfig.FastSync && !alwaysStartHeaderSync)
{
throw new InvalidOperationException("Entered fast sync with fast blocks mode without fast sync enabled in configuration.");
throw new InvalidOperationException("Entered fast headers mode without fast sync enabled in configuration.");
}

_historicalOverrides.TryGetValue(_blockTree.NetworkId, out _expectedDifficultyOverride);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ public ReceiptsSyncFeed(
_syncReport = syncReport ?? throw new ArgumentNullException(nameof(syncReport));
_blockTree = blockTree ?? throw new ArgumentNullException(nameof(blockTree));

// maybe remove fastblock(now replaced with fastsync) condition completely?
if (!_syncConfig.FastSync)
{
throw new InvalidOperationException("Entered fast blocks mode without fast blocks enabled in configuration.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,9 @@ public class MultiSyncModeSelector : ISyncModeSelector
private long _pivotNumber;
private bool FastSyncEnabled => _syncConfig.FastSync;
private bool SnapSyncEnabled => _syncConfig.SnapSync && !_isSnapSyncDisabledAfterAnyStateSync;
private bool FastBlocksEnabled => _syncConfig.FastSync; // maybe replace all reference with FastSyycEnabled?
private bool FastBodiesEnabled => FastBlocksEnabled && _syncConfig.DownloadBodiesInFastSync;
private bool FastReceiptsEnabled => FastBlocksEnabled && _syncConfig.DownloadReceiptsInFastSync;
private bool FastBlocksHeadersFinished => !FastBlocksEnabled || _syncProgressResolver.IsFastBlocksHeadersFinished();
private bool FastBodiesEnabled => FastSyncEnabled && _syncConfig.DownloadBodiesInFastSync;
private bool FastReceiptsEnabled => FastSyncEnabled && _syncConfig.DownloadReceiptsInFastSync;
private bool FastBlocksHeadersFinished => !FastSyncEnabled || _syncProgressResolver.IsFastBlocksHeadersFinished();
private bool FastBlocksBodiesFinished => !FastBodiesEnabled || _syncProgressResolver.IsFastBlocksBodiesFinished();
private bool FastBlocksReceiptsFinished => !FastReceiptsEnabled || _syncProgressResolver.IsFastBlocksReceiptsFinished();
private long FastSyncCatchUpHeightDelta => _syncConfig.FastSyncCatchUpHeightDelta ?? FastSyncLag;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,32 +83,18 @@ public long FindBestFullState()
return _blockTree.FindHeader(blockHash)?.TotalDifficulty == 0 ? null : _blockTree.FindHeader(blockHash)?.TotalDifficulty;
}

public bool IsFastBlocksHeadersFinished() => !IsFastBlocks() || (!_syncConfig.DownloadHeadersInFastSync ||
(_headersSyncFeed?.IsFinished == true));
public bool IsFastBlocksHeadersFinished() => !IsFastBlocks() || !_syncConfig.DownloadHeadersInFastSync || _headersSyncFeed?.IsFinished == true;

public bool IsFastBlocksBodiesFinished() => !IsFastBlocks() || (!_syncConfig.DownloadBodiesInFastSync ||
(_bodiesSyncFeed?.IsFinished == true));
public bool IsFastBlocksBodiesFinished() => !IsFastBlocks() || !_syncConfig.DownloadBodiesInFastSync || _bodiesSyncFeed?.IsFinished == true;

public bool IsFastBlocksReceiptsFinished() => !IsFastBlocks() || (!_syncConfig.DownloadReceiptsInFastSync ||
(_receiptsSyncFeed?.IsFinished == true));
public bool IsFastBlocksReceiptsFinished() => !IsFastBlocks() || !_syncConfig.DownloadReceiptsInFastSync || _receiptsSyncFeed?.IsFinished == true;

public bool IsSnapGetRangesFinished() => _snapSyncFeed?.IsFinished ?? true;

public void RecalculateProgressPointers() => _blockTree.RecalculateTreeLevels();

private bool IsFastBlocks()
{
// maybe eliminate the entire code refeferencing this function why? fast block depends on fastsync so this code might just be check that is now unnecessay!
// name isFastBlocks left in place for correlation with protocol specification.
bool isFastBlocks = _syncConfig.FastSync;
private bool IsFastBlocks() => _syncConfig.FastSync && _syncConfig.PivotNumberParsed != 0L; // if pivot number is 0 then it is equivalent to fast blocks disabled
LukaszRozmej marked this conversation as resolved.
Show resolved Hide resolved

// if pivot number is 0 then it is equivalent to fast blocks disabled
if (!isFastBlocks || _syncConfig.PivotNumberParsed == 0L)
{
return false;
}

return true;
}
}
}
Loading