Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add functions for serializing keys as text #272

Merged
merged 2 commits into from
May 16, 2019

Conversation

rvl
Copy link
Contributor

@rvl rvl commented May 16, 2019

Relates to #143.

Overview

To store XPrv and XPub in a database column or text file, a hex-encoded string is suitable.

Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍
Looks good, let's add unit tests corresponding to these too.

@KtorZ KtorZ assigned jonathanknowles and rvl and unassigned jonathanknowles May 16, 2019
@paweljakubas paweljakubas force-pushed the rvl/143/serialize-keys branch 3 times, most recently from 546945d to a88694b Compare May 16, 2019 15:22
To store XPrv and XPub in a database column or text file, a
hex-encoded string is best.
-> Property
prop_roundtripXPriv xpriv = do
let (Right xpriv') = (deserializeXPrv . serializeXPrv) xpriv
xpriv === xpriv'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should rather be written:

xpriv' = (deserializeXPrv . serializeXPrv) xpriv 
Right xpriv === xpriv'

With the latter version, in case the property fails, we get a nice error output with a counter example from Quickcheck. With the former, it blows up because of an invalid pattern-match 😬

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

corrected

genRootKeys :: Gen (Key 'RootK XPrv)
genRootKeys = do
(s, g, e) <- (,,)
<$> genPassphrase @"seed" (0, 32)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor remark but the seed should be at least 16-bit according to cardano-crypto:

-- | Generate a new XPrv from an entropy seed
--
-- The seed should be at least 16 bytes, although it is not enforced
--
-- The passphrase encrypt the secret key in memory
generateNew :: (ByteArrayAccess keyPassPhrase, ByteArrayAccess generationPassPhrase, ByteArrayAccess seed)

We don't actually have any comment on the matter in our code either. I'll put a point on the blackboard.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

corrected

@paweljakubas paweljakubas force-pushed the rvl/143/serialize-keys branch from a88694b to e224d4b Compare May 16, 2019 16:08
address review remarks
@paweljakubas paweljakubas merged commit 3990811 into master May 16, 2019
@iohk-bors iohk-bors bot deleted the rvl/143/serialize-keys branch May 16, 2019 18:31
@KtorZ KtorZ removed this from the SQLite implementation for the DB Layer milestone Jun 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants