Skip to content

Update BIP173: new reference impls + tests fix#565

Merged
luke-jr merged 3 commits intobitcoin:masterfrom
sipa:bip-witaddr
Aug 7, 2017
Merged

Update BIP173: new reference impls + tests fix#565
luke-jr merged 3 commits intobitcoin:masterfrom
sipa:bip-witaddr

Conversation

@sipa
Copy link
Copy Markdown
Member

@sipa sipa commented Aug 2, 2017

No description provided.

@gmaxwell
Copy link
Copy Markdown
Contributor

gmaxwell commented Aug 3, 2017

ACK but at the bottom of decoding add:

Implementations should take special care when converting the address to a ScriptPubkey: In Bitcoin script while 0 is encoded as 0x00 the numbers 1 through 16 are encoded as 0x51 though 0x60 (81 to 96 in decimal). If a bech32 address is converted to an incorrect scrippubkey the result will likely be either unspendable or insecure, either of which is undesirable.

@clarkmoody
Copy link
Copy Markdown
Contributor

I agree with @gmaxwell that additional explanation is in order. Thankfully OP_1 through OP_16 have contiguous values ;-)

@sipa
Copy link
Copy Markdown
Member Author

sipa commented Aug 3, 2017

@gmaxwell Rewrote that a bit, and added it.

Copy link
Copy Markdown
Contributor

@gmaxwell gmaxwell left a comment

Choose a reason for hiding this comment

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

ACK

sipa added a commit to sipa/bech32 that referenced this pull request Aug 4, 2017
0692344 Fix tests for non-v0 witnesses (Pieter Wuille)

Pull request description:

  See corresponding BIP update: bitcoin/bips#565

  Thanks @afk11 for pointing this out.

Tree-SHA512: 02feee63502cabcefdae5dfdb87f6d7dd0171d7119a5d7a5f6a593c86aaabe603c2fc0fc47397084f7b86ac08a0723f8de06861814ca971d83a73ee85ea14a33
@sipa
Copy link
Copy Markdown
Member Author

sipa commented Aug 7, 2017

@luke-jr Ping?

@luke-jr luke-jr merged commit dd4467d into bitcoin:master Aug 7, 2017
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