Create file with Cardano minimum versions in repository

[jpraynaud]

Why

Following the discussion #1606, we want to advertise the supported Cardano node versions for the Mithril signer.

What

Add a cardano-min-versions.json file at the root of the repository which details the minimum supported versions:

{
  "about": "minimum supported Cardano node versions for Mithril signers",
  "network":{
    "mainnet": "8.7.3",
    "preprod": "8.9.1",
    "preview": "8.9.1",
    "sanchonet": "8.9.1"
 }
}

How

• Create the file in the repository
• Create a runbook for maintaining the file
• Add check points in Release 2418 distribution #1614
• Add a section in the Run a Mithril signer as an SPO to explain how to check compatibility
• Update section

  mithril/docs/website/root/manual/getting-started/run-signer-node.md

  Line 96 in a7d6f38

  * Install a recent version of [`cardano-cli`](https://github.com/input-output-hk/cardano-node/releases/tag/8.7.3) (version 8.7.3+).

[ch1bo]

Would be great if we can make this future-proof to e.g. also have aggregatorEndpoint per network at some point?

In the hydra project we also created a networks.json file for convenient lookup, BUT we made the mistake of having plain data at the network key (in that case it was for the --hydra-script-tx-id argument).

It's not a big deal, and maybe YAGNI, but would be great if we can have something more future proof like:

{
  "mainnet": {
    "minCardanoNodeVersion": "8.7.3",
    // ....
  }
}

Maybe having a descriptive field name (e.g. minCardanoNodeVersion) makes a use-case specific file name and about field even redundant?

[jpraynaud]

This issue only aims at handling the Cardano minimum versions for the signer, but it could lead to a broader issue about Mithril configurations. In that case, the file used would be named differently (with a different JSON model). The current cardano-min-versions.json would also be deprecated and removed.

Maybe the following format is more future proof as Cardano minimum versions could also be related to other types of nodes on the network (at least the aggregator):

{
  "about": "minimum supported Cardano node versions",
  "network":{
    "mainnet": {
      "minCardanoNodeVersionForMithrilSigner": "8.7.3",
    }
    "preprod": {
      "minCardanoNodeVersionForMithrilSigner": "8.7.3",
    }
    "preview": {
      "minCardanoNodeVersionForMithrilSigner": "8.7.3",
    }
    "sanchonet": {
      "minCardanoNodeVersionForMithrilSigner": "8.9.0",
    }
 }
}

WDYT?

[dlachaume]

This issue only aims at handling the Cardano minimum versions for the signer, but it could lead to a broader issue about Mithril configurations. In that case, the file used would be named differently (with a different JSON model). The current cardano-min-versions.json would also be deprecated and removed.

Maybe the following format is more future proof as Cardano minimum versions could also be related to other types of nodes on the network (at least the aggregator):

{
  "about": "minimum supported Cardano node versions",
  "network":{
    "mainnet": {
      "minCardanoNodeVersionForMithrilSigner": "8.7.3",
    }
    "preprod": {
      "minCardanoNodeVersionForMithrilSigner": "8.7.3",
    }
    "preview": {
      "minCardanoNodeVersionForMithrilSigner": "8.7.3",
    }
    "sanchonet": {
      "minCardanoNodeVersionForMithrilSigner": "8.9.0",
    }
 }
}

WDYT?

LGTM 👍, also remove the about field as suggested by @ch1bo.

[ch1bo]

In that case, the file used would be named differently (with a different JSON model). The current cardano-min-versions.json would also be deprecated and removed.

I realize if we only aim for that, you don't need to consider my feedback because it does not need to be future proof. I guess my comment only holds if you already want to build something more generice (i.e. a networks.json)

If we go for minCardanoNodeVersionForMithrilSigner we should definitely not need about as this is very specific indeed. However, being this specific with a single purpose for the file (as its named cardano-min-versions.json) feels odd. Given the file name, mithrilSigner would be enough IMO. But then the question is the original purpose and scope of all this. I think I have opened pandoras YAGNI box full of scope creep 😅

[jpraynaud]

In that case, the file used would be named differently (with a different JSON model). The current cardano-min-versions.json would also be deprecated and removed.

I realize if we only aim for that, you don't need to consider my feedback because it does not need to be future proof. I guess my comment only holds if you already want to build something more generice (i.e. a networks.json)

If we go for minCardanoNodeVersionForMithrilSigner we should definitely not need about as this is very specific indeed. However, being this specific with a single purpose for the file (as its named cardano-min-versions.json) feels odd. Given the file name, mithrilSigner would be enough IMO. But then the question is the original purpose and scope of all this. I think I have opened pandoras YAGNI box full of scope creep 😅

I agree, let's use this format 🙂 instead

{
  "mainnet": {
    "mithrilSigner": "8.7.3",
  },
  "preprod": {
    "mithrilSigner": "8.7.3",
  },
  "preview": {
    "mithrilSigner": "8.7.3",
  },
  "sanchonet": {
    "mithrilSigner": "8.9.0",
  }
}

[TrevorBenson]

In that case, the file used would be named differently (with a different JSON model). The current cardano-min-versions.json would also be deprecated and removed.

I realize if we only aim for that, you don't need to consider my feedback because it does not need to be future proof. I guess my comment only holds if you already want to build something more generice (i.e. a networks.json)
If we go for minCardanoNodeVersionForMithrilSigner we should definitely not need about as this is very specific indeed. However, being this specific with a single purpose for the file (as its named cardano-min-versions.json) feels odd. Given the file name, mithrilSigner would be enough IMO. But then the question is the original purpose and scope of all this. I think I have opened pandoras YAGNI box full of scope creep 😅

I agree, let's use this format 🙂 instead

{
  "mainnet": {
    "mithrilSigner": "8.7.3",
  },
  "preprod": {
    "mithrilSigner": "8.7.3",
  },
  "preview": {
    "mithrilSigner": "8.7.3",
  },
  "sanchonet": {
    "mithrilSigner": "8.9.0",
  }
}

I like all the thought going into this, as well as the future proofing by explicitly defining which component the Cardano version relates to.

As for the name of the file, I have no strong preference between cardano-min-versions.json, network.json. My only comment would be using the filename that is (currently presumed) least likely to require changing may reduce the "breaking change" a rename will introduce for external/thirdparty tools and scripts that will rely on parsing the file.

[TrevorBenson]

@jpraynaud @dlachaume

Is the name style meant to match something? My first glance at the example I presumed camel case matched a crate/module in the repo, so didn't think much more of it at the time. I was starting on the proof of concept from discussion #1624 based on the example code today. I decided just to poke around a bit more into the current Mithril code and noticed camel case results in zero hits in the repo, so dug in a bit more:

Camel Case

• No instances

  $ grep -r -I mithrilSigner | wc -l
  0
  

Pascal Case

• 25 instances
• Appear to be modules or crates
  • forgive my lack of rust experience and nomenclature if this is incorrect wording, but appears "re-usable"

  $ grep -r -I MithrilSigner | wc -l
  25
  

Snake Case

• 171 instances

• Appears to primarily be variations on variable names

  $ grep -r -I mithril_signer | wc -l
  171
  

Kebab Case

• 361 instances
• Appears to primarily be directory and file names

  $ grep -r -I mithril-signer | wc -l
  361
  

Since snake case is related to variables I'll just drop it from the list with the camel case 0 results. I didn't see anything about naming conventions in the Contributions guidelines other than to use rustfmt. I could also be missing there is a standard JSON library or that rustfmt itself enforces a naming convention of camelCase. Short of either of those, and based solely on Type/Use and commonality, I have 2 suggestions:

1. Go with Pascal Case.

• Aligned with the what appear to be the actual style used for "re-usable" parts of the rust code.
• Not aligned with JSON naming standards.

{
  "mainnet": {
    "MithrilSigner": "8.7.3",
  },
  "preprod": {
    "MithrilSigner": "8.7.3",
  },
  "preview": {
    "MithrilSigner": "8.7.3",
  },
  "sanchonet": {
    "MithrilSigner": "8.9.0",
  }
}

2. Go with Kebab Case

• Aligned with the binaries produced
• Aligned with JSON naming standards.

{
  "mainnet": {
    "mithril-signer": "8.7.3",
  },
  "preprod": {
    "mithril-signer": "8.7.3",
  },
  "preview": {
    "mithril-signer": "8.7.3",
  },
  "sanchonet": {
    "mithril-signer": "8.9.0",
  }
}

My apologies if this seems overly meticulous for a request to define minimum versions. Anything is better than nothing, so if I overlooked a reason to persist camel case please feel free to ignore my request and continue with the camel case style.

EDIT:
My apologies for also ignoring the Google JSON Style Guide. I do tend to use many of their conventions in my Python including strong Typing. Short of a unpublished style guide or rustfmt convention I decided to provide alternatives, which I fully understand if the preference is to stick with camelCase.

[dlachaume]

Thanks @TrevorBenson for your comments.

We've made several changes:

• use of Kebab case
• rename the file name to networks.json
• update the structure to anticipate the addition of further data in the future

networks.json

{
  "mainnet": {
    "cardano-minimum-version": {
      "mithril-signer": "8.7.3"
    }
  },
  "preprod": {
    "cardano-minimum-version": {
      "mithril-signer": "8.7.3"
    }
  },
  "preview": {
    "cardano-minimum-version": {
      "mithril-signer": "8.7.3"
    }
  },
  "sanchonet": {
    "cardano-minimum-version": {
      "mithril-signer": "8.9.0"
    }
  }
}