Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add missing block and receipt message fields #9
base: main
Are you sure you want to change the base?
Add missing block and receipt message fields #9
Changes from 2 commits
9e638e8
3da1088
1692596
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
transactoins and events were not supposed to be part of the block body. this can be discussed. the idea was that with stark proofs we don't need to validate transactions by executing them. This is when there's proof which is K blocks behind. Until then the idea was to rely on the consensus (similarly to how Ethereum has block times and epochs).
The protocol can be defined so that txs are mandatory until K and then are optional.
Similarly for receipts & events (which were separated).
minor comment: numbering jumps from 4 to 6
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.
Are all forms of nodes expected to serve the RPC spec? If so, then having transactions and receipts as optional until after K blocks seems unnecessary because all nodes would be fetching them separately to serve the RPC requests. And since block bodies are chunked I don't see any benefit in fetching them separately as that would increase network overhead.
I will update
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.
It means that to have the data needed to sync with the chain, a node doesn't need to keep transactions before the block with the proof. They can clean their DB. If a node wants to, it can of course keep whatever it wants.
As mentioned, they are not part of the block body, so no need for a client to fetch them. Even if they are included for new blocks, they would be optional and with a separate endpoint to fetch them (from those nodes that keep them beyond the optional point)
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.
What defines whether something should be a part of the block body?
We don't need
State Diff
orClasses
to be part of the block body because State Diff has a commitment which includes the Classes declared in the blocks, therefore, a peer can fetch them separately and verify the state diff commitment with the signature which is over the block hash and state diff commitment.Note we will have to add
BlockID
and possiblyClasses
toState Diff
. Also State diff commitment is over the state diff defined by feeder gateway not p2p state diff.Why can't we include the BlockProof in the BlockHeader? Given other fields are so small
142KB+ other fields' size < 1MB
.If we make the above changes the block bodies can be removed.
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.
Right, they can fetch them separately. So the thought was what will peers have to fetch in order to be an operational node. They need the current state, they don't need past transactions.
I agree the proof may be part of the header, as optional. Note that we can think of returning many headers in one message (and then it makes sense to have the proof outside)
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.
Can you please elaborate on what you mean by an operational node and fetch which messages?
Also, fetching can be done in order but there is no guarantee the messages will arrive in order. So I am not sure what you mean.
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.
I was replying to "... includes the Classes declared in the blocks, therefore, a peer can fetch them separately". Operational means a node that can follow the chain. Such a node doesn't need transactions. If a client wants to check transactions state, they will enable that in a node, if they just want to track events, they will enable that. If they want to check state, they can do that.
What messages won't arrive in order? If we support out of order messages in a stream then each will have a reference to be able to order them, if needed.
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.
Why is the RPC spec not sufficient to serve this? I don't see the need to replicate this behaviour on the p2p side. Also, how does a peer verify that the events (or any other date) received are correct?
Messages from a single peer will indeed come in order over a stream but libp2p doesn't allow for ordered messages from multiple peers on a single stream without further multiplexing. If a peer requests information for a block from multiple peers that peer will have multiple streams open, per peer, and thus there is no guarantee block messages part will arrive in order.