aboutsummaryrefslogtreecommitdiff
path: root/test
AgeCommit message (Collapse)Author
2021-11-10scripted-diff: Remove redundant sync_all and sync_blocksMarcoFalke
The sync calls are redundant after a call to generate, because generate already syncs itself. -BEGIN VERIFY SCRIPT- perl -0777 -pi -e 's/(generate[^\n]*\)[^\n]*)(\n|\s)+self.sync_(all|blocks)\([^\)]*\)\n/\1\n/g' $(git grep -l generate ./test) -END VERIFY SCRIPT-
2021-11-10test: Properly set sync_fun in NodeNetworkLimitedTestMarcoFalke
2021-11-10test: Use 4 spaces for indentationMarcoFalke
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-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-21Merge bitcoin/bitcoin#22839: log: improve addrman loggingfanquake
b65a25a84666d41a0af4ad98ffadfa4ac802d1bb log: improve addrman logging (Martin Zumsande) Pull request description: The addrman helper functions `GetNewBucket()` and `GetTriedBucket()` 1) log into the wrong category (`BCLog::NET` instead of `BCLog::ADDRMAN`) 2) log too unspecifically - especially `GetTriedBucket()` gets called from many different places (e.g. `Check_()`, `Serialize()`), it seems sufficient to me logging these when moving an address from new to tried. Running a node with `-checkaddrman=1`and net logging currently results in a lot of repetitive log entries. This PR moves these log entries to `Add_()` and `Good_()` and also adds logging for `Select_()` (allowing statistics about New/Tried success probabilities), `GetAddr_()`, `ClearNew()` and `MakeTried()`. ACKs for top commit: jnewbery: ACK b65a25a84666d41a0af4ad98ffadfa4ac802d1bb vasild: ACK b65a25a84666d41a0af4ad98ffadfa4ac802d1bb Tree-SHA512: 90ab0f64eb44b7388a198efccb613577b74989fea73194bda7de8bfbd50bdb19127cb12f5ec645c7859afdb89290614a79e255f3af0a63a58d4f21aa8fe7b696
2021-10-20wallet: Use PACKAGE_NAME to mention our softwareHennadii Stepanov
2021-10-20Make explicit the node param in init_wallet()lsilva01
2021-10-19tests: Add feature_segwit.py --descriptors to test_runner.pyAndrew Chow
2021-10-19tests: restrict feature_segwit legacy wallet import testsAndrew Chow
A portion of feature_segwit deals with the legacy wallet IsMine and import behavior. This is now hidden behind --legacy-wallet
2021-10-19tests: Use descriptors for feature_segwit multisig setupAndrew Chow
When setting up the multisig addresses in feature_segwit.py, use descriptors rather than addmultisigaddress.
2021-10-19Merge bitcoin/bitcoin#22918: rpc: Add level 3 verbosity to getblock RPC call ↵W. J. van der Laan
(#21245 modified) 5c34507ecbbdc29c086276d1c62835b461823507 core_write: Rename calculate_fee to have_undo for clarity (fyquah) 8edf6204a87057a451160d1e61e79d8be112e81f release-notes: Add release note about getblock verbosity level 3. (fyquah) 459104b2aae6eeaadfa5a7e47944f1a34780dacd rest: Add test for prevout fields in getblock (fyquah) 4330af6f72172848f5971a052a8f325ed50eb576 rpc: Add test for level 3 verbosity getblock rpc call. (fyquah) 51dbc167e98daab317baa80cf80bfda337672dab rpc: Add level 3 verbosity to getblock RPC call. (fyquah) 3cc95345ca49b87e8caca9a0e6418c63ae1e463a rpc: Replace boolean argument for tx details with enum class. (fyquah) Pull request description: Author of #21245 expressed [time issues](https://github.com/bitcoin/bitcoin/pull/21245#issuecomment-902332088) in the original PR. Given that #21245 has received a lot of review*, I have decided to open this new pull request with [modifications required to get ACK from luke-jr ](https://github.com/bitcoin/bitcoin/pull/21245#issuecomment-905150806) and a few nits of mine. ### Original PR description > Display the prevout in transaction inputs when calling getblock level 3 verbosity. This PR affects the existing `/rest/block` API by adding a `prevout` fields to tx inputs. This is mentioned in the change to the release notes. > > I added some functional tests that > > * checks that the RPC call still works when TxUndo can't be found > > * Doesn't display the "value" or "scriptPubKey" of the previous output when at a lower verbosity level > > > This "completes" the issue #18771 ### Possible improvements * https://github.com/kiminuo/bitcoin/commit/b0bf4f255f86aeaddce68889087c22f9068f4d97 - I can include even this commit to this PR if deemed useful or I can leave it for a follow-up PR. See https://github.com/bitcoin/bitcoin/pull/21245#issuecomment-894853784 for more context. ### Examples Examples of the `getblock` output with various verbose levels. Note that `000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5` contains only 2 transactions. #### Verbose level 0 ```bash ./bitcoin-cli -testnet getblock 000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5 0 ``` ##### Verbose level 1 ```bash ./bitcoin-cli -testnet getblock 000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5 1 ``` ##### Verbose level 2 ```bash ./bitcoin-cli -testnet getblock 000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5 2 ``` ##### Verbose level 3 ```bash ./bitcoin-cli -testnet getblock 000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5 3 ``` #### REST ```bash curl -H "content-type:text/plain;" http://127.0.0.1:18332/rest/block/000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5.json ``` <sub>* ... and my everyday obsessive checking of my email inbox whether the PR moves forward.</sub> Edit laanwj: Removed at symbol from message, and large example output to prevent it from all ending up in the commit message. ACKs for top commit: 0xB10C: ACK 5c34507ecbbdc29c086276d1c62835b461823507 meshcollider: utACK 5c34507ecbbdc29c086276d1c62835b461823507 theStack: ACK 5c34507ecbbdc29c086276d1c62835b461823507 👘 promag: Concept ACK 5c34507ecbbdc29c086276d1c62835b461823507 Tree-SHA512: bbff120d8fd76e617b723b102b0c606e0d8eb27f21c631d5f4cdab0892137c4bc7c65b1df144993405f942c91be47a26e80480102af55bff22621c19f518aea3
2021-10-19test: Remove unused node from mining_prioritisetransactionMarcoFalke
2021-10-19log: improve addrman loggingMartin Zumsande
2021-10-18test: Fix wallet_multisig_descriptor_psbt.pyHennadii Stepanov
2021-10-18Merge bitcoin/bitcoin#22067: Test and document a basic M-of-N multisig using ↵W. J. van der Laan
descriptor wallets and PSBTs 9de0d94508828f5fdfaf688ccda5a91d38b32c58 doc: add disclaimer highlighting shortcomings of the basic multisig example (Michael Dietz) f9479e4626f6b5126ff8cdab3a7e718c609429ef test, doc: basic M-of-N multisig minor cleanup and clarifications (Michael Dietz) e05cd0546a155afcd45c43ce730c4abecd40dfed doc: add another signing flow for multisig with descriptor wallets and PSBTs (Michael Dietz) 17dd6573008c8aca9fc0da9419225c85a4f94330 doc: M-of-N multisig using descriptor wallets and PSBTs, as well as a signing flow (Michael Dietz) 1f20501efce041d34e63ab9a11359bedf4a82cd5 test: add functional test for multisig flow with descriptor wallets and PSBTs (Michael Dietz) Pull request description: Aims to resolve issue https://github.com/bitcoin/bitcoin/issues/21278. I try to follow the steps laanwj outlined there exactly, with the exception of using `combinepsbt` instead of `joinpsbts`. I wrote a functional test to make sure it works as expected before doing the docs, and figured it would also be a good source of documentation. So I kept the test as simple as possible and didn't go crazy with edge-cases and various checks. I do have a lot more test-cases I've written that I will follow up with (either in a separate PR or another commit - lmk if you have a preference), but I want to do it in a way that doesn't bloat this test so it remains useful as a quickstart (unless that's a bad idea)? ACKs for top commit: S3RK: Code review ACK 9de0d94. Rspigler's argument convinced me that we should leave the workflow with two wallets. I assume using multisig with external signers is a popular use-case and it's important to keep compatibility. laanwj: Code and documentation review ACK 9de0d94508828f5fdfaf688ccda5a91d38b32c58 Tree-SHA512: 6c76e787c21f09d8be5eaa11f3ca3eaa4868497824050562bdfb2095c73b90f5e8987a8775119891d6bfde586e3f31ad1b13e4b67b0802e1d23ef050227a1211
2021-10-18Merge bitcoin/bitcoin#23207: test: Delete generate* calls from TestNodeMarcoFalke
fac62e6ff594f03832f5c0057f9b67c9118c21f4 test: Delete generate* calls from TestNode (MarcoFalke) fac7f6102feb1eb1c47ea8cb1c75c4f4dbf2f6b0 test: Use generate* node RPC, not wallet RPC (MarcoFalke) faac1cda6e2ca1d86b1551fc90453132f249d511 test: Use generate* from TestFramework, not TestNode (MarcoFalke) Pull request description: Deleting the methods is needed for #22567 to pave the way to make it easier to implicitly call the `sync_all` member function. Without the methods being deleted, nothing prevents developers from adding calls to it. As history showed, developers *will* add calls to it. For example, see commit eb02dbba3cd9f7294cd81e268cf85a1de7a71d02 from today or the first commit in this pull request. ACKs for top commit: stratospher: Tested ACK fac62e6. brunoerg: tACK fac62e6ff594f03832f5c0057f9b67c9118c21f4 promag: Code review ACK fac62e6ff594f03832f5c0057f9b67c9118c21f4. Tree-SHA512: 6d4dea8f95ead954acfef2e6a5d98897ce0c2d02265c5b137bb149d0265543bd51d7e8403e1945b9af75df5524ca50064fe1d2a432b25c8abc71bbb28ed6ed53
2021-10-18Merge bitcoin/bitcoin#23080: test: check abandoned tx in listsinceblockfanquake
bda620aecd690004c52e550ad7de187ce0eb655d test: check abandoned tx in listsinceblock (brunoerg) Pull request description: This PR tests if the abandoned transaction is correct in listsinceblock return (wallet_abandonconflict.py). ACKs for top commit: jonatack: ACK bda620aecd690004c52e550ad7de187ce0eb655d theStack: ACK bda620aecd690004c52e550ad7de187ce0eb655d stratospher: Tested ACK bda620a. This PR verifies whether the transaction txAB1 has been abandoned in listsinceblock and is a nice addition to the test! Tree-SHA512: e4dce344cf621de7a8b5bd8660d252419772a293080fc881f6f448b6df85c6b1c8f0df619e855a40b6393f53c836f0d7fadbd3916c20cccd3a95830b8b502991
2021-10-17test: get and decode tx with a single `gettransaction` RPC callSebastian Falbesoner
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 be replaced by: node.gettransaction(txid=txid, verbose=True)['decoded']
2021-10-16lint: enable mypy checking for missing importsjosibake
Achieve this by adding some ignore, and making data/ importable. Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
2021-10-16lint: install pyzmq (22.3.0) into linter environmentjosibake
mypy stubs were introduced in 21.0.1
2021-10-16doc: remove pointlessly duplicated linter version / install infofanquake
2021-10-15test: check that bumpfee RPC fails for txs with descendants in mempoolSebastian Falbesoner
This commit adds missing test coverage for the bumpfee RPC error "Transaction has descendants in the mempool", which is thrown if the bumped tx has descendants in the mempool and is _not_ connected to the bitcoin wallet. To achieve that, the test framework's MiniWallet is used.