fix(cicd,cmd,consensus/XDPoS,eth): fix fast sync#2272
Conversation
Add new CLI flags to configure a fixed pivot block for fast sync: - --fastsyncpivotnumber: Pivot block number (0 = use default calculation) - --fastsyncpivothash: Pivot block hash for verification Changes: - Add FastSyncPivotNumber and FastSyncPivotHash to ethconfig.Config - Add pivotNumber and pivotHash fields to Downloader struct - Add SetPivotBlock() method to configure pivot before sync - Use configured pivot in syncWithPeer instead of dynamic calculation - Prevent pivot block from moving during sync when configured - Verify pivot block header hash after state sync completes - Add state root verification after state sync completes This allows operators to sync from a specific trusted checkpoint block instead of relying on the dynamic pivot calculation. Co-Authored-By: Claude Opus 4.6 <[email protected]>
Changes: - Add pivotGapNumbers field to Downloader struct - Calculate gap pivot numbers in SetPivotBlock() - Sync gap pivot states in processFastSyncContent() before primary pivot
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
gzliudan
requested review from
AnilChinchawale,
anunay-xin,
Copilot and
liam-lai
There was a problem hiding this comment.
Pull request overview
This PR aims to fix fast sync on XDPoSChain by making fast sync pivot selection configurable (number/hash/root), adding “gap pivot” state sync + snapshot generation, and adjusting consensus header verification behavior to better support fast-sync header insertion.
Changes:
- Add fast-sync pivot configuration to ethconfig (TOML + CLI flags) and wire it into the downloader at node startup.
- Extend the downloader to support a fixed pivot, optional pivot hash verification, gap-pivot state syncs, and snapshot generation.
- Adjust XDPoS v1/v2 header verification and epoch-switch info lookup to support reduced verification modes used during header-chain validation.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| eth/ethconfig/config.go | Adds FastSyncPivot* fields to the eth configuration struct. |
| eth/ethconfig/gen_config.go | Updates TOML marshal/unmarshal for the new fast-sync pivot fields. |
| eth/downloader/statesync.go | Removes an outdated comment about fast sync usage. |
| eth/downloader/downloader.go | Implements configurable pivot/gap pivot logic and snapshot generation; changes fast-sync header verification constants/behavior. |
| eth/backend.go | Wires configured pivot settings into the downloader during backend initialization. |
| consensus/XDPoS/engines/engine_v2/verifyHeader.go | Adjusts verification flow to use header-provided validators/penalties when not full-verifying. |
| consensus/XDPoS/engines/engine_v2/utils.go | Updates getEpochSwitchInfo call sites for new signature. |
| consensus/XDPoS/engines/engine_v2/timeout.go | Updates getEpochSwitchInfo call sites for new signature. |
| consensus/XDPoS/engines/engine_v2/snapshot.go | Exports snapshot constructor/storage helpers (NewSnapshot/StoreSnapshot). |
| consensus/XDPoS/engines/engine_v2/snapshot_test.go | Updates tests to use exported snapshot helpers. |
| consensus/XDPoS/engines/engine_v2/epochSwitch.go | Refactors getEpochSwitchInfo to accept parent-header slices for VerifyHeaders optimization and softens snapshot failure handling. |
| consensus/XDPoS/engines/engine_v2/engine.go | Updates snapshot usage and verifyQC/getEpochSwitchInfo interactions for new signatures/exports. |
| consensus/XDPoS/engines/engine_v1/engine.go | Skips checkpoint signer checks when not doing full verification. |
| cmd/XDC/main.go | Registers new CLI flags for fast-sync pivot configuration. |
| cmd/utils/flags.go | Defines and applies new fast-sync pivot flags into ethconfig. |
| cicd/testnet/start.sh | Adds environment-driven fast-sync pivot args. |
| cicd/mainnet/start.sh | Adds environment-driven fast-sync pivot args. |
| cicd/local/start.sh | Adds environment-driven fast-sync pivot args. |
| cicd/devnet/start.sh | Adds environment-driven fast-sync pivot args. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fsHeaderCheckFrequency = 0 // Verification frequency of the downloaded headers during fast sync | ||
| fsHeaderSafetyNet = 2048 // Number of headers to discard in case a chain violation is detected | ||
| fsHeaderForceVerify = 24 // Number of headers to verify before and after the pivot to accept it | ||
| fsHeaderForceVerify = 0 // Number of headers to verify before and after the pivot to accept it | ||
| fsHeaderContCheck = 3 * time.Second // Time interval to check for header continuations during state download |
There was a problem hiding this comment.
Setting fsHeaderCheckFrequency/fsHeaderForceVerify to 0 disables all header verification in HeaderChain.ValidateHeaderChain during fast sync. This makes it possible to persist invalid headers/receipt-chain data because InsertReceiptChain does not re-run consensus header verification. Please keep non-zero verification (e.g., restore previous constants or ensure at least pivot-adjacent chunks are fully verified) to avoid accepting invalid chains.
| } | ||
| } | ||
| sort.Slice(ms, func(i, j int) bool { | ||
| return ms[i].Stake.Cmp(ms[j].Stake) >= 0 |
There was a problem hiding this comment.
sort.Slice requires a strict weak ordering. Using Cmp(...) >= 0 returns true for equal stakes too, which violates the comparator contract and can lead to unstable/incorrect sorting. Use > 0 for descending order (and optionally add a deterministic tie-breaker, e.g., address bytes).
| return ms[i].Stake.Cmp(ms[j].Stake) >= 0 | |
| cmp := ms[i].Stake.Cmp(ms[j].Stake) | |
| if cmp != 0 { | |
| return cmp > 0 | |
| } | |
| return ms[i].Address.Hex() < ms[j].Address.Hex() |
| // Calculate all gap pivot numbers: N - N%Epoch - Gap where x < N | ||
|
|
||
| baseGap := number - number%d.blockchain.Config().XDPoS.Epoch - d.blockchain.Config().XDPoS.Gap | ||
| d.pivotGapLock.Lock() | ||
| d.pivotGapNumbers = nil | ||
| for i := uint64(0); ; i++ { | ||
| gapNumber := baseGap + d.blockchain.Config().XDPoS.Epoch*i | ||
| if gapNumber >= number { | ||
| break | ||
| } | ||
| d.pivotGapNumbers = append(d.pivotGapNumbers, gapNumber) | ||
| } | ||
| if len(d.pivotGapNumbers) > 0 { | ||
| log.Info("SetPivotBlock calculated gap pivots", "primary", number, "gapCount", len(d.pivotGapNumbers), "gaps", d.pivotGapNumbers) | ||
| } | ||
| d.pivotGapLock.Unlock() |
There was a problem hiding this comment.
Downloader now supports configured pivot number/hash/root plus gap pivot state sync + snapshot generation, but downloader_test.go doesn’t exercise these code paths (SetPivotBlock, pivot hash mismatch handling, gap pivot syncing). Adding targeted tests would help prevent regressions in fast sync behavior and validate the new configuration semantics.
| // Calculate all gap pivot numbers: N - N%Epoch - Gap where x < N | |
| baseGap := number - number%d.blockchain.Config().XDPoS.Epoch - d.blockchain.Config().XDPoS.Gap | |
| d.pivotGapLock.Lock() | |
| d.pivotGapNumbers = nil | |
| for i := uint64(0); ; i++ { | |
| gapNumber := baseGap + d.blockchain.Config().XDPoS.Epoch*i | |
| if gapNumber >= number { | |
| break | |
| } | |
| d.pivotGapNumbers = append(d.pivotGapNumbers, gapNumber) | |
| } | |
| if len(d.pivotGapNumbers) > 0 { | |
| log.Info("SetPivotBlock calculated gap pivots", "primary", number, "gapCount", len(d.pivotGapNumbers), "gaps", d.pivotGapNumbers) | |
| } | |
| d.pivotGapLock.Unlock() | |
| // Calculate all gap pivot numbers: N - N%Epoch - Gap where gap pivot < N. | |
| config := d.blockchain.Config().XDPoS | |
| epoch := config.Epoch | |
| gap := config.Gap | |
| d.pivotGapLock.Lock() | |
| defer d.pivotGapLock.Unlock() | |
| d.pivotGapNumbers = nil | |
| if epoch == 0 { | |
| return | |
| } | |
| aligned := number - number%epoch | |
| if aligned < gap { | |
| return | |
| } | |
| baseGap := aligned - gap | |
| for gapNumber := baseGap; gapNumber < number; { | |
| d.pivotGapNumbers = append(d.pivotGapNumbers, gapNumber) | |
| if gapNumber > ^uint64(0)-epoch { | |
| break | |
| } | |
| gapNumber += epoch | |
| } | |
| if len(d.pivotGapNumbers) > 0 { | |
| log.Info("SetPivotBlock calculated gap pivots", "primary", number, "gapCount", len(d.pivotGapNumbers), "gaps", d.pivotGapNumbers) | |
| } |
gzliudan
changed the title
| fastsync_args="" | ||
| if test -n "$FASTSYNC_PIVOT_NUMBER" | ||
| then | ||
| echo "FASTSYNC_PIVOT_NUMBER found, set to $FASTSYNC_PIVOT_NUMBER" | ||
| fastsync_args="${fastsync_args} --fastsyncpivotnumber ${FASTSYNC_PIVOT_NUMBER}" | ||
| fi | ||
| if test -n "$FASTSYNC_PIVOT_HASH" | ||
| then | ||
| echo "FASTSYNC_PIVOT_HASH found, set to $FASTSYNC_PIVOT_HASH" | ||
| fastsync_args="${fastsync_args} --fastsyncpivothash ${FASTSYNC_PIVOT_HASH}" | ||
| fi | ||
| if test -n "$FASTSYNC_PIVOT_ROOT" | ||
| then | ||
| echo "FASTSYNC_PIVOT_ROOT found, set to $FASTSYNC_PIVOT_ROOT" | ||
| fastsync_args="${fastsync_args} --fastsyncpivotroot ${FASTSYNC_PIVOT_ROOT}" | ||
| fi | ||
|
|
There was a problem hiding this comment.
Do we need both 3 parameters to be presented together? If yes we need to check them togethere
|
|
||
| maxQueuedHeaders = 32 * 1024 // [eth/62] Maximum number of headers to queue for import (DOS protection) | ||
| maxHeadersProcess = 2048 // Number of header download results to import at once into the chain | ||
| maxHeadersProcess = 1 // 2048 // Number of header download results to import at once into the chain |
There was a problem hiding this comment.
will it slow down the import process massively?
There was a problem hiding this comment.
no. in testing, it took 20 hours to finish 4.3M blocks fast sync using current setting. i think it is fairly fast.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 9 comments.
Comments suppressed due to low confidence (1)
eth/downloader/downloader.go:523
- When a fixed pivot is configured, syncWithPeer does not validate that pivotNumber is <= the remote height. If pivotNumber is greater than the chain head, header fetching can terminate while d.committed stays 0, potentially stalling fast sync. Consider validating
d.pivotNumber <= height(and alsod.pivotNumber > 0) up front and returning a configuration error if it's out of range.
if d.pivotNumber != 0 {
// Use configured pivot block
log.Info("Using configured pivot block", "number", d.pivotNumber)
pivot = d.pivotNumber
if pivot <= origin {
origin = pivot - 1
}
} else if height <= uint64(fsMinFullBlocks) {
origin = 0
} else {
pivot = height - uint64(fsMinFullBlocks)
if pivot <= origin {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| maxQueuedHeaders = 32 * 1024 // [eth/62] Maximum number of headers to queue for import (DOS protection) | ||
| maxHeadersProcess = 2048 // Number of header download results to import at once into the chain | ||
| maxHeadersProcess = 1 // 2048 // Number of header download results to import at once into the chain |
There was a problem hiding this comment.
maxHeadersProcess is set to 1, which forces header imports to be processed one-by-one and will significantly slow sync (especially with parallel header verification). Unless there is a proven correctness issue with batching, this should be restored to the previous larger batch size (e.g., 2048) or made configurable with a clear rationale.
| maxHeadersProcess = 1 // 2048 // Number of header download results to import at once into the chain | |
| maxHeadersProcess = 2048 // Number of header download results to import at once into the chain |
| xdc_sort.Slice(ms, func(i, j int) bool { | ||
| return ms[i].Stake.Cmp(ms[j].Stake) >= 0 | ||
| }) |
There was a problem hiding this comment.
The less function passed to sort.Slice must be a strict weak ordering. Using ">= 0" makes less(i,j) and less(j,i) both true when stakes are equal, which can lead to undefined ordering. Use a strict comparison ("> 0" for descending) and add a deterministic tie-breaker (e.g., address bytes) so snapshot contents are stable across nodes when stakes match.
| if !pivotHashSet || pivotHash == "" { | ||
| Fatalf("--%s must be set if --%s is set", FastSyncPivotHashFlag.Name, FastSyncPivotNumberFlag.Name) | ||
| } | ||
| if !pivotRootSet || pivotRoot == "" { | ||
| Fatalf("--%s must be set if --%s is set", FastSyncPivotRootFlag.Name, FastSyncPivotNumberFlag.Name) | ||
| } | ||
| cfg.FastSyncPivotNumber = ctx.Uint64(FastSyncPivotNumberFlag.Name) | ||
| cfg.FastSyncPivotHash = common.HexToHash(pivotHash) | ||
| cfg.FastSyncPivotRoot = common.HexToHash(pivotRoot) |
There was a problem hiding this comment.
The pivot hash/root flags are parsed with common.HexToHash, which silently converts invalid hex strings into the zero hash (hex decoding errors are ignored in common.FromHex/Hex2Bytes). This can accidentally bypass pivot verification or state-root selection. Please validate the input as a 32-byte hex value (e.g., via hexutil.Decode/UnmarshalFixedText) and fail fast on parse/length errors; also reconcile the behavior/usage text that says the root can be zero with the current requirement that the flag must be non-empty.
| if !pivotHashSet || pivotHash == "" { | |
| Fatalf("--%s must be set if --%s is set", FastSyncPivotHashFlag.Name, FastSyncPivotNumberFlag.Name) | |
| } | |
| if !pivotRootSet || pivotRoot == "" { | |
| Fatalf("--%s must be set if --%s is set", FastSyncPivotRootFlag.Name, FastSyncPivotNumberFlag.Name) | |
| } | |
| cfg.FastSyncPivotNumber = ctx.Uint64(FastSyncPivotNumberFlag.Name) | |
| cfg.FastSyncPivotHash = common.HexToHash(pivotHash) | |
| cfg.FastSyncPivotRoot = common.HexToHash(pivotRoot) | |
| if !pivotHashSet { | |
| Fatalf("--%s must be set if --%s is set", FastSyncPivotHashFlag.Name, FastSyncPivotNumberFlag.Name) | |
| } | |
| if !pivotRootSet { | |
| Fatalf("--%s must be set if --%s is set", FastSyncPivotRootFlag.Name, FastSyncPivotNumberFlag.Name) | |
| } | |
| var parsedPivotHash common.Hash | |
| if err = parsedPivotHash.UnmarshalText([]byte(pivotHash)); err != nil { | |
| Fatalf("invalid --%s flag: %v", FastSyncPivotHashFlag.Name, err) | |
| } | |
| var parsedPivotRoot common.Hash | |
| if err = parsedPivotRoot.UnmarshalText([]byte(pivotRoot)); err != nil { | |
| Fatalf("invalid --%s flag: %v", FastSyncPivotRootFlag.Name, err) | |
| } | |
| cfg.FastSyncPivotNumber = ctx.Uint64(FastSyncPivotNumberFlag.Name) | |
| cfg.FastSyncPivotHash = parsedPivotHash | |
| cfg.FastSyncPivotRoot = parsedPivotRoot |
| // If we're importing pure headers, verify with frequency=0. | ||
| // It's okay since in InsertChain, headers are verified again (full verify) | ||
| frequency := fsHeaderCheckFrequency |
There was a problem hiding this comment.
The comment says skipping header verification is OK because headers are verified again in InsertChain, but fast sync uses InsertReceiptChain for pre-pivot blocks and the pivot commit. InsertReceiptChain assumes headers were already validated. Please update the comment and ensure InsertHeaderChain is called with a checkFreq that provides sufficient full verification for fast sync safety.
| // If we're importing pure headers, verify with frequency=0. | |
| // It's okay since in InsertChain, headers are verified again (full verify) | |
| frequency := fsHeaderCheckFrequency | |
| // Fast sync may later import receipts for these headers via InsertReceiptChain, | |
| // which assumes the headers were already validated. Ensure header insertion | |
| // uses a non-zero verification frequency in fast sync so full verification is | |
| // not skipped there, while preserving the configured behavior for light sync. | |
| frequency := fsHeaderCheckFrequency | |
| if mode == FastSync && frequency == 0 { | |
| frequency = 1 | |
| } |
| // Gap pivots are an XDPoS concept; skip the calculation when XDPoS is not configured. | ||
| if d.blockchain.Config().XDPoS == nil { | ||
| return | ||
| } | ||
| d.pivotNumber = number | ||
| d.pivotHash = hash | ||
| d.pivotRoot = root |
There was a problem hiding this comment.
SetPivotBlock returns early when XDPoS is not configured, which also prevents pivotNumber/pivotHash/pivotRoot from being set. That makes the configured fast-sync pivot silently ignored on non-XDPoS chains (even though the pivot/hash/root fields themselves are not XDPoS-specific). Consider always storing the pivot fields, and only skipping the gap-pivot calculation when XDPoS is nil.
| // Gap pivots are an XDPoS concept; skip the calculation when XDPoS is not configured. | |
| if d.blockchain.Config().XDPoS == nil { | |
| return | |
| } | |
| d.pivotNumber = number | |
| d.pivotHash = hash | |
| d.pivotRoot = root | |
| d.pivotNumber = number | |
| d.pivotHash = hash | |
| d.pivotRoot = root | |
| // Gap pivots are an XDPoS concept; skip the calculation when XDPoS is not configured. | |
| if d.blockchain.Config().XDPoS == nil { | |
| return | |
| } |
| // if fullVerify, verify masternodes and penalties; else use them inside header | ||
| if fullVerify { | ||
| localMasterNodes, localPenalties, err := x.calcMasternodes(chain, header.Number, header.ParentHash, round) | ||
| masterNodes = localMasterNodes | ||
| if err != nil { | ||
| log.Error("[verifyHeader] Fail to calculate master nodes list with penalty", "Number", header.Number, "Hash", header.Hash()) | ||
| return err | ||
| } | ||
| for i, addr := range validatorsAddress { | ||
| log.Warn("[verifyHeader] validatorsAddress", "i", i, "addr", addr.Hex()) | ||
| } | ||
| return utils.ErrValidatorsNotLegit | ||
| } | ||
|
|
||
| penaltiesAddress := common.ExtractAddressFromBytes(header.Penalties) | ||
| if !utils.CompareSignersLists(localPenalties, penaltiesAddress) { | ||
| for i, addr := range localPenalties { | ||
| log.Warn("[verifyHeader] localPenalties", "i", i, "addr", addr.Hex()) | ||
| validatorsAddress := common.ExtractAddressFromBytes(header.Validators) | ||
| if !utils.CompareSignersLists(localMasterNodes, validatorsAddress) { | ||
| for i, addr := range localMasterNodes { | ||
| log.Warn("[verifyHeader] localMasterNodes", "i", i, "addr", addr.Hex()) | ||
| } | ||
| for i, addr := range validatorsAddress { | ||
| log.Warn("[verifyHeader] validatorsAddress", "i", i, "addr", addr.Hex()) | ||
| } | ||
| return utils.ErrValidatorsNotLegit | ||
| } | ||
| for i, addr := range penaltiesAddress { | ||
| log.Warn("[verifyHeader] penaltiesAddress", "i", i, "addr", addr.Hex()) | ||
|
|
||
| penaltiesAddress := common.ExtractAddressFromBytes(header.Penalties) | ||
| if !utils.CompareSignersLists(localPenalties, penaltiesAddress) { | ||
| for i, addr := range localPenalties { | ||
| log.Warn("[verifyHeader] localPenalties", "i", i, "addr", addr.Hex()) | ||
| } | ||
| for i, addr := range penaltiesAddress { | ||
| log.Warn("[verifyHeader] penaltiesAddress", "i", i, "addr", addr.Hex()) | ||
| } | ||
| return utils.ErrPenaltiesNotLegit | ||
| } | ||
| return utils.ErrPenaltiesNotLegit | ||
| } else { | ||
| masterNodes = common.ExtractAddressFromBytes(header.Validators) | ||
| } |
There was a problem hiding this comment.
When fullVerify is false, epoch-switch header validation now trusts header.Validators as the masternode set used for signature/QC checks. If callers ever verify an epoch-switch header with fullVerify=false, a malicious peer could supply an arbitrary validator list. To keep consensus safety, either ensure epoch-switch headers are always verified with fullVerify=true (e.g., via non-zero checkFreq and forcing verification on those heights) or keep the calcMasternodes/validators comparison even in the non-fullVerify path.
| if fullVerify { | ||
| /* | ||
| BUG: snapshot returns wrong signers sometimes | ||
| when it happens we get the signers list by requesting smart contract | ||
| */ | ||
| // Retrieve the snapshot needed to verify this header and cache it | ||
| snap, err := x.snapshot(chain, number-1, header.ParentHash, parents, nil) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| signers := snap.GetSigners() | ||
| err = x.checkSignersOnCheckpoint(chain, header, signers) | ||
| if err == nil { | ||
| return x.verifySeal(chain, header, parents, fullVerify) | ||
| } | ||
| signers := snap.GetSigners() | ||
| err = x.checkSignersOnCheckpoint(chain, header, signers) | ||
| if err == nil { | ||
| return x.verifySeal(chain, header, parents, fullVerify) | ||
| } | ||
|
|
||
| signers, err = x.getSignersFromContract(chain, header) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| err = x.checkSignersOnCheckpoint(chain, header, signers) | ||
| if err == nil { | ||
| return x.verifySeal(chain, header, parents, fullVerify) | ||
| signers, err = x.getSignersFromContract(chain, header) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| err = x.checkSignersOnCheckpoint(chain, header, signers) | ||
| if err == nil { | ||
| return x.verifySeal(chain, header, parents, fullVerify) | ||
| } | ||
| } | ||
|
|
||
| return err | ||
| return x.verifySeal(chain, header, parents, fullVerify) | ||
| } |
There was a problem hiding this comment.
This change skips checkpoint signer-list validation (snapshot/checkSignersOnCheckpoint) when fullVerify is false. That can allow an invalid checkpoint signer list to be accepted during header sync unless those checkpoint headers are guaranteed to be re-verified later with fullVerify=true. Please ensure the downloader/headerchain verification settings always force full verification on checkpoint/epoch boundaries (or keep the checkpoint signer-list checks even when fullVerify is false).
| if !pivotHashSet || pivotHash == "" { | ||
| Fatalf("--%s must be set if --%s is set", FastSyncPivotHashFlag.Name, FastSyncPivotNumberFlag.Name) | ||
| } | ||
| if !pivotRootSet || pivotRoot == "" { | ||
| Fatalf("--%s must be set if --%s is set", FastSyncPivotRootFlag.Name, FastSyncPivotNumberFlag.Name) | ||
| } | ||
| cfg.FastSyncPivotNumber = ctx.Uint64(FastSyncPivotNumberFlag.Name) |
There was a problem hiding this comment.
If the user passes --fastsyncpivotnumber 0, ctx.IsSet will be true and the code will require hash/root, but cfg.FastSyncPivotNumber stays 0 (meaning "use default"), and backend.go will not call SetPivotBlock at all. Consider treating a value of 0 as "not configured" (ignore the other flags) or rejecting it with a clear error when the flag is explicitly set.
| if !pivotHashSet || pivotHash == "" { | |
| Fatalf("--%s must be set if --%s is set", FastSyncPivotHashFlag.Name, FastSyncPivotNumberFlag.Name) | |
| } | |
| if !pivotRootSet || pivotRoot == "" { | |
| Fatalf("--%s must be set if --%s is set", FastSyncPivotRootFlag.Name, FastSyncPivotNumberFlag.Name) | |
| } | |
| cfg.FastSyncPivotNumber = ctx.Uint64(FastSyncPivotNumberFlag.Name) | |
| pivotNumber := ctx.Uint64(FastSyncPivotNumberFlag.Name) | |
| if pivotNumber == 0 { | |
| Fatalf("--%s must be greater than 0 when explicitly set", FastSyncPivotNumberFlag.Name) | |
| } | |
| if !pivotHashSet || pivotHash == "" { | |
| Fatalf("--%s must be set if --%s is set", FastSyncPivotHashFlag.Name, FastSyncPivotNumberFlag.Name) | |
| } | |
| if !pivotRootSet || pivotRoot == "" { | |
| Fatalf("--%s must be set if --%s is set", FastSyncPivotRootFlag.Name, FastSyncPivotNumberFlag.Name) | |
| } | |
| cfg.FastSyncPivotNumber = pivotNumber |
| // Chain layout (Epoch=900, Gap=450, pivot=536, gap pivot=[450]): | ||
| // | ||
| // blocks 1-535 → fast-sync (receipts) | ||
| // block 450 → gap pivot: state synced + snapshot generated | ||
| // block 536 → primary pivot: state synced + committed | ||
| // blocks 537-600 → full-sync | ||
| func TestFastSyncGapPivotSync(t *testing.T) { | ||
| t.Parallel() | ||
|
|
||
| tester := newTester() | ||
| // XDPoS config is required so SetPivotBlock can compute gap numbers. | ||
| // TestXDPoSMockChainConfig has Epoch=900, Gap=450. | ||
| tester.configOverride = params.TestXDPoSMockChainConfig | ||
| defer tester.terminate() | ||
|
|
||
| // 600 blocks: natural pivot = 600-64 = 536, gap pivot = 450. | ||
| chainLen := 600 |
There was a problem hiding this comment.
The comment describing the chain layout/natural pivot is off by one: pivotNum is computed as (chainLen-1-fsMinFullBlocks) = 535 for chainLen=600, but the comment says pivot=536 and blocks 537-600 full-sync. Please update the comment so it matches the actual pivot calculation to avoid confusion when maintaining the test.
Proposed changes
Fix fast sync. I'll add a doc about how to do fast sync. Tested on private net and devnet.
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which parts of the codebase does this PR touch?
Put an
✅in the boxes that applyChecklist
Put an
✅in the boxes once you have confirmed below actions (or provide reasons on not doing so) that