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

non-node clients grpc api proposal #560

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ members = [
"cardano-legacy-address",
"sparse-array",
"typed-bytes",
"chain-watch",
]

[profile.bench]
Expand Down
21 changes: 21 additions & 0 deletions chain-watch/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
[package]
name = "chain-watch"
version = "0.1.0"
authors = ["Enzo Cioppettini <[email protected]>"]
edition = "2018"

[dependencies]
prost = "0.7"

[dependencies.tonic]
version = "0.4"
default-features = false
features = ["codegen", "prost"]

[build-dependencies.tonic-build]
version = "0.4"
default-features = false
features = ["prost"]

[features]
default = ["tonic/transport", "tonic-build/transport"]
Copy link
Contributor

Choose a reason for hiding this comment

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

A newline please :)

3 changes: 3 additions & 0 deletions chain-watch/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
fn main() {
tonic_build::compile_protos("proto/watch.proto").unwrap();
}
60 changes: 60 additions & 0 deletions chain-watch/proto/watch.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
syntax = "proto3";

package iohk.chain.watch;


message Block {
bytes content = 1;
}

message BlockSubscriptionRequest {}

message TipSubscriptionRequest {}

message MempoolSubscriptionRequest {}

message SyncMultiverseRequest {
// send blocks with this chain_length or greater
uint32 from = 1;
}

message BlockId {
bytes content = 1;
}

message MempoolEvent {
bytes fragment_id = 1;
oneof event {
MempoolFragmentInserted inserted = 2;
MempoolFragmentRejected rejected = 3;
MempoolFragmentInABlock in_a_block = 4;
};
}


message MempoolFragmentInserted {}

message MempoolFragmentRejected {
string reason = 1;
}

message MempoolFragmentInABlock {
BlockId block = 1;
}


service SubscriptionService {
// get a stream of blocks succesfully processed by the node, this means they
// are already validated.
// the parent of a block will always be streamed before the block itself.
rpc BlockSubscription(BlockSubscriptionRequest) returns (stream Block);

// get tip updates
rpc TipSubscription(TipSubscriptionRequest) returns (stream BlockId);

rpc MempoolSubscription(MempoolSubscriptionRequest) returns (stream MempoolEvent);
Copy link
Contributor

Choose a reason for hiding this comment

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

And now I am not sure that this should belong to chain-libs. We usually did not introduce the concept of mempool at the chain-libs level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, current plan is to move it to chain-network, actually. If we put this in jormungandr... And we want to re-use from a different repository, then we would have to include jormungandr there somehow?

Although, @mzabaluev motivation to put it in chain-network was the dependencies/build process, I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

motivation to put it in chain-network was the dependencies/build process, I think

Yeah, mostly that I think.

then we would have to include jormungandr there somehow

Not a huge problem, we are doing that for jormungandr-lib already.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather disentangle protocol things from jormungandr.

And yes, I would prefer to move this into chain-network as a separate module.


// fetch all blocks from the given initial chainlength to the tip, from all
// possible branches and in increasing order
rpc SyncMultiverse(SyncMultiverseRequest) returns (stream Block);
Copy link
Contributor

Choose a reason for hiding this comment

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

There might be consistency issues, e.g. branches starting from different roots. But I guess we just let the client to deal with that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm curious about when that can happen, you mean branches with different genesis?

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I meant something like that:

o - o - o - o - o - o - o
         \ - o - o - o
                 ^
                 |
                 We make request from this height and
                 get two incomplete branches instead
                 of a "proper" tree

Copy link
Contributor

Choose a reason for hiding this comment

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

There should be more documentation on how the blocks will be ordered in presence of multiple branches. Perhaps the service needs only to guarantee the order of parent-child relationships, i.e. never send a child before its parent.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's clarify how would a client use this API.

If I run an explorer and have several branches with different chain lengths in storage, should I request sync with the shortest length I have? Or should I start from the stability depth, since the branches I have stored might have been discarded by the consensus?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Until the explorer has a database I guess you need to start from 0. But after that, I was thinking on starting from min(stability_depth, last_block_I_have). It may lead to sending a lot of redundant blocks, though... And you would need a way of knowing the current length (or the current stable block)

Also, I was thinking on sending all blocks of a given height before the next one, separate branches can be applied potentially in parallel, so I think it's better than sending one branch at a time.

I did write this because it was the simplest thing I could think of, though... Originally I thought about the explorer sending the list of tips it has, but that's not that simple I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've been also been wondering if this endpoint shouldn't be merged with the live block subscription, because otherwise there is a chance of missing a block in the middle if you switch from one stream to the other.

In that case the block subscription could have an optional parameter to sync before, and then start sending new blocks.

But clients can work around that by starting both I guess, I'm not really sure what's idiomatic here.

}
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

1 change: 1 addition & 0 deletions chain-watch/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
tonic::include_proto!("iohk.chain.watch");