6038 Commits

Author SHA1 Message Date
furszy
e9014042a6
settings: add auto-generated warning msg for editing the file manually
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-22 10:50:03 -03:00
furszy
966f5de99a
init: improve corrupted/empty settings file error msg
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-22 10:50:03 -03:00
Richard Myers
2d58629ee6
wallet: fix coin selection tracing to return -1 when no change pos 2024-01-20 14:56:41 +01:00
stickies-v
3bfc5bd36e
test: ensure output is large enough to pay for its fees
Fixes a (rare) intermittency issue in wallet_import_rescan.

Since we use `subtract_fee_from_outputs=[0]` in the `send` command,
the output amount must at least be as large as the fee we're paying.
2024-01-19 14:19:25 +00:00
josibake
435fe5cd96
test: add tests for fundrawtx and sendmany rpcs
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-19 15:04:56 +01:00
Hennadii Stepanov
cbea49c0d3
build: Pass sanitize flags to instrument libsecp256k1 code
Also a new UBSan suppression has been added.
2024-01-19 10:08:41 +00:00
Ava Chow
5f3a0574c4
Merge bitcoin/bitcoin#29262: rpc: Fix race in loadtxoutset
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-18 13:17:35 -05:00
Ava Chow
ac3901ebd0
Merge bitcoin/bitcoin#29228: test: Remove all-lint.py script
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](fa2b95cf3f)
  achow101:
    ACK fa2b95cf3f5148d27a8fd4fb3763ca1fc139bdd9
  TheCharlatan:
    ACK fa2b95cf3f5148d27a8fd4fb3763ca1fc139bdd9
  pablomartin4btc:
    tACK fa2b95cf3f5148d27a8fd4fb3763ca1fc139bdd9
  brunoerg:
    utACK fa2b95cf3f5148d27a8fd4fb3763ca1fc139bdd9

Tree-SHA512: 43fac9acb4e9a6744d695dd49c7202e19ab4bf480f4cccff768647d0157a065f40e6ad70b9f6a65ba42048cc5fa9834365aa8e7aa0ed64c09e0cd4eb8dccb831
2024-01-18 13:02:15 -05:00
fanquake
03c5b0064d
Merge bitcoin/bitcoin#29085: refactor: C++20: Use std::rotl
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-18 09:40:44 +00:00
MarcoFalke
5555d8db33
test: Use blocks_path where possible 2024-01-17 16:48:47 +01:00
Ava Chow
8106b268cd
Merge bitcoin/bitcoin#29239: rpc: Make v2transport default for addnode RPC 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-16 16:50:03 -05:00
Ava Chow
27d935f58b
Merge bitcoin/bitcoin#29179: test: wallet rescan with reorged parent + 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-16 12:49:09 -05:00
MarcoFalke
fa2b95cf3f
test: Remove all-lint.py script 2024-01-16 10:54:42 +01:00
MarcoFalke
fadb06c361
doc: move-only lint docs to one place
Can be reviewed with --color-moved=dimmed-zebra
2024-01-16 10:54:14 +01:00
glozow
df30247705 [test] import descriptor wallet with reorged parent + IsFromMe child in mempool
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-12 14:51:16 +00:00
Pieter Wuille
3ba815b42d Make v2transport default for addnode RPC when enabled 2024-01-12 09:31:31 -05:00
glozow
cd603361a4
Merge bitcoin/bitcoin#28885: mempool / rpc: followup to 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 12:03:52 +00:00
Gloria Zhao
c3d02be536 [test] rescan legacy wallet with reorged parent + IsFromMe child in mempool
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-12 09:54:57 +00:00
Andrew Chow
c11c404281 tests: Test migration of blank wallets 2024-01-11 15:50:05 -05:00
Ava Chow
bb6de1befb
Merge bitcoin/bitcoin#29034: test: detect OS in functional tests 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](878d914777)
  achow101:
    ACK 878d914777a03a04ecb84217152e8b7fd73a5062
  hebasto:
    ACK 878d914777a03a04ecb84217152e8b7fd73a5062, I have reviewed the code and it looks OK.
  pablomartin4btc:
    tACK 878d914777a03a04ecb84217152e8b7fd73a5062

Tree-SHA512: 24513d493e47f572028c843260b81c47c2c29bfb701991050255c9f9529cd19065ecbc7b3b6e15619da7f3f608b4825c345ce6fee30d8fd1eaadbd08cff400fc
2024-01-11 12:17:01 -05:00
Ava Chow
4e104e2381
Merge bitcoin/bitcoin#28838: test: add assumeutxo wallet test
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-11 11:30:18 -05:00
kevkevin
0eebd6fe7d
test: Assert that a new tx with a delta of 0 is never added 2024-01-11 08:16:42 -06:00
kevkevin
cfdbcd19b3
rpc: exposing modified_fee in getprioritisedtransactions
Instead of having modified_fee be hidden we are now exposing it to avoid
having useless code
2024-01-11 08:16:22 -06:00
Ava Chow
fcacbab487
Merge bitcoin/bitcoin#29204: test: wallet migration, add coverage for tx 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 016cc807f7
  achow101:
    ACK 016cc807f77a9128d430a0df1edd133521628a33
  pablomartin4btc:
    ACK 016cc807f77a9128d430a0df1edd133521628a33
  BrandonOdiwuor:
    lgtm ACK 016cc807f77a9128d430a0df1edd133521628a33

Tree-SHA512: 697cabece730cbe5c5947bf98455e80a8877c0352fbe2a66362ce5ea530b67882b0bec561a67d48fee200cdad717cd62c57fd809e2a94ff83c3fad30021e1d9e
2024-01-10 14:35:22 -05:00
Ava Chow
7ff8e6b240
Merge bitcoin/bitcoin#28318: logging: Simplify API for level based logging
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-10 14:11:32 -05:00
Sebastian Falbesoner
931575418e test: assumeutxo: spend coin from snapshot chainstate after loading
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-10 01:18:27 +01:00
furszy
016cc807f7
test: wallet migration, add coverage for tx extra data
Verifying that the 'replaced_by_txid' and 'replaces_txid'
tx data is preserved after migration, as well as the
extra tx comments.
2024-01-08 12:04:31 -03:00
fanquake
c2d04f1319
Merge bitcoin/bitcoin#28610: wallet: Migrate entire address book entries to 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-08 14:44:47 +00:00
Fabian Jahr
6044628543
crypto, hash: replace custom rotl32 with std::rotl 2024-01-05 17:12:38 +01:00
fanquake
143ace65db
Merge bitcoin/bitcoin#28890: rpc: Remove deprecated -rpcserialversion
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
2024-01-05 10:42:10 +00:00
Anthony Towns
f7ce5ac08c logging: add LogError, LogWarning, LogInfo, LogDebug, LogTrace
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-20 15:59:48 +10:00
ismaelsadeeq
8dec9c560b wallet, mempool: propagete checkChainLimits error message to wallet
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-17 21:13:44 +01:00
Ava Chow
3695ecbf68
Merge bitcoin/bitcoin#29088: tests: Don't depend on value of 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 7b45744df3
  ajtowns:
    > crACK [7b45744](7b45744df3)
  achow101:
    ACK 7b45744df33c6a4759eae1a3984f389cbac837c2
  glozow:
    ACK 7b45744df33c6a4759eae1a3984f389cbac837c2, changed default locally and all tests passed

Tree-SHA512: f89f9e2bb11f07662cfd57390196df9e531064e1bd662e1db7dcfc97694394ae5e8014e9d209b9405aa09195bf46fc331b7fba10378065cdb270cbd0669ae904
2023-12-15 16:22:54 -05:00
Anthony Towns
7b45744df3 tests: ensure functional tests set permitbaremultisig=1 when needed
The mempool_dust and mempool_sigoplimits functional tests both use bare
multisig txs, so ensure they're allowed by policy.
2023-12-15 18:37:29 +10:00
Ava Chow
08e6aaabef
Merge bitcoin/bitcoin#28920: wallet: birth time update during tx scanning
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-14 16:27:40 -05:00
Ava Chow
9860471708
Merge bitcoin/bitcoin#29070: test: add TestNode wait_until helper
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 bf0f7dbec6
  achow101:
    ACK bf0f7dbec6590a54ec890e7a2ca5d85427995334
  BrandonOdiwuor:
    Code review ACK bf0f7dbec6590a54ec890e7a2ca5d85427995334

Tree-SHA512: 05aab589c814f51a14e1483eb57c10b88385714e3eb2d0973c0ee2877f2b963a76837f34215fe2e6bd1c8d735f5af7dd2098331e1eda28587f39e513bc6e1a6a
2023-12-14 14:19:16 -05:00
Ava Chow
4d7b787ad6
Merge bitcoin/bitcoin#29022: Make bitcoin-tx replaceable value optional
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 98afe78661

Tree-SHA512: ea1384aba7b0014c8cbeb7280d66b1e617d406fb02471dff33873057132b80518c94c7caa4b0426c26d17ce8aa393107de319dde781ace8df72f0314c8c75159
2023-12-14 13:54:00 -05:00
Nikodemas Tuckus
bf0f7dbec6 test: add TestNode wait_until helper 2023-12-13 11:24:03 +01:00
MarcoFalke
fa0534d7e4
test: Actually fail when a python unit test fails 2023-12-12 21:18:08 +01:00
MarcoFalke
fa46cc22bc
Remove deprecated -rpcserialversion 2023-12-11 18:22:13 +01:00
fanquake
36fabb01b1
Merge bitcoin/bitcoin#29041: test: fix intermittent error in rpc_net.py (#29030)
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-11 11:41:46 +01:00
Sebastian Falbesoner
ea00f982d2 test: fix intermittent error in rpc_net.py (#29030)
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-09 13:26:18 +01:00
Kashif Smith
94feaf2b66 tests: Add unit tests for bitcoin-tx replaceable command 2023-12-08 20:27:13 -05:00
Sebastian Falbesoner
fd0bde2793 test: fix addnode functional test failure on OpenBSD
This is the functional test counterpart of PR #28891 /
commit 007d6f0e85bc329040bb405ef6016339518caa66.
2023-12-08 18:27:15 +01:00
Sebastian Falbesoner
878d914777 doc: test: mention OS detection preferences in style guideline 2023-12-08 18:16:27 +01:00
Sebastian Falbesoner
4c65ac96f8 test: detect OS consistently using platform.system() 2023-12-08 18:16:24 +01:00
Sebastian Falbesoner
37324ae3df test: use skip_if_platform_not_linux helper where possible
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-08 18:15:34 +01:00
fanquake
03042fb6bb
Merge bitcoin/bitcoin#29006: test: fix v2 transport intermittent test 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`:
  6d5790956f/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](00e0658e77)
  maflcko:
    Ok, lgtm ACK 00e0658e77f66103ebdeb29def99dc9f937c049d
  stratospher:
    ACK 00e0658.

Tree-SHA512: 0118b87f54ea5e6e080ff44f29d6af6674c757a588534b3add040da435f4359e71bf85bc0a5eb7170f99cc9956e1a03c35cce653d642d31eed41bbed1f94f44f
2023-12-08 11:33:27 +00:00
fanquake
1f352cf2fd
Merge bitcoin/bitcoin#28485: test: Extends MEMPOOL msg functional test
97c0dfa8942c7fd62c920deee03b4d0c59aba641 test: Extends MEMPOOL msg functional test (Sergi Delgado Segura)

Pull request description:

  Currently, p2p_filter.py::test_msg_mempool is not testing much. This extends the tests so the interaction between sending `MEMPOOL` messages with a filter that does not include all transactions in the mempool reacts, plus how it interacts with `INV` messages, especially after the changes introduced by #27675

ACKs for top commit:
  instagibbs:
    ACK 97c0dfa894
  theStack:
    re-ACK 97c0dfa8942c7fd62c920deee03b4d0c59aba641

Tree-SHA512: 746fdc867630f40509e6341f484a238dd855ae6d1be5eca121974491e4ca272dee88af4b90dda55ea9a5a19cbff198fa91ffa0c5bf1ddf0232b2c1b215b05b9a
2023-12-08 11:28:59 +00:00
Sergi Delgado Segura
97c0dfa894 test: Extends MEMPOOL msg functional test
Currently, p2p_filter.py::test_msg_mempool is not testing much.
This extends the tests so the interaction between sending MEMPOOL messages with
a filter that does not include all transactions in the mempool reacts, plus how
it interacts with INV messages
2023-12-06 13:21:58 -05:00