fix(consensus): skip a height CommitBlock reports as already committed

[envestcc]

Description

blockchain/filedao/filedao.go and blockchain/blockchain.go each declared their own ErrInvalidTipHeight sentinel with the same message. The block store raises the filedao one when it refuses a block whose height is not tip + 1, and blockchain.CommitBlock propagates it verbatim — but callers compare against the blockchain one, so the comparison never matches.

In rolldposCtx.Commit that turns an expected outcome into an error path: if block sync has already committed height H and the consensus round then calls CommitBlock for its own block at H, the switch misses blockchain.ErrInvalidTipHeight (which returns true, nil), falls through to default, logs error when committing the block, and returns an error. The FSM then stays in its pre-commit state until the round TTL expires instead of recognising the height as done.

chainservice/builder.go:690 has the same comparison, but it is masked there because the preceding ValidateBlock returns the blockchain variable first.

There is no finality or state impact — the block store guard still refuses the second block. This is a robustness fix.

Fixes #5010

Changes

• blockchain.ErrInvalidTipHeight now aliases filedao.ErrInvalidTipHeight, so both names refer to a single sentinel and a height rejection from the block store matches what callers switch on. (blockchain already imports filedao; the reverse direction would be an import cycle.)
• rolldposCtx.Commit matches with errors.Is instead of errors.Cause equality, so the skip branch keeps working if the storage error is ever wrapped, and the skip intent is documented in a comment.

Tests

• TestCommitAlreadyCommittedHeight (consensus/scheme/rolldpos): drives the reported scenario end to end on a real chain — the round registers its own block at H, block sync commits a different block at H, then the round collects a majority of COMMIT endorsements and calls Commit. Asserts (true, nil) and that the block-sync block is still the tip. Fails on master with error when committing a block / invalid tip height.
• TestCommitBlockInvalidTipHeight (blockchain): pins the contract at the layer where it broke — a height rejection from the block store must satisfy errors.Is(err, blockchain.ErrInvalidTipHeight). Fails on master.

Full ./blockchain/... ./consensus/... ./chainservice/... run is green apart from TestBlockIndexerChecker_CheckIndexer, which fails identically on unmodified master.

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 90.90909% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.67%. Comparing base (436e4d1) to head (31e12d0).
⚠️ Report is 343 commits behind head on master.

Files with missing lines	Patch %	Lines
chainservice/builder.go	86.95%	3 Missing ⚠️

❌ Your project check has failed because the head coverage (62.67%) is below the target coverage (85.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #5014       +/-   ##
===========================================
- Coverage   74.83%   62.67%   -12.16%     
===========================================
  Files         378      502      +124     
  Lines       31624    50114    +18490     
===========================================
+ Hits        23666    31410     +7744     
- Misses       6747    14969     +8222     
- Partials     1211     3735     +2524     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[envestcc]

Thanks for the review. Addressed the two actionable points in 31e12d0.

codecov/patch (was 40%, 6 lines missing in rolldposctx.go) — real. The paused and generic-error branches of the commit switch were untouched by the one test. Factored the three cases onto a shared fixture and added TestCommitPausedChain and TestCommitBlockError; every block in the changed range 618–628 now reports a non-zero count in the coverage profile.

Block-sync callback behavior change lacks direct coverage — also real, and it was the more interesting gap. The alias unmasks the blockchain.ErrInvalidTipHeight case in chainservice/builder.go: a height rejection raised by the block store on CommitBlock (rather than caught earlier by ValidateBlock) now skips instead of returning an error. That path sat in an anonymous closure inside buildBlockSyncer with no way to reach it from a test. Extracted it as commitSyncedBlock, following the same pattern as estimateTipHeight/blockDistance in that file, and covered it — including the case that changes behavior, which fails if the alias is reverted.

The extraction is a relocation only. One thing worth a look during review: the original returned nil from the closure on the skip, so it deliberately did not log "Successfully committed block" or call consens.Calibrate. That is preserved by returning committed=false and short-circuiting before the Calibrate call, rather than a bare nil. Classification (errors.Cause), retry counts, and the ErrRemoteHeightTooLow retry bump are unchanged.

codecov/project — I don't think this one is on the PR. The report is 343 commits behind head and compares against base 436e4d1, producing a +124 files / +18486 lines diff and a head coverage of 62.63% against the configured 85% target. It should go green once the base report catches up; happy to rebase if that helps.

Full ./blockchain/... ./consensus/... ./chainservice/... remains green apart from TestBlockIndexerChecker_CheckIndexer, which fails identically on unmodified master.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud