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

feat(nodebuilder/blob/cmd): Allow submit multiple blobs through file input #3008

Merged
merged 22 commits into from
Feb 14, 2024

Conversation

sontrinh16
Copy link
Contributor

@sontrinh16 sontrinh16 commented Dec 14, 2023

Description

Closed: #2552

Testing:

testing with mocha testnet and get the following result ( Note: the commitments field contains an array of commitments from all the blobs):

Screenshot 2023-12-14 at 14 55 00

where test.json file contain:
{ "Blobs": [ { "namespace": "0x00010203040506070809", "blobData": "0x676d" }, { "namespace": "0x42690c204d39600fddd3", "blobData": "0x676d" } ] }

@github-actions github-actions bot added the external Issues created by non node team members label Dec 14, 2023
@Wondertan
Copy link
Member

@vgonkivs, please take a look

Copy link
Member

@vgonkivs vgonkivs left a 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

nodebuilder/blob/cmd/blob.go Outdated Show resolved Hide resolved
nodebuilder/blob/cmd/blob.go Outdated Show resolved Hide resolved
Copy link
Member

@vgonkivs vgonkivs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some nits but other than that great work 🚀 . Thank you.

PS. tested locally. Works for me
Снимок экрана 2024-01-09 в 16 45 20

nodebuilder/blob/cmd/blob.go Outdated Show resolved Hide resolved
nodebuilder/blob/cmd/blob.go Outdated Show resolved Hide resolved
nodebuilder/blob/cmd/blob.go Outdated Show resolved Hide resolved
@jcstein
Copy link
Member

jcstein commented Jan 9, 2024

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="
    ]
  }
}

distractedm1nd
distractedm1nd previously approved these changes Jan 11, 2024
Copy link
Member

@distractedm1nd distractedm1nd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@codecov-commenter
Copy link

codecov-commenter commented Jan 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (87544d7) 51.97% compared to head (455122c) 52.05%.

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.
📢 Have feedback on the report? Share it here.

renaynay
renaynay previously approved these changes Jan 11, 2024
Copy link
Member

@renaynay renaynay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utack

vgonkivs
vgonkivs previously approved these changes Jan 11, 2024
@vgonkivs vgonkivs added the kind:feat Attached to feature PRs label Jan 11, 2024
ramin
ramin previously approved these changes Jan 11, 2024
@ramin ramin enabled auto-merge (squash) January 16, 2024 14:56
@ramin
Copy link
Contributor

ramin commented Jan 16, 2024

@sontrinh16 can you rebase main back onto this please? Then it'll be ready to merge

@ramin
Copy link
Contributor

ramin commented Jan 23, 2024

@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

auto-merge was automatically disabled January 25, 2024 03:01

Head branch was pushed to by a user without write access

@sontrinh16
Copy link
Contributor Author

@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

hmmm i can't find where to toggle but i can add you guys as collaborators

@sontrinh16 sontrinh16 requested a review from ramin January 31, 2024 08:48
Copy link
Member

@walldiss walldiss left a 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 👍

@jcstein
Copy link
Member

jcstein commented Feb 13, 2024

NOTE: I have not tested this on the current version of the PR, just at 7587f80

@sontrinh16
Copy link
Contributor Author

i have tested with the newest commit

image

@vgonkivs vgonkivs merged commit 5e1895c into celestiaorg:main Feb 14, 2024
23 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:blob external Issues created by non node team members kind:feat Attached to feature PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(cmd): add flag that allows to read blob's data from file