IntersectMBO / IntersectMBO/ouroboros-consensus
[BUG] - `NodeToClientV_16` protocol changed unexpectedly
- 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**

**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
Assessment
This issue has not been assessed yet.