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

BIP 32: Add simpler explanations where I got confused originally reading this. #785

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ddustin
Copy link
Contributor

@ddustin ddustin commented Jun 5, 2019

Essentially I added the simple explanation for some things next to the more complex ones originally used. If I had these in there when I first came upon this document, I would have had a much easier time digesting it.

Hopefully these changes help the next peson who reads it.

@ddustin ddustin changed the title Add simpler explanations where I got confused originally reading this. [BIP32] Add simpler explanations where I got confused originally reading this. Jun 5, 2019
@ddustin ddustin changed the title [BIP32] Add simpler explanations where I got confused originally reading this. BIP32: Add simpler explanations where I got confused originally reading this. Jun 5, 2019
@ddustin ddustin changed the title BIP32: Add simpler explanations where I got confused originally reading this. BIP 32: Add simpler explanations where I got confused originally reading this. Jun 5, 2019
@luke-jr
Copy link
Member

luke-jr commented Sep 19, 2019

@sipa

bip-0032.mediawiki Outdated Show resolved Hide resolved
@jonatack
Copy link
Member

Pinging BIP author @sipa for feedback (thanks!)

@@ -51,10 +51,11 @@ Addition (+) of two coordinate pair is defined as application of the EC group op
Concatenation (||) is the operation of appending one byte sequence onto another.

As standard conversion functions, we assume:
* point(p): returns the coordinate pair resulting from EC point multiplication (repeated application of the EC group operation) of the secp256k1 base point with the integer p.
* point(p): returns the public key for p. This is the coordinate pair resulting from EC point multiplication (repeated application of the EC group operation) of the secp256k1 base point with the integer p.
Copy link
Member

Choose a reason for hiding this comment

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

I find this to be somewhat cyclic. We define public/private keys in function of this operation (i.e., the corresponding public key to private key p is point(p)). The change here is adding the reverse that (i.e., defining point(p) as the public key to p).

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I'm in agreement with @sipa here. Points are used as public keys but aren't equivalent to them in certain use contexts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about this rewording:

"point(p): returns the coordinate pair resulting from EC point multiplication (repeated application of the EC group operation) of the secp256k1 base point with the integer p (similar to making public key)."

@@ -64,6 +65,8 @@ In what follows, we will define a function that derives a number of child keys f

We represent an extended private key as (k, c), with k the normal private key, and c the chain code. An extended public key is represented as (K, c), with K = point(k) and c the chain code.

When deriving child keys, a 'hardened' child key can only be generated using a private key. This provides security advantages but reduces the usefulness of HD key derivation. It is typically used to separate 'accounts' from each other.
Copy link
Contributor

Choose a reason for hiding this comment

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

ACK this change.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer the last sentence be dropped. Given the weird security properties of "normal" derivation, I think we should word things as though hardened derivation were the ordinary case, and non-hardened derivation is the one that has a sentence explaining why somebody would want to use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about this wording to make it sound more ordinary:

"When deriving child keys, a 'hardened' child key can only be generated using a private key. This provides security advantages and prevents adversarial public key tracing. It is typically used to separate 'accounts' from each other."

And another sentence added for non-hardened keys:

"This non-hardened result is typically used by a server to continually generate receive addresses without the ability to spend funds."

@jonatack
Copy link
Member

jonatack commented May 1, 2024

@ddustin mind updating per the new feedback?

@murchandamus murchandamus added the PR Author action required Needs updates, has unaddressed review comments, or is otherwise waiting for PR author label May 8, 2024
@ddustin
Copy link
Contributor Author

ddustin commented May 9, 2024

@ddustin mind updating per the new feedback?

Absolutely, will do. Traveling at the moment 👍

Essentially I added the simple explanation for some things next to the more complex ones originally used. If I had these in there when I first came upon this document, I would have had a much easier time digesting it.

Hopefully these changes help the next peson who reads it.
@jonatack jonatack removed the PR Author action required Needs updates, has unaddressed review comments, or is otherwise waiting for PR author label Jul 11, 2024
@@ -51,10 +51,11 @@ Addition (+) of two coordinate pair is defined as application of the EC group op
Concatenation (||) is the operation of appending one byte sequence onto another.

As standard conversion functions, we assume:
* point(p): returns the coordinate pair resulting from EC point multiplication (repeated application of the EC group operation) of the secp256k1 base point with the integer p.
* point(p): returns the coordinate pair resulting from EC point multiplication (repeated application of the EC group operation) of the secp256k1 base point with the integer p (similar to making public key).
Copy link
Contributor

Choose a reason for hiding this comment

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

There seems to be a word missing here:

Suggested change
* point(p): returns the coordinate pair resulting from EC point multiplication (repeated application of the EC group operation) of the secp256k1 base point with the integer p (similar to making public key).
* point(p): returns the coordinate pair resulting from EC point multiplication (repeated application of the EC group operation) of the secp256k1 base point with the integer p (similar to making a public key).

@murchandamus murchandamus requested a review from sipa July 16, 2024 15:34
@murchandamus murchandamus added the Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified label Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pending acceptance This BIP modification requires sign-off by the champion of the BIP being modified Proposed BIP modification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants