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.

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

[Feature Request]: standard error codes #65

Open
tuxcanfly opened this issue Apr 24, 2024 · 6 comments · May be fixed by #115
Open

[Feature Request]: standard error codes #65

tuxcanfly opened this issue Apr 24, 2024 · 6 comments · May be fixed by #115
Assignees
@tzdybal
Labels
enhancement New feature or request

Comments

@tuxcanfly
Copy link
Contributor

tuxcanfly commented Apr 24, 2024 •
edited
Loading

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
@MSevey MSevey mentioned this issue Apr 29, 2024
Closed
11 tasks
@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)

@ramin ramin mentioned this issue May 2, 2024
Closed
@Manav-Aggarwal
Copy link
Member

celestiaorg/celestia-node#3529

@tzdybal tzdybal linked a pull request Oct 7, 2024 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tzdybal tzdybal

Labels
enhancement New feature or request
Projects
Rollkit
Status: In Progress
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: implement custom error types for DA rollkit/go-da
5 participants
@ramin @tuxcanfly @tzdybal @gupadhyaya @Manav-Aggarwal