Use cardano-api Key interface for Hydra keys #398

ch1bo · 2022-06-14T14:44:33Z

This PR rolls up the previous custom types into cardano-api Keyinterface and yields SigningKey Hydra and VerificationKey Hydra key types.

Requires a patch upstream to have the Keytype class fully exposed.

See this PR: IntersectMBO/cardano-node#4048

github-actions · 2022-06-14T18:15:46Z

Unit Test Results

237 tests 235 ✔️ 12m 56s ⏱️
  87 suites     2 💤
    5 files     0 ❌

Results for commit 3b68e69.

♻️ This comment has been updated with latest results.

github-actions · 2022-06-14T18:16:51Z

Transactions Costs

Sizes and execution budgets for Hydra protocol transactions. Note that unlisted parameters are currently using arbitrary values and results are not fully deterministic and comparable to previous runs.

Metadata
Generated at	2022-08-02 14:04:56.190827698 UTC
Max. memory units	14000000.00
Max. CPU units	10000000000.00
Max. tx size (kB)	16384

Cost of Init Transaction

Parties	Tx size	% max Mem	% max CPU
1	4831	11.94	4.76
2	5038	12.35	4.87
3	5243	14.53	5.72
5	5652	17.89	7.00
10	6677	26.97	10.47
45	13855	98.47	38.08

Cost of Commit Transaction

Currently only one UTxO per commit allowed (this is about to change soon)

UTxO	Tx size	% max Mem	% max CPU
1	5771	19.84	8.01

Cost of CollectCom Transaction

Parties	Tx size	% max Mem	% max CPU
1	13065	20.77	8.32
2	13450	37.31	15.12
3	13666	54.34	22.16
4	14093	77.63	31.84
5	14238	98.65	40.63

Cost of Close Transaction

Parties	Tx size	% max Mem	% max CPU
1	9254	7.59	2.99
2	9532	9.41	3.98
3	9692	10.19	4.44
5	10016	11.75	5.34
10	10812	15.21	7.42
30	14184	31.86	16.86
62	15411	37.63	14.26

Cost of Contest Transaction

Parties	Tx size	% max Mem	% max CPU
1	9328	8.16	3.34
2	9526	9.34	3.95
3	9661	9.72	4.24
5	10057	12.12	5.47
10	10848	15.57	7.56
30	14221	32.23	17.00
43	16368	42.55	22.94

Cost of Abort Transaction

Parties	Tx size	% max Mem	% max CPU

Cost of FanOut Transaction

Involves spending head output and burning head tokens. Uses ada-only UTxO for better comparability.

UTxO	Tx size	% max Mem	% max CPU
1	13424	10.13	4.41
2	13459	11.70	5.31
3	13562	13.60	6.34
5	13628	16.34	7.98
10	13747	23.87	12.35
50	15248	84.88	47.60
59	15576	98.85	55.63

4048: Expose Key interface in Cardano.Api.Shelley r=ch1bo a=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 4137: Give myself and John permission to merge any PR r=Jimbo4350 a=Jimbo4350 Co-authored-by: Sebastian Nagel <[email protected]> Co-authored-by: Jordan Millar <[email protected]>

ch1bo · 2022-07-06T09:01:25Z

The upstream PR in cardano-node has been merged. We could be trying to use 0cd6878769a10097e7c780e15efb9919fb94f669, but likely only after #408 has been merged.

ch1bo · 2022-07-11T16:09:39Z

Fast-forwarded this one, but will fail tests as cardano-node HEAD is still missing IntersectMBO/cardano-node#4138.

ch1bo · 2022-08-01T14:47:32Z

Update: The required changes are now on cardano-node master branch, so I bumped the dependencies on this and made it compile & pass tests. I also updated the binary cardano-node in scope to the latest one, in the hopes of its behavior being most closely to the components we depend on via source.

KtorZ · 2022-08-02T09:45:39Z

hydra-node/src/Hydra/Crypto.hs

+-- | The used hash algorithm for 'HydraKey' verification key hashes.
+type HashAlg = Blake2b_256
+
+-- | The used signature algorithm for 'HydraKey' signatures.
 type SignAlg = Ed25519DSIGN


(strong 😅) suggestion: do not use type-aliases here.

This obfuscates things, and provide little benefits. It's replacing one word by another. I would imagine having a type-class that spits back the algorithm from a given Key HydraKey for example if consistency is needed, but the alias makes signatures more opaque.

Plus, this is relatively inconsistent with the rest of the Cardano codebase, which does not use type-aliases for these but rather (a) generic type parameters (.e.g alg -- lowercase) or (b) specialized types like Key PaymentKey where the hash algorithm is fixed anyway behind the scene.

Yeah, makes sense. I was following the pattern we also used in other modules where we fixed the Era or Block types etc. I will follow your suggestion and remove this one indirection 👍

KtorZ · 2022-08-02T09:50:04Z

hydra-node/src/Hydra/Crypto.hs

+ where
+  needed = fromIntegral $ seedSizeDSIGN (Proxy :: Proxy SignAlg)
+  provided = BS.length seed
+  padded = seed <> BS.pack (replicate (needed - provided) 0)


That is slightly odd 🤔 ...

This is already on master :P But it's currently getting fixed by @ffakenz to not pad, but hash the seed for a consistent key derivation.

KtorZ · 2022-08-02T09:51:12Z

hydra-node/src/Hydra/Crypto.hs

-hashVerificationKey (HydraVerificationKey vk) =
-  castHash $ hashVerKeyDSIGN vk
+-- | Create a new 'SigningKey' from a 'ByteString' seed. The created keys are
+-- not random and insecure, so don't use this in production code!


Can we mark it as {-# DEPRECATED #-} ? I think this would prevent us from compiling test code with -WError but .. I see some obvious benefits. This is also enforceable by hlint I think (where we can whitelist some modules where a particular function is allowed) but that supposes we do all run and obey to hlint.

It would probably trip the call site above in deterministicSigningKey (iirc)

KtorZ · 2022-08-02T09:51:45Z

hydra-node/src/Hydra/Node.hs

@@ -69,14 +67,18 @@ initEnvironment Options{hydraSigningKey, hydraVerificationKeys} = do
      , otherParties
      }
 where
+  -- TODO: use text envelopes instead of this maybe fail stuff


KtorZ · 2022-08-02T09:52:50Z

shell.nix

@@ -18,8 +18,8 @@ let
  cardano-node = import
    (pkgs.fetchgit {
      url = "https://github.com/input-output-hk/cardano-node";
-      rev = "1.35.0";
-      sha256 = "06arx9hv7dn3qxfy83f0b6018rxbsvh841nvfyg5w6qclm1hddj7";
+      rev = "1.35.3-testnetonly";


😭 why does this even exist.

This is makes key representation more uniform between Cardano and Hydra credentials, yet distinct.

This commit holds both of our upstream contributions: * The Key data family exposed * A fixed orphan instance of ToJSON Script We also needed to update the dependencies on cardano-base, cardano-ledger and ouroboros-network. Note that these source dependencies are further ahead on master than the binary cardano-node we use.

…tion

Also bump hydra-cluster package version as it seems to not have been updated

This was introduced with babbage (CIP-55) and the cardano-api is (now?) pedantic about this (errors if it is missing).

4048: Expose Key interface in Cardano.Api.Shelley r=ch1bo a=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 4137: Give myself and John permission to merge any PR r=Jimbo4350 a=Jimbo4350 Co-authored-by: Sebastian Nagel <[email protected]> Co-authored-by: Jordan Millar <[email protected]>

ch1bo requested review from KtorZ and a user June 14, 2022 14:44

ch1bo mentioned this pull request Jun 14, 2022

Expose Key interface in Cardano.Api.Shelley IntersectMBO/cardano-node#4048

Merged

ch1bo force-pushed the ch1bo/cardano-api-keys branch from 4e5a9c7 to a1883f7 Compare June 14, 2022 17:10

ch1bo force-pushed the ch1bo/cardano-api-keys branch from a1883f7 to e413f47 Compare July 11, 2022 16:09

ch1bo force-pushed the ch1bo/cardano-api-keys branch from df515ba to e7fb2a5 Compare July 27, 2022 15:59

ch1bo self-assigned this Jul 28, 2022

ch1bo force-pushed the ch1bo/cardano-api-keys branch 3 times, most recently from 628156e to ea7a26b Compare August 1, 2022 14:40

ch1bo force-pushed the ch1bo/cardano-api-keys branch from ea7a26b to 8d572f4 Compare August 1, 2022 14:48

ch1bo requested a review from ffakenz August 1, 2022 15:39

KtorZ approved these changes Aug 2, 2022

View reviewed changes

ch1bo added 10 commits August 2, 2022 15:53

Define Hydra keys via the cardano-api Key interface

db86617

This is makes key representation more uniform between Cardano and Hydra credentials, yet distinct.

Update usage of Hydra.Crypto to new interface

0a84ced

Update cardano-node to have the Key interface exposed

c43a593

Add new/missing golden file for VerificationKey HydraKey

a1c9acc

Re-export Key interface in Hydra.Crypto and fix hydra-cluster compila…

8ee7ce6

…tion

Bring latest tagged cardano-node (1.35.3) in scope

95ead0e

Also bump hydra-cluster package version as it seems to not have been updated

Add utxoCostPerByte to our protocol-parameters.json

125ae96

This was introduced with babbage (CIP-55) and the cardano-api is (now?) pedantic about this (errors if it is missing).

Fix a redundant import in CryptoSpec

581b946

Do not use type aliases in Hydra.Crypto

3b68e69

ch1bo force-pushed the ch1bo/cardano-api-keys branch from e3b9839 to 3b68e69 Compare August 2, 2022 13:53

ch1bo merged commit cd4d773 into master Aug 2, 2022

ch1bo deleted the ch1bo/cardano-api-keys branch August 2, 2022 15:01

ch1bo mentioned this pull request Oct 4, 2022

Use text envelopes for hydra keys #530

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use cardano-api Key interface for Hydra keys #398

Use cardano-api Key interface for Hydra keys #398

ch1bo commented Jun 14, 2022 •

edited

Loading

github-actions bot commented Jun 14, 2022 •

edited

Loading

github-actions bot commented Jun 14, 2022 •

edited

Loading

ch1bo commented Jul 6, 2022

ch1bo commented Jul 11, 2022

ch1bo commented Aug 1, 2022

KtorZ Aug 2, 2022

ch1bo Aug 2, 2022

KtorZ Aug 2, 2022

ch1bo Aug 2, 2022

KtorZ Aug 2, 2022

ch1bo Aug 2, 2022

KtorZ Aug 2, 2022

KtorZ Aug 2, 2022

Use cardano-api Key interface for Hydra keys #398

Use cardano-api Key interface for Hydra keys #398

Conversation

ch1bo commented Jun 14, 2022 • edited Loading

github-actions bot commented Jun 14, 2022 • edited Loading

Unit Test Results

github-actions bot commented Jun 14, 2022 • edited Loading

Transactions Costs

Cost of Init Transaction

Cost of Commit Transaction

Cost of CollectCom Transaction

Cost of Close Transaction

Cost of Contest Transaction

Cost of Abort Transaction

Cost of FanOut Transaction

ch1bo commented Jul 6, 2022

ch1bo commented Jul 11, 2022

ch1bo commented Aug 1, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ch1bo commented Jun 14, 2022 •

edited

Loading

github-actions bot commented Jun 14, 2022 •

edited

Loading

github-actions bot commented Jun 14, 2022 •

edited

Loading