iotexproject / iotexproject/iotex-core

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

Open
#5,010 0 comments 0 reactions 1 assignee Claimed by @envestcc View on GitHub
Dominant language
Go
Stars
1.6k
Forks
382
Avg merge
4d 22h
Merged PRs (30d)
17

Description

## 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`.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.