Refine cardano-node executable dependencies

[kk-hainq]

Previously we were building cardano-node & cardano-cli executables from source with cabal. This is not very nice because:

• It builds the executables with this codebase's cabal.project, forcing the codebase to follow cardano-node's cabal.project as tightly as possible, which is error-prone and inflexible. For instance, bumping the commit of plutus in our cabal.project to get smaller script sizes (new compiler optimizations or something) risks building an untested version of the node with a new plutus-ledger-api leading to failed tests and unexpected behaviors.
• It is slow as it doesn't reuse most of cardano-node's Nix cache and adds extra overheads to our own Cabal caches.
• It complicates our own Cabal setup with dependencies we don't even use for Hydra, like ekg-forward. In fact, if we remove cardano-node (only keep cardano-api) from cabal.project for good then we'll be able to get rid of optparse-applicative, hedgehog-extras, and cardano-config. The trade-off is to (re)define a few orphan instances (only for tracing) in Hydra.Network.Ouroboros, which I guess is acceptable?

As a solution, this PR avoids building the cardano-node executables from source with our own cabal.project, and populates more standard ones to the default nix-shell for convenience. Outside of it, we should advise people to get a stable binary directly, build from source, etc.

Reference: IntersectMBO/plutus-apps#213.

[ch1bo]

We had it like this in the past. Not sure though why we moved away from it 🤔

[abailly-iohk]

I did not dug into the commits but IIRC the main reason why we moved away from pulling the cardano-node and cardano-cli executables in nix was that this would introduce a potential discrepancy between the version of the API we are using inside Hydra node and the version of Cardano node (which includes API, ledger, plutus) we are using for our tests.
Given that we unfortunately still need to have source-level dependencies to the whole Cardano stack in the cabal.project for compiling Hydra, there's a nice mechanism to pull executables from those dependencies within the test context available in Cabal (for a once, something convenient in Cabal...), we can still benefit from cache hits from CI builds and hydra-node.cachix.com.
The real issue should be fixed upstream, by publishing versioned packages to hackage or another repository, so that we can stop needing to juggle with Git commits across 5 repositories to get things right.

[kk-hainq]

I did not dug into the commits but IIRC the main reason why we moved away from pulling the cardano-node and cardano-cli executables in nix was that this would introduce a potential discrepancy between the version of the API we are using inside Hydra node and the version of Cardano node (which includes API, ledger, plutus) we are using for our tests.

I guess the relevant question to ask is what is the relationship between Hydra and a Cardano node? If we don't test against "standard" builds of the node then wouldn't it risk forcing Hydra operators to build a Cardano node from the source with the dependencies that Hydra specifies for stability? On the other hand, pushing for standard node support means Hydra users can just integrate it with whatever (standard) node setup they are using. The current setup already tracks Cardano node dependencies so it does aim for the same effects, just with more overheads.

The real issue should be fixed upstream, by publishing versioned packages to hackage or another repository, so that we can stop needing to juggle with Git commits across 5 repositories to get things right.

Exactly! We've been discussing this with the Plutus team to promote it for a while now. It's non-trivial though as it would add more operation overheads to upstream maintainers, and require a coordinated effort across the board. If you're interested, I would love to invite the Hydra team to our developer experience presentations too!

[abailly-iohk]

We are testing against standard node: The dependencies we have is tied to some cardano-node version (1.31.0). At some point, we should make it clearer what are the ties with Cardano.

[kk-hainq]

@abailly-iohk The current cabal.project comments so, but actually doesn't test against a standard node. Cabal cannot guarantee that all dependencies are tied. For example, the listed cabal index here is quite old, yielding many dependency mismatches among Hackage packages. cabal build cannot also guarantee a "correctly" patched libsodium when building the node, and more.

I might be wrong but doesn't Hydra only want the same ledger (for transaction validation) with layer one as a common ground for consensus, with everything else like node, consensus, etc. unrelated? If so, while cardano-api makes sense for ledger interactions, cardano-node as a dependency is a huge confusion. It is not healthy to drive Hydra dependencies just to build an as-standard-as-possible node out of the codebase. If anything it should be an as-standard-as-possible cardano-ledger.

Realistically, a Cardano node should only be an external agent for Hydra to interact with, not an internal dependency. We should not manage it inside this codebase. Even the standard binaries in nix-shell should only be a convenience for testing.

Other dApps also need to interact with a Cardano node to sync data and submit transactions, etc. Listing and building a node inside every smart contract codebase to test such integration sounds strange.

[abailly-iohk]

@kk-hainq I am sorry but I don't really understand your arguments. What I know is that I spent a couple days battling with inconsistencies between shell.nix and cabal.project back in October: Here is a link to our Logbook with some explanations why this change was done: https://github.com/input-output-hk/hydra-poc/wiki/Logbook-2021-H2#2021-10-15

I personally don't want to impose the use of nix to our users, so the more we do outside of nix the better IMO.

[kk-hainq]

What I know is that I spent a couple days battling with inconsistencies between shell.nix and cabal.project back in October: Here is a link to our Logbook with some explanations why this change was done: https://github.com/input-output-hk/hydra-poc/wiki/Logbook-2021-H2#2021-10-15

The log and corresponding PR were too vague to tell what was the problem. However, if we could not make Hydra integrate with the cardano-node fetched in nix-shell like this then it wouldn't work with the nodes most SPOs are running today. Removing it from the nix-shell doesn't change that fact. If anything it actually meant you avoided the standard node back then.

I personally don't want to impose the use of nix to our users, so the more we do outside of nix the better IMO.

As I've mentioned, the cardano-node executables in the nix-shell are just a convenience for testing like in CI or during the development of Nix users. The main point is that if anyone needs a Cardano node to integrate (testing) with Hydra, they should get a standard one through Nix caches, pre-built binaries, Docker, or building from the source of cardano-node, anything but building from the source of hydra-poc. Hydra and a Cardano node are two separate agents that talk to each other, not a build dependency of each other.

Another example: If your web servers need nginx for a proper integration test, you should get nginx from a package manager, Docker, or building from the source of nginx, not from your web servers code-base.

[abailly-iohk]

I totally agree that having to build the cardano-node from source is a problem. And that having dependencies on specific commits is also a problem. And that mixing dependencies which are needed for the Head's ledger and the End-to-end test is another problem. I am just saying I am reluctant with changing the statu quo because I know for a fact that each time we have needed to update dependencies has been a major PITA, but it will need to change for 1.0.0.

[abailly-iohk]

Also, one depends on the cardano-node binary for running the end-to-end tests, not for running the hydra-node, so users of Hydra will never need that. And we will distribute binaries anyway. And we'll have to provide some kind of compatibility matrix because we are somewhat dependent on the version of the node we are talking to, at least the version of the protocols.

[abailly-iohk]

What's the standard way of installing a node? According to https://docs.cardano.org/getting-started/installing-the-cardano-node one can download binaries from https://hydra.iohk.io/build/12243146/download/1/cardano-node-1.33.0-linux.tar.gz.
What I would like to avoid is inflicting to ourselves and other developers hard to troubleshoot errors in our integration and end-to-end tests just because of dependencies drift: If there are 2 places where on need to change a version to get things right, that's one 2 many.
Regarding your remark about nginx: If I were to depend on nginx or any executable for that matter, to run integration tests, I would try very hard to make the installation of the executable part of the build process and avoid manual operations from developers. It's not always possible, and we are not always consistent with that, but I think that's an important goal to keep in mind. Our current setting is cumbersome, because cabal is cumbersome, but it ensures we only need to change dependencies in a single place and when one does cabal test hydra-cluster she does not need to care about having an executable in her path, it's all handled by the build system. What you propose would be objectively better if not for the fact it happens at an additional layer, eg. in the nix-shell and thus requires the user to install and use nix, or install manually the dependency.
How about enhancing the CardanoCluster and/or CardanoNode code to check executables' version and report a failure if it's not the one expected (say 1.33.0 for example)? Or download it automatically as part of test setup? Or at the very least provide alternative instructions to nix-shell?

[kk-hainq]

I am just saying I am reluctant with changing the statu quo because I know for a fact that each time we have needed to update dependencies has been a major PITA, but it will need to change for 1.0.0.

I genuinely believe that building the node from our source contributes greatly to the problem. If we remove it for good then we naturally get rid of optparse-applicative, hedgehog-extras, ekg-forward, cardano-config, and many more packages as the node has a massive dependency tree. There is no good reason to add extra maintenance from our side here, especially when there is no upstream Hackage support yet.

Also, one depends on the cardano-node binary for running the end-to-end tests, not for running the hydra-node, so users of Hydra will never need that.

Exactly why we should remove it from our dependency tree, to perform integration testing with standard nodes from Nix caches, Docker, etc. If such an integration test fails, building the node from our source is a dodge, not a fix. We should instead slap Hydra until it starts talking to the standard nodes properly.

And we will distribute binaries anyway. And we'll have to provide some kind of compatibility matrix because we are somewhat dependent on the version of the node we are talking to, at least the version of the protocols.

Very nice!

What's the standard way of installing a node? According to https://docs.cardano.org/getting-started/installing-the-cardano-node one can download binaries from https://hydra.iohk.io/build/12243146/download/1/cardano-node-1.33.0-linux.tar.gz.

I would say downloading pre-built binaries/Docker images/Nix caches from IOG, or building from the source of cardano-node following their instructions.

What I would like to avoid is inflicting to ourselves and other developers hard to troubleshoot errors in our integration and end-to-end tests just because of dependencies drift: If there are 2 places where on need to change a version to get things right, that's one 2 many.

I agree. I guess a compatibility matrix and a few options to auto-fetch stable compatible binaries would be best.

Regarding your remark about nginx: If I were to depend on nginx or any executable for that matter, to run integration tests, I would try very hard to make the installation of the executable part of the build process and avoid manual operations from developers. It's not always possible, and we are not always consistent with that, but I think that's an important goal to keep in mind.

I totally agree.

Our current setting is cumbersome, because cabal is cumbersome, but it ensures we only need to change dependencies in a single place and when one does cabal test hydra-cluster she does not need to care about having an executable in her path, it's all handled by the build system.

Yeah, the lack of Hackage support doesn't help either. Let's push for it!

What you propose would be objectively better if not for the fact it happens at an additional layer, eg. in the nix-shell and thus requires the user to install and use nix, or install manually the dependency.

The Nix thing was really just a convenient example, and to also keep CI sane. We can, and as you said, really should add more options. For what it's worth, I've gone to great lengths to criticize Nix for bad developer experience on Cardano, like in IntersectMBO/plutus-apps#180.

How about enhancing the CardanoCluster and/or CardanoNode code to check executables' version and report a failure if it's not the one expected (say 1.33.0 for example)? Or download it automatically as part of test setup? Or at the very least provide alternative instructions to nix-shell?

All sound wonderful to me! For the last one I think we can have nix-shell, docker-compose, and a shell script that auto-fetches compatible binaries to a local temp directory.

Anyhow, we have been actively working on improving dependency management across the ecosystem and are happy to serve Hydra in at least 2022 too :).

[ch1bo]

We had a quick discussion also internally and would like to merge this, but only if

• the relevant tests/benches do pin the cardano-node --version expected in scope, and
• mention the new dependency in the https://github.com/input-output-hk/hydra-poc#without-nix section of the README.

Rationale for the pinning: If someone from the community does run our (integration) tests, but would use a non-standard environment for that - for example cabal and ghc set up using ghcup - then, the test suite might work.. or it might not, depending on which version of cardano-node they also have in scope. In that case, we really want to spending time on figuring out why they have failures we would not be able to reproduce at the cost of bumping cardano-node versions now and then.

WDYT?

(@abailly-iohk I just realize that using the nix-shell environment to not pin the version twice makes no sense, as it would only be necessary for non-nix-shell users and then the ENV naturally is not there 😓)

[ch1bo]

@kk-hainq Made your branch fast-forward and added the mentioned check from above. Needed to open a new PR as I have no write permission on your fork: #329