Replies: 11 comments 60 replies
-
It might be helpful to add the index of the Blob when you get them, in case people need to link them to a PFB that was published (eg to link them to a certain transaction submitter). |
Beta Was this translation helpful? Give feedback.
-
Also, there is one feature request from rollkit that I want to understand fully: the need to manage blob inclusion on the rollkit side rather than delegating it to celestia-node. By manage, I mean handling networking and verifying them. On the other side, I am proposing to hide/abstract away most of the proof-related logic as an implementation detail of Celestia Node, as we do for DAHeader syncing and sampling. Pros:
Cons:
|
Beta Was this translation helpful? Give feedback.
-
The alternative API which forces users to handle Proofs: (only affected methods are shown) // Awaits inclusion and provides users proofs for the user(Rollkit) to gossip over their network
Submit(context.Context, Blob...) (height uint64, []Proof, error)
// Only find the correct DAHeader by height and perform an inclusion check against it(usefull for Rollkit Light clients)
Available(context.Context, height uint64, namespace.ID, hash.Hash, Proof) (bool, error) |
Beta Was this translation helpful? Give feedback.
-
API looks like a great start to me!
I think in both cases it remains a bit too vague what commit refers to. Is it the hash of the blob (probs not) is it the same substree root included in the pfb? |
Beta Was this translation helpful? Give feedback.
-
Rollkit will need an inclusion check of arbitrary shares that might not convert into a commitment. Let me give you the example: [S1][S2][S3][S4][S5][S6][S7][S8] This Blob is 8 Shares long and and you can generate a PFB commitment over it with ease. Now lets say a full node found fraud in those shares and wants to share it with the LN. They will need to share a blob inclusion proof to the shares those specific shares. Those shares might be in the middle of the block. Because of non-imnteractive default rules you cannot have a pfb commitment over those. Lets say the fraudulent shares are in [S2][S3] The merkle tree over the shares looks like this: Thats why there is no PFB commitment over One way to reference those 2 Shares is by (height,index,length) and not by commitment. |
Beta Was this translation helpful? Give feedback.
-
We also use the share version, which is added to the share and is needed to properly recreate any commitments. Currently, we only have a share version of 0, but we might want to consider adding it here given its importance. ref share specs |
Beta Was this translation helpful? Give feedback.
-
Should 'Submit' receive [ ] of Blobs? If not how do you submit multiple blobs under the same PFB? With this Interface, we would do this:
I thought inclusion was supposed to be verified by proof for now as we gossip it over rollkit p2p. |
Beta Was this translation helpful? Give feedback.
-
Idea: would be helpful to add a |
Beta Was this translation helpful? Give feedback.
-
[non-blocking] I think it's a bit cumbersome that the following methods accept as an argument namespace and commitment. Given a commitment is sufficient to uniquely identify a blob, it seems a bit easier for a caller to only specify the commitment and not both the namespace and commitment. Given this is a UX concern and the caller likely knows the namespace they are querying for, this issue isn't blocking and could be considered a future improvement. // Get retrieves the blob by commitment.
- Get(context.Context, uint64, namespace.ID, Commitment) (Blob, error)
+ Get(context.Context, uint64, Commitment) (Blob, error)
// GetProof gets the Proof for a blob if it exists at the given height under the namespace.
- GetProof(context.Context, height uint64, namespace.ID, Commitment) (Proof, error)
+ GetProof(context.Context, height uint64, Commitment) (Proof, error)
// Included checks whether a blob's given commitment(Merkle subtree root) is included at the given height under a particular namespace.
// Useful for light clients wishing to check the inclusion of the data.
- Included(context.Context, height uint64, namespace.ID, Commitment, Proof) (bool, error)
+ Included(context.Context, height uint64, Commitment, Proof) (bool, error) |
Beta Was this translation helpful? Give feedback.
-
In order to verify the Proof, we will also need rowRoots(to verify nmt.Proofs against them), so I'd suggest extending BlobModule interface with the: |
Beta Was this translation helpful? Give feedback.
-
leaving a comment about paying for gas fees when submitting blobs here after discussing with @vgonkivs: We likely want the user to have some input that determines the price of gas. If we have that, then we can estimate the gas limit, and then determine the fee. Both of those are then signed over. This simplifies the api, because the user only enters one thing, instead of fee and gas limit, which are both difficult to calculate. Determining gas price could be as simple as the user specifying a direct price, or more complex where we estimate what the lowest gas price is to achieve some outcome, such as getting included in the next block, or getting included in any of the next 10 blocks etc. |
Beta Was this translation helpful? Give feedback.
-
Two uncovered/undecided features:
Exposing inclusion proofs from the API. I am very hesitant to do this for reasons I will explain in a diff thread.
Inclusion checks for data at a particular share index rather than the whole blob.
cc @musalbas, @liamsi, @tzdybal, @nashqueue
Beta Was this translation helpful? Give feedback.
All reactions