Rename scriptPubKey to witness_script in ExecuteWitnessScript() function to correctly reflect which script is being executed.
For example in segwitv0, this scriptPubKey refers to the script of the form OP_0 <script_hash>, but witness_script refers to the script that actually hashes to the script_hash.
If there is a reason why it's named this way, I would love to know
Concept ACK
AFAIK, scriptPubKey is only used for the output scripts, and "witness script" is indeed more appropriate here.
If there is a reason why it's named this way, I would love to know
Me too ;) Maybe someone deeper involved in BIP141 can comment on this.
Slightly related: #22429 .
According to https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki#witness-program:
If the version byte is 0, and the witness program is 32 bytes: [...]
- The witness must consist of an input stack to feed to the script, followed by a serialized script (
witnessScript).
review ACK 3941a8730a9b9ea9f7144ec07d1b0a78a347beaf, also checked that the bitcoind binary does not change with -O2 compiler flags 🚎
<details><summary>Show signature and timestamp</summary>
Signature:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
review ACK 3941a8730a9b9ea9f7144ec07d1b0a78a347beaf, also checked that the bitcoind binary does not change with -O2 compiler flags 🚎
-----BEGIN PGP SIGNATURE-----
iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUgCVQv+LybfVNUUOZBdE5KkQ4LCwuIzSa0p3vfTolM3p9iGs2mg51lpmL+1wv1k
VMvnnlf++xTs+bVlwusd5PDnitHIE4UYegcEOKrk/NKEOF37fZMkYYc09o7Ug+k4
i2FWZv4vc43hkRvtxkYk6pogLNuqG+NXwrxBRoM/jDK87FGmFndOlZtx0VppElVS
gle4kVwrAjNc72Pcnyz61PrYZ95z9kst7A7h5RG3L5pJv+wtHczEgyNiVCSdU/w1
Mv9VsSSBmb9slK4EY7qsNqQ5ISkViXcuAREnDNAD8Mn/YICkfgD6oB3+HcGqZoN6
QvMsvCdZhZGx2aPps0uDe5NJ0hdP2LavtwFJFoyQsWVb9SZtPYk5kus3h9SXEW0b
wvaLqVx4Q6pbcBp7eIKtsZxw9cC2UZMK+21G6gnxw/+CRiRU5RHU5Jc9XXS7WQ1q
bZ9dQHKbPfBf97L0qr0g0LSdgOuXkdsJ7IF9gisQNpjnDTqG1E9bci5kFTXxPgax
g0BO+qcq
=eTOz
-----END PGP SIGNATURE-----
Timestamp of file with hash 9367635b93c07f0c3fe5f104b15a884e4562ca2461611d8f73249fa88423eb37 -
</details>
After I posted my review, I realized that the BIP does not name the P2WPKH "witness script" (OP_DUP << OP_HASH160 << program << OP_EQUALVERIFY << OP_CHECKSIG). So maybe leave it as-is or rename it to exec_script, which is the name in the calling function?
Rename misleading scriptPubKey in ExecuteWitnessScript() function to
correctly relfect what is being executed.
After I posted my review, I realized that the BIP does not name the P2WPKH "witness script" (OP_DUP << OP_HASH160 << program << OP_EQUALVERIFY << OP_CHECKSIG). So maybe leave it as-is or rename it to exec_script, which is the name in the calling function?
I renamed it to exec_script. After this rationale, I am okay with leaving it as it is. But I still lean towards exec_script as the current ScriptPubKey was confusing(to me) at first.
ACK 007910388b54abc97057e44a7a8f7241e83c203b
review ACK 007910388b54abc97057e44a7a8f7241e83c203b 🖖
<details><summary>Show signature and timestamp</summary>
Signature:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
review ACK 007910388b54abc97057e44a7a8f7241e83c203b 🖖
-----BEGIN PGP SIGNATURE-----
iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUgpFgv+N9drfsSTAyD6fsUbH+rg1qR02T5eW1R9cvl/uRkyMDxMnltVviN1jjy2
CvOl9dvLCjLuJs60WFZDtih+iPobeEuspmXM/Gi6D7KQAa2D1/kpNvDTK2/SpLdP
NNumtWHLHzzNYZg7jwOseFkxFBtCD+2eJtilkdakNdGa/zRnczbTQ42+jfFMrn16
1p0cxVQSz3LMzH7g0Uf/y8CrD73NPcU3QR2YI3A4wC4jxe3y2iO/fclDSllu3DLQ
NLvzOQJS+bMUXzvPs7C12cCkQLF7fAGmVRgF3y0ktGOUg+A6ldalG9Zwz++ACThT
uQkIWhblZ8yayMUOC+IWWEHcO9yfSolNYgEldusb3KBj9FJQZKMR0zgLf6YfuEtR
TOOGT/q5tiBgHz0lZNs3spSvK8o0YV1kA7eCL3Ieg6SnR3sd60r/KbGhcg41+8v7
cnuJfxfoi5LOTwBCZEa5z/DjV9a+az8myZQHA2n4y5w1WUrVVexewHumNSu+xWb0
lY86zdvu
=0hTV
-----END PGP SIGNATURE-----
Timestamp of file with hash c13a542aeb019f542128734ea1f0328af19d0667a6fb8662eb80a01ea47a375e -
</details>