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

[Feature Request]: standard error codes #65

Closed
tuxcanfly opened this issue Apr 24, 2024 · 7 comments · Fixed by #115
Closed

[Feature Request]: standard error codes #65

tuxcanfly opened this issue Apr 24, 2024 · 7 comments · Fixed by #115
Assignees
Labels
enhancement New feature or request released

Comments

@tuxcanfly
Copy link
Contributor

tuxcanfly commented Apr 24, 2024

Implementation ideas

Implementations should return standard error codes so that clients have a way to handle the error and retry if possible.

We can reuse JSONRPC error codes in the range --32000 to -32099 , clients should map their errors into the standard error codes for e.g.:

  • 32001 - request timeout
  • 32002 - mempool congested
  • 32003 - insufficient fee
  • 32004 - incorrect sequence / nonce
  • 32005 - blob size too large

gRPC can reuse the JSONRPC error codes for simplicity.

Based on the error, the client can retry with a higher fee, different nonce or smaller blob.

Implementations should wrap their error with the corresponding typed error.

E.g.:

pkg someda

...
var (
	// Err32003 is returned when the blob transaction has insufficient fee
	Err32003 = errors.New("32003")
)
...


// Submit submits the Blobs to Data Availability layer.
func (c *SomeDA) Submit(ctx context.Context, blobs []da.Blob, gasPrice float64, ns da.Namespace) ([]da.ID, error) {
        ...
	height, err := c.Submit(ctx, blobs, gasPrice, ns)
	if err != nil {
		if errors.Is(err, ErrInsufficientFee) {
			return errors.Wrap(da.Err32003, err)
		}
		return nil, err
	}
        ...
}
@tuxcanfly tuxcanfly added the enhancement New feature or request label Apr 24, 2024
@gupadhyaya
Copy link
Member

@tuxcanfly when do we expect this error? 32002 - mempool congested?

@tuxcanfly
Copy link
Contributor Author

@tuxcanfly when do we expect this error? 32002 - mempool congested?

When blob transactions are failing to be included in blocks due to mempool being full. It would signify two things:

  • this is a temporary error
  • it may be retried with a higher fee or higher priority fee when fee market is implemented

@gupadhyaya
Copy link
Member

@tuxcanfly when do we expect this error? 32002 - mempool congested?

When blob transactions are failing to be included in blocks due to mempool being full. It would signify two things:

  • this is a temporary error
  • it may be retried with a higher fee or higher priority fee when fee market is implemented

is this different from 32001 - request timeout?

@tuxcanfly
Copy link
Contributor Author

is this different from 32001 - request timeout?

Yes, it indicates a generic connection error and maybe retried without modifying the transaction.

@ramin
Copy link
Contributor

ramin commented Apr 30, 2024

@tuxcanfly I like this. What are your thoughts on introducing some kind of error for this use case (ie: blob not found) celestiaorg/celestia-node#3292 (there's a lot of context in that thread, sorry)

@Manav-Aggarwal
Copy link
Member

Copy link

github-actions bot commented Nov 6, 2024

🎉 This issue has been resolved in version 0.9.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants