8000 Consider exporting error for `ErrTimedOutWaitingForTx` · Issue #2014 · cometbft/cometbft · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Consider exporting error for ErrTimedOutWaitingForTx #2014

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.

Sign up for GitHub

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

Jump to bottom
Closed
rootulp opened this issue Jan 10, 2024 · 2 comments
Closed

Consider exporting error for ErrTimedOutWaitingForTx #2014

rootulp opened this issue Jan 10, 2024 · 2 comments

Comments

@rootulp
Copy link
Contributor
rootulp commented Jan 10, 2024 •
edited
Loading

Context

Celestia maintains a fork of CometBFT in celestia-core. We're considering exporting an error so that downstream consumers can use it rather than checking for an error based on string. See celestiaorg/celestia-core#1169

Problem

cometbft/rpc/core/mempool.go

Line 148 in 00e5270

err = errors.New("timed out waiting for tx to be included in a block")
defines an ad-hoc error that can't be used by downstream consumers. Ideally this error was exported from CometBFT so that downstream consumers can use it in errors.Is

Celestia-core can workaround this by exporting it but that introduces a divergence from this repo.

Proposal

  1. Export an error for ErrTimedOutWaitingForTx. Basically feat: export ErrTimedOutWaitingForTx celestiaorg/celestia-core#1169
  2. If there are other un-exported errors, consider exporting them.
@rootulp rootulp mentioned this issue Jan 10, 2024
Merged
@melekes
Copy link
Contributor
melekes commented Jan 11, 2024

Hey @rootulp 👋 We already have an issue for this: #1140. If you want to help upstreaming the above chan 9342 ges, let us know (feel free to open a PR).

@melekes melekes closed this as completed Jan 11, 2024
@distractedm1nd
Copy link
distractedm1nd commented Jan 11, 2024 •
edited
Loading

@melekes The referenced error is not a named error, so the already merged PR to export errors from mempool does not include it. Is the issue you linked only for exporting errors or should I bring it up there as well?

Sorry, just noticed that the mempool PR isn't the same file, so assuming it will be added under the rpc subtask. You can disregard this comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@melekes @rootulp @distractedm1nd
0