aboutsummaryrefslogtreecommitdiff
path: root/test/functional
AgeCommit message (Collapse)Author
2022-09-13Merge bitcoin/bitcoin#24991: init: allow startup with -onlynet=onion ↡fanquake
-listenonion=1 2d0b4e4ff66e60c85f86c526a53f8fb242ebb7d0 init: allow startup with -onlynet=onion -listenonion=1 (Vasil Dimov) Pull request description: It does not make sense to specify `-onlynet=onion` without providing a Tor proxy (even if other `-onlynet=...` are given). This is checked during startup. However, it was forgotten that a Tor proxy can also be retrieved from "Tor control" to which we connect if `-listenonion=1`. So, the full Tor proxy retrieval logic is: 1. get it from `-onion` 2. get it from `-proxy` 3. if `-listenonion=1`, then connect to "Tor control" and get the proxy from there (was forgotten before this change) Fixes https://github.com/bitcoin/bitcoin/issues/24980 ACKs for top commit: mzumsande: Tested ACK 2d0b4e4ff66e60c85f86c526a53f8fb242ebb7d0 MarcoFalke: ACK 2d0b4e4ff66e60c85f86c526a53f8fb242ebb7d0 πŸ•Έ Tree-SHA512: d1d18e07a8a40a47b7f00c31cb291a3d3a9b24eeb28c5e4720d5df4997f488583a3a010d46902b4b600d2ed1136a368e1051c133847ae165e0748b8167603dc3
2022-09-12RPC: unify arg type error messagefurszy
We were throwing two different errors for the same problematic: * "Expected type {expected], got {type}" --> RPCTypeCheckArgument() * "JSON value of type {type} is not of expected type {expected}" --> UniValue::checkType()
2022-09-10Merge bitcoin/bitcoin#26054: test: verify best blockhash after invalidating ↡MacroFake
an unknown block 4f67336f1105b7c34a9e8cdafa603edc1d899fb9 test: verify best blockhash after invalidating an unknown block (brunoerg) Pull request description: Fixes #26051 Verify the best blockhash is the same after invalidating an unknown block, not the whole `getchaintip` response. ACKs for top commit: instagibbs: ACK 4f67336f1105b7c34a9e8cdafa603edc1d899fb9 Tree-SHA512: 2d71743c1d3a317ef7b750f88437df71d1aed2728d9edac8b763a343406e168b97865ab25ec4c89caf09d002e076458376618cbd0845496375f7179633c88af9
2022-09-09test: verify best blockhash after invalidating an unknown blockbrunoerg
2022-09-09Merge bitcoin/bitcoin#25990: test: apply fixed feerate to avoid variable ↡MacroFake
dynamic fees in wallet_groups.py 2186608172b0e6c3a9907e06fed27dfd927abd45 test: apply fixed feerate to avoid variable dynamic fees (stickies-v) Pull request description: Without specifying a feerate, we let the wallet decide on an appropriate feerate, which can be influenced by various factors such as what's in the mempool. Since wallet_groups.py fails when feerates are unstable, we should use a fixed feerate across all nodes. The assumed feerate was 20 sats/vbyte, so this PR adopts that. Closes #25940. I'm not 100% sure, but I think the increased tx relay speed introduced by #25865 caused the transactions to more quickly and often enter the other nodes' mempools, affecting their feerate calculation done in [`wallet:GetMinimumFeeRate()`](https://github.com/bitcoin/bitcoin/blob/ea67232cdb80c4bc3f16fcd823f6f811fd8903e1/src/wallet/fees.cpp#L68-L72) and thus deviating slightly from the expected 20 sats/vbyte. Ran `wallet_groups.py` over 400 times without failure. ACKs for top commit: aureleoules: ACK 2186608172b0e6c3a9907e06fed27dfd927abd45. glozow: Approach ACK 2186608172b0e6c3a9907e06fed27dfd927abd45 Tree-SHA512: 0ea467a67747e6f27369ccd0adacfb21cc36ef0ae728fb28b8ea18e409aab5bd3ede559d6cebb82da0b9703c0c8b2709d686feb3ae009ddf525aa253f44d5816
2022-09-09Merge bitcoin/bitcoin#26031: test: Display skipped tests reasonMacroFake
07b6e743147f410d8ceb4b6b3282e9fba4cafe50 test: Display skipped tests reason (Aurèle Oulès) Pull request description: Attempt to fix #26023. ACKs for top commit: brunoerg: ACK 07b6e743147f410d8ceb4b6b3282e9fba4cafe50 Tree-SHA512: 5d8f7fbd8d65772000a5da8c01276948b157d93d359203c6442cf2681cdcc2426b1fee7ec62cee100019c59a486a96ad98d5e819bffe1fd37624dcd28f42aed2
2022-09-08Merge bitcoin/bitcoin#26038: test: invalidating an unknown block throws an errorMacroFake
4b1d5a10537ab48e3457606ba1cf2ae26a1cb2b2 test: invalidating an unknown block throws an error (brunoerg) Pull request description: While playing with `invalidateblock`, I unintentionally tried to invalidate an unknown block and it threw an error. Looking at the tests I just realized there is no test coverage for this case. This PR adds it. Top commit has no ACKs. Tree-SHA512: 25286ead809b3ad022e759127ef3134b271fbe76cb7b50ec2b0c7e2409da8d1b01dc5e80afe73e4564cc9c9c03487a1fe772aea3456988552d2f9c8fb34c730b
2022-09-08test: invalidating an unknown block throws an errorbrunoerg
2022-09-08test: Display skipped tests reasonAurèle Oulès
2022-09-08Merge bitcoin/bitcoin#26037: test: Fix `wallet_{basic,listsinceblock}.py` ↡MacroFake
for BDB-only wallets 9f3a315c6f6af37d562a72f7d4fb2c53b5940871 test: Fix `wallet_listsinceblock.py` for BDB-only wallets (Hennadii Stepanov) 1941ce6cd15a1123db8769d17a6fd9b1cb3debcc test: Fix `wallet_basic.py` for BDB-only wallets (Hennadii Stepanov) Pull request description: Fixes bitcoin/bitcoin#26029. ACKs for top commit: brunoerg: crACK 9f3a315c6f6af37d562a72f7d4fb2c53b5940871 Tree-SHA512: d31c76e558dedea689ff487644e9f2d2f1df1cc2bb9bb041ede4b272884871167fdb19ccc717394c6ba6af8b8c70e9575b344988e0ce55b241a3a4922d0b7f73
2022-09-07test: Fix `wallet_listsinceblock.py` for BDB-only walletsHennadii Stepanov
2022-09-07test: Fix `wallet_basic.py` for BDB-only walletsHennadii Stepanov
2022-09-07Merge bitcoin/bitcoin#25678: p2p: skip querying dns seeds if `-onlynet` ↡fanquake
disables IPv4 and IPv6 385f5a4c3feb716fcf3f2b4823535df6da6bb67b p2p: Don't query DNS seeds when both IPv4 and IPv6 are unreachable (Martin Zumsande) 91f0a7fbb79fe81a75370a4b60dcdd2e55edfa81 p2p: add only reachable addresses to addrman (Martin Zumsande) Pull request description: Currently, `-onlynet` does not work well in connection with initial peer discovery, because DNS seeds only resolve to IPv6 and IPv4 adresses: With `-onlynet=i2p`, we would load clearnet addresses from DNS seeds into addrman, be content our addrman isn't empty so we don't try to query hardcoded seeds (although these exist for i2p!), and never attempt to make an automatic outbound connection. With `-onlynet=onion` and `-proxy` set, we wouldn't load addresses via DNS, but will make AddrFetch connections (through a tor exit node) to a random clearnet peer the DNS seed resolves to (see https://github.com/bitcoin/bitcoin/issues/6808#issuecomment-147652505), thus breaching the `-onlynet` preference of the user - this has been reported in the two issues listed below. This PR proposes two changes: 1.) Don't load addresses that are unreachable (so that we wouldn't connect to them) into addrman. This is already the case for addresses received via p2p addr messages, this PR implements the same for addresses received from DNS seeds and fixed seeds. This means that in the case of `-onlynet=onion`, we wouldn't load fixed seed IPv4 addresses into addrman, only the onion ones. 2.) Skip trying the DNS seeds if neither IPv4 nor IPv6 are reachable and move directly to adding the hardcoded seeds from networks we can connect to. This is done by soft-setting `-dnsseed` to 0 in this case, unless `-dnsseed=1` was explicitly specified, in which case we abort with an `InitError`. Fixes #6808 Fixes #12344 ACKs for top commit: naumenkogs: utACK 385f5a4c3feb716fcf3f2b4823535df6da6bb67b vasild: ACK 385f5a4c3feb716fcf3f2b4823535df6da6bb67b Tree-SHA512: 33a8c29faccb2d9b937b017dba4ef72c10e05e458ccf258f1aed3893bcc37c2e984ec8de998d2ecfa54282abbf44a132e97d98bbcc24a0dcf1871566016a9b91
2022-09-06p2p: Don't query DNS seeds when both IPv4 and IPv6 are unreachableMartin Zumsande
This happens, for example, if the user specified -onlynet=onion or -onlynet=i2p. DNS seeds only resolve to IPv4 / IPv6 addresses, making their answers useless to us, since we don't want to make connections to these. If, within the DNS seed thread, we'd instead do fallback AddrFetch connections to one of the clearnet addresses the DNS seed resolves to, we might get usable addresses from other networks if lucky, but would be violating our -onlynet user preference in doing so. Therefore, in this case it is better to rely on fixed seeds for networks we want to connect to. Co-authored-by: Vasil Dimov <vd@FreeBSD.org>
2022-09-05init: allow startup with -onlynet=onion -listenonion=1Vasil Dimov
It does not make sense to specify `-onlynet=onion` without providing a Tor proxy (even if other `-onlynet=...` are given). This is checked during startup. However, it was forgotten that a Tor proxy can also be retrieved from "Tor control" to which we connect if `-listenonion=1`. So, the full Tor proxy retrieval logic is: 1. get it from `-onion` 2. get it from `-proxy` 3. if `-listenonion=1`, then connect to "Tor control" and get the proxy from there (was forgotten before this change) Fixes https://github.com/bitcoin/bitcoin/issues/24980
2022-09-05Merge bitcoin/bitcoin#25768: wallet: Properly rebroadcast unconfirmed ↡glozow
transaction chains 3405f3eed5cf841b23a569b64a376c2e5b5026cd test: Test that an unconfirmed not-in-mempool chain is rebroadcast (Andrew Chow) 10d91c5abe9ed7dcc237c9d52c588e7d26e162a4 wallet: Deduplicate Resend and ReacceptWalletTransactions (Andrew Chow) Pull request description: Currently `ResendWalletTransactions` (used for normal rebroadcasts) will attempt to rebroadcast all of the transactions in the wallet in the order they are stored in `mapWallet`. This ends up being random as `mapWallet` is a `std::unordered_map`. However `ReacceptWalletTransactions` (used for adding to the mempool on loading) first sorts the txs by wallet insertion order, then submits them. The result is that `ResendWalletTranactions` will fail to rebroadcast child transactions if their txids happen to be lexicographically less than their parent's txid. This PR resolves this issue by combining `ReacceptWalletTransactions` and `ResendWalletTransactions` into a new `ResubmitWalletTransactions` so that the iteration code and basic checks are shared. A test has also been added that checks that such transaction chains are rebroadcast correctly. ACKs for top commit: naumenkogs: utACK 3405f3eed5cf841b23a569b64a376c2e5b5026cd 1440000bytes: reACK https://github.com/bitcoin/bitcoin/pull/25768/commits/3405f3eed5cf841b23a569b64a376c2e5b5026cd furszy: Late code review ACK 3405f3ee stickies-v: ACK 3405f3eed5cf841b23a569b64a376c2e5b5026cd Tree-SHA512: 1240d9690ecc2ae8d476286b79e2386f537a90c41dd2b8b8a5a9c2a917aa3af85d6aee019fbbb05e772985a2b197e2788305586d9d5dac78ccba1ee5aa31d77a
2022-09-05Merge bitcoin/bitcoin#25976: QA: rpc_blockchain: Test output of getblock ↡MacroFake
verbosity 0, False, and True f663b43df041da7777e6f45a8df04fa852f79106 QA: rpc_blockchain: Test output of getblock verbosity 0, False, and True (Luke Dashjr) Pull request description: Currently getblock's "verbosity" is documented as a NUM, though it has a fallback to Boolean for the (deprecated?) "verbose" alias. Since we've been doing more generic type-checking on RPC stuff, I think it would be a good idea to actually test the Boolean values work. I didn't see an existing test for verbosity=0, so this adds that too. ACKs for top commit: aureleoules: ACK f663b43df041da7777e6f45a8df04fa852f79106. Tree-SHA512: 321a7795a2f32e469d28879dd323c85cb6b221828030e2a33ad9afd35a648191151a79b04e359b2f58314e43360f81c25f05be07deb42f61efdf556850a7266c
2022-09-04Merge bitcoin/bitcoin#25978: test: fix non-determinism in ↡fanquake
p2p_headers_sync_with_minchainwork.py 88e7807e771a568ac34c320b4055d832990049df test: fix non-determinism in p2p_headers_sync_with_minchainwork.py (Suhas Daftuar) Pull request description: The test for node3's chaintips (added in PR25960) needs some sort of synchronization in order to be reliable. ACKs for top commit: mzumsande: Code Review ACK 88e7807e771a568ac34c320b4055d832990049df satsie: ACK 88e7807e771a568ac34c320b4055d832990049df Tree-SHA512: 5607c5b1a95d91e7cf81b695eb356b782cbb303bcc7fd9044e1058c0c0625c5f9e5fe4f4dde9d2bffa27a80d83fc060336720f7becbba505ccfb8a04fcc81705
2022-09-04Merge bitcoin/bitcoin#25914: test: Fix intermittent issue in p2p_leak.pyfanquake
fa2aae597c42b4f74460c58f35e7e1ace8a82796 test: Fix intermittent issue in p2p_leak.py (MacroFake) Pull request description: Diff to reproduce: ```diff diff --git a/src/net.cpp b/src/net.cpp index 865ce2ea97..ccf289d77b 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1150,6 +1150,7 @@ bool CConnman::InactivityCheck(const CNode& node) const if (last_recv.count() == 0 || last_send.count() == 0) { LogPrint(BCLog::NET, "socket no message in first %i seconds, %d %d peer=%d\n", count_seconds(m_peer_connect_timeout), last_recv.count() != 0, last_send.count() != 0, node.GetId()); + UninterruptibleSleep(6s); return true; } ``` Example in CI: ``` node0 2022-08-12T09:51:56.015288Z [net] [net.cpp:1152] [InactivityCheck] [net] socket no message in first 3 seconds, 0 0 peer=0 test 2022-08-12T09:51:57.658000Z TestFramework (ERROR): Assertion failed Traceback (most recent call last): File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 133, in main self.run_test() File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/p2p_leak.py", line 155, in run_test assert not no_version_idle_peer.is_connected AssertionError ``` https://cirrus-ci.com/task/5346634421764096?logs=ci#L3683 ACKs for top commit: satsie: ACK fa2aae597c42b4f74460c58f35e7e1ace8a82796 luke-jr: tACK fa2aae597c42b4f74460c58f35e7e1ace8a82796 Tree-SHA512: e6ddf5b985f7da365b18b699ff8d0719b71b44e4e6bc5576d4099d1bad2c702495afd85f69f4edba89a883e13756a340946db2e7f4be41b1ac0e3c4f515ca4fd
2022-09-03test: apply fixed feerate to avoid variable dynamic feesstickies-v
Without specifying a feerate, we let the wallet decide on an appropriate feerate, which can be influenced by various factors such as what's in the mempool. Since wallet_groups.py fails when feerates are unstable, we should use a fixed feerate across all nodes. Closes #25940
2022-09-01Merge bitcoin/bitcoin#25614: Severity-based logging, step 2Andrew Chow
958048057087e6562b474f9028316c00ec03c2e4 Update debug logging section in the developer notes (Jon Atack) 1abaa31aa3d833caf2290d6c90f57f7f79d146c0 Update -debug and -debugexclude help docs for severity level logging (Jon Atack) 45f92821621a60891044f57c7a7bc4ab4c7d8a01 Create BCLog::Level::Trace log severity level (Jon Atack) 2a8712db4fb5d06f1a525a79bb0f793cb733aaa6 Unit test coverage for -loglevel configuration option (klementtan) eb7bee5f84d41e35cb4296e01bea2aa5ac80a856 Create -loglevel configuration option (klementtan) 98a1f9c68744074f29fa5fa67514218b5ee9edc4 Unit test coverage for log severity levels (klementtan) 9c7507bf76e79da99766a69df939520ea0a125d1 Create BCLog::Logger::LogLevelsString() helper function (klementtan) 8fe3457dbb4146952b92fb9509bbe4e97dc1f05b Update LogAcceptCategory() and unit tests with log severity levels (klementtan) c2797cfc602c5cdd899a7c11b37bb5711cebff38 Add BCLog::Logger::SetLogLevel()/SetCategoryLogLevel() for string inputs (klementtan) f6c0cc03509255ffa4dfd6e2822fce840dd0b181 Add BCLog::Logger::m_category_log_levels data member and getter/setter (Jon Atack) 2978b387bffc226fb1eaca4d30f24a0deedb2a36 Add BCLog::Logger::m_log_level data member and getter/setter (Jon Atack) f1379aeca9d3a8c4d3528de4d0af8298cb42fee4 Simplify BCLog::Level enum class and LogLevelToStr() function (Jon Atack) Pull request description: This is an updated version of https://github.com/bitcoin/bitcoin/pull/25287 and the next steps in parent PR #25203 implementing, with Klement Tan, user-configurable, per-category severity log levels based on an idea by John Newbery and refined in GitHub discussions by Wladimir Van der Laan and Marco Falke. - simplify the `BCLog::Level` enum class and the `LogLevelToStr()` function and add documentation - update the logging logic to filter logs by log level both globally and per-category - add a hidden `-loglevel` help-debug config option to allow testing setting the global or per-category severity level on startup for logging categories enabled with the `-debug` configuration option or the logging RPC (Klement Tan) - add a `trace` log severity level selectable by the user; the plan is for the current debug messages to become trace, LogPrint ones to become debug, and LogPrintf ones to become info, warning, or error ``` $ ./src/bitcoind -help-debug | grep -A10 loglevel -loglevel=<level>|<category>:<level> Set the global or per-category severity level for logging categories enabled with the -debug configuration option or the logging RPC: info, debug, trace (default=info); warning and error levels are always logged. If <category>:<level> is supplied, the setting will override the global one and may be specified multiple times to set multiple category-specific levels. <category> can be: addrman, bench, blockstorage, cmpctblock, coindb, estimatefee, http, i2p, ipc, leveldb, libevent, lock, mempool, mempoolrej, net, proxy, prune, qt, rand, reindex, rpc, selectcoins, tor, util, validation, walletdb, zmq. ``` See the individual commit messages for details. ACKs for top commit: jonatack: One final push per `git range-diff a5d5569 ce3c4c9 9580480` (should be trivial to re-ACK) to ensure this pull changes no default behavior in any way for users or the tests/CI in order to be completely v24 compatible, to update the unit test setup in general, and to update the debug logging section in the developer notes. klementtan: reACK https://github.com/bitcoin/bitcoin/commit/958048057087e6562b474f9028316c00ec03c2e4 1440000bytes: reACK https://github.com/bitcoin/bitcoin/pull/25614/commits/958048057087e6562b474f9028316c00ec03c2e4 vasild: ACK 958048057087e6562b474f9028316c00ec03c2e4 dunxen: reACK 9580480 brunoerg: reACK 958048057087e6562b474f9028316c00ec03c2e4 Tree-SHA512: 476a638e0581f40b5d058a9992691722e8b546471ec85e07cbc990798d1197fbffbd02e1b3d081b4978404e07a428378cdc8e159c0004b81f58be7fb01b7cba0
2022-09-01test: fix non-determinism in p2p_headers_sync_with_minchainwork.pySuhas Daftuar
The test for node3's chaintips needs some sort of synchronization in order to be reliable.
2022-09-01Merge bitcoin/bitcoin#19602: wallet: Migrate legacy wallets to descriptor ↡Andrew Chow
wallets 53e7ed075c49f853cc845afc7b2f058cabad0cb0 doc: Release notes and other docs for migration (Andrew Chow) 9c44bfe244f35f08ba576d8b979a90dcd68d2c77 Test migratewallet (Andrew Chow) 0b26e7cdf2659fd8b54d21fd2bd749f9f3e87af8 descriptors: addr() and raw() should return false for ToPrivateString (Andrew Chow) 31764c3f872f4f01b48d50585f86e97c41554954 Add migratewallet RPC (Andrew Chow) 0bf7b38bff422e7413bcd3dc0abe2568dd918ddc Implement MigrateLegacyToDescriptor (Andrew Chow) e7b16f925ae5b117e8b74ce814b63e19b19b50f4 Implement MigrateToSQLite (Andrew Chow) 5b62f095e790a0d4e2a70ece89465b64fc68358a wallet: Refactor SetupDescSPKMs to take CExtKey (Andrew Chow) 22401f17e026ead4bc3fe96967eec56a719a4f75 Implement LegacyScriptPubKeyMan::DeleteRecords (Andrew Chow) 35f428fae68ad974abdce0fa905148f620a9443c Implement LegacyScriptPubKeyMan::MigrateToDescriptor (Andrew Chow) ea1ab390e4dac128e3a37d4884528c3f4128ed83 scriptpubkeyman: Implement GetScriptPubKeys in Legacy (Andrew Chow) e664af29760527e75cd7e290be5f102b6d29ebee Apply label to all scriptPubKeys of imported combo() (Andrew Chow) Pull request description: This PR adds a new `migratewallet` RPC which migrates a legacy wallet to a descriptor wallet. Migrated wallets will need a new backup. If a wallet has watchonly stuff in it, a new watchonly descriptor wallet will be created containing those watchonly things. The related transactions, labels, and descriptors for those watchonly things will be removed from the original wallet. Migrated wallets will not have any of the legacy things be available for fetching from `getnewaddress` or `getrawchangeaddress`. Wallets that have private keys enabled will have newly generated descriptors. Wallets with private keys disabled will not have any active `ScriptPubKeyMan`s. For the basic HD wallet case of just generated keys, in addition to the standard descriptor wallet descriptors using the master key derived from the pre-existing hd seed, the migration will also create 3 descriptors for each HD chain in: a ranged combo external, a ranged combo internal, and a single key combo for the seed (the seed is a valid key that we can receive coins at!). The migrated wallet will then have newly generated descriptors as the active `ScriptPubKeyMan`s. This is equivalent to creating a new descriptor wallet and importing the 3 descriptors for each HD chain. For wallets containing non-HD keys, each key will have its own combo descriptor. There are also tests. ACKs for top commit: Sjors: tACK 53e7ed075c49f853cc845afc7b2f058cabad0cb0 w0xlt: reACK https://github.com/bitcoin/bitcoin/commit/53e7ed075c49f853cc845afc7b2f058cabad0cb0 Tree-SHA512: c0c003694ca2e17064922d08e8464278d314e970efb7df874b4fe04ec5d124c7206409ca701c65c099d17779ab2136ae63f1da2a9dba39b45f6d62cf93b5c60a
2022-09-01QA: rpc_blockchain: Test output of getblock verbosity 0, False, and TrueLuke Dashjr
2022-09-01Merge bitcoin/bitcoin#25960: p2p: Headers-sync followupsfanquake
94af3e43e20aa00b18e7a3f6d0f5fe3ad9494d97 Fix typo from PR25717 (Suhas Daftuar) e5982ecdc4650e9b6de38f190f3a97d792499e2a Bypass headers anti-DoS checks for NoBan peers (Suhas Daftuar) 132ed7eaaa4a47ab94db72ebfab0ef0e03caa488 Move headerssync logging to BCLog::NET (Suhas Daftuar) Pull request description: Remove BCLog::HEADERSSYNC and move all headerssync logging to BCLog::NET. Bypass headers anti-DoS checks for NoBan peers Also fix a typo that was introduced in PR25717. ACKs for top commit: Sjors: tACK 94af3e43e20aa00b18e7a3f6d0f5fe3ad9494d97 ajtowns: ACK 94af3e43e20aa00b18e7a3f6d0f5fe3ad9494d97 sipa: ACK 94af3e43e20aa00b18e7a3f6d0f5fe3ad9494d97 naumenkogs: ACK 94af3e43e20aa00b18e7a3f6d0f5fe3ad9494d97 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25960/commits/94af3e43e20aa00b18e7a3f6d0f5fe3ad9494d97 Tree-SHA512: 612d594eddace977359bcc8234b2093d273fd50662f4ac70cb90903d28fb831f6e1aecff51a4ef6c0bb0f6fb5d1aa7ff1eb8798fac5ac142783788f3080717dc
2022-08-31Merge bitcoin/bitcoin#25915: test: Fix wallet_balance intermittent issueAndrew Chow
fae5bd920007c33de0b794bf2b2d698bfccc12ee test: Fix wallet_balance intermittent issue (MacroFake) Pull request description: Diff to reproduce: ```diff index d2ed97ca76..25cc2d5734 100755 --- a/test/functional/wallet_balance.py +++ b/test/functional/wallet_balance.py @@ -265,7 +265,7 @@ class WalletTest(BitcoinTestFramework): self.nodes[0].invalidateblock(block_reorg) self.nodes[1].invalidateblock(block_reorg) assert_equal(self.nodes[0].getbalance(minconf=0), 0) # wallet txs not in the mempool are untrusted - self.generatetoaddress(self.nodes[0], 1, ADDRESS_WATCHONLY, sync_fun=self.no_op) + self.generatetoaddress(self.nodes[0], 1, ADDRESS_WATCHONLY) assert_equal(self.nodes[0].getbalance(minconf=0), 0) # wallet txs not in the mempool are untrusted # Now confirm tx_orig ``` Example in CI: ``` test 2022-08-24T10:09:22.486000Z TestFramework (ERROR): Assertion failed Traceback (most recent call last): File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 133, in main self.run_test() File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/wallet_balance.py", line 269, in run_test assert_equal(self.nodes[0].getbalance(minconf=0), 0) # wallet txs not in the mempool are untrusted File "/tmp/cirrus-ci-build/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/util.py", line 56, in assert_equal raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args)) AssertionError: not(98.85983340 == 0) ``` https://cirrus-ci.com/task/4981266251513856?logs=ci#L3269 ACKs for top commit: achow101: ACK fae5bd920007c33de0b794bf2b2d698bfccc12ee w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25915/commits/fae5bd920007c33de0b794bf2b2d698bfccc12ee Tree-SHA512: 470f366720615c4a9326ec4c581fff569ecce9877f9134bb1975ec3d6f1d13a6403051418a91a80b2a86de617f43e539ec11bbf4f1713d0354d5b0ab98d22437
2022-08-31Merge bitcoin/bitcoin#25955: test: use `sendall` when emptying walletMacroFake
28ea4c7039710541e70ec01abefc3eb8268e06f5 test: simplify splitment with `sendall` in wallet_basic (brunoerg) 923d24583d826f4c6ecad30b185e0e043ea11dfc test: use `sendall` when emptying wallet (brunoerg) Pull request description: In some tests they have used `sendtoaddress` in order to empty a wallet. With the addition of `sendall`, it makes sense to use it for that. ACKs for top commit: achow101: ACK 28ea4c7039710541e70ec01abefc3eb8268e06f5 ishaanam: utACK 28ea4c7039710541e70ec01abefc3eb8268e06f5 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25955/commits/28ea4c7039710541e70ec01abefc3eb8268e06f5 Tree-SHA512: 903136d7df5c65d3c02310d5a84241c9fd11070f69d932b4e188b8ad45c38ab5bc1bd5a9242b3e52d2576665ead14be0a03971a9ad8c00431fed442eba4ca48f
2022-08-30test: simplify splitment with `sendall` in wallet_basicbrunoerg
recipients receive equal share of the unspecified amount
2022-08-30Bypass headers anti-DoS checks for NoBan peersSuhas Daftuar
2022-08-30Merge bitcoin/bitcoin#25717: p2p: Implement anti-DoS headers syncfanquake
3add23454624c4c79c9eebc060b6fbed4e3131a7 ui: show header pre-synchronization progress (Pieter Wuille) 738421c50f2dbd7395b50a5dbdf6168b07435e62 Emit NotifyHeaderTip signals for pre-synchronization progress (Pieter Wuille) 376086fc5a187f5b2ab3a0d1202ed4e6c22bdb50 Make validation interface capable of signalling header presync (Pieter Wuille) 93eae27031a65b4156df49015ae45b2b541b4e5a Test large reorgs with headerssync logic (Suhas Daftuar) 355547334f7d08640ee1fa291227356d61145d1a Track headers presync progress and log it (Pieter Wuille) 03712dddfbb9fe0dc7a2ead53c65106189f5c803 Expose HeadersSyncState::m_current_height in getpeerinfo() (Suhas Daftuar) 150a5486db50ff77c91765392149000029c8a309 Test headers sync using minchainwork threshold (Suhas Daftuar) 0b6aa826b53470c9cc8ef4a153fa710dce80882f Add unit test for HeadersSyncState (Suhas Daftuar) 83c6a0c5249c4ecbd11f7828c84a50fb473faba3 Reduce spurious messages during headers sync (Suhas Daftuar) ed6cddd98e32263fc116a4380af6d66da20da990 Require callers of AcceptBlockHeader() to perform anti-dos checks (Suhas Daftuar) 551a8d957c4c44afbd0d608fcdf7c6a4352babce Utilize anti-DoS headers download strategy (Suhas Daftuar) ed470940cddbeb40425960d51cefeec4948febe4 Add functions to construct locators without CChain (Pieter Wuille) 84852bb6bb3579e475ce78fe729fd125ddbc715f Add bitdeque, an std::deque<bool> analogue that does bit packing. (Pieter Wuille) 1d4cfa4272cf2c8b980cc8762c1ff2220d3e8d51 Add function to validate difficulty changes (Suhas Daftuar) Pull request description: New nodes starting up for the first time lack protection against DoS from low-difficulty headers. While checkpoints serve as our protection against headers that fork from the main chain below the known checkpointed values, this protection only applies to nodes that have been able to download the honest chain to the checkpointed heights. We can protect all nodes from DoS from low-difficulty headers by adopting a different strategy: before we commit to storing a header in permanent storage, first verify that the header is part of a chain that has sufficiently high work (either `nMinimumChainWork`, or something comparable to our tip). This means that we will download headers from a given peer twice: once to verify the work on the chain, and a second time when permanently storing the headers. The p2p protocol doesn't provide an easy way for us to ensure that we receive the same headers during the second download of peer's headers chain. To ensure that a peer doesn't (say) give us the main chain in phase 1 to trick us into permanently storing an alternate, low-work chain in phase 2, we store commitments to the headers during our first download, which we validate in the second download. Some parameters must be chosen for commitment size/frequency in phase 1, and validation of commitments in phase 2. In this PR, those parameters are chosen to both (a) minimize the per-peer memory usage that an attacker could utilize, and (b) bound the expected amount of permanent memory that an attacker could get us to use to be well-below the memory growth that we'd get from the honest chain (where we expect 1 new block header every 10 minutes). After this PR, we should be able to remove checkpoints from our code, which is a nice philosophical change for us to make as well, as there has been confusion over the years about the role checkpoints play in Bitcoin's consensus algorithm. Thanks to Pieter Wuille for collaborating on this design. ACKs for top commit: Sjors: re-tACK 3add23454624c4c79c9eebc060b6fbed4e3131a7 mzumsande: re-ACK 3add23454624c4c79c9eebc060b6fbed4e3131a7 sipa: re-ACK 3add23454624c4c79c9eebc060b6fbed4e3131a7 glozow: ACK 3add234546 Tree-SHA512: e7789d65f62f72141b8899eb4a2fb3d0621278394d2d7adaa004675250118f89a4e4cb42777fe56649d744ec445ad95141e10f6def65f0a58b7b35b2e654a875
2022-08-30test: use `sendall` when emptying walletbrunoerg
2022-08-29Test migratewalletAndrew Chow
Co-Authored-By: furszy <matiasfurszyfer@protonmail.com>
2022-08-29test: Test that an unconfirmed not-in-mempool chain is rebroadcastAndrew Chow
The test checks that parent txs are broadcast before child txs. The previous behavior is that the rebroadcasting would simply iterate mapWallet. As mapWallet is a std::unsorted_map, the child can sometimes come before the parent and thus be rebroadcast in the wrong order and fail the test.
2022-08-29wallet: Deduplicate Resend and ReacceptWalletTransactionsAndrew Chow
Both of these functions do almost the exact same thing. They can be deduplicated so that their behavior matches except for the filtering aspect. As this function will now always be called on wallet loading, nNextResend will also always be initialized, so wallet_resendwallettransactions.py is updated to account for that. This also resolves a bug where ResendWalletTransactions would fail to rebroadcast txs in insertion order thereby potentially rebroadcasting a child transaction before its parent and causing the child to not actually get rebroadcast. Also names the combined function to ResubmitWalletTransactions as the function just submits the transactions to the mempool rather than doing any sending by itself.
2022-08-29Test large reorgs with headerssync logicSuhas Daftuar
2022-08-29Expose HeadersSyncState::m_current_height in getpeerinfo()Suhas Daftuar
2022-08-29Test headers sync using minchainwork thresholdSuhas Daftuar
2022-08-29Require callers of AcceptBlockHeader() to perform anti-dos checksSuhas Daftuar
In order to prevent memory DoS, we must ensure that we don't accept a new header into memory until we've performed anti-DoS checks, such as verifying that the header is part of a sufficiently high work chain. This commit adds a new argument to AcceptBlockHeader() so that we can ensure that all call-sites which might cause a new header to be accepted into memory have to grapple with the question of whether the header is safe to accept, or needs further validation. This patch also fixes two places where low-difficulty-headers could have been processed without such validation (processing an unrequested block from the network, and processing a compact block). Credit to Niklas GΓΆgge for noticing this issue, and thanks to Sjors Provoost for test code.
2022-08-29Utilize anti-DoS headers download strategySuhas Daftuar
Avoid permanently storing headers from a peer, unless the headers are part of a chain with sufficiently high work. This prevents memory attacks using low-work headers. Designed and co-authored with Pieter Wuille.
2022-08-27test: Fix wallet_balance intermittent issueMacroFake
Fix it by removing a duplicate balance check on the same node.
2022-08-26Merge bitcoin/bitcoin#25922: wallet: trigger MaybeResendWalletTxs() every minuteAndrew Chow
5ef8c2c9fc4ebce6cbfea6a55a89a0ab7ee98a1a test: fix typo for MaybeResendWalletTxs (stickies-v) fbba4a131647c991afc53b6a3dfb9721f5c430b2 wallet: trigger MaybeResendWalletTxs() every minute (stickies-v) Pull request description: ResendWalletTransactions() only executes every [12-36h (24h average)](https://github.com/bitcoin/bitcoin/blob/1420547ec30a24fc82ba3ae5ac18374e8e5af5e5/src/wallet/wallet.cpp#L1947). Triggering it every second is excessive, once per minute should be plenty. The goal of this PR is to reduce the amount of (unnecessary) schedule executions by ~60x without meaningfully altering transaction rebroadcast logic/assumptions which would require more significant review. ACKs for top commit: achow101: ACK 5ef8c2c9fc4ebce6cbfea6a55a89a0ab7ee98a1a 1440000bytes: ACK https://github.com/bitcoin/bitcoin/pull/25922/commits/5ef8c2c9fc4ebce6cbfea6a55a89a0ab7ee98a1a Tree-SHA512: 4a077e3579b289c11c347eaa0d3601ef2dbb9fee66ab918d56b4a0c2e08222560a0e6be295297a74831836e001a997ecc143adb0c132faaba96a669dac1cd9e6
2022-08-26Merge bitcoin/bitcoin#25355: I2P: add support for transient addresses for ↡Andrew Chow
outbound connections 59aa54f7312f3441692c89feed86b8756d9d6b7a i2p: log "SAM session" instead of "session" (Vasil Dimov) d7ec30b648721133b5a5ac3f52275f779c54310f doc: add release notes about the I2P transient addresses (Vasil Dimov) 47c0d02f126c73755288c3084402098567964329 doc: document I2P transient addresses usage in doc/i2p.md (Vasil Dimov) 3914e472f5685c29aa3d1c6dc5af9a758313d6c1 test: add a test that -i2pacceptincoming=0 creates a transient session (Vasil Dimov) ae1e97ce863609e06be44a2632fb9d1fbb8e5698 net: use transient I2P session for outbound if -i2pacceptincoming=0 (Vasil Dimov) a1580a04f5d7c9ecb30ee0d3bfdae519843a67ac net: store an optional I2P session in CNode (Vasil Dimov) 2b781ad66e34000037f589c71366c203255ed058 i2p: add support for creating transient sessions (Vasil Dimov) Pull request description: Add support for generating a transient, one-time I2P address for ourselves when making I2P outbound connection and discard it once the connection is closed. Background --- In I2P connections, the host that receives the connection knows the I2P address of the connection initiator. This is unlike the Tor network where the recipient does not know who is connecting to them, not even the initiator's Tor address. Persistent vs transient I2P addresses --- Even if an I2P node is not accepting incoming connections, they are known to other nodes by their outgoing I2P address. This creates an opportunity to white-list given nodes or treat them differently based on their I2P address. However, this also creates an opportunity to fingerprint or analyze a given node because it always uses the same I2P address when it connects to other nodes. If this is undesirable, then a node operator can use the newly introduced `-i2ptransientout` to generate a transient (disposable), one-time I2P address for each new outgoing connection. That address is never going to be reused again, not even if reconnecting to the same peer later. ACKs for top commit: mzumsande: ACK 59aa54f7312f3441692c89feed86b8756d9d6b7a (verified via range-diff that just a typo / `unique_ptr` initialisation were fixed) achow101: re-ACK 59aa54f7312f3441692c89feed86b8756d9d6b7a jonatack: utACK 59aa54f7312f3441692c89feed86b8756d9d6b7a reviewed range diff, rebased to master, debug build + relevant tests + review at each commit Tree-SHA512: 2be9b9dd7502b2d44a75e095aaece61700766bff9af0a2846c29ca4e152b0a92bdfa30f61e8e32b6edb1225f74f1a78d19b7bf069f00b8f8173e69705414a93e
2022-08-25Apply label to all scriptPubKeys of imported combo()Andrew Chow
2022-08-25test: fix typo for MaybeResendWalletTxsstickies-v
2022-08-25wallet: trigger MaybeResendWalletTxs() every minutestickies-v
ResendWalletTransactions() only executes every 12-36h (24h average). Triggering it every second is excessive, once per minute should be plenty.
2022-08-24test: Fix intermittent issue in p2p_leak.pyMacroFake
2022-08-24Merge bitcoin/bitcoin#25865: test: speedup wallet tests by whitelisting ↡MacroFake
peers (immediate tx relay) b21e522ce47a13e024488e43f1cd33a0f1769197 test: speedup wallet tests by whitelisting peers (immediate tx relay) (Sebastian Falbesoner) Pull request description: In the course of testing #25297 by running all wallet-related functional tests (see https://github.com/bitcoin/bitcoin/pull/25297#issuecomment-1203365589), I noticed that the run-time of those tests vary a lot between runs, in fact too much for a useful comparison. This PR fixes this by making the tests both more deterministic and also faster, using the good ol' immediate tx relay trick (parameter `-whitelist=noban@127.0.0.1`). master branch: ``` wallet_abandonconflict.py --descriptors | βœ“ Passed | 7 s wallet_abandonconflict.py --legacy-wallet | βœ“ Passed | 23 s wallet_balance.py --descriptors | βœ“ Passed | 17 s wallet_balance.py --legacy-wallet | βœ“ Passed | 21 s wallet_basic.py --descriptors | βœ“ Passed | 32 s wallet_basic.py --legacy-wallet | βœ“ Passed | 56 s wallet_bumpfee.py --descriptors | βœ“ Passed | 44 s wallet_bumpfee.py --legacy-wallet | βœ“ Passed | 45 s wallet_groups.py --descriptors | βœ“ Passed | 89 s wallet_groups.py --legacy-wallet | βœ“ Passed | 94 s wallet_hd.py --descriptors | βœ“ Passed | 7 s wallet_hd.py --legacy-wallet | βœ“ Passed | 13 s wallet_importdescriptors.py --descriptors | βœ“ Passed | 26 s wallet_listreceivedby.py --descriptors | βœ“ Passed | 28 s wallet_listreceivedby.py --legacy-wallet | βœ“ Passed | 18 s ALL | βœ“ Passed | 520 s (accumulated) Runtime: 526 s ``` PR branch: ``` wallet_abandonconflict.py --descriptors | βœ“ Passed | 7 s wallet_abandonconflict.py --legacy-wallet | βœ“ Passed | 11 s wallet_balance.py --descriptors | βœ“ Passed | 8 s wallet_balance.py --legacy-wallet | βœ“ Passed | 8 s wallet_basic.py --descriptors | βœ“ Passed | 29 s wallet_basic.py --legacy-wallet | βœ“ Passed | 36 s wallet_bumpfee.py --descriptors | βœ“ Passed | 39 s wallet_bumpfee.py --legacy-wallet | βœ“ Passed | 32 s wallet_groups.py --descriptors | βœ“ Passed | 39 s wallet_groups.py --legacy-wallet | βœ“ Passed | 41 s wallet_hd.py --descriptors | βœ“ Passed | 8 s wallet_hd.py --legacy-wallet | βœ“ Passed | 11 s wallet_importdescriptors.py --descriptors | βœ“ Passed | 17 s wallet_listreceivedby.py --descriptors | βœ“ Passed | 7 s wallet_listreceivedby.py --legacy-wallet | βœ“ Passed | 9 s ALL | βœ“ Passed | 302 s (accumulated) Runtime: 309 s ``` Note that an alternative approach could be to whitelist peers by default for nodes in the functional test framework and only enable the trickle relay for the few tests where it's really needed. ACKs for top commit: naumenkogs: utACK b21e522ce47a13e024488e43f1cd33a0f1769197 Tree-SHA512: ac3c8f8f5a401d1b6af60ece9c77e72449f18920c2cb4a1bd65fb4d62cf428779ebf4e1d29009a882977b2252922df4e7183541e0da8de932f8cd479149e8a86
2022-08-24Merge bitcoin/bitcoin#25906: test: add coverage for invalid parameters for ↡MacroFake
`rescanblockchain` d1a00046214c02684438adcfcd23eea39b86bc7f test: add coverage for invalid parameters for `rescanblockchain` (brunoerg) Pull request description: This PR adds test coverage for the following errors: https://github.com/bitcoin/bitcoin/blob/2bd9aa5a44b88c866c4d98f8a7bf7154049cba31/src/wallet/rpc/transactions.cpp#L880-L894 ACKs for top commit: w0xlt: reACK https://github.com/bitcoin/bitcoin/pull/25906/commits/d1a00046214c02684438adcfcd23eea39b86bc7f Tree-SHA512: c357fbda3d261e4d06a29d2a5350482db5f97a815adf59abdac1971eb19b69cfd4d54e4d21836851e2e3b116aa2a820ea1437c7aededf86b06df435cca16ac90
2022-08-23test: add coverage for invalid parameters for `rescanblockchain`brunoerg
2022-08-22Merge bitcoin/bitcoin#25775: docs: remove non-signaling mentions of BIP125fanquake
1dc03dda05e9dce128e57f05bb7b1bb02b3cfb9e [doc] remove non-signaling mentions of BIP125 (glozow) 32024d40f03fbf47c64d814fa5f2c2a73ec14cb7 scripted-diff: remove mention of BIP125 from non-signaling var names (glozow) Pull request description: We have pretty thorough documentation of our RBF policy in doc/policy/mempool-replacements.md. It enumerates each rule with several sentences of rationale. Also, each rule pretty much has its own function (3 and 4 share one), with extensive comments. The doc states explicitly that our rules are similar but differ from BIP125, and contains a record of historical changes to RBF policy. We should not use "BIP125" as synonymous with our RBF policy because: - Our RBF policy is different from what is specified in BIP125, for example: - the BIP does not mention our rule about the replacement feerate being higher (our Rule 6) - the BIP uses minimum relay feerate for Rule 4, while we have used incremental relay feerate since #9380 - the "inherited signaling" question (CVE-2021-31876). Call it discrepancy, ambiguous wording, doc misinterpretation, or implementation details, I would recommend users refer to doc/policy/mempool-replacements.md - the signaling policy is configurable, see #25353 - Our RBF policy may change further - We have already marked BIP125 as only "partially implemented" in docs/bips.md since 1fd49eb498c75a1d14193bb736d195a3dc75ae12 - See comments from people who are not me recently: - https://github.com/bitcoin/bitcoin/pull/25038#discussion_r909507429 - https://github.com/bitcoin/bitcoin/pull/25575#issuecomment-1179519204 This PR removes all non-signaling mentions of BIP125 (if people feel strongly, we can remove all mentions of BIP125 period). It may be useful to refer to the concept of "tx opts in to RBF if it has at least one nSequence less than (0xffffffff - 1)" as "BIP125 signaling" because: - It is succint. - It has already been widely marketed as BIP125 opt-in signaling. - Our API uses it when referring to signaling (e.g. getmempoolentry["bip125-replaceable"] and wallet error message "not BIP 125 replaceable"). Changing those is more invasive. - If/when we have other ways to signal in the future, we can disambiguate them this way. See #25038 which proposes another way of signaling, and where I pulled these commits from. Alternatives: - Changing our policy to match BIP125. This doesn't make sense as, for example, we would have to remove the requirement that a replacement tx has a higher feerate (Rule 6). - Changing BIP125 to match what we have. This doesn't make sense as it would be a significant change to a BIP years after it was finalized and already used as a spec to implement RBF in other places. - Document our policy as a new BIP and give it a number. This might make sense if we don't expect things to change a lot, and can be done as a next step. ACKs for top commit: darosior: ACK 1dc03dda05e9dce128e57f05bb7b1bb02b3cfb9e ariard: ACK 1dc03dda t-bast: ACK https://github.com/bitcoin/bitcoin/commit/1dc03dda05e9dce128e57f05bb7b1bb02b3cfb9e Tree-SHA512: a3adc2039ec5785892d230ec442e50f47f7062717392728152bbbe27ce1c564141f85253143f53cb44e1331cf47476d74f5d2f4b3cd873fc3433d7a0aa783e02