-
Notifications
You must be signed in to change notification settings - Fork 972
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
feat(nodebuilder/blob/cmd): Allow submit multiple blobs through file input #3008
Conversation
@vgonkivs, please take a look |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your PR @sontrinh16. Left some comments
Co-authored-by: Viacheslav <[email protected]>
…celestia-node into submit_multiple_blobs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
testing on mocha-4 testnet using 2 identical blobs (link to celestiascan) test.json { "Blobs": [ { "namespace": "0x0000000000004a6f7368", "blobData": "0x676d" }, { "namespace": "0x0000000000004a6f7368", "blobData": "0x676d" } ] } $ celestia blob submit --input-file test.json --node.store $NODE_STORE
{
"result": {
"height": 910885,
"commitments": [
"uYcW7vh+XB1VCbWSjTQJFA3PV89/089LyhaNQ5hXxjY=",
"uYcW7vh+XB1VCbWSjTQJFA3PV89/089LyhaNQ5hXxjY="
]
}
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3008 +/- ##
==========================================
+ Coverage 51.97% 52.05% +0.07%
==========================================
Files 178 178
Lines 11286 11286
==========================================
+ Hits 5866 5875 +9
+ Misses 4916 4909 -7
+ Partials 504 502 -2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utack
@sontrinh16 can you rebase main back onto this please? Then it'll be ready to merge |
@sontrinh16 can you rebase main one more time, or if you toggle the "let admin make changes to PR" we can do this and get it merged, thanks |
Head branch was pushed to by a user without write access
62c950b
hmmm i can't find where to toggle but i can add you guys as collaborators |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for contribution! Looks good 👍
NOTE: I have not tested this on the current version of the PR, just at 7587f80 |
Description
Closed: #2552
Testing:
testing with
mocha
testnet and get the following result ( Note: thecommitments
field contains an array ofcommitments
from all the blobs):where
test.json
file contain:{ "Blobs": [ { "namespace": "0x00010203040506070809", "blobData": "0x676d" }, { "namespace": "0x42690c204d39600fddd3", "blobData": "0x676d" } ] }