Since #8589 CTxWitness was removed and instead replaced with CScriptWitness inside each CTxIn.
Primitives: Correct CTransaction deserialization docstring #24350
pull TheCharlatan wants to merge 1 commits into bitcoin:master from TheCharlatan:rmCTxWitness changing 1 files +1 −1-
TheCharlatan commented at 11:30 AM on February 15, 2022: member
-
d4b3483cec
Primitives: Correct CTransaction deserialization docstring
Since https://github.com/bitcoin/bitcoin/pull/8589 CTxWitness was removed and instead replaced with CScriptWitness inside each CTxIn.
- fanquake added the label Docs on Feb 15, 2022
-
laanwj commented at 12:04 PM on February 15, 2022: member
Concept ACK, thanks for keeping the documentation up to date.
- w0xlt approved
-
w0xlt commented at 2:03 PM on February 18, 2022: contributor
ACK d4b3483
-
MarcoFalke commented at 2:21 PM on February 18, 2022: member
No objection, but the test should be fixed up here or in a follow-up as well?
git grep CTxWitness -
TheCharlatan commented at 3:21 PM on February 18, 2022: member
I guess the functional test CTransaction class could mimic what is going on in the actual code as well. Not sure if such a change is desirable though. i only found this discrepancy, if you can call it that, after taking a tour of witness serialization throughout the code. After all, the changed comment is still not entirely accurate, since it's multiple script witness tokens that are serialized/de-serialized. @MarcoFalke if desired, I can attempt to remove the CTxWitness class from the tests as well.
-
MarcoFalke commented at 3:37 PM on February 18, 2022: member
On a first glance the
CTxWitnessin the test looks unused, so there might even be a bug -
TheCharlatan commented at 5:28 PM on February 18, 2022: member
On a first glance the
CTxWitnessin the test looks unused, so there might even be a bugTo me it seems to be extensively used in the tests, try
git grep \\\.wit. This would be quite a refactor in the tests to align the test de-serialization behavior with the actual code. -
MarcoFalke commented at 8:34 AM on February 19, 2022: member
Ah ok, I grepped too little.
- MarcoFalke merged this on Feb 19, 2022
- MarcoFalke closed this on Feb 19, 2022
- sidhujag referenced this in commit f27ea6ae72 on Feb 20, 2022
- DrahtBot locked this on Feb 19, 2023
Contributors
Labels