aboutsummaryrefslogtreecommitdiff
path: root/test
AgeCommit message (Collapse)Author
2024-01-24Merge bitcoin/bitcoin#29304: fuzz: Exit and log stderr for parse_test_list ↵fanquake
errors 9d09c873a50d344e2a9cb35fe246a52688b9fa34 fuzz: Exit and log stderr for parse_test_list errors (dergoegge) Pull request description: We should log all errors that occur when attempting to print the harness list in the fuzz test runner. ACKs for top commit: maflcko: lgtm ACK 9d09c873a50d344e2a9cb35fe246a52688b9fa34 Tree-SHA512: 50471b732c8cbe287dacba14487e7c8a5826f146432d93aa3bb55d063a8ba158d01641d6cb1360241dd4cd54ef5e045b0412f9cc34d06c181134921d1f1ceced
2024-01-24fuzz: Exit and log stderr for parse_test_list errorsdergoegge
2024-01-23Merge bitcoin/bitcoin#28560: wallet, rpc: `FundTransaction` refactorAva Chow
18ad1b9142e91cef2f5c6a693eeb2d0fbb8c517d refactor: pass CRecipient to FundTransaction (josibake) 5ad19668dbcc47486d1c18f711cea3d8a9d2e7e2 refactor: simplify `CreateRecipients` (josibake) 47353a608dc6e20e5fd2ca53850d6f9aa3240d4a refactor: remove out param from `ParseRecipients` (josibake) f7384b921c3460c7a3cc7827a68b2c613bd98f8e refactor: move parsing to new function (josibake) 6f569ac903e5ddaac275996a5d0c31b2220b7b81 refactor: move normalization to new function (josibake) 435fe5cd96599c518e26efe444c9d94d1277996b test: add tests for fundrawtx and sendmany rpcs (josibake) Pull request description: ## Motivation The primary motivation for this PR is to enable `FundTransaction` to take a vector of `CRecipient` objects to allow passing BIP352 silent payment addresses to RPCs that use `FundTransaction` (e.g. `send`, `walletcreatefundedpsbt`). To do that, SFFO logic needs to be moved out of `FundTransaction` so the `CRecipient` objects with the correct SFFO information can be created and then passed to `FundTransaction`. As a secondary motivation, this PR moves the SFFO stuff closer to the caller, making the code cleaner and easier to understand. This is done by having a single function which parses RPC inputs for SFFO and consistently using the `set<int>` method for communicating SFFO. I'm also not convinced we need to pass a full `CMutableTx` object to `FundTransaction`, but I'm leaving that for a follow-up PR/discussion, as its not a blocker for silent payments. ACKs for top commit: S3RK: reACK 18ad1b9142e91cef2f5c6a693eeb2d0fbb8c517d josibake: > According to my `range-diff` nothing changed. reACK [18ad1b9](https://github.com/bitcoin/bitcoin/commit/18ad1b9142e91cef2f5c6a693eeb2d0fbb8c517d) achow101: ACK 18ad1b9142e91cef2f5c6a693eeb2d0fbb8c517d Tree-SHA512: d61f017cf7d98489ef216475b68693fd77e7b53a26a6477dcd73e7e5ceff5036b2d21476e377839e710bb73644759d42c4f9f4b14ed96b3e56ed87b07aa6d1a7
2024-01-23Merge bitcoin/bitcoin#29144: init: handle empty settings file gracefullyAva Chow
e9014042a6bed8c16cc9a31fc35cb709d4b3c766 settings: add auto-generated warning msg for editing the file manually (furszy) 966f5de99a9f5da05c91378ad1e8ea8ed37ac3b3 init: improve corrupted/empty settings file error msg (furszy) Pull request description: Small and simple issue reported [here](https://community.umbrel.com/t/bitcoin-docker-container-keeps-restarting/2144). Improving a confusing situation reported by users who did not understand why a settings parsing error occurred when the file was empty and did not know how to solve it. Empty setting file could be due (1) corruption or (2) an user manually cleaning up the file content. In both scenarios, the 'Unable to parse settings file' error does not help the user move forward. ACKs for top commit: achow101: ACK e9014042a6bed8c16cc9a31fc35cb709d4b3c766 hebasto: re-ACK e9014042a6bed8c16cc9a31fc35cb709d4b3c766. ryanofsky: Code review ACK e9014042a6bed8c16cc9a31fc35cb709d4b3c766. Just whitespace formatting changes and shortening a test string literal since last review shaavan: Code review ACK e9014042a6bed8c16cc9a31fc35cb709d4b3c766 Tree-SHA512: 2910654c6b9e9112de391eedb8e46980280f822fa3059724dd278db7436804dd27fae628d2003f2c6ac1599b07ac5c589af016be693486e949f558515e662bec
2024-01-23Merge bitcoin/bitcoin#29272: wallet: fix coin selection tracing to return -1 ↵Ava Chow
when no change pos d55fdb1a495190e213b1b5127f5d91e4a409765e Move TRACEx parameters to seperate lines (Richard Myers) 2d58629ee63eebc760e2a9226afcd0c46d3ec2bd wallet: fix coin selection tracing to return -1 when no change pos (Richard Myers) Pull request description: This is a bugfix for from when [optional was introduced](https://github.com/bitcoin/bitcoin/pull/25273/commits/758501b71391136c33b525b1a0109b990d4f463e) for `change_pos` in the wallet. When optional `change_pos` is unset, we should return -1 and not 0. I added two new checks to the `test/functional/interface_usdt_coinselection.py` which adds coverage for the situations when `normal_create_tx_internal` and `aps_create_tx_internal` events occur with no change. You can reproduce this bug using the coin-selection-simulation scripts as described in [issue #16](https://github.com/achow101/coin-selection-simulation/issues/16). You can also run the `interface_usdt_coinselection.py` test without the changes to `wallet/spend.cpp`. ACKs for top commit: 0xB10C: ACK d55fdb1a495190e213b1b5127f5d91e4a409765e achow101: ACK d55fdb1a495190e213b1b5127f5d91e4a409765e murchandamus: ACK d55fdb1a495190e213b1b5127f5d91e4a409765e Tree-SHA512: 6efac3b756bdf51debbcb759dc3c4b7a4304626bc047b70025cec02f3a04937ace7712e9558ac71e560fd136005a98c518ac5bb4b90c3282d776beccd0de9749
2024-01-22settings: add auto-generated warning msg for editing the file manuallyfurszy
Hopefully, refraining users from modifying the file unless they are certain about the potential consequences. Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2024-01-22init: improve corrupted/empty settings file error msgfurszy
The preceding "Unable to parse settings file" message lacked the necessary detail and guidance for users on what steps to take next in order to resolve the startup error. Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2024-01-20wallet: fix coin selection tracing to return -1 when no change posRichard Myers
2024-01-19test: add tests for fundrawtx and sendmany rpcsjosibake
If the serialized transaction passed to `fundrawtransaction` contains duplicates, they will be deserialized and added to the transaction. Add a test to ensure this behavior is not changed during the refactor. A user can pass any number of duplicated and unrelated addresses as an SFFO argument to `sendmany` and the RPC will not throw an error (note, all the rest of the RPCs which take SFFO as an argument will error if the user passes duplicates or specifies outputs not present in the transaction). Add a test to ensure this behavior is not changed during the refactor.
2024-01-18Merge bitcoin/bitcoin#29262: rpc: Fix race in loadtxoutsetAva Chow
5555d8db3313f893609eb0cf549bb597361d4466 test: Use blocks_path where possible (MarcoFalke) fa9108941fa1a0e83484114e2d8a99d264c2ad09 rpc: Fix race in loadtxoutset (MarcoFalke) Pull request description: The tip may have advanced, also if it did not, there is no reason to have two variables point to the same block. Fixes https://github.com/bitcoin/bitcoin/pull/27596#discussion_r1344694600 ACKs for top commit: achow101: ACK 5555d8db3313f893609eb0cf549bb597361d4466 pablomartin4btc: ACK 5555d8db3313f893609eb0cf549bb597361d4466 BrandonOdiwuor: Code Review ACK 5555d8db3313f893609eb0cf549bb597361d4466 Tree-SHA512: 23a82924a915b61bb1adab8ad20ec8914139c8ee647817af34ca27ee310a2e45833d8b285503e0feebe63e4667193d6d98cfcbbc1509bf40712225e04dd19e8b
2024-01-18Merge bitcoin/bitcoin#29228: test: Remove all-lint.py scriptAva Chow
fa2b95cf3f5148d27a8fd4fb3763ca1fc139bdd9 test: Remove all-lint.py script (MarcoFalke) fadb06c361bbda78192d89fd06195f0aa5418c56 doc: move-only lint docs to one place (MarcoFalke) Pull request description: Seems confusing to have a test runner that calls another runner (`all-lint.py`), which calls a subset of the lint tests. Fix that by just calling this subset of lint tests in the test runner directly, and remove the now unused `all-lint.py`. To run all lint checks locally, refer to the documentation: https://github.com/bitcoin/bitcoin/blob/master/test/lint/README.md#running-locally ACKs for top commit: kevkevinpal: ACK [fa2b95c](https://github.com/bitcoin/bitcoin/pull/29228/commits/fa2b95cf3f5148d27a8fd4fb3763ca1fc139bdd9) achow101: ACK fa2b95cf3f5148d27a8fd4fb3763ca1fc139bdd9 TheCharlatan: ACK fa2b95cf3f5148d27a8fd4fb3763ca1fc139bdd9 pablomartin4btc: tACK fa2b95cf3f5148d27a8fd4fb3763ca1fc139bdd9 brunoerg: utACK fa2b95cf3f5148d27a8fd4fb3763ca1fc139bdd9 Tree-SHA512: 43fac9acb4e9a6744d695dd49c7202e19ab4bf480f4cccff768647d0157a065f40e6ad70b9f6a65ba42048cc5fa9834365aa8e7aa0ed64c09e0cd4eb8dccb831
2024-01-18Merge bitcoin/bitcoin#29085: refactor: C++20: Use std::rotlfanquake
60446285436da62adef1c0a9b11c3336d82b4d89 crypto, hash: replace custom rotl32 with std::rotl (Fabian Jahr) Pull request description: While exploring some C++20 changes and checking against our code I found this potential improvement: 1. We can replace our custom implementation of `rotl32` in crypto/chacha20 with `std::rotl` from the [new `bit` header](https://en.cppreference.com/w/cpp/header/bit). ACKs for top commit: fanquake: ACK 60446285436da62adef1c0a9b11c3336d82b4d89 Tree-SHA512: db55b366f20fca2ef62e5f10a838f8a709d531678c35c1dba20898754029c788a2fd47995208ed6d187cf814109a7ca397bc2c301504500aee79da04c95d6895
2024-01-17test: Use blocks_path where possibleMarcoFalke
2024-01-16Merge bitcoin/bitcoin#29239: rpc: Make v2transport default for addnode RPC ↵Ava Chow
when enabled 3ba815b42db74804e341ce15f648c2b297af55ca Make v2transport default for addnode RPC when enabled (Pieter Wuille) Pull request description: Since #29058, several types of manually configured connections will attempt v2 connections when `-v2transport` is enabled, except for the `addnode` RPC, as that one has an explicit argument to enable or disable. Make the default for that RPC match the `-v2transport` setting so the behavior matches that of other manual connections from a user perspective. ACKs for top commit: achow101: ACK 3ba815b42db74804e341ce15f648c2b297af55ca kristapsk: ACK 3ba815b42db74804e341ce15f648c2b297af55ca theStack: Code-review ACK 3ba815b42db74804e341ce15f648c2b297af55ca Tree-SHA512: 31ef48cf1e533abb17866020378c004df929e626074dc98b3229fb60a66de58435e95c8fda8d1b463e1208aa39d1f42d239818e7e58595a3738089920598befc
2024-01-16Merge bitcoin/bitcoin#29179: test: wallet rescan with reorged parent + ↵Ava Chow
IsFromMe child in mempool df30247705940c50c5eaafd74e2abbeb8b0cec07 [test] import descriptor wallet with reorged parent + IsFromMe child in mempool (glozow) c3d02be536ac3f35c10efa03653186a17ebbfc12 [test] rescan legacy wallet with reorged parent + IsFromMe child in mempool (Gloria Zhao) Pull request description: Originally motivated by #29019, which reverts back to having `requestMempoolTransactions` emit `transactionAddedToMempool` in `mapTx` default order instead of `GetSortedDepthAndScore` order. It's important that these notifications happen in topological order, otherwise the wallet rescan may miss transactions that belong to it. Notably, checking whether a transaction `IsFromMe` requires knowing its inputs, which may be from a mempool parent. When using `mapTx` order, a parent may come later than its child if it was added from a block disconnected in a reorg. This PR adds a test for this case. ACKs for top commit: achow101: ACK df30247705940c50c5eaafd74e2abbeb8b0cec07 furszy: Code review ACK df30247705, nits can be disregarded. Tree-SHA512: 2f1d9ef92313228adbbef94e634e5f7a9ec6e6a2c88e16aa343bdc95ffc9b9f9c82a569b412c9a3841db9d789e52f9283e8b9385731668d59355903e26e58a5d
2024-01-16test: Remove all-lint.py scriptMarcoFalke
2024-01-16doc: move-only lint docs to one placeMarcoFalke
Can be reviewed with --color-moved=dimmed-zebra
2024-01-12[test] import descriptor wallet with reorged parent + IsFromMe child in mempoolglozow
Test that wallet rescans process transactions topologically, even if a parent's entry into the mempool is later than that of its child. This behavior is important because IsFromMe requires the ability to look up a transaction's inputs. Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
2024-01-12Make v2transport default for addnode RPC when enabledPieter Wuille
2024-01-12Merge bitcoin/bitcoin#28885: mempool / rpc: followup to ↵glozow
getprioritisedtransactions and delete a mapDeltas entry when delta==0 0eebd6fe7d01ddc7f6b7f13a6ed6e705c7aeae4e test: Assert that a new tx with a delta of 0 is never added (kevkevin) cfdbcd19b32fd63954d7947dcc639aef291fb6b2 rpc: exposing modified_fee in getprioritisedtransactions (kevkevin) 252a86729a15e47ed168d8da7c4a8d6113673909 rpc: renaming txid -> transactionid (kevkevin) 2fca6c2dd03c3955d86efb0b8d2a7961e42115fd rpc: changed prioritisation-map -> "" (kevkevin) 3a118e19e100110300d3290d4c1434f963721d94 test: Directly constructing 2 entry map for getprioritisedtransactions (kevkevin) Pull request description: In this PR I am addressing some comments in https://github.com/bitcoin/bitcoin/pull/27501 as a followup. - changed `prioritisation-map` in the `RPCResult` to `""` - Directly constructing 2 entry map for getprioritisedtransactions in functional tests - renamed `txid` to `transactionid` in `RPCResult` to be more consistent with naming elsewhere - exposed the `modified_fee` field instead of having it be a useless arg - Created a new test that asserts when `prioritisedtransaction` is called with a fee_delta of 0 it is not added to mempool ACKs for top commit: glozow: reACK 0eebd6fe7d01ddc7f6b7f13a6ed6e705c7aeae4e, only change is the doc suggestion Tree-SHA512: e99056e37a8b1cfc511d87c83edba7c928b50d2cd6c2fd7c038976779850677ad37fddeb2b983e8bc007ca8567eb21ebb78d7eae9b773657c2b297299993ec05
2024-01-12[test] rescan legacy wallet with reorged parent + IsFromMe child in mempoolGloria Zhao
Test that wallet rescans process transactions topologically, even if a parent's entry into the mempool is later than that of its child. This behavior is important because IsFromMe requires the ability to look up a transaction's inputs.
2024-01-11Merge bitcoin/bitcoin#29034: test: detect OS in functional tests ↵Ava Chow
consistently using `platform.system()` 878d914777a03a04ecb84217152e8b7fd73a5062 doc: test: mention OS detection preferences in style guideline (Sebastian Falbesoner) 4c65ac96f8b021c107783adce3e8afe4f8edee6e test: detect OS consistently using `platform.system()` (Sebastian Falbesoner) 37324ae3dfb0e50daaf752dc863a880559fa4637 test: use `skip_if_platform_not_linux` helper where possible (Sebastian Falbesoner) Pull request description: There are at least three ways to detect the operating system in Python3: - `os.name` (https://docs.python.org/3.9/library/os.html#os.name) - `sys.platform` (https://docs.python.org/3.9/library/sys.html#sys.platform) - `platform.system()` (https://docs.python.org/3.9/library/platform.html#platform.system) We are currently using all of them in functional tests (both in individual tests and shared test framework code), which seems a bit messy. This PR consolidates into using `platform.system()`, as it appears to be one most consistent and easy to read (see also [IRC discussion](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2023-12-08#989301;) and table below). `sys.platform` is inconsistent as it has the major version number encoded for BSD systems, which doesn't make much sense for e.g. OpenBSD, where there is no concept of major versions, but instead the version is simply increased by 0.1 on each release. Note that `os.name` is still useful to detect whether we are running a POSIX system (see `BitcoinTestFramework.skip_if_platform_not_posix`), so for this use-case it is kept as only exception. The following table shows values for common operating systems, found via ``` $ python3 -c "import os; import sys; import platform; print(os.name, sys.platform, platform.system())" ``` | OS | os.name | sys.platform | platform.system() | |--------------|---------|--------------|--------------------| | Linux 6.2.0 | posix | linux | Linux | | MacOS* | posix | darwin | Darwin | | OpenBSD 7.4 | posix | openbsd7 | OpenBSD | | Windows* | nt | win32 | Windows | \* = I neither have a MacOS nor a Windows machine available, so I extracted the values from documentation and our current code. Also I'm relying on CI for testing the relevant code-paths. Having reviewers to this this locally would be very appreciated, if this gets Concept ACKed. ACKs for top commit: kevkevinpal: ACK [878d914](https://github.com/bitcoin/bitcoin/pull/29034/commits/878d914777a03a04ecb84217152e8b7fd73a5062) achow101: ACK 878d914777a03a04ecb84217152e8b7fd73a5062 hebasto: ACK 878d914777a03a04ecb84217152e8b7fd73a5062, I have reviewed the code and it looks OK. pablomartin4btc: tACK 878d914777a03a04ecb84217152e8b7fd73a5062 Tree-SHA512: 24513d493e47f572028c843260b81c47c2c29bfb701991050255c9f9529cd19065ecbc7b3b6e15619da7f3f608b4825c345ce6fee30d8fd1eaadbd08cff400fc
2024-01-11Merge bitcoin/bitcoin#28838: test: add assumeutxo wallet testAva Chow
997b9a73e5166b4244f7c5b4fe144d524f3005f4 test: add assumeutxo wallet test (Sjors Provoost) Pull request description: Extracted from #28616, this adds a (very) basic wallet test for assume utxo. It checks some circumstances where a backup can and can't be loaded. ACKs for top commit: maflcko: lgtm ACK 997b9a73e5166b4244f7c5b4fe144d524f3005f4 achow101: ACK 997b9a73e5166b4244f7c5b4fe144d524f3005f4 theStack: Code-review ACK 997b9a73e5166b4244f7c5b4fe144d524f3005f4 Tree-SHA512: 69474e56c6a46bb4f30fc54f8e5844766ac2a5f8226bb0b168d11ae1e3d4eae58570c1f1b4cc2b2f6f51b5d0e055bbe2bbd11684265215e01d4eb81ab4b7b0bb
2024-01-11test: Assert that a new tx with a delta of 0 is never addedkevkevin
2024-01-11rpc: exposing modified_fee in getprioritisedtransactionskevkevin
Instead of having modified_fee be hidden we are now exposing it to avoid having useless code
2024-01-10Merge bitcoin/bitcoin#29204: test: wallet migration, add coverage for tx ↵Ava Chow
extra data 016cc807f77a9128d430a0df1edd133521628a33 test: wallet migration, add coverage for tx extra data (furszy) Pull request description: Quick follow-up to #28610, coming from https://github.com/bitcoin/bitcoin/pull/28610#pullrequestreview-1802823938. Verifying that the 'replaced_by_txid' and 'replaces_txid' tx data is preserved after migration, as well as the extra tx comments. ACKs for top commit: jamesob: Nice, ACK https://github.com/bitcoin/bitcoin/pull/29204/commits/016cc807f77a9128d430a0df1edd133521628a33 achow101: ACK 016cc807f77a9128d430a0df1edd133521628a33 pablomartin4btc: ACK 016cc807f77a9128d430a0df1edd133521628a33 BrandonOdiwuor: lgtm ACK 016cc807f77a9128d430a0df1edd133521628a33 Tree-SHA512: 697cabece730cbe5c5947bf98455e80a8877c0352fbe2a66362ce5ea530b67882b0bec561a67d48fee200cdad717cd62c57fd809e2a94ff83c3fad30021e1d9e
2024-01-10Merge bitcoin/bitcoin#28318: logging: Simplify API for level based loggingAva Chow
e60fc7d5d34f23cccbff6e4f5f3d716fa8dad50c logging: Replace uses of LogPrintfCategory (Anthony Towns) f7ce5ac08c669ac763e275bb7c82dcfb2b1b6c33 logging: add LogError, LogWarning, LogInfo, LogDebug, LogTrace (Anthony Towns) fbd7642c8e5b70327e019382320f5ef0a651ecc5 logging: add -loglevelalways=1 option (Anthony Towns) 782bb6a05663ad7a53908e910d0f42b49b881e09 logging: treat BCLog::ALL like BCLog::NONE (Anthony Towns) 667ce3e3297645527b07314e1d5a82275fb25845 logging: Drop BCLog::Level::None (Anthony Towns) ab34dc6012351e7b8aab871dd9d2b38ade1cd9bc logging: Log Info messages unconditionally (Anthony Towns) dfe98b6874da04e45f68d17575c1e8a5431ca9bc logging: make [cat:debug] and [info] implicit (Anthony Towns) c5c76dc615677d226c9f6b3f2b66d833315d40da logging: refactor: pull prefix code out (Anthony Towns) Pull request description: Replace `LogPrint*` functions with severity based logging functions: * `LogInfo(...)`, `LogWarning(...)`, `LogError(...)` for unconditional (uncategorised) logging (replaces `LogPrintf`) * `LogDebug(CATEGORY, ...)` and `LogTrace(CATEGORY, ...)` for conditional logging (replaces `LogPrint`) * `LogPrintLevel(CATEGORY, LEVEL, ...)` for when the level isn't known in advance, or a category needs to be added for an info/warning/error log message (mostly unchanged, but rarely needed) Logs look roughly as they do now with `LogInfo` not having an `[info]` prefix, and `LogDebug` having a `[cat]` prefix, rather than a `[cat:debug]` prefix. This removes `BCLog::Level::None` entirely -- for `LogFlags::NONE` just use `Level::Info`, for any actual category, use `Level::Debug`. Adds docs to developer-notes about when to use which level. Adds `-loglevelalways=1` option so that you get `[net:debug]`, `[all:info]`, `[all:warning]` etc, which might be helpful for automated parsing, or just if you like everything to be consistent. Defaults to off to reduce noise in the default config, and to avoid unnecessary changes on upgrades. Changes the behaviour of `LogPrintLevel(CATEGORY, BCLog::Level::Info, ...)` to be logged unconditionally, rather than only being an additional optional logging level in addition to trace and debug. Does not change the behaviour of `LogPrintLevel(NONE, Debug, ...)` and `LogPrintLevel(NONE, Trace, ...)` being no-ops. ACKs for top commit: maflcko: re-ACK e60fc7d5d34f23cccbff6e4f5f3d716fa8dad50c 🌚 achow101: ACK e60fc7d5d34f23cccbff6e4f5f3d716fa8dad50c stickies-v: ACK e60fc7d5d34f23cccbff6e4f5f3d716fa8dad50c jamesob: ACK e60fc7d5d34f23cccbff6e4f5f3d716fa8dad50c ([`jamesob/ackr/28318.1.ajtowns.logging_simplify_api_for`](https://github.com/jamesob/bitcoin/tree/ackr/28318.1.ajtowns.logging_simplify_api_for)) Tree-SHA512: e7a4588779b148242495b7b6f64198a00c314cd57100affab11c43e9d39c9bbf85118ee2002792087fdcffdea08c84576e20844b3079f27083e26ddd7ca15d7f
2024-01-10test: assumeutxo: spend coin from snapshot chainstate after loadingSebastian Falbesoner
Check that an UTXO that is only available in the snapshot chainstate is also visible to the mempool by submitting a spending transaction.
2024-01-08test: wallet migration, add coverage for tx extra datafurszy
Verifying that the 'replaced_by_txid' and 'replaces_txid' tx data is preserved after migration, as well as the extra tx comments.
2024-01-08Merge bitcoin/bitcoin#28610: wallet: Migrate entire address book entries to ↵fanquake
watchonly and solvables too 406b71abcb72f234ddf9245a3f57e748343c774f wallet: Migrate entire address book entries (Andrew Chow) Pull request description: Not all of the data in an address book entry was being copied to the watchonly and solvables wallets. This includes information such as whether the address was previously spent, and any receive requests that may exist. A test has been added to check that the previously spent information is copied, although it passes without the changes in this PR since this information is also regenerated when a transaction is loaded/added into a wallet. ACKs for top commit: ryanofsky: Code review ACK 406b71abcb72f234ddf9245a3f57e748343c774f. Just suggested change since last review furszy: Code review ACK 406b71ab Tree-SHA512: 13de42b16a1d8524fe0555764744139566b2e7d29741ceffc1158a905dd537136b762330568b3b5cac28cbee1bfd363a20de97d0a6c5296738cb3aa99133945b
2024-01-05crypto, hash: replace custom rotl32 with std::rotlFabian Jahr
2024-01-05Merge bitcoin/bitcoin#28890: rpc: Remove deprecated -rpcserialversionfanquake
fa46cc22bc696e6845915ae91d6b68e36bf4c242 Remove deprecated -rpcserialversion (MarcoFalke) Pull request description: The flag is problematic for many reasons: * It is deprecated * It is a global flag, requiring a restart to change, as opposed to a flag that can be set on each RPC invocation * It may be hidden in config files by accident, hard to debug, causing LND crashes and bugs, see https://github.com/bitcoin/bitcoin/issues/28730#issuecomment-1780940868 * It makes performance improvements harder to implement: https://github.com/bitcoin/bitcoin/pull/17529#issuecomment-556082818 Fix all issues by removing it. If there is a use-case, likely a per-RPC flag can be added, if needed. ACKs for top commit: ajtowns: crACK fa46cc22bc696e6845915ae91d6b68e36bf4c242 TheCharlatan: lgtm ACK fa46cc22bc696e6845915ae91d6b68e36bf4c242 Tree-SHA512: 96ba1c60356ce93954fe5c2a59045771c6d1516ad0d9dc436ef1800a1f1b0153f0d5fb78ca99d53ad54ba25fbce36962bdf1d4325aceedfc8154a61347a6a915
2023-12-20logging: add LogError, LogWarning, LogInfo, LogDebug, LogTraceAnthony Towns
These provide simple and clear ways to write the most common logging operations: LogInfo("msg"); LogDebug(BCLog::LogFlags::NET, "msg"); LogError("msg"); LogWarning("msg"); LogTrace(BCLog::LogFlags::NET, "msg"); For cases where the level cannot be hardcoded, LogPrintLevel(category, level, ...) remains available.
2023-12-17wallet, mempool: propagete `checkChainLimits` error message to walletismaelsadeeq
Update CheckPackageLimits to use util::Result to pass the error message instead of out parameter. Also update test to reflect the error message from `CTxMempool` `CheckPackageLimits` output.
2023-12-15Merge bitcoin/bitcoin#29088: tests: Don't depend on value of ↵Ava Chow
DEFAULT_PERMIT_BAREMULTISIG 7b45744df33c6a4759eae1a3984f389cbac837c2 tests: ensure functional tests set permitbaremultisig=1 when needed (Anthony Towns) 7dfabdcf860c529772a54b0e8fa235cbb4a78b4d tests: test both settings for permitbaremultisig in p2sh tests (Anthony Towns) Pull request description: Update unit and functional tests so that they continue to work if the default for `-permitbaremultisig` is changed. ACKs for top commit: maflcko: lgtm ACK 7b45744df33c6a4759eae1a3984f389cbac837c2 instagibbs: crACK https://github.com/bitcoin/bitcoin/pull/29088/commits/7b45744df33c6a4759eae1a3984f389cbac837c2 ajtowns: > crACK [7b45744](https://github.com/bitcoin/bitcoin/commit/7b45744df33c6a4759eae1a3984f389cbac837c2) achow101: ACK 7b45744df33c6a4759eae1a3984f389cbac837c2 glozow: ACK 7b45744df33c6a4759eae1a3984f389cbac837c2, changed default locally and all tests passed Tree-SHA512: f89f9e2bb11f07662cfd57390196df9e531064e1bd662e1db7dcfc97694394ae5e8014e9d209b9405aa09195bf46fc331b7fba10378065cdb270cbd0669ae904
2023-12-15tests: ensure functional tests set permitbaremultisig=1 when neededAnthony Towns
The mempool_dust and mempool_sigoplimits functional tests both use bare multisig txs, so ensure they're allowed by policy.
2023-12-14Merge bitcoin/bitcoin#28920: wallet: birth time update during tx scanningAva Chow
1ce45baed7dd2da3f1cb85c9c25110e5537451ae rpc: getwalletinfo, return wallet 'birthtime' (furszy) 83c66444d0604f0a9ec3bc3f89d4f1a810b7cda0 test: coverage for wallet birth time interaction with -reindex (furszy) 6f497377aa17cb8a590fd7717fa8ededf4249999 wallet: fix legacy spkm default birth time (furszy) 75fbf444c1e13c6ba0e79a34871534c845a13849 wallet: birth time update during tx scanning (furszy) b4306e3c8db6cbaedc8845c6d21c750b39f682bf refactor: rename FirstKeyTimeChanged to MaybeUpdateBirthTime (furszy) Pull request description: Fixing #28897. As the user may have imported a descriptor with a timestamp newer than the actual birth time of the first key (by setting 'timestamp=now'), the wallet needs to update the birth time when it detects a transaction older than the oldest descriptor timestamp. Testing Notes: Can cherry-pick the test commit on top of master. It will fail there. ACKs for top commit: Sjors: re-utACK 1ce45baed7dd2da3f1cb85c9c25110e5537451ae achow101: ACK 1ce45baed7dd2da3f1cb85c9c25110e5537451ae Tree-SHA512: 10c2382f87356ae9ea3fcb637d7edc5ed0e51e13cc2729c314c9ffb57c684b9ac3c4b757b85810c0a674020b7287c43d3be8273bcf75e2aff0cc1c037f1159f9
2023-12-14Merge bitcoin/bitcoin#29070: test: add TestNode wait_until helperAva Chow
bf0f7dbec6590a54ec890e7a2ca5d85427995334 test: add TestNode wait_until helper (Nikodemas Tuckus) Pull request description: Add `wait_until` method that wraps the `wait_until_helper_internal` call. Closes https://github.com/bitcoin/bitcoin/issues/29029. ACKs for top commit: maflcko: lgtm ACK bf0f7dbec6590a54ec890e7a2ca5d85427995334 mohamedawnallah: LGTM! Code Review ACK https://github.com/bitcoin/bitcoin/commit/bf0f7dbec6590a54ec890e7a2ca5d85427995334 achow101: ACK bf0f7dbec6590a54ec890e7a2ca5d85427995334 BrandonOdiwuor: Code review ACK bf0f7dbec6590a54ec890e7a2ca5d85427995334 Tree-SHA512: 05aab589c814f51a14e1483eb57c10b88385714e3eb2d0973c0ee2877f2b963a76837f34215fe2e6bd1c8d735f5af7dd2098331e1eda28587f39e513bc6e1a6a
2023-12-14Merge bitcoin/bitcoin#29022: Make bitcoin-tx replaceable value optionalAva Chow
98afe7866185ed4157ffc581763e11dc02fcbae0 doc: Update bitcoin-tx replaceable documentation (Kashif Smith) 94feaf2b66d68b3c849375e1d9d3a81c17cd2045 tests: Add unit tests for bitcoin-tx replaceable command (Kashif Smith) c2b836b119eeed8727d73bcca5e95055eb93fb1a bitcoin-tx: Make replaceable value optional (Kashif Smith) Pull request description: This fixes #28638. The issue was originally raised by dooglus, who also suggested the patch found in this code. Additionally, test coverage has been added and documentation has been updated. ACKs for top commit: achow101: ACK 98afe7866185ed4157ffc581763e11dc02fcbae0 pinheadmz: ACK 98afe7866185ed4157ffc581763e11dc02fcbae0 hernanmarino: Tested ACK 98afe7866185ed4157ffc581763e11dc02fcbae0 instagibbs: untested ACK https://github.com/bitcoin/bitcoin/pull/29022/commits/98afe7866185ed4157ffc581763e11dc02fcbae0 Tree-SHA512: ea1384aba7b0014c8cbeb7280d66b1e617d406fb02471dff33873057132b80518c94c7caa4b0426c26d17ce8aa393107de319dde781ace8df72f0314c8c75159
2023-12-13test: add TestNode wait_until helperNikodemas Tuckus
2023-12-12test: Actually fail when a python unit test failsMarcoFalke
2023-12-11Remove deprecated -rpcserialversionMarcoFalke
2023-12-11Merge bitcoin/bitcoin#29041: test: fix intermittent error in rpc_net.py (#29030)fanquake
ea00f982d21aab51001d422225f00626a74db298 test: fix intermittent error in rpc_net.py (#29030) (Sebastian Falbesoner) Pull request description: Asserting for the debug log message "Added connection peer=" is insufficient for ensuring that this new connection will show up in a following getpeerinfo() call, as the debug message is written in the CNode ctor, which means it hasn't necessarily been added to CConnman.m_nodes at this point. Solve this by using the recently introduced `wait_for_new_peer` helper (see #29006, commit 00e0658e77f66103ebdeb29def99dc9f937c049d), which is more robust. Fixes #29030. ACKs for top commit: maflcko: lgtm ACK ea00f982d21aab51001d422225f00626a74db298 Tree-SHA512: dda307949a466fb3b24408a8c213d307e0af2155f2e8b4e52c836a22397f9d218bf9d8c54ca55bae62a96d7566f27167db9311dd8801785c327234783af5ed00
2023-12-09test: fix intermittent error in rpc_net.py (#29030)Sebastian Falbesoner
Asserting for the debug log message "Added connection peer=" is insufficient for ensuring that this new connection will show up in a following getpeerinfo() call, as the debug message is written in the CNode ctor, which means it hasn't necessarily been added to CConnman.m_nodes at this point. Solve this by using the recently introduced `wait_for_new_peer` helper, which is more robust. Fixes #29030.
2023-12-08tests: Add unit tests for bitcoin-tx replaceable commandKashif Smith
2023-12-08test: fix `addnode` functional test failure on OpenBSDSebastian Falbesoner
This is the functional test counterpart of PR #28891 / commit 007d6f0e85bc329040bb405ef6016339518caa66.
2023-12-08doc: test: mention OS detection preferences in style guidelineSebastian Falbesoner
2023-12-08test: detect OS consistently using `platform.system()`Sebastian Falbesoner
2023-12-08test: use `skip_if_platform_not_linux` helper where possibleSebastian Falbesoner
Rather than re-implementing these checks, we can use this test framework's helper (introduced in commit c934087b627f7d368458781944f990b0eb479634, PR #24358) called in a test's `skip_test_if_missing_module` method instead.
2023-12-08Merge bitcoin/bitcoin#29006: test: fix v2 transport intermittent test ↵fanquake
failure (#29002) 00e0658e77f66103ebdeb29def99dc9f937c049d test: fix v2 transport intermittent test failure (#29002) (Sebastian Falbesoner) Pull request description: This PR improves the following fragile construct for detection of a new connection to the node under test in `p2p_v2_transport.py`: https://github.com/bitcoin/bitcoin/blob/6d5790956f45e3de5c6c4ee6fda21878b0d1287b/test/functional/p2p_v2_transport.py#L154-L156 Only relying on the number of peers for that suffers from race conditions, as unrelated previous peers could disconnect at anytime in-between. In the test run in #29002, the following happens: - `getpeerinfo()` is called the first time -> assigned to `num_peers` - **previous peer disconnects**, the node's peer count is now `num_peers - 1` (in most test runs, this happens before the first getpeerinfo call) - new peer connects, the node's peer count is now `num_peers` - the condition that the node's peer count is `num_peers + 1` is never true, test fails Use the more robust approach of watching for an increased highest peer id instead (again using the `getpeerinfo` RPC call), with a newly introduced context manager method `TestNode.wait_for_new_peer()`. Note that for the opposite case of a disconnect, no new method is introduced; this is currently used only once in the test and is also simpler. Still happy to take suggestions for alternative solutions. Fixes #29002. ACKs for top commit: kevkevinpal: Concept ACK [00e0658](https://github.com/bitcoin/bitcoin/pull/29006/commits/00e0658e77f66103ebdeb29def99dc9f937c049d) maflcko: Ok, lgtm ACK 00e0658e77f66103ebdeb29def99dc9f937c049d stratospher: ACK 00e0658. Tree-SHA512: 0118b87f54ea5e6e080ff44f29d6af6674c757a588534b3add040da435f4359e71bf85bc0a5eb7170f99cc9956e1a03c35cce653d642d31eed41bbed1f94f44f
generated by cgit v1.2.3 (git 2.26.2) at 2025-01-21 01:01:50 +0000