Expose Key interface in Cardano.Api.Shelley

[ch1bo]

Sorry for this unsolicited PR, it is not crucial for us, but we wanted to get also your feedback on:

In the Hydra project we are using the cardano-api quite heavily and thought we could distinguish Hydra key types using the same framework, but just with a new Hydra key role.

Unfortunately it was not possible because some of the type class definition was not exported.

This PR exposes the full type class to be able to define instances of it and having a Hydra key role, this yields SigningKey Hydra and VerificationKey Hydra key types.

See also this PR: cardano-scaling/hydra#398

[ch1bo]

Any chance to get a review on this?
@dcoutts
@Jimbo4350
@erikd
@newhoggy
@JaredCorduan

[JaredCorduan]

it looks good to me, and I especially love ❤️ the added documentation. but I'm not really an owner of the API and I don't know if not exposing the full type class was intentional...

[ch1bo]

I especially love the added documentation.

😮 There should not be any documentation changes. Was probably in diff because it was not fast-forward to master again. Rebased now.

[Jimbo4350]

@ch1bo So to me, I think you should implement the instance in cardano-api. Why? Because then we don't have scattered cardano-api Key instances . Exporting the Key typeclass I think is a bad idea.

[ch1bo]

@ch1bo So to me, I think you should implement the instance in cardano-api. Why? Because then we don't have scattered cardano-api Key instances . Exporting the Key typeclass I think is a bad idea.

What in particular makes you think it's a bad idea?

Adding the Hydra Key instances to the cardano-api would make them part of this API and any breaking change to it would mean a breaking change of the API, e.g. when switching the digital signature scheme - this is even likely to happen.

We understood the Key typeclass to be an extension point and wanted to avoid some boilerplate downstream. Is this not the case?

[Jimbo4350]

What in particular makes you think it's a bad idea?

A user wanting to interact with "cardano" via the api will now have to search for the hydra key instances (if they also want to use them). It would be better if everything lived in cardano-api so a user does not have to go searching for the things they need.

Adding the Hydra Key instances to the cardano-api would make them part of this API and any breaking change to it would mean a breaking change of the API, e.g. when switching the digital signature scheme - this is even likely to happen.

This is fine

We understood the Key typeclass to be an extension point and wanted to avoid some boilerplate downstream. Is this not the case?

You can add what you need to cardano-api. What about a Cardano.Api.Hydra module? I can give you permission in the CODEOWNERs file to make changes to it without my approval.

[ch1bo]

A user wanting to interact with "cardano" via the api will now have to search for the hydra key instances (if they also want to use them). It would be better if everything lived in cardano-api so a user does not have to go searching for the things they need.

A user wanting to interact with "cardano" does not need the Key Hydra instance. This will only be used from within the hydra-node to manage signing keys to be used in the Hydra Head protocol. Even users of the hydra-node would not need this Key instance, e.g. when implementing a Hydra client. Should they need/want access to some key management functionality for these keys in Haskell, we would provide a hydra-api package containing these things (does not exist right now). I think it would makes sense that a layer two protocol, which is also operated via a software component running on top of the cardano-node, to provide an api on top of the cardano-api.

Despite this, are there any practical issues with exposing the full Key type class?

You can add what you need to cardano-api. What about a Cardano.Api.Hydra module? I can give you permission in the CODEOWNERs file to make changes to it without my approval.

Thanks @Jimbo4350, I'm happy to help maintaining cardano-api and cardano-cli, i.e. the parts most likely seeing contributions from Hydra.

[Jimbo4350]

This will only be used from within the hydra-node to manage signing keys to be used in the Hydra Head protocol

Should they need/want access to some key management functionality for these keys in Haskell, we would provide a hydra-api package containing these things (does not exist right now)

Ok fair enough.

[ch1bo]

bors r+

[iohk-bors]

Build succeeded:

• buildkite/cardano-node
• ci/hydra-build:required

[dcoutts]

I don't understand why this PR also stopped exporting some existing functions. Was this a mistake or is there some reason?