Skip to content

Conversation

jl2012
Copy link
Contributor

@jl2012 jl2012 commented Jan 14, 2016

BIP141: New witness program definition
BIP142: Title change; new address definition
BIP143: Title change and update reference implementation
BIP144: Update reference implementation

@jl2012 jl2012 force-pushed the patch-5 branch 3 times, most recently from 363063e to 06dee85 Compare January 14, 2016 10:16
@jl2012 jl2012 changed the title WIP: New witness program in BIP141, and related revision in 142 and 143 New witness program definition in BIP141, and related revision in 142 and 143 Jan 14, 2016
@CodeShark
Copy link
Contributor

ACK on the 141 and 143 changes

@jl2012 jl2012 force-pushed the patch-5 branch 2 times, most recently from 0245041 to 62957bf Compare January 16, 2016 16:51
@jl2012 jl2012 changed the title New witness program definition in BIP141, and related revision in 142 and 143 New witness program definition in BIP141, and related revision in 142 - 144 Jan 16, 2016
@CodeShark
Copy link
Contributor

ACK on all BIPs.

@dcousens
Copy link
Contributor

ACK BIP 142, much improved.

Copy link
Contributor

Choose a reason for hiding this comment

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

signle > single

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@schildbach Corrected. Thanks

@btcdrak
Copy link
Contributor

btcdrak commented Jan 17, 2016

ACK

@NicolasDorier
Copy link
Contributor

NACK for 142. This would make a conflict between two version bytes, breaking client who depended on the base58 to infer the network of a base58 string.

Main EXT_PUBLIC_KEY 4,136,178,30 <=> TestNet WITNESS_P2WPKH 4

Solution without conflict:

base58Prefixes[(int)Base58Type.WITNESS_P2WPKH] = new byte[] { (0x03) };
base58Prefixes[(int)Base58Type.WITNESS_P2WSH] = new byte[] { (40) };

base58Prefixes[(int)Base58Type.WITNESS_P2WPKH] = new byte[] { 0x6 };
base58Prefixes[(int)Base58Type.WITNESS_P2WSH] = new byte[] { (10) };

I also want to note there is conflict with the segnet and mainnet as well

Main SCRIPT_ADDRESS 5 <=> segnet EXT_PUBLIC_KEY 5,53,135,207
Main SCRIPT_ADDRESS 5 <=> segnet EXT_SECRET_KEY 5,53,131,148

@sipa I think this should be changed for the public segnet, or people trying the segnet will break when they wanted to extract the network from the base58 string.

It's a weak NACK, I understand that not making collision will be harder and harder though.

@jl2012
Copy link
Contributor Author

jl2012 commented Jan 18, 2016

BIP142 revised to resolve the conflict

@NicolasDorier
Copy link
Contributor

Thanks, last thing I am thinking. Why making a scheme address specific for v0 and not something general to all segwit payment ? If we use the BIP142 scheme, the problem is that when a new segwit will release, no wallet will be able to send to it... all wallets want to do is sending money, they should not need to implement a new address scheme for a new segwit version.

There should be one address version for both wsh and wpkh.

@jl2012
Copy link
Contributor Author

jl2012 commented Jan 18, 2016

@NicolasDorier This is actually generalizable, as long as the future version is 20 byte or 32 byte or both. It's very likely that future versions will be 32 byte.

In the very first version of BIP142 I actually made it completely generalizable to any script length but that idea was not very popular: #267

@NicolasDorier
Copy link
Contributor

well, in future version a new address version should be found. And during a while, all wallet will not be able to send to it.

@NicolasDorier
Copy link
Contributor

I see, ACK. In the future when a new segwit version will be out, maybe a new address scheme will be implemented which is generaly secure for scripts.

README.mediawiki Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Don't need a BOM here...

luke-jr added a commit that referenced this pull request Jan 18, 2016
New witness program definition in BIP141, and related revision in 142 - 144
@luke-jr luke-jr merged commit b882d29 into bitcoin:master Jan 18, 2016
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.

7 participants