Skip to content

Commit

Permalink
ABCI calls should crash/exit in all cases when the call itself report…
Browse files Browse the repository at this point in the history
…s and error (cometbft#496)

* Review call hierarchies of ABCI methods to make sure they panic on error

* Added changelog

* Trying 1.20.2 explicitly

(cherry picked from commit 2ab8598)

# Conflicts:
#	.changelog/v0.37.0/bug-fixes/496-error-on-applyblock-should-panic.md
  • Loading branch information
sergio-mena authored and mergify[bot] committed Mar 13, 2023
1 parent b3c3482 commit 825ac9e
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- `[consensus]` Unexpected error conditions in `ApplyBlock` are non-recoverable, so ignoring the error and carrying on is a bug. We replaced a `return` that disregarded the error by a `panic`.
([\#496](https://github.com/cometbft/cometbft/pull/496))
2 changes: 1 addition & 1 deletion .github/workflows/govulncheck.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
steps:
- uses: actions/setup-go@v3
with:
go-version: "1.20"
go-version: "1.20.2"
- uses: actions/checkout@v3
- uses: technote-space/get-diff-action@v6
with:
Expand Down
3 changes: 1 addition & 2 deletions consensus/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -1706,8 +1706,7 @@ func (cs *State) finalizeCommit(height int64) {
block,
)
if err != nil {
logger.Error("failed to apply block", "err", err)
return
panic(fmt.Sprintf("failed to apply block; error %v", err))
}

fail.Fail() // XXX
Expand Down
2 changes: 1 addition & 1 deletion state/execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ func (blockExec *BlockExecutor) ProcessProposal(
NextValidatorsHash: block.NextValidatorsHash,
})
if err != nil {
return false, ErrInvalidBlock(err)
return false, err
}
if resp.IsStatusUnknown() {
panic(fmt.Sprintf("ProcessProposal responded with status %s", resp.Status.String()))
Expand Down

0 comments on commit 825ac9e

Please sign in to comment.