IntersectMBO / ouroboros-consensus

Implementation of a Consensus Layer for the Ouroboros family of protocols
https://ouroboros-consensus.cardano.intersectmbo.org
Apache License 2.0
31 stars 22 forks source link

Shelley tests must not use FakeVRF in the leadership check #640

Open nfrisby opened 4 years ago

nfrisby commented 4 years ago

I'm labeling this as testing for now. However, this is urgent because we're not yet sure if it's localized to the tests. I'll update this description once we know more. (Edit: I believe it is localized to the tests.)

When the OBFT overlay is in control, active slots have one leader. But when Praos determines leaders, every active slot is lead by all nodes. Specifically, the argument to checkLeaderValue called from checkIsLeader is the same for all nodes (as observed via Debug.Trace).

cc: @edsko @mrBliss @nc6

nfrisby commented 4 years ago

I think this is a problem in the tests only.

ouroboros-consensus-shelley-test uses ConcreteCrypto.

https://github.com/input-output-hk/ouroboros-network/blob/96781ffae95ac68937b4bea0150f265c8441ae0c/ouroboros-consensus-shelley-test/src/Test/ThreadNet/Infra/Shelley.hs#L61

https://github.com/input-output-hk/ouroboros-network/blob/96781ffae95ac68937b4bea0150f265c8441ae0c/ouroboros-consensus-shelley-test/src/Test/Consensus/Shelley/MockCrypto.hs#L21

cardano-ledger-specs defines ConcreteCrypto to use FakeVRF.

https://github.com/input-output-hk/cardano-ledger-specs/blob/d9bdbfd4d35892e40487fba2913b4da196b5dcea/shelley/chain-and-ledger/executable-spec/test/Test/Shelley/Spec/Ledger/ConcreteCryptoTypes.hs#L53

  type VRF (ConcreteCrypto h) = FakeVRF

cardano-ledger-specs defines FakeVRF so that the signing key does not influence the output.

https://github.com/input-output-hk/cardano-ledger-specs/blob/d9bdbfd4d35892e40487fba2913b4da196b5dcea/shelley/chain-and-ledger/executable-spec/test/Test/Cardano/Crypto/VRF/Fake.hs#L130-L139

evalVRF' ::
  SneakilyContainResult a =>
  a ->
  SignKeyVRF FakeVRF ->
  (OutputVRF FakeVRF, CertVRF FakeVRF)
evalVRF' a (SignKeyFakeVRF n) =
  let y = sneakilyExtractResult a
      p = unsneakilyExtractPayload a
      realValue = fromIntegral . fromHash . hashWithSerialiser @MD5 id $ toCBOR p
   in (y, CertFakeVRF n realValue)

The key fact about VRFs here is that the signing key is the only part of the leader check that is specific to each node.

mrBliss commented 4 years ago

@nc6 is going to fix this in cardano-ledger-specs: the key will be taken into account in evalVRF'.