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

Create and spend from P2WPKH, P2SH-P2WPKH, and P2WSH #2418

Merged
merged 3 commits into from
Feb 28, 2020

Conversation

andrewtoth
Copy link
Contributor

This PR adds full support for generating and spending from P2WPKH, P2SH-wrapped-P2WPKH, and P2WSH multisig addresses.

Note, I removed the nestedWitness parameter for MultiSigScriptHashInput since it can be inferred by what the publickeys and threshold hash to. This makes it easier to create and spend from these addresses since the method of generating doesn't need to be stored.

@andrewtoth andrewtoth force-pushed the segwit-receive-send branch 7 times, most recently from bceb18a to b7e01ab Compare September 26, 2019 16:14
@matiu
Copy link
Contributor

matiu commented Oct 1, 2019

Thank you so much for summiting this. Code and tests (wow!) looks great

We definitely want to support it. Please give us a little time to review it in detail, giving this is a big change, but initial overview looks great. Great work!

@andrewtoth
Copy link
Contributor Author

@matiu Any more feedback on this PR?

@andrewtoth
Copy link
Contributor Author

@matiu Happy new year! Will 2020 be the year of native segwit in Bitcore?

@matiu
Copy link
Contributor

matiu commented Jan 6, 2020

Hi Andrew, thanks for your collaboration and patience.

We definitely want to add native segwit to bitcore in the coming months. We will review soon, as soon as we finish some other wallet related tasks.

@justinkook
Copy link
Contributor

Tested on Copay sending and Node repl requiring bitcore-lib with segwit changes.

Screen Shot 2020-01-28 at 4 46 06 PM

justinkook
justinkook previously approved these changes Jan 28, 2020
@justinkook
Copy link
Contributor

justinkook commented Jan 28, 2020

Also derive receiving addresses in BECH32 format

Node
Screen Shot 2020-01-28 at 5 06 40 PM

Copay
Screen Shot 2020-01-28 at 5 08 17 PM

Copy link
Contributor

@matiu matiu left a comment

Choose a reason for hiding this comment

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

thanks for this great PR!

I left two small comments that lines that need to be removed.

@andrewtoth andrewtoth dismissed stale reviews from ghost and justinkook via 3a540f7 February 28, 2020 04:12

it('should output this known mainnet wrapped witness address correctly', function() {
var pk = new PublicKey('03c87bd0e162f26969da8509cafcb7b8c8d202af30b928c582e263dd13ee9a9781');
var address = pk.toAddress('livenet', Address.PayToScriptHash);
Copy link
Contributor

Choose a reason for hiding this comment

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

JFYI:

Older copay version used P2SH addresses even for 1-1 wallets.
See: https://github.com/bitpay/bitcore/blob/master/packages/bitcore-wallet-service/src/lib/model/address.ts#L100

We should not confuse this 2 schema, with the same inputs:

  1. One Pub key to address, using p2sh & Address.createMultisig, 1-of-1.
  2. One pub key to address, using p2sh & wrapped witness.

input.countMissingSignatures().should.equal(0);
input.isFullySigned().should.equal(true);
});
it('returns a list of public keys with missing signatures', function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

nice!

@matiu matiu merged commit a482dc8 into bitpay:master Feb 28, 2020
@andrewtoth andrewtoth deleted the segwit-receive-send branch February 28, 2020 15:21
ghost pushed a commit to Ternio/bitcore that referenced this pull request Sep 22, 2020
Create and spend from P2WPKH, P2SH-P2WPKH, and P2WSH
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