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

Usability/DevUX Meta Issue #3335

Closed
10 of 11 tasks
liamsi opened this issue Apr 23, 2024 · 7 comments
Closed
10 of 11 tasks

Usability/DevUX Meta Issue #3335

liamsi opened this issue Apr 23, 2024 · 7 comments
Assignees
Labels
area:api Related to celestia-node API area:cli external Issues created by non node team members v0.17.0 Intended for v0.17.0 release

Comments

@liamsi
Copy link
Member

liamsi commented Apr 23, 2024

This issue collects smaller quirks with the current API/docs/CLI in one place. I will list them here and break them out into their own issues where it makes sense:

celestia blob submit 0x42690c204d39600fddd3 0x676d                                                                                                                  
{
  "result": {
    "height": 1671475,
    "commitments": [
      "0MFhYKQUi2BU+U1jxPzG7QY2BVV1lb3kiU+zAK7nUiY="
    ]
  }
}
⋊> ~/G/celestia-node on main ⨯ celestia blob get-all 1671475  0x42690c204d39600fddd3                                                                                                               12:06:50
{
  "result": [
    {
      "namespace": "AAAAAAAAAAAAAAAAAAAAAAAAAEJpDCBNOWAP3dM=",
      "data": "0x676d",
      "share_version": 0,
      "commitment": "0MFhYKQUi2BU+U1jxPzG7QY2BVV1lb3kiU+zAK7nUiY=",
      "index": 531
    }
  ]
}
@github-actions github-actions bot added needs:triage external Issues created by non node team members labels Apr 23, 2024
@Bidon15
Copy link
Member

Bidon15 commented Apr 23, 2024

Should be populated in node somehow

Blobstream APIs in node

celestiaorg/celestia-core#1306

Many Execution Environments to 1 DA Node

celestiaorg/celestia-app#3259
#3295
#3022

Separation of endpoints for DA Node (RPC/GRPC)

#2931

JWT Secret Rotation

#3293

@ramin ramin added area:api Related to celestia-node API and removed needs:triage labels Apr 24, 2024
@tuxcanfly
Copy link
Contributor

tuxcanfly commented Apr 26, 2024

I would like to add a couple of improvement ideas for reliable blob submission which is a blocker for rollup liveness:

  • "rpc error: code = Unknown desc = timed out waiting for tx to be included in a block" error is returned frequently possibly due to 1) mempool congestion 2) big blobs 3) too small timeout_broadcast_tx_commit
  • ": tx already in mempool" always happens immediately after the above error and requires waiting out until the tx drops from the mempool
  • ": incorrect account sequence" occasionally happens in place of the above error but also requires waiting out until the tx drops from the mempool
  • rpc error: code = Unknown desc = error on broadcastTxCommit: tx size is too big: 1962442, max: 1962441 happens when submitting big blobs due to padding shares even though the supposed limit is 1974272 so this breaks the contract with node.
  • rpc error: code = Unknown desc = error on broadcastTxCommit: tx too large occasionally big blobs return this in place of the above. I see that there's also Tx too large. Max size is %d, but got %d in the code.

Since there is no unique error code for these errors, we're currently grepping the string for these error strings which is not ideal.

It would be good to have a standardised error list and unique error codes and if possible, a error severity, and hint to allow clients to retry gracefully.

@tuxcanfly tuxcanfly reopened this Apr 26, 2024
@MSevey
Copy link
Member

MSevey commented Apr 29, 2024

Related: rollkit/go-da#65

@vgonkivs
Copy link
Member

vgonkivs commented Jul 1, 2024

#3537

@vgonkivs
Copy link
Member

vgonkivs commented Jul 5, 2024

also related to the above: #3164

After the fix on the app the issue "invalid account sequence" gone.

@vgonkivs
Copy link
Member

vgonkivs commented Jul 5, 2024

the comment here is wrong

#3530

GetAll is confusing and edge-cases are under-specified

#3223

some examples in API docs are not representative; e.g. blob.Get returns a different commitment than requested

#3479

CLI: when looking at the for usage returned by --help encoding related guidance is often missing, eg. celestia blob get [height] [namespace] [commitment] [flags] should state which format the inputs are expected in
CLI: namespaces are submitted as hex but returned base64 encoded (which is very confusing); example:

#3553
#3537

CLI/API: the node daemon gets started with only one account and one can only submit PFBs from that single account; this does not reflect how people interact with chains using different wallets for different purposes (applies to e.g. celestia blob submit and a bunch of state related commands / rpcs

#3349

#3345

#3391

API: Clarify State.SubmitPayForBlob vs Blob.Submit: both methods are too similar and it's unclear when to use which

#3349 (comment)

@renaynay renaynay added v0.17.0 Intended for v0.17.0 release and removed v0.15.0 labels Oct 2, 2024
@liamsi
Copy link
Member Author

liamsi commented Oct 8, 2024

As all issues here have been tackled I think it is good to close this one for now. If there is a list of new issues regarding the API, we should open a new meta issue. Massive shoutout to @vgonkivs for tackling all these 🙏🏼 ❤️ 🎉

@liamsi liamsi closed this as completed Oct 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:api Related to celestia-node API area:cli external Issues created by non node team members v0.17.0 Intended for v0.17.0 release
Projects
None yet
Development

No branches or pull requests

7 participants