test: add reason checks for non-standard txs in test_IsStandard #17299

While taking a look at #17272 I noticed that for some reason the unit test test_IsStandard (which was not adapted to the policy change in the referenced PR commits) didn't fail as expected: https://github.com/bitcoin/bitcoin/blob/6a97e8a060f7632bbaee27d3de8035dc6ebe3895/src/test/transaction_tests.cpp#L758-L762 It turned out that IsStandardTx() returned "dust" as rejection reason (instead of the expected "multi-op-return"), leading to the conclusion that https://github.com/bitcoin/bitcoin/pull/17272/commits/5fe6f052bd37a16b2849e05f5cf18d7e194bc705 erroneously performs the IsDust() check also for TX_NULL_DATA transactions. To avoid cases like this in the future, this PR makes the unit test test_IsStandard more strict by also checking for the concrete reason after each occurence of IsStandardTx() returning false.

Concept ACK, but the reason string isn't wiped on success or between checks. It slightly increases review burden, but having IsStandardTx wipe the reason string immediately on entrance is a small reasoning improvement to make sure the string isn't accidentally being kept between calls.

AFAIK the only "real" usage of it is in src_validation.cpp mempool checks, so should be easy to verify correctness of change.

@instagibbs: Thanks for the feedback, solid point. I think the easiest way would just be to add a reason.clear() before any occurence of BOOST_CHECK(!IsStandardTx(...));? I don't see that the reason string is wiped out on entrance of IsStandardTx, and I'm hesitating changing policy.cpp for a PR that should merely extend a unit test.

that works too :+1:

unrelated appveyor failure

utACK https://github.com/bitcoin/bitcoin/pull/17299/commits/c1c6c410a66996b2d60d5172189b5a5ec8100842

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #17272 ([POLICY] Make multiple OP_RETURNS in a single TX standard by Bushstar)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.