aboutsummaryrefslogtreecommitdiff
path: root/src/test/util
AgeCommit message (Collapse)Author
2024-05-23Merge bitcoin/bitcoin#29873: policy: restrict all TRUC (v3) transactions to ↵Ava Chow
10kvB 154b2b2296edccb5ed24e829798dacb6195edc11 [fuzz] V3_MAX_VSIZE and effective ancestor/descendant size limits (glozow) a29f1df289cf27c6cbd565448548b3dc1392a9b0 [policy] restrict all v3 transactions to 10kvB (glozow) d578e2e3540e085942001350ff3aeb047bdac973 [policy] explicitly require non-v3 for CPFP carve out (glozow) Pull request description: Opening for discussion / conceptual review. We like the idea of a smaller maximum transaction size because: - It lowers potential replacement cost (i.e. harder to do Rule 3 pinning via gigantic transaction) - They are easier to bin-pack in block template production - They equate to a tighter memory limit in data structures that are bounded by a number of transactions (e.g. orphanage and vExtraTxnForCompact). For example, the current memory bounds for orphanage is 100KvB * 100 = 40MB, and guaranteeing 1 tx per peer would require reserving a pretty large space. History for `MAX_STANDARD_TX_WEIGHT=100KvB` (copied from https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2115459510): - 2010-09-13 In https://github.com/bitcoin/bitcoin/commit/3df62878c3cece15a8921fbbdee7859ee9368768 satoshi added a 100kB (MAX_BLOCK_SIZE_GEN/5 with MBS_GEN = MAX_BLOCK_SIZE/2) limit on new transactions in CreateTransaction() - 2013-02-04 https://github.com/bitcoin/bitcoin/pull/2273 In gavin gave that constant a name, and made it apply to transaction relay as well Lowering `MAX_STANDARD_TX_WEIGHT` for all txns is not being proposed, as there are existing apps/protocols that rely on large transactions. However, it's been brought up that we should consider this for TRUCs (which is especially designed to avoid Rule 3 pinning). This reduction should be ok because using nVersion=3 isn't standard yet, so this wouldn't break somebody's existing use case. If we find that this is too small, we can always increase it later. Decreasing would be much more difficult. ~[Expected size of a commitment transaction](https://github.com/lightning/bolts/blob/master/03-transactions.md#expected-weight-of-the-commitment-transaction) is within (900 + 172 * 483 + 224) / 4 = 21050vB~ EDIT: this is incorrect, but perhaps not something that should affect how we choose this number. ACKs for top commit: sdaftuar: ACK 154b2b2296edccb5ed24e829798dacb6195edc11 achow101: ACK 154b2b2296edccb5ed24e829798dacb6195edc11 instagibbs: ACK 154b2b2296edccb5ed24e829798dacb6195edc11 t-bast: ACK https://github.com/bitcoin/bitcoin/commit/154b2b2296edccb5ed24e829798dacb6195edc11 murchandamus: crACK 154b2b2296edccb5ed24e829798dacb6195edc11 Tree-SHA512: 89392a460908a8ea9f547d90e00f5181de0eaa9d2c4f2766140a91294ade3229b3d181833cad9afc93a0d0e8c4b96ee2f5aeda7c50ad7e6f3a8320b9e0c5ae97
2024-05-21[fuzz] V3_MAX_VSIZE and effective ancestor/descendant size limitsglozow
2024-05-21doc: remove comment about using thread_localfanquake
Followup to https://github.com/bitcoin/bitcoin/pull/30095#discussion_r1605655974.
2024-05-17[[refactor]] Check CTxMemPool options in constructorTheCharlatan
This ensures that the tests run the same checks on the mempool options that the init code also applies.
2024-05-16kernel: De-globalize fReindexTheCharlatan
fReindex is one of the last remaining globals exposed by the kernel library, so move it into the blockstorage class to reduce the amount of global mutable state and make the kernel library a bit less awkward to use.
2024-05-14Merge bitcoin/bitcoin#29086: refactor: Simply include CTxMemPool::Options in ↵Ava Chow
CTxMemPool directly rather than duplicating definition cc67d33fdac45357b593b1faff3d1735e5fe91ba refactor: Simply include CTxMemPool::Options in CTxMemPool directly rather than duplicating definition (Luke Dashjr) Pull request description: Instead of duplicating mempool options two places, just include the Options struct directly on the CTxMemPool ACKs for top commit: achow101: ACK cc67d33fdac45357b593b1faff3d1735e5fe91ba kristapsk: cr utACK cc67d33fdac45357b593b1faff3d1735e5fe91ba jonatack: ACK cc67d33fdac45357b593b1faff3d1735e5fe91ba Tree-SHA512: 9deb5ea6f85eeb1c7e04536cded65303b0ec459936a97e4f257aff2c50b0984a4ddbf69a4651f48455b9c80200a1fd24e9c74926874fdd9be436bbbe406251ce
2024-05-09kernel: Remove key module from kernel libraryTheCharlatan
The key module's functionality is not used by the kernel library, but currently kernel users are still required to initialize the key module's `secp256k1_context_sign` global as part of the `kernel::Context` through `ECC_Start`.
2024-05-07Merge bitcoin/bitcoin#29494: build: Assume HAVE_CONFIG_H, Add IWYU pragma ↵Ava Chow
keep to bitcoin-config.h includes fa09451f8e6799682d7e7c863f25334fd1c7dce3 Add lint check for bitcoin-config.h include IWYU pragma (MarcoFalke) dddd40ba8267dea11a3eb03d5cf8b51dbb99be5d scripted-diff: Add IWYU pragma keep to bitcoin-config.h includes (MarcoFalke) Pull request description: The `bitcoin-config.h` includes have issues: * The header is incompatible with iwyu, because symbols may be defined or not defined. So the `IWYU pragma: keep` is needed to keep the include when a symbol is not defined on a platform. Compare the previous discussion in https://github.com/bitcoin/bitcoin/pull/29408#issuecomment-1948959711 * Guarding the includes by `HAVE_CONFIG_H` is verbose and brittle. Now that all build config dependencies have been removed from low level headers, the benefits are questionable, and the guard can be removed. The linter could also be tricked by guarding the include by `#if defined(HAVE_C0NFIG_H)` (`O` replaced by `0`). Compare the previous discussion in https://github.com/bitcoin/bitcoin/pull/29404#discussion_r1483189853 . ACKs for top commit: achow101: ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3 TheCharlatan: ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3 hebasto: re-ACK fa09451f8e6799682d7e7c863f25334fd1c7dce3, only rebased since my recent [review](https://github.com/bitcoin/bitcoin/pull/29494#pullrequestreview-2028864535) (`timedata.cpp` removed in https://github.com/bitcoin/bitcoin/pull/29623). Tree-SHA512: 47cb973f7f24bc625acc4e78683371863675d186780236d55d886cf4130e05a78bb04f1d731aae7088313b8e963a9677cc77cf518187dbd99d776f6421ca9b52
2024-05-06refactor: Simply include CTxMemPool::Options in CTxMemPool directly rather ↵Luke Dashjr
than duplicating definition
2024-05-01[refactor] make MempoolAcceptResult::m_replaced_transactions non-optionalglozow
2024-05-01scripted-diff: Add IWYU pragma keep to bitcoin-config.h includesMarcoFalke
-BEGIN VERIFY SCRIPT- perl -0777 -pi -e 's/#if defined\(HAVE_CONFIG_H\)\n#include <config\/bitcoin-config.h>.*\n#endif.*\n/#include <config\/bitcoin-config.h> \/\/ IWYU pragma: keep\n/g' $( git grep -l '#include <config/bitcoin-config.h>' ) -END VERIFY SCRIPT-
2024-04-26validation: allow to specify frequency for -checkblockindexMartin Zumsande
This makes it similar to -checkaddrman and -checkmempool, which also allow to run the check occasionally instead of always / never. Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2024-04-24refactor: Remove hooking code for urlDecodeFabian Jahr
The point of this was to be able to build bitcoin-tx and bitcoin-wallet without libevent, see #18504. Now that we use our own implementation of urlDecode this is not needed anymore. Co-authored-by: stickies-v <stickies-v@protonmail.com>
2024-03-18assumeutxo: Get rid of faked nTx and nChainTx valuesRyan Ofsky
The `PopulateAndValidateSnapshot` function introduced in f6e2da5fb7c6406c37612c838c998078ea8d2252 from #19806 has been setting fake `nTx` and `nChainTx` values that can show up in RPC results (see #29328) and make `CBlockIndex` state hard to reason about, because it is difficult to know whether the values are real or fake. Revert to previous behavior of setting `nTx` and `nChainTx` to 0 when the values are unknown, instead of faking them. This commit fixes at least two assert failures in the (pindex->nChainTx == pindex->nTx + prev_chain_tx) check that would happen previously. Tests for these failures are added separately in the next two commits. Compatibility note: This change could result in -checkblockindex failures if a snapshot was loaded by a previous version of Bitcoin Core and not fully validated, because fake nTx values will have been saved to the block index. It would be pretty easy to avoid these failures by adding some compatibility code to `LoadBlockIndex` and changing `nTx` values from 1 to 0 when they are fake (when `(pindex->nStatus & BLOCK_VALID_MASK) < BLOCK_VALID_TRANSACTIONS`), but a little simpler not to worry about being compatible in this case.
2024-03-11Merge bitcoin/bitcoin#26564: test: test_bitcoin: allow -testdatadir=<datadir>Ava Chow
d27e2d87b95b7982c05b4c88e463cc9626ab9f0a test: test_bitcoin: allow -testdatadir=<datadir> (Larry Ruane) Pull request description: This backward-compatible change would help with code review, testing, and debugging. When `test_bitcoin` runs, it creates a working or data directory within `/tmp/test_common_Bitcoin\ Core/`, named as a long random (hex) string. This small patch does three things: - If the (new) argument `-testdatadir=<datadir>` is given, use `<datadir>/test_temp/<test-name>/datadir` as the working directory - When the test starts, remove `<datadir>/test_temp/<test-name>/datadir` if it exists from an earlier run (currently, it's presumed not to exist due to the long random string) - Don't delete the working directory at the end of the test if a custom data directory is being used Example usage, which will remove, create, use `/somewhere/test_temp/getarg_tests/boolarg`, and leave it afterward: ``` $ test_bitcoin --run_test=getarg_tests/boolarg -- -testdatadir=/somewhere Running 1 test case... Test directory (will not be deleted): "/somewhere/test_temp/getarg_tests/boolarg/datadir" *** No errors detected $ ls -l /somewhere/test_temp/getarg_tests/boolarg/datadir total 8 drwxrwxr-x 2 larry larry 4096 Feb 22 10:28 blocks -rw-rw-r-- 1 larry larry 1273 Feb 22 10:28 debug.log ``` (A relative pathname also works.) This change affects only `test_bitcoin`; it could also be applied to `test_bitcoin-qt` but that's slightly more involved so I'm skipping that for now. The rationale for this change is that, when running the test using the debugger, it's often useful to watch `debug.log` as the test runs and inspect some of the other files (I've looked at the generated `blknnnn.dat` files for example). Currently, that requires figuring out where the test's working directory is since it changes on every test run. Tests can be run with `-printtoconsole=1` to show debug logging to the terminal, but it's nice to keep `debug.log` continuously open in an editor, for example. Even if not using a debugger, it's sometimes helpful to see `debug.log` and other artifacts after the test completes. Similar functionality is already possible with the functional tests using the `--tmpdir=` and `--nocleanup` arguments. ACKs for top commit: davidgumberg: ACK https://github.com/bitcoin/bitcoin/pull/26564/commits/d27e2d87b95b7982c05b4c88e463cc9626ab9f0a tdb3: re-ACK for d27e2d87b95b7982c05b4c88e463cc9626ab9f0a achow101: ACK d27e2d87b95b7982c05b4c88e463cc9626ab9f0a cbergqvist: ACK d27e2d87b95b7982c05b4c88e463cc9626ab9f0a! (Already did some testing with `fs::remove()` to make sure it was compatible with the `util::Lock/UnlockDirectory` implementation). marcofleon: ACK d27e2d87b95b7982c05b4c88e463cc9626ab9f0a. I ran all the tests with my previous open file limit and no errors were detected. Also ran some individual tests with no, relative, and absolute paths and everything looks good. furszy: ACK d27e2d8 Tree-SHA512: a8f535f34a48b6699cb440f97f5562ec643f3bfba4ea685768980b871fc8b6e1135f70fc05dbe19aa2c8bacb1ddeaff212d63473605a7422ff76332b3a6b1f68
2024-03-08Merge bitcoin/bitcoin#28960: kernel: Remove dependency on CSchedulerAva Chow
d5228efb5391b31a9a0673019e43e7fa2cd4ac07 kernel: Remove dependency on CScheduler (TheCharlatan) 06069b3913dda048f5d640a662b0852f86346ace scripted-diff: Rename MainSignals to ValidationSignals (TheCharlatan) 0d6d2b650d1017691f48c9109a6cd020ab46aa73 scripted-diff: Rename SingleThreadedSchedulerClient to SerialTaskRunner (TheCharlatan) 4abde2c4e3fd9b66394b79874583bdc2a9132c36 [refactor] Make MainSignals RAII styled (TheCharlatan) 84f5c135b8118cbe15b8bfb4db80d61237987f64 refactor: De-globalize g_signals (TheCharlatan) 473dd4b97ae40e43e1a1a97fdbeb40be4855e9bc [refactor] Prepare for g_signals de-globalization (TheCharlatan) 3fba3d5deec6d7bae33823b8da7682f9b03d9deb [refactor] Make signals optional in mempool and chainman (TheCharlatan) Pull request description: By defining a virtual interface class for the scheduler client, users of the kernel can now define their own event consuming infrastructure, without having to spawn threads or rely on the scheduler design. Removing `CScheduler` also allows removing the thread and exception modules from the kernel library. To make the `CMainSignals` class easier to use from a kernel library perspective, remove its global instantiation and adopt RAII practices. Renames `CMainSignals` to `ValidationSignals`, which more accurately describes its purpose and scope. Also make the `ValidationSignals` in the `ChainstateManager` and CTxMemPool` optional. This could be useful in the future for using or testing these classes without having to instantiate any form of signal handling. --- This PR is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587). It improves the kernel API and removes two modules from the kernel library. ACKs for top commit: maflcko: re-ACK d5228efb5391b31a9a0673019e43e7fa2cd4ac07 🌄 ryanofsky: Code review ACK d5228efb5391b31a9a0673019e43e7fa2cd4ac07. Just comment change since last review. vasild: ACK d5228efb5391b31a9a0673019e43e7fa2cd4ac07 furszy: diff ACK d5228ef Tree-SHA512: e93a5f10eb6182effb84bb981859a7ce750e466efd8171045d8d9e7fe46e4065631d9f6f533c5967c4d34c9bb7d7a67e9f4593bd4c5b30cd7b3bbad7be7b331b
2024-03-07test: test_bitcoin: allow -testdatadir=<datadir>Larry Ruane
Specifying this argument overrides the path location for test_bitcoin; it becomes <datadir>/test_common_Bitcoin Core/<testname>/datadir. Also, this directory isn't removed after the test completes. This can make it easier for developers to study the results of a test (see the state of the data directory after the test runs), and also (for example) have an editor open on debug.log to monitor it across multiple test runs instead of having to re-open a different pathname each time. Example usage (note the "--" is needed): test_bitcoin --run_test=getarg_tests/boolarg -- \ -testdatadir=/somewhere/mydatadir This will create (if necessary) and use the data directory: /somewhere/mydatadir/test_common_Bitcoin Core/getarg_tests/boolarg/datadir Co-authored-by: furszy <mfurszy@protonmail.com>
2024-02-20Merge bitcoin/bitcoin#29404: refactor: bitcoin-config.h includes cleanupfanquake
9d1dbbd4ceb8c04340927f5127195dc306adf3fc scripted-diff: Fix bitcoin_config_h includes (TheCharlatan) Pull request description: As mentioned in https://github.com/bitcoin/bitcoin/pull/26924#issuecomment-1403449932 and https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1922334399, it is currently not safe to remove `bitcoin-config.h` includes from headers because some unrelated file might be depending on it. See also #26972 for discussion. Solve this by including the file directly everywhere it's required, regardless of whether or not it's already included by another header. There should be no functional change here, but it will allow us to safely remove includes from headers in the future. ~I'm afraid it's a bit tedious to reproduce these commits, but it's reasonably straightforward:~ Edit: See note below ```bash # All commands executed from the src/ subdir. # Collect all tokens from bitcoin-config.h.in # Isolate the tokens and remove blank lines # Replace newlines with | and remove the last trailing one # Collect all files which use these tokens # Filter out subprojects (proper forwarding can be verified from Makefiles) # Filter out .rc files # Save to a text file git grep -E -l `grep undef config/bitcoin-config.h.in | cut -d" " -f2 | grep -v '^$' | tr '\n' '|' | sed 's/|$//'` | grep -v -e "^leveldb/" -e "^secp256k1/" -e "^crc32c/" -e "^minisketch/" -e "^Makefile" -e "\.rc$" > files-with-config-include.txt # Find all files from the above list which don't include bitcoin-config.h git grep -L -E "config/bitcoin-config.h" -- `cat files-with-config-include.txt` # Include them manually with the exception of some files in crypto: # crypto/sha256_arm_shani.cpp crypto/sha256_avx2.cpp crypto/sha256_sse41.cpp crypto/sha256_x86_shani.cpp # These are exceptions which don't use bitcoin-config.h, rather the Makefile.am adds these cppflags manually. # Commit changes. This should match the first commit of this PR. # Use the same search as above to find all files which DON'T use any config tokens git grep -E -L `grep undef config/bitcoin-config.h.in | cut -d" " -f2 | grep -v '^$' | tr '\n' '|' | sed 's/|$//'` | grep -v -e "^leveldb/" -e "^secp256k1/" -e "^crc32c/" -e "^minisketch/" -e "^Makefile" -e "\.rc$" > files-without-config-include.txt # Manually remove the includes and commit changes. This should match the second commit of this PR. ``` Edit: I'll keep this old description for posterity, but the manual approach has been replaced with a scripted diff from TheCharlatan ACKs for top commit: maflcko: ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3f 🚪 TheCharlatan: ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3fc hebasto: ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3fc, I have reviewed the code and it looks OK. fanquake: ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3fc Tree-SHA512: f11ddc4ae6a887f96b954a6b77f310558ddb271088a3fda3edc833669c4251b7f392515224bbb8e5f67eb2c799b4ffed3b07d96454e82ec635c686d0df545872
2024-02-16kernel: Remove dependency on CSchedulerTheCharlatan
By defining a virtual interface class for the scheduler client, users of the kernel can now define their own event consuming infrastructure, without having to spawn threads or rely on the scheduler design. Removing CScheduler also allows removing the thread and exception modules from the kernel library.
2024-02-15scripted-diff: Rename MainSignals to ValidationSignalsTheCharlatan
-BEGIN VERIFY SCRIPT- s() { git grep -l "$1" src | xargs sed -i "s/$1/$2/g"; } s 'CMainSignals' 'ValidationSignals' s 'MainSignalsImpl' 'ValidationSignalsImpl' -END VERIFY SCRIPT-
2024-02-15[refactor] Make MainSignals RAII styledTheCharlatan
2024-02-15refactor: De-globalize g_signalsTheCharlatan
2024-02-15[refactor] Make signals optional in mempool and chainmanTheCharlatan
This is done in preparation for the next two commits, where the CMainSignals are de-globalized. This avoids adding new constructor arguments to the ChainstateManager and CTxMemPool classes over the next two commits. This could also allow future tests that are only interested in the internal behaviour of the classes to forgo instantiating the signals.
2024-02-13scripted-diff: Fix bitcoin_config_h includesTheCharlatan
-BEGIN VERIFY SCRIPT- regex_string='^(?!//).*(AC_APPLE_UNIVERSAL_BUILD|BOOST_PROCESS_USE_STD_FS|CHAR_EQUALS_INT8|CLIENT_VERSION_BUILD|CLIENT_VERSION_IS_RELEASE|CLIENT_VERSION_MAJOR|CLIENT_VERSION_MINOR|COPYRIGHT_HOLDERS|COPYRIGHT_HOLDERS_FINAL|COPYRIGHT_HOLDERS_SUBSTITUTION|COPYRIGHT_YEAR|ENABLE_ARM_SHANI|ENABLE_AVX2|ENABLE_EXTERNAL_SIGNER|ENABLE_SSE41|ENABLE_TRACING|ENABLE_WALLET|ENABLE_X86_SHANI|ENABLE_ZMQ|HAVE_BOOST|HAVE_BUILTIN_CLZL|HAVE_BUILTIN_CLZLL|HAVE_BYTESWAP_H|HAVE_CLMUL|HAVE_CONSENSUS_LIB|HAVE_CXX20|HAVE_DECL_BE16TOH|HAVE_DECL_BE32TOH|HAVE_DECL_BE64TOH|HAVE_DECL_BSWAP_16|HAVE_DECL_BSWAP_32|HAVE_DECL_BSWAP_64|HAVE_DECL_FORK|HAVE_DECL_FREEIFADDRS|HAVE_DECL_GETIFADDRS|HAVE_DECL_HTOBE16|HAVE_DECL_HTOBE32|HAVE_DECL_HTOBE64|HAVE_DECL_HTOLE16|HAVE_DECL_HTOLE32|HAVE_DECL_HTOLE64|HAVE_DECL_LE16TOH|HAVE_DECL_LE32TOH|HAVE_DECL_LE64TOH|HAVE_DECL_PIPE2|HAVE_DECL_SETSID|HAVE_DECL_STRERROR_R|HAVE_DEFAULT_VISIBILITY_ATTRIBUTE|HAVE_DLFCN_H|HAVE_DLLEXPORT_ATTRIBUTE|HAVE_ENDIAN_H|HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR|HAVE_FDATASYNC|HAVE_GETENTROPY_RAND|HAVE_GETRANDOM|HAVE_GMTIME_R|HAVE_INTTYPES_H|HAVE_LIBADVAPI32|HAVE_LIBCOMCTL32|HAVE_LIBCOMDLG32|HAVE_LIBGDI32|HAVE_LIBIPHLPAPI|HAVE_LIBKERNEL32|HAVE_LIBOLE32|HAVE_LIBOLEAUT32|HAVE_LIBSHELL32|HAVE_LIBSHLWAPI|HAVE_LIBUSER32|HAVE_LIBUUID|HAVE_LIBWINMM|HAVE_LIBWS2_32|HAVE_MALLOC_INFO|HAVE_MALLOPT_ARENA_MAX|HAVE_MINIUPNPC_MINIUPNPC_H|HAVE_MINIUPNPC_UPNPCOMMANDS_H|HAVE_MINIUPNPC_UPNPERRORS_H|HAVE_NATPMP_H|HAVE_O_CLOEXEC|HAVE_POSIX_FALLOCATE|HAVE_PTHREAD|HAVE_PTHREAD_PRIO_INHERIT|HAVE_STDINT_H|HAVE_STDIO_H|HAVE_STDLIB_H|HAVE_STRERROR_R|HAVE_STRINGS_H|HAVE_STRING_H|HAVE_STRONG_GETAUXVAL|HAVE_SYSCTL|HAVE_SYSCTL_ARND|HAVE_SYSTEM|HAVE_SYS_ENDIAN_H|HAVE_SYS_PRCTL_H|HAVE_SYS_RESOURCES_H|HAVE_SYS_SELECT_H|HAVE_SYS_STAT_H|HAVE_SYS_SYSCTL_H|HAVE_SYS_TYPES_H|HAVE_SYS_VMMETER_H|HAVE_THREAD_LOCAL|HAVE_TIMINGSAFE_BCMP|HAVE_UNISTD_H|HAVE_VM_VM_PARAM_H|LT_OBJDIR|PACKAGE_BUGREPORT|PACKAGE_NAME|PACKAGE_STRING|PACKAGE_TARNAME|PACKAGE_URL|PACKAGE_VERSION|PTHREAD_CREATE_JOINABLE|QT_QPA_PLATFORM_ANDROID|QT_QPA_PLATFORM_COCOA|QT_QPA_PLATFORM_MINIMAL|QT_QPA_PLATFORM_WINDOWS|QT_QPA_PLATFORM_XCB|QT_STATICPLUGIN|STDC_HEADERS|STRERROR_R_CHAR_P|USE_ASM|USE_BDB|USE_DBUS|USE_NATPMP|USE_QRCODE|USE_SQLITE|USE_UPNP|_FILE_OFFSET_BITS|_LARGE_FILES)' exclusion_files=":(exclude)src/minisketch :(exclude)src/crc32c :(exclude)src/secp256k1 :(exclude)src/crypto/sha256_arm_shani.cpp :(exclude)src/crypto/sha256_avx2.cpp :(exclude)src/crypto/sha256_sse41.cpp :(exclude)src/crypto/sha256_x86_shani.cpp" git grep --perl-regexp --files-with-matches "$regex_string" -- '*.cpp' $exclusion_files | xargs git grep -L "bitcoin-config.h" | while read -r file; do line_number=$(awk -v my_file="$file" '/\/\/ file COPYING or https?:\/\/www.opensource.org\/licenses\/mit-license.php\./ {line = NR} /^\/\// && NR == line + 1 {while(getline && /^\/\//) line = NR} END {print line+1}' "$file"); sed -i "${line_number}i\\\\n\#if defined(HAVE_CONFIG_H)\\n#include <config/bitcoin-config.h>\\n\#endif" "$file"; done; git grep --perl-regexp --files-with-matches "$regex_string" -- '*.h' $exclusion_files | xargs git grep -L "bitcoin-config.h" | while read -r file; do sed -i "/#define.*_H/a \\\\n\#if defined(HAVE_CONFIG_H)\\n#include <config/bitcoin-config.h>\\n\#endif" "$file"; done; for file in $(git grep --files-with-matches 'bitcoin-config.h' -- '*.cpp' '*.h' $exclusion_files); do if ! grep -q --perl-regexp "$regex_string" $file; then sed -i '/HAVE_CONFIG_H/{N;N;N;d;}' $file; fi; done; -END VERIFY SCRIPT- The first command creates a regular expression for matching all bitcoin-config.h symbols in the following form: ^(?!//).*(AC_APPLE_UNIVERSAL_BUILD|BOOST_PROCESS_USE_STD_FS|...|_LARGE_FILES). It was generated with: ./autogen.sh && printf '^(?!//).*(%s)' $(awk '/^#undef/ {print $2}' src/config/bitcoin-config.h.in | paste -sd "|" -) The second command holds a list of files and directories that should not be processed. These include subtree directories as well as some crypto files that already get their symbols through the makefile. The third command checks for missing bitcoin-config headers in .cpp files and adds the header if it is missing. The fourth command checks for missing bitcoin-config headers in .h files and adds the header if it is missing. The fifth command checks for unneeded bitcoin-config headers in sources files and removes the header if it is unneeded.
2024-02-09Merge bitcoin/bitcoin#28948: v3 transaction policy for anti-pinningAva Chow
29029df5c700e6940c712028303761d91ae15847 [doc] v3 signaling in mempool-replacements.md (glozow) e643ea795e4b6fea4a6bbb3d72870ee6a4c836b1 [fuzz] v3 transactions and sigop-adjusted vsize (glozow) 1fd16b5c62f54c7f4c60122acd65d852f63d1e8b [functional test] v3 transaction submission (glozow) 27c8786ba918a42c860e6a50eaee9fdf56d7c646 test framework: Add and use option for tx-version in MiniWallet methods (MarcoFalke) 9a1fea55b29fe025355b06b45e3d77d192acc635 [policy/validation] allow v3 transactions with certain restrictions (glozow) eb8d5a2e7d939dd3ee683486e98702079e0dfcc0 [policy] add v3 policy rules (glozow) 9a29d470fbb62bbb27d517efeafe46ff03c25f54 [rpc] return full string for package_msg and package-error (glozow) 158623b8e0726dff7eae4288138f1710e727db9c [refactor] change Workspace::m_conflicts and adjacent funcs/structs to use Txid (glozow) Pull request description: See #27463 for overall package relay tracking. Delving Bitcoin discussion thread: https://delvingbitcoin.org/t/v3-transaction-policy-for-anti-pinning/340 Delving Bitcoin discussion for LN usage: https://delvingbitcoin.org/t/lightning-transactions-with-v3-and-ephemeral-anchors/418 Rationale: - There are various pinning problems with RBF and our general ancestor/descendant limits. These policies help mitigate many pinning attacks and make package RBF feasible (see #28984 which implements package RBF on top of this). I would focus the most here on Rule 3 pinning. [1][2] - Switching to a cluster-based mempool (see #27677 and #28676) requires the removal of CPFP carve out, which applications depend on. V3 + package RBF + ephemeral anchors + 1-parent-1-child package relay provides an intermediate solution. V3 policy is for "Priority Transactions." [3][4] It allows users to opt in to more restrictive topological limits for shared transactions, in exchange for the more robust fee-bumping abilities that offers. Even though we don't have cluster limits, we are able to treat these transactions as having as having a maximum cluster size of 2. Immediate benefits: - You can presign a transaction with 0 fees (not just 1sat/vB!) and add a fee-bump later. - Rule 3 pinning is reduced by a significant amount, since the attacker can only attach a maximum of 1000vB to your shared transaction. This also enables some other cool things (again see #27463 for overall roadmap): - Ephemeral Anchors - Package RBF for these 1-parent-1-child packages. That means e.g. a commitment tx + child can replace another commitment tx using the child's fees. - We can transition to a "single anchor" universe without worrying about package limit pinning. So current users of CPFP carve out would have something else to use. - We can switch to a cluster-based mempool [5] (#27677 #28676), which removes CPFP carve out [6]. [1]: Original mailing list post and discussion about RBF pinning problems https://gist.github.com/glozow/25d9662c52453bd08b4b4b1d3783b9ff, https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-January/019817.html [2]: A FAQ is "we need this for cluster mempool, but is this still necessary afterwards?" There are some pinning issues that are fixed here and not fully fixed in cluster mempool, so we will still want this or something similar afterward. [3]: Mailing list post for v3 https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-September/020937.html [4]: Original PR #25038 also contains a lot of the discussion [5]: https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393/7 [6]: https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393#the-cpfp-carveout-rule-can-no-longer-be-supported-12 ACKs for top commit: sdaftuar: ACK 29029df5c700e6940c712028303761d91ae15847 achow101: ACK 29029df5c700e6940c712028303761d91ae15847 instagibbs: ACK 29029df5c700e6940c712028303761d91ae15847 modulo that Tree-SHA512: 9664b078890cfdca2a146439f8835c9d9ab483f43b30af8c7cd6962f09aa557fb1ce7689d5e130a2ec142235dbc8f21213881baa75241c5881660f9008d68450
2024-02-08[fuzz] v3 transactions and sigop-adjusted vsizeglozow
Ensure we are checking sigop-adjusted virtual size by creating setups and packages where sigop cost is larger than bip141 vsize. Co-authored-by: Gregory Sanders <gsanders87@gmail.com>
2024-02-01refactor: Fix timedata includesMarcoFalke
2024-01-05Remove GetAdjustedTimedergoegge
2023-12-14Merge bitcoin/bitcoin#28051: Get rid of shutdown.cpp/shutdown.h, use ↵Ava Chow
SignalInterrupt directly 6db04be102807ee0120981a9b8de62a55439dabb Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directly (Ryan Ofsky) 213542b625a6a4885fcbdfe236629a5f381eeb05 refactor: Add InitContext function to initialize NodeContext with global pointers (Ryan Ofsky) feeb7b816affa790e02e7ba0780c4ef33d2310ff refactor: Remove calls to StartShutdown from KernelNotifications (Ryan Ofsky) 6824eecaf1e74624cf149ed20abd9145c49d614a refactor: Remove call to StartShutdown from stop RPC (Ryan Ofsky) 1d92d89edbb1812dc353084c62772ebb1024d632 util: Get rid of uncaught exceptions thrown by SignalInterrupt class (Ryan Ofsky) ba93966368d3aaa426b97837ef475ec5aa612f5f refactor: Remove call to ShutdownRequested from IndexWaitSynced (Ryan Ofsky) 42e5829d9710ebebda5de356fab01dd7c149d5fa refactor: Remove call to ShutdownRequested from HTTPRequest (Ryan Ofsky) 73133c36aa9cc09546eabac18d0ea35274dd5d72 refactor: Add NodeContext::shutdown member (Ryan Ofsky) f4a8bd6e2f03e786a84dd7763d1c04665e6371f2 refactor: Remove call to StartShutdown from qt (Ryan Ofsky) f0c73c1336bee74fe2d58474ac36bca28c219e85 refactor: Remove call to ShutdownRequested from rpc/mining (Ryan Ofsky) 263b23f0082c60516acced1b03abb8e4d8f9ee46 refactor: Remove call to ShutdownRequested from chainstate init (Ryan Ofsky) Pull request description: This change drops `shutdown.h` and `shutdown.cpp` files, replacing them with a `NodeContext::shutdown` member which is used to trigger shutdowns directly. This gets rid of an unnecessary layer of indirection, and allows getting rid of the `kernel::g_context` global. Additionally, this PR tries to improve error handling of `SignalInterrupt` code by marking relevant methods `[[nodiscard]]` to avoid the possibility of uncaught exceptions mentioned https://github.com/bitcoin/bitcoin/pull/27861#discussion_r1255496707. Behavior is changing In a few cases which are noted in individual commit messages. Particularly: GUI code more consistently interrupts RPCs when it is shutting down, shutdown state no longer persists between unit tests, the stop RPC now returns an RPC error if requesting shutdown fails instead of aborting, and other failed shutdown calls now log errors instead of aborting. This PR is a net reduction in lines of code, but in some cases the explicit error handling and lack of global shutdown functions do make it more verbose. The verbosity can be seen as good thing if it discourages more code from directly triggering shutdowns, and instead encourages code to return errors or send notifications that could be translated into shutdowns. Probably a number of existing shutdown calls could just be replaced by better error handling. ACKs for top commit: achow101: ACK 6db04be102807ee0120981a9b8de62a55439dabb TheCharlatan: Re-ACK 6db04be102807ee0120981a9b8de62a55439dabb maflcko: ACK 6db04be102807ee0120981a9b8de62a55439dabb 👗 stickies-v: re-ACK 6db04be102807ee0120981a9b8de62a55439dabb Tree-SHA512: 7a34cb69085f37e813c43bdaded1a0cbf6c53bd95fdde96f0cb45346127fc934604c43bccd3328231ca2f1faf712a7418d047ceabd22ef2dca3c32ebb659e634
2023-12-06fuzz: p2p: Detect peer deadlocksMarcoFalke
2023-12-04Get rid of shutdown.cpp/shutdown.h, use SignalInterrupt directlyRyan Ofsky
This change is mostly a refectoring that removes some code and gets rid of an unnecessary layer of indirection after #27861 But it is not a pure refactoring since StartShutdown, AbortShutdown, and WaitForShutdown functions used to abort on failure, and the replacement code logs or returns errors instead.
2023-12-04refactor: Remove calls to StartShutdown from KernelNotificationsRyan Ofsky
Use SignalInterrupt object instead. There is a slight change in behavior here because the previous StartShutdown code used to abort on failure and the new code logs errors instead.
2023-12-04refactor: Remove call to ShutdownRequested from IndexWaitSyncedRyan Ofsky
Use the node interrupt object instead. There is no change in behavior in this commit.
2023-12-04refactor: Add NodeContext::shutdown memberRyan Ofsky
Add NodeContext::shutdown variable and start using it to replace the kernel::Context::interrupt variable. The latter can't easily be removed right away but will be removed later in this PR. Moving the interrupt object from the kernel context to the node context increases flexibility of the kernel API so it is possible to use multiple interrupt objects, or avoid creating one if one is not needed. It will also allow getting rid of the kernel::g_context global later in this PR, replacing it with a private SignalInterrupt instance in init.cpp There is no change in behavior in this commit outside of unit tests. In unit tests there should be no visible change either, but internally now each test has its own interrupt variable so the variable will be automatically reset between tests.
2023-12-01Merge bitcoin/bitcoin#28368: Fee Estimator updates from Validation ↵Andrew Chow
Interface/CScheduler thread 91504cbe0de2b74ef1aa2709761aaf0597ec66a2 rpc: `SyncWithValidationInterfaceQueue` on fee estimation RPC's (ismaelsadeeq) 714523918ba2b853fc69bee6b04a33ba0c828bf5 tx fees, policy: CBlockPolicyEstimator update from `CValidationInterface` notifications (ismaelsadeeq) dff5ad3b9944cbb56126ba37a8da180d1327ba39 CValidationInterface: modify the parameter of `TransactionAddedToMempool` (ismaelsadeeq) 91532bd38223d7d04166e05de11d0d0b55e60f13 tx fees, policy: update `CBlockPolicyEstimator::processBlock` parameter (ismaelsadeeq) bfcd401368fc0dc43827a8969a37b7e038d5ca79 CValidationInterface, mempool: add new callback to `CValidationInterface` (ismaelsadeeq) 0889e07987294d4ef2814abfca16d8e2a0c5f541 tx fees, policy: cast with static_cast instead of C-Style cast (ismaelsadeeq) a0e3eb7549d2ba4dd3af12b9ce65e29158f59078 tx fees, policy: bugfix: move `removeTx` into reason != `BLOCK` condition (ismaelsadeeq) Pull request description: This is an attempt to #11775 This Pr will enable fee estimator to listen to ValidationInterface notifications to process new transactions added and removed from the mempool. This PR includes the following changes: - Added a new callback to the Validation Interface `MempoolTransactionsRemovedForConnectedBlock`, which notifies listeners about the transactions that have been removed due to a new block being connected, along with the height at which the transactions were removed. - Modified the `TransactionAddedToMempool` callback parameter to include additional information about the transaction needed for fee estimation. - Updated `CBlockPolicyEstimator` to process transactions using` CTransactionRef` instead of `CTxMempoolEntry.` - Implemented the `CValidationInterface` interface in `CBlockPolicyEstimater` and overridden the `TransactionAddedToMempool`, `TransactionRemovedFromMempool`, and `MempoolTransactionsRemovedForConnectedBlock` methods to receive updates from their notifications. Prior to this PR, the fee estimator updates from the mempool, i.e whenever a new block is connected all transactions in the block that are in our mempool are going to be removed using the `removeForBlock` function in `txmempool.cpp`. This removal triggered updates to the fee estimator. As a result, the fee estimator would block mempool's `cs` until it finished updating every time a new block was connected. Instead of being blocked only on mempool tx removal, we were blocking on both tx removal and fee estimator updating. If we want to further improve fee estimation, or add heavy-calulation steps to it, it is currently not viable as we would be slowing down block relay in the process This PR is smaller in terms of the changes made compared to #11775, as it focuses solely on enabling fee estimator updates from the validationInterface/cscheduler thread notifications. I have not split the validation interface because, as I understand it, the rationale behind the split in #11775 was to have `MempoolInterface` signals come from the mempool and `CValidationInterface` events come from validation. I believe this separation can be achieved in a separate refactoring PR when the need arises. Also left out some commits from #11775 - Some refactoring which are no longer needed. - Handle reorgs much better in fee estimator. - Track witness hash malleation in fee estimator I believe they are a separate change that can come in a follow-up after this. ACKs for top commit: achow101: ACK 91504cbe0de2b74ef1aa2709761aaf0597ec66a2 TheCharlatan: Re-ACK 91504cbe0de2b74ef1aa2709761aaf0597ec66a2 willcl-ark: ACK 91504cbe0de2b74ef1aa2709761aaf0597ec66a2 Tree-SHA512: 846dfb9da57a8a42458827b8975722d153907fe6302ad65748d74f311e1925557ad951c3d95fe71fb90ddcc8a3710c45abb343ab86b88780871cb9c38c72c7b1
2023-11-30Merge bitcoin/bitcoin#26762: bugfix: Make `CCheckQueue` RAII-styled (attempt 2)Andrew Chow
5b3ea5fa2e7f6dc1c9161ed8b74c9be4bd1e92dd refactor: Move `{MAX,DEFAULT}_SCRIPTCHECK_THREADS` constants (Hennadii Stepanov) 6e17b3168072ab77ed7170ab81327c017877133a refactor: Make `CCheckQueue` non-copyable and non-movable explicitly (Hennadii Stepanov) 8111e74653dc5c93cb510672d99048c3f741d8dc refactor: Drop unneeded declaration (Hennadii Stepanov) 9cf89f7a5b81197e38f58b24be0793b28fe41477 refactor: Make `CCheckQueue` constructor start worker threads (Hennadii Stepanov) d03eaacbcfb276fb638db1b423113ff43bd7ec41 Make `CCheckQueue` destructor stop worker threads (Hennadii Stepanov) be4ff3060b7b43b496dfb5a2c02b114b2b717106 Move global `scriptcheckqueue` into `ChainstateManager` class (Hennadii Stepanov) Pull request description: This PR: - makes `CCheckQueue` RAII-styled - gets rid of the global `scriptcheckqueue` - fixes https://github.com/bitcoin/bitcoin/issues/25448 The previous attempt was in https://github.com/bitcoin/bitcoin/pull/18731. ACKs for top commit: martinus: ACK 5b3ea5fa2e7 achow101: ACK 5b3ea5fa2e7f6dc1c9161ed8b74c9be4bd1e92dd TheCharlatan: ACK 5b3ea5fa2e7f6dc1c9161ed8b74c9be4bd1e92dd Tree-SHA512: 45cca846e7ed107e3930149f0b616ddbaf2648d6cde381f815331b861b5d67ab39e154883ae174b8abb1dae485bc904318c50c51e5d6b46923d89de51c5eadb0
2023-11-30Remove unused version.h includeMarcoFalke
2023-11-30Remove unused CDataStreamMarcoFalke
2023-11-29Merge bitcoin/bitcoin#28486: test, bench: Initialize and terminate use of ↵fanquake
Winsock properly fd4c6a10f2285f16c5d0215eb56a3060441f3ef2 test: Setup networking globally (Hennadii Stepanov) Pull request description: On the master branch, when compiling without external signer support, the `bench_bitcoin.exe` does not initialize Winsock DLL that is required, for example, here: https://github.com/bitcoin/bitcoin/blob/459272d639b9547f68000d2b9a5a0d991d477de5/src/bench/addrman.cpp#L124 Moreover, Windows docs explicitly [state](https://learn.microsoft.com/en-us/windows/win32/api/winsock/nf-winsock-wsacleanup) that `WSAStartup` and `WSACleanup` must be balanced: > There must be a call to `WSACleanup` for each successful call to `WSAStartup`. Only the final `WSACleanup` function call performs the actual cleanup. The preceding calls simply decrement an internal reference count in the WS2_32.DLL. That is not the case for our unit tests because the `SetupNetworking()` call is a part of the `BasicTestingSetup` fixture and is invoked multiple times, while `~CNetCleanup()` is invoked once only, at the end of the test binary execution. This PR fixes Winsock DLL initialization and termination. More docs: - https://learn.microsoft.com/en-us/windows/win32/winsock/initializing-winsock - https://learn.microsoft.com/en-us/windows/win32/api/winsock/nf-winsock-wsastartup - https://learn.microsoft.com/en-us/windows/win32/api/winsock/nf-winsock-wsacleanup Fix https://github.com/bitcoin/bitcoin/issues/28940. ACKs for top commit: maflcko: lgtm ACK fd4c6a10f2285f16c5d0215eb56a3060441f3ef2 Tree-SHA512: d360eaf776943f7f7a35ed5a5f9f3228d9e3d18eb824e5997cdc8eadddf466abe9f2da4910ee3bb86bf5411061e758259f7e1ec344f234ef7996f1bf8781dcda
2023-11-29fuzz: Fix nullptr deref in scriptpubkeymanMarcoFalke
Also, add missing includes to scriptpubkeyman. Also, export dependecies of the BasicTestingSetup from setup_common.h, to avoid having to include them when setup_common.h is already included.
2023-11-28test: Setup networking globallyHennadii Stepanov
2023-11-28Merge bitcoin/bitcoin#28892: refactor: P2P transport without serialize ↵fanquake
version and type fa79a881ce0537e1d74da296a7513730438d2a02 refactor: P2P transport without serialize version and type (MarcoFalke) fa9b5f4fe32c0cfe2e477bb11912756f84a52cfe refactor: NetMsg::Make() without nVersion (MarcoFalke) 66669da4a5ca9edf2a40d20879d9a8aaf2b9e2ee Remove unused Make() overload in netmessagemaker.h (MarcoFalke) fa0ed0794161d937d2d3385963c1aa5624b60d17 refactor: VectorWriter without nVersion (MarcoFalke) Pull request description: Now that the serialize framework ignores the serialize version and serialize type, everything related to it can be removed from the code. This is the first step, removing dead code from the P2P stack. A different pull will remove it from the wallet and other parts. ACKs for top commit: ajtowns: reACK fa79a881ce0537e1d74da296a7513730438d2a02 Tree-SHA512: 785b413580d980f51f0d4f70ea5e0a99ce14cd12cb065393de2f5254891be94a14f4266110c8b87bd2dbc37467676655bce13bdb295ab139749fcd8b61bd5110
2023-11-22tx fees, policy: CBlockPolicyEstimator update from `CValidationInterface` ↵ismaelsadeeq
notifications `CBlockPolicyEstimator` will implement `CValidationInterface` and subscribe to its notification to process transactions added and removed from the mempool. Re-delegate calculation of `validForFeeEstimation` from validation to fee estimator. Also clean up the validForFeeEstimation arg thats no longer needed in `CTxMempool`. Co-authored-by: Matt Corallo <git@bluematt.me>
2023-11-21Use Txid in COutpointdergoegge
2023-11-20refactor: NetMsg::Make() without nVersionMarcoFalke
The nVersion field is unused, so remove it. This is also required for future commits. Also, add PushMessage aliases in PeerManagerImpl to make calling code less verbose. Co-Authored-By: Anthony Towns <aj@erisian.com.au>
2023-11-17Merge bitcoin/bitcoin#28878: Remove version field from GetSerializeSizefanquake
83986f464c59a6517f790a960a72574e167f3f72 Include version.h in fewer places (Anthony Towns) c7b61fd61b199cbefda660c9d394bb4035a49528 Convert some CDataStream to DataStream (Anthony Towns) 1410d300df7e57a895f2697d9849a2201021c973 serialize: Drop useless version param from GetSerializeSize() (Anthony Towns) bf574a75016123309b894da895ab1c7a81731933 serialize: drop GetSerializeSizeMany (Anthony Towns) efa9eb6d7c8012fe4ed85699d81c8fe5dd18da1e serialize: Drop nVersion from [C]SizeComputer (Anthony Towns) Pull request description: Drops the version field from `GetSerializeSize()`, simplifying the code in various places. Also drop `GetSerializeSizeMany()` (as just removing the version parameter could result in silent bugs) and remove unnecessary instances of `#include <version.h>`. ACKs for top commit: maflcko: ACK 83986f464c59a6517f790a960a72574e167f3f72 📒 theuni: ACK 83986f464c59a6517f790a960a72574e167f3f72. Tree-SHA512: 36617b6dfbb1b4b0afbf673e905525fc6d623d3f568d3f86e3b9d4f69820db97d099e83a88007bfff881f731ddca6755ebf1549e8d8a7762437dfadbf434c62e
2023-11-16Merge bitcoin/bitcoin#28825: fuzz: Minor improvements to tx_package_eval targetfanquake
6a917918b76eef154c6757fe9ecf7713d526c3dd fuzz: allow fake and duplicate inputs in tx_package_eval target (Greg Sanders) a0626ccdadc0e965dc818d8a7c862e8c81b54fd1 fuzz: allow reaching MempoolAcceptResult::ResultType::DIFFERENT_WITNESS in tx_package_eval target (Greg Sanders) Pull request description: Exercises `DIFFERENT_WITNESS` by using "blank" WSH() and allowing witness to determine wtxid, and attempts to make invalid/duplicate inputs. ACKs for top commit: dergoegge: Coverage looks good to me ACK 6a917918b76eef154c6757fe9ecf7713d526c3dd Tree-SHA512: db894f5f5b81c6b454874baf11f296462832285f41ccb09f23c0db92b9abc98f8ecacd72fc8f60dc92cb7947f543a2e55bed2fd210b0e8ca7c7d5389d90b14af
2023-11-16Include version.h in fewer placesAnthony Towns
2023-11-14Use ParamsWrapper for witness serializationAnthony Towns
2023-11-09fuzz: allow reaching MempoolAcceptResult::ResultType::DIFFERENT_WITNESS in ↵Greg Sanders
tx_package_eval target