aboutsummaryrefslogtreecommitdiff
path: root/test
AgeCommit message (Collapse)Author
2021-11-15Merge bitcoin/bitcoin#23515: test: Return the largest utxo in ↵MarcoFalke
MiniWallet.get_utxo fa62207737657e76ba45d5bf826fc0ccac658df6 test: Return the largest utxo in MiniWallet.get_utxo (MarcoFalke) Pull request description: This is for consistency with the `send_self_transfer` method. Also, remove the feature that the change of the last transfer can be retrieved via `get_utxo`. This can trivially and clearer be achieved by simply passing the txid of the transfer. Also, this fixes the bug in `feature_txindex_compatibility` in current master after a silent merge conflict. Fixes #23514 Top commit has no ACKs. Tree-SHA512: edd066d372aaa72b4e0fc7526f84931c8d1f6d14f53678cb7832bc8e3d211f44b90ec9c59b7d915ef24acc63a36e7d66c8d3b7598355bd490ac637ed3bcc3dff
2021-11-15Merge bitcoin/bitcoin#23462: test: Enable SC2046 and SC2086 shellcheck rulesW. J. van der Laan
fe0ff569ea6c353f88609c0f5f9b6fa80ff74f15 test: Enable SC2046 shellcheck rule (Hennadii Stepanov) 9a1ad7bc0dd8a0769738ca4dffbeb8d55438b0dc test: Enable SC2086 shellcheck rule (Hennadii Stepanov) Pull request description: Closes #20879. Replaces #22695. **Note for reviewers**. Some touched shell scripts are not being run in CI, therefore they require more thorough reviewing: - `contrib/devtools/gen-manpages.sh` - `contrib/macdeploy/detached-sig-apply.sh` - `contrib/windeploy/detached-sig-create.sh` - `src/qt/res/animation/makespinner.sh` ACKs for top commit: laanwj: Code review re-ACK fe0ff569ea6c353f88609c0f5f9b6fa80ff74f15 Tree-SHA512: 73619b9a7bcb6cf0dfc4189a753ef550d40c82a3432bb9d8d8a994310d42594576038daac7e0c2fc004d716976bb1413b9a77848ecf088b25b69ed0773b77e8e
2021-11-15test: Return the largest utxo in MiniWallet.get_utxoMarcoFalke
2021-11-15Merge bitcoin/bitcoin#23046: test: Add txindex migration testMarcoFalke
fadc4c72723d67a59d80d843c85b090ae22b463b test: Add txindex migration test (MarcoFalke) Pull request description: Test for #22626 ACKs for top commit: theStack: Tested ACK fadc4c72723d67a59d80d843c85b090ae22b463b 🌁 Tree-SHA512: fc7133ef52826bf0d4fa2ac72c3f1bed4a185ff7492396552ff2cbf6531b053238039211a710cbb949379c56875cd7715f1ed49a514dd3b3f1b46554e3d4bef5
2021-11-15Merge bitcoin/bitcoin#23153: Add an argparse abbreviated mode to --failfastMarcoFalke
2198f79e87b3a8bfcda59e51225ab4cb789ccc53 Add an argparse abbreviated mode to --failfast (katesalazar) Pull request description: Happy Hacktoberfest, Bitcoin! (this PR doesn't really pursue Hacktoberfest) ACKs for top commit: theStack: Tested ACK 2198f79e87b3a8bfcda59e51225ab4cb789ccc53 Tree-SHA512: 9b21b2044107685514dc298b52850206faed7306a714c007f500733b632f3f8e0e64e4785313a0c305e5908ccfc87ad27cda7554ea11630acfeaf658956eeab8
2021-11-15Merge bitcoin/bitcoin#23498: test: remove unnecessary block rehashing prior ↵fanquake
to solving a9872e1478af10ca335cca32abd248e87e103c02 test: remove unnecessary block rehashing prior to solving (Sebastian Falbesoner) Pull request description: Solving a block involves continously rehashing it, https://github.com/bitcoin/bitcoin/blob/c9dd5c8d6e59e27af98e99d2844d6ead8eec3162/test/functional/test_framework/messages.py#L759-L764 i.e. any extra `rehash` calls before are not necessary and can be dropped. The instances were identified by searching for all block solving calls via `git grep "solve("` in `./test/functional/`. From 95 instances of `CBlock.solve()` calls, 20 contained an unnecessary rehashing instruction before that are removed in this PR. ACKs for top commit: brunoerg: tACK a9872e1478af10ca335cca32abd248e87e103c02 rajarshimaitra: tACK https://github.com/bitcoin/bitcoin/pull/23498/commits/a9872e1478af10ca335cca32abd248e87e103c02 Tree-SHA512: 160092be717d0d250778b8ab091ebd77cc6865d2754ef150cf3b4d4ac7304d4bf3d2ebb61ec6b04a55040c8895b9fb4d28653ea4b099d56d90776c9111cf173f
2021-11-13depends, wallet: fix typosDimitris Apostolou
2021-11-13test: Enable SC2046 shellcheck ruleHennadii Stepanov
2021-11-13test: Enable SC2086 shellcheck ruleHennadii Stepanov
2021-11-12test: remove unnecessary block rehashing prior to solvingSebastian Falbesoner
Solving a block involves continously rehashing it, i.e. any extra calls to rehash it before are not necessary and can be dropped.
2021-11-12Merge bitcoin/bitcoin#23114: Add minisketch subtree and integrate into ↵fanquake
build/test 29173d6c6ca0cc3be9fa6bf2409a509ffea1a02a ubsan: add minisketch exceptions (Cory Fields) 54b5e1aeab73953c1f12ec2c041572038f6f59da Add thin Minisketch wrapper to pick best implementation (Pieter Wuille) ee9dc71c1bc16205494f2a0aebe575a3c062ff52 Add basic minisketch tests (Pieter Wuille) 0659f12b131fc5915fe7a493306af197f4fb838b Add minisketch dependency (Gleb Naumenko) 0eb7928ab8d9dcb840e4965bfa81deb752b00dfa Add MSVC build configuration for libminisketch (Pieter Wuille) 8bc166d5b179205fc56855e2b462aa273a6f8661 build: add minisketch build file and include it (Cory Fields) b2904ceb85b4d440b1f4bbd716fcb601411cc2c9 build: add configure checks for minisketch (Cory Fields) b6487dc4ef47ec9ea894eceac25f37d0b806f8aa Squashed 'src/minisketch/' content from commit 89629eb2c7 (fanquake) Pull request description: This takes over #21859, which has [recently switched](https://github.com/bitcoin/bitcoin/pull/21859#issuecomment-921899200) to my integration branch. A few more build issues came up (and have been fixed) since, and after discussing with sipa it was decided I would open a PR to shepherd any final changes through. > This adds a `src/minisketch` subtree, taken from the master branch of https://github.com/sipa/minisketch, to prepare for Erlay implementation (see #21515). It gets configured for just supporting 32-bit fields (the only ones we're interested in in the context of Erlay), and some code on top is added: > * A very basic unit test (just to make sure compilation & running works; actual correctness checking is done through minisketch's own tests). > * A wrapper in `minisketchwrapper.{cpp,h}` that runs a benchmark to determine which field implementation to use. Only changes since my last update to the branch in the previous PR have been rebasing on master and fixing an issue with a header in an introduced file. ACKs for top commit: naumenkogs: ACK 29173d6c6ca0cc3be9fa6bf2409a509ffea1a02a Tree-SHA512: 1217d3228db1dd0de12c2919314e1c3626c18a416cf6291fec99d37e34fb6eec8e28d9e9fb935f8590273b8836cbadac313a15f05b4fd9f9d3024c8ce2c80d02
2021-11-11test: Correct MyPy typing for subtest decoratorPavel Safronov
2021-11-10Merge bitcoin/bitcoin#22872: log: improve checkaddrman logging with duration ↵MarcoFalke
in milliseconds 22b44fc696dc1078c40d17e2d497c74c7b4ae750 p2p: improve checkaddrman logging with duration in milliseconds (Jon Atack) ec65bed00ee2e403e39b3c5977caf4abd31ccc87 log, timer: add LOG_TIME_MILLIS_WITH_CATEGORY_MSG_ONCE macro (Jon Atack) 325da75a5396f3161a6eade74b349105ed5722ab log, timer: allow not repeating log message on completion (Jon Atack) Pull request description: This patch: - updates the `logging/timer.h::Timer` class to allow not repeating the log message on completion - adds a `LOG_TIME_MILLIS_WITH_CATEGORY_MSG_ONCE` macro that prints the descriptive message when logging the start but not when logging the completion - updates the checkaddrman logging to log the duration, and renames the function like the `-checkaddrman` configuration option in order to prefix every log message with `CheckAddrman` instead of the longer, less pleasant, and different-from-checkaddrman `ForceCheckAddrman` (the Doxygen documentation on the function already makes clear that it is unaffected by `m_consistency_check_ratio`). before ``` 2021-09-21T18:42:50Z [opencon] Addrman checks started: new 64864, tried 1690, total 66554 2021-09-21T18:42:50Z [opencon] Addrman checks completed successfully ``` after ``` 2021-09-21T18:42:50Z [opencon] CheckAddrman: new 64864, tried 1690, total 66554 started 2021-09-21T18:42:50Z [opencon] CheckAddrman: completed (76.21ms) ``` To test, build and run bitcoind with `-debug=addrman -checkaddrman=<n>` for a value of `n` in the range of, say, 10 to 40. ACKs for top commit: laanwj: Code review ACK 22b44fc696dc1078c40d17e2d497c74c7b4ae750 Tree-SHA512: 658c0dfaaa9d07092e2418f2d05007c58cc35be6593f22b3c592ce793334a885dd92dacc46bdeddc9d37939cf11174660a094c07c0fa117fbb282953aa45a94d
2021-11-10Merge bitcoin/bitcoin#23398: rpc: add return message to savemempool RPCW. J. van der Laan
aa1a4c920495076ed5b5e3f05cb64d644bab6810 Add file validation to savemempool RPC test (lsilva01) 871e64d22f6db8f2e1b312227354aa172a0347c6 Add filename to savemempool RPC result (lsilva01) Pull request description: Currently, if the user calls the `savemempool` RPC method, there is no way to know where the file was created (unless the user knows internal implementation details). This PR adds a return message stating the file name and path where the mempool was saved and changes `mempool_persist.py` to validate this new return message. ACKs for top commit: laanwj: Code review ACK aa1a4c920495076ed5b5e3f05cb64d644bab6810 Tree-SHA512: e8b1dd0a8976e5eb15f7476c9651e492d2c621a67e0b726721fa7a2ae0ddd272ee28b87a2d0c650bd635e07fa96bdefe77bece4deb6486ef3ee9a4f83423a840
2021-11-10Merge bitcoin/bitcoin#23370: test: Add ios_base::width tsan suppressionW. J. van der Laan
96c7db9373014ce232ab01d11333650c9ddf9ee5 test: Add ios_base::width tsan suppression (Hennadii Stepanov) Pull request description: This PR: - adds tsan suppression for intermittent failures in CI ``` SUMMARY: ThreadSanitizer: data race /usr/lib/llvm-12/bin/../include/c++/v1/ios:523:12 in std::__1::ios_base::width() const ``` - fixes #23366 ACKs for top commit: laanwj: Concept and code review ACK 96c7db9373014ce232ab01d11333650c9ddf9ee5 Tree-SHA512: fcad296e8da4a6d94dcbb011c3d9b3d07f6983818be16cfff8341a035fa6abe2777ae72409c9bc83083097660408a850c1e9cd6f0ad3ea7976e4a4768f1e1858
2021-11-09Add file validation to savemempool RPC testlsilva01
2021-11-09test: MiniWallet: add P2TR support and use it per defaultSebastian Falbesoner
2021-11-09test: generate blocks to MiniWallet address in rpc_blockchain.pySebastian Falbesoner
2021-11-09test: Add txindex migration testMarcoFalke
2021-11-09Merge bitcoin/bitcoin#23300: test: Implicitly sync after generate*, unless ↵MarcoFalke
opted out facc352648e4fe1ed9b406400b6e4a9d51f30349 test: Implicitly sync after generate*, unless opted out (MarcoFalke) Pull request description: The most frequent failure in functional tests are intermittent races. Fixing such bugs is cumbersome because it involves: * Noticing the failure * Fetching and reading the log to determine the test case that failed * Adding a `self.sync_all()` where it was forgotten * Spamming out a pr and waiting for review, which is already sparse Also, writing a linter to catch those is not possible, nor is review effective in finding these bugs prior to merge. Fix all future intermittent races caused by a missing sync_block call by calling `sync_all` implicitly after each `generate*`, unless opted out. This ensures that the code is race-free (with regards to blocks) when the tests pass once, instead of our current approach where the code can never be guaranteed to be race-free. There are some scripted-diff cleanups (see https://github.com/bitcoin/bitcoin/pull/22567), but they will be submitted in a follow-up to reduce the conflicts in this pull. ACKs for top commit: lsilva01: tACK facc352 on Ubuntu 20.04 brunoerg: tACK facc352648e4fe1ed9b406400b6e4a9d51f30349 on MacOS 11.6 Tree-SHA512: 046a40a066b4a3bd28a3077bd654fa8887442dd1f0ec6fd11671865809ef02376f126eb667a1320ebd67b6e372c78c00dbf8bd25d86ed86f1d9a25363103ed97
2021-11-08Merge bitcoin/bitcoin#23077: Full CJDNS supportW. J. van der Laan
420695c1933e2b9c6e594fcd8885f1c261e435cf contrib: recognize CJDNS seeds as such (Vasil Dimov) f9c28330a0e77ed077f342e4669e855b3e6b20a1 net: take the first 4 random bits from CJDNS addresses in GetGroup() (Vasil Dimov) 29ff79c0a2a95abf50b78dd2be6ead2abeeaec9f net: relay CJDNS addresses even if we are not connected to CJDNS (Vasil Dimov) d96f8d304c872b21070245c1b6aacc8b1f5da697 net: don't skip CJDNS from GetNetworkNames() (Vasil Dimov) c2d751abbae3811adaf856b1dd1b71b33e54d315 net: take CJDNS into account in CNetAddr::GetReachabilityFrom() (Vasil Dimov) 9b43b3b257a00f777538fcc6e2550702055a1488 test: extend feature_proxy.py to test CJDNS (Vasil Dimov) 508eb258fd569cabda6fe15699f911fd627e0c56 test: remove default argument of feature_proxy.py:node_test() (Vasil Dimov) 6387f397b323b0fb4ca303fe418550f5465147c6 net: recognize CJDNS addresses as such (Vasil Dimov) e6890fcb440245c9a24ded0b7af46267453433f1 net: don't skip CJDNS from GetNetworksInfo() (Vasil Dimov) e9d90d3c11cee8ea70056f69afaa548cee898f40 net: introduce a new config option to enable CJDNS (Vasil Dimov) 78f456c57677e6a3a839426e211078ddf0b3e194 net: recognize CJDNS from ParseNetwork() (Vasil Dimov) de01e312b333b65b09c8dc72f0cea6295ab8e43f net: use -proxy for connecting to the CJDNS network (Vasil Dimov) aedd02ef2750329019d5698b14b17d67c5a563ad net: make it possible to connect to CJDNS addresses (Vasil Dimov) Pull request description: CJDNS overview ===== CJDNS is like a distributed, shared VPN with multiple entry points where every participant can reach any other participant. All participants use addresses from the `fc00::/8` network (reserved IPv6 range). Installation and configuration is done outside of applications, similarly to VPN (either in the host/OS or on the network router). Motivation ===== Even without this PR it is possible to connect two Bitcoin Core nodes through CJDNS manually by using e.g. `-addnode` in environments where CJDNS is set up. However, this PR is necessary for address relay to work properly and automatic connections to be made to CJDNS peers. I.e. to make CJDNS a first class citizen network like IPv4, IPv6, Tor and I2P. Considerations ===== An address from the `fc00::/8` network, could mean two things: 1. Part of a local network, as defined in RFC 4193. Like `10.0.0.0/8`. Bitcoin Core could be running on a machine with such address and have peers with those (e.g. in a local network), but those addresses are not relayed to other peers because they are not globally routable on the internet. 2. Part of the CJDNS network. This is like Tor or I2P - if we have connectivity to that network then we could reach such peers and we do relay them to other peers. So, Bitcoin Core needs to be able to tell which one is it when it encounters a bare `fc00::/8` address, e.g. from `-externalip=` or by looking up the machine's own addresses. Thus a new config option is introduced `-cjdnsreacable`: * `-cjdnsreacable=0`: it is assumed a `fc00::/8` address is a private IPv6 (1.) * `-cjdnsreacable=1`: it is assumed a `fc00::/8` address is a CJDNS one (2.) After setting up CJDNS outside of Bitcoin Core, a node operator only needs to enable this option. Addresses from P2P relay/gossip don't need that because they are properly tagged as IPv6 or as CJDNS. For testing ===== ``` [fc32:17ea:e415:c3bf:9808:149d:b5a2:c9aa]:8333 [fc68:7026:cb27:b014:5910:e609:dcdb:22a2]:8333 [fcb3:dc50:e1ae:7998:7dc0:7fa6:4582:8e46]:8333 [fcc7:be49:ccd1:dc91:3125:f0da:457d:8ce]:8333 [fcf2:d9e:3a25:4eef:8f84:251b:1b4d:c596]:8333 ``` ACKs for top commit: dunxen: ACK 420695c jonatack: re-ACK 420695c1933e2b9c6e594fcd8885f1c261e435cf per `git range-diff 23ae793 4fbff39 420695c` laanwj: Code review ACK 420695c1933e2b9c6e594fcd8885f1c261e435cf Tree-SHA512: 21559886271aa84671d52b120fa3fa5a50fdcf0fcb26e5b32049c56fab0d606438d19dd366a9c8ce612d3894237ae6d552ead3338b326487e3534399b88a317a
2021-11-05Merge bitcoin/bitcoin#22949: fee: Round up fee calculation to avoid a lower ↵Samuel Dobson
than expected feerate 80dc829be7f8c3914074b85bb4c125baba18cb2c tests: Calculate fees more similarly to CFeeRate::GetFee (Andrew Chow) ce2cc44afd51f3df4ee7f14ea05b8da229183923 tests: Test for assertion when feerate is rounded down (Andrew Chow) 0fbaef9676a1dcb84bcf95afd8d994831ab327b6 fees: Always round up fee calculated from a feerate (Andrew Chow) Pull request description: When calculating the fee for a feerate, it is possible that the final calculation will have fractional satoshis. Currently those are ignored via truncation which results in the absolute fee being rounded down. Rounding down is problematic because it results in a feerate that is slightly lower than the feerate represented by the `CFeeRate` object. A slightly lower feerate particularly causes issues for coin selection as it can trigger an assertion error. To avoid potentially underpaying the feerate (and the assertion), always round up the calculated fee. A test is added for the assertion, along with a comment explaining what happens. It is unlikely that a user can trigger this as it requires a very specific set of rounding errors to occur as well as the transaction not needing any change and being right on the lower bound of the exact match window. However I was able to trigger the assertion while running coin selection simulations, albeit after thousands of transactions and with some weird feerates. ACKs for top commit: ryanofsky: Code review ACK 80dc829be7f8c3914074b85bb4c125baba18cb2c promag: Tested ACK 80dc829be7f8c3914074b85bb4c125baba18cb2c. lsilva01: tACK 80dc829 meshcollider: utACK 80dc829be7f8c3914074b85bb4c125baba18cb2c Tree-SHA512: fe26684c60f236cab48ea6a4600c141ce766dbe59504ec77595dcbd7fd0b34559acc617007f4f499c9155d8fda0a336954413410ba862b19c765c0cfac79d642
2021-11-03net: don't skip CJDNS from GetNetworkNames()Vasil Dimov
2021-11-03test: extend feature_proxy.py to test CJDNSVasil Dimov
2021-11-03test: remove default argument of feature_proxy.py:node_test()Vasil Dimov
The default bool argument makes it harder to read because the last but one argument is also bool. Pass all of them as named arguments to increase readability. Another bool argument will be added to indicate whether to test CJDNS. Co-authored-by: Jon Atack <jon@atack.com>
2021-11-03net: don't skip CJDNS from GetNetworksInfo()Vasil Dimov
2021-11-03net: recognize CJDNS from ParseNetwork()Vasil Dimov
This allows to use "cjdns" as an argument to the `getnodeaddresses` RPC and to the `-onlynet=` parameter.
2021-11-01Merge bitcoin/bitcoin#23380: addrman: Fix AddrMan::Add() return semantics ↵fanquake
and logging 61ec0539b26a902a41a2602187a71f9dba3c6935 [MOVEONLY] reorder functions in addrman_impl.h and addrman.cpp (John Newbery) 2095df7b7bfcb9ab0c5710a93112f7f341e753c9 [addrman] Add Add_() inner function, fix Add() return semantics (John Newbery) 2658eb6d68460272deefb3fcc653b03f6ec6e7cf [addrman] Rename Add_() to AddSingle() (John Newbery) e58598e833d5737900fe3c4369e26f2a08166892 [addrman] Add doxygen comment to AddrMan::Add() (John Newbery) Pull request description: Previously, Add() would return true if the function created a new AddressInfo object, even if that object could not be successfully entered into the new table and was deleted. That would happen if the new table position was already taken and the existing entry could not be removed. Instead, return true if the new AddressInfo object is successfully entered into the new table. This fixes a bug in the "Added %i addresses" log, which would not always accurately log how many addresses had been added. ACKs for top commit: naumenkogs: ACK 61ec0539b26a902a41a2602187a71f9dba3c6935 mzumsande: ACK 61ec0539b26a902a41a2602187a71f9dba3c6935 shaavan: ACK 61ec0539b26a902a41a2602187a71f9dba3c6935 Tree-SHA512: 276f1e8297d4b6d411d05d06ffc7c176f6290a784da039926ab6c471a8ed8e9159ab4f56c893b1285737ae292954930f0d28012d89dfb3f2f825d7df41016feb
2021-10-29test: Implicitly sync after generate*, unless opted outMarcoFalke
2021-10-29Merge bitcoin/bitcoin#23354: Introduce new V4 format addrmanMarcoFalke
d891ae768185b464cae476c16c74c365372d4a3c Introduce new V4 format addrman (Pieter Wuille) Pull request description: #23306 effectively changed the on-disk format in an incompatible way: old deserializers cannot deal with multiple entries for the same IP. Introduce a `V4_MULTIPORT` format, and increment the compatibility base, so that old versions correctly recognize it as an incompatible future version, rather than corruption. ACKs for top commit: naumenkogs: ACK d891ae768185b464cae476c16c74c365372d4a3c ajtowns: utACK d891ae768185b464cae476c16c74c365372d4a3c vasild: ACK d891ae768185b464cae476c16c74c365372d4a3c Tree-SHA512: de2153beb59152504ee0656dd0cc0b879b09136eb07e3ce0426d2fea778adfabacebbce5cf1a9a65dc99ad4e99cda42ab26743fe672fb82a9fbfec49c4cccb4d
2021-10-29Merge bitcoin/bitcoin#23375: test: MiniWallet: more deterministic coin ↵MarcoFalke
selection for coinbase UTXOs (oldest first) d2c4904ef707e2023ceb8dfbfe61a92c4060e100 test: MiniWallet: more deterministic coin selection for coinbase UTXOs (oldest first) (Sebastian Falbesoner) Pull request description: The coin selection strategy for MiniWallet is quite straight-forward: simply pick a single UTXO with the largest value: https://github.com/bitcoin/bitcoin/blob/ab25ef8c7f767258d5fe44f53b35ad8bd51ed5cd/test/functional/test_framework/wallet.py#L173-L174 If there are several candidates with the same value, however, it is not clear which one is taken. This can be particularly problematic for coinbase outputs with fixed block subsidy, since spending could lead to a `bad-txns-premature-spend-of-coinbase` reject if an UTXO from a too-recent block is picked. Introduce block height as second criteria (saved in `self._utxos` in the methods `generate(...)` and `rescan_utxos(...)`), in order to avoid potential issues with coinbases that are not matured yet. If there is a tie between coinbase UTXOs and non-coinbase UTXOs (the latter are added via `scan_tx(...)`), prefer the non-coinbase UTXOs, since those don't need to mature. The issue came up while refactoring the test rpc_blockchain.py, see https://github.com/bitcoin/bitcoin/pull/23371#discussion_r737401936 (PR #23371). ACKs for top commit: MarcoFalke: review ACK d2c4904ef707e2023ceb8dfbfe61a92c4060e100 shaavan: ACK d2c4904ef707e2023ceb8dfbfe61a92c4060e100 Tree-SHA512: 15d67b42fb8b77fd53022ea2ab8a6ed2b615567f3ce73bab16c06bfcb687c1a04dcb0360d0c2287c526b604cd3ac5eef7b14ce46fc31e23047ce1a3290027306
2021-10-29Merge bitcoin/bitcoin#22972: test: fix misleading fee unit in mempool_limit.pyMarcoFalke
2600db6c36c11bf49a0a113ee2e2274406ade61c test: fix misleading fee unit in mempool_limit.py (Sebastian Falbesoner) Pull request description: The PR is a follow-up to #22543. The helper `send_large_txs` in its current interface has a fee_rate parameter, implying that it would create a transaction with exactly that rate. Unfortunately, this fee rate is only passed to MiniWallet's `create_self_transfer` method, which can't know that we append several tx outputs after, increasing the tx's vsize and decreasing it's fee rate accordingly. In our case, the fee rate is off by several orders of magnitude, as the tx's vsize changes changes from 96 to 67552 vbytes (>700x), i.e. the value passed to this function is neither really a fee rate nor an absolute fee, but something in-between, which is very confusing. It was suggested to simply in-line this helper as it's currently only used in this single test (https://github.com/bitcoin/bitcoin/pull/22543#discussion_r701685136, https://github.com/bitcoin/bitcoin/pull/22543#issuecomment-918986896), but I could imagine that this helper may also become useful for other tests and may be moved to a library (e.g. wallet.py) in the future. Clarify the interface by passing an absolute fee that is deducted in the end (and also verified, via testmempoolaccept) and also describe how we come up with the value passed. On master, the comment says that the fee rate needs to increased "massively"; this word is also removed because the fee rate only needs to be higher for the test to succeed. ACKs for top commit: stratospher: ACK 2600db6. Tree-SHA512: 0bfacc3fa87603970d86c1d0186e51511f6c20c64b0559e19e7e12a68647f79dcb4f436000dee718fd832ce6a68e3bbacacb29145e0287811f1cb03d2f316843
2021-10-28[addrman] Add Add_() inner function, fix Add() return semanticsJohn Newbery
Previously, Add() would return true if the function created a new AddressInfo object, even if that object could not be successfully entered into the new table and was deleted. That would happen if the new table position was already taken and the existing entry could not be removed. Instead, return true if the new AddressInfo object is successfully entered into the new table. This fixes a bug in the "Added %i addresses" log, which would not always accurately log how many addresses had been added. p2p_addrv2_relay.py and p2p_addr_relay.py need to be updated since they were incorrectly asserting on the buggy log (assuming that addresses are added to addrman, when there could in fact be new table position collisions that prevent some of those address records from being added).
2021-10-27test: MiniWallet: more deterministic coin selection for coinbase UTXOs ↵Sebastian Falbesoner
(oldest first) The coin selection strategy for MiniWallet is quite straight-forward: simply pick a single UTXO with the largest value: self._utxos = sorted(self._utxos, key=lambda k: k['value']) utxo_to_spend = utxo_to_spend or self._utxos.pop() If there are several candidates with the same value, however, it is not clear which one is taken. This can be particularly problematic for coinbase outputs with fixed block subsidy, since spending could lead to a 'bad-txns-premature-spend-of-coinbase' reject if an UTXO from a too-recent block is picked. Introduce block height as second criteria (saved in self._utxos in the methods generate(...) and rescan_utxos(...)), in order to avoid potential issues with coinbases that are not matured yet.
2021-10-27test: Add ios_base::width tsan suppressionHennadii Stepanov
2021-10-26test: add script_util helper for creating bare multisig scriptsSebastian Falbesoner
2021-10-25Introduce new V4 format addrmanPieter Wuille
92617b7a758c0425330fba4b886296730567927c effectively changed the on-disk format in an incompatible way: old deserializers cannot deal with multiple entries for the same IP. Introduce a V4_MULTIPORT format, and increment the compatibility base, so that old versions correctly recognize it as an incompatible future version.
2021-10-25Merge bitcoin/bitcoin#23312: tests: reduce feature_segwit.py usage of the ↵MarcoFalke
legacy wallet e9ade032f3283971025943d750b1305bc8da56fc tests: Add feature_segwit.py --descriptors to test_runner.py (Andrew Chow) ae6cbcc90926b65099ed8747e7a13a4aefba787a tests: restrict feature_segwit legacy wallet import tests (Andrew Chow) 1d13c44a4cb02b0acef6c6a108410cfc7eb20f71 tests: Use descriptors for feature_segwit multisig setup (Andrew Chow) Pull request description: `feature_segwit.py` has tests for some legacy wallet behavior, but otherwise does not really need the legacy wallet. Those parts now require `--legacy-wallet` to be provided (as with other legacy wallet tests). Other parts of the test are changed to work with descriptor wallets as well. ACKs for top commit: lsilva01: tACK e9ade03 Tree-SHA512: 4ae76a4d2a8d318e7f8ad4c984748e3cdd67ed00359fcd798a08492ed9399b1d01be88c9ebea3d6c996fbe190cf41708a15c98f088f0cb5c47d6d00fff258944
2021-10-25Merge bitcoin/bitcoin#23311: wallet: Use PACKAGE_NAME to mention our softwareMarcoFalke
da791c7f66243080177f92ce5f38c49305da63dc wallet: Use PACKAGE_NAME to mention our software (Hennadii Stepanov) Pull request description: This PR replaces "bitcoin" and "bitcoind" with `PACKAGE_NAME` in wallet log and error messages. ACKs for top commit: jonatack: ACK da791c7f66243080177f92ce5f38c49305da63dc lsilva01: Tested ACK da791c7 on Ubuntu 20.04. brunoerg: tACK da791c7f66243080177f92ce5f38c49305da63dc stratospher: Tested ACK da791c7 shaavan: ACK da791c7f66243080177f92ce5f38c49305da63dc Tree-SHA512: c613446d9c8c3f85e6f5fec77382c9bc17746a853c89e72e1a3a79bf355d7bd9e455bbde8f9e02a894d225a67029c732cdc68ec8c58ac8237dde27d39dae8be7
2021-10-25Merge bitcoin/bitcoin#22711: test: check for specific block reject reasons ↵MarcoFalke
in p2p_segwit.py 45827fd718d51acffdbeb3bb12d1eb96e3fa8bc0 test: check for block reject reasons in p2p_segwit.py [2/2] (Sebastian Falbesoner) 4eb532ff8b5ea9338e6cb1b927abaa43f8e3c94e test: check for block reject reasons in p2p_segwit.py [1/2] (Sebastian Falbesoner) b1488c4dcecb9dda9d7c583c1c9e1bc0852c79b2 test: fix reference to block processing test in p2p_segwit.py (Sebastian Falbesoner) Pull request description: In the test `p2p_segwit.py`, there are many instances where we send a segwit block to a node with the expectation that it is rejected. For this purpose, the helper function `test_witness_block` exists which allows also to check for a specific reject `reason` that is asserted in the debug log: https://github.com/bitcoin/bitcoin/blob/502d22ceed1f90ed41336260f8eb428d3acaf514/test/functional/p2p_segwit.py#L119-L120 This PR aims to increase the precision of the tests by adding the expected reject reasons to all `test_witness_block` call instances (found via `grep`ing after `test_witness_block(.*accepted=False`). For some blocks that are rejected due to failed script verification, the exact reason is only shown in the debug log if parallel script verification is disabled. For this reason, the addition of the reasons is split up in two commits: * first, all instances are tackled that are not subject to script verification, i.e. it doesn't matter whether parallel script verification is enabled or not (e.g. `bad-blk-weight`, `bad-witness-merkle-match`...) * then, we explicitely set `-par=1` to only use one script thread, and add the remaining reasons (`non-mandatory-script-verify-flag` with the more specific reason in parantheses) ACKs for top commit: stratospher: tested ACK 45827fd. Tree-SHA512: 00f31867f11d48b38a42b1e79a1303bda1c797ccf3d8c73e6107d70b062604d51ee2a3f2251e7f068dfafdaf09469d27dfee438d9bc9ebaef7febc4b6ef90a95
2021-10-22Merge bitcoin/bitcoin#23139: rpc: fix "trusted" field in ↵W. J. van der Laan
TransactionDescriptionString(), add coverage 66f6efc70a72cc1613906fd3c10281f9af0ba0db rpc: improve TransactionDescriptionString() "generated" help (Jon Atack) 296cfa312fd9ce19f1f820aeafa37d87764ad21d test: add listtransactions/listsinceblock "trusted" coverage (Jon Atack) d95913fc432f0fde9dec743884b14c5df83727af rpc: fix "trusted" description in TransactionDescriptionString (Jon Atack) Pull request description: The RPC gettransaction, listtransactions, and listsinceblock helps returned by `TransactionDescriptionString()` inform the user that the `trusted` boolean field is only present if the transaction is trusted and safe to spend from. The field is in fact returned by `WalletTxToJSON()` when the transaction has 0 confirmations (or negative confirmations, if conflicted), and it can be true or false. This patch fixes the help, adds test coverage, and touches up the help for the neighboring `generate` field. ACKs for top commit: rajarshimaitra: tACK https://github.com/bitcoin/bitcoin/pull/23139/commits/66f6efc70a72cc1613906fd3c10281f9af0ba0db theStack: Tested ACK 66f6efc70a72cc1613906fd3c10281f9af0ba0db Tree-SHA512: 4c2127765b82780e07bbdbf519d27163d414d9f15598e01e02210f210e6009be344c84951d7274e747b1386991d4c3b082cd25aebe885fb8cf0b92d57178f68e
2021-10-22Merge bitcoin/bitcoin#23002: Make descriptor wallets by defaultMarcoFalke
9c1052a5218e191fd23c0d9fc06f2fca34b03411 wallet: Default new wallets to descriptor wallets (Andrew Chow) f19ad404631010a5e2dac2c7cbecd057b005fe2a rpc, wallet: Descriptor wallets are no longer experimental (Andrew Chow) Pull request description: Changes the default wallet type from legacy to descriptors. Descriptor wallets will now by the default type. Additionally, descriptor wallets will no longer be marked as experimental. This follows the timeline proposed in #20160 ACKs for top commit: lsilva01: Tested ACK https://github.com/bitcoin/bitcoin/pull/23002/commits/9c1052a5218e191fd23c0d9fc06f2fca34b03411 on Ubuntu 20.04 prayank23: tACK https://github.com/bitcoin/bitcoin/pull/23002/commits/9c1052a5218e191fd23c0d9fc06f2fca34b03411 meshcollider: Code review ACK 9c1052a5218e191fd23c0d9fc06f2fca34b03411 Tree-SHA512: 834e6fec88e0c18673af7ebe135bd5333694d1be502164eb93a90e3e76c27974165aa4e59426945100c88e4eca07356e16886ef5b05cf789683ecb23fc71a12a
2021-10-21Merge bitcoin/bitcoin#23331: test: Replace log with assert_equal in ↵MarcoFalke
wallet_abandonconflict fa6c62f34b50818102ad58f2ffd152189f54d973 test: Replace log with assert_equal in wallet_abandonconflict (MarcoFalke) Pull request description: This will make the test fail as soon as the bug is fixed, forcing it to be updated. Otherwise, the bug might be fixed (or made worse) accidentally, leaving the test in an inconsistent state. ACKs for top commit: theStack: Concept and code-review ACK fa6c62f34b50818102ad58f2ffd152189f54d973 brunoerg: tACK fa6c62f34b50818102ad58f2ffd152189f54d973 Tree-SHA512: 416f72380164bf3f93332a5cfa81a8e61d8ada3714ef6815889ed3cf2d16c09411760dee4acf19629227e565b765b3dea491a0b23efd38eb370254cfadf7c441
2021-10-21Merge bitcoin/bitcoin#23218: p2p: Use mocktime for ping timeoutW. J. van der Laan
fadf1186c899f45787a91c28120b0608bdc6c246 p2p: Use mocktime for ping timeout (MarcoFalke) Pull request description: It is slightly confusing to use mocktime for some times, but not others. Start fixing that by making the ping timeout use mocktime. The only downside would be that tests that use mocktime disconnect peers after this patch. However, I don't think this is an issue, as the inactivity check is already disabled for all functional tests after commit 6d76b57ca0cdf6f9c19ce065b9a4a628930a78b5. Only one unit test needed the inactivity check disabled as part of this patch. A nice side effect of this patch is that the `p2p_ping` functional test now runs 4 seconds faster. ACKs for top commit: laanwj: Code review ACK fadf1186c899f45787a91c28120b0608bdc6c246 Tree-SHA512: e9e7b21040a89d9d574b3038f85a67e6336de6cd6e41aa286769cd03cada6e75a94ec01700e052e56d822ef85d7813cc06bf7e67b81543eff8917a16cdccf942
2021-10-21test: Replace log with assert_equal in wallet_abandonconflictMarcoFalke
This will make the test fail as soon as the bug is fixed, forcing it to be updated. Otherwise, the bug might be fixed (or made worse) accidentally, leaving the test in an inconsistent state.
2021-10-21Merge bitcoin/bitcoin#23267: test: bip125-replaceable in listsinceblockMarcoFalke
b7884dd1b68814c59ff4fb5f7a199e306b015e85 test: bip125-replaceable in listsinceblock (brunoerg) Pull request description: This PR adds test coverage for bip125-replaceable in listsinceblock. I added this test into wallet_listtransactions.py instead of putting it into wallet_listsinceblock.py to utilize the scenario already created in wallet_listtransactions.py and avoid repetition. ACKs for top commit: theStack: ACK b7884dd1b68814c59ff4fb5f7a199e306b015e85 promag: ACK b7884dd1b68814c59ff4fb5f7a199e306b015e85. stratospher: tested ACK b7884dd. Verified the bip125-replaceable status of some transactions with listsinceblock. lsilva01: tACK b7884dd on Ubuntu 20.04 Tree-SHA512: 510dfe5a6f9d68e5a656514d356dc8fe99324296ed8caa78f0eb4b6c6906cf70b1fb50bde80aa6f61d726b2fa1d4ce1fe48c635ce24285588e56ceff92291617
2021-10-21Merge bitcoin/bitcoin#23281: test: check that bumpfee RPC fails for txs with ↵MarcoFalke
descendants in mempool 4ac8c89ad96de9ad61cad756b10c9dee2d9e1405 test: check that bumpfee RPC fails for txs with descendants in mempool (Sebastian Falbesoner) Pull request description: This PR adds missing test coverage for the bumpfee RPC error _"Transaction has descendants in the mempool"_, https://github.com/bitcoin/bitcoin/blob/6419bdfeb130b20ccfed229d9ba7eca7f385d036/src/wallet/feebumper.cpp#L29-L32 which is thrown if the bumped tx has descendants in the mempool and is _not_ connected to the bitcoin wallet (for those, the error "Transaction has descendants in the Wallet" is thrown a few lines above). To achieve that, the test framework's MiniWallet is used. ACKs for top commit: brunoerg: tACK 4ac8c89ad96de9ad61cad756b10c9dee2d9e1405 promag: Code review ACK 4ac8c89ad96de9ad61cad756b10c9dee2d9e1405. Nice stuff! lsilva01: Tested ACK 4ac8c89 cad756b10c9dee2d9e1405 on Ubuntu 20.04. stratospher: tested ACK 4ac8c89. Tree-SHA512: 83e99f9dd2b140c0c0597c0c36c9c948fa334871be40e58a5e004440698d9685661c69bb83ab937d30f692545a3799705f991b31904f2ef31a2fbc3ae1179fa8
2021-10-21Merge bitcoin/bitcoin#23287: test: get and decode tx with a single ↵MarcoFalke
`gettransaction` RPC call 130ee481082d2612d452d7d69131ade935b225b5 test: get and decode tx with a single `gettransaction` RPC call (Sebastian Falbesoner) Pull request description: Rather than subsequently calling `gettransaction` and `decoderawtransaction` to get the decoded information for a specific tx-id, we can simply use the verbose version of `gettransaction`, which returns this in a 'decoded' key. I.e. ``` node.decoderawtransaction(node.gettransaction(txid)['hex']) ``` can simply be replaced by: ``` node.gettransaction(txid=txid, verbose=True)['decoded'] ``` Rationale: shorter code, shorter test logs, less RPC calls. ACKs for top commit: stratospher: tested ACK 130ee48 amadeuszpawlik: tACK 130ee481082d2612d452d7d69131ade935b225b5 lsilva01: Tested ACK 130ee48 on Ubuntu 20.04. shaavan: ACK 130ee481082d2612d452d7d69131ade935b225b5 Tree-SHA512: cf0bd26e1e21b8022fb8062857906e0706f0ee32d3277f985c461e2519405afe445ab005f5f763fb268c7b4d6e48b2d47eda7af8621b3bce67cece8dfc9bc153
2021-10-21Merge bitcoin/bitcoin#23137: Move-only: bloom to src/commonfanquake
fa2d611bedc2a755dcf84a82699c70b57b903cf6 style: Sort (MarcoFalke) fa1e5de2db2c7c95b96773a4ac231ab4249317e9 scripted-diff: Move bloom to src/common (MarcoFalke) fac303c504ab19b863fddc7a0093068fee9d4ef3 refactor: Remove unused MakeUCharSpan (MarcoFalke) Pull request description: To avoid having all files at the top level `./src` directory, start moving them to their respective sub directory according to https://github.com/bitcoin/bitcoin/issues/15732. `bloom` currently depends on libconsensus (`CTransaction`, `CScript`, ...) and it is currently located in the libcommon. Thus, move it to `src/common/`. (libutil in `src/util/` is for stuff that doesn't depend on libconsensus). ACKs for top commit: theStack: Code-review ACK fa2d611bedc2a755dcf84a82699c70b57b903cf6 ryanofsky: Code review ACK fa2d611bedc2a755dcf84a82699c70b57b903cf6 fanquake: ACK fa2d611bedc2a755dcf84a82699c70b57b903cf6 - source shuffle starts now. Tree-SHA512: d2fbc31b81741e9f0be539e1149542c9ca39958c240e12e8e757d882beccd0f0debdc10dcce146a05f03ef9f5c6247900a461a7a4799b515e8716dfb9af1fde2
2021-10-21ubsan: add minisketch exceptionsCory Fields