Skip to content

fix: assert that EC finalized tipset height is never negative in v2 APIs #13180

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

masih
Copy link
Member

@masih masih commented Jun 17, 2025

Fix an edge case where if the head epoch is below the chain finality policy (defaulting to 900) EC finalized head height would become negative.

Fixes: #13167

Fix an edge case where if the head epoch is below the chain finality
policy (defaulting to 900) EC finalized head height would become
negative.

Fixes: #13167
@Copilot Copilot AI review requested due to automatic review settings June 17, 2025 14:36
@masih masih added the skip/changelog This change does not require CHANGELOG.md update label Jun 17, 2025
@github-project-automation github-project-automation bot moved this to 📌 Triage in FilOz Jun 17, 2025
@masih masih self-assigned this Jun 17, 2025
Copy link
Contributor

@Copilot 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

This pull request fixes an edge case where the EC finalized tipset height could become negative by ensuring it is set to a minimum of zero.

  • Updated the calculation of finalizedHeight using max(0, head.Height()-policy.ChainFinality)
  • Prevents negative tipset height in v2 APIs
Comments suppressed due to low confidence (1)

node/impl/full/chain_v2.go:156

  • Ensure that the function 'max' is defined for integer comparisons. If not, consider implementing an integer-specific max function to avoid potential type issues.
	finalizedHeight := max(0, head.Height()-policy.ChainFinality)

@masih masih requested a review from Kubuxu June 17, 2025 14:36
@masih masih moved this from 📌 Triage to 🔎 Awaiting Review in FilOz Jun 17, 2025
@BigLep BigLep added this to F3 Jun 19, 2025
@github-project-automation github-project-automation bot moved this to Todo in F3 Jun 19, 2025
@BigLep BigLep moved this from Todo to In review in F3 Jun 19, 2025
@github-project-automation github-project-automation bot moved this from 🔎 Awaiting Review to ✔️ Approved by reviewer in FilOz Jun 19, 2025
@rvagg
Copy link
Member

rvagg commented Jun 19, 2025

oh, v2 tests are failing with this, TestAPIV2_ThroughRPC/ChainGetTipSet/height_with_anchor_to_latest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip/changelog This change does not require CHANGELOG.md update
Projects
Status: In review
Status: ✔️ Approved by reviewer
Development

Successfully merging this pull request may close these issues.

API v2 request to Filecoin.GetChainTipset tipset with safe selector results in negative height.
2 participants