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

blob.GetAll not returning correct base64 data, GetShare and GetSharesByNamespace also don't work #2503

Closed
jcstein opened this issue Jul 19, 2023 · 10 comments · Fixed by #2515
Labels
bug Something isn't working external Issues created by non node team members

Comments

@jcstein
Copy link
Member

jcstein commented Jul 19, 2023

Celestia Node version

v0.11.0-rc8

OS

mac

Install tools

docs

Others

nope

Steps to reproduce it

Q: if i post plain-text data '"gm"' and retrieve it, the response is in base64, correct?
i’m updating the based64.xyz tutorial, and during testing I used '"gm"' as a check, the base64 share that i retrieve is: SW1kdElnPT0= which converts from base64 to text: ImdtIg==, not gm (plus some metadata bytes). i’m on arabica-9 v0.11.0-rc8
i previously had celestia rpc share GetSharesByNamespace "$(celestia rpc header GetByHeight $HEIGHT | jq '.result.dah' -r)" GHTmQvXd5Yk= but now am using celestia rpc blob GetAll $HEIGHT 0x42690c204d39600fddd3

the commands that previously worked, also do not work anymore as expected, detailed in this issue:
celestiaorg/docs#914

updated tutorial is at https://based64.xyz/

i also went through the node tutorial page, and the examples on retrieval work with CLI when submitting hex.

Expected result

returns blob in base64 which can be converted to base64 to text. this is how it worked originally and there seems to be a bug on the serialization or deserialization in rpc.go (based on what Hlib and I were looking at yesterday)

Actual result

returns data in wrong format

Relevant log output

celestia rpc share GetSharesByNamespace "$(celestia rpc header GetByHeight 103692 | jq '.result.dah' -r)" 0x42690c204d39600fddd3
{
  "jsonrpc": "2.0",
  "id": 1,
  "error": {
    "code": -32700,
    "message": "unmarshaling params for 'share.GetSharesByNamespace' (param: *share.Namespace): illegal base64 data at input byte 20"
  }
}

celestia rpc share GetShare "$(celestia rpc header GetByHeight 101810 | jq '.result.dah' -r)" 101810 0
{
  "jsonrpc": "2.0",
  "id": 1,
  "error": {
    "code": 0,
    "message": "fatal error calling 'share.GetShare': panic in rpc method 'share.GetShare': runtime error: index out of range [101810] with length 8"
  }
}

Notes

I know for a fact that I submitted data to the namespace and at the block heights above.

the methods themselves did not change. i am using the correct length of namespace.

image
image
image

@jcstein jcstein added the bug Something isn't working label Jul 19, 2023
@github-actions github-actions bot added the external Issues created by non node team members label Jul 19, 2023
@jcstein
Copy link
Member Author

jcstein commented Jul 19, 2023

only steps you need to repro:

  1. run arabica light node on v0.11.0-rc8: celestia light start --core.ip consensus-full-arabica-9.celestia-arabica.com --p2p.network arabica
  2. export CELESTIA_NODE_AUTH_TOKEN=$(celestia light auth admin --p2p.network arabica)
  3. celestia rpc blob Submit 0x42690c204d39600fddd3 '"gm"'
  4. celestia rpc blob GetAll [blockheightfrom#3] 0x42690c204d39600fddd3
  5. convert the data to hex here: https://base64.guru/converter/decode/text

@renaynay
Copy link
Member

Duplicate to #2500

@jcstein
Copy link
Member Author

jcstein commented Jul 19, 2023

ok now context is linked :)

@jcstein jcstein closed this as completed Jul 19, 2023
@jcstein jcstein reopened this Jul 19, 2023
@jcstein
Copy link
Member Author

jcstein commented Jul 19, 2023

other issue doesn't seem to address the issues with the response from the RPC, though, so reopening. but please close if wrong

@vgonkivs
Copy link
Member

Input params are incorrect here:
celestia rpc share GetShare "$(celestia rpc header GetByHeight 101810 | jq '.result.dah' -r)" 101810 0
In the second param instead of header.Height(101810) you should pass a row index.

@vgonkivs
Copy link
Member

But I agree that there is an issue because we don't check indexes before handing in the request.

@jcstein
Copy link
Member Author

jcstein commented Jul 27, 2023

Input params are incorrect here:
celestia rpc share GetShare "$(celestia rpc header GetByHeight 101810 | jq '.result.dah' -r)" 101810 0
In the second param instead of header.Height(101810) you should pass a row index.

what is the correct format?

@vgonkivs
Copy link
Member

vgonkivs commented Jul 27, 2023

celestia rpc share GetShare "$(celestia rpc header GetByHeight 101810 | jq '.result.dah' -r)" rowIndex colIndex

@jcstein
Copy link
Member Author

jcstein commented Jul 27, 2023

so what namespace is this for?

celestia rpc share GetShare "$(celestia rpc header GetByHeight 101810 | jq '.result.dah' -r)"  0 0
{
  "jsonrpc": "2.0",
  "result": "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAQBAAACuAAAACbcAgrQAgqgAQqdAQogL2NlbGVzdGlhLmJsb2IudjEuTXNnUGF5Rm9yQmxvYnMSeQovY2VsZXN0aWExNXJkZHB3amY1M3Fsa21xcnkzbmE5Y2FzY3dsYzl4Z3J2czRraDcSHQAAAAAAAAAAAAAAAAAAAAAAAAA+6F39tPkv7BI9GgKLDCIgAW7uZlBgQDnTc8czrN6voDfKphfmVcZLebBsjzEVpoRCAQASaQpRCkYKHy9jb3Ntb3MuY3J5cHRvLnNlY3AyNTZrMS5QdWJLZXkSIwohA9QZjbTNT9DKL9qkJKO/SBiv4uqoulCE8pngYUyJRQ1jEgQKAggBGNN1EhQKDgoEdXRpYRIGMjAwMDAwEICJehpAEdmeKwe6M1bi+Yzj/2wpsfUjA2d7BOxnGsJTpFZzEW0RzhBRa52BD54LsV4xk3zwGE0QormkbQIvO5k9KFD8qxIBAhoESU5EWNgCCswCCp8BCpwBCiAvY2VsZXN0aWEuYmxvYi52MS5Nc2dQYXlGb3JCbG9icxJ4Ci9jZWxlc3RpYTF5aDM2OTh4MzJwYTVtN2NxM2U2azg3bTQ0NWZjOG1wdWU4bWU2dhIdAAAAAAAAAAAAAAAAAAAAAAAAAEJpDCBNOWA=",
  "id": 1
}

@vgonkivs
Copy link
Member

This request is not about the namespace, it fetches the share at a particular position(I actually do not understand why do we need it to be exposed via rpc).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working external Issues created by non node team members
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants