Age | Commit message (Collapse) | Author |
|
refactor: CBlockHeaderAndShortTxIDs constructor now always takes an explicit nonce.
test: Make blockencodings_tests deterministic using fixed seed providing deterministic
CBlockHeaderAndShortTxID nonces and dummy transaction IDs.
Fixes very rare flaky test failures, where the ShortIDs of test transactions collide, leading to
`READ_STATUS_FAILED` from PartiallyDownloadedBlock::InitData and/or `IsTxAvailable` giving `false`
when the transaction should actually be available.
* Use a new `FastRandomContext` with a fixed seed in each test, to ensure 'random' uint256s
used as fake prevouts are deterministic, so in-turn test txids and short IDs are deterministic
and don't collide causing very rare but flaky test failures.
* Add new test-only/internal initializer for `CBlockHeaderAndShortTxIDs` that takes a specified
nonce to further ensure determinism and avoid rare but undesireable short ID collisions.
In a test context this nonce is set to a fixed known-good value. Normally it is random, as
previously.
Flaky test failures can be reproduced with:
```patch
diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp
index 695e8d806a..64d635a97a 100644
--- a/src/blockencodings.cpp
+++ b/src/blockencodings.cpp
@@ -44,7 +44,8 @@ void CBlockHeaderAndShortTxIDs::FillShortTxIDSelector() const {
uint64_t CBlockHeaderAndShortTxIDs::GetShortID(const Wtxid& wtxid) const {
static_assert(SHORTTXIDS_LENGTH == 6, "shorttxids calculation assumes 6-byte shorttxids");
- return SipHashUint256(shorttxidk0, shorttxidk1, wtxid) & 0xffffffffffffL;
+ // return SipHashUint256(shorttxidk0, shorttxidk1, wtxid) & 0xffffffffffffL;
+ return SipHashUint256(shorttxidk0, shorttxidk1, wtxid) & 0x0f;
}
```
to increase the likelihood of a short ID collision; and running
```shell
set -e;
n=0;
while (( n++ < 5000 )); do
src/test/test_bitcoin --run_test=blockencodings_tests;
done
```
|
|
This new test uses the `vExtraTxnForCompact` (`extra_txn`) vector of
optional orphan/conflicted/etc. transactions to provide a transaction
in a compact block that was not otherwise present in our mempool.
This also covers an improbable nullptr deref bug addressed in
bf031a517c79cec5b43420bcd40291ab0e9f68a8 (#29752) where the
`extra_txn` vec/circular-buffer was sometimes null-initialized and
not yet filled when dereferenced in `PartiallyDownloadedBlock::InitData`.
|
|
all test cases
|
|
vec of pair<Wtxid, CTransactionRef>
All `CTransactionRef` have `.GetWitnessHash()` that returns a cached `const Wtxid` (since fac1223a568fa1ad6dd602350598eed278d115e8),
so we don't need to pass transaction refs around with their IDs as they're easy to get from a ref.
|
|
ambiguous uint256.
Wtxid/Txid types introduced in #28107
|
|
The remaining places are handled easier outside a scripted-diff.
-BEGIN VERIFY SCRIPT-
sed --regexp-extended -i 's/CDataStream ([0-9a-zA-Z_]+)\(SER_[A-Z]+, [A-Z_]+_VERSION\);/DataStream \1{};/g' $( git grep -l CDataStream)
sed -i 's/, CDataStream/, DataStream/g' src/wallet/walletdb.cpp
-END VERIFY SCRIPT-
|
|
|
|
-BEGIN VERIFY SCRIPT-
git grep -l "vTxHashesIdx" src | xargs sed -i "s/vTxHashesIdx/idx_randomized/g"
git grep -l "vTxHashes" src | xargs sed -i "s/vTxHashes/txns_randomized/g"
-END VERIFY SCRIPT-
|
|
vTxHashes exposes a complex mapTx iterator type that its external users
don't need. Directly populate it with CTransactionRef instead.
|
|
|
|
as many of the unit tests don't use this code
|
|
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
Commits of previous years:
- 2021: f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7
- 2020: fa0074e2d82928016a43ca408717154a1c70a4db
- 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
|
|
|
|
After this commit, there should be no explicit instantiation of
CTxMemPool in src/test other than those in fuzz/ and setup_common
-BEGIN VERIFY SCRIPT-
find_regex="CTxMemPool\s+([^;({]+)(|\(\)|\{\});" \
&& git grep -l -E "$find_regex" -- src/test \
| grep -v -e "^src/test/util/setup_common.cpp$" \
-e "^src/test/fuzz/" \
| xargs sed -i -E "s@$find_regex@CTxMemPool\& \1 = *Assert(m_node.mempool);@g"
-END VERIFY SCRIPT-
|
|
All uses of CBlockHeaderAndShortTxIDs in the product code are
constructed with fUseWTXID=true, so remove the parameter.
There is one use of the CBlockHeaderAndShortTxIDs constructor with
fUseWTXID=false in the unit tests. This is used to construct a
CBlockHeaderAndShortTxIDs for a block with only the coinbase
transaction, so setting fUseWTXID to true or false makes no difference.
Suggested in https://github.com/bitcoin/bitcoin/pull/20799#pullrequestreview-963480278
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
|
|
|
|
-BEGIN VERIFY SCRIPT-
# Move files
for f in $(git ls-files src/test/lib/); do git mv $f src/test/util/; done
git mv src/test/setup_common.cpp src/test/util/
git mv src/test/setup_common.h src/test/util/
# Replace Windows paths
sed -i -e 's|\\setup_common|\\util\\setup_common|g' $(git grep -l '\\setup_common')
sed -i -e 's|src\\test\\lib\\|src\\test\\util\\|g' build_msvc/test_bitcoin/test_bitcoin.vcxproj
# Everything else
sed -i -e 's|/setup_common|/util/setup_common|g' $(git grep -l 'setup_common')
sed -i -e 's|test/lib/|test/util/|g' $(git grep -l 'test/lib/')
# Fix include guard
sed -i -e 's|BITCOIN_TEST_SETUP_COMMON_H|BITCOIN_TEST_UTIL_SETUP_COMMON_H|g' ./src/test/util/setup_common.h
sed -i -e 's|BITCOIN_TEST_LIB_|BITCOIN_TEST_UTIL_|g' $(git grep -l 'BITCOIN_TEST_LIB_')
-END VERIFY SCRIPT-
|
|
|
|
|
|
unused includes in tests
|
|
0aef39d067 Silence "Test case [...] did not check any assertions" warnings when running "test_bitcoin --log_level=test_suite" (practicalswift)
5fd73c8694 Avoid repeated log messages in tests by connecting to signal handlers (ThreadSafeMessageBox, etc.) only once (practicalswift)
e502c3c515 tests: Reduce noise level in test_bitcoin output (practicalswift)
Pull request description:
Reduce noise level in `test_bitcoin` output.
Context: When working on the non-determinism issues in the unit tests (see #15296) I got a bit tired of the amount of noise in the `test_bitcoin` output :-)
Before:
```
$ src/test/test_bitcoin --log_level=test_suite 2>&1 | grep -vE '(Entering|Leaving)' | uniq -c
1 Running 341 test cases...
1 Test case blockencodings_tests/TransactionsRequestDeserializationOverflowTest did not check any assertions
1 CheckSplitTorReplyLine(PROTOCOLINFO PIVERSION)
1 CheckSplitTorReplyLine(AUTH METHODS=COOKIE,SAFECOOKIE COOKIEFILE="/home/x/.tor/control_auth_cookie")
1 CheckSplitTorReplyLine(AUTH METHODS=NULL)
1 CheckSplitTorReplyLine(AUTH METHODS=HASHEDPASSWORD)
1 CheckSplitTorReplyLine(VERSION Tor="0.2.9.8 (git-a0df013ea241b026)")
1 CheckSplitTorReplyLine(AUTHCHALLENGE SERVERHASH=aaaa SERVERNONCE=bbbb)
1 CheckSplitTorReplyLine(COMMAND)
1 CheckSplitTorReplyLine(COMMAND SOME ARGS)
1 CheckSplitTorReplyLine(COMMAND ARGS)
1 CheckSplitTorReplyLine(COMMAND EVEN+more ARGS)
1 CheckParseTorReplyMapping(METHODS=COOKIE,SAFECOOKIE COOKIEFILE="/home/x/.tor/control_auth_cookie")
1 CheckParseTorReplyMapping(METHODS=NULL)
1 CheckParseTorReplyMapping(METHODS=HASHEDPASSWORD)
1 CheckParseTorReplyMapping(Tor="0.2.9.8 (git-a0df013ea241b026)")
1 CheckParseTorReplyMapping(SERVERHASH=aaaa SERVERNONCE=bbbb)
1 CheckParseTorReplyMapping(ServiceID=exampleonion1234)
1 CheckParseTorReplyMapping(PrivateKey=RSA1024:BLOB)
1 CheckParseTorReplyMapping(ClientAuth=bob:BLOB)
1 CheckParseTorReplyMapping(Foo=Bar=Baz Spam=Eggs)
1 CheckParseTorReplyMapping(Foo="Bar=Baz")
1 CheckParseTorReplyMapping(Foo="Bar Baz")
1 CheckParseTorReplyMapping(Foo="Bar\ Baz")
1 CheckParseTorReplyMapping(Foo="Bar\Baz")
1 CheckParseTorReplyMapping(Foo="Bar\@Baz")
1 CheckParseTorReplyMapping(Foo="Bar\"Baz" Spam="\"Eggs\"")
1 CheckParseTorReplyMapping(Foo="Bar\\Baz")
1 CheckParseTorReplyMapping(Foo="Bar\nBaz\t" Spam="\rEggs" Octals="\1a\11\17\18\81\377\378\400\2222" Final=Check)
1 CheckParseTorReplyMapping(Valid=Mapping Escaped="Escape\\")
1 CheckParseTorReplyMapping(Valid=Mapping Bare="Escape\")
1 CheckParseTorReplyMapping(OneOctal="OneEnd\1" TwoOctal="TwoEnd\11")
1 CheckParseTorReplyMapping(Null="\0")
1 CheckParseTorReplyMapping(SOME=args,here MORE optional=arguments here)
1 CheckParseTorReplyMapping(ARGS)
1 CheckParseTorReplyMapping(MORE ARGS)
1 CheckParseTorReplyMapping(MORE ARGS)
1 CheckParseTorReplyMapping(EVEN more=ARGS)
1 CheckParseTorReplyMapping(EVEN+more ARGS)
1 Test case util_tests/util_criticalsection did not check any assertions
1 Testing known outcomes
326 Error: Specified -walletdir "/tmp/test_bitcoin/1553850209_943311758/tempdir/path_does_not_exist" does not exist
327 Error: Specified -walletdir "/tmp/test_bitcoin/1553850209_643733972/tempdir/not_a_directory.dat" is not a directory
328 Error: Specified -walletdir "wallets" is a relative path
1
1 *** No errors detected
```
After:
```
$ src/test/test_bitcoin --log_level=test_suite 2>&1 | grep -vE '(Entering|Leaving)' | uniq -c
1 Running 341 test cases...
1 Error: Specified -walletdir "/tmp/test_bitcoin/1553850026_943311758/tempdir/path_does_not_exist" does not exist
1 Error: Specified -walletdir "/tmp/test_bitcoin/1553850026_643733972/tempdir/not_a_directory.dat" is not a directory
1 Error: Specified -walletdir "wallets" is a relative path
1
1 *** No errors detected
```
ACKs for commit 0aef39:
MarcoFalke:
utACK 0aef39d0678ec2f26633028d44eea0ba0087e7c0
Tree-SHA512: 9cc22f64aa5c875861bae6533d18675ad978c623f053754deef6a8e271ea70bda3f72fb4ec5c8fd19b841757f21380639051d5f5b44301b9d2464b57655e9c05
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./src/bench/
./contrib/devtools/copyright_header.py update ./src/test/
-END VERIFY SCRIPT-
|
|
-BEGIN VERIFY SCRIPT-
sed -i --regexp-extended -e 's/test_bitcoin\.(h|cpp)/setup_common.\1/g' $(git grep -l test_bitcoin)
git mv ./src/test/test_bitcoin.h ./src/test/setup_common.h
git mv ./src/test/test_bitcoin.cpp ./src/test/setup_common.cpp
sed -i -e 's/BITCOIN_TEST_TEST_BITCOIN_H/BITCOIN_TEST_SETUP_COMMON_H/g' ./src/test/setup_common.h
-END VERIFY SCRIPT-
|
|
"test_bitcoin --log_level=test_suite"
|
|
|
|
Also add a test that the highest legal index is accepted.
|
|
(scripted-diff)
fa587773e59721e187cadc998f4dc236ad3aef0b scripted-diff: Remove unused first argument to addUnchecked (MarcoFalke)
fe5c49766c0dc5beaf186d77b568361242b20d5e tx pool: Use the entry's hash instead of the one passed to addUnchecked (MarcoFalke)
ddd395f968a050be5dd0ae21ba7d189b6b7f73fd Mark CTxMemPoolEntry members that should not be modified const (MarcoFalke)
Pull request description:
Several years ago the transaction hash was not cached. For optimization the hash was instead passed into `addUnchecked` to avoid re-calculating it. See f77654a0e9424f13cad04f82c014abd78fbb5e38
Passing in the hash is now redundant and the argument can safely be removed.
Tree-SHA512: 0206b65c7a014295f67574120e8c5397bf1b1bd70c918ae1360ab093676f7f89a6f084fd2c7000a141baebfe63fe6f515559e38c4ac71810ba64f949f9c0467f
|
|
3fc20632a3ad30809356a58d2cf0ea4a4ad4cec3 qt: Set BLOCK_CHAIN_SIZE = 220 (DrahtBot)
2b6a2f4a28792f2fe9dc1be843b1ff1ecae35e8a Regenerate manpages (DrahtBot)
eb7daf4d600eeb631427c018a984a77a34aca66e Update copyright headers to 2018 (DrahtBot)
Pull request description:
Some trivial maintenance to avoid having to do it again after the 0.17 branch off.
(The scripts to do this are in `./contrib/`)
Tree-SHA512: 16b2af45e0351b1c691c5311d48025dc6828079e98c2aa2e600dc5910ee8aa01858ca6c356538150dc46fe14c8819ed8ec8e4ec9a0f682b9950dd41bc50518fa
|
|
-BEGIN VERIFY SCRIPT-
git grep -l addUnchecked | xargs sed --regexp-extended -i -e 's/addUnchecked\([^)][^,]+,\s*/addUnchecked(/g'
-END VERIFY SCRIPT-
|
|
|
|
|
|
chain.h does not actually depend on the methods defined in pow.h, just its
include of consensus/params.h, which is standalone and can be included instead.
Confirmed by inspection and successful build.
|
|
|
|
|
|
-BEGIN VERIFY SCRIPT-
for f in \
src/*.cpp \
src/*.h \
src/bench/*.cpp \
src/bench/*.h \
src/compat/*.cpp \
src/compat/*.h \
src/consensus/*.cpp \
src/consensus/*.h \
src/crypto/*.cpp \
src/crypto/*.h \
src/crypto/ctaes/*.h \
src/policy/*.cpp \
src/policy/*.h \
src/primitives/*.cpp \
src/primitives/*.h \
src/qt/*.cpp \
src/qt/*.h \
src/qt/test/*.cpp \
src/qt/test/*.h \
src/rpc/*.cpp \
src/rpc/*.h \
src/script/*.cpp \
src/script/*.h \
src/support/*.cpp \
src/support/*.h \
src/support/allocators/*.h \
src/test/*.cpp \
src/test/*.h \
src/wallet/*.cpp \
src/wallet/*.h \
src/wallet/test/*.cpp \
src/wallet/test/*.h \
src/zmq/*.cpp \
src/zmq/*.h
do
base=${f%/*}/ relbase=${base#src/} sed -i "s:#include \"\(.*\)\"\(.*\):if test -e \$base'\\1'; then echo \"#include <\"\$relbase\"\\1>\\2\"; else echo \"#include <\\1>\\2\"; fi:e" $f
done
-END VERIFY SCRIPT-
|
|
Add missing locks to tests to satisfy lock requirements (such as
EXCLUSIVE_LOCKS_REQUIRED(...) (Clang Thread Safety Analysis),
AssertLockHeld(...) and implicit lock assumptions).
|
|
In order to avoid unintended implicit conversions.
|
|
|
|
-BEGIN VERIFY SCRIPT-
sed -i 's/\<insecure_randbits(/InsecureRandBits(/g' src/test/*.cpp src/test/*.h src/wallet/test/*.cpp
sed -i 's/\<insecure_randbool(/InsecureRandBool(/g' src/test/*.cpp src/test/*.h src/wallet/test/*.cpp
sed -i 's/\<insecure_randrange(/InsecureRandRange(/g' src/test/*.cpp src/test/*.h src/wallet/test/*.cpp
sed -i 's/\<insecure_randbytes(/InsecureRandBytes(/g' src/test/*.cpp src/test/*.h src/wallet/test/*.cpp
sed -i 's/\<insecure_rand256(/InsecureRand256(/g' src/test/*.cpp src/test/*.h src/wallet/test/*.cpp
sed -i 's/\<insecure_rand(/InsecureRand32(/g' src/test/*.cpp src/test/*.h src/wallet/test/*.cpp
sed -i 's/\<seed_insecure_rand(/SeedInsecureRand(/g' src/test/*.cpp src/test/*.h src/wallet/test/*.cpp
-END VERIFY SCRIPT-
|
|
-BEGIN VERIFY SCRIPT-
sed -i "s/\<GetRandHash(/insecure_rand256(/" src/test/*_tests.cpp
sed -i "s/\<GetRand(/insecure_randrange(/" src/test/*_tests.cpp src/test/test_bitcoin.cpp
sed -i 's/\<insecure_rand() % \([0-9]\+\)/insecure_randrange(\1)/g' src/test/*_tests.cpp
-END VERIFY SCRIPT-
|
|
ad82cb0 Remove unnecessary min fee argument in CTxMemPool constructor (Alex Morcos)
2a7b56c CBlockPolicyEstimator now uses hard coded minimum bucket feerate (Alex Morcos)
ac9d3d2 Change fee estimation bucket limit variable names (Alex Morcos)
Tree-SHA512: 6e3bc7df3497ed60c7620845d222063e33a0238020f5c3316e61e0eff758078588ea8dd51196ceb59aa561ba106f8cdae62cebe521adb3247108bb49f15252d6
|
|
|
|
|
|
91335ba Remove unused MakeTransactionRef overloads (Pieter Wuille)
6713f0f Make FillBlock consume txn_available to avoid shared_ptr copies (Pieter Wuille)
62607d7 Convert COrphanTx to keep a CTransactionRef (Pieter Wuille)
c44e4c4 Make AcceptToMemoryPool take CTransactionRef (Pieter Wuille)
|
|
Edited via:
$ contrib/devtools/copyright_header.py update .
|
|
|
|
|
|
|