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

share/p2p/shrexnd: Support GetAll with > 1mb blob #2419

Closed
musalbas opened this issue Jul 2, 2023 · 4 comments · Fixed by #2444
Closed

share/p2p/shrexnd: Support GetAll with > 1mb blob #2419

musalbas opened this issue Jul 2, 2023 · 4 comments · Fixed by #2444
Assignees
Labels
area:p2p area:shares Shares and samples external Issues created by non node team members

Comments

@musalbas
Copy link
Member

musalbas commented Jul 2, 2023

./celestia rpc blob GetAll 13281 0x42690c204d39600fddd3 on arabica-9

leads to:

2023-07-02T17:05:38.643+0100 WARN shrex/nd shrexnd/client.go:78 client-nd: peer returned err {"err": "client-nd: reading response: stream reset"}

@github-actions github-actions bot added needs:triage external Issues created by non node team members labels Jul 2, 2023
@Wondertan
Copy link
Member

The ND protocol does not support messages bigger than 1mb atm. The easiest short-term solution is to increase the single message size limit, which causes the stream reset. The better option is to support message chunking which won't have performance implications.

@renaynay renaynay added area:shares Shares and samples area:p2p and removed needs:triage labels Jul 3, 2023
@renaynay renaynay changed the title GetAll fails due to stream reset when reading namespace with 1mb blob share/p2p/shrexnd: Support GetAll with > 1mb blob Jul 3, 2023
@musalbas
Copy link
Member Author

musalbas commented Jul 3, 2023

here's a script I use to submit big pfbs in case needed for testing:

#!/bin/bash

export CELESTIA_NODE_AUTH_TOKEN=$(./celestia light auth admin --p2p.network arabica)

printf '{
  "id": 1,
  "jsonrpc": "2.0",
  "method": "blob.Submit",
  "params": [
   [
        {
          "namespace": "AAAAAAAAAAAAAAAAAAAAAAAAAEJpDCBNOWAP3dM=",
          "data": "' > curldata.txt
printf `cat /dev/urandom | head -c $1 | base64 --wrap=0` >> curldata.txt
printf '",
          "share_version": 0,
          "commitment": "yqCkSLJCudll7PWbO26ktWMR4gZOITNFUffMLmXLTcg="
        }
   ]
  ]
 }' >> curldata.txt

curl -X POST -H "Authorization: Bearer ${CELESTIA_NODE_AUTH_TOKEN}" \
   -H 'Content-Type: application/json' \
   -d @curldata.txt \
   http://localhost:26658

usage:
./script.sh <pfb size in bytes>

@musalbas
Copy link
Member Author

musalbas commented Jul 3, 2023

The ND protocol does not support messages bigger than 1mb atm. The easiest short-term solution is to increase the single message size limit, which causes the stream reset. The better option is to support message chunking which won't have performance implications.

Wouldn't it look the same as the shrexeds implementation (which should support 8mb blocks) anyway - or does that support message chunking?

@Wondertan
Copy link
Member

Wondertan commented Jul 4, 2023

@musalbas, shrexeds uses modified CAR format for which we've implemented streaming Read and Write primitives so that it can handle big blocks. The ND protocol sends a single protobuf encoded message, as namespace data is not encoded in CAR format. We avoided the CAR format for ND to minimize overhead for LN requesting namespace data, which the CAR format brings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:p2p area:shares Shares and samples external Issues created by non node team members
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants