iotexproject / iotexproject/iotex-core
consensus: CommitBlock on an already-committed height is treated as an error instead of a skip
- 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
Assessment
This issue has not been assessed yet.