Skip to content

Remove Address::p2pk#222

Merged
dongcarl merged 1 commit intorust-bitcoin:masterfrom
stevenroose:no-p2pk-addr
Feb 7, 2019
Merged

Remove Address::p2pk#222
dongcarl merged 1 commit intorust-bitcoin:masterfrom
stevenroose:no-p2pk-addr

Conversation

@stevenroose
Copy link
Copy Markdown
Collaborator

@stevenroose stevenroose commented Jan 21, 2019

There is no address format for p2pk.

Script Descriptors should probably be used to construct scriptPubkeys and scriptSigs for using them instead of the Address type.

@tamasblummer
Copy link
Copy Markdown
Contributor

Pay to public key is used in Bitcoin blockchain, with most of satoshi's coins. Why should it be removed?

@stevenroose
Copy link
Copy Markdown
Collaborator Author

@tamasblummer Because it's not an address. It's just a script. Script descriptors make it possible to construct scriptPubKeys for them, so we don't need to have them awkwardly put in the Address type.

@tamasblummer
Copy link
Copy Markdown
Contributor

This script is usually interpreted by explorer as sending to an address. I do not see value in a PR removing an interpretation that is common.

@stevenroose
Copy link
Copy Markdown
Collaborator Author

Yeah, that's a wrong interpretation of that script. And more modern explorers will also not show it as such.

I agree that it's unfortunate to remove features from a codebase. But it's in the wrong place and script descriptors can cover the lost functionality.

@dongcarl
Copy link
Copy Markdown
Member

Hmmm I remember @apoelstra had some thoughts on this.

@apoelstra
Copy link
Copy Markdown
Member

Yeah, fine by me. If the ambiguous interpretation is not even universally used I'd rather drop it because it's confusing and could lead to lost coins.

@dongcarl
Copy link
Copy Markdown
Member

dongcarl commented Feb 7, 2019

Sounds good. @stevenroose please rebase!

@stevenroose
Copy link
Copy Markdown
Collaborator Author

@dongcarl done.

Copy link
Copy Markdown
Member

@dongcarl dongcarl left a comment

Choose a reason for hiding this comment

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

utACK f80e882

There is no address format for p2pk.
Copy link
Copy Markdown
Contributor

@sgeisler sgeisler left a comment

Choose a reason for hiding this comment

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

ACK since it gets the Address type closer to always supporting round trips (I don't know if it's the case yet and think we should add fuzz tests for it at some point).

@dongcarl dongcarl merged commit e386d9e into rust-bitcoin:master Feb 7, 2019
casey pushed a commit to casey/rust-bitcoin that referenced this pull request Nov 28, 2023
PastaPastaPasta pushed a commit to PastaPastaPasta/rust-dashcore that referenced this pull request Feb 2, 2026
The processing parts already does the same checks as the "dummy adding" parts.
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.

5 participants