IntersectMBO / IntersectMBO/ouroboros-consensus

[BUG] - `NodeToClientV_16` protocol changed unexpectedly

Open
#1,130 5 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Haskell
Stars
67
Forks
43
Avg merge
5d 13h
Merged PRs (30d)
43

Description

**Internal/External**
*External*

**Summary**

The `NodeToClientV_16` protocol has changed unexpectedly between two released versions of the cardano-node: version `8.9.2` accepts queries that `8.11.0-sancho` rejects, and vice versa.

**Steps to reproduce**
(Discovered while testing `cardano-wallet` against Sanchonet. Screenshot of an example query failure below.)

**Expected behavior**

Changes in protocol semantics should come with a new version, here `NodeToClientV_17`.

Downstream projects such as `cardano-wallet` are mainnet-ready and therefore need to be tested against `cardano-node` versions that are also mainnet-ready, in this case `8.9.2`. However, in order to make Sanchonet more useful, it is also desirable to have `cardano-wallet` work on Sanchonet. However, having a node that is ready for mainnet and a different node that is compatible with Sanchonet now requires us to test against two different node versions — especially if their interface changes.

One can argue that `NodeToClientV_16` is experimental and subject to change (that's why it can be disabled in the node configuration file). However, the point of version numbers is that any particular version is *not* subject to change, e.g. `cardano-node-8.9.2` is *not* subject to change anymore, it only becomes obsolete. Still, there is the matter of granularity: In the case of the `NodeToClientVersion` type, it was probably felt that adding `NodeToClientV_17` would spam the version number space for no good reason. Well, now there is a good reason.

To solve this, I have the feeling that it would be best if `NodeToClientVersion` simply tracks the version of the `ouroboros-consensus` library. When negotiating the protocol, a higher version of `ouroboros-consensus` can choose to explicitly support a lower version, e.g. `ouroboros-consensus-0.18` can include code that works with `ouroboros-consensus-0.15` as well. I believe that this would semi-automated the version tracking. However, this solution is also too good to be true: The block type may differ between versions of a dependency, which is *not* tracked in the version of `ouroboros-consensus`. 🤔 The version number of `cardano-node` would provide a good tracking reference (and in practice, it is used as such), but that is a downstream package, which imports the `NodeToClientVersion` type rather than exporting it.

**System info**
- OS Name: NixOS, `x86_64-linux`.
- Consensus version: `ouroboros-consensus-0.14.0.0` vs `ouroboros-consensus-0.18.0.0`.

**Screenshots and attachments**

![node-to-client](https://github.com/IntersectMBO/ouroboros-consensus/assets/564592/c1758ab6-820b-41c6-bf72-726e901a4a33)

**Additional context**

This defect was introduced in https://github.com/IntersectMBO/ouroboros-consensus/pull/1015 .

The change to the semantics of the `NodeToClientV_16` protocol happened here:
https://github.com/IntersectMBO/ouroboros-consensus/commit/3fc6d28f00b0a1b0d3187cb7916f1e192dd79fcb#diff-28148a724bec997ce56e6643032266b891d3e841987ec42462d88e29245f18ae

A better solution would have been to define `CardanoNodeToClientVersion13` and to add a line

```hs
supportedNodeToClientVersions _ = Map.fromList $
[ …
, (NodeToClientV_17, CardanoNodeToClientVersion13)
]
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.