| Age | Commit message (Collapse) | Author |
|---|
|
|
|
|
|
only integrals
597d10ceb9fd2a118c7e551cd6263379691d9295 tests: Add fuzzing harness for various functions consuming only integrals (practicalswift)
575383b3e1361e60ba88738a34d92b1662f915a7 tests: Add corpora suppression (FUZZERS_MISSING_CORPORA) for fuzzers missing in https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus (practicalswift)
Pull request description:
Add fuzzing harness for various functions consuming only integrals.
**Testing this PR**
Run:
```
$ CC=clang CXX=clang++ ./configure --enable-fuzz \
--with-sanitizers=address,fuzzer,undefined
$ make
$ src/test/fuzz/integer
```
Top commit has no ACKs.
Tree-SHA512: f0ccbd63671636f8e661385b682e16ad287fef8f92e7f91327ee2093afc36fcd424e1646fe90279388e28a760bcc795766eb80cf6375e0f873efff37fc7e2393
|
|
functions
d5766f223f627bf2eb731ce8552dfafa2b824378 tests: Add corpora suppression (FUZZERS_MISSING_CORPORA) for fuzzers missing in https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus (practicalswift)
e75ecb91c730115290e1201371492c2cd334e9b4 tests: Add fuzzing harness for various CTxOut related functions (practicalswift)
ce935292c041162e160d95fc6afeda3dceded2cf tests: Add fuzzing harness for various CTxIn related functions (practicalswift)
Pull request description:
Add fuzzing harness for various `CTx{In,Out}` related functions.
**Testing this PR**
Run:
```
$ CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined
$ make
$ src/test/fuzz/tx_in
…
$ src/test/fuzz/tx_out
…
# And to to quickly verify that the relevant code regions are triggered, that the
# fuzzing throughput seems reasonable, etc.
$ contrib/devtools/test_fuzzing_harnesses.sh '^tx_'
```
`test_fuzzing_harnesses.sh` can be found in PR #17000.
Top commit has no ACKs.
Tree-SHA512: f1374307a2581ebc3968d012ea2438061bbb84ece068e584fae9750669a6cd003723dde14db88e77c9579281ecd4eaa2a7ff0614f253d8c075e6dd16dd2e68d5
|
|
Test round-trip equality where possible.
709afb2a7de283a9188e7df51476830012e0a4e5 tests: Test serialisation as part of deserialisation fuzzing. Test round-trip equality where possible. Avoid code repetition. (practicalswift)
Pull request description:
Test serialisation as part of deserialisation fuzzing. Test round-trip equality where possible.
ACKs for top commit:
MarcoFalke:
ACK 709afb2a7de283a9188e7df51476830012e0a4e5 🍲
Tree-SHA512: b8c9c24538ee516607608ac685d2e9b01eca5c15213def3fd096b16516db84bfd45516fbee43e25b28cb3481a5d4ec3f7a34713e2da35b2902081ed42b85224d
|
|
|
|
round-trip equality where possible. Avoid code repetition.
|
|
|
|
|
|
|
|
897849d8c225045f0dd3a2fe99b5d69bdf84b4e2 tests: Add deserialization fuzzing harnesses (practicalswift)
16f0a186dcee563bb1000e1ffc51da87e7623bc6 tests: Add corpora suppression (FUZZERS_MISSING_CORPORA) for fuzzers missing in https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus (practicalswift)
Pull request description:
Add deserialization fuzzing harnesses.
**Testing this PR**
Run:
```
$ CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined
$ make
$ contrib/devtools/test_fuzzing_harnesses.sh 'addr_info|block_file_info|block_filter|block_header|ext_key|ext_pub_key|fee_rate|flat_file|key_origin|merkle_block|mutable_transaction|out_point|partial_merkle_tree|partially_signed_transaction|prefilled_transaction|psbt_input|psbt_output|pub_key|script_deserialize|sub_net|tx_in' 10
```
`test_fuzzing_harnesses.sh` can be found in PR #17000.
ACKs for top commit:
laanwj:
thanks, ACK 897849d8c225045f0dd3a2fe99b5d69bdf84b4e2
Tree-SHA512: 5a270a3002cc23b725f7b35476a43777b2b00b4d089cc006372e2fcc7afa430afaa3c1430f778ae08fc53dd85a13e7bd2fab0449c319f676423226e189a417f6
|
|
fa40e48c50d8ccf42ce5e66c12390e2ed4b60e75 ci: Remove unparseable lines from supp file for old xenial clang tsan (MarcoFalke)
fa1bfc476c9208a4c412c8ca74d05f52bb47766f ci: ubsan report_error_type=1 and add suppressions (MarcoFalke)
fa69cef13e5aab8264339eb3d50a9e89d59efd87 test: Print stderr when subprocess fails (MarcoFalke)
2222c305866a77065ab5be24c1c252bae252bb59 test: Use char instead of unsigned char (MarcoFalke)
faa8023ce9a47b282e1fac3ca8b3a7bb0042935a ci: Bump to clang-8 for asan build to avoid segfaults on ppc64le (MarcoFalke)
Pull request description:
Use clang-8 instead of default clang (which is clang-6 on Bionic) to avoid spurious segfaults when running the ci system on ppc64le
ACKs for top commit:
practicalswift:
ACK fa40e48c50d8ccf42ce5e66c12390e2ed4b60e75 assuming Travis is happy -- diff looks correct :)
Tree-SHA512: f4f26232d3a0ef38da245869340f723d279a3db9823befbc735fb5a00024dae041c7306d7ae55d2488e6f86aa96cdea155b007aefb561fba505141e8dbc717dc
|
|
|
|
76303f65f92a0fbe9a90c0e807554a6daa860636 test: add unit test for non-standard txs with wrong nVersion (Dominik Spicher)
Pull request description:
Takes care of one of the missing cases of #17394: nVersion must be within the allowed range.
ACKs for top commit:
instagibbs:
ACK https://github.com/bitcoin/bitcoin/pull/17555/commits/76303f65f92a0fbe9a90c0e807554a6daa860636
Tree-SHA512: 94464f781cf70a5616f7cea2014ae0a97a338c34411cc989c60389de2ce00368374811db78c919bda30e0ebf341fb830998a5e97c124dd8afc8feb726cedfd3a
|
|
70ed2ab7ef9e7ebf56f77b7c410a345ff455938f Add unit test for DB creation with unicode path (Aaron Clauson)
Pull request description:
An issue arose when attempting to switch back to the main repo version of leveldb when the bitcoin data directory uses a unicode path. The leveldb windows file IO wrapper was using the *A ANSI win32 calls instead of the Unicode *W ones. This unit test will catch if the path created by leveldb doesn't match what we're expecting. For more info see https://github.com/google/leveldb/issues/755.
ACKs for top commit:
laanwj:
ACK 70ed2ab7ef9e7ebf56f77b7c410a345ff455938f
Tree-SHA512: fc6dbd3aa26a439016e63e8d4d931f218ce99094fc7887a13b54562ad4133047020288ecbcd622a8309f422ee1eda5df50bcb8c8e44442af36ed57b22c069004
|
|
An issue arose when attempting to switch back to the main repo version of leveldb when the bitcoin data directory uses a unicode path. The leveldb windows file IO wrapper was using the *A ANSI win32 calls instead of the Unicode *W ones. This unit test will catch if the path created by leveldb doesn't match what we're expecting. For more info see https://github.com/google/leveldb/issues/755.
|
|
|
|
|
|
|
|
fa538813b1c382cf135cbf2a0cc3fa01f36964d8 scripted-diff: Replace ::mempool with m_node.mempool in tests (MarcoFalke)
8888ad02e204b0fa7a2ea2cfed2fc3f298cf1623 test: Replace recursive lock with locking annotations (MarcoFalke)
fac07f2038a3ccd5edadc6e6122c02fa30e697bd node: Add reference to mempool in NodeContext (MarcoFalke)
Pull request description:
This is the first step toward making the mempool a global that is not initialized before main.
#### Motivation
Currently the mempool is a global that is initialized before the `main` function. This is confusing and easy to get wrong. E.g. the mempool constructor queries state that has not been initialized, like randomness (fixed), or command line arguments (not an issue last time I checked). Also without having the chainstate (chain tip) initialized first, it doesn't make conceptually sense to have a mempool, since the mempool builds txs on top of the utxo set (chain tip).
Finally, in the future someone might want to run a consensus-only full node (`-nowallet -noblockfilter -no... -nomempool` command line options) that only verifies blocks and updates the utxo set.
This is conceptually the same change that has already been done for the connection manager `CConnman`.
ACKs for top commit:
jnewbery:
utACK fa538813b1c382cf135cbf2a0cc3fa01f36964d8
ariard:
Tested ACK fa53881.
Tree-SHA512: 2c446a8a51476354aad7126c2b833500d36b24490caa94f847b2bdc622054de0dae28980f23e3d91b1b492dc32931656d98dbd019af9e4e58f2f8c5375aac694
|
|
49f4c7f0699e5e19ac6e41ef5b607392dd7a2983 tests: Add fuzzing harness for various PSBT related functions (practicalswift)
Pull request description:
Add fuzzing harness for various PSBT related functions.
**Testing this PR**
Run:
```
$ CC=clang CXX=clang++ ./configure --enable-fuzz \
--with-sanitizers=address,fuzzer,undefined
$ make
$ src/test/fuzz/psbt
```
ACKs for top commit:
MarcoFalke:
re-ACK 49f4c7f0699e5e19ac6e41ef5b607392dd7a2983 🐟
Tree-SHA512: 4cebe62bd8c244ee40a43e829f5bd175ab40e1dfbbab1affb1529374858225820d6c9fa9ba45862bf56c1522845422fd96d620cedbdec52a67ac1449dec4e1b2
|
|
|
|
-BEGIN VERIFY SCRIPT-
# tx pool member access (mempool followed by dot)
sed --regexp-extended -i -e 's/(::)?\<mempool\>\.([a-zA-Z])/m_node.mempool->\2/g' $(git grep -l mempool ./src/test)
# plain global (mempool not preceeded by dot, but followed by comma)
sed --regexp-extended -i -e 's/([^\.])(::)?\<mempool\>,/\1*m_node.mempool,/g' $(git grep -l mempool ./src/test)
-END VERIFY SCRIPT-
|
|
Also, use m_node.mempool instead of the global
|
|
Currently it is an alias to the global ::mempool and should be used as
follows.
* Node code (validation and transaction relay) can use either ::mempool
or node.mempool, whichever seems a better fit.
* RPC code should use the added convenience getter EnsureMempool, which
makes sure the mempool exists before use. This prepares the RPC code
to a future where the mempool might be disabled at runtime or compile
time.
* Test code should use m_node.mempool directly, as the mempool is always
initialized for tests.
|
|
The function IsStandardTx() returns rejection reason "scriptsig-size" if any
one the inputs' scriptSig is larger than 1650 bytes.
|
|
|
|
083c954b02a4e7d0708349eeaf3bac2b5947fb0e Add settings_tests (Russell Yanofsky)
7f40528cd50fc43ac0bd3e785de24d661adddb7a Deduplicate settings merge code (Russell Yanofsky)
9dcb952fe5f85529ab28e091af7534e72c21c90f Add util::Settings struct and helper functions. (Russell Yanofsky)
e2e37cfe8af088bd8ea884be2f79f0f3cac555d5 Remove includeconf nested scope (Russell Yanofsky)
5a84aa880f6da0bac0e2144733fdef3b8558c761 Rename includeconf variables for clarity (Russell Yanofsky)
dc8e1e75487461ec9bff433144f0db831b682403 Clarify emptyIncludeConf logic (Russell Yanofsky)
Pull request description:
This is a refactoring-only change that makes it easier to add a new settings source.
This PR doesn't change behavior. The [`util_ArgsMerge`](https://github.com/bitcoin/bitcoin/blob/deb2327b435925c6a39ca654a79283b8eb6aeb86/src/test/util_tests.cpp#L626-L822) and [`util_ChainMerge`](https://github.com/bitcoin/bitcoin/blob/deb2327b435925c6a39ca654a79283b8eb6aeb86/src/test/util_tests.cpp#L843-L924) tests added in #15869 and #15988 were written specifically to confirm that ArgsManager settings are parsed, merged, and returned the same way before and after this change.
This change:
- Makes it easier to add new settings sources that can get merged with existing sources (see 70675c3e4975203ad6222ba2b00c83b4e4213793 from #15935).
- Separates parsing of settings from merging of settings, and deduplicates merging code so it doesn't happen five different places ([GetArg](https://github.com/bitcoin/bitcoin/blob/c459c5f70176928adcee4935813a2dbe7f4dbd51/src/util/system.cpp#L221-L244), [GetNetBoolArg](https://github.com/bitcoin/bitcoin/blob/c459c5f70176928adcee4935813a2dbe7f4dbd51/src/util/system.cpp#L255-L261), [GetArgs](https://github.com/bitcoin/bitcoin/blob/c459c5f70176928adcee4935813a2dbe7f4dbd51/src/util/system.cpp#L460-L467), [IsArgNegated](https://github.com/bitcoin/bitcoin/blob/c459c5f70176928adcee4935813a2dbe7f4dbd51/src/util/system.cpp#L482-L491), [GetUnsuitableSectionOnlyArgs](https://github.com/bitcoin/bitcoin/blob/c459c5f70176928adcee4935813a2dbe7f4dbd51/src/util/system.cpp#L343-L352)) in inconsistent ways.
- Documents and tests current strange merging behaviors, so they be cleaned up in the future if resulting code simplifications and UX improvements warrant loss of backwards compatibility. The newly documented behaviors are: command line [ignored arguments](https://github.com/ryanofsky/bitcoin/blob/69d44f3cc75a68d404ca0e1ca2b4831fd2bac4bb/src/util/system.cpp#L323-L326) and [more ignored arguments](https://github.com/ryanofsky/bitcoin/blob/69d44f3cc75a68d404ca0e1ca2b4831fd2bac4bb/src/util/settings.cpp#L67-L72), and config file [reverse precedence](https://github.com/ryanofsky/bitcoin/blob/69d44f3cc75a68d404ca0e1ca2b4831fd2bac4bb/src/util/settings.cpp#L61-L65), [inconsistently applied top-level settings](https://github.com/ryanofsky/bitcoin/blob/69d44f3cc75a68d404ca0e1ca2b4831fd2bac4bb/src/util/settings.cpp#L55-L59), and [zombie values](https://github.com/ryanofsky/bitcoin/blob/69d44f3cc75a68d404ca0e1ca2b4831fd2bac4bb/src/util/settings.cpp#L101-L108).
The original motivation for this change was to make it easy to add a new persistent setting source without introducing more bugs and inconsistencies. Two commits building on top of this to add a persistent `-wallet` setting are pretty straightforward and show how the new code can be extended:
* 70675c3e4975203ad6222ba2b00c83b4e4213793 from #15935 – _Add \<datadir>/settings.json persistent settings storage_
* 04c80c40df9fc6f4734ba238ea7f65607cf88089 from #15937 – _Add loadwallet and createwallet RPC load_on_startup options_
ACKs for top commit:
ariard:
ACK 083c954
jnewbery:
ACK 083c954b02a4e7d0708349eeaf3bac2b5947fb0e
jamesob:
ACK 083c954b02a4e7d0708349eeaf3bac2b5947fb0e
Tree-SHA512: 5d106746a44d64d3963c4ef3f4a2fa668a4bedcc9018d3ea12c86beae2fda48a0b036241665837f68685712366f70f2e1faba84d193fa1f456013503097b7659
|
|
36b68de5b2938722911db900ca299f7008780d01 Remove getBlockDepth method from Chain::interface (Antoine Riard)
b66c429c56c85fa16c309be0b2bca9c25fdd3e1a Remove locked_chain from GetDepthInMainChain and its callers (Antoine Riard)
0ff03871add000f8b4d8f82aeb168eed2fc9dc5f Use CWallet::m_last_block_processed_height in GetDepthInMainChain (Antoine Riard)
f77b1de16feee097a88e99d2ecdd4d84beb4f915 Only return early from BlockUntilSyncedToCurrentChain if current tip is exact match (Antoine Riard)
769ff05e48fb53d4b62c59060424a0fea71d0aab Refactor some importprunedfunds checks with guard clause (Antoine Riard)
5971d3848e09abf571e5308185275296127efca4 Add block_height field in struct Confirmation (Antoine Riard)
9700fcb47feca9d78e005b8d18b41148c8f6b25f Replace CWalletTx::SetConf by Confirmation initialization list (Antoine Riard)
5aacc3eff15b9b5bdc951f1e274f00d581f63bce Add m_last_block_processed_height field in CWallet (Antoine Riard)
10b4729e33f76092bd8cfa06d1a5e0a066436f76 Pass block height in Chain::BlockConnected/Chain::BlockDisconnected (Antoine Riard)
Pull request description:
Work starter to remove Chain::Lock interface by adding m_last_block_processed_height in CWallet and m_block_height in CMerkleTx to avoid GetDepthInMainChain having to keep a lock . Once this one done, it should ease work to wipe out more cs_main locks from wallet code.
I think it's ready for a first round of review before to get further.
- `BlockUntilSyncedToCurrent` : restrain isPotentialTip to isTip because we want to be sure that wallet see BlockDisconnected callbacks if its height differs from the Chain one. It means during a reorg, an RPC could return before the BlockDisconnected callback had been triggered. This could cause a tx that had been included in the disconnected block to be displayed as confirmed, for example.
~~- `AbandonTransaction` : in case of conflicted tx (nIndex = -1), we set its m_block_height to the one of conflicting blocks, but if this height is superior to CWallet::m_last_block_processed_height, that means tx isn't conflicted anymore so we return 0 as tx is again unconfirmed~~ After #16624, we instead rely on Confirmation.
~~- `AddToWalletIfInvolvingMe`: in case of block disconnected, transactions are added to mempool again, so we need to replace old txn in `mapWallet` with a height set to zero so we remove check on block_hash.IsNull~~ Already done in #16624
ACKs for top commit:
jnewbery:
@jkczyz you've ACKed an intermediate commit (github annoyingly orders commits in date order, not commit order). Did you mean to ACK the final commit in this branch (36b68de5b2938722911db900ca299f7008780d01).
jkczyz:
> @jkczyz you've ACKed an intermediate commit (github annoyingly orders commits in date order, not commit order). Did you mean to ACK the final commit in this branch ([36b68de](https://github.com/bitcoin/bitcoin/commit/36b68de5b2938722911db900ca299f7008780d01)).
meshcollider:
utACK 36b68de5b2938722911db900ca299f7008780d01
ryanofsky:
Code review ACK 36b68de5b2938722911db900ca299f7008780d01. Changes since last review: new jkczyz refactor importprunedfunds commit, changed BlockUntilSyncedToCurrentChainChanges commit title and description, changed Confirmation struct field order and line-wrapped comment
jnewbery:
utACK 36b68de5b2938722911db900ca299f7008780d01
promag:
Code review ACK 36b68de5b2938722911db900ca299f7008780d01.
Tree-SHA512: 08b89a0bcc39f67c82a6cb6aee195e6a11697770c788ba737b90986b4893f44e90d1ab9ef87239ea3766508b7e24ea882b7199df41173ab27a3d000328c14644
|
|
Co-authored-by: James O'Beirne <james.obeirne@gmail.com>
|
|
Get rid of settings merging code in util/system.cpp repeated 5 places,
inconsistently:
- ArgsManagerHelper::GetArg
- ArgsManagerHelper::GetNetBoolArg
- ArgsManager::GetArgs
- ArgsManager::IsArgNegated
- ArgsManager::GetUnsuitableSectionOnlyArgs
Having settings merging code separated from parsing simplifies parsing somewhat
(for example negated values can simply be represented as false values instead
of partially cleared or emply placeholder lists).
Having settings merge happen one place instead of 5 makes it easier to add new
settings sources and harder to introduce new inconsistencies in the way
settings are merged.
This commit does not change behavior in any way.
|
|
fae43a97ca947cd0802392e9bb86d9d0572c0fba test: Seed test RNG context for each test case, print seed (MarcoFalke)
Pull request description:
Debugging failing unit tests is hard if the failure is non-deterministic and the seed is not known.
Fix that by printing the seed and making it possible to set the seed from outside.
ACKs for top commit:
davereikher:
Tested ACK fae43a97ca947cd0802392e9bb86d9d0572c0fba
Tree-SHA512: 33d848dd1f4180d3664ecf60e9810c2a93590c05276b2c46b1e4fe6e376b45916a46b90c803bb602750ab666da3a05ce499e550024685a90b8cc38fab6667cb8
|
|
5506ecfe7a65d5705616bc048f2f1735b89993fb [refactor] Replace global int nScriptCheckThreads with bool (John Newbery)
d9957623b48a7c3eff0ac750d1245fabfb1843a2 [tests] Don't use TestingSetup in the checkqueue_tests (John Newbery)
Pull request description:
The meaning of this value is confusing. Refactor it and add comments.
ACKs for top commit:
sipa:
ACK 5506ecfe7a65d5705616bc048f2f1735b89993fb
promag:
ACK 5506ecfe7a65d5705616bc048f2f1735b89993fb, only change was addressing my nits.
laanwj:
Code review ACK 5506ecfe7a65d5705616bc048f2f1735b89993fb
MarcoFalke:
ACK 5506ecfe7a65d5705616bc048f2f1735b89993fb 🥐
Tree-SHA512: 78536727c98d2c23f3c0f3f169131474fef9a4486ae65029011caf06eab30f6f70ff73a65b2fb04a5d969fc1150858d1c6ea4767f04d48c1eea6b829316d0e63
|
|
fa4c6fa9b1139791f45f1495d662c1c7cd2f7ed6 doc: Add documentation for new test/lib (MarcoFalke)
faec28252cf4f8e754c689a7a44fd421f631db50 scripted-diff: test: Move setup_common to test library (MarcoFalke)
Pull request description:
Sorry for clickbait, this is only a move-only scripted-diff commit and one documentation commit.
Longer term, someone who knows something about build systems can make this an actual library. Motivation for this is that each module gets compiled for each target that includes it. For example, setup_common is compiled 27 times (for the fuzz suite) and another 3 times for the other tests (bench, unit test, gui)
ACKs for top commit:
practicalswift:
ACK fa4c6fa9b1139791f45f1495d662c1c7cd2f7ed6 -- diff looks correct and Travis is happy
jonatack:
ACK fa4c6fa9b1139791f45f1495d662c1c7cd2f7ed6 with the reserve that the commit messages (and PR description) contain the motivation for this change. Built, ran tests, light code review.
ryanofsky:
Code review ACK fa4c6fa9b1139791f45f1495d662c1c7cd2f7ed6. I didn't realize `lib` was actually name of existing directory, not a new name. But in any case this looks good and nice to have one scripted diff instead of two.
Tree-SHA512: 2e176df90c60578276e4a6dc83ff57ff59d8e666ecf30c5ceacb8c326725da91baa4cac3dfa7a2e1605f58122a3e3e27e4938ff33e3a0ce7ea53afffebbf57a4
|
|
The global nScriptCheckThreads int is confusing and is only needed for
its int-ness in AppInitMain. Move all `-par` parsing logic there and
replace the int nScriptCheckThreads with a bool
g_parallel_script_checks.
Also tidy up logic and improve comments.
|
|
It's only needed for a hardcoded int, which we can define locally.
|
|
|
|
-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-
|
|
286f197704e82045c762d332aba5d1ac52e0212d Add util_ArgParsing test (Russell Yanofsky)
Pull request description:
ArgsManager test coverage for parsing of integer and boolean values is
currently very poor and doesn't give us a way of knowing whether changes to
ArgsManager may unintentionally break backwards compatibility, so this adds a
new test to catch regressions.
ACKs for top commit:
promag:
ACK 286f197, more surprising results 😱
laanwj:
ACK 286f197704e82045c762d332aba5d1ac52e0212d
Tree-SHA512: 9e1db3ef87e55abbc280af60c088f35765a1f9e2ec20507ad0c1992027b875490016868dcb8cc287e6df279dd0e00f10550901af3de3d36287867249e0bd8207
|
|
3645e4ca0033bb6365f41ef710111780c139370f Add missing newline in util_ChainMerge test (Russell Yanofsky)
Pull request description:
This was causing a lot of test cases not not be very meaningful because
multiple configuration options were combined into one line.
The changes in test output with this fix make sense and look like:
```diff
- testnet=1 regtest=1 || test
+ testnet=1 regtest=1 || error: Invalid combination of -regtest, -testnet and -chain. Can use at most one.
```
Issue was reported and debugged by
Wladimir J. van der Laan <laanwj@protonmail.com> in
https://github.com/bitcoin/bitcoin/pull/17385#issuecomment-550033222
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`). Contributors should note which tests cover
modified code. If no tests exist for a region of modified code, new tests
should accompany the change.
* Bug fixes are most welcome when they come with steps to reproduce or an
explanation of the potential issue as well as reasoning for the way the bug
was fixed.
* Features are welcome, but might be rejected due to design or scope issues.
If a feature is based on a lot of dependencies, contributors should first
consider building the system outside of Bitcoin Core, if possible.
* Refactoring changes are only accepted if they are required for a feature or
bug fix or otherwise improve developer experience significantly. For example,
most "code style" refactoring changes require a thorough explanation why they
are useful, what downsides they have and why they *significantly* improve
developer experience or avoid serious programming bugs. Note that code style
is often a subjective matter. Unless they are explicitly mentioned to be
preferred in the [developer notes](/doc/developer-notes.md), stylistic code
changes are usually rejected.
-->
<!--
Bitcoin Core has a thorough review process and even the most trivial change
needs to pass a lot of eyes and requires non-zero or even substantial time
effort to review. There is a huge lack of active reviewers on the project, so
patches often sit for a long time.
-->
ACKs for top commit:
laanwj:
ACK 3645e4ca0033bb6365f41ef710111780c139370f
practicalswift:
ACK 3645e4ca0033bb6365f41ef710111780c139370f -- diff looks correct
Tree-SHA512: ca5bde9b9f553811d4827113f4880d15d7b8f4f1455b95bbf34c9a1512fdd53062f1a2133c50d9b54f94160a1ee77a54bc82681a5f3bf25d2b0d01f8a8e95165
|
|
ArgsManager test coverage for parsing of integer and boolean values is
currently very poor and doesn't give us a way of knowing whether changes to
ArgsManager may unintentionally break backwards compatibility, so this adds a
new test to catch regressions.
|
|
dcef9a2922317cb2849f397366b6c56d755db6c9 logs: add timing information to FlushStateToDisk() (James O'Beirne)
41edaf227a69bc4846d5996675e8763fdfe0f367 logs: add BCLog::Timer and related macros (James O'Beirne)
Pull request description:
It's currently annoying to detect FlushStateToDisk() calls when benchmarking since they have to be inferred from a drop in coins count from the `UpdateTip: ` log messages. This adds a new logging utility, `BCLog::Timer`, and some related macros that are generally useful for printing timing-related logging messages, and a message that is unconditionally written when the coins cache is flushed to disk.
```
2019-09-04T20:17:51Z FlushStateToDisk: write block and undo data to disk completed (3ms)
2019-09-04T20:17:51Z FlushStateToDisk: write block index to disk completed (370ms)
2019-09-04T20:17:51Z FlushStateToDisk: write coins cache to disk (2068451 coins, 294967kB) completed (21481ms)
```
ACKs for top commit:
laanwj:
Thanks, ACK dcef9a2922317cb2849f397366b6c56d755db6c9
ryanofsky:
Code review ACK dcef9a2922317cb2849f397366b6c56d755db6c9. No changes since last review other than moving code to new timer.h header
Tree-SHA512: 6d61e48a062d3edb48d0e056a6f0b1f8031773cc99289ee4544f8349d24526b88519e1e304009d56e428f1eaf76c857bf8e7e1c0b6873a6f270306accb5edc3d
|
|
This was causing a lot of test cases not not be very meaningful because
multiple configuration options were combined into one line.
The changes in test output with this fix make sense and look like:
```diff
- testnet=1 regtest=1 || test
+ testnet=1 regtest=1 || error: Invalid combination of -regtest, -testnet and -chain. Can use at most one.
```
Issue was reported and debugged by
Wladimir J. van der Laan <laanwj@protonmail.com> in
https://github.com/bitcoin/bitcoin/pull/17385#issuecomment-550033222
|
|
fa2c44c3ccc3e7a54e2afc862addd555d5a6e51b test: Add ASSERT_DEBUG_LOG to unit test framework (MarcoFalke)
fa1936f57bbf5aebb1f8fc18701441d79219d443 logging: Add member for arbitrary print callbacks (MarcoFalke)
Pull request description:
Similar to `assert_debug_log` in the functional test framework
Top commit has no ACKs.
Tree-SHA512: aa9eaeca386b61d806867c04a33275f6eb4624fa5bf50f2928d16c83f5634bac96bcac46f9e8eda3b00b4251c5f12d7b01d6ffd84ba8e05c09eeec810cc31251
|
|
b2ff500fb3e4fa05de366ab1900825bea1f70377 test: add "diamond" unit test to MempoolAncestryTests (Sebastian Falbesoner)
Pull request description:
Approaches #17271 (_Missing Unit Test for Ancestors "diamond"_).
If ancestors are represented more than once (in this case `ta` and `tb`), check that those are not overcounted.
ACKs for top commit:
laanwj:
ACK b2ff500fb3e4fa05de366ab1900825bea1f70377
Tree-SHA512: 82a6573cc7f0e82bf6fcfe207d7ddecbf297d2a203d22e95b73d887e3cb280f45a3c5f649161561c1be1eb560ff81b9b385868f205d1c12284211c2377e5ad99
|
|
To do so we update CValidationInterface::BlockDisconnect to take a
CBlockIndex pointing to the block being disconnected.
This new parameter will be use in the following commit to establish
wallet height.
|
|
b7541705d0abfbddf682a0134f3fa8a8e1d06cdf tests: Add fuzzing harness for Bech32 encoding/decoding (practicalswift)
85a34b1683233060c4500cfb8e3f5d84c8b8f0da tests: Move CaseInsensitiveEqual to test/util/str (practicalswift)
Pull request description:
Add fuzzing harness for Bech32 encoding/decoding.
**Testing this PR**
Run:
```
$ make distclean
$ ./autogen.sh
$ CC=clang CXX=clang++ ./configure --enable-fuzz \
--with-sanitizers=address,fuzzer,undefined
$ make
$ src/test/fuzz/bech32 -max_total_time=60
…
```
ACKs for top commit:
jonatack:
ACK b7541705d0abfbddf682a0134f3fa8a8e1d06cdf
Tree-SHA512: ade01d30c6886a083b806dbfff08999cc0d08e687701c670c895e261ed242c789e8a0062d4ebbe8f82676b8f168dc37e83351a88822c9c0eab478572a9e1ec02
|
|
1a8f0d5a74d5cc0000456932babf35301f5c1686 [tools] update nNextInvSend to use mockable time (Amiti Uttarwar)
4de630354fc6808b9b13b9e82da1a82f2f50f26a [tools] add PoissonNextSend method that returns mockable time (Amiti Uttarwar)
Pull request description:
Introduce a Poisson helper method that wraps the existing method to return `std::chrono::duration` type, which is mockable.
Needed for https://github.com/bitcoin/bitcoin/pull/16698.
ACKs for top commit:
ajtowns:
ACK 1a8f0d5a74d5cc0000456932babf35301f5c1686
MarcoFalke:
re-ACK 1a8f0d5a74d5cc0000456932babf35301f5c1686
naumenkogs:
ACK 1a8f0d5, and let's merge it and come back to it later.
Tree-SHA512: 7e2325d7c55fc0b4357cb86b83e0c218ba269f678c1786342d8bc380bfd9696373bc24ff124b9ff17a6e761c62b2b44ff5247c3911e2afdc7cc5c20417e8290b
|
|
|
|
|
