In the "invalid output witnessScript typed key" case, the value no longer hashes to the redeemScript's P2WSH program: 65f0b3dd trimmed it to its last six bytes while fixing a short read. This restores the original 37-byte script as the value and keeps the key one byte too long — the only defect, same layout the redeemScript case already uses, same total length.
Made with my usual tools: a computer, the Internet and an LLM. The mistakes, as usual, are all mine.
@achow101 as BIP174's author, could you take a look? In short: the "invalid output witnessScript typed key" vector is meant to fail on the key length alone, but the pushed key in its witnessScript is not a point on secp256k1, so an implementation that validates the key and skips the length check rejects the case for the wrong reason and the missing check goes unnoticed. This swaps in a valid point; nothing else changes and CI is green.
Tend to NACK
The pr description is needlessly verbose and confusing. I've re-read it 5 times and do not understand what the actual problem is. What do curve points have to do with witness scripts? The keydata for a PSBT_OUT_WITNESS_SCRIPT does not contain any interpretable data, and the point of the test is to make sure that parsers are checking the length and not interpreting any data.
FWIW, per Grok the description is confusing but the fix is correct: https://grok.com/share/bGVnYWN5_3c8755f5-bf63-4673-b6cf-6942c3a71cf1.
You're right about the curve points — the keydata is filler, so nothing off-curve is reachable; my description was wrong. The real defect: since 65f0b3dd the witnessScript value no longer hashes to the redeemScript's P2WSH program. This restores the original 37-byte script as the value and keeps the key one byte too long — the same layout the redeemScript case already uses. Happy to close if that check doesn't belong here.
In the "PSBT with invalid output witnessScript typed key" case, the
value of the PSBT_OUT_WITNESS_SCRIPT record no longer hashes to the
P2WSH program in the output's redeemScript: 65f0b3dd trimmed it to its
last six bytes while fixing a short read.
Re-encode the record as `02 0100 25 <script>`: the key stays one byte
too long, which is what the case tests, and the value is the original
37-byte script again. Same total length, same layout as the "invalid
output redeemScript typed key" case.
@fametrano: I struggle to derive value from your contributions in this repository. It’s nice that your scanning is finding actual issues, but your PR descriptions are often hard to parse and putting an undue burden on your reviewers. I’ve also found your review comments to suffer from similar issues. Could you please be more selective in your contributions, but take more time to explain yourself in a comprehensible manner for the ones you open?
@fametrano has hundreds of GitHub actions per day, so they appear to be highly automated?
Agree that it is appreciable to be finding actual issues, but I tend to have to feed the descriptions and comments into an AI just to parse them. Maybe the future is AI-to-AI without a meat proxy in the middle, but we're not there yet in this project, so please take @murchandamus's comment above to heart.
To be fair, (I think) I have seen a real improvement in @fametrano's review comments and PR descriptions recently.
Thank you both, I take the request to heart.
I have stopped opening new PRs here. Whatever I open or comment on from now on, I will choose with care and explain plainly.
On the automation question: my work is highly automated for btclib-org and LLM-assisted everywhere else.
ACK
Thanks @fametrano for the PR, @jonatack for translating, and @achow101 for review.
Thank you both, I take the request to heart.
I have stopped opening new PRs here. Whatever I open or comment on from now on, I will choose with care and explain plainly.
On the automation question: my work is highly automated for btclib-org and LLM-assisted everywhere else.
Thanks, @fametrano, I appreciate that.