mempool: check p2wsh standard

[ChrisCho-H]

Change Description

AFAIK, btcd curretly doesn't check standard rule for witness unlike bitcoin core. I add method CheckWitnessStandard to check standard rule for p2wsh, and execute it inside the method validateStandardness for mempool validation.

Steps to Test

If this PR makes sense, I will add test vectors following the reviews.

Pull Request Checklist

Testing

• [x] Your PR passes all CI checks.
• [ ] Tests covering the positive and negative (error paths) are included.
• [ ] Bug fixes contain tests triggering the bug to prevent regressions.

Code Style and Documentation

• [x] The change is not insubstantial. Typo fixes are not accepted to fight bot spam.
• [x] The change obeys the Code Documentation and Commenting guidelines, and lines wrap at 80.
• [x] Commits follow the Ideal Git Commit Structure.
• [x] Any new logging statements use an appropriate subsystem and logging level.

📝 Please see our Contribution Guidelines for further guidance.

[coveralls]

Pull Request Test Coverage Report for Build 15042853022

Details

• 11 of 46 (23.91%) changed or added relevant lines in 2 files are covered.
• 86 unchanged lines in 3 files lost coverage.
• Overall coverage decreased (-0.2%) to 56.539%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
mempool/mempool.go	2	13	15.38%
mempool/policy.go	9	33	27.27%
<!--	Total:	11	46

Files with Coverage Reduction	New Missed Lines	%
btcutil/gcs/gcs.go	1	80.95%
mempool/mempool.go	1	65.87%
rpcclient/infrastructure.go	84	39.79%
<!--	Total:	86

Totals
Change from base Build 14871723979:	-0.2%
Covered Lines:	31057
Relevant Lines:	54930

---

💛 - Coveralls

[kcalvinalvin]

Verified that the added logic implements just the P2WSH standardness check in Core.

https://github.com/bitcoin/bitcoin/blob/725c9f7780e0def3d79be151c08a36fe7a9dc59c/src/policy/policy.cpp#L264-L275

It does seem like the original commit is very old. Not quite sure about adding it but am indifferent if it does get added. https://github.com/bitcoin/bitcoin/pull/8499

[Roasbeef]

Not quite sure about adding it but am indifferent if it does get added.

FWIW, there're quite a few newer standardness rules that bitcoind has added over the years that we never adopted. The new rules are generally a moving target, and don't have BIP implementations so all we have is the code to go for.

IMO, the only standardness/policy rules that are actually important, are those that directly impact the relay of time-sensitive transactions across the network. One such example is: TRUC and co.

[ChrisCho-H]

Original PR of bitcoind explains the purpose of standardness/policy rules of p2wsh like following

• 3600 bytes witnessScript and 100 stack items are adequate for a n-of-100 multisig using OP_CHECKSIG, OP_ADD, and OP_EQUAL.
• The max size for ECDSA signature is 73 bytes and nothing should use more than that with the current scripting language.
• This is to prevent abuse of witness space, and reduce the risks of DoS attack with some unknown special and big scripts.

I agree that absence of this doesn't directly impact, but what explained in bitcoind also makes sense to mitigate the possible risks.