btcd
btcd copied to clipboard
Nested witness signing
This PR adds SignTxWitness which handles signing normal outputs + any that use segwit. It will return a signature script and witness structure after signing whatever it can and merging signatures. The function is a superset of what SignTxOutput supports. I left SignTxOutput's intact to preserve BC, though SignTxWitness also works with old outputs and should probably be used going forward for easy segwit support.
I went with using [][]byte to pass around input script data because it's easily converted back to a pushonly scriptSig, or can be used as a witness if called for.
Also added test cases comparable to SignTxOutput:
- P2WPKH: bare and P2SH
- P2PKH: P2SH, P2WSH and P2SH|P2WSH
- P2PK: P2SH, P2WSH and P2SH|P2WSH
- Multisig 2of2 (basic, step by step, & with key duplication): P2SH, P2WSH and P2SH|P2WSH
@jcvernaleo (as per #1530)
- Low priority
- Enhancement
- Outdated
any progress on this?
@sunsiansong It seems the PR was lost in the backlog. I've added this to our internal review board to prioritize this.
In the meantime, if you can help with reviewing and testing out this PR, that'd be very helpful!
@afk11 If you could rebase your branch on the current master to run the CI checks and fix the outdated Glide stuff, that'd really get the ball rolling.
Hi all
Thanks for the ping. I know most apps got by without this feature, but did think it was worthwhile to bring everything up to date. I'll try rebase this in the next day or so.
Re tests, I covered most nested forms of P2PK/P2PKH/multisig in P2SH, P2WSH, and nested P2WSH in P2SH with the results of each verified by the script interpreter. So I'm pretty confident it was working :) https://github.com/btcsuite/btcd/pull/1304/files#diff-c4fcde2c18ebe144b7b71b53efb1021c7bb089a8c02602134402999ca9fa1417R1436
I'll check in more depth later and report back
Pull Request Test Coverage Report for Build 343822214
- 184 of 195 (94.36%) changed or added relevant lines in 1 file are covered.
- 12 unchanged lines in 4 files lost coverage.
- Overall coverage increased (+0.2%) to 53.581%
Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
---|---|---|---|
txscript/sign.go | 184 | 195 | 94.36% |
<!-- | Total: | 184 | 195 |
Files with Coverage Reduction | New Missed Lines | % |
---|---|---|
txscript/sign.go | 1 | 90.6% |
peer/peer.go | 2 | 75.55% |
connmgr/connmanager.go | 3 | 86.07% |
btcec/signature.go | 6 | 91.64% |
<!-- | Total: | 12 |
Totals | |
---|---|
Change from base Build 329751959: | 0.2% |
Covered Lines: | 20910 |
Relevant Lines: | 39025 |
💛 - Coveralls
Squashed @jcvernaleo
@jcvernaleo @onyb anything else you'd like done on this?