bitcoin-tx: fix comment about pay-to-witness script type ("addoutpubkey" command) #16833

pull theStack wants to merge 2 commits into bitcoin:master from theStack:20190908-bitcoin-tx_fix_comment_p2wsh_to_p2wpkh changing 1 files +2 −2

theStack commented at 7:22 PM on September 8, 2019: member

The function GetScriptForWitness() returns a P2WPKH script if the passed redeem script is P2PK or P2PKH, otherwise it returns a P2WSH. Since we pass a P2PK script here (constructed a few lines above through GetScriptForRawPubKey()) the result is rather P2WPKH than P2WSH and the comment is adapted accordingly.

bitcoin-tx: fix comment about pay-to-witness script type ("addoutpubkey" command)

The function GetScriptForWitness() returns a P2WPKH script if the
passed redeem script is P2PK or P2PKH, otherwise it returns a P2WSH.
Since we pass a P2PK script here (constructed a few lines above
through GetScriptForRawPubKey()) the result is rather P2WPKH than
P2WSH and the comment is adapted accordingly.

f542215156

fanquake added the label Docs on Sep 8, 2019
fanquake requested review from meshcollider on Sep 8, 2019
meshcollider commented at 11:09 AM on September 9, 2019: contributor

ACK f542215156022dcc17fd067a9fa53afff4b900fd

Looks like it was incorrectly duplicated from the multisig version of the function below it.
instagibbs commented at 1:05 PM on September 9, 2019: member

honestly I think the whole comment is pretty redundant? I'd be more inclined to just delete it
bitcoin-tx: shortened comments about pay-to-witness script types ("addoutpubkey" command) 2a965dbd87
theStack commented at 3:13 PM on September 9, 2019: member

honestly I think the whole comment is pretty redundant? I'd be more inclined to just delete it

I agree that it mostly describes what can be seen in the code anyway (name of the function being called, name of the variable the result is being assigned to), the only informational part is the resulting script type. Shortened the comments it for both the pubkey (-> P2WPKH) and the multisig (-> P2WSH) comments before the according function call in 2a965db.
instagibbs commented at 9:11 PM on September 9, 2019: member

utACK https://github.com/bitcoin/bitcoin/pull/16833/commits/2a965dbd8744dfa78f91d9f0a4f1a156918a17b1

restarted unrelated job failure
DrahtBot commented at 2:31 AM on September 10, 2019: member


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #16841 (Replace GetScriptForWitness with GetScriptForDestination by meshcollider)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
meshcollider commented at 2:38 AM on September 10, 2019: contributor

#16841 improves these comments
fanquake commented at 5:25 AM on September 11, 2019: member

Going to close this in favor of #16841, which also improves these comments. @theStack Thanks for the contribution, did you want to review over there? @instagibbs too.
fanquake closed this on Sep 11, 2019
DrahtBot locked this on Dec 16, 2021

Contributors

Labels

bitcoin-tx: fix comment about pay-to-witness script type ("addoutpubkey" command) #16833

Conflicts