Skip to content

fix(ecfinality): account for null rounds in chain walk#13565

Merged
rvagg merged 3 commits intomasterfrom
rvagg/ecfinality-null-rounds
Mar 31, 2026
Merged

fix(ecfinality): account for null rounds in chain walk#13565
rvagg merged 3 commits intomasterfrom
rvagg/ecfinality-null-rounds

Conversation

@rvagg
Copy link
Copy Markdown
Member

@rvagg rvagg commented Mar 30, 2026

Include null round epochs as 0-block entries in the chain data fed to the FRC-0089 calculator, rather than silently skipping them. This aligns the implementation with the theoretical model where null rounds are epochs where the adversary gains ground while the honest chain doesn't grow. Also fixes a depth-to-height conversion problem which previously could return a shallower tipset than intended when null rounds were present between head and the finalized epoch.

Increase BisectHigh from 200 to 450 to accommodate degraded chains where null round inclusion pushes the threshold deeper (notably current calibnet which is near 200).

Thanks to @hanabi1224 for questioning the implementation on this point while doing ChainSafe/forest#6811, and @guy-goren for confirming that null's are statistically important to include.

Current mainnet shows no difference in the calculation because nulls are so rare, but the difference is ~5 epochs on calibnet where we're averaging 2.2 blocks per tipset and have a lot more null rounds.

Include null round epochs as 0-block entries in the chain data fed to
the FRC-0089 calculator, rather than silently skipping them. This
aligns the implementation with the theoretical model where null rounds
are epochs where the adversary gains ground while the honest chain
doesn't grow. Also fixes a depth-to-height conversion problem which
previously could return a shallower tipset than intended when null
rounds were present between head and the finalized epoch.

Increase BisectHigh from 200 to 450 to accommodate degraded chains
where null round inclusion pushes the threshold deeper (notably current
calibnet which is near 200).
Copilot AI review requested due to automatic review settings March 30, 2026 23:26
@github-project-automation github-project-automation Bot moved this to 📌 Triage in FilOz Mar 30, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the EC finality chain-walk logic to include null rounds as explicit 0-block epochs so the FRC-0089 calculator operates over a true epoch timeline (and depth maps cleanly to height), and expands the bisect search range to handle more degraded chains.

Changes:

  • Include null-round epochs as 0 entries when walking back the chain (both in lotus-shed finality-calculator and in the in-node ECFinalityCache).
  • Increase ecfinality.BisectHigh from 200 to 450 and update related documentation/comments.
  • Update changelog and adjust test comments accordingly.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
cmd/lotus-shed/finality.go Inserts 0-block epochs for null rounds during API chain walk; trims to fixed window and preserves epoch-to-depth correctness for output.
chain/ecfinality/cache.go Updates cache chain walk to include null rounds as 0 entries so returned depth aligns with epoch height differences.
chain/ecfinality/calculator.go Expands bisect search upper bound from 200 to 450.
chain/ecfinality/calculator_test.go Updates test comment to reflect new BisectHigh.
CHANGELOG.md Adds an UNRELEASED entry documenting the null-round handling fix and associated depth/height correction.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread chain/ecfinality/cache.go Outdated
Comment thread chain/ecfinality/calculator.go
@rvagg rvagg enabled auto-merge (squash) March 31, 2026 02:41
@rvagg rvagg merged commit ee062d6 into master Mar 31, 2026
98 checks passed
@rvagg rvagg deleted the rvagg/ecfinality-null-rounds branch March 31, 2026 04:31
@github-project-automation github-project-automation Bot moved this from 📌 Triage to 🎉 Done in FilOz Mar 31, 2026
rjan90 pushed a commit that referenced this pull request Mar 31, 2026
Include null round epochs as 0-block entries in the chain data fed to
the FRC-0089 calculator, rather than silently skipping them. This
aligns the implementation with the theoretical model where null rounds
are epochs where the adversary gains ground while the honest chain
doesn't grow. Also fixes a depth-to-height conversion problem which
previously could return a shallower tipset than intended when null
rounds were present between head and the finalized epoch.

Increase BisectHigh from 200 to 450 to accommodate degraded chains
where null round inclusion pushes the threshold deeper (notably current
calibnet which is near 200).
rjan90 pushed a commit that referenced this pull request Mar 31, 2026
Include null round epochs as 0-block entries in the chain data fed to
the FRC-0089 calculator, rather than silently skipping them. This
aligns the implementation with the theoretical model where null rounds
are epochs where the adversary gains ground while the honest chain
doesn't grow. Also fixes a depth-to-height conversion problem which
previously could return a shallower tipset than intended when null
rounds were present between head and the finalized epoch.

Increase BisectHigh from 200 to 450 to accommodate degraded chains
where null round inclusion pushes the threshold deeper (notably current
calibnet which is near 200).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🎉 Done

Development

Successfully merging this pull request may close these issues.

4 participants