-
Notifications
You must be signed in to change notification settings - Fork 0
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
Draft Cardano Decentralized Message Queue CIP #1
Conversation
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.
Looks good and I could have done this review already on the upstream repo. That we would also get some guidance on structuring the document (i.e. my appendix comment)
In this section, we propose an architecture for `Cardano` and `Mithril`. Note, | ||
in this section, `Mithril` is used as a placeholder for a possible `Mithril` | ||
application: we've seen many ideas about how `Mithril` can be used in `Cardano` | ||
and all of them can follow this design. |
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.
This is a weird section and does not fit as a top-level section the CIP template.
I think it's great that we can give more details how mithril and cardano processes would interact, but ultimately the CIP is about the network protocol.
Maybe it would fit better as an Appendix titled "Cardano/Mithril node architecture" or so?
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 disagree I think this should be clear from the beginning and it is not. In the abstract, motivation and protocol implementation sections, there's only mention of cardano-node and its network and it leads the reader to think this is going to be an addition to them. Sure mithril is about the protocol, but as we already have discussed there's no strong reason why this protocol should be added to the protocol stack of cardano-node. The viable option that we settled on was what this "architechure" section describes. This is the optimal route, even for Peras and Leios.
In this section, we propose an architecture for `Cardano` and `Mithril`. Note, | ||
in this section, `Mithril` is used as a placeholder for a possible `Mithril` | ||
application: we've seen many ideas about how `Mithril` can be used in `Cardano` | ||
and all of them can follow this design. |
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 disagree I think this should be clear from the beginning and it is not. In the abstract, motivation and protocol implementation sections, there's only mention of cardano-node and its network and it leads the reader to think this is going to be an addition to them. Sure mithril is about the protocol, but as we already have discussed there's no strong reason why this protocol should be added to the protocol stack of cardano-node. The viable option that we settled on was what this "architechure" section describes. This is the optimal route, even for Peras and Leios.
|
||
## Acceptance Criteria | ||
|
||
1. A Cardano node implementing the previously described mini-protocols is released for production. |
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 don't think the wording is correct here. It should not be "a cardano node" but rather "a mithril node", right?
1. A message producer node (e.g. a Mithril signer) publishing messages to the local Cardano node through mini-protocols is released. | ||
1. A message subscriber node (e.g. a Mithril aggregator) receiving messages from the local Cardano node is released. |
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 so I think it should be made even more clearer (in the earlier sections or even in the architecture section) that the final solution implies 3 pieces of software:
- the mithril node
- the signer
- the aggregator
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 feels like the Mithril node will be considered part of the Cardano node as they will be bundled together.
|
||
We propose to create a decentralized message diffusion protocol leveraging the Cardano network layer. This protocol allows to follow a topic based publish-subscribe pattern to diffuse messages from publisher nodes to subscriber nodes in a decentralized way. | ||
|
||
The messages can be sent and received by nodes running in a non intrusive way side by side to the Cardano node in order to enable inter-nodes communications. |
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.
non intrusive way side by side to the Cardano node
This is a weird way to hint at the architecture section. I would just say it instead of being mysterious about it.
The messages can be sent and received by nodes running in a non intrusive way side by side to the Cardano node in order to enable inter-nodes communications. | |
The messages will be sent and received by mithril nodes. Mithril nodes are implemented using the cardano node network stack and will have to run side by side with full cardano nodes. |
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.
We want to stay generic in the formulation as Mithril is only one of the use cases
CIP-0XXX/README.md
Outdated
> [!WARNING] | ||
> We also probably need to make sure that the KES key used to sign is from the latest rotation: | ||
> - either the last seen opcert number in the block headers of the chain. | ||
> - or the last seen opcert number from a previous message diffused. | ||
> | ||
> If the opcert number received is strictly lower than the previous one which has been seen, it should be considered as a protocol violation. |
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 would also add a note saying that there might be needed an extra step of handshake to negotiate a mithril magic to forward the connection to the mithril node.
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.
Maybe it's a bit too much of an implementation detail?
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.
A standalone Mithril node will use its own handshake
; it can introduce its own protocol parameters, but quite likely it will start with NodeToNodeVersionData
:
data NodeToNodeVersionData = NodeToNodeVersionData
{ networkMagic :: !NetworkMagic
, diffusionMode :: !DiffusionMode
, peerSharing :: !PeerSharing
, query :: !Bool
}
deriving (Show, Typeable, Eq)
networkMagic
- I think that's useful for debugging, and making sure theMithril
node runs on the right network; but it should be different than the existingnetworkMagic
sdiffusionMode
- this is useful if there are initiator-only nodes, e.g. a Mithril node running next to an edge node (a wallet)peerSharing
- this will be useful, since we want peer sharing in the Mithril networkquery
- that's useful for tools likecardano-cli ping
, so I'd keep it.
|
||
### Description | ||
|
||
The local message submission mini-protocol is used by local clients to submit message to a local node. This mini-protocol is **not** used to diffuse messages from a core node to another. |
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.
from a core node to another
I don't like this wording as it hints that the protocol will run on cardano-node.
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.
From our discussions, the cardano node would be called directly by the n2c mini-protocols
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.
yes but it won't be cardano-node running the protocols right? You'll just be querying its chain/state
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.
The entry point for the n2c will be the cardano-node, and the n2n would be done in the mithril-node (for the use case of Mithril)
|
||
### Description | ||
|
||
The local message notification mini-protocol is used by local clients to be notified about new message received by the core node. |
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.
same here
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.
Same answer for this n2c mini-protocol
c64492e
to
ad8a76a
Compare
@ch1bo, @bolt12 I have taken into consideration your comments and updated the PR accordingly. I feel that the PR is now in a draft version that is ready to be pushed to the CIPs repository upstream. |
7e41918
to
c317daa
Compare
c317daa
to
c1514e1
Compare
Thank you! Yes I agree 👍 |
Available upstream now with cardano-foundation#876 |
Important
This is a buffer PR used for final reviews/comments before submitting to the https://github.com/cardano-foundation/CIPs repository.
This proposal specifies a decentralized message queue leveraging the Cardano network layer. This protocol allows a topic based mechanism to diffuse messages from publishers to consumers in a decentralized way.
(rendered proposal)