test: Unit tests for taproot/tapscript coverage in interpreter.cpp
#25097
Conversation
|
( |
david-bakin
force-pushed
the
23279-taproot-unit-tests
branch
from
5753025 to
cd89899
Compare
david-bakin
changed the title
interpreter.cppinterpreter.cpp
david-bakin
changed the title
interpreter.cppinterpreter.cpp
david-bakin
changed the title
interpreter.cppinterpreter.cpp
|
Squashed and rebased this branch to master and seeing this when running the tests: |
Yes - that's actually expected. Those are death tests - testing asserts. The Boost Test framework will print that the assertion failed but then it is properly caught with an [Following paragraph was initially wrong but now is correct:] Do you have any suggestions on how I can improve the documentation or the test to avoid confusion such as this? Perhaps I could use (There's a big comment explaining this in the code @1366 but of course people will just see the logs first ...) P.S. Thank you for checking this PR out! Very encouraging for me... |
|
Oh ok, I hadn't looked at the code yet (the fixup and merge commits really need to be squashed to encourage people to look at it) but indeed there is a large comment. Agree, logging a message to explain would be helpful. |
|
I will squash now (actually, in the morning ...) - I was waiting until I finished the last couple of tasks but no problem doing it now. Thanks for the tip. Also, unfortunately, |
|
I tend to run individual tests with In any case, the unit test runner invoked with Edit: nit fixups signaled by Edit 2: I did see this error with |
Fixed, and I'll run
I do not see this when running And the CI checks all pass too. Not sure how to proceed on this report. |
david-bakin
force-pushed
the
23279-taproot-unit-tests
branch
from
b8e049e to
d692ae7
Compare
|
Squashed now as requested. Still 2 tests left to write: They're tough ones as both are for However, all general-purpose "test only" internal helpers - data structures, functions for visibility, etc. - are now in |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
|
DrahtBot said:
Referenced PR is a refactoring PR. I tried it out in my branch. Only merge conflict here is to |
david-bakin
force-pushed
the
23279-taproot-unit-tests
branch
from
3dba72f to
fedc037
Compare
…eTaprootMerkleRoot` bd7c5e2 Add BIP-341 specified constraints to `ComputeTaprootMerkleRoot` (David Bakin) Pull request description: [**N.B.:** This PR **_does not change the consensus_**. It only adds `assert` statements according to the current consensus in consensus-sensitive code (`interpreter.cpp`). So that's why the bot added the "consensus" tag and I prefixed the PR title with "consensus".] BIP 341 specifies [constraints on the size of the control block _c_ used to compute the taproot merkle root](https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#script-validation-rules). > The last stack element is called the control block _c_, and must have length _33 + 32m_, for a value of _m_ that is an integer between 0 and 128, inclusive. Fail if it does not have such a length. The actual merkle root is computed in `ComputeTaprootMerkleRoot` ([interpreter.cpp@1833](https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L1833)) - this code does _not_ check these constraints. All the callers do check the constraints before calling `ComputeTaprootMerkleRoot`. But in the future there may be more callers, and these checks may be inadvertently omitted at those future calls. Also, code at/near the current call sites may also change and skip these checks. Therefore _this PR adds those checks as `asserts` directly in `ComputeTaprootMerkleRoot`_ to help prevent that error. No unit tests provided: they'd have to be death tests as these are `assert` statements which raise `SIGABRT` and kill the program. Boost Test has a way to implement death tests (see the in-progress draft PR #25097 at [this code (you may have to click to expand the diff)](https://github.com/bitcoin/bitcoin/pull/25097/files#diff-21483d0e032747850208f21325b29cde89e9c1f55f83a7a166a388cc5c27115aR1089) and could be added here if desired by reviewers. Current callers of `ComputeTaprootMerkleRoot`: - `InferTaprootTree` ([standard.cpp@1552](https://github.com/bitcoin/bitcoin/blob/master/src/script/standard.cpp#L546)) - `VerifyTaprootCommittment` ([interpreter.cpp@1859](https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L1859)) does a partial check, but it is called from `VerifyWitnessProgram` ([interpreter.cpp@1922](https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L1918)) where a full check is done ACKs for top commit: sipa: ACK bd7c5e2 theStack: ACK bd7c5e2 Tree-SHA512: cec5aebfdf9fc9d13011abdf8427c934edf1df1ffc32b3e7cc5580f12809f24cf83e64ab0c843fabfce3591873bfcfa248277b30820fd786684319a196e4e550
…`ComputeTaprootMerkleRoot` bd7c5e2 Add BIP-341 specified constraints to `ComputeTaprootMerkleRoot` (David Bakin) Pull request description: [**N.B.:** This PR **_does not change the consensus_**. It only adds `assert` statements according to the current consensus in consensus-sensitive code (`interpreter.cpp`). So that's why the bot added the "consensus" tag and I prefixed the PR title with "consensus".] BIP 341 specifies [constraints on the size of the control block _c_ used to compute the taproot merkle root](https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#script-validation-rules). > The last stack element is called the control block _c_, and must have length _33 + 32m_, for a value of _m_ that is an integer between 0 and 128, inclusive. Fail if it does not have such a length. The actual merkle root is computed in `ComputeTaprootMerkleRoot` ([interpreter.cpp@1833](https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L1833)) - this code does _not_ check these constraints. All the callers do check the constraints before calling `ComputeTaprootMerkleRoot`. But in the future there may be more callers, and these checks may be inadvertently omitted at those future calls. Also, code at/near the current call sites may also change and skip these checks. Therefore _this PR adds those checks as `asserts` directly in `ComputeTaprootMerkleRoot`_ to help prevent that error. No unit tests provided: they'd have to be death tests as these are `assert` statements which raise `SIGABRT` and kill the program. Boost Test has a way to implement death tests (see the in-progress draft PR bitcoin#25097 at [this code (you may have to click to expand the diff)](https://github.com/bitcoin/bitcoin/pull/25097/files#diff-21483d0e032747850208f21325b29cde89e9c1f55f83a7a166a388cc5c27115aR1089) and could be added here if desired by reviewers. Current callers of `ComputeTaprootMerkleRoot`: - `InferTaprootTree` ([standard.cpp@1552](https://github.com/bitcoin/bitcoin/blob/master/src/script/standard.cpp#L546)) - `VerifyTaprootCommittment` ([interpreter.cpp@1859](https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L1859)) does a partial check, but it is called from `VerifyWitnessProgram` ([interpreter.cpp@1922](https://github.com/bitcoin/bitcoin/blob/master/src/script/interpreter.cpp#L1918)) where a full check is done ACKs for top commit: sipa: ACK bd7c5e2 theStack: ACK bd7c5e2 Tree-SHA512: cec5aebfdf9fc9d13011abdf8427c934edf1df1ffc32b3e7cc5580f12809f24cf83e64ab0c843fabfce3591873bfcfa248277b30820fd786684319a196e4e550
- Add new test utility methods for use in various unit tests. - Refactor some existing methods out of tests where they're currently embedded into the test util area. - Ensure that the conversions of string name to/from ScriptError_t have all the defined enum values. - And provide unit tests for the whole shebang.
This commit provides unit tests for Taproot functionality in `script/interpreter.cpp`, bitcoin#23279. Goal is to show correctness vs specifications in BIPs 341/342, and to improve code coverage in Taproot functionality there. Intent is to unit test completely without changing `interpreter.cpp` itself to provide visibility into static functions/private methods. Thus some of the tests must be "indirect", testing the unit-under-test by probing a visible caller. This makes those specific somewhat fragile as they depend on code paths in the caller, which may change in the future. However, such tests are written so they will fail if the unit-under-test isn't actually called. This is noted in the code of those specific tests. No changes to Bitcoin Core are made - just unit tests added. The Boost Test framework file `boost/test/execution_monitor.hpp` was added to the lint whitelist - this is acceptable because it is a documented public header of the Boost Test framework, which is already part of the project. - [x] `EvalChecksigTapscript` - [x] `HandleMissingData` - [x] `SignatureHashSchnorr` - now with full _path_ coverage - [x] `GenericTransactionSignatureChecker<T>::VerifySchnorrSignature` - [x] `GenericTransactionSignatureChecker<T>::CheckSchnorrSignature` - [x] `ComputeTaprootMerkleRoot` - [ ] `SigVersion::TAPSCRIPT` code paths in `ExecuteWitnessScript` - [x] Taproot code paths in `VerifyWitnessProgram` - [ ] Tapscript code paths in `VerifyWitnessProgram` **TODO:** Full Tapscript (BIP-342) unit tests yet to be completed. **TODO:** Does anything need to be done with `libconsensus`? Please advise! **N.B.:** These unit tests include "death tests" (name comes from Googletest framework): Tests that check that an `assert` actually fails. These are handled with a Boost `execution_monitor` (mentioned above) but the asserts _still issue messages_ even when by correctly trapped by the Boost Test framework. You'll see the following: ``` gitpod /workspace/bitcoin/src/test (23279-taproot-unit-tests) $ ./test_bitcoin Running 527 test cases... test_bitcoin: script/interpreter.cpp:1495: bool SignatureHashSchnorr(uint256 &, ScriptExecutionData &, const T &, uint32_t, uint8_t, SigVersion, const PrecomputedTransactionData &, MissingDataBehavior) [T = CMutableTransaction]: Assertion `false' failed. test_bitcoin: script/interpreter.cpp:1497: bool SignatureHashSchnorr(uint256 &, ScriptExecutionData &, const T &, uint32_t, uint8_t, SigVersion, const PrecomputedTransactionData &, MissingDataBehavior) [T = CMutableTransaction]: Assertion `in_pos < tx_to.vin.size()' failed. test_bitcoin: script/interpreter.cpp:1528: bool SignatureHashSchnorr(uint256 &, ScriptExecutionData &, const T &, uint32_t, uint8_t, SigVersion, const PrecomputedTransactionData &, MissingDataBehavior) [T = CMutableTransaction]: Assertion `execdata.m_annex_init' failed. test_bitcoin: script/interpreter.cpp:1556: bool SignatureHashSchnorr(uint256 &, ScriptExecutionData &, const T &, uint32_t, uint8_t, SigVersion, const PrecomputedTransactionData &, MissingDataBehavior) [T = CMutableTransaction]: Assertion `execdata.m_tapleaf_hash_init' failed. test_bitcoin: script/interpreter.cpp:1559: bool SignatureHashSchnorr(uint256 &, ScriptExecutionData &, const T &, uint32_t, uint8_t, SigVersion, const PrecomputedTransactionData &, MissingDataBehavior) [T = CMutableTransaction]: Assertion `execdata.m_codeseparator_pos_init' failed. test_bitcoin: script/interpreter.cpp:1469: bool HandleMissingData(MissingDataBehavior): Assertion `!"Missing data"' failed. test_bitcoin: script/interpreter.cpp:1474: bool HandleMissingData(MissingDataBehavior): Assertion `!"Unknown MissingDataBehavior value"' failed. *** No errors detected gitpod /workspace/bitcoin/src/test (23279-taproot-unit-tests) $ ``` These specific `Assertion ... failed` messages are _expected_. The tests triggering them properly _pass_ iff the `assert` (and no other) is _triggered_.
david-bakin
force-pushed
the
23279-taproot-unit-tests
branch
from
fedc037 to
eeefec3
Compare
david-bakin
changed the title
interpreter.cppinterpreter.cpp
|
DrahtBot said:
Referenced PR updates Most of the resulting conflicts are easy to resolve; the tricky one was the update to
So in fact I am ready to update this PR if #22793 lands first. And also, except for that, I can update this code now so that there won't be any conflict if #22793 lands later. (I am of the opinion that there are routines in |
src/test/util/pretty_data.cpp
Outdated
| for (const auto& se : script_errors) | ||
| if (se.name == name) | ||
| return se.err; | ||
| if (issue_boost_error) BOOST_ERROR("Unknown scripterror \"" << name << "\" in test description"); |
There was a problem hiding this comment.
You may want to move these BOOST_ERROR(...) changes back to the previous commit (where this file is introduced), since attempting to build the previous commit fails with this error:
/usr/bin/ld: /usr/bin/ld: DWARF error: could not find variable specification at offset 7218
libtest_util.a(libtest_util_a-pretty_data.o): in function `ParseScriptFlags(std::basic_string_view<char, std::char_traits<char> >, bool)':
./src/test/util/pretty_data.cpp:138: undefined reference to `boost::unit_test::unit_test_log_t::set_checkpoint(boost::unit_test::basic_cstring<char const>, unsigned long, boost::unit_test::basic_cstring<char const>)'
/usr/bin/ld: libtest_util.a(libtest_util_a-pretty_data.o): in function `lazy_ostream_impl':
/usr/include/boost/test/utils/lazy_ostream.hpp:74: undefined reference to `boost::unit_test::lazy_ostream::inst'
/usr/bin/ld: libtest_util.a(libtest_util_a-pretty_data.o): in function `ParseScriptFlags(std::basic_string_view<char, std::char_traits<char> >, bool)':
./src/test/util/pretty_data.cpp:138: undefined reference to `boost::test_tools::tt_detail::report_assertion(boost::test_tools::assertion_result const&, boost::unit_test::lazy_ostream const&, boost::unit_test::basic_cstring<char const>, unsigned long, boost::test_tools::tt_detail::tool_level, boost::test_tools::tt_detail::check_type, unsigned long, ...)'
/usr/bin/ld: libtest_util.a(libtest_util_a-pretty_data.o): in function `FormatScriptError[abi:cxx11](ScriptError_t, bool)':
./src/test/util/pretty_data.cpp:167: undefined reference to `boost::unit_test::unit_test_log_t::set_checkpoint(boost::unit_test::basic_cstring<char const>, unsigned long, boost::unit_test::basic_cstring<char const>)'
/usr/bin/ld: libtest_util.a(libtest_util_a-pretty_data.o): in function `lazy_ostream_impl':
/usr/include/boost/test/utils/lazy_ostream.hpp:74: undefined reference to `boost::unit_test::lazy_ostream::inst'
/usr/bin/ld: libtest_util.a(libtest_util_a-pretty_data.o): in function `FormatScriptError[abi:cxx11](ScriptError_t, bool)':
./src/test/util/pretty_data.cpp:167: undefined reference to `boost::test_tools::tt_detail::report_assertion(boost::test_tools::assertion_result const&, boost::unit_test::lazy_ostream const&, boost::unit_test::basic_cstring<char const>, unsigned long, boost::test_tools::tt_detail::tool_level, boost::test_tools::tt_detail::check_type, unsigned long, ...)'
/usr/bin/ld: libtest_util.a(libtest_util_a-pretty_data.o): in function `ParseScriptError(std::basic_string_view<char, std::char_traits<char> >, bool)':
./src/test/util/pretty_data.cpp:176: undefined reference to `boost::unit_test::unit_test_log_t::set_checkpoint(boost::unit_test::basic_cstring<char const>, unsigned long, boost::unit_test::basic_cstring<char const>)'
/usr/bin/ld: libtest_util.a(libtest_util_a-pretty_data.o): in function `lazy_ostream_impl':
/usr/include/boost/test/utils/lazy_ostream.hpp:74: undefined reference to `boost::unit_test::lazy_ostream::inst'
/usr/bin/ld: libtest_util.a(libtest_util_a-pretty_data.o): in function `ParseScriptError(std::basic_string_view<char, std::char_traits<char> >, bool)':
./src/test/util/pretty_data.cpp:176: undefined reference to `boost::test_tools::tt_detail::report_assertion(boost::test_tools::assertion_result const&, boost::unit_test::lazy_ostream const&, boost::unit_test::basic_cstring<char const>, unsigned long, boost::test_tools::tt_detail::tool_level, boost::test_tools::tt_detail::check_type, unsigned long, ...)'
/usr/bin/ld: libtest_util.a(libtest_util_a-pretty_data.o): in function `_GLOBAL__sub_I_pretty_data.cpp':
/usr/include/boost/test/unit_test_log.hpp:227: undefined reference to `boost::unit_test::unit_test_log_t::instance()'
clang: error: linker command failed with exit code 1 (use -v to see invocation)
There was a problem hiding this comment.
Partial review done
Code is high quality so far (if not a little complex), and I really like the look of these tests. Will finish review early next week, probably Monday.
- 12ae337 Add new test utility methods; break out some existing ones
A lot of this code is fairly elaborate in terms of use of C++ templating
features - for example, all the machinations necessary to get Hex() to work -
which I think I'd be hesitant to include in non-test code. But since these are
tests, the complexity/convenience trade-off seems okay.
|
|
||
| #define EVAL(...) []() -> bool { return __VA_ARGS__; }() | ||
|
|
||
| BOOST_AUTO_TEST_CASE(is_base_of_trait) |
There was a problem hiding this comment.
Is there a reason to include this test case? This doesn't look like it's testing any non-std functionality. It also seems like a direct adaptation of the example usage here: https://en.cppreference.com/w/cpp/types/is_base_of
Addressing these comments now.
Thank you, and looking forward to more comments! Is it preferred (i.e., do you prefer) squashing during a review or should I fix things in an additional commit on top of this one while you're in progress and squash it later? |
Typically squashes are done immediately because if you squash at some point later, all reviewers will have to re-review to ensure that the code hasn't actually changed unexpectedly during the squash. |
There was a problem hiding this comment.
approach ACK eeefec3 (jamesob/ackr/25097.1.david-bakin.test_unit_tests_for_tapr)
Wow, thanks for this contribution @david-bakin. These are some really
fine-grained tests for various Taproot related parts of the script interpreter.
The code here is very readable and seems nicely extensible. Clearly a
lot of work went into this.
For many parts of the codebase, this sort of testing would be overkill IMO
because it is so tightly coupled to the tested code. But in the case of script
interpretation - debatably the heart of bitcoin validation - I think it's
worthwhile to have painstakingly involved tests at the cost of slight
maintenance burden if the underlying code happens to change somehow. Maybe
someone more experienced than me w.r.t. changing the script interpreter can
weigh in here and confirm my intuition.
All of my comments are pretty minor. I'd say it might be wortwhile to break
out the Parse*() optional changes in the second commit either into their own
commit, or as I mentioned in a previous review, fold it into the first.
| // This is the known sighash of the Tx and input data we set up (precomputed) | ||
| h.SetHex("f614d8ae6dcc49e2ca2ef1c03f93c7326189e5575d446e825e5a2700fb1cb83c"); | ||
| return h; | ||
| }(); |
There was a problem hiding this comment.
You can use uint256S() instead of this lambda. (https://github.com/jamesob/bitcoin/blob/eeefec343592c6de85048e3b055ba49755cfb072/src/uint256.h#L139-L144)
|
|
||
| enum class if_as_expected_return { False, | ||
| True }; | ||
| if_as_expected_return m_iae{if_as_expected_return::True}; |
There was a problem hiding this comment.
What's the rationale not just to make m_iae a bool?
| } | ||
| { | ||
| // Negative tests: last byte is _not_ SIGHASH_DEFAULT, but we early exit _without changing | ||
| // serror_ because we don't provide a txDataIn (🡄 this requires knowledge of how |
There was a problem hiding this comment.
Isn't "txDataIn" actually called txdata on the original class? (https://github.com/jamesob/bitcoin/blob/eeefec343592c6de85048e3b055ba49755cfb072/src/script/interpreter.h#L289)
| } | ||
|
|
||
| { | ||
| // Now check that, given the parameters, if `SignatureHashSchnorr fails there's an error exit. |
There was a problem hiding this comment.
nit: missing a tick
| { | ||
| m_hash_type = hash_type; | ||
| m_witness_signature = m_sig; | ||
| if (hash_type) m_witness_signature.push_back(hash_type); |
There was a problem hiding this comment.
If you wind up having to retouch this commit, I might advise changing if (hash_type) to if (hash_type != SIGHASH_DEFAULT) here and below; to me it's a little clearer and less tricky... I had to verify that SIGHASH_DEFAULT would evaluate to false in this context.
| if (m_p2sh_wrapped) { | ||
| // But BIP-341 allows all SegWit v1 P2SH-wrapped outputs to pass | ||
| valtype fake_hash(20, 0x00); | ||
| script_sig << OP_0 << fake_hash; |
There was a problem hiding this comment.
Interesting... if I change this to OP_1 or remove the OP_0 << altogether, all tests still pass.
| Context& CheckSignatureCheckerWasCalled() | ||
| { | ||
| BOOST_CHECK_MESSAGE(m_checker_was_called, | ||
| Descr() << ": Schnoor signature checker was called, as expected"); |
There was a problem hiding this comment.
"Schnoor" misspelling here and elsewhere
|
🐙 This pull request conflicts with the target branch and needs rebase. Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft". |
|
Are you still working on this? |
|
yes sorry - i will update by tomorrow!
…On Wed, Oct 12, 2022 at 12:21 PM Andrew Chow ***@***.***> wrote:
Are you still working on this?
—
Reply to this email directly, view it on GitHub
<#25097 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA7YLBE6IOKK4MJDCZ47JYDWC4FUPANCNFSM5VOZLXLQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
It's been several weeks since this comment with no additional updates, so I'm going to close this for now. If you do get around to working on this, please leave a comment so that it can be reopened. |
|
Sad to see this closed as it contains some novel unittest coverage. Please feel free to reopen with updates @david-bakin. Hopefully others will step in to review as well. |
|
Soon! Tied up temporarily
…On Thu, Nov 10, 2022, 12:12 jamesob ***@***.***> wrote:
Sad to see this closed as it contains some novel unittest coverage. Please
feel free to reopen with updates @david-bakin
<https://github.com/david-bakin>. Hopefully others will step in to review
as well.
—
Reply to this email directly, view it on GitHub
<#25097 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA7YLBHXF2WHJN3HJ2HN4E3WHVJKNANCNFSM5VOZLXLQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
6 participants
[Unit tests are not complete to resolve #23279 - yet. But those additional tests - see this space for future developments! - will not change the code presented here.]
This PR provides unit tests for Taproot functionality in
script/interpreter.cpp, #23279.Goal is to show correctness vs specifications in BIPs 341/342, and to improve code coverage in Taproot functionality there.
Intent is to unit test completely without changing
interpreter.cppitself to provide visibility into static functions/private methods. Thus some of the tests must be "indirect", testing the unit-under-test by probing a visible caller. This makes those specific tests somewhat fragile as they depend on code paths in the caller, which may change in the future. However, such tests are written so they will fail if the unit-under-test isn't actually called. This is noted in the code of those specific tests.No changes to Bitcoin Core are made - just unit tests added. The Boost Test framework file
boost/test/execution_monitor.hppwas added to the lint whitelist - this is acceptable because it is a documented public header of the Boost Test framework, which is already part of the project.EvalChecksigTapscriptHandleMissingDataSignatureHashSchnorr- now with full path coverageGenericTransactionSignatureChecker<T>::VerifySchnorrSignatureGenericTransactionSignatureChecker<T>::CheckSchnorrSignatureComputeTaprootMerkleRootSigVersion::TAPSCRIPTcode paths inExecuteWitnessScriptVerifyWitnessProgramVerifyWitnessProgramTODO: Full Tapscript (BIP-342) unit tests yet to be completed.
TODO: Does anything need to be done with
libconsensus? Please advise!2 commits:
src/test/utilthat are not only used in these tests but may also be useful for future unit tests. Also, refactor some existing utility functions from current tests (mainly,script_tests.cpp) intosrc/test/utilso they can be more widely used too. All of these new test utility functions have unit tests themselves, insrc/test/test_utils_tests.cpp.src/test/script_tapscript_tests.cpp. Code coverage runs show 100% coverage of lines and near 100% coverage of branches.DEATH TRAP AHEAD!: These unit tests include "death tests" (name comes from Googletest framework): Tests that check that an
assertactually fails. These are handled with a Boostexecution_monitor(mentioned above) but the asserts still issue messages even when correctly trapped by the Boost Test framework. You'll see the following in the logs:These specific
Assertion ... failedmessages are expected. The tests triggering them properly pass iff theassert(and no other) is triggered.Coverage results 2022-06-05 showing Taproot coverage (Tapscript tests not yet complete) when only running
script_tapscript_tests.cpp:EvalChecksigTapscriptHandleMissingDataSignatureHashSchnorrGenericTransaction-SignatureChecker<T>::CheckSchnorrSignatureComputeTaprootMerkleRootVerifyWitnessProgram(Taproot only)