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 wallet address interfaces. #1474

Merged
merged 1 commit into from
Jun 11, 2019
Merged

Add wallet address interfaces. #1474

merged 1 commit into from
Jun 11, 2019

Conversation

jrick
Copy link
Member

@jrick jrick commented Jun 6, 2019

The following new interfaces are introduced:

  • V0Scripter: for addresses used to create version 0 scripts
  • SecpPubKeyHasher: for addresses created from a secp256k1 pubkey hash
  • SecpPubKeyer: for addresses with a known secp256k1 pubkey
  • SecpPubKeyHash160er: for addresses paying to a secp256k1 pubkey HASH160
  • BIP44AccountXpubPather: for BIP0044 addresses derived from an account xpub
  • AddressP2PKHv0: a union interface for v0 P2PKH addrs
  • BIP44XpubP2PKHv0: a union interface for v0 P2PKH addrs with known xpubs

These interfaces describe the behavior and features of an address and allow
callers to operate on their distinctive capabilities without switching on a
concrete type. More interfaces are planned to be introduced in the future as
address and script usage evolves.

The wallet methods NewExternalAddress, NewInternalAddress, and NewChangeAddress
have been modified to return addresses implementing all of the new interfaces.

@jrick
Copy link
Member Author

jrick commented Jun 6, 2019

Something that needs discussion: should xpubAddress implement P2SHv0? These are valid scripts, but payments to these scripts are not watched and the String method won't return an address equal or compatible with the p2sh pkscript. It might be wiser to only implement this method by addresses (such as our p2sh multisig addresses) that are only used as p2sh.

@jrick
Copy link
Member Author

jrick commented Jun 6, 2019

This is also clearly missing a PublicKey accessor.

@jrick jrick force-pushed the xpubaddress branch 6 times, most recently from 1477591 to df7d47c Compare June 7, 2019 15:17
if err != nil {
return nil, translateError(err)
var pubKeyAddrString string
if secp, ok := addr.(wallet.SecpPubKeyer); ok {
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this changing the error handling semantics? Previously, if it got to this point and the address can't be parsed into a pubkey, it would return an error. Now, it will ignore it and return what is almost surely an invalid address.

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct, but this method should still work on accounts even if the addresses represented by the account are not p2pkh.

The following new interfaces are introduced:

- V0Scripter: for addresses used to create version 0 scripts
- SecpPubKeyHasher: for addresses created from a secp256k1 pubkey hash
- SecpPubKeyer: for addresses with a known secp256k1 pubkey
- SecpPubKeyHash160er: for addresses paying to a secp256k1 pubkey HASH160
- BIP44AccountXpubPather: for BIP0044 addresses derived from an account xpub
- AddressP2PKHv0: a union interface for v0 P2PKH addrs
- BIP44XpubP2PKHv0: a union interface for v0 P2PKH addrs with known xpubs

These interfaces describe the behavior and features of an address and allow
callers to operate on their distinctive capabilities without switching on a
concrete type.  More interfaces are planned to be introduced in the future as
address and script usage evolves.

The wallet methods NewExternalAddress, NewInternalAddress, and NewChangeAddress
have been modified to return addresses implementing all of the new interfaces.
@jrick jrick merged commit 958ad8b into decred:master Jun 11, 2019
@jrick jrick deleted the xpubaddress branch June 11, 2019 13:37
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.

3 participants