Skip to content

fix(mempool): panic when the app returns error on CheckTx request (backport #2894)#2904

Merged
hvanz merged 3 commits intov0.38.xfrom
mergify/bp/v0.38.x/pr-2894
Apr 26, 2024
Merged

fix(mempool): panic when the app returns error on CheckTx request (backport #2894)#2904
hvanz merged 3 commits intov0.38.xfrom
mergify/bp/v0.38.x/pr-2894

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Apr 26, 2024

Closes #2225

If the app returns an error on an ABCI call (in particular CheckTx), CometBFT should stop, because the error is unrecoverable.


PR checklist

  • Tests written/updated
  • Changelog entry added in .changelog (we use unclog to manage our changelog)
  • Updated relevant documentation (docs/ or spec/) and code comments
  • Title follows the Conventional Commits spec

This is an automatic backport of pull request #2894 done by [Mergify](https://mergify.com).

)

Closes #2225

If the app returns an error on an ABCI call (in particular CheckTx),
CometBFT should stop, because the error is unrecoverable.

---

#### PR checklist

- [X] Tests written/updated
- [X] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [X] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

(cherry picked from commit 98983fc)

# Conflicts:
#	mempool/clist_mempool.go
#	mempool/clist_mempool_test.go
@mergify
Copy link
Contributor Author

mergify bot commented Apr 26, 2024

Cherry-pick of 98983fc has failed:

On branch mergify/bp/v0.38.x/pr-2894
Your branch is up to date with 'origin/v0.38.x'.

You are currently cherry-picking commit 98983fc64.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   .changelog/unreleased/bug-fixes/2225-fix-checktx-request-returns-error.md
	modified:   mempool/errors.go

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   mempool/clist_mempool.go
	both modified:   mempool/clist_mempool_test.go

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@mergify mergify bot requested a review from a team as a code owner April 26, 2024 08:53
@mergify mergify bot added the conflicts label Apr 26, 2024
return errors.As(err, &ErrPreCheck{})
}

type ErrCheckTxAsync struct {
Copy link
Collaborator

@melekes melekes Apr 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this make it Go API breaking? I think we can keep the struct, but mark it as deprecated and unused (for linter).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error file was merged into v0.38 two months ago. I'd say it's safe to remove it. #2277

@hvanz hvanz merged commit b67ea6f into v0.38.x Apr 26, 2024
@hvanz hvanz deleted the mergify/bp/v0.38.x/pr-2894 branch April 26, 2024 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants