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

Get multiple message shares from blobs helper #1234

Closed
rach-id opened this issue Jan 13, 2023 · 7 comments
Closed

Get multiple message shares from blobs helper #1234

rach-id opened this issue Jan 13, 2023 · 7 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@rach-id
Copy link
Member

rach-id commented Jan 13, 2023

As discussed under https://github.com/celestiaorg/celestia-app/pull/1233/files#r1069423514, we can extend the functionality of:

func MsgSharesPosition(tx types.Tx) (beginShare uint64, endShare uint64, err error)

to also support transactions containing multiple messages and return their share ranges.

@rach-id rach-id added enhancement New feature or request good first issue Good for newcomers labels Jan 13, 2023
@evan-forbes
Copy link
Member

to clarify a bit, were referring to sdk.Msgs here afaiu, correct?

@rach-id
Copy link
Member Author

rach-id commented Jan 17, 2023

Now that multiple blobs is merged, I guess we can also support multiple blobs, what do you think?

@evan-forbes
Copy link
Member

Now that multiple blobs is merged, I guess we can also support multiple blobs, what do you think?

not sure what is meant by this mind clarifying, multiple PFBs per tx? that is on the stretch goals yeah

@rootulp
Copy link
Collaborator

rootulp commented Feb 8, 2023

This issue is confusing with respect to the usage of "message". I'm pretty confident it's referring to getting the positions of multiple blobs. Proposal to close this issue in favor of #1315 which conveys the goal a little more clearly. Since this issue is specific to a particular helper function and there's little point to updating the helper to support multiple blobs if we don't have support for share proofs for multiple blobs. Thoughts @sweexordious ?

@rach-id
Copy link
Member Author

rach-id commented Feb 8, 2023

We can prove multiple blobs if they are in the same namespace. However, for multiple namespaces, it's not supported. Is this what you meant?

@rootulp
Copy link
Collaborator

rootulp commented Feb 8, 2023

We can prove multiple blobs if they are in the same namespace

I see this is technically possible if a user invoked

func NewShareInclusionProof(
with a startShare - endShare that spanned multiple blobs but only one namespace.

However, for multiple namespaces, it's not supported.

Agreed. Do we need this issue and #1315? I think we can close this in favor of #1315.

@rach-id
Copy link
Member Author

rach-id commented Feb 8, 2023

Yes of course, that one is more general, go for it 👍 Thanks a lot

@rootulp rootulp closed this as not planned Won't fix, can't repro, duplicate, stale Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants