The function IsStandardTx can return a number of reasons for non-standard transactions. A few of those are currently tested on one hand by the unit test test_IsStandard (transaction_tests.cpp), directly checking the result of the function call, on the other hand by the functional test mempool_accept.py, indirectly testing by using the testmempoolaccept RPC and checking the reject-reason field in the result. On the current master branch (7967104aee055476107dc17265cefc4ae4e75378), the test coverage looks like the following:
| IsStandardTx() Reject reason | Functional test (mempool_accept.py) | Unit test (transaction_tests.cpp) |
| ------------- |:-------------:|:-----:|
|"version"| :heavy_check_mark: | |
|"tx-size"| :heavy_check_mark: | |
|"scriptsig-size"|
|"scriptsig-not-pushonly"| :heavy_check_mark: | |
|"scriptpubkey"| :heavy_check_mark: | :heavy_check_mark: |
|"bare-multisig"|
|"dust"| :heavy_check_mark: | :heavy_check_mark: |
|"multi-op-return"| :heavy_check_mark: | :heavy_check_mark: |
I think it would make sense to have at least one test for each reason by either a unit test or functional test -- for "scriptsig-size" and "bare-multisig" we don't have any, so those are most needed as now. Given that, that would lead to the following list of seven small tasks (roughly ordered by descending priority):
"scriptsig-size" reason (PR #17480, commit 5e8a56348b5e1026e9ddcae0b2fa2a68faf4439e, done by my humble self)"bare-multisig" reason (PR #17502, commit 1bb5d517aa616c1d5b5801d2ea36a2de5fb61eba, done by my humble self)"scriptsig-size" reason (PR #17532, commit 8f2d7737cc236b6122f30e31856eb3181960fba1, done by my humble self)"bare-multisig" reason (PR #17541, commit 1be0b1fb2adcf95d76f879195564c0bf84162e31, done by my humble self)"scriptsig-not-pushonly" reason (PR #17720, commit 5aab011805ceb12801644170700b1a62e0bf4a5d, done by my humble self)"tx-size" reason (PR #17947, commit 4537ba5f21ad8afb705325cd8e15dd43877eb28f, dony by humble self)"version" reason (PR #17555, commit https://github.com/bitcoin/bitcoin/pull/17555/commits/76303f65f92a0fbe9a90c0e807554a6daa860636, done by dspicher)This would be a good "good first issue" candidate I guess -- I'm tempted to work on that myself, but to encourage new contributors (and knowing how helpful "good first issues" can be to get into) I'll not touch it for a while.
I would like to work on this.
@theStack did you already get started on the "bare-multisig" tests?
@dspicher: Glad to hear that you are interested! PRs for both the unit and functional test for "bare-multisig" have been already opened by me within the last days (see #17502 and #17541 -- both are not merged yet though), hence right now the following three tests are missing:
"scriptsig-not-pushonly" reason"tx-size" reason"version" reasonFeel free to work on those, and have fun! :)
All missing unit tests and functional tests are implemented and merged in the master branch now :tada: :beer: so I'm closing this issue.
Most helpful comment
@dspicher: Glad to hear that you are interested! PRs for both the unit and functional test for
"bare-multisig"have been already opened by me within the last days (see #17502 and #17541 -- both are not merged yet though), hence right now the following three tests are missing:"scriptsig-not-pushonly"reason"tx-size"reason"version"reasonFeel free to work on those, and have fun! :)