SPO on-chain poll commands adjustments

[KtorZ]

Description

Follow-up from a conversation with @dcoutts, I proceeded with the following changes:

• The option to prove with the VRF secret key has been removed, the only option now is to use the cold key
• The witness is no longer included in the metadata themselves, but expected to be part of the standard transaction witness set (requires extra signatories on the submitting transaction)

I have also adjusted the tutorial accordingly.

Checklist

• Commit sequence broadly makes sense and commits have useful messages
• New tests are added if needed and existing tests are updated. These may include:
  • golden tests
  • property tests
  • roundtrip tests
  • integration tests
    See Runnings tests for more details
• Any changes are noted in the CHANGELOG.md for affected package
• The version bounds in .cabal files are updated
• Code is linted with hlint. See .github/workflows/check-hlint.yml for to get the hlint version
• Code is formatted with stylish-haskell. See .github/workflows/stylish-haskell.yml for to get the stylish-haskell version
• Self-reviewed the diff

---

History

• 📍 Remove VRF signing option for producing SPO on-chain poll answers
  And also, remove the poll witnesses altogether from metadata.
  Since we'll now only rely on cold key signing, we can rely on the
  existing witness mechanism of transactions (combined with the
  'required extra signatories' field).

  This slightly adjust the 'verifyPollAnswer' function to now work
  directly from a transaction and return the actual signatories.
  Consumers can then verify whether signatories meet their expected
  criteria.

• 📍 Re-implement command-line handler for 'governance verify-poll'
  As well as finalize the metadata extraction logic on the API's side.
  This is mostly the same thing as before, but it now takes a signed
  transaction file for verification instead of a standalone metadata.

  ❯ cardano-cli governance verify-poll --poll-file polls/basic.json --tx-file verify/valid
  Found valid poll answer, signed by:
  [
      "f8db28823f8ebd01a2d9e24efb2f0d18e387665770274513e370b5d5"
  ]
  

  ❯ cardano-cli -- governance verify-poll --poll-file polls/basic.json --tx-file verify/none
  Command failed: governance verify-poll  Error: No answer found in the provided transaction's metadata.
  

  ❯ cardano-cli -- governance verify-poll --poll-file polls/basic.json --tx-file verify/malformed
  Command failed: governance verify-poll  Error: Malformed metadata; couldn't deserialise answer: An error occured while decoding GovernancePollAnswer.2.
  Error: missing mandatory field
  

  ❯ cardano-cli -- governance verify-poll --poll-file polls/basic.json --tx-file verify/invalid
  Command failed: governance verify-poll  Error: Invalid answer (42) not part of the poll.
  Accepted answers:
  0 → yes
  1 → no
  

  ❯ cardano-cli -- governance verify-poll --poll-file polls/basic.json --tx-file verify/mismatch
  Command failed: governance verify-poll  Error: Answer's poll doesn't match provided poll (hash mismatch).
  

• 📍 Use more meaningful (phantom) types for files around governance commands.

• 📍 Rework integration tests for governance commands

  • Cover new error cases for 'verify'
  • Create full-blown test transactions for 'verify'
  • Slightly re-organised and re-structure data folder
  • Remove now-unnecessary old files

  ❯ tree cardano-cli/test/data/golden/shelley/governance
  .
  ├── answer
  │   └── basic.json
  ├── cold.sk
  ├── cold.vk
  ├── create
  │   ├── basic.json
  │   └── long-text.json
  ├── polls
  │   ├── basic.json
  │   └── long-text.json
  └── verify
      ├── invalid
      ├── malformed
      ├── mismatch
      ├── none
      └── valid
  

[dcoutts]

This is great. Thanks @KtorZ, especially for helping us get this feature in on short notice.

A couple general comments:

I think it could be clearer that the final step of answer verification is that one needs to verify who is answering the poll, and that this step is manual. It's not enough to just run the command, it merely checks that someone answered the poll. I don't have a very strong opinion on where that should be clearer. The options I suppose are the cli output "and now do this", any readme/docs or in the CIP. Your original PR has a nice tutorial section but I didn't see that end up in any permanent docs. It'd be nice to capture that. That'd be a good place to call it out.

On a similar point, I don't see any way to verify poll questions. As an SPO I don't want to be tricked into signing some hash and index by anyone. I want to know the poll question came from the CF or whatever. I guess this is perhaps something to address in the CIP first rather than here in this PR. In an implementation perhaps it could be done automagically by checking the signer is a delegated governance key. Or at a minimum doing what the verify poll command does for answers and print out the vk hash of who proposed the poll question.

[dcoutts]

Your original PR has a nice tutorial section but I didn't see that end up in any permanent docs. It'd be nice to capture that.

Oops, missed the fact that you've got a tutorial at https://hackmd.io/@KtorZ/cip-0094-tutorial

[KtorZ]

Regarding the verification of the poll answers, I am not too worried about that because this a step mainly taken by power users (eg: the CF). I originally introduced this command because of the VRF option and the lack of general support for VRF in other ecosystems. Now that this solely rely on Ed25519 keys I don't think this verify command has much benefits and I don't think we'll even be using it when processing results (but simply replicate the same logic on scripts consuming that data).

Having said that, it wouldn't hurt to have one or two extra lines in the cli output (or at minima in the tutorial) to highlight the expected verification steps.

Regarding the verification of the question, that's a fair point. I expect in practice to be enough ceremony around questions with proper announcements, bells and whistles to make the authenticity of a survey sufficiently obvious. We should however encourage people, in the tutorial at the very least, to check the signatories and and control that a poll was emitted from a transaction signed by a genesis delegate. This is something we can certainly provide from a CF perspective (genesis key hashes are publicly known anyway and part of the genesis configuration). I expect IOG to also provide adequate disclosure of their genesis verification key (hashes) should you decide to also conduct your own survey through this method.

[Jimbo4350]

Rendering the failures in the property tests will make it easier for us to debug failures if they ever arise.

Potential way to distinguish stake pool keys amongst the additional key witnesses: #5132 (comment)

[KtorZ]

Review feedback integrated.

• verifyPollAnswer now only returns the list of extra signatories, and fails with a proper error when there's none. (cc @dcoutts)

• The command's output of answer-poll now also indicates that the transaction carrying the answer must be signed with a valid key and hint to the cold key. I've left the message relatively "flexible" so that we can easily introduce proxy keys later without having to change the command-line (cc @dcoutts)

• Integration tests now use failWith & displayError when relevant. (cc @Jimbo4350)

• Option to verify-poll renamed to --signed-tx-file (cc @newhoggy)

[newhoggy]

I was cleaning up the golden tests and something came up whilst I was doing it.

I noticed the negative test case for governance answer-poll was option 3 which is curious because option 2 also is a failure case.

I then tried 0 which is a success case.

The behaviour is that the numbering is 0 based but the test suggests you were going for 1 based.

I wanted to ask to be sure the 0 base was actually the intention.