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 bip32 pub key serialization #6215

Merged
merged 1 commit into from Apr 15, 2016

Conversation

Projects
None yet
6 participants
@jonasschnelli
Member

jonasschnelli commented Jun 1, 2015

CExtPubKey should be serializable like CPubKey.

EDIT:
This would allow storing extended private and public key to support BIP32/HD wallets.
Related to #6265

@luke-jr

This comment has been minimized.

Show comment
Hide comment
@luke-jr

luke-jr Jun 2, 2015

Member

Why?

Member

luke-jr commented Jun 2, 2015

Why?

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Jun 2, 2015

Member

This is a valid question.
I'm working on a new wallet and i'm trying to split of and create PRs for everything which could be used independent and might reduce the diff-size when merging a bigger wallet change later.
[1] https://github.com/jonasschnelli/bitcoin/tree/2015/05/corewallet

Because there is already unused bip32 stuff in bitcoin core, why not complete it with things which would be necessary in case of a full bip32 support.

Member

jonasschnelli commented Jun 2, 2015

This is a valid question.
I'm working on a new wallet and i'm trying to split of and create PRs for everything which could be used independent and might reduce the diff-size when merging a bigger wallet change later.
[1] https://github.com/jonasschnelli/bitcoin/tree/2015/05/corewallet

Because there is already unused bip32 stuff in bitcoin core, why not complete it with things which would be necessary in case of a full bip32 support.

@luke-jr

This comment has been minimized.

Show comment
Hide comment
@luke-jr

luke-jr Jun 2, 2015

Member

Oh, I guess this might be useful for storing watch-only HD seeds in the wallet file?

Member

luke-jr commented Jun 2, 2015

Oh, I guess this might be useful for storing watch-only HD seeds in the wallet file?

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Jun 2, 2015

Member

Exactly!.

Member

jonasschnelli commented Jun 2, 2015

Exactly!.

@laanwj laanwj added the Wallet label Jun 3, 2015

@jgarzik

This comment has been minimized.

Show comment
Hide comment
@jgarzik

jgarzik Jun 7, 2015

Contributor

Too many naked constants (74, 75) without a named constant, IMO

Contributor

jgarzik commented Jun 7, 2015

Too many naked constants (74, 75) without a named constant, IMO

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Jun 7, 2015

Member

Agreed.
Factored out the ext keysize of 74 bytes to a constant.

Member

jonasschnelli commented Jun 7, 2015

Agreed.
Factored out the ext keysize of 74 bytes to a constant.

@jgarzik

This comment has been minimized.

Show comment
Hide comment
@jgarzik

jgarzik Jul 23, 2015

Contributor

ACK, with comment

  • A better test is probably deserialize then serialize, check for byte match.
Contributor

jgarzik commented Jul 23, 2015

ACK, with comment

  • A better test is probably deserialize then serialize, check for byte match.
@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Jul 24, 2015

Member

Force push fixed: replace magic number 74 bytes into BIP32_EXTKEY_SIZE in base58.h

Member

jonasschnelli commented Jul 24, 2015

Force push fixed: replace magic number 74 bytes into BIP32_EXTKEY_SIZE in base58.h

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Jul 28, 2015

Member

Added serializing/deserializing of a bip32 extended private key.
Followed @jgarzik advice of checking the serialized output by deserialize again and compare the keys (bytes).

Member

jonasschnelli commented Jul 28, 2015

Added serializing/deserializing of a bip32 extended private key.
Followed @jgarzik advice of checking the serialized output by deserialize again and compare the keys (bytes).

@jgarzik

This comment has been minimized.

Show comment
Hide comment
@jgarzik

jgarzik Sep 15, 2015

Contributor

Ping, reviewers

Contributor

jgarzik commented Sep 15, 2015

Ping, reviewers

@dcousens

This comment has been minimized.

Show comment
Hide comment
@dcousens

dcousens Sep 16, 2015

Contributor

utACK

Contributor

dcousens commented Sep 16, 2015

utACK

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Oct 1, 2015

Member

utACK (retracted for now, see below)

Member

laanwj commented Oct 1, 2015

utACK (retracted for now, see below)

Show outdated Hide outdated src/pubkey.h
@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Nov 28, 2015

Member

@laanwj All good now?

Member

sipa commented Nov 28, 2015

@laanwj All good now?

Show outdated Hide outdated src/pubkey.h
add bip32 pubkey serialization
CExtPubKey should be serializable like CPubKey
@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Apr 14, 2016

Member

Fixed @laanwj nit.
Rebased & squashed.

Member

jonasschnelli commented Apr 14, 2016

Fixed @laanwj nit.
Rebased & squashed.

@laanwj laanwj merged commit 90604f1 into bitcoin:master Apr 15, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Apr 15, 2016

Merge #6215: add bip32 pub key serialization
90604f1 add bip32 pubkey serialization (Jonas Schnelli)

zkbot added a commit to zcash/zcash that referenced this pull request Apr 17, 2018

Auto merge of #3180 - str4d:transaction-serialization, r=<try>
Upstream serialization improvements

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#5264
- bitcoin/bitcoin#6914
- bitcoin/bitcoin#6215
- bitcoin/bitcoin#8068
  - Only the `COMPACTSIZE` wrapper commit
- bitcoin/bitcoin#8658
- bitcoin/bitcoin#8708
  - Only the serializer variadics commit
- bitcoin/bitcoin#9039
- bitcoin/bitcoin#9125
  - Only the first two commits (the last two block on other upstream PRs)

Part of #2074.

zkbot added a commit to zcash/zcash that referenced this pull request Apr 18, 2018

Auto merge of #3180 - str4d:transaction-serialization, r=<try>
Upstream serialization improvements

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#5264
- bitcoin/bitcoin#6914
- bitcoin/bitcoin#6215
- bitcoin/bitcoin#8068
  - Only the `COMPACTSIZE` wrapper commit
- bitcoin/bitcoin#8658
- bitcoin/bitcoin#8708
  - Only the serializer variadics commit
- bitcoin/bitcoin#9039
- bitcoin/bitcoin#9125
  - Only the first two commits (the last two block on other upstream PRs)

Part of #2074.

zkbot added a commit to zcash/zcash that referenced this pull request Apr 19, 2018

Auto merge of #3180 - str4d:transaction-serialization, r=ebfull
Upstream serialization improvements

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#5264
- bitcoin/bitcoin#6914
- bitcoin/bitcoin#6215
- bitcoin/bitcoin#8068
  - Only the `COMPACTSIZE` wrapper commit
- bitcoin/bitcoin#8658
- bitcoin/bitcoin#8708
  - Only the serializer variadics commit
- bitcoin/bitcoin#9039
- bitcoin/bitcoin#9125
  - Only the first two commits (the last two block on other upstream PRs)

Part of #2074.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment