7 commits

Author	SHA1	Message	Date
MarcoFalke	fa57fa1a2e	Enable clang-tidy bugprone-argument-comment and fix violations	2021-09-07 09:11:10 +02:00
fanquake	bd65a76b9d	Merge #21330: Deal with missing data in signature hashes more consistently ... 725d7ae049 Use PrecomputedTransactionData in signet check (Pieter Wuille) 497718b467 Treat amount<0 also as missing data for P2WPKH/P2WSH (Pieter Wuille) 3820090bd6 Make all SignatureChecker explicit about missing data (Pieter Wuille) b77b0cc507 Add MissingDataBehavior and make TransactionSignatureChecker handle it (Pieter Wuille) Pull request description: Currently we have 2 levels of potentially-missing data in the transaction signature hashes: * P2WPKH/P2WSH hashes need the spent amount * P2TR hashes need all spent outputs (amount + scriptPubKey) Missing amounts are treated as -1 (thus leading to unexpected signature failures), while missing outputs in P2TR validation cause assertion failure. This is hard to extend for signing support, and also quite ugly in general. In this PR, an explicit configuration option to {Mutable,}TransactionSignatureChecker is added (MissingDataBehavior enum class) to either select ASSERT_FAIL or FAIL. Validation code passes ASSERT_FAIL (as at validation time all data should always be passed, and anything else is a serious bug in the code), while signing code uses FAIL. The existence of the ASSERT_FAIL option is really just an abundance of caution. Always using FAIL should be just fine, but if there were for some reason a code path in consensus code was introduced that misses certain data, I think we prefer as assertion failure over silently introducing a consensus change. Potentially useful follow-ups (not for this PR, in my preference): * Having an explicit script validation error code for missing data. * Having a MissingDataBehavior::SUCCEED option as well, for use in script/sign.cpp DataFromTransaction (if a signature is present in a witness, and we don't have enough data to fully validate it, we should probably treat it as valid and not touch it). ACKs for top commit: sanket1729: reACK 725d7ae049 Sjors: ACK 725d7ae049 achow101: re-ACK 725d7ae049 benthecarman: ACK 725d7ae049 fjahr: Code review ACK 725d7ae049 Tree-SHA512: d67dc51bae9ca7ef6eb9acccefd682529f397830f77d74cd305500a081ef55aede0e9fa380648c3a8dd4857aa7eeb1ab54fe808979d79db0784ac94ceb31b657	2021-04-13 10:24:31 +08:00
Pieter Wuille	725d7ae049	Use PrecomputedTransactionData in signet check ... This is out of an abundance of caution only, as signet currently doesn't enable taproot validation flags. Still, it seems cleaner to make sure that all non-test code that passes MissingDataBehavior::ASSERT_FAIL also actually makes sure no data can be missing.	2021-03-15 17:29:39 -07:00
Pieter Wuille	3820090bd6	Make all SignatureChecker explicit about missing data ... Remove the implicit MissingDataBehavior::ASSERT_FAIL in the *TransationSignatureChecker constructors, and instead specify it explicit in all call sites: * Test code uses ASSERT_FAIL * Validation uses ASSERT_FAIL (through CachingTransactionSignatureChecker) (including signet) * libconsensus uses FAIL, matching the existing behavior of the non-amount API (and the extended required data for taproot validation is not available yet) * Signing code uses FAIL	2021-03-15 17:29:39 -07:00
fanquake	57e980d13c	scripted-diff: remove Optional & nullopt ... -BEGIN VERIFY SCRIPT- git rm src/optional.h sed -i -e 's/Optional</std::optional</g' $(git grep -l 'Optional<' src) sed -i -e 's/{nullopt}/{std::nullopt}/g' $(git grep -l 'nullopt' src) sed -i -e 's/ nullopt;/ std::nullopt;/g' $(git grep -l 'nullopt' src) sed -i -e 's/ nullopt)/ std::nullopt)/g' $(git grep -l 'nullopt' src) sed -i -e 's/(nullopt)/(std::nullopt)/g' $(git grep -l 'nullopt' src) sed -i -e 's/ nullopt,/ std::nullopt,/g' $(git grep -l 'nullopt' src) sed -i -e 's/? nullopt :/? std::nullopt :/g' $(git grep -l 'nullopt' src) sed -i -e 's/: nullopt}/: std::nullopt}/g' $(git grep -l 'nullopt' src) sed -i -e '/optional.h \\/d' src/Makefile.am sed -i -e '/#include <optional.h>/d' src/test/fuzz/autofile.cpp src/test/fuzz/buffered_file.cpp src/test/fuzz/node_eviction.cpp sed -i -e 's/#include <optional.h>/#include <optional>/g' $(git grep -l '#include <optional.h>' src) -END VERIFY SCRIPT-	2021-03-15 10:41:30 +08:00
MarcoFalke	77771a03df	refactor: Remove SignetTxs::m_valid and use optional instead ... m_valid implies the block solution has been checked, which is not the case. It only means the txs could be parsed. C++17 comes with std::optional, so just use that instead.	2020-09-22 22:31:31 +02:00
Karl-Johan Alm	404682b7cd	add signet basic support (signet.cpp) ... Co-authored-by: Anthony Towns <aj@erisian.com.au>	2020-09-10 10:47:40 +09:00