Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Apr 26, 2024

Conversation

mergify[bot]
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
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
@@ -59,18 +59,6 @@ func IsPreCheckError(err error) bool {
return errors.As(err, &ErrPreCheck{})
}

type ErrCheckTxAsync struct {
Copy link
Contributor

@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
Member

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
21 checks passed
@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