celestia-node icon indicating copy to clipboard operation
celestia-node copied to clipboard

service/rpc: Return `/namespaced_data` and `/namespaced_shares` as flattened hex

Open renaynay opened this issue 2 years ago • 2 comments

Instead of returning some base64 encoded string, we should return /namespaced_data and /namespaced_shares as flattened, hex-encoded bytes.

GET /namespaced_shares/{nID}/height/{height}

Request

curl -X GET <ip>:26658/namespaced_shares/ca0724d103a7c5bb/height/173350

Response

{"shares":"ca0724d103a7c5bbc801d9a51f3b9c31a0af6ab37586e6cf50826e89e0dce53977b719e0ee221cbafde6b9eb02615485c94085d5e109938ba5b62c849e139e105690c4ab18ab824bb29fc872a0a245093a62db5a9f972d8a84d19866fc3ecd7192aaebe5218454fa7baa1e95896c44a0b3a09e6e033ee645dfa39c33f3bddcdb9fae07b26c37c6eed839b7d55c941c2c2aeb0e066596ec2c713f7f2e30453fa3445cee1e77fff91375b42a042dbba88d154bc851f5d214896d3418cc4d69a4cef50bcf707e9f9ab84e261675b22d9888973f00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000","height":173350}

GET /namespaced_data/{nID}/height/{height}

Request

curl -X GET <ip>:26658/namespaced_data/ca0724d103a7c5bb/height/173350

Response

{"data":"d9a51f3b9c31a0af6ab37586e6cf50826e89e0dce53977b719e0ee221cbafde6b9eb02615485c94085d5e109938ba5b62c849e139e105690c4ab18ab824bb29fc872a0a245093a62db5a9f972d8a84d19866fc3ecd7192aaebe5218454fa7baa1e95896c44a0b3a09e6e033ee645dfa39c33f3bddcdb9fae07b26c37c6eed839b7d55c941c2c2aeb0e066596ec2c713f7f2e30453fa3445cee1e77fff91375b42a042dbba88d154bc851f5d214896d3418cc4d69a4cef50bcf707e9f9ab84e261675b22d9888973f","height":173350}

renaynay avatar Aug 11 '22 12:08 renaynay

Codecov Report

Merging #1002 (6524343) into main (2cc8077) will decrease coverage by 0.00%. The diff coverage is 62.50%.

@@            Coverage Diff             @@
##             main    #1002      +/-   ##
==========================================
- Coverage   58.53%   58.53%   -0.01%     
==========================================
  Files         132      132              
  Lines        7947     7953       +6     
==========================================
+ Hits         4652     4655       +3     
- Misses       2818     2820       +2     
- Partials      477      478       +1     
Impacted Files Coverage Δ
service/rpc/share.go 52.57% <62.50%> (-0.17%) :arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

codecov-commenter avatar Aug 11 '22 13:08 codecov-commenter

Failed test will be fixed with https://github.com/celestiaorg/celestia-node/pull/1000

vgonkivs avatar Aug 11 '22 16:08 vgonkivs

@distractedm1nd, pls rebase on main so we see that TestFullReconstructFromBridge is really fixed

Wondertan avatar Aug 17 '22 13:08 Wondertan

@Wondertan I'm not sure if @tzdybal wants this.

renaynay avatar Aug 17 '22 13:08 renaynay

I just don't see any reason for this change. It doesn't fix any "burning" issue. I thought we're waiting for Public API to replace this "beta" completely. For now, I'm afraid this will cause a lot of confusion and errors. Developers will have to know what version of node they query. Plus, we have to change go-cnc and optimint accordingly, and the reason for compatibility break in un-versioned public API is only "cosmetics" IIUC.

But, OFC, I'll adopt the changes if this is merged :)

tzdybal avatar Aug 18 '22 10:08 tzdybal

Closing due to ^

renaynay avatar Aug 18 '22 14:08 renaynay