IntersectMBO / cardano-ledger

The ledger implementation and specifications of the Cardano blockchain.
Apache License 2.0
261 stars 157 forks source link

Use mappings from verification key hashs, not verification keys. #754

Closed dcoutts closed 5 years ago

dcoutts commented 5 years ago

In general we should not be comparing or ordering keys directly. When we need to compare or index on keys we should be using key hashes.

More specifically, the approach is that verification keys have an Eq but no Ord, while signing keys have no Eq or Ord at all.

More or less we do do that everywhere. But in the executable spec for Updates.hs we do have one place where we have a mapping from genesis verification keys to other things. It looks like it would not be a significant change to switch that over to genesis key hashes (in both executable and formal spec)

Specifically:

And we should check for any others. Trying to compile will the latest cardano-prelude crypto classes will reveal any remaining cases.

nc6 commented 5 years ago

Assigning this to myself, since I need to change it for integration with ouroboros-network

JaredCorduan commented 5 years ago

In both the Byron and Shelley formal specs, the genesis key delegation mapping (dms) has type VKeyG |-> VKey. We will need to change these to hashes as well.

JaredCorduan commented 5 years ago

here are the changes I am expecting for the shelley formal spec:

Finally, we should probably also change the UTxO witness mapping to be a set.

nc6 commented 5 years ago

Needs to be fixed in the Byron spec also

dcoutts commented 5 years ago

In the actual implementation for Byron we do not use verification keys in maps, except for the AVVM/redemption case, and the auditors signed off on that exception.

So for Byron the only thing to do is to check the specs agree with the impl about the use of keys vs key hashes.

For the Shelley rules, there's more to do, as Jared notes above.

nc6 commented 5 years ago

@dcoutts Yes, sorry, should have been clearer above - the thing that needs doing for Byron is bringing the specs in line with the implementation

dnadales commented 5 years ago

I've created this issue for Byron https://github.com/input-output-hk/cardano-ledger-specs/issues/760