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

ADR-0015 - Externalise chain server #230

Closed

Conversation

abailly-iohk
Copy link
Contributor

No description provided.

Copy link
Collaborator

@ch1bo ch1bo left a comment

Choose a reason for hiding this comment

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

I'm not convinced.. why would we want to do that (right now)?

* Configuration (and code) of Hydra node is simplified
* Deployment and operations have more moving parts but are more flexible
* The lifecycle of the Hydra node and the Chain server can be completely separated, we might want to cope with timeouts/disconnections within the _chain client_ to insulate core node logic from the hiccups of the network but that's pretty standard practice in service-based architectures
* The state of the interaction between the Hydra node and the _Chain server_ is completely driven by the former, updated from whatever `OnChainTx` it retrieves
Copy link
Collaborator

Choose a reason for hiding this comment

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

A very concrete consequence would be that the toOnChainTx function will be duplicated in the behavior spec and the mock chain server.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a 10 lines function.

## Consequences

* Keeping the `ChainComponent` logic abstracted is important for testing purposes and ensuring proper separation of concerns
* Configuration (and code) of Hydra node is simplified
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think it's simpler, but complexity of chain stuff is moved out of the hydra-node into another executable in exchange of an additional communication channel. IPC is more complex than just function calls.

Copy link
Collaborator

@KtorZ KtorZ Feb 25, 2022

Choose a reason for hiding this comment

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

It would make the code simpler (in the end IPC are just function calls from within the code ^.^), but it'd make the deployment / infrastructure a bit more complex.. although we already have to start a node (or mock-chain) started next to the hydra-node.

We could even put the cardano-node and its adapter (chain server) within the same container and use containers as the logical abstraction for those component. Whatever's inside the container does not matter so long it provides a chain-server interface.

* The lifecycle of the Hydra node and the Chain server can be completely separated, we might want to cope with timeouts/disconnections within the _chain client_ to insulate core node logic from the hiccups of the network but that's pretty standard practice in service-based architectures
* The state of the interaction between the Hydra node and the _Chain server_ is completely driven by the former, updated from whatever `OnChainTx` it retrieves

**NOTE**: The same principle could be applied for other components of the system, namely the `Network` and the `Ledger`. By making it possible and easy to run those as external services we decrease the footprint of the Hydra Node proper and increase its flexibility of use.
Copy link
Collaborator

Choose a reason for hiding this comment

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

For the network layer this principle does not really make sense IMO. Moving the responsibility to connect to other nodes and distribute generic, serializable messages into a dedicated executable in exchange for needing to connect and exchange the very same messages to/from it via an additional communication channel is not gaining us anything? Other than more moving parts

Copy link
Collaborator

Choose a reason for hiding this comment

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

I concur here; While I can see long-term use-cases for a decoupled network, the network is arguably quite tied to the underlying hydra-node requirements already and having it externalized does strike me as an obvious benefit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea, again, is to separate the complexity of managing the underlying communication channels which the hydra node proper does not care about. What it cares for is the ability to broadcast something to some identified set of parties (multicast address) and receive stuff from it. So the hydra-node could just connect to some service providing that, having to care for and maintain a single connection, and let the magic happens behind the scene whether through ourorobos network, 0mq, rabbitmq, Gcloud queues...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same goes for ledger


* Keeping the `ChainComponent` logic abstracted is important for testing purposes and ensuring proper separation of concerns
* Configuration (and code) of Hydra node is simplified
* Deployment and operations have more moving parts but are more flexible
Copy link
Collaborator

Choose a reason for hiding this comment

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

Correct. And to me, the added moving parts outweigh the flexibility .. right now at least.

Should we want to support more chains or want less coupling because third parties would develop and provide chain access as a standalone component, that trade-off makes maybe sense.


* To be able to scale our benchmarks and test Hydra Heads with more realistic configurations, we resurrected our `MockChain` component. While this is not strictly needed as we could also have set up a cardano private network with each Hydra node having its own Cardano node, it seemed much simpler and still provide us with the information we need as we are interested in the performance of the Head network, not the On-Chain part
* This requires the ability to configure the `hydra-node` executable to use either a `MockChain` or `Direct` chain component to post and observe transactions, which means more command-line options and more code in the node
* There is a clear separation of responsibilities, defined as a [component](./0007-with-pattern-component-interfaces.md) `ChainComponent`, between the core `Node` logic and the `Chain`. The interaction is mediated by "messages" representing the relevant transitions of the Hydra Head state machine, for posting transaction to the chain and observing them. This interface is completely agnostic of the actual chain semantics and, as hinted in the [Hydra paper]() could very well be implemented in a lot of different ways, and over a lot of different blockchains.
Copy link
Collaborator

Choose a reason for hiding this comment

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

## Consequences

* Keeping the `ChainComponent` logic abstracted is important for testing purposes and ensuring proper separation of concerns
* Configuration (and code) of Hydra node is simplified
Copy link
Collaborator

@KtorZ KtorZ Feb 25, 2022

Choose a reason for hiding this comment

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

It would make the code simpler (in the end IPC are just function calls from within the code ^.^), but it'd make the deployment / infrastructure a bit more complex.. although we already have to start a node (or mock-chain) started next to the hydra-node.

We could even put the cardano-node and its adapter (chain server) within the same container and use containers as the logical abstraction for those component. Whatever's inside the container does not matter so long it provides a chain-server interface.

* The lifecycle of the Hydra node and the Chain server can be completely separated, we might want to cope with timeouts/disconnections within the _chain client_ to insulate core node logic from the hiccups of the network but that's pretty standard practice in service-based architectures
* The state of the interaction between the Hydra node and the _Chain server_ is completely driven by the former, updated from whatever `OnChainTx` it retrieves

**NOTE**: The same principle could be applied for other components of the system, namely the `Network` and the `Ledger`. By making it possible and easy to run those as external services we decrease the footprint of the Hydra Node proper and increase its flexibility of use.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I concur here; While I can see long-term use-cases for a decoupled network, the network is arguably quite tied to the underlying hydra-node requirements already and having it externalized does strike me as an obvious benefit.

## Decision

* The Hydra node _executable_ uses a `ChainComponent` that is implemented as a _generic client_ and configured using a single "port" (could be TCP or WS)
* This client interacts with whatever _server_ it connects sending and receiving `PostChainTx` and `OnChainTx` messages in full-duplex mode
Copy link
Collaborator

Choose a reason for hiding this comment

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

So, what I get from this is that this server wouldn't be generic but still quite tailored and aware of Hydra specifics; since it would have to yield OnChainTx and therefore, know how to construct / identify such transactions.

Is it really what we want? I'd rather have that kind of logic lives inside the hydra-node. So perhaps, that chain server needs a slightly higher level interface to yield transactions or some representation of transactions ? Or, are you already thinking long-term about possibly having this piece integrating with something else than Cardano.. thus not leaking Cardano specifics bits here and indeed, pack that transaction identification logic within the external chain-server 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Post and OnChainTx are just a representation of the SM transition for the case of the cardano blockchain.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Post- and OnChainTx are even parameterized by tx right now

@abailly-iohk
Copy link
Contributor Author

We probably don't need this right now. I think we will need it later, and it will be a PITA to do this later.
While working on Mithril we realised it would be great to reuse the networking layer of Cardano, but this is not easy because it's deeply intertwined with the consensus layer. I am aware there is a rationale for that: Actually, there is always a good reason to do things. But then it makes the thing heavier and heavier, it produces a monolith that becomes hard to split once you need to separate things.
I am not going to spend hours arguing for this, my main goal was to materialise some thoughts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants