aboutsummaryrefslogtreecommitdiff
path: root/src/test
AgeCommit message (Collapse)Author
2022-03-30refactor: merge ScriptPubKeyToUniv & ScriptToUniv into one functionMichael Dietz
2022-03-30util/check: stop using lambda for Assert/AssumeAnthony Towns
2022-03-30refactor: account for requiring libevent 2.1.8+fanquake
2022-03-30Merge bitcoin/bitcoin#24692: refactoring: [Net Processing] Follow-ups to #21160fanquake
a40978dcbd6608695bc7f5191c4d0a3e48cbca0b [fuzz] Assert that Peer.m_tx_relay.m_relay_txs has been set correctly (John Newbery) 0bca5f2b465616d7ac915ddcc1acd9a021e40abb [net processing] PushNodeVersion() takes a const Peer& (John Newbery) 21154ff9270e4bbae02d9012a4a73cce86a00334 net_processing: move CNode data access out of lock (John Newbery) Pull request description: #21160 ([net/net processing]: Move tx inventory into net_processing) had some unaddressed review comments when it was merged. This branch addresses those comments. ACKs for top commit: MarcoFalke: review ACK a40978dcbd6608695bc7f5191c4d0a3e48cbca0b dergoegge: ACK a40978dcbd6608695bc7f5191c4d0a3e48cbca0b ajtowns: ACK a40978dcbd6608695bc7f5191c4d0a3e48cbca0b Tree-SHA512: 46624e275f918c5f32d0adab0766e9b3ef8ebdbc74a3c8886d8a2e2ff1079029dcc371b40ef0d787609e9c05219b7456f3e2dfe4fb0cb7bf23ef966769aef1a1
2022-03-29[fuzz] Assert that Peer.m_tx_relay.m_relay_txs has been set correctlyJohn Newbery
2022-03-29Merge bitcoin/bitcoin#24523: build: Fix Boost.Process test for Boost 1.78laanwj
532c64a7264dd3c7329e8839547837c57da7dbe8 build: Fix Boost.Process test for Boost 1.78 (Hennadii Stepanov) Pull request description: Rebased #24415 with Luke's suggestion. Fixes #24413. ACKs for top commit: hebasto: ACK 532c64a7264dd3c7329e8839547837c57da7dbe8, tested on Mac mini (M1, 2020) + macOS Monterey 12.3 (21E230). Tree-SHA512: 74f779695f6bbc45a2b7341a1402f747cc0d433d74825c7196cb9f156db0c0299895365f01665bd0bff12a8ebb5ea33a29b9a52f5eac0007ec35d1dca6544705
2022-03-25Merge bitcoin/bitcoin#21160: net/net processing: Move tx inventory into ↵fanquake
net_processing 1066d10f71e6800c78012d789ff6ae19df0243fe scripted-diff: rename TxRelay members (John Newbery) 575bbd0dea6d12510fdf3220d0f0e47d969da6e9 [net processing] Move tx relay data to Peer (John Newbery) 785f55f7eeab0dedbeb8e0d0b459f3bdc538b621 [net processing] Move m_wtxid_relay to Peer (John Newbery) 36346703f8558d6781c079c29ddece5a97477beb [net] Add CNode.m_relays_txs and CNode.m_bloom_filter_loaded (John Newbery) Pull request description: This continues the work of moving application layer data into net_processing, by moving all tx data into the new Peer object added in #19607. For motivation, see #19398. ACKs for top commit: dergoegge: ACK 1066d10f71e6800c78012d789ff6ae19df0243fe - This is a good layer separation improvement with no behavior changes. glozow: utACK 1066d10f71e6800c78012d789ff6ae19df0243fe Tree-SHA512: 0c9d6b8a0a05e2d816b6d6588b7df133842ec960ae67667813422aa7bd8eb5308599c714f3822a98ddbdf364ffab9050b055079277ba4aff24092557ff99ebcc
2022-03-24Merge bitcoin/bitcoin#24205: init, test: improve network reachability test ↵MarcoFalke
coverage and safety 58a14795b89a6bd812e0b71cb8b3088b8ab55c11 test: passing -onlynet=onion with -onion=0/-noonion raises expected init error (Jon Atack) 7000f66d367123d1de303fc15ce2ce60df379c11 test: passing -onlynet=onion without -proxy/-onion raises expected init error (Jon Atack) 8332e6e4cf45455fea0bf1f7527256cdb7bb1e6d test: passing invalid -onion raises expected init error (Jon Atack) d5edb087082a50e6f7d413c3b43fdf1e6a20d29b test: passing invalid -proxy raises expected init error (Jon Atack) bd57dcbaf2b5e5f50833912c894a1f1239ceb25b test: hoist proxy out of 2 network loops in feature_proxy.py (Jon Atack) afdf2de28296660fd0284453a241aece8494eea8 test: add CJDNS to LimitedAndReachable_Network unit tests (Jon Atack) 2b7a8180a94738c2fcb21232a2eca07a7b27656d net, init: assert each network reachability is true by default (Jon Atack) Pull request description: Adds missing network reachability test coverage and an assertion during init, noticed while reviewing #22834: - assert during init that each network reachability is true by default - add CJDNS to the `LimitedAndReachable_Network` unit tests - hoist proxy out of two network loops in feature_proxy.py - test that passing invalid `-proxy` raises expected init error - test that passing invalid `-onion` raises expected init error - test that passing `-onlynet=onion` without `-proxy` and `-onion` raises expected init error - test that passing `-onlynet=onion` with `-onion=0` and with `-noonion` raises expected init error ACKs for top commit: vasild: ACK 58a14795b89a6bd812e0b71cb8b3088b8ab55c11 brunoerg: ACK 58a14795b89a6bd812e0b71cb8b3088b8ab55c11 dongcarl: Code Review ACK 58a14795b89a6bd812e0b71cb8b3088b8ab55c11 Tree-SHA512: bdee6dd0c12bb63591ce7c9321fe77b509ab1265123054e774adc38a187746dddafe1627cbe89e990bcc78b45e194bfef8dc782710d5b217e2e2106ab0158827
2022-03-23refactor: use Span in random.*pasta
2022-03-23Merge bitcoin/bitcoin#24462: For descriptor pubkey parse errors, include ↵MarcoFalke
context information 9b526727000509dc6ef90f2ce6a9049edebf959c For descriptor pubkey parse errors, include context information (Ben Woosley) Pull request description: This adds readily-available context information to the error string, for further disambiguation. This is a revival of #16123 which was largely addressed in #16542. Note 'Multi:' is used rather than 'multi():' as it also encompasses 'sortedmulti():' ACKs for top commit: achow101: ACK 9b526727000509dc6ef90f2ce6a9049edebf959c theStack: ACK 9b526727000509dc6ef90f2ce6a9049edebf959c Tree-SHA512: 96533ea8c3ac7010f9b62e75b4bd20b65aff843030eb91c7a88312975acecaaf17909b7d1841f45edc86dbf7fa402d208adb85f0673bd79b857dbebacb8c9395
2022-03-21build: Fix Boost.Process test for Boost 1.78Hennadii Stepanov
2022-03-21test: Limit scope of id global which is shared between subtestsMarcoFalke
This is needed to use ASSERT_DEBUG_LOG, which may include a fixed node number
2022-03-18scripted-diff: rename TxRelay membersJohn Newbery
-BEGIN VERIFY SCRIPT- ren() { sed -i "s:\<$1\>:$2:g" $(git grep -l "\<$1\>" ./src ./test); } ren cs_filter m_bloom_filter_mutex ren fRelayTxes m_relay_txs ren pfilter m_bloom_filter ren cs_tx_inventory m_tx_inventory_mutex ren filterInventoryKnown m_tx_inventory_known_filter ren setInventoryTxToSend m_tx_inventory_to_send ren fSendMempool m_send_mempool ren nNextInvSend m_next_inv_send_time ren minFeeFilter m_fee_filter_received ren lastSentFeeFilter m_fee_filter_sent -END VERIFY SCRIPT-
2022-03-18[net processing] Move tx relay data to PeerJohn Newbery
2022-03-17fuzz: add a fuzz target for Miniscript decoding from ScriptAntoine Poinsot
2022-03-17Miniscript: ops limit and stack size computationPieter Wuille
Co-Authored-By: Antoine Poinsot <darosior@protonmail.com>
2022-03-17Miniscript: conversion from scriptPieter Wuille
Co-Authored-By: Antoine Poinsot <darosior@protonmail.com> Co-Authored-By: Samuel Dobson <dobsonsa68@gmail.com>
2022-03-17Miniscript: type system, script creation, text notation, testsPieter Wuille
More information about Miniscript can be found at https://bitcoin.sipa.be/miniscript/ (the website source is hosted at https://github.com/sipa/miniscript/). This commit defines all fragments, their composition, parsing from string representation and conversion to Script. Co-Authored-By: Antoine Poinsot <darosior@protonmail.com> Co-Authored-By: Sanket Kanjalkar <sanket1729@gmail.com> Co-Authored-By: Samuel Dobson <dobsonsa68@gmail.com>
2022-03-17Merge bitcoin/bitcoin#24472: fuzz: execute each file in dir without fuzz engineMarcoFalke
f59bee3fb242c9e02781a35272cf9644f37e7fc1 fuzz: execute each file in dir without fuzz engine (Anthony Towns) Pull request description: Phony fuzzing (phuzzing)! Run the fuzz testing code against known inputs to detect errors. Advantage is you can easily test using the existing qa-assets datasets without having to compile with fuzzing enabled; disadvantage is that it doesn't do any actual fuzzing. Example usage: ``` $ for a in ${QA_ASSETS}/fuzz_seed_corpus/*; do echo ${a##*/}; done | xargs -P8 -I {} /bin/sh -c "FUZZ={} test/fuzz/fuzz ${QA_ASSETS}/fuzz_seed_corpus/{}" No fuzzer for address_deserialize. No fuzzer for addrdb. No fuzzer for banentry_deserialize. addition_overflow: succeeded against 848 files in 0s. asmap: succeeded against 981 files in 0s. checkqueue: succeeded against 211 files in 0s. ... ``` (`-P8` says run 8 of the tasks in parallel) If there are failures, the first one will be reported and the program will abort with output like: ``` fuzz: test/fuzz/versionbits.cpp:336: void (anonymous namespace)::versionbits_fuzz_target(FuzzBufferType): Assertion `exp_state != ThresholdState::FAILED' failed. Error processing seed "corpus/versionbits/35345ae8e722234095810b1117a29b63af7621af" ``` Rebase of #22763, which was a rebase of #21496, but also reports the name of the fuzzer and the time taken. Fixes #21461 Top commit has no ACKs. Tree-SHA512: d8d046d4a309652eb13de42116276bf992480bc887ad3535a8ff18b354cb24826bc562b06af63802ec945c637f046563b6a5601d6321b46a5543127daafea09b
2022-03-17fuzz: execute each file in dir without fuzz engineAnthony Towns
Co-Authored-By: Anthony Ronning <anthonyronning@gmail.com>
2022-03-16Merge bitcoin/bitcoin#14752: tests: Unit tests for IsPayToWitnessScriptHash ↵MarcoFalke
and IsWitnessProgram bce9aaf31e2b0428e686e151324f8561ad71f11f Unit tests for IsWitnessProgram and IsP2WSH. (Daniel Kraft) Pull request description: This adds basic unit tests for `CScript::IsPayToWitnessScriptHash` and `CScript::IsWitnessProgram`, similar to the existing tests for `CScript::IsPayToScriptHash`. These tests are probably not super important given the other existing tests for segwit related code, but may be useful in catching some errors early. This implements #14737. ACKs for top commit: aureleoules: tACK bce9aaf31e2b0428e686e151324f8561ad71f11f (`make check)`. Tree-SHA512: 3cff5efc4ac53079289c72bfba8b1937bc103baadd32bb1fba41e78017f65f9cca17678c3202ad0711eae42b351d4132d9ed9b4e2dc07d138298691a09c4e822
2022-03-14[unit test] prioritisation in miningglozow
2022-03-14MOVEONLY: group miner tests into MinerTestingSetup functionsglozow
No behavior changes. Recommend using --color-moved=dimmed_zebra.
2022-03-14Merge bitcoin/bitcoin#24080: policy: Remove unused locktime flagsMarcoFalke
fa8d4d9128c35de0fe715f2e2b99269d23c09cc1 scripted-diff: Clarify CheckFinalTxAtTip name (MarcoFalke) fa4e30b0f36f2e7a09db7d30dca9008ed9dbcb35 policy: Remove unused locktime flags (MarcoFalke) Pull request description: The locktime flags have many issues: * They are passed in by a default argument, which is fragile. It has already lead to bugs like the one fixed in commit e30b6ea194fee3bb95a45e7b732a99566b88f1f5. * They are negative (signed), which doesn't make sense for flags (unsigned in general). According to the review comments when the code was added: "The max on the flags is a fairly weird operation." (https://github.com/bitcoin/bitcoin/pull/6566#issuecomment-150310861) * No call site relies on the default argument and they all pass in a single compile-time constant, rendering most of the code dead and untested. * The dead code calls `GetAdjustedTime` (network adjusted time), which has its own issues. See https://github.com/bitcoin/bitcoin/issues/4521 Fix all issues by removing them ACKs for top commit: ajtowns: ACK fa8d4d9128c35de0fe715f2e2b99269d23c09cc1 theStack: Code-review ACK fa8d4d9128c35de0fe715f2e2b99269d23c09cc1 glozow: ACK fa8d4d9128c35de0fe715f2e2b99269d23c09cc1, agree the default arg `flags` is a massive footgun and just setting max flags is weird. Adding `AtTip` to the names makes sense to me, since they're both testing for *next* block and only ever used for {,re}addition to mempool. Tree-SHA512: 79f4a52f34909eb598d88bbae7afe8abe5f85f45c128483d16aa83dacd0e5579e561b725d01b1e9a931d1821012a51ad2bc6fb2867f8d09ee541f9d234d696f8
2022-03-10Merge bitcoin/bitcoin#24469: test: Correctly decode UTF-8 literal string pathsMarcoFalke
2f5fd3cf9225aed439d1de767312bb340972d665 test: Correctly decode UTF-8 literal string paths (Ryan Ofsky) Pull request description: Call `fs::u8path()` to convert some UTF-8 string literals to paths, instead of relying on the implicit conversion. Fake Macro pointed out in https://github.com/bitcoin/bitcoin/pull/24306#discussion_r818566106 that `fs_tests` are incorrectly decoding some literal UTF-8 paths using the current windows codepage, instead of treating them as UTF-8. This could cause test failures depending what environment windows tests are run under. The `fs::path` class exists to avoid problems like this, but because it is lenient with `const char*` conversions, under assumption that they are ["safe as long as the literals are ASCII"](https://github.com/bitcoin/bitcoin/blob/727b0cb59259ac63c627b09b503faada1a89bfb8/src/fs.h#L39), bugs like this are still possible. If we think this is a concern, followup options to try to prevent this bug in the future are: 0. Do nothing 1. Improve the "safe as long as the literals are ASCII" comment. Make it clear that non-ASCII strings are invalid. 2. Drop the implicit `const char*` conversion functions. This would be nice because it would simplifify the `fs::path` class a little, while making it safer. Drawback is that it would require some more verbosity from callers. For example, instead of `GetDataDirNet() / "mempool.dat"` they would have to write `GetDataDirNet() / fs::u8path("mempool.dat")` 3. Keep the implicit `const char*` conversion functions, but make them call `fs::u8path()` internally. Change the "safe as long as the literals are *ASCII*" comment to "safe as long as the literals are *UTF-8*". I'd be happy with 0, 1, or 2. I'd be a little resistant to 3 even though it was would add more safety, because it would slightly increase complexity, and because I think it would encourage representing paths as strings, when I think there are so many footguns associated with paths as strings, that it's best to convert strings to paths at the earliest point possible, and convert paths to strings at the latest point possible. ACKs for top commit: laanwj: Code review ACK 2f5fd3cf9225aed439d1de767312bb340972d665 w0xlt: crACK 2f5fd3c Tree-SHA512: 9c56714744592094d873b79843b526d20f31ed05eff957d698368d66025764eae8bfd5305d5f7b6cc38803f0d85fa5552003e5c6cacf1e076ea6d313bcbc960c
2022-03-10Add GetQueryParameter helper functionstickies-v
Easily get the query parameter from the URI, with optional default value.
2022-03-10Handle query string when parsing data formatstickies-v
URLs may contain a query string (prefixed with '?') and this should be ignored when parsing the data format. To facilitate testing this functionality, ParseDataFormat has been made non-static.
2022-03-10Merge bitcoin/bitcoin#24371: util: Fix `ReadBinaryFile` reading beyond maxsizeMarcoFalke
a84650ebd5ac2cbb49f14eb7c98736a3f8215bf1 util: Fix ReadBinaryFile reading beyond maxsize (klementtan) Pull request description: Currently `ReadBinaryFile` will read beyond `maxsize` if `maxsize` is not a multiple of `128` (size of buffer) This is due to `fread` being called with `count = 128` instead of `count = min(128, maxsize - retval.size()` at every iteration The following unit test will fail: ```cpp BOOST_AUTO_TEST_CASE(util_ReadWriteFile) { fs::path tmpfolder = m_args.GetDataDirBase(); fs::path tmpfile = tmpfolder / "read_binary.dat"; std::string expected_text(300,'c'); { std::ofstream file{tmpfile}; file << expected_text; } { // read half the contents in file auto [valid, text] = ReadBinaryFile(tmpfile, expected_text.size() / 2); BOOST_CHECK_EQUAL(text.size(), 150); } } ``` Error: ``` test/util_tests.cpp:2593: error: in "util_tests/util_ReadWriteFile": check text.size() == 150 has failed [256 != 150] ``` ACKs for top commit: laanwj: Code review ACK a84650ebd5ac2cbb49f14eb7c98736a3f8215bf1 theStack: Code-review ACK a84650ebd5ac2cbb49f14eb7c98736a3f8215bf1 Tree-SHA512: 752eebe58bc2102dec199b6775f8c3304d899f0ce36d6a022a58e27b076ba945ccd572858b19137b769effd8c6de73a9277f641be24dfb17657fb7173ea0eda0
2022-03-09Merge bitcoin/bitcoin#24498: qt: Avoid crash on startup if int specified in ↵Andrew Chow
settings.json 5b1aae12ca4a99c6b09349981a4902717a6a5d3e qt: Avoid crash on startup if int specified in settings.json (Ryan Ofsky) 84b0973e35dae63cd1b60199b481e24d54e58c97 test: Add tests for GetArg methods / settings.json type coercion (Ryan Ofsky) Pull request description: Should probably add this change to 23.x as suggested by Luke https://github.com/bitcoin/bitcoin/issues/24457#issuecomment-1059825678. If settings like `prune` are added to `settings.json` in the future, it would be preferable for 23.x releases to respect the setting instead of crash. --- Fix GUI startup crash reported by Rspigler in https://github.com/bitcoin/bitcoin/issues/24457 that happens if `settings.json` contains an integer value for any of the configuration options which GUI settings can currently clash with (-dbcache, -par, -spendzeroconfchange, -signer, -upnp, -natpmp, -listen, -server, -proxy, -proxy, -onion, -onion, -lang, and -prune). The fix is a one-line change in `ArgsManager::GetArg`. The rest of the PR just adds a regression test for the GUI and unit tests for ArgsManager::GetArg methods. ACKs for top commit: laanwj: Code review ACK 5b1aae12ca4a99c6b09349981a4902717a6a5d3e achow101: ACK 5b1aae12ca4a99c6b09349981a4902717a6a5d3e jonatack: Code review ACK 5b1aae12ca4a99c6b09349981a4902717a6a5d3e Tree-SHA512: 958991b4bead9b82a3879fdca0f8d6405e2a212b7c46cf356f078843a4f156e27fd75fc46e2013aa5159582ead06d343c1ed248d678b3e5bbd312f247e37894c
2022-03-09Merge bitcoin/bitcoin#24138: index: Commit MuHash and best block together ↵MarcoFalke
for coinstatsindex 691d45fdc83ec14f87a400f548553168ac70263f Add coinstatsindex_unclean_shutdown test (Ryan Ofsky) eb6cc05da32c5bde122725a0bc907d3767a791cd index: Commit DB_MUHASH and DB_BEST_BLOCK to disk together (Martin Zumsande) Pull request description: Fixes #24076 Coinstatsindex currently writes the MuHash (`DB_MUHASH`) to disk in `CoinStatsIndex::WriteBlock()` and `CoinStatsIndex::ReverseBlock()`, but the best synced block is written in `BaseIndex::Commit()`. These are called at different points in time, both during the ThreadSync phase, and also after the initial sync is finished and validation callbacks (`BlockConnected()` vs `ChainStateFlushed()`) perform the syncing. As a result, the index DB is temporarily in an inconsistent state, and if bitcoind is terminated uncleanly (so that there is no time to call `Commit()` by receiving an interrupt or by flushing the chainstate) this leads to problems: On the next startup, `Init()` will read the best block and a MuHash that corresponds to a different (higher) block. Indexing will be picked up at the the best block processing some blocks again, but since MuHash is a rolling hash, it will process some utxos twice and the muhashes for all future blocks will be wrong, as was observed in #24076. Fix this by always committing `DB_MUHASH` together with `DB_BEST_BLOCK`. Note that the block data for the index is still written at different times, but this does not corrupt the index - at worst, these entries will be processed another time and overwritten after an unclean shutdown and restart. ACKs for top commit: ryanofsky: Code review ACK 691d45fdc83ec14f87a400f548553168ac70263f. Only change since last review is adding test fjahr: ACK 691d45fdc83ec14f87a400f548553168ac70263f Tree-SHA512: e1c3b5f06fa4baacd1b070abb0f8111fe2ea4a001ca8b8bf892e96597cf8b5d5ea10fa8fb837cfbf46648f052c742d912add4ce26d4406294fc5fc20809a0e1b
2022-03-07qt: Avoid crash on startup if int specified in settings.jsonRyan Ofsky
Fix GUI startup crash reported by Rspigler in https://github.com/bitcoin/bitcoin/issues/24457 that happens if settings.json contains an integer value for any of the configuration options which GUI settings can currently clash with (-dbcache, -par, -spendzeroconfchange, -signer, -upnp, -natpmp, -listen, -server, -proxy, -proxy, -onion, -onion, -lang, and -prune). Fix is a one-line change in ArgsManager::GetArg.
2022-03-07test: Add tests for GetArg methods / settings.json type coercionRyan Ofsky
Just add tests. No changes to application behavior. Tests will be updated in the next commit changing & improving current behavior. Include a Qt test for GUI startup crash reported by Rspigler in https://github.com/bitcoin/bitcoin/issues/24457 caused by GetArg behavior that happens if settings.json contains an integer value for any of the configuration options which GUI settings can currently clash with (-dbcache, -par, -spendzeroconfchange, -signer, -upnp, -natpmp, -listen, -server, -proxy, -proxy, -onion, -onion, -lang, and -prune).
2022-03-07Merge bitcoin/bitcoin#24050: validation: Give `m_block_index` ownership of ↵MarcoFalke
`CBlockIndex`s 6c23c415613d8b847e6f6a2f872be893da9f4384 refactor: Rewrite AddToBlockIndex with try_emplace (Carl Dong) c05cf7aa1e1c15089753897a10c14762027d4b99 style: Modernize range-based loops over m_block_index (Carl Dong) c2a1655799c5d5dab9b14bd2a6b2d2296efd6964 style-only: Use using instead of typedef for BlockMap (Carl Dong) dd79dad17545424d145e846026518d70da594380 refactor: Rewrite InsertBlockIndex with try_emplace (Carl Dong) 531dce034718523967808a89c18ba69a1e3e5a1f tests: Remove now-unnecessary manual Unload's (Carl Dong) bec86ae32683ac56b4e6ba9c9b7d21cfbdf4ac03 blockstorage: Make m_block_index own CBlockIndex's (Carl Dong) Pull request description: Part of: #24303 Split off from: #22564 ``` Instead of having CBlockIndex's live on the heap, which requires manual memory management, have them be owned by m_block_index. This means that they will live and die with BlockManager. ``` The second commit demonstrates how this makes calls to `Unload()` to satisfy the address sanitizer unnecessary. ACKs for top commit: ajtowns: ACK 6c23c415613d8b847e6f6a2f872be893da9f4384 MarcoFalke: re-ACK 6c23c415613d8b847e6f6a2f872be893da9f4384 🎨 Tree-SHA512: 81b2b5119be27cc0f8a9457b11da60cc60930315d2a5be36be89fe253d32073ffe622348ff153114b9b3212197bddbc791810913a43811b33cc58e7162bd105b
2022-03-07Merge bitcoin/bitcoin#24299: validation, refactor: UnloadBlockIndex and ↵laanwj
ChainstateManager::Reset thread safety cleanups ae9ceed3e23288b163b7d7b1840b06b8d332f4ce validation, refactoring: remove ChainstateManager::Reset() (Jon Atack) daad0093e3d1466789d0ce687902636c80cd74a1 validation: replace lock with annotation in UnloadBlockIndex() (Jon Atack) Pull request description: Thread safety refactoring seen in #24177: - replace re-acquiring lock cs_main with a thread safety annotation in UnloadBlockIndex() - remove ChainstateManager::Reset(), as it is currently unused (can be reintroduced in the test utilities if needed for unit testing) ACKs for top commit: laanwj: Code review ACK ae9ceed3e23288b163b7d7b1840b06b8d332f4ce vasild: ACK ae9ceed3e23288b163b7d7b1840b06b8d332f4ce klementtan: crACK ae9ceed3e23288b163b7d7b1840b06b8d332f4ce Tree-SHA512: cebb782572997cc2dda01590d6bb6c5e479e8202324d8b6ff459b814ce09e818b996c881736bfebd1b8bf4b6d7a0f79faf3ffea176a4699dd7d7429de2db2d13
2022-03-07Merge bitcoin/bitcoin#24306: util: Make ArgsManager::GetPathArg more widely ↵MarcoFalke
usable 60aa179d8f9a675efa2d78eaadc09e3ba450f50f Use GetPathArg where possible (Pavol Rusnak) 5b946edd73640c6ecdfb4cbac1d4351e634678dc util, refactor: Use GetPathArg to read "-settings" value (Ryan Ofsky) 687e655ae2970f2f13aca0267c7de86dc69be763 util: Add GetPathArg default path argument (Ryan Ofsky) Pull request description: Improve `ArgsManager::GetPathArg` method added in recent PR #24265, so it is usable more places. This PR starts to use it for the `-settings` option. This can also be helpful for #24274 which is parsing more path options. - Add `GetPathArg` default argument so it is less awkward to use to parse options that have default values. - Fix `GetPathArg` negated argument handling. Return path{} not path{"0"} when path argument is negated. - Add unit tests for default and negated cases - Move `GetPathArg` method declaration next to `GetArg` declaration. The two methods are close substitutes for each, so this should help keep them consistent and make them more discoverable. ACKs for top commit: w0xlt: Tested ACK 60aa179 on Ubuntu 21.10 hebasto: re-ACK 60aa179d8f9a675efa2d78eaadc09e3ba450f50f Tree-SHA512: 3d24b885d8bbeef39ea5d0556e2f09b9e5f4a21179cef11cbbbc1b84da29c8fb66ba698889054ce28d80bc25926687654c8532ed46054bf5b2dd1837866bd1cd
2022-03-04Merge bitcoin/bitcoin#24441: fuzz: Limit script_format to 100kBfanquake
bbbbeaf9c87030eb6b033b6a22002ca8d6635d51 fuzz: Limit script_format to 100kB (MarcoFalke) Pull request description: The target is still one of the slowest ones, but doesn't seem incredibly important. Especially for sizes larger than the standard tx size. Fix that by limiting the script size. ACKs for top commit: fanquake: ACK bbbbeaf9c87030eb6b033b6a22002ca8d6635d51 Tree-SHA512: b6cf7248753909ef2f21d8824f187e7c05732dd3b99619c0067f862f3c2b0f9a87779d4ddbbd3a7a4bae5c794280e2f0a223bf835d6bc6ccaba01817d69479a2
2022-03-03test: Correctly decode UTF-8 literal string pathsRyan Ofsky
Call fs::u8path to convert some UTF-8 string literals to paths, instead of relying on implicit conversions. The implicit conversions incorrectly decode const char* paths using the current windows codepage, instead of treating them as UTF-8. This could cause test failures depending what environment windows tests are run in. Issue was reported by MarcoFalke <falke.marco@gmail.com> in https://github.com/bitcoin/bitcoin/pull/24306#discussion_r818566106
2022-03-03For descriptor pubkey parse errors, include context informationBen Woosley
Note 'Multi:' is used rather than 'multi():' as it also encompasses 'sortedmulti():'
2022-03-02net: fix GetListenPort() to derive the proper portVasil Dimov
`GetListenPort()` uses a simple logic: "if `-port=P` is given, then we must be listening on `P`, otherwise we must be listening on `8333`". This is however not true if `-bind=` has been provided with `:port` part or if `-whitebind=` has been provided. Thus, extend `GetListenPort()` to return the port from `-bind=` or `-whitebind=`, if any. Fixes https://github.com/bitcoin/bitcoin/issues/20184 (cases 1. 2. 3. 5.)
2022-03-02timedata: make it possible to reset the stateVasil Dimov
Add a new function `TestOnlyResetTimeData()` which would reset the internal state used by `GetTimeOffset()`, `GetAdjustedTime()` and `AddTimeData()`. This is needed so that unit tests that call `AddTimeData()` can restore the state in order not to confuse other tests that rely on it. Currently `timedata_tests/addtimedata` is the only test that modifies the state (via `AddTimeData()`) and also the only test that relies on that state.
2022-03-02Merge bitcoin/bitcoin#24375: Do not use `LocalTestingSetup` in getarg_tests ↵MarcoFalke
test file. 5d7f22595ff2de9b9883e468e3ce7182fc3f183b Do not use `LocalTestingSetup` in getarg_tests test file. (Kiminuo) Pull request description: Avoid using a test fixture in getarg_tests for better readability. Change was implemented by _kiminuo_ and posted https://github.com/bitcoin/bitcoin/pull/24306#issuecomment-1036643216 ACKs for top commit: kiminuo: ACK 5d7f22595ff2de9b9883e468e3ce7182fc3f183b Tree-SHA512: 0fd98622010e6923e91c66447a1d0861bf344a65d86a313dff7d428c089b1740a25f699327f6ed4c163255f270bcbd4f7be962bb551862214f9b9e395d40df04
2022-03-02util: Add GetPathArg default path argumentRyan Ofsky
Let GetPathArg method be used more places for path arguments that have default values, like "-settings" and BITCOIN_SETTINGS_FILENAME in the next commit. Also: - Fix negated argument handling. Return path{} not path{"0"} when path argument is negated. - Add new tests for default and negated cases - Move GetPathArg() method declaration next to GetArg() declarations. The two methods are close substitutes for each other, so this should help keep them consistent and make them more discoverable.
2022-03-02Merge bitcoin/bitcoin#24165: p2p: extend inbound eviction protection by ↵laanwj
network to CJDNS peers b7be28cac50046b9f2ddfe63ecafccc80649a36c test: add combined CJDNS/I2P/localhost/onion eviction protection tests (Jon Atack) 0a1bb84770b403ab5cbd9d5474c76f91ce58e8f6 test: add tests for inbound eviction protection of CJDNS peers (Jon Atack) 0c00c0c981fc0b6cec101e68e8c1aeda1ccf33bb test: fix off-by-one logic in an eviction protection test (Jon Atack) f7b8094d611531c6b41a94715dbc01f56257ccd2 p2p: extend inbound eviction protection by network to CJDNS peers (Jon Atack) Pull request description: Extend inbound eviction protection for peers connected over CJDNS, as is the case for peers connected via onion, localhost, and I2P since #21261 and #20197. CJDNS peers seem to have better min ping latency than onion and I2P peers but still higher than that of unencrypted IPv4/6 peers and can be disadvantaged under our eviction criteria. They are also very few in number, which is a further reason to protect them, as the goal of this logic is to favorise the diversity of our peer connections. CJDNS support was added in #23077 for the upcoming v23 release. ACKs for top commit: laanwj: Concept and code review ACK b7be28cac50046b9f2ddfe63ecafccc80649a36c w0xlt: tACK b7be28c Tree-SHA512: 89ebdd217602e16ae14b9bd0d5a25fc09f9b2384c951f820bc0f5a6d8452bbc9042065db817d5d5296c0ad22988491a83fc5b9a611e660c40ebd4f03448c4061
2022-03-02Merge bitcoin/bitcoin#23542: net: open p2p connections to nodes that listen ↵laanwj
on non-default ports 36ee76d1afbb278500fc8aa01606ec933b52c17d net: remove unused CNetAddr::GetHash() (Vasil Dimov) d0abce9a50dd4f507e3a30348eabffb7552471d5 net: include the port when deciding a relay destination (Vasil Dimov) 2e38a0e6865187d1f0d0f016d3df7cce414a7c4f net: add CServiceHash constructor so the caller can provide the salts (Vasil Dimov) 97208634b96f2d9a55f2ead7b0ef407da729d7bd net: open p2p connections to nodes that listen on non-default ports (Vasil Dimov) Pull request description: By default, for mainnet, the p2p listening port is 8333. Bitcoin Core has a strong preference for only connecting to nodes that listen on that port. Remove that preference because connections over clearnet that involve port 8333 make it easy to detect, analyze, block or divert Bitcoin p2p traffic before the connection is even established (at TCP SYN time). For further justification see the OP of: https://github.com/bitcoin/bitcoin/pull/23306 ACKs for top commit: laanwj: Concept and light code review ACK 36ee76d1afbb278500fc8aa01606ec933b52c17d prayank23: ACK https://github.com/bitcoin/bitcoin/pull/23542/commits/36ee76d1afbb278500fc8aa01606ec933b52c17d stickies-v: tACK 36ee76d1a jonatack: ACK 36ee76d1afbb278500fc8aa01606ec933b52c17d glozow: utACK 36ee76d1afbb278500fc8aa01606ec933b52c17d Tree-SHA512: 7f45ab7567c51c19fc50fabbaf84f0cc8883a8eef84272b76435c014c31d89144271d70dd387212cc1114213165d76b4d20a5ddb8dbc958fe7e74e6ddbd56d11
2022-03-01test: add CJDNS to LimitedAndReachable_Network unit testsJon Atack
2022-02-27fuzz: FuzzedFileProvider::write should not return negative valueeugene
Doing so can lead to a glibc crash. Also the manpage for fopencookie warns against this: https://man7.org/linux/man-pages/man3/fopencookie.3.html
2022-02-25Add coinstatsindex_unclean_shutdown testRyan Ofsky
2022-02-25fuzz: Limit script_format to 100kBMarcoFalke
2022-02-23Merge bitcoin/bitcoin#24381: test: Run symlink regression tests on Windowslaanwj
fad7ddf9e3710405d727f61d8200d5efed1e705b test: Run symlink regression tests on Windows (MarcoFalke) Pull request description: Seems odd to add tests, but not run them on the platform that needs them most. ACKs for top commit: laanwj: Code review ACK fad7ddf9e3710405d727f61d8200d5efed1e705b ryanofsky: Code review ACK fad7ddf9e3710405d727f61d8200d5efed1e705b, just removing new test. Would be nice if the test could be added later, of course. Tree-SHA512: 64b235967a38c2eb90657e8d7a0447bcc8ce81d1b75a275b6c48bd42efd9ea7e7939257e484f297ee84598def3738eaeb289561aeba1dd6a99b258d389995139
2022-02-23Merge bitcoin/bitcoin#24406: test: Fix Wambiguous-reversed-operator compiler ↵fanquake
warnings fafc4eb3637be0a85644c89c355fe68678a62c17 test: Fix Wambiguous-reversed-operator compiler warnings (MarcoFalke) Pull request description: Add a missing const to avoid the C++20 clang **compiler warning**: ``` test/fuzz/addrman.cpp:325:22: error: ISO C++20 considers use of overloaded operator '==' (with operand types 'AddrManDeterministic' and 'AddrManDeterministic') to be ambiguous despite there being a unique best viable function [-Werror,-Wambiguous-reversed-operator] assert(addr_man1 == addr_man2); ~~~~~~~~~ ^ ~~~~~~~~~ /usr/include/assert.h:93:27: note: expanded from macro 'assert' (static_cast <bool> (expr) \ ^~~~ test/fuzz/addrman.cpp:140:10: note: ambiguity is between a regular call to this operator and a call with the argument order reversed bool operator==(const AddrManDeterministic& other) ^ 1 error generated. ``` This patch also fixes the **compile error** if the first operand is `const`: ``` test/fuzz/addrman.cpp:326:23: error: invalid operands to binary expression ('const AddrManDeterministic' and 'AddrManDeterministic') assert(addr_man_1 == addr_man2); ~~~~~~~~~~ ^ ~~~~~~~~~ /usr/include/assert.h:90:27: note: expanded from macro 'assert' (static_cast <bool> (expr) \ ^~~~ test/fuzz/addrman.cpp:140:10: note: candidate function not viable: 'this' argument has type 'const AddrManDeterministic', but method is not marked const bool operator==(const AddrManDeterministic& other) ^ 1 error generated. ACKs for top commit: hebasto: ACK fafc4eb3637be0a85644c89c355fe68678a62c17, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 92cd62ae06ee1393a6dc2ea6f3f553595a8f8d66f51592d231b42122bfb71ed4801a016daafc85360040339c5ae59b76888265cec37449c4688d6c7768f4567e
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-16 11:17:18 +0000