aboutsummaryrefslogtreecommitdiff
path: root/test/functional
AgeCommit message (Collapse)Author
10 hoursMerge bitcoin/bitcoin#30793: rpc: add getorphantxsHEADmasterglozow
98c1536852d1de9a978b11046e7414e79ed40b46 test: add getorphantxs tests (tdb3) 93f48fceb7dd332ef980ce890ff7750b995d6077 test: add tx_in_orphanage() (tdb3) 34a9c10e8cdb3e9cd40fc3a420df8f73e0208a48 rpc: add getorphantxs (tdb3) f511ff3654d999951a64098c8a9f2f8e29771dad refactor: move verbosity parsing to rpc/util (tdb3) 532491faf1aa90053af52cbedce403b9eccf0bc3 net: add GetOrphanTransactions() to PeerManager (tdb3) 91b65adff2aaf16f42c5ccca6e16b951e0e84f9a refactor: add OrphanTxBase for external use (tdb3) Pull request description: This PR adds a new hidden rpc, `getorphantxs`, that provides the caller with a list of orphan transactions. This rpc may be helpful when checking orphan behavior/scenarios (e.g. in tests like `p2p_orphan_handling`) or providing additional data for statistics/visualization. ``` getorphantxs ( verbosity ) Shows transactions in the tx orphanage. EXPERIMENTAL warning: this call may be changed in future releases. Arguments: 1. verbosity (numeric, optional, default=0) 0 for an array of txids (may contain duplicates), 1 for an array of objects with tx details, and 2 for details from (1) and tx hex Result (for verbose = 0): [ (json array) "hex", (string) The transaction hash in hex ... ] Result (for verbose = 1): [ (json array) { (json object) "txid" : "hex", (string) The transaction hash in hex "wtxid" : "hex", (string) The transaction witness hash in hex "bytes" : n, (numeric) The serialized transaction size in bytes "vsize" : n, (numeric) The virtual transaction size as defined in BIP 141. This is different from actual serialized size for witness transactions as witness data is discounted. "weight" : n, (numeric) The transaction weight as defined in BIP 141. "expiration" : xxx, (numeric) The orphan expiration time expressed in UNIX epoch time "from" : [ (json array) n, (numeric) Peer ID ... ] }, ... ] Result (for verbose = 2): [ (json array) { (json object) "txid" : "hex", (string) The transaction hash in hex "wtxid" : "hex", (string) The transaction witness hash in hex "bytes" : n, (numeric) The serialized transaction size in bytes "vsize" : n, (numeric) The virtual transaction size as defined in BIP 141. This is different from actual serialized size for witness transactions as witness data is discounted. "weight" : n, (numeric) The transaction weight as defined in BIP 141. "expiration" : xxx, (numeric) The orphan expiration time expressed in UNIX epoch time "from" : [ (json array) n, (numeric) Peer ID ... ], "hex" : "hex" (string) The serialized, hex-encoded transaction data }, ... ] Examples: > bitcoin-cli getorphantxs 2 > curl --user myusername --data-binary '{"jsonrpc": "2.0", "id": "curltest", "method": "getorphantxs", "params": [2]}' -H 'content-type: application/json' http://127.0.0.1:8332/ ``` ``` $ build/src/bitcoin-cli getorphantxs 2 [ { "txid": "50128aac5deab548228d74d846675ad4def91cd92453d81a2daa778df12a63f2", "wtxid": "bb61659336f59fcf23acb47c05dc4bbea63ab533a98c412f3a12cb813308d52c", "bytes": 133, "vsize": 104, "weight": 415, "expiration": 1725663854, "from": [ 1 ], "hex": "020000000001010b992959eaa2018bbf31a4a3f9aa30896a8144dbd5cfaf263bf07c0845a3a6620000000000000000000140fe042a010000002251202913b252fe537830f843bfdc5fa7d20ba48639a87c86ff837b92d083c55ad7c102015121c0000000000000000000000000000000000000000000000000000000000000000100000000" }, { "txid": "330bb7f701604a40ade20aa129e9a3eb8a7bf024e599084ca1026d3222b9f8a1", "wtxid": "b7651f7d4c1a40c4d01f6a1e43a121967091fa0f56bb460146c1c5c068e824f6", "bytes": 133, "vsize": 104, "weight": 415, "expiration": 1725663854, "from": [ 2 ], "hex": "020000000001013600adfe41e0ebd2454838963d270916d2b47239c9eebb93a992b720d3589a080000000000000000000140fe042a010000002251202913b252fe537830f843bfdc5fa7d20ba48639a87c86ff837b92d083c55ad7c102015121c0000000000000000000000000000000000000000000000000000000000000000100000000" } ] ``` ACKs for top commit: glozow: reACK 98c1536852d hodlinator: re-ACK 98c1536852d1de9a978b11046e7414e79ed40b46 danielabrozzoni: ACK 98c1536852d1de9a978b11046e7414e79ed40b46 pablomartin4btc: tACK 98c1536852d1de9a978b11046e7414e79ed40b46 itornaza: reACK 98c1536852d1de9a978b11046e7414e79ed40b46 Tree-SHA512: 66075f9faa83748350b87397302100d08af92cbef5fadb27f2b4903f028c08020bf34a23e17262b41abb3f379ca9f46cf6cd5459b8681f2b83bffbbaf3c03ff9
3 daystest: add getorphantxs teststdb3
Adds functional tests for getorphantxs
3 daystest: add tx_in_orphanage()tdb3
Allows tests to check if a transaction is contained within the orphanage
4 daystest: add missing sync to feature_fee_estimation.pyMartin Zumsande
Fixes a race between node 1 catching up with the chain and mining a new block in the sanity_check_rbf_estimates subtest.
7 daystest: refactor: introduce and use `TRUC_CHILD_MAX_VSIZE` constantSebastian Falbesoner
7 daystest: switch MiniWallet padding unit from weight to vsizeSebastian Falbesoner
The weight unit is merely a consensus rule detail and is largely irrelevant for fee-rate calculations and mempool policy rules (e.g. for package relay and TRUC limits), so there doesn't seem to be any value of using a granularity that we can't even guarantee to reach exactly anyway. Switch to the more natural unit of vsize instead, which simplifies both the padding implementation and the current tests that take use of this padding. The rather annoying multiplications by `WITNESS_SCALE_FACTOR` can then be removed and weird-looking magic numbers like `4004` can be replaced by numbers that are more connected to actual policy limit constants from the codebase, e.g. `1001` for exceeding `TRUC_CHILD_MAX_VSIZE` by one.
9 daysMerge bitcoin/bitcoin#30879: test: re-bucket long-running testsmerge-script
f5a2000579b140a1f51fc433706c775ca560c62c test: re-bucket long-running tests (willcl-ark) Pull request description: Re-bucket: - `p2p_node_network_limited -v*transport` - `feature_assume_utxo` On CI runners these tests are taking longer than their current bucket suggests, often being among the last to finish. Re-bucket them to improve CI efficiency. ACKs for top commit: maflcko: review ACK f5a2000579b140a1f51fc433706c775ca560c62c Tree-SHA512: 3da5c888db64a311276338270ba1dcad3eb2a24e205f6bb86fc92f767ecfa63682f13fafffff569fa0cfaea607ccb538f31e3934a086d482c3fe1be5d39f8791
9 daysMerge bitcoin/bitcoin#30948: test: Add missing sync_mempools() to fill_mempool()merge-script
faf801515f8fcd11a3454105cab66c38f6f240fe test: Add missing sync_mempools() to fill_mempool() (MarcoFalke) fa48be6f0233cfbd5c1eab7b832c913912062fa5 test: Refactor fill_mempool to extract send_batch helper (MarcoFalke) Pull request description: Not doing the sync will lead to (intermittent) issues, as explained in https://github.com/bitcoin/bitcoin/issues/30922#issuecomment-2364529013. Fix all issues by doing the sync by default and disable it in places that do not need the sync. Fixes #30922 ACKs for top commit: mzumsande: Tested ACK faf801515f8fcd11a3454105cab66c38f6f240fe ismaelsadeeq: Tested ACK faf801515f8fcd11a3454105cab66c38f6f240fe marcofleon: Tested ACK faf801515f8fcd11a3454105cab66c38f6f240fe Tree-SHA512: 2de62d168cbb6857a9fb8bc12c42a9093fedf5e9beb6f83a32b3fa72a5ba3cf03631055fd25ef553399a27a6fe0d71c44cfe60660a4d31986debd13b8ab00228
12 daystest: Add missing sync_mempools() to fill_mempool()MarcoFalke
Also disable the function, when it is not needed.
12 daystest: Refactor fill_mempool to extract send_batch helperMarcoFalke
This is needed for the next commit
12 daysMerge bitcoin/bitcoin#30678: wallet: Write best block to disk before backupAva Chow
f20fe33e94c6752e5d2ed92511c0bf51a10716ee test: Add basic balance coverage to wallet_assumeutxo.py (Fabian Jahr) 037b101e808ccf9e717751619e04f6e87d614efd test: Add coverage for best block locator write in wallet_backup (Fabian Jahr) 31c0df038909e40fe9618a4595254907ed1de907 wallet: migration, write best locator before unloading wallet (furszy) 7e3dbe4180cbeb65e59b53d9fa98509e9189549d wallet: Write best block to disk before backup (Fabian Jahr) Pull request description: I discovered that we don't write the best block to disk when trying to explain the behavior described here: https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1719951882 In the context of that test, the behavior is confusing and I think it also shows that one of the already existing tests in `wallet_assumeutxo.py` doesn't actually test what it says. It only fails because the best block isn't written and actually, the height of the backup that is loaded is at the snapshot height during backup. So it really shouldn't fail since it's past the background sync blocks already. I'm not sure if this is super relevant in practice though so I am first looking for concept ACKs on the `BackupWallet` code change. Either way, I think this behavior should be documented better if it is left as is and the test should be changed. ACKs for top commit: achow101: ACK f20fe33e94c6752e5d2ed92511c0bf51a10716ee furszy: ACK f20fe33 Tree-SHA512: bb384a940df5c942fffe2eb06314ade4fc5d9b924012bfef3b1c456c4182a30825d1e137d8ae561d93d3a8a2f4d1c1ffe568132d20fa7d04844f1e289ab4a28b
12 daysMerge bitcoin/bitcoin#30409: Introduce waitTipChanged() mining interface, ↵Ava Chow
replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block 7942951e3fcc58f7db0059546d03be9ea243f1db Remove unused g_best_block (Ryan Ofsky) e3a560ca68d79b056a105a65ed0c174a9631aba9 rpc: use waitTipChanged for longpoll (Ryan Ofsky) 460687a09c2af336fce853d9ffb790d01429eec6 Remove unused CRPCSignals (Sjors Provoost) dca923150e3ac10a57c23a7e29e76516d32ec10d Replace RPCNotifyBlockChange with waitTipChanged() (Sjors Provoost) 2a40ee1121903847cdd3f6c5b4796e4d5471b2df rpc: check for negative timeout arg in waitfor* (Sjors Provoost) de7c855b3af99fe6b62279c5c2d08888a5437c4a rpc: recommend -rpcclienttimeout=0 for waitfor* (Sjors Provoost) 77ec072925a6d558b3c6b075becbed44727c5989 rpc: fix waitfornewblock description (Sjors Provoost) 285fe9fb51c808a9edd91b05bd3134fc18de0fb6 rpc: add test for waitforblock and waitfornewblock (Sjors Provoost) b94b27cf05c709674117e308e441a8d1efddcd0a Add waitTipChanged to Mining interface (Sjors Provoost) 7eccdaf16081d6f624c4dc21df75b0474e049d2b node: Track last block that received a blockTip notification (Sjors Provoost) ebb8215f23644f901c46fd4977b7d4b08fae5104 Rename getTipHash() to getTip() and return BlockRef (Sjors Provoost) 89a8f74bbbb900abfb3d8e946eea18ad7b1513ad refactor: rename BlockKey to BlockRef (Sjors Provoost) Pull request description: This continues the work in #30200 so that a future Stratum v2 Template Provider (see #29432) can avoid accessing node internals. It needs to know when a new block arrives in order to push new templates to connected clients. `waitTipChanged()` uses a new kernel notification `notifications().m_tip_block_mutex`, which this PR also introduces (a previous version used `g_best_block`). In order to ensure the new method works as intended, the `waitfornewblock`, `waitforblock` and `waitforblockheight` RPC methods are refactored to use it. This allows removing `RPCNotifyBlockChange`. There's a commit to add (direct) tests for the methods that are about to be refactored: - `waitfornewblock` was already implicitly tested by `feature_shutdown.py`. - `waitforblockheight` by `feature_coinstatsindex.py` and `example_test.py` This PR renames `getTipHash()` to `getTip()` and returns a `BlockRef` (renamed from `BlockKey`) so that callers can use either the height or hash. The later commits make trivial improvements to the `waitfor*` RPC calls (not needed for this PR). The `waitTipChanged()` method could probably also be used for the longpoll functionality in `getblocktemplate`, but I'm a bit reluctant to touch that. `RPCServer::OnStarted` no longer does anything and `RPCServer::OnStopped` merely prints a log statement. They were added in #5711 as a refactor. This PR drops them entirely. Finally `g_best_block` is also dropped. ACKs for top commit: achow101: ACK 7942951e3fcc58f7db0059546d03be9ea243f1db ryanofsky: Code review ACK 7942951e3fcc58f7db0059546d03be9ea243f1db. Just rebased since last review TheCharlatan: Re-ACK 7942951e3fcc58f7db0059546d03be9ea243f1db Tree-SHA512: a5559446b4000c95e07aad33284b7ee2e57aafd87e1ae778b3825d59689566d047a8047e47a10f76e6e341e7dc72fd265a65afbc0a9c011d17c4cafd55031837
12 daystest: re-bucket long-running testswillcl-ark
- p2p_node_network_limited -v*transport - feature_assume_utxo On CI runners these tests are taking longer than their current bucket suggests, often being among the last to finish. Re-bucket them to improve CI efficiency.
13 daystest: Add basic balance coverage to wallet_assumeutxo.pyFabian Jahr
2024-09-20Merge bitcoin/bitcoin#30679: fix: handle invalid `-rpcbind` port earlierAva Chow
e6994efe08b282dd9e46602bcbad69567fe91dcd fix: increase rpcbind check robustness (tdb3) d38e3aed89eea665399eef07f5c3b64b14d4f586 fix: handle invalid rpcbind port earlier (tdb3) 83b67f2e6d59ea5de6573314ea4fe54ae52b7c12 refactor: move host/port checking (tdb3) 73c243965ab256ece089d14173c2d285955e83d5 test: add tests for invalid rpcbind ports (tdb3) Pull request description: Previously, when an invalid port was specified in `-rpcbind`, the `SplitHostPort()` return value in `HTTPBindAddresses()` was ignored and attempt would be made to bind to the default rpcbind port (with the host/port treated as a host). This rearranges port checking code in `AppInitMain()` to handle the invalid port before reaching `HTTPBindAddresses()`. Also adds a check in `HTTPBindAddresses()` as a defensive measure for future changes. Adds then updates associated functional tests as well. ACKs for top commit: achow101: ACK e6994efe08b282dd9e46602bcbad69567fe91dcd ryanofsky: Code review ACK e6994efe08b282dd9e46602bcbad69567fe91dcd zaidmstrr: Code review ACK [e6994ef](https://github.com/bitcoin/bitcoin/commit/e6994efe08b282dd9e46602bcbad69567fe91dcd) Tree-SHA512: bcc3e5ceef21963821cd16ce6ecb83d5c5657755882c05872a7cfe661a1492b1d631f54de22f41fdd173512d62dd15dc37e394fe1a7abe4de484b82cd2438b92
2024-09-20Merge bitcoin/bitcoin#26990: cli: Improve error message on multiwallet ↵Ava Chow
cli-side commands 54227e681a4efa8961f1ad05d43366d88a9b686a rpc, cli: improve error message on multiwallet mode (pablomartin4btc) Pull request description: Running a CLI command when multiple wallets are loaded and `-rpcwallet` is not specified, should return a clearer error. Currently in `master`: ``` $ bitcoin-cli -regtest -generate 1 error code: -19 error message: Wallet file not specified (must request wallet RPC through /wallet/<filename> uri-path). Try adding "-rpcwallet=<filename>" option to bitcoin-cli command line. ``` With this change: ``` $ bitcoin-cli -regtest -generate 1 error code: -19 error message: Multiple wallets are loaded. Please select which wallet to use by requesting the RPC through the /wallet/<walletname> URI path. Or for the CLI, specify the "-rpcwallet=<walletname>" option before the command (run "bitcoin-cli -h" for help or "bitcoin-cli listwallets" to see which wallets are currently loaded). ``` ACKs for top commit: maflcko: review ACK 54227e681a4efa8961f1ad05d43366d88a9b686a achow101: ACK 54227e681a4efa8961f1ad05d43366d88a9b686a furszy: utACK 54227e681a4 mzumsande: Code Review ACK 54227e681a4efa8961f1ad05d43366d88a9b686a jonatack: ACK 54227e681a4efa8961f1ad05d43366d88a9b686a Tree-SHA512: 51ff24f64858aa6be6adf6f20105c9f076ebea743780bf2a4399f7fe8b5239cbb1ea06d32b2ef5e850da2369abb0ef7a52c50c2b8f31f4ca90d3a486abc9b77e
2024-09-20test: Add coverage for best block locator write in wallet_backupFabian Jahr
2024-09-20wallet: Write best block to disk before backupFabian Jahr
This ensures that the best block is included in the backup which leads to a more consistent behavior when loading the backup.
2024-09-18Merge bitcoin/bitcoin#30875: doc: fixed inconsistencies in documentation ↵merge-script
between autotools to cmake change a9964c04447745435747d9cc557165c43902783b doc: Updating docs from autotools to cmake (kevkevinpal) Pull request description: A bit of a followup from https://github.com/bitcoin/bitcoin/pull/30840 - In this change the documentation where we refer to the `./configure` script which is now gone and have converted the configure params to use the `cmake` equivalent. ACKs for top commit: maflcko: ACK a9964c04447745435747d9cc557165c43902783b jonatack: utACK a9964c04447745435747d9cc557165c43902783b jarolrod: ACK a9964c04447745435747d9cc557165c43902783b tdb3: ACK a9964c04447745435747d9cc557165c43902783b pablomartin4btc: re-ACK a9964c04447745435747d9cc557165c43902783b Tree-SHA512: f7ed20b8ad61f028c0d242b9cc70650d8da63057d4a8f7da88f0117a8d3241c5fe8fcf19d56ec82088160b9fee9b175fe9f64e5a845260d3696dc7e94bfdd0bd
2024-09-18doc: Updating docs from autotools to cmakekevkevinpal
replaced --enable-debug with -DCMAKE_BUILD_TYPE=Debug in developer-notes replaced --enable-multiprocess with -DWITH_MULTIPROCESS=ON replaced --disable-zmq with -DWITH_ZMQ=OFF
2024-09-17fix: handle invalid rpcbind port earliertdb3
Previously, when an invalid port was specified in `-rpcbind`, the `SplitHostPort()` return value in `HTTPBindAddresses()` was ignored and attempt would be made to bind to the default rpcbind port (with the host/port treated as a host). This rearranges port checking code in `AppInitMain()` to handle the invalid port before reaching `HTTPBindAddresses()`. Also adjusts associated functional tests.
2024-09-17test: add tests for invalid rpcbind portstdb3
2024-09-17rpc, cli: improve error message on multiwallet modepablomartin4btc
The primary objective is to provide users with clearer and more informative error messages when encountering the RPC_WALLET_NOT_SPECIFIED error, which occurs when multiple wallets are loadad. This commit also rectifies the error message consistency by bringing the error message in line with the definition established in protocol.h ("error when there are multiple wallets loaded").
2024-09-17rpc: add test for waitforblock and waitfornewblockSjors Provoost
2024-09-16Merge bitcoin/bitcoin#30410: rpc, rest: Improve block rpc error handling, ↵Ava Chow
check header before attempting to read block data. 6a1aa510e31e8b77793341473aa5afc9d023a6e3 rpc: check block index before reading block / undo data (Martin Zumsande) 6cbf2e5f8197e51b8f3d789ba9f5874a2fd7b93a rpc: Improve gettxoutproof error when only header is available. (Martin Zumsande) 69fc867ea19ba3bd8c38a18b5e3f0e366c46af5b test: add coverage to getblock and getblockstats (Martin Zumsande) 5290cbd58504dcbab97cb0944b3ae0a0f79c4502 rpc: Improve getblock / getblockstats error when only header is available. (Martin Zumsande) e5b537bbdfbfb43ac7bd8e91958e4a2bbd2577ff rest: improve error when only header of a block is available. (Martin Zumsande) Pull request description: Fixes #20978 If a block was pruned, `getblock` already returns a specific error: "Block not available (pruned data)". But if we haven't received the full block yet (e.g. in a race with block downloading after a new block was received headers-first, or during IBD) we just return an unspecific "Block not found on disk" error and log `ERROR: ReadBlockFromDisk: OpenBlockFile failed for FlatFilePos(nFile=-1, nPos=0) ` which suggest something went wrong even though this is a completely normal and expected situation. This PR improves the error message and stops calling `ReadRawBlockFromDisk()`, when we already know from the header that the block is not available on disk. Similarly, it prevents all other rpcs from calling blockstorage read functions unless we expect the data to be there, so that `LogError()` will only be thrown when there is an actual file system problem. I'm not completely sure if the cause is important enough to change the wording of the rpc error, that some scripts may rely on. If reviewers prefer it, an alternative solution would be to keep returning the current "Block not found on disk" error, but return it immediately instead of calling `ReadRawBlockFromDisk`, which would at least prevent the log error and also be an improvement in my opinion. ACKs for top commit: fjahr: re-ACK 6a1aa510e31e8b77793341473aa5afc9d023a6e3 achow101: ACK 6a1aa510e31e8b77793341473aa5afc9d023a6e3 andrewtoth: re-ACK 6a1aa510e31e8b77793341473aa5afc9d023a6e3 Tree-SHA512: 491aef880e8298a05841c4bf8eb913ef84820d1ad5415fd17d9b441bff181959ebfdd432b5eb8347dc9c568433f9a2384ca9d84cd72c79d8a58323ca117538fe
2024-09-13Merge bitcoin/bitcoin#30892: test: Check already deactivated network stays ↵Ava Chow
suspended after dumptxoutset 72c9a1fe94f927220d3159f516fda684ae9d4caa test: Check that network stays suspended after dumptxoutset if it was off before (Fabian Jahr) Pull request description: Follow-up to #30817 which covered the robustness of `dumptxoutset`: network is deactivated during the run but re-activated even when an issue was encountered. But it did not cover the case if the user had deactivated the network themselves before. In that case the user may want the network to stay off so the network is not reactivated after `dumptxoutset` finishes. A test for this behavior is added here. ACKs for top commit: achow101: ACK 72c9a1fe94f927220d3159f516fda684ae9d4caa pablomartin4btc: ACK 72c9a1fe94f927220d3159f516fda684ae9d4caa theStack: utACK 72c9a1fe94f927220d3159f516fda684ae9d4caa tdb3: tested ACK 72c9a1fe94f927220d3159f516fda684ae9d4caa Tree-SHA512: 18a57c5782e99a018414db0597e88debeb32666712c2a75dddbb55135d8f1ddd1eeacba8bbbd35fc03b6c4ab0522fe074ec08edea729560b018f51efabf00e89
2024-09-13rpc: check block index before reading block / undo dataMartin Zumsande
This avoids low-level log errors that are supposed to only occur when there is an actual problem with the block on disk missing unexpectedly, but not in the case where the block and/or undo data are expected not to be there. It changes behavior such that in the first case (block index indicates data is available but retrieving it fails) an error is thrown. It also adjusts a functional tests that tried to simulate not having undo data (but having block data) by deleting the undo file. This situation should occur reality because block and undo data are pruned together. Instead, test this situation with a block that hasn't been connected.
2024-09-13rpc: Improve gettxoutproof error when only header is available.Martin Zumsande
2024-09-13test: add coverage to getblock and getblockstatsMartin Zumsande
also removes an unnecessary newline. Co-authored-by: tdb3 <106488469+tdb3@users.noreply.github.com>
2024-09-13rpc: Improve getblock / getblockstats error when only header is available.Martin Zumsande
This improves the error message of the getblock and getblockstats rpc and prevents calls to ReadRawBlockFromDisk(), which are unnecessary if we know from the header nStatus field that the block is not available.
2024-09-12test: Check that network stays suspended after dumptxoutset if it was off beforeFabian Jahr
2024-09-12Merge bitcoin/bitcoin#30880: test: Wait for local services to update in ↵Ava Chow
feature_assumeutxo 19f4a7c95a99162122068d4badffeea240967a65 test: Wait for local services to update in feature_assumeutxo (Fabian Jahr) Pull request description: Closes #30878 It seems like there is a race where the block is stored locally and `getblock` does not error anymore, but `ActivateBestChain` has not finished yet, so the local services are not updated yet either. Fix this by waiting for the local services to update. Can be reproduced locally by adding the sleep here: ```cpp ──────────────────────────────────────────────────────────────────────────────────────────────────────────┐ src/validation.cpp:3567: bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< │ ──────────────────────────────────────────────────────────────────────────────────────────────────────────┘ } if (WITH_LOCK(::cs_main, return m_disabled)) { std::this_thread::sleep_for(std::chrono::seconds(10)); // Background chainstate has reached the snapshot base block, so exit. // Restart indexes to resume indexing for all blocks unique to the snapshot ``` ACKs for top commit: maflcko: review-only ACK 19f4a7c95a99162122068d4badffeea240967a65 achow101: ACK 19f4a7c95a99162122068d4badffeea240967a65 pablomartin4btc: tACK 19f4a7c95a99162122068d4badffeea240967a65 furszy: Code review ACK [19f4a7c](https://github.com/bitcoin/bitcoin/pull/30880/commits/19f4a7c95a99162122068d4badffeea240967a65). Tree-SHA512: 70dad3795988956c5e20f2d2d895fb56c5e3ce257c7547d3fd729c88949f0e24cb34594da1537bce8794ad02b2db44e7e46e995aa32539cd4dd84c4f1d4bb1c4
2024-09-12test: Wait for local services to update in feature_assumeutxoFabian Jahr
2024-09-12Merge bitcoin/bitcoin#30872: test: fix exclude parsing for functional runnermerge-script
72b46f28bf8d37a166961b5aa2a22302b932b756 test: fix exclude parsing for functional runner (Max Edwards) Pull request description: This restores previous behaviour of being able to exclude a test by name without having to specify .py extension. It was noticed in https://github.com/bitcoin/bitcoin/issues/30851 that tests were no longer being excluded. PR https://github.com/bitcoin/bitcoin/pull/30244 introduced being able to exclude a specific tests based on args (such as `--exclude "rpc_bind.py --ipv6`) but it made the wrong assumption that test names intended to be excluded would include the .py extension. The following https://github.com/bitcoin/bitcoin/pull/30244#issuecomment-2344009687 shows that this is not how the `--exclude` flag was used in CI. https://github.com/bitcoin/bitcoin/pull/30244#issuecomment-2344009687 gave three examples of `--exclude` being used in CI so I compared the number of tests that the runner would run for these three examples in three situations, before #30244 was introduced, in master today and with this PR applied. Example: `--previous-releases --coverage --extended --exclude feature_dbcrash` Test count: Before #30244 introduced: 314 Master: 315 With this PR: 314 Example: `--exclude feature_init,rpc_bind,feature_bind_extra` Test count: Before #30244 introduced: 306 Master 311 With this PR: 306 Example: `--exclude rpc_bind,feature_bind_extra` Before #30244 introduced: 307 Master 311 With this PR: 307 I've also tested that the functionality introduced with #30244 remains and we can still exclude specific tests by argument. ACKs for top commit: maflcko: review ACK 72b46f28bf8d37a166961b5aa2a22302b932b756 willcl-ark: ACK 72b46f28bf8d37a166961b5aa2a22302b932b756 Tree-SHA512: 37c0e3115f4e3efdf9705f4ff8cd86a5cc906aacc1ab26b0f767f5fb6a953034332b29b0667073f8382a48a2fe9d649b7e60493daf04061260adaa421419d8c8
2024-09-12test: fix exclude parsing for functional runnerMax Edwards
This restores previous behaviour of being able to exclude a test by name without having to specify .py extension.
2024-09-12Merge bitcoin/bitcoin#30733: test: remove unused src_dir param from ↵merge-script
run_tests after CMake migration 2ad560139b80e149844f285cd1456bb7cc0eb1ee Remove unused src_dir param from run_tests (Lőrinc) Pull request description: The `src_dir` usage was removed in https://github.com/bitcoin/bitcoin/commit/a8a2e364acf55bbe18404ab21f852d52257bcb6d#diff-437d7f6e9f2229879b60aae574a8217f14c643bbf3cfa9225d8011d6d52df00cL598, making the parameter unused. Top commit has no ACKs. Tree-SHA512: 1fd8b93811b4ab467ba5a160a4fe204e9606e1bf237c7595ed6f8b7821cf59d2a776c0e1e154852a45b2a35e5bdbd8996314e4f63a9c750f21b9a17875cb636a
2024-09-11Merge bitcoin/bitcoin#30807: Fix peers abruptly disconnecting from ↵Ava Chow
AssumeUTXO nodes during IBD 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac test: add coverage for assumeUTXO honest peers disconnection (furszy) 6d5812e5c852c233bd7ead2ceef051f8567619ed assumeUTXO: fix peers disconnection during sync (furszy) Pull request description: Because AssumeUTXO nodes prioritize tip synchronization, they relay their local address through the network before completing the background chain sync. This, combined with the advertising of full-node service (`NODE_NETWORK`), can result in an honest peer in IBD connecting to the AssumeUTXO node (while syncing) and requesting an historical block the node does not have. This behavior leads to an abrupt disconnection due to perceived unresponsiveness from the AssumeUTXO node. This lack of response occurs because nodes ignore `getdata` requests when they do not have the block data available (further discussion can be found in #30385). Fix this by refraining from signaling full-node service support while the background chain is being synced. During this period, the node will only signal `NODE_NETWORK_LIMITED` support. Then, full-node (`NODE_NETWORK`) support will be re-enabled once the background chain sync is completed. Thanks mzumsande for a post-#30385 convo too. Testing notes: Just cherry-pick the second commit (bb08c22) on master. It will fail there, due to the IBD node requesting historical blocks to the snapshot node - which is bad because the snapshot node will ignore the requests and stall + disconnect after some time. ACKs for top commit: achow101: ACK 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac naumenkogs: ACK 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac mzumsande: ACK 992f83bb6f4b29b44f4eaace1d1a2c0001d43cac Tree-SHA512: fef525d1cf3200c2dd89a346be9c82d77f2e28ddaaea1f490a435e180d1a47a371cadea508349777d740ab56e94be536ad8f7d61cc81f6550c58b609b3779ed3
2024-09-10Merge bitcoin/bitcoin#30805: test: Add explicit onion bind to p2p_permissionsglozow
082779d6062c5b72f3497bb864e4dbb4373a3a4c test: Add explicit onion bind to p2p_permissions (Ava Chow) Pull request description: When the bind option is replaced in the bitcoin.conf, bitcoind will attempd to bind to the default tor listening port. If another bitcoind is running that is already bound to that port, the bind will fail which, since #22729, causes the test to fail. This failure can be avoided by explicitly binding the tor port when the bind is removed. ACKs for top commit: tdb3: ACK 082779d6062c5b72f3497bb864e4dbb4373a3a4c theStack: re-ACK 082779d6062c5b72f3497bb864e4dbb4373a3a4c glozow: ACK 082779d6062c5b72f3497bb864e4dbb4373a3a4c Tree-SHA512: 4acb69ea2e00aeacf9e7c9ab9595ceaf0e0d2adbd795602034b2184197d9bad54c7bc9f3da43ef9c52a71869fe96ba8c87fc5b7c37880f258f5a2aaab2b4046c
2024-09-10test: add coverage for assumeUTXO honest peers disconnectionfurszy
Exercising and verifying the following points: 1. An IBD node can sync headers from an AssumeUTXO node at any time. 2. IBD nodes do not request historical blocks from AssumeUTXO nodes while they are syncing the background-chain. 3. The assumeUTXO node dynamically adjusts the network services it offers according to its state. 4. IBD nodes can fully sync from AssumeUTXO nodes after they finish the background-chain sync.
2024-09-10assumeUTXO: fix peers disconnection during syncfurszy
Because AssumeUTXO nodes prioritize tip synchronization, they relay their local address through the network before completing the background chain sync. This, combined with the advertising of full-node service (NODE_NETWORK), can result in an honest peer in IBD connecting to the AssumeUTXO node (while syncing) and requesting an historical block the node does not have. This behavior leads to an abrupt disconnection due to perceived unresponsiveness (lack of response) from the AssumeUTXO node. This lack of response occurs because nodes ignore getdata requests when they do not have the block data available (further discussion can be found in PR 30385). Fix this by refraining from signaling full-node service support while the background chain is being synced. During this period, the node will only signal 'NODE_NETWORK_LIMITED' support. Then, full-node ('NODE_NETWORK') support will be re-enabled once the background chain sync is completed.
2024-09-10test: Add explicit onion bind to p2p_permissionsAva Chow
When the bind option is replaced in the bitcoin.conf, bitcoind will attempd to bind to the default tor listening port. If another bitcoind is running that is already bound to that port, the bind will fail which, since #22729, causes the test to fail. This failure can be avoided by explicitly binding the tor port when the bind is removed.
2024-09-09Merge bitcoin/bitcoin#30817: test: Add coverage for dumptxoutset failure ↵Ava Chow
robustness c2b779da4e7f1bf1a5c5d67ec94cba3027b42ee7 refactor: Manage dumptxoutset RAII classes with std::optional (Fabian Jahr) 4b5bf335adabd1586043caa72a98356a8255bc29 test: Add coverage for failing dumptxoutset behavior (Fabian Jahr) Pull request description: This adds a test that checks that network activity is not suspended if dumptxoutset fails in the middle of its process which is implemented with the `NetworkDisable` RAII class. I would have liked to add coverage for the `TemporaryRollback` RAII class but that seems a lot more tricky since the failure needs to happen at some point after the rollback and on the scale of our test chain here I couldn't find a way to do it yet. This was requested by pablomartin4btc here: https://github.com/bitcoin/bitcoin/pull/30808#pullrequestreview-2280450117. To test the test you can comment out the content of the destructor of `NetworkDisable`. It also addresses the feedback by ryanofsky to use `std::optional` instead of `std::unique_ptr` for the management of the RAII object: https://github.com/bitcoin/bitcoin/pull/30808#discussion_r1744149228 ACKs for top commit: achow101: ACK c2b779da4e7f1bf1a5c5d67ec94cba3027b42ee7 pablomartin4btc: cr & tACK c2b779da4e7f1bf1a5c5d67ec94cba3027b42ee7 tdb3: ACK c2b779da4e7f1bf1a5c5d67ec94cba3027b42ee7 BrandonOdiwuor: Code Review ACK c2b779da4e7f1bf1a5c5d67ec94cba3027b42ee7 theStack: ACK c2b779da4e7f1bf1a5c5d67ec94cba3027b42ee7 Tree-SHA512: 9556e75014a2599bb870b70faf887608b332f2312626333f771d4ec11c04f863a2cf17e223ec473d4e8b0c9e8008394a4e0c321561f7ef3a2eec713dcfaea58a
2024-09-09Merge bitcoin/bitcoin#30684: init: fix init fatal error on invalid negated ↵Ava Chow
option value ee47ca29d6ef55650a0af63bca817c5d494f31ef init: fix fatal error on '-wallet' negated option value (furszy) Pull request description: Currently, if users provide a double negated value such as '-nowallet=0' or a non-boolean convertible value to a negated option such as '-nowallet=not_a_boolean', the initialization process results in a fatal error, causing an unclean shutdown and displaying a poorly descriptive error message: "JSON value of type bool is not of expected type string." (On bitcoind. The GUI does not display any error msg - upcoming PR -). This PR fixes the issue by ensuring that only string values are returned in the the "wallet" settings list, failing otherwise. It also improves the clarity of the returned error message. Note: This bug was introduced in https://github.com/bitcoin/bitcoin/pull/22217. Where the `GetArgs("-wallet")` call was replaced by `GetSettingsList("-wallet")`. ACKs for top commit: achow101: ACK ee47ca29d6ef55650a0af63bca817c5d494f31ef ryanofsky: Code review ACK ee47ca29d6ef55650a0af63bca817c5d494f31ef, just adding the suggested test since last review TheCharlatan: ACK ee47ca29d6ef55650a0af63bca817c5d494f31ef ismaelsadeeq: Tested ACK ee47ca29d6ef55650a0af63bca817c5d494f31ef Tree-SHA512: 5f01076f74a048019bb70791160f0accc2db7a457d969cb23687bed81ccbbdec1dda68311e7c6e2dd56250e23e8d926d4066e5014b2a99a2fc202e24ed264fbd
2024-09-09Merge bitcoin/bitcoin#30401: fix: increase consistency of rpcauth parsingAva Chow
27c976d11a68d66db97d9a7a30c6a6a71c6ab586 fix: increase consistency of rpcauth parsing (tdb3) 2ad3689512a36eaff957df9ac28e65b2fedbc36c test: add norpcauth test (tdb3) 67df0dec1abe547773e532aa60aff0317e018123 test: blank rpcauth CLI interaction (tdb3) ecc98ccff25b7e758337e764e59d764076772fec test: add cases for blank rpcauth (tdb3) Pull request description: The current `rpcauth` parsing behavior is inconsistent and unintuitive (see https://github.com/bitcoin/bitcoin/pull/29141#issuecomment-1972085251 and additional details below). The current behavior inconsistently treats empty `rpcauth` as an error (or not) depending on the location within CLI/bitcoin.conf and the location of adjacent valid `rpcauth` params. Empty `rpcauth` is now consistently treated as an error and prevents bitcoind from starting. Continuation of the upforgrabs PR #29141. ### Additional details: Current `rpcauth` behavior is nonsensical: - If an empty `rpcauth` argument was specified as the last command line argument, it would cause all other `rpcauth` arguments to be ignored. - If an empty `rpcauth` argument was specified on the command line followed by any nonempty `rpcauth` argument, it would cause an error. - If an empty `rpcauth=` line was specified after non-empty rpcauth line in the config file it would cause an error. - If an empty `rpcauth=` line in a config file was first it would cause other rpcauth entries in the config file to be ignored, unless there were `-rpcauth` command line arguments and the last one was nonempty, in which case it would cause an error. New behavior is simple: - If an empty rpcauth config line or command line argument is used it will cause an error ACKs for top commit: naiyoma: Tested ACK [https://github.com/bitcoin/bitcoin/commit/27c976d11a68d66db97d9a7a30c6a6a71c6ab586](https://github.com/bitcoin/bitcoin/commit/27c976d11a68d66db97d9a7a30c6a6a71c6ab586) achow101: ACK 27c976d11a68d66db97d9a7a30c6a6a71c6ab586 ryanofsky: Code review ACK 27c976d11a68d66db97d9a7a30c6a6a71c6ab586. Since last review commit message was just tweaked to clarify previous behavior. Tree-SHA512: af2e9dd60d1ad030409ae2c3805ab139c7435327823d9f8bbeede815f376cb696a5929b08a6e8c8b5f7278ed49cfb231789f9041bd57f1f03ec96501b669da5b
2024-09-05test: Add coverage for failing dumptxoutset behaviorFabian Jahr
In case of a failure to create the dump, the node should not be left in an inconsistent state like deactivated network activity or an invalidated blockchain.
2024-09-04Merge bitcoin/bitcoin#30244: ci: parse TEST_RUNNER_EXTRA into an arrayAva Chow
8131bf7483c0ea10d3573c9f2e977d19d8569b7f ci: parse TEST_RUNNER_EXTRA into an array (Max Edwards) c4762b0aa06f2654d108bc7ca05887ffd88cf6f8 test: allow excluding func test by name and arg (Max Edwards) Pull request description: While working on CI I wanted to disable some functional tests so I used the `TEST_RUNNER_EXTRA` var. The problem I had was tests that have flags such as `rpc_bind.py --ipv6` must be passed in quotes otherwise the `--ipv6` portion will be considered an argument to `test_runner.py` rather than a test name. This change allows proper parsing of quotes and complex values such as: ```shell TEST_RUNNER_EXTRA='--exclude "rpc_bind.py --ipv6,feature_proxy.py"' ``` Update: While testing this it was noticed that `test_runner.py` when given `--exclude "rpc_bind.py --ipv6"` will exclude all `rpc_bind.py` tests so this PR has been updated to include a change to the test runner to only exclude the specific test if you pass an arg or exclude all tests of that name if you do not pass an arg. `--exclude rpc_bind.py` will exclude all three variants and `--exclude rpc_bind --ipv6` will only exclude the IPV6 variant. ACKs for top commit: maflcko: ACK 8131bf7483c0ea10d3573c9f2e977d19d8569b7f achow101: ACK 8131bf7483c0ea10d3573c9f2e977d19d8569b7f hebasto: ACK 8131bf7483c0ea10d3573c9f2e977d19d8569b7f, tested on Ubuntu 23.10 and Windows 11. Tree-SHA512: 82b73f12d627f533d8e5be4a518d455ef4427a755bbe03ccd11d0bb70c7ff3cee76220b0264fcfb236661c4cf5deba034cbfc2372b96d5861f3436c21eae8264
2024-09-04Merge bitcoin/bitcoin#30723: lint: Speed up and fix flake8 checksAva Chow
fafdb7df34507eee735893aa871da6ae529e6372 lint: Speed up flake8 checks (MarcoFalke) faf17df7fb88590d936d10c471a9ea6a2ce4454d lint: Document missing py_lint dependency (MarcoFalke) faebeb828f5f0ec68d90e7f76add66bc562f6fa3 lint: Remove python whitespace and shadowing lint rules (MarcoFalke) 77770478355ce6c1ab077dbc12ec898875ec5620 lint: Remove python lint rules that are SyntaxError (MarcoFalke) faaf3e53f09c73278e36674db0af14a262f0bd94 test: [refactor] Fix F841 flake8 (MarcoFalke) 444421db69539b74077306b6d0cb23e82afeb891 test: [refactor] Fix E714 pycodestyle (MarcoFalke) Pull request description: The checks have many issues: * Some checks that could in theory hide bugs are not applied -> Fix them and apply them going forward * Some checks are redundant Python 2 checks, or of low value -> Remove them * The checks are slow -> Speed them up from ~10 seconds to about ~20 milliseconds ACKs for top commit: davidgumberg: review and tested reACK https://github.com/bitcoin/bitcoin/commit/fafdb7df34507eee735893aa871da6ae529e6372 kevkevinpal: ACK [fafdb7d](https://github.com/bitcoin/bitcoin/pull/30723/commits/fafdb7df34507eee735893aa871da6ae529e6372) achow101: ACK fafdb7df34507eee735893aa871da6ae529e6372 Tree-SHA512: a0488b722cfaf7071bd6848cd3be002e0b6c38af80d8b5cbb08613c0b174ef63277289f960db8ac31adb09fe563a4973203b8fb10b83cbcfdc6f0ef39bd04410
2024-09-04Merge bitcoin/bitcoin#30148: cli: restrict multiple exclusive argument usage ↵Ava Chow
in bitcoin-cli c8e6771af002eaf15567794fcdc57fdb0e3fb140 test: restrict multiple CLI arguments (naiyoma) 8838c4f171f3c3e568d237b216167fddb521d0a7 common/args.h: automate check for multiple cli commands (naiyoma) Pull request description: This PR is a continuation of the validation suggested [here](https://github.com/bitcoin/bitcoin/pull/27815) to ensure that only one Request Handler can be specified at a time. ACKs for top commit: stratospher: reACK c8e6771. achow101: ACK c8e6771af002eaf15567794fcdc57fdb0e3fb140 tdb3: cr re ACK c8e6771af002eaf15567794fcdc57fdb0e3fb140 Tree-SHA512: f4fe036fee342a54f1a7ac702ac35c40bf3d420fde6ab16313a75327292d5ee5c8ece1be9f852a13fcf73da1148b143b37b4894e294052abdde6eefb2e8c6f3f
2024-09-04Merge bitcoin/bitcoin#29566: test: update satoshi_round functionAva Chow
ec317bc44b0cb089419d809b5fef38ecb9423051 test: update satoshi_round function (naiyoma) Pull request description: This PR refactors `satoshi_round` to accept different rounding modes and make rounding a required argument. Continuation of https://github.com/bitcoin/bitcoin/pull/23225 ACKs for top commit: maflcko: review ACK ec317bc44b0cb089419d809b5fef38ecb9423051 achow101: ACK ec317bc44b0cb089419d809b5fef38ecb9423051 AngusP: ACK ec317bc44b0cb089419d809b5fef38ecb9423051 Tree-SHA512: 070f0aa6f66e58bff7412cae6b71f5f4ab8c718c7b5eeba4bb604fe22c6416a1ced0474294f504e1c28943ddc073104466b5944b43bae27e42bee5ca85afa468
2024-09-04Merge bitcoin/bitcoin#28417: contrib/signet/miner updatesAva Chow
fb6d51eb25a2bb69a3ecdfc4796a88d4d1aacc65 signet/miner: Use argparse exclusive groups (Anthony Towns) 338a266a9a08e47bc6dd02175c8fa649f701515d signet/miner: add support for a poolnum/poolid tag in mined blocks (Anthony Towns) 409ab7d35b3cfa255a83e1004c55691515e4e3f5 signet/miner: add Generate.mine function (Anthony Towns) 7b3133237072a77231b38e59d619fd50fa769a6f signet/miner: add Generate.gbt function (Anthony Towns) 85c5c0bea9d45e93a9fb20988457480798d68637 signet/miner: add Generate.next_block_time function (Anthony Towns) 5540e6ca4930f99a1c0a1ee7b6e1c6ed75f95b55 signet/miner: move next_block_* functions into new Generator class (Anthony Towns) 35f46311969261f42727de4faac38dd9651f8d78 signet/miner: rename do_decode_psbt to decode_psbt (Anthony Towns) aac040b439fd917274eabfc81b89eb6ed2fee325 signet/miner: drop create_coinbase function (Anthony Towns) 16951f549eba8540311d52c4ee387714ea9f7d4c signet/miner: drop do_createpsbt function (Anthony Towns) 3aed0a4284d1d44144649140e947aef6943d2967 signet/miner: drop get_reward_address function (Anthony Towns) Pull request description: Refactors the code a bunch, and adds `--poolnum` / `--poolid` options so that signers can tag their coinbases in a way that explorers can recognise (see also https://github.com/bitcoin-data/mining-pools/pull/82 and https://github.com/mempool/mempool/issues/2903). The refactoring in particular helps enable the "try using inquisition's getblocktemplate, and if that doesn't work fall back to core's getblocktemplate" logic, as described/implemented in https://github.com/bitcoin-inquisition/bitcoin/pull/7 ACKs for top commit: achow101: ACK fb6d51eb25a2bb69a3ecdfc4796a88d4d1aacc65 danielabrozzoni: Code review ACK fb6d51eb25a2bb69a3ecdfc4796a88d4d1aacc65 Tree-SHA512: d84095c4045ab196685b847e04ce2cdaedf387bc2527430ede918318dc5b70bf3d87b754264016f895f506fac70d4fdea5ef3cd8c3c375fd586afeae01e045e5
generated by cgit v1.2.3 (git 2.26.2) at 2024-10-06 00:52:08 +0000