Skip to content

consensus: CommitBlock on an already-committed height is treated as an error instead of a skip #5010

Description

@guo

Summary

blockchain/filedao/filedao.go:43 and blockchain/blockchain.go:51 each declare their own ErrInvalidTipHeight variable with the same message. blockchain.CommitBlock propagates the filedao one when the block store refuses a block whose height is not tip + 1, but the callers switch on the blockchain one.

Where it bites

  • consensus/scheme/rolldpos/rolldposctx.go:618-629: if block sync already committed height H and the consensus round then calls CommitBlock for its own block at H, the switch does not hit case blockchain.ErrInvalidTipHeight (which returns true, nil), falls through to default, logs error when committing the block, and returns an error. The FSM 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 is masked there because the preceding ValidateBlock returns the blockchain variable first.

No finality or state impact: the block store guard still refuses the second block. This is a robustness issue that turns an expected "already committed" outcome into an error path.

Suggested fix

Make filedao return blockchain.ErrInvalidTipHeight (or have blockchain re-export the filedao one), or wrap the storage error so errors.Is matches both. Add a test that commits H through the block-sync path and then through rolldposCtx.Commit and asserts the skip branch.

Found during the CORE-22 finality-reversal review at 30714c91e.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions