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.

Sign up for GitHub

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Add missing block and receipt message fields #9

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9e638e8
Add `CurrentBlockHeaderRequest{}`
IronGauntlets Nov 9, 2023
3da1088
Add missing block header and body fields
IronGauntlets Nov 9, 2023
1692596
Remove extra_data and hash from BlockHeader
IronGauntlets Nov 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions p2p/proto/block.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
syntax = "proto3";
import "p2p/proto/common.proto";
import "p2p/proto/state.proto";
import "p2p/proto/transaction.proto";
import "p2p/proto/receipt.proto";
import "google/protobuf/timestamp.proto";

// for now, we assume a small consensus, so this fits in 1M. Else, these will be repeated
Expand All @@ -14,7 +16,7 @@ message Signatures {
// Note: commitments may change to be for the previous blocks like comet/tendermint
// hash of block header sent to L1
message BlockHeader {
Hash parent_header = 1;
Hash parent_hash = 1;
uint64 number = 2;
google.protobuf.Timestamp time = 3; // TODO: see if this needs to be Felt252 or can be converted
Address sequencer_address = 4;
Expand All @@ -29,6 +31,8 @@ message BlockHeader {
Merkle transactions = 8; // By order of execution. TBD: required? the client can execute (powerful machine) and match state diff
Merkle events = 9; // By order of issuance. TBD: in receipts?
Merkle receipts = 10; // By order of issuance.
string protocol_version = 11; // Starknet version
Felt252 gas_price = 12;
}

message BlockProof {
Expand All @@ -45,6 +49,8 @@ message NewBlock {
}
}

// Requests a peer's CurrentBlockHeader
message CurrentBlockHeaderRequest {}
IronGauntlets marked this conversation as resolved.
Show resolved Hide resolved

// result is (BlockHeader, Signature?)* in order of creation (incr/dec)
message BlockHeadersRequest {
Expand Down Expand Up @@ -72,10 +78,12 @@ message BlockBodiesResponse {
optional BlockID id = 1; // may not appear if Fin is sent to end the whole response

oneof body_message {
StateDiff diff = 2;
Classes classes = 3;
BlockProof proof = 4;
Fin fin = 5;
StateDiff diff = 2;
Classes classes = 3;
BlockProof proof = 4;
Transactions transactions = 5;
Receipts receipts = 6;
Comment on lines +84 to +85
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why should transactions and receipts be included in the response stream?
Why not just use separate queries for them (as in the current spec version)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added the transactions and receipts as part of the block body for completeness as it was represented by the block returned by the feeder gateway. However, you are correct we can fetch them separately.

Based on the same reasoning I don't think we should have a body message at all as we have state commitment which can be used to verify StateDiff and Classes, while the BlockProof can be included in the Header. Also StateDiff, Classes and Block proof are already sent separately just wrapped in a body message.

This also ties in with having multiple protocols instead of one. I may be missing some context but I cannot see under which situation a node would be interested in just transactions/events/receipts/headers etc. How would the node verify the validity of the single message which they retrieved without also retrieving other information such as block header, state diff transactions etc? And why would RPC not be enough (cc:@joshklop)? We can decide to add web sockets for whatever data the user is interested in. In my opinion, we should have a single protocol.

Please see this convo for more context.

Fin fin = 7;
}
}

Expand Down
5 changes: 4 additions & 1 deletion p2p/proto/receipt.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
syntax = "proto3";
import "p2p/proto/common.proto";
import "p2p/proto/event.proto";
IronGauntlets marked this conversation as resolved.
Show resolved Hide resolved

message MessageToL1 {
Felt252 from_address = 1;
Expand Down Expand Up @@ -29,6 +30,7 @@ message Receipt {
uint32 range_check = 5;
uint32 poseidon = 6;
uint32 keccak = 7;
uint32 output = 8;
}

BuiltinCounter builtins = 1;
Expand All @@ -42,6 +44,7 @@ message Receipt {
repeated MessageToL1 messages_sent = 3;
ExecutionResources execution_resources = 4;
string revert_reason = 5;
Events events = 6;
IronGauntlets marked this conversation as resolved.
Show resolved Hide resolved
IronGauntlets marked this conversation as resolved.
Show resolved Hide resolved
}


Expand All @@ -68,7 +71,7 @@ message Receipt {
Felt252 contract_address = 2;
}

oneof receipt {
oneof type {
Invoke invoke = 1;
L1Handler l1_handler = 2;
Declare declare = 3;
Expand Down