Commit Graph

1795 Commits

Author SHA1 Message Date
MarcoFalke
4a3e3af6e7
Merge #20813: scripted-diff: Bump copyright headers
fa0074e2d82928016a43ca408717154a1c70a4db scripted-diff: Bump copyright headers (MarcoFalke)

Pull request description:

  Needs to be done because no one has removed the years yet

ACKs for top commit:
  practicalswift:
    ACK fa0074e2d82928016a43ca408717154a1c70a4db

Tree-SHA512: 210e92acd7d400b556cf8259c3ec9967797420cfd19f0c2a4fa54cb2b3d32ad9ae27e771269201e7d554c0f4cd73a8b1c1a42c9f65d8685ca4d52e5134b071a3
2024-04-10 03:19:34 +07:00
MarcoFalke
e36eacd868
Merge #18772: rpc: calculate fees in getblock using BlockUndo data
66d012ad7f9381bacfc9b8388fa2ebf82cb86c9e test: RPC: getblock fee calculations (Elliott Jin)
bf7d6e31b1062ab5f90e14e83c56309f499fa2e9 RPC: getblock: tx fee calculation for verbosity 2 via Undo data (Elliott Jin)

Pull request description:

  This change is progress towards #18771 .  It adapts the fee calculation part of #16083 and addresses some feedback.  The additional "verbosity level 3" features are planned for a future PR.

  **Original PR description:**

  > Using block undo data (like in #14802) we can now show fee information for each transaction in a block without the need for additional -txindex and/or a ton of costly lookups. For a start we'll add transaction fee information to getblock verbosity level 2. This comes at a negligible speed penalty (<1%).

ACKs for top commit:
  luke-jr:
    tACK 66d012ad7f9381bacfc9b8388fa2ebf82cb86c9e
  fjahr:
    tACK 66d012ad7f9381bacfc9b8388fa2ebf82cb86c9e
  MarcoFalke:
    review ACK 66d012ad7f9381bacfc9b8388fa2ebf82cb86c9e 🗜

Tree-SHA512: be1fe4b866946a8dc36427f7dc72a20e10860e320a28fa49bc85bd2a93a0d699768179be29fa52e18b2ed8505d3ec272e586753ef2239b4230e0aefd233acaa2
2024-04-09 22:34:38 +07:00
Kittywhiskers Van Gogh
03ab144b8f
merge bitcoin#21785: Fix intermittent issue in p2p_addr_relay.py 2024-04-03 16:10:16 +00:00
Kittywhiskers Van Gogh
6d27db58d1
merge bitcoin#21707: Extend functional tests for addr relay 2024-04-03 16:10:16 +00:00
Kittywhiskers Van Gogh
1d4f10a378
merge bitcoin#19315: Allow outbound & block-relay-only connections in functional tests 2024-04-03 16:06:40 +00:00
Kittywhiskers Van Gogh
017d1b40e3
merge bitcoin#19763: don't relay to the address' originator 2024-04-03 16:05:30 +00:00
MarcoFalke
eba325d7a2
Merge #16551: test: Test that low difficulty chain fork is rejected
Backport notice:
 - data have real blocks from testnet
 - due to big gap in blocks before checkpoints (half-year) for sack of this test is added one more checkpoint, otherwise blocks are not accepted due to non-mockable time for other chains except regtest
 - data for 2 blocks in split chain are generated locally for testnet
--------------
333317ce6b67aa92f7363d48cd750712190b4b6b test: Test that low difficulty chain fork is rejected (MarcoFalke)
fa31dc1bf4ee471c4641eef8de02702ba0619ae7 test: Pass down correct chain name in tests (MarcoFalke)

Pull request description:

  To prevent OOM, Bitcoin Core will reject chain forks at low difficulty by default. This is the only use-case of checkpoints, so add a test for it to make sure the feature works as expected. If it didn't work, checkpoints would have no use-case and we might as well remove them

ACKs for top commit:
  Sjors:
    Thanks for adding the node 1 example. Code review ACK 333317c

Tree-SHA512: 90dffa540d0904f3cffb61d2382b1a26f84fe9560b7013e4461546383add31a8757b350616a6d43217c59ef7b8b2a1b62bb3bab582c679cbb2c660a782ce7be1
2024-04-03 14:16:43 +07:00
MarcoFalke
436a5783c7
Merge bitcoin/bitcoin#22736: log, sync: change lock contention from preprocessor directive to log category
7e698732836121912f179b7c743a72dd6fdffa72 sync: remove DEBUG_LOCKCONTENTION preprocessor directives (Jon Atack)
9b08006bc502e67956d6ab518388fad6397cac8d log, sync: improve lock contention logging and add time duration (Jon Atack)
3f4c6b87f1098436693c4990f2082515ec0ece26 log, timer: add timing macro in usec LOG_TIME_MICROS_WITH_CATEGORY (Jon Atack)
b7a17444e0746c562ae97b26eba431577947b06a log, sync: add LOCK logging category, apply it to lock contention (Jon Atack)

Pull request description:

  To enable lock contention logging, `DEBUG_LOCKCONTENTION` has to be defined at compilation. Once built, the logging is not limited to a category and is high frequency, verbose and in all-caps. With these factors combined, it seems likely to be rarely used.

  This patch:
  - adds a `lock` logging category
  - adds a timing macro in microseconds, `LOG_TIME_MICROS_WITH_CATEGORY`
  - updates `BCLog::LogMsg()` to omit irrelevant decimals for microseconds and skip unneeded code and math
  - improves the lock contention logging, drops the all-caps, and displays the duration in microseconds
  - removes the conditional compilation directives
  - allows lock contentions to be logged on startup with `-debug=lock` or at run time with `bitcoin-cli logging '["lock"]'`

  ```
  $ bitcoind -signet -debug=lock
  2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1920 started
  2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1920 completed (4μs)
  2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1302 started
  2021-09-01T12:40:01Z LockContention: cs_vNodes, net.cpp:1302 completed (4μs)
  2021-09-01T12:40:02Z LockContention: cs_vNodes, net.cpp:2242 started
  2021-09-01T12:40:02Z LockContention: cs_vNodes, net.cpp:2242 completed (20μs)
  2021-09-01T12:43:04Z LockContention: ::cs_main, validation.cpp:4980 started
  2021-09-01T12:43:04Z LockContention: ::cs_main, validation.cpp:4980 completed (3μs)

  $ bitcoin-cli -signet logging
    "lock": true,

  $ bitcoin-cli -signet logging [] '["lock"]'
    "lock": false,

  $ bitcoin-cli -signet logging '["lock"]'
    "lock": true,
  ```

  I've tested this with Clang 13 and GCC 10.2.1, on Debian, with and without `--enable-debug`.

ACKs for top commit:
  hebasto:
    re-ACK 7e698732836121912f179b7c743a72dd6fdffa72, added a contention duration to the log message since my [previous](https://github.com/bitcoin/bitcoin/pull/22736#pullrequestreview-743764606) review.
  theStack:
    re-ACK 7e698732836121912f179b7c743a72dd6fdffa72 🔏 ⏲️

Tree-SHA512: c4b5eb88d3a2c051acaa842b3055ce30efde1f114f61da6e55fcaa27476c1c33a60bc419f7f5ccda532e1bdbe70815222ec2b2b6d9226f29c8e94e598aacfee7
2024-04-02 12:09:35 -05:00
UdjinM6
c79f8b5ea2
fix: keep ADDRS outside 2024-04-02 18:28:22 +03:00
Kittywhiskers Van Gogh
a39065be88
test: fix incorrect nServices assertion, use NODE_NETWORK value
The value of nServices for `NODE_NETWORK | NODE_WITNESS`, the default for
p2p_addr{v2}_relay.py in Bitcoin Core, is 9. Dash doesn't implement SegWit
and so the corresponding nServices value is `NODE_NETWORK`, which is 1.
2024-04-02 17:40:19 +03:00
UdjinM6
0e78555a5b
fix: add missing check for sending addrs 2024-04-02 17:38:58 +03:00
UdjinM6
3cd69377b6
fix: test nodes should use mocktime 2024-04-02 17:38:35 +03:00
UdjinM6
acbbe8c9a2
fix: relayed addresses should use mocktime 2024-04-02 17:37:37 +03:00
MarcoFalke
8daef64f04
Merge #20691: ci, doc: Travis CI features and mentions cleanup
95487b055328b590ba83f258de9637ab0f9a2f17 doc: Drop mentions of Travis CI as it is no longer used (Hennadii Stepanov)
09d105ef0f8b4b06bf248721a1209c9e16e9db75 ci: Drop travis_fold feature as Travis CI is no longer used (Hennadii Stepanov)

Pull request description:

  As Travis CI is no longer used, this PR:
  - drops `travis_fold` feature
  - drops mentions of Travis CI in docs

ACKs for top commit:
  MarcoFalke:
    ACK 95487b055328b590ba83f258de9637ab0f9a2f17

Tree-SHA512: 2e259bb8b1e37bcefc1251737bb2716f06ddb57c490010b373825c4e70f42ca38efae69a2f63f21f577d7cee3725b94097bdddbd313f8ebf499281cf97c53cef
2024-03-27 00:48:26 +07:00
pasta
f2a42a01b1
Merge #5951: backport: trivial 2024 03 22
d5d1a714fb Merge bitcoin/bitcoin#24390: test: Remove suppression no longer needed with headers-only Boost.Test (fanquake)
51630d2e5e Merge bitcoin/bitcoin#22824: refactor: remove RecursiveMutex cs_nBlockSequenceId (MarcoFalke)
a9b1575fe8 Merge bitcoin/bitcoin#22781: wallet: fix the behavior of IsHDEnabled, return false in case of a blank hd wallet. (Samuel Dobson)
0505229c89 Merge bitcoin/bitcoin#22327: cli: Avoid truncating -rpcwaittimeout (MarcoFalke)
1dc97c7679 Merge bitcoin/bitcoin#22149: test: Add temporary logging to debug #20975 (W. J. van der Laan)
44f91cbc9a Merge #21597: test: Document race:validation_chainstatemanager_tests suppression (fanquake)
c326830f48 Merge bitcoin-core/gui#243: fix issue when disabling the auto-enabled blank wallet checkbox (MarcoFalke)
267f42fd6a Merge #21382: build: Clean remnants of QTBUG-34748 fix (fanquake)
1fcc5f1101 Merge #20540: test: Fix wallet_multiwallet issue on windows (MarcoFalke)
4afbaf2ea1 Merge #20322: test: Fix intermittent issue in wallet_listsinceblock (MarcoFalke)

Pull request description:

  ## Issue being fixed or feature implemented
  Batch of backports

  ## What was done?
  Trivial batch of backports

  ## How Has This Been Tested?
  CI looks good

  ## Breaking Changes
  None

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

Top commit has no ACKs.

Tree-SHA512: 8eeac54f011eb1111888c745dd56184ac9601de290f2b0f7b7ad02240e8dc1cab5a47fed26bfed2bd6f1066e0710827a3e5b2426f0bf66821cf1cd09099d5160
2024-03-25 22:46:15 -05:00
W. J. van der Laan
1dc97c7679
Merge bitcoin/bitcoin#22149: test: Add temporary logging to debug #20975
faa94961d6e38392ba068381726ed4e033367b03 test: Add temporary logging to debug #20975 (MarcoFalke)

Pull request description:

  to be reverted after a fix

ACKs for top commit:
  laanwj:
    Code review ACK faa94961d6

Tree-SHA512: 1f3103fcf4cad0af54e26c4d257bd824b128b5f2d2b81c302e861a829fd55d6a099fa476b79b30a71fe98975ae604b9e3ff31fd48a51d442389a9bd515e60ba0
2024-03-25 11:21:48 -05:00
Kittywhiskers Van Gogh
0d9046586b
merge bitcoin#21254: Avoid connecting to real network when running tests 2024-03-25 11:55:07 +00:00
Kittywhiskers Van Gogh
0d46acbfef
merge bitcoin#21165: Use mocktime in test_seed_peers 2024-03-25 11:55:07 +00:00
Kittywhiskers Van Gogh
8f40769385
merge bitcoin#19884: No delay in adding fixed seeds if -dnsseed=0 and peers.dat is empty 2024-03-25 11:55:06 +00:00
Kittywhiskers Van Gogh
446076d094
test: add missing dnsseed=0 in configuration
line added in fa31dc1bf4 (diff-e20e1f68486e5c096fdc11bca1cda063aacef524411f011d9c3497eb650c5eafR301)
2024-03-25 11:55:06 +00:00
Kittywhiskers Van Gogh
bcd383c2d6
merge bitcoin#20724: Cleanup of -debug=net log messages 2024-03-25 11:55:05 +00:00
MarcoFalke
1fcc5f1101
Merge #20540: test: Fix wallet_multiwallet issue on windows
fada2dfcac1c4b47ee76b877d91d515cf1d36410 test: Fix wallet_multiwallet issue on windows (MarcoFalke)

Pull request description:

  The error message on windows:

  > 2020-11-30T18:10:47.536032Z ListWalletDir: Error scanning C:\Users\user\AppData\Local\Temp\test_runner_₿_🏃_20201130_181042\wallet_multiwallet_0\node0\regtest\wallets\self_walletdat_symlink: boost::filesystem::status: The name of the file cannot be resolved by the system: "C:\Users\user\AppData\Local\Temp\test_runner_₿_🏃_20201130_181042\wallet_multiwallet_0\node0\regtest\wallets\self_walletdat_symlink\wallet.dat"

ACKs for top commit:
  promag:
    Code review ACK fada2dfcac1c4b47ee76b877d91d515cf1d36410. Although it could ignore (don't log) directories that lead to no permission error.
  fanquake:
    ACK fada2dfcac1c4b47ee76b877d91d515cf1d36410

Tree-SHA512: b475162cc3cd1574209d916605b229a79c8089714295f5e16569b71f958f0007d54dc76833938492d931387784588b11b73e3ef00f963540af42c079417f8d72
2024-03-22 11:20:57 -05:00
MarcoFalke
4afbaf2ea1
Merge #20322: test: Fix intermittent issue in wallet_listsinceblock
444412821e5349ce0e0b039d884583edb70c4399 test: Fix intermittent issue in wallet_listsinceblock (MarcoFalke)

Pull request description:

ACKs for top commit:
  Empact:
    Code Review ACK 444412821e

Tree-SHA512: 86d47b1e3c8681dd479654589c894016ac81a3c96a34c3b4a75278b2af85054ea8c6f768e518a5322a4928d82d5e99105bbce0f4fa6a7a18c40e3e0799f9ab54
2024-03-22 11:20:51 -05:00
Wladimir J. van der Laan
d7985aa36a
Merge #19512: p2p: banscore updates to gui, tests, release notes
fa108d6a757838225179a8df942cfb6d99c98c90 test: update tests for peer discouragement (Jon Atack)
1a9f462caa63fa16d7b4415312d2032a42b3fe0b gui, doc: rm Ban Score in GUI Peers window/release notes updates (Jon Atack)

Pull request description:

  This is the third `-banscore` PR in the mini-series described in #19464. See that PR for the intention and reasoning.

  - no longer display "Ban Score" in the GUI peers window and add a release note, plus release note fixups per https://github.com/bitcoin/bitcoin/pull/19464#pullrequestreview-447452052
  - update tests (`src/test/denialofservice_tests.cpp` and `test/functional/p2p_leak.py`) from banning to discouragement and per https://github.com/bitcoin/bitcoin/pull/19464#issuecomment-658052518

ACKs for top commit:
  jnewbery:
    ACK fa108d6a757838225179a8df942cfb6d99c98c90
  laanwj:
    ACK fa108d6a757838225179a8df942cfb6d99c98c90

Tree-SHA512: 58a449b3f47b8cb5490b34e4442ee8675bfad1ce48af4e4fd5c67715b0c1a596fb8e731d42e576b4c3b64627f76e0a68cbb1da9ea9f588a5932fe119baf40d50
2024-03-22 11:08:11 -05:00
MarcoFalke
b9799de985
Merge #19464: net: remove -banscore configuration option
06059b0c2a6c2db70c87a7715f8a344a13400fa1 net: rename DEFAULT_BANSCORE_THRESHOLD to DISCOURAGEMENT_THRESHOLD (Jon Atack)
1d4024bca8086cceff7539dd8c15e0b7fe1cc5ea net: remove -banscore configuration option (Jon Atack)

Pull request description:

  per https://github.com/bitcoin/bitcoin/pull/19219#issuecomment-652684340, https://github.com/bitcoin/bitcoin/pull/19219#discussion_r443074487 and https://github.com/bitcoin/bitcoin/pull/19219#issuecomment-652699592. Edit: now split into 3 straightforward PRs:
  - net: remove -banscore configuration option (this PR)
  - rpc: deprecate banscore field in getpeerinfo (#19469, *merged*)
  - gui: no longer display banscores (TBA in the gui repo)

ACKs for top commit:
  MarcoFalke:
    review ACK 06059b0c2a6c2db70c87a7715f8a344a13400fa1 📙
  vasild:
    ACK 06059b0c

Tree-SHA512: 03fad249986e0896697033fbb8ba2cbfaae7d7603b1fb2a38b3d41db697630d238623f4d732b9098c82af249ce5a1767dd432b7ca0fec10544e23d24fbd57c50
2024-03-22 11:08:10 -05:00
fanquake
c0c5d05419
Merge #19469: rpc: deprecate banscore field in getpeerinfo
41d55d30579358c805036201664ad6a1c1d48681 doc: getpeerinfo banscore deprecation release note (Jon Atack)
dd54e3796e633cfdf6954af306afd26eadc25116 test: getpeerinfo banscore deprecation test (Jon Atack)
8c7647b3fbbab03ea84071cf3cd2d0d2bf8be255 rpc: deprecate banscore field in rpc getpeerinfo (Jon Atack)

Pull request description:

  Per https://github.com/bitcoin/bitcoin/pull/19219#discussion_r443074487 and https://github.com/bitcoin/bitcoin/pull/19219#issuecomment-652699592, this PR deprecates returning the `banscore` field in the `getpeerinfo` RPC, updates the help, adds a test, and updates the release notes. Related to #19464.

ACKs for top commit:
  fanquake:
    ACK 41d55d30579358c805036201664ad6a1c1d48681

Tree-SHA512: 8eca08332581e2fe191a2aafff6ba89ce39413f0491ed0de8b86577739f0ec430b1a8fbff2914b0f3138a229563dfcc1981c0cf5b7dd6061b5c48680a28423bc
2024-03-22 11:08:10 -05:00
MarcoFalke
57b6fe7327
Merge bitcoin/bitcoin#22257: test: refactor: various (de)serialization helpers cleanups/improvements
bdb8b9a347e68f80a2e8d44ce5590a2e8214b6bb test: doc: improve doc for `from_hex` helper (mention `to_hex` alternative) (Sebastian Falbesoner)
191405420815d49ab50184513717a303fc2744d6 scripted-diff: test: rename `FromHex` to `from_hex` (Sebastian Falbesoner)
a79396fe5f8f81c78cf84117a87074c6ff6c9d95 test: remove `ToHex` helper, use .serialize().hex() instead (Sebastian Falbesoner)
2ce7b47958c4a10ba20dc86c011d71cda4b070a5 test: introduce `tx_from_hex` helper for tx deserialization (Sebastian Falbesoner)

Pull request description:

  There are still many functional tests that perform conversions from a hex-string to a message object (deserialization) manually. This PR identifies all those instances and replaces them with a newly introduced helper `tx_from_hex`.

  Instances were found via
  * `git grep "deserialize.*BytesIO"`

  and some of them manually, when it were not one-liners.

  Further, the helper `ToHex` was removed and simply replaced by `.serialize().hex()`, since now both variants are in use (sometimes even within the same test) and using the helper doesn't really have an advantage in readability. (see discussion https://github.com/bitcoin/bitcoin/pull/22257#discussion_r652404782)

ACKs for top commit:
  MarcoFalke:
    review re-ACK bdb8b9a347e68f80a2e8d44ce5590a2e8214b6bb 😁

Tree-SHA512: e25d7dc85918de1d6755a5cea65471b07a743204c20ad1c2f71ff07ef48cc1b9ad3fe5f515c1efaba2b2e3d89384e7980380c5d81895f9826e2046808cd3266e
2024-03-22 10:29:15 -05:00
W. J. van der Laan
5c85b7dc14
Merge bitcoin/bitcoin#21056: rpc: Add a -rpcwaittimeout parameter to limit time spent waiting
b9e76f1bf08c52fcd402b2314e00db4ad247ebc8 rpc: Add test for -rpcwaittimeout (Christian Decker)
f76cb10d7dc9a7b0c55d28011161606399417664 rpc: Prefix rpcwaittimeout error with details on its nature (Christian Decker)
c490e17ef698a1695050f82ef6567b3b87a21861 doc: Add release notes for the `-rpcwaittimeout` cli parameter (Christian Decker)
a7fcc8eb59fe51473571661316214156fbdbdcae rpc: Add a `-rpcwaittimeout` parameter to limit time spent waiting (Christian Decker)

Pull request description:

  Adds a new numeric `-rpcwaittimeout` that can be used to limit the
  time we spend waiting on the RPC server to appear. This is used by
  downstream projects to provide a bit of slack when `bitcoind`s RPC
  interface is not available right away.

  This makes the `-rpcwait` argument more useful, since we can now limit
  how long we'll ultimately wait, before potentially giving up and reporting
  an error to the caller. It was discussed in the context of the BTCPayServer
  wanting to have c-lightning wait for the RPC interface to become available
  but still have the option of giving up eventually ([4355]).

  I checked with laanwj whether this is already possible ([comment]), and
  whether this would be a welcome change. Initially I intended to repurpose
  the (optional) argument to `-rpcwait`, however I decided against it since it
  would potentially break existing configurations, using things like `rpcwait=1`,
  or `rpcwait=true` (the former would have an unintended short timeout, when
  old behavior was to wait indefinitely).

  ~Due to its simplicity I didn't implement a test for it yet, but if that's desired I
  can provide one.~ Test was added during reviews.

  [4355]: https://github.com/ElementsProject/lightning/issues/4355
  [comment]: https://github.com/ElementsProject/lightning/issues/4355#issuecomment-768288261

ACKs for top commit:
  laanwj:
    Code review ACK b9e76f1bf08c52fcd402b2314e00db4ad247ebc8
  promag:
    ACK b9e76f1bf08c52fcd402b2314e00db4ad247ebc8.

Tree-SHA512: 3cd6728038ec7ca7c35c2e7ccb213bfbe963f99a49bb48bbc1e511c4dd23d9957c04f9af1f8ec57120e47b26eaf580b46817b099d5fc5083c98da7aa92db8638

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
2024-03-22 10:29:15 -05:00
MarcoFalke
17598db793
Merge bitcoin/bitcoin#22408: test: add tests for bad-txns-prevout-null reject reason
1f449586a9e39bc4fb53cb5c7a31362e47aea19b test: add `bad-txns-prevout-null` test to mempool_accept.py (Sebastian Falbesoner)
aa0a5bb70d77739d43d5a9ceae78fb0c6fafd435 test: add `bad-txns-prevout-null` test case to invalid_txs.py (Sebastian Falbesoner)

Pull request description:

  This simple PR adds missing tests for the reject reason `bad-txns-prevout-null`, which is thrown in the function `CheckTransaction()`: a62fc35a15/src/consensus/tx_check.cpp (L52-L54)

  Basically this condition is met for non-coinbase transactions (the code snippet above only hits if `!tx.IsCoinBase()`) with coinbase-like outpoints, i.e. hash=0, n=0xffffffff.

  Can be tested by running the functional tests `feature_block.py`, `p2p_invalid_tx.py` and `mempool_accept.py`. Not sure if the redundancy in the tests is desired (I guess it would make sense if the mempool acceptance test also makes use of the invalid_txs templates?).

ACKs for top commit:
  rajarshimaitra:
    tACK 1f449586a9
  brunoerg:
    tACK 1f449586a9e39bc4fb53cb5c7a31362e47aea19b
  kristapsk:
    ACK 1f449586a9e39bc4fb53cb5c7a31362e47aea19b, code looks correct and all tests pass.

Tree-SHA512: 2d4f940a6ac8e0d80d2670c9e1111cbf43ae6ac62809a2ccf17cffee9a41d387ea4d889ee300eb4a407c055b13bfa5d37102a32ed59964a9b6950bd907ba7204
2024-03-22 10:29:12 -05:00
pasta
c90fd21378
Merge #5930: backport: bitcoin#14582, #16404, #17445, #19507, #19538, #19743, #19756, #19773, #19830, partial bitcoin#27053
5a6b8b6b1f partial Merge bitcoin/bitcoin#27053: wallet: reuse change dest when re-creating TX with avoidpartialspends (fanquake)
2f788aa76d fix: change port to use for zmq in interface_zmq_dash.py (Konstantin Akimov)
0ce66fd477 Merge #19507: Expand functional zmq transaction tests (Wladimir J. van der Laan)
a1c2386153 Merge #17445: zmq: Fix due to invalid argument and multiple notifiers (Wladimir J. van der Laan)
44929bad82 Merge #16404: qa: Test ZMQ notification after chain reorg (MarcoFalke)
1707f01309 fix: follow-up changes from bitcoin/bitcoin#22220 for maxapsfee (Konstantin Akimov)
eb4270deae Merge #19743: -maxapsfee follow-up (Samuel Dobson)
6a6d379711 Merge #19756: tests: add sync_all to fix race condition in wallet groups test (MarcoFalke)
5821a1d23a Merge #14582: wallet: always do avoid partial spends if fees are within a specified range (Samuel Dobson)
59d5a4ef39 Merge #19773: wallet: Avoid recursive lock in IsTrusted (Samuel Dobson)
2489f29f0e Merge #19830: test: Add tsan supp for leveldb::DBImpl::DeleteObsoleteFiles (fanquake)
10fa7a66b6 Merge #19538: ci: Add tsan suppression for race in DatabaseBatch (MarcoFalke)

Pull request description:

  ## Issue being fixed or feature implemented
  Regular backports from bitcoin v21

  ## What was done?
   - bitcoin/bitcoin#19538
   - bitcoin/bitcoin#19830
   - bitcoin/bitcoin#19773
   - bitcoin/bitcoin#14582
   - bitcoin/bitcoin#19756
   - bitcoin/bitcoin#19743
   - bitcoin/bitcoin#16404
   - bitcoin/bitcoin#17445
   - bitcoin/bitcoin#19507
   - partial bitcoin/bitcoin#27053

  +some extra fixes and missing changes from bitcoin/bitcoin#22220 for `maxapsfee`

  +changed port for zmq in `interface_zmq_dash.py` to prevent intermittent error in `interface_zmq.py`

  ## How Has This Been Tested?
  Run unit & functional tests

  ## Breaking Changes
  `CreateTransaction` now uses sometime 2 private keys for one transaction instead one

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  PastaPastaPasta:
    utACK 5a6b8b6b1f

Tree-SHA512: 7efd8a31808f155c08035d0fb7ceaac369e3e44e68d2c91a88e52815a60efba5fe9458f41d93352627c2c062d414fb0207dcf216fa75b54af210b503f9123de6
2024-03-19 10:12:39 -05:00
fanquake
5a6b8b6b1f
partial Merge bitcoin/bitcoin#27053: wallet: reuse change dest when re-creating TX with avoidpartialspends
14b4921a91920df25b19ff420bfe2bff8c56f71e wallet: reuse change dest when recreating TX with avoidpartialspends (Matthew Zipkin)

Pull request description:

  Closes https://github.com/bitcoin/bitcoin/issues/27051

  When the wallet creates a transaction internally, it will also create an alternative that spends using destination groups and see if the fee difference is negligible. If it costs the user the same to send the grouped version, we send it (even if the user has `avoidpartialspends` set to `false` which is default). This patch ensures that the second transaction creation attempt re-uses the change destination selected by the first attempt. Otherwise, the first change address remains reserved, will not be used in the second attempt, and then will never be used by the wallet, leaving gaps in the BIP44 chain.

  If the user had `avoidpartialspends` set to true, there is no second version of the created transaction and the change addresses are not affected.

  I believe this behavior was introduced in https://github.com/bitcoin/bitcoin/pull/14582

ACKs for top commit:
  achow101:
    ACK 14b4921a91920df25b19ff420bfe2bff8c56f71e

Tree-SHA512: a3d56f251ff4b333fc11325f30d05513e34ab0a2eb703fadd0ad98d167ae074493df1a24068298336c6ed2da6b31aa2befa490bc790bbc260ed357c8f2397659
2024-03-18 16:30:45 +07:00
Konstantin Akimov
2f788aa76d
fix: change port to use for zmq in interface_zmq_dash.py
It resolves a conflict with interface_zmq.py and intermittent failure
2024-03-18 16:01:41 +07:00
Wladimir J. van der Laan
0ce66fd477
Merge #19507: Expand functional zmq transaction tests
7356292e1d7a44da8a2bd31c02c58d550bf38009 Have zmq reorg test cover mempool txns (Gregory Sanders)
a0f4f9c983e57cc97ecbc56d0177eaf1854c842c Add zmq test for transaction pub during reorg (Gregory Sanders)
2399a0600ca9c4b676fa2f97520b8ecc44642246 Add test case for mempool->block zmq notification (Gregory Sanders)
e70512a83c69bc85e96b08ade725594eda3e230f Make ordering of zmq consumption irrelevant to functional test (Gregory Sanders)

Pull request description:

  Tests written to better define what messages are sent when. Also did a bit of refactoring to make sure the exact notification channel ordering doesn't matter.

  Confusions below aside, I believe having these more descriptive tests helps describe what behavior we expect from ZMQ notificaitons.

  Remaining confusion:
  1) Notification patterns seem to vary wildly with the inclusion of mempool transactions being reorg'ed. See difference between "Add zmq test for transaction pub during reorg" and "Have zmq reorg test cover mempool txns" commits for specifics.
  2) Why does a reorg'ed transaction get announced 3 times? From what I understand it can get announced once for disconnected block, once for mempool entry. What's the third? It occurs a 4th time when included in a block(not added in test)

ACKs for top commit:
  laanwj:
    code review ACK 7356292e1d7a44da8a2bd31c02c58d550bf38009
  promag:
    Code review ACK 7356292e1d7a44da8a2bd31c02c58d550bf38009.

Tree-SHA512: 573662429523fd6a1af23dd907117320bc68cb51a93fba9483c9a2160bdce51fb590fcd97bcd2b2751d543d5c1148efa4e22e1c3901144f882b990ed2b450038
2024-03-18 16:01:41 +07:00
Wladimir J. van der Laan
a1c2386153
Merge #17445: zmq: Fix due to invalid argument and multiple notifiers
3e730bf90aaf53c41ff3a778f6aac15d163d1c0c zmq: Fix due to invalid argument and multiple notifiers (João Barbosa)

Pull request description:

  ZMQ initialization is interrupted if any notifier fails, and in that case all notifiers are destroyed. The notifier shutdown assumes that the initialization had occurred. This is not valid when there are multiple notifiers and any except the last fails to initialize.

  Can be tested by running test/functional/interface_zmq.py from this branch with bitcoind from master.

  Closes #17185.

ACKs for top commit:
  laanwj:
    Code review ACK 3e730bf90aaf53c41ff3a778f6aac15d163d1c0c, thanks for adding a test

Tree-SHA512: 5da710e97dcbaa94896d019e75162d470f6d381ee07c60e5b3e9db93d11e8f7ca9bf2c509efa4486199e88c96c3e720cc96b4e35b62725d4c7db8e8e9bf6e09d
2024-03-18 16:01:40 +07:00
MarcoFalke
44929bad82
Merge #16404: qa: Test ZMQ notification after chain reorg
abdfc5e89b687f73de4ab97e924c29cc27e71c15 qa: Test ZMQ notification after chain reorg (João Barbosa)
aa2622a726bc0f02152d79c888a332694678a989 qa: Refactor ZMQ test (João Barbosa)
6bc1ff915dd495f05985d3402a34dbfc3b6a08b4 doc: Add note regarding ZMQ block notification (João Barbosa)

Pull request description:

Top commit has no ACKs.

Tree-SHA512: b93237adc8c84b3aa72ccc28097090eabcb006cf408083218bebf6fec703bd0de2ded80b6879e77096872e14ba9402a6d3f923b146a54d4c4e41dcb862c3e765
2024-03-18 16:01:40 +07:00
Samuel Dobson
eb4270deae
Merge #19743: -maxapsfee follow-up
7e31ea9fa0a59ced2293057acb14c71ec97db689 -maxapsfee: follow-up fixes (Karl-Johan Alm)
9f77b821764dcaebc97a5ae76c3052936418308d doc: release notes for -maxapsfee (Karl-Johan Alm)

Pull request description:

  Addresses feedback from jonatack and meshcollider in #14582.

ACKs for top commit:
  jonatack:
    ACK 7e31ea9fa0a
  meshcollider:
    re-ACK 7e31ea9fa0a59ced2293057acb14c71ec97db689

Tree-SHA512: 5d159d78e917e41140e1394bef05e821430dbeac585e9bd708f897041dd7104c2a6b563bfab8b2c85e6d923441160a3880d7864d768aa8e0e66860e0a2c4f56b
2024-03-18 16:01:39 +07:00
MarcoFalke
6a6d379711
Merge #19756: tests: add sync_all to fix race condition in wallet groups test
72ae20fc142457a200278cb2fedc5e32a3766b58 tests: add sync_all to fix race condition in wallet groups test (Karl-Johan Alm)

Pull request description:

  This most likely fixes #19749, the intermittent CI issues with wallet_groups.

  This fix is also included in #19743.

Top commit has no ACKs.

Tree-SHA512: dd6ef7f89829483e2278191c21fe0912b51fd2187c10a0fa158339c5ab9f22d93b733ae10f17ef25d8b64f44e596e66dba8d7db5c009343472f422ce4cd67d8f
2024-03-18 16:01:39 +07:00
Samuel Dobson
5821a1d23a
Merge #14582: wallet: always do avoid partial spends if fees are within a specified range
7f13dfb587dd6a7a5b7dfbfe689ae0ce818fe5c9 test: test the implicit avoid partial spends functionality (Karl-Johan Alm)
b82067bf696c53f22536f9ca2e51949c164f6b06 wallet: try -avoidpartialspends mode and use its result if fees are below threshold (Karl-Johan Alm)

Pull request description:

  The `-avoidpartialspends` feature is normally disabled, as it may affect the optimal fee for payments. This PR introduces a new parameter `-maxapsfee` (max avoid partial spends fee) which acts on the following values:
  * -1: disable partial spend avoidance completely (do not even try it)
  * 0: only do partial spend avoidance if fees are the same or better as the regular coin selection
  * 1..∞: use APS variant if the absolute fee difference is less than or equal to the max APS fee

  For values other than -1, the code will now try partial spend avoidance once, and if that gives a value within the accepted range, it will use that.

  Example: -maxapsfee=0.00001000 means the wallet will do regular coin select, APS coin select, and then pick AKS iff the absolute fee difference is <= 1000 satoshi.

  Edit: updated this to reflect the fact we are now using a max fee.

ACKs for top commit:
  fjahr:
    tested ACK 7f13dfb587dd6a7a5b7dfbfe689ae0ce818fe5c9
  achow101:
    ACK 7f13dfb587dd6a7a5b7dfbfe689ae0ce818fe5c9
  jonatack:
    ACK 7f13dfb58, code review, debug build, verified the test fails with `AssertionError: not(2 == 1)` for the number of vouts when `-maxapsfee=0.0001` is changed to 0, and verified the new logging with an added assertion.
  meshcollider:
    Code review ACK 7f13dfb587dd6a7a5b7dfbfe689ae0ce818fe5c9

Tree-SHA512: 475929df57f6191bb4e36bfbcad5a280a64bb0ecd8767b76cb2e44e2301235d0eb294a3f2fac5bbf15d35d7ecfba47acb2285feadb883c9ce31c08377e3afb3c
2024-03-18 16:01:38 +07:00
Konstantin Akimov
af9eb81e56
fix: wrong name of arguments for RPC 2024-03-17 13:02:59 -05:00
MarcoFalke
7ac1ee0fb4
Merge #19717: rpc: Assert that RPCArg names are equal to CRPCCommand ones (mining,zmq,rpcdump)
fa3d9ce3254882c545d700990fe8e9a678f31eed rpc: Assert that RPCArg names are equal to CRPCCommand ones (rpcdump) (MarcoFalke)
fa32c1d5ec25bc53bf989a8ae68e688593d2859d rpc: Assert that RPCArg names are equal to CRPCCommand ones (zmq) (MarcoFalke)
faaa46dc204d6d714f71dbc6f0bf02215dba0f0f rpc: Assert that RPCArg names are equal to CRPCCommand ones (mining) (MarcoFalke)
fa93bc14c7411a108dd024d391344fabf0f76369 rpc: Remove unused return type from appendCommand (MarcoFalke)

Pull request description:

  This is split out from #18531 to just touch the RPC methods in misc. Description from the main pr:

  ### Motivation

  RPCArg names in the rpc help are currently only used for documentation. However, in the future they could be used to teach the server the named arguments. Named arguments are currently registered by the `CRPCCommand`s and duplicate the RPCArg names from the documentation. This redundancy is fragile, and has lead to errors in the past (despite having linters to catch those kind of errors). See section "bugs found" for a list of bugs that have been found as a result of the changes here.

  ### Changes

  The changes here add an assert in the `CRPCCommand` constructor that the RPCArg names are identical to the ones in the `CRPCCommand`.

  ### Future work

  > Here or follow up, makes sense to also assert type of returned UniValue?

  Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including:

  * Removing the CRPCCommand arguments, now that they are asserted to be equal and thus redundant
  * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table
  * Auto-formatting and sanity checking the RPCExamples with RPCMan
  * Checking passed-in json in self-check. Removing redundant checks
  * Checking returned json against documentation to avoid regressions or false documentation
  * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static

  ### Bugs found

  * The assert identified issue #18607
  * The changes itself fixed bug #19250

ACKs for top commit:
  fjahr:
    tested ACK fa3d9ce3254882c545d700990fe8e9a678f31eed
  promag:
    Code review ACK fa3d9ce3254882c545d700990fe8e9a678f31eed.

Tree-SHA512: 068ade4b55cc195868d53b7f9a27151d45b440857bb069e261a49d102a49a38fdba5d68868516a1d66a54a73ba34681362f934ded7349e894042bde873b75719
2024-03-17 13:02:58 -05:00
Wladimir J. van der Laan
860d31f504
Merge #19455: rpc generate: print useful help and error message
f0aa8aeea5a183ea44a877255d12db7732f2e0a8 test: add rpc_generate functional test (Jon Atack)
92d94ffb8d07cc0d2665c901de5903a3a90d5fd0 rpc: print useful help and error message for generate (Jon Atack)
8d32d2011d3f4e1d9e587d6f80dfa4a3e9f9393d test: consider generate covered in _get_uncovered_rpc_commands() (Jon Atack)

Pull request description:

  This was a requested follow-up to #19133 and #17700 to alleviate confusion and head-scratching by people following tutorials that use `generate`. See https://github.com/bitcoin/bitcoin/pull/19455#issuecomment-668172916 below, https://github.com/bitcoin/bitcoin/pull/19133#issuecomment-636860943 and https://github.com/bitcoin/bitcoin/pull/17700#issuecomment-566159096.

  before
  ```
  $ bitcoin-cli help generate
  help: unknown command: generate

  $ bitcoin-cli generate
  error code: -32601
  error message:
  Method not found
  ```

  after
  ```
  $ bitcoin-cli help generate
  generate ( nblocks maxtries ) has been replaced by the -generate cli option. Refer to -help for more information.

  $ bitcoin-cli generate
  error code: -32601
  error message:
  generate ( nblocks maxtries ) has been replaced by the -generate cli option. Refer to -help for more information.
  ```

  In the general help it remains hidden, as requested by laanwj.
  ```
  $ bitcoin-cli help

  == Generating ==
  generateblock "output" ["rawtx/txid",...]
  generatetoaddress nblocks "address" ( maxtries )
  generatetodescriptor num_blocks "descriptor" ( maxtries )
  ```

ACKs for top commit:
  adamjonas:
    utACK f0aa8aeea5a183ea44a877255d12db7732f2e0a8
  pinheadmz:
    ACK f0aa8aeea5a183ea44a877255d12db7732f2e0a8

Tree-SHA512: d083652589ad3e8228c733455245001db22397559c3946e7e573cf9bd01c46e9e88b72d934728ec7f4361436ae4c74adb8f579670b09f479011924357e729af5
2024-03-17 13:02:57 -05:00
MarcoFalke
58d923cd5b
Merge #19528: rpc: Assert that RPCArg names are equal to CRPCCommand ones (misc)
fa77de2baa40ee828c850ef4068c76cc3619e87b rpc: Assert that RPCArg names are equal to CRPCCommand ones (misc) (MarcoFalke)
fa50bdc755489b2e291ea5ba0e39e44a20c6c6de rpc: Limit echo to 10 args (MarcoFalke)
fa89ca9b5bd334813fd7e7edb202c56b35076e8d refactor: Use C++11 range based for loops to simplify rpc code (MarcoFalke)
fa459bdc87bbb050ca1c8d469023a96ed798540e rpc: Treat all args after a hidden arg as hidden as well (MarcoFalke)

Pull request description:

  This is split out from #18531 to just touch the RPC methods in misc. Description from the main pr:

  ### Motivation

  RPCArg names in the rpc help are currently only used for documentation. However, in the future they could be used to teach the server the named arguments. Named arguments are currently registered by the `CRPCCommand`s and duplicate the RPCArg names from the documentation. This redundancy is fragile, and has lead to errors in the past (despite having linters to catch those kind of errors). See section "bugs found" for a list of bugs that have been found as a result of the changes here.

  ### Changes

  The changes here add an assert in the `CRPCCommand` constructor that the RPCArg names are identical to the ones in the `CRPCCommand`.

  ### Future work

  > Here or follow up, makes sense to also assert type of returned UniValue?

  Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including:

  * Removing the CRPCCommand arguments, now that they are asserted to be equal and thus redundant
  * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table
  * Auto-formatting and sanity checking the RPCExamples with RPCMan
  * Checking passed-in json in self-check. Removing redundant checks
  * Checking returned json against documentation to avoid regressions or false documentation
  * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static

  ### Bugs found

  * The assert identified issue #18607
  * The changes itself fixed bug #19250

ACKs for top commit:
  laanwj:
    Code review ACK fa77de2baa40ee828c850ef4068c76cc3619e87b
  fjahr:
    tested ACK fa77de2baa40ee828c850ef4068c76cc3619e87b
  theStack:
    ACK fa77de2baa
  ryanofsky:
    Code review ACK fa77de2baa40ee828c850ef4068c76cc3619e87b. Pretty straightfoward changes

Tree-SHA512: badae1606518c0b55ce2c0bb9025d14f05556532375eb20fd6f3bfadae1e5e6568860bff8599d037e655bf1d23f1f464ca17f4db10a6ab3d502b6e9e61c7b3d3
2024-03-17 13:02:57 -05:00
pasta
c23514dc49
Merge #5823: backport: bitcoin#19200, #19405, #20282
9c54cb16de Merge #19405: rpc, cli: add network in/out connections to `getnetworkinfo` and `-getinfo` (Wladimir J. van der Laan)
19aba38cab Merge #19200: rpc: remove deprecated getaddressinfo fields (Samuel Dobson)
f5642281cc Merge #20282: wallet: change upgradewallet return type to be an object (Samuel Dobson)

Pull request description:

  ## Issue being fixed or feature implemented
  Bitcoin backports with breaking changes

  ## What was done?
   - bitcoin/bitcoin#20282
   - bitcoin/bitcoin#19200
   - bitcoin/bitcoin#19405

  ## How Has This Been Tested?
  Run unit/functional tests

  ## Breaking Changes
  ### RPC:
  - `upgradewallet` now returns object for future extensibility (#20282)

  - `getnetworkinfo` now returns fields `connections_in`, `connections_out`,
    `connections_mn_in`, `connections_mn_out`, `connections_mn`
    that provide the number of inbound and outbound peer
    connections. These new fields are in addition to the existing `connections`
    field, which returns the total number of peer connections. Old fields
    `inboundconnections`, `outboundconnections`, `inboundmnconnections`,
    `outboundmnconnections` and `mnconnections` are removed (#19405)

  - Backwards compatibility has been dropped for two `getaddressinfo` RPC
    deprecations, as notified in the 19.1.0 and 19.2.0 release notes.
    The deprecated `label` field has been removed as well as the deprecated `labels` behavior of
    returning a JSON object containing `name` and `purpose` key-value pairs. Since
    20.1, the `labels` field returns a JSON array of label names. (#19200)

  ### CLI

  - The `connections` field of `bitcoin-cli -getinfo` is expanded to return a JSON
    object with `in`, `out` and `total` numbers of peer connections and `mn_in`,
    `mn_out` and `mn_total` numbers of verified mn connections. It previously
    returned a single integer value for the total number of peer connections. (#19405)

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  PastaPastaPasta:
    utACK 9c54cb16de

Tree-SHA512: 9710cdd062d02d64e2eebcecca1b5c2e6ccda5ca6d9bd6d1833700f4273fcfb206ae99134f71bbc8b0843cb8ebba208c72139f5a624d79ec7362bd73b117bfb2
2024-03-17 12:58:46 -05:00
Wladimir J. van der Laan
9c54cb16de
Merge #19405: rpc, cli: add network in/out connections to getnetworkinfo and -getinfo
581b343d5bf517510ab0236583ca96628751177d Add in/out connections to cli -getinfo (Jon Atack)
d9cc13e88d096c1a171159c01cbb96444f7f8d7f UNIX_EPOCH_TIME fixup in rpc getnettotals (Jon Atack)
1ab49b81cf32b6ef9e312a0a8ac45c68a3262f0d Add in/out connections to rpc getnetworkinfo (Jon Atack)

Pull request description:

  This is basic info that is present in the GUI that I've been wishing to have exposed via the RPC and CLI without needing a bash workaround or script. For human users it would also be useful to have it in `-getinfo`.

  `bitcoin-cli getnetworkinfo`
  ```
    "connections": 15,
    "connections_in": 6,
    "connections_out": 9,
  ```

  `bitcoin-cli -getinfo`
  ```
    "connections": {
      "in": 6,
      "out": 9,
      "total": 15
    },
  ```

  Update the tests, RPC help, and release notes for the changes. Also fixup the `getnettotals` timemillis help while touching `rpc/net.cpp`.

  -----

  Reviewers can manually test this PR by [building from source](https://jonatack.github.io/articles/how-to-compile-bitcoin-core-and-run-the-tests), launching bitcoind, and then running `bitcoin-cli -getinfo`, `bitcoin-cli getnetworkinfo`, `bitcoin-cli help getnetworkinfo`, and `bitcoin-cli help getnettotals` (for the UNIX epoch time change).

ACKs for top commit:
  eriknylund:
    > tACK [581b343](581b343d5b) on master at [a0a422c](a0a422c34c), ran unit & functional tests and and confirmed changes on an existing datadir ✌️
  benthecarman:
    tACK `581b343`
  willcl-ark:
    tACK for 581b343d5bf517510ab0236583ca96628751177d, this time rebased onto master at 862fde88be706adb20a211178253636442c3ae00.
  shesek:
    tACK `581b343`. This provides what I needed, thanks!
  n-thumann:
    tACK 581b343 on master at a0a422c, ran unit & functional tests and and confirmed changes on an existing datadir ✌️

Tree-SHA512: 08dd3ac8fefae401bd8253ff3ac027603c528eeccba53cedcb127771316173a7052fce44af8fa33ac98ebc4cf2a2b11cdefd949995d55e9b9a5942b876d00dc5
2024-03-16 02:39:45 +07:00
Samuel Dobson
19aba38cab
Merge #19200: rpc: remove deprecated getaddressinfo fields
BACKPORT NOTICE:
These backports #17578 and #17585 are included to 19.1 and 19.2. That's long enough!
------------------------------------------
bc01f7ae0538d3c647ce8dfbc29f7914d5df3fbb doc: release note for rpc getaddressinfo removals (Jon Atack)
90e989390ee50633fff0e4f210a1ea23ff00e012 rpc: getaddressinfo RPCResult fixup (Jon Atack)
a8507c99da10791aa69ca277128e06753942e976 rpc: remove deprecated getaddressinfo `labels: purpose` (Jon Atack)
645a8653c895e4fc7717e9e5ac045612b5deaa60 rpc: remove deprecated getaddressinfo `label` field (Jon Atack)

Pull request description:

  These were deprecated in #17578 and #17585, with expected 0.21 removal notified in the 0.20 release notes.
  ```
  - The `getaddressinfo` RPC has had its `label` field deprecated
    (re-enable for this release using the configuration parameter
    `-deprecatedrpc=label`).  The `labels` field is altered from returning
    JSON objects to returning a JSON array of label names (re-enable
    previous behavior for this release using the configuration parameter
    `-deprecatedrpc=labelspurpose`).  Backwards compatibility using the
    deprecated configuration parameters is expected to be dropped in the
    0.21 release.  (#17585, #17578)
  ```

ACKs for top commit:
  Sjors:
    utACK bc01f7a
  adamjonas:
    utACK bc01f7a
  meshcollider:
    utACK bc01f7ae0538d3c647ce8dfbc29f7914d5df3fbb

Tree-SHA512: ae1af381e32c4c3bde8b061a56382838513a9a82c88767843cdeae3a2ab8aa7d8c2e66e106d2b31ea07d74bb80c191a2f842c9aaecc7c5438ad9a9bc66d1b251
2024-03-16 02:39:42 +07:00
fanquake
85fa6e4585
Merge bitcoin/bitcoin#22446: test: Fix wallet_listdescriptors.py if bdb is not compiled
0c845e3f8995eb8dc543a63899e5633a46091b4e test: Fix wallet_listdescriptors.py if bdb is not compiled (Hennadii Stepanov)

Pull request description:

  If build system is configured `--without-bdb`, the `wallet_listdescriptors.py` fails:
  ```
  $ test/functional/wallet_listdescriptors.py --descriptors
  2021-07-14T13:20:52.931000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_02p7o1c9
  2021-07-14T13:21:23.377000Z TestFramework (INFO): Test that the command is not available for legacy wallets.
  2021-07-14T13:21:23.381000Z TestFramework (ERROR): JSONRPC error
  Traceback (most recent call last):
    File "/home/hebasto/GitHub/bitcoin/test/functional/test_framework/test_framework.py", line 128, in main
      self.run_test()
    File "test/functional/wallet_listdescriptors.py", line 34, in run_test
      node.createwallet(wallet_name='w1', descriptors=False)
    File "/home/hebasto/GitHub/bitcoin/test/functional/test_framework/test_node.py", line 685, in createwallet
      return self.__getattr__('createwallet')(wallet_name, disable_private_keys, blank, passphrase, avoid_reuse, descriptors, load_on_startup, external_signer)
    File "/home/hebasto/GitHub/bitcoin/test/functional/test_framework/coverage.py", line 47, in __call__
      return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
    File "/home/hebasto/GitHub/bitcoin/test/functional/test_framework/authproxy.py", line 146, in __call__
      raise JSONRPCException(response['error'], status)
  test_framework.authproxy.JSONRPCException: Compiled without bdb support (required for legacy wallets) (-4)
  2021-07-14T13:21:23.436000Z TestFramework (INFO): Stopping nodes
  2021-07-14T13:21:24.092000Z TestFramework (WARNING): Not cleaning up dir /tmp/bitcoin_func_test_02p7o1c9
  2021-07-14T13:21:24.092000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/bitcoin_func_test_02p7o1c9/test_framework.log
  2021-07-14T13:21:24.092000Z TestFramework (ERROR):
  2021-07-14T13:21:24.092000Z TestFramework (ERROR): Hint: Call /home/hebasto/GitHub/bitcoin/test/functional/combine_logs.py '/tmp/bitcoin_func_test_02p7o1c9' to consolidate all logs
  2021-07-14T13:21:24.092000Z TestFramework (ERROR):
  2021-07-14T13:21:24.092000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
  2021-07-14T13:21:24.092000Z TestFramework (ERROR): https://github.com/bitcoin/bitcoin/issues
  2021-07-14T13:21:24.092000Z TestFramework (ERROR):
  ```

  This PR fixes this issue.

  Also see #20267.

ACKs for top commit:
  achow101:
    ACK 0c845e3f8995eb8dc543a63899e5633a46091b4e

Tree-SHA512: d7116a9ae30c7b7e3f55f55d2eea66f9e293c38d6757ed66d0477e4256ff5fedca5ddedafa0ef0c09f4dc1f7f973163e5a46090da26b067fdddbd9ea2ee76633
2024-03-09 03:01:27 +07:00
Wladimir J. van der Laan
953b6706f4
Merge #18788: tests: Update more tests to work with descriptor wallets
c7b7e0a69265946aecc885be911c7650911ba2e3 tests: Make only desc wallets for wallet_multwallet.py --descriptors (Andrew Chow)
d4b67ad214ada7645c4ce2d5ec336fe5c3f7f7ca Avoid creating legacy wallets in wallet_importdescriptors.py (Andrew Chow)
6c9c12bf87f95066acc28ea2270a00196eb77703 Update feature_backwards_compatibility for descriptor wallets (Andrew Chow)
9a4c631e1c00eb1661c000978b133d7aa0226290 Update wallet_labels.py to not require descriptors=False (Andrew Chow)
242aed7cc1d003e8fed574bbebd19c7e54e23402 tests: Add a --legacy-wallet that is mutually exclusive with --descriptors (Andrew Chow)
388053e1722632c2e485c56a444bc75cf0152188 Disable some tests for tool_wallet when descriptors (Andrew Chow)
47d3243160fdec7e464cfb8f869be7f5d4ee25fe Make raw multisig tests legacy wallet only in rpc_rawtransaction.py (Andrew Chow)
59d3da5bce4ebd9c2291d8f201a53ee087938b21 Do addmultisigaddress tests in legacy wallet mode in wallet_address_types.py (Andrew Chow)
25bc5dccbfd52691adca6edd418dd54290300c28 Use importdescriptors when in descriptor wallet mode in wallet_createwallet.py (Andrew Chow)
0bd1860300b13b12a25d330ba3a93ff2d13aa379 Avoid dumpprivkey and watchonly behavior in rpc_signrawtransaction.py (Andrew Chow)
08067aebfd7e838e6ce6b030c31a69422260fc6f Add script equivalent of functions in address.py (Andrew Chow)
86968882a8a26312a7af29c572313c4aff488c11 Add descriptor wallet output to tool_wallet.py (Andrew Chow)
3457679870e8eff2a7d14fe59a479692738c48b6 Use separate watchonly wallet for multisig in feature_nulldummy.py (Andrew Chow)
a42652ec10c733a5bf37e418e45d4841f54331b4 Move import and watchonly tests to be legacy wallet only in wallet_balance.py (Andrew Chow)
4b871909d6e4a51888e062d322bf53263deda15e Use importdescriptors for descriptor wallets in wallet_bumpfee.py (Andrew Chow)
c2711e4230d9a423ead24f6609691fb338b1d26b Avoid dumpprivkey in wallet_listsinceblock.py (Andrew Chow)
553dbf9af4dea96e6a3e79bba9607003342029bd Make import tests in wallet_listtransactions.py legacy wallet only (Andrew Chow)
dc81418fd01021070f3f66bab5fee1484456691a Use a separate watchonly wallet in rpc_fundrawtransaction.py (Andrew Chow)
a357111047411f18c156cd34a002a38430f2901c Update wallet_importprunedfunds to avoid dumpprivkey (Andrew Chow)

Pull request description:

  I went through all the tests and checked whether they passed with descriptor wallets. This partially informed some changes in #16528. Some tests needed changes to work with descriptor wallets. These were primarily due to import and watchonly behavior. There are some tests and test cases that only test legacy wallet behavior so those tests won't be run with descriptor wallets.

  This PR updates more tests to have to the `--descriptors` switch in `test_runner.py`. Additionally a mutually exclusive `--legacy-wallet` option has been added to force legacy wallets. This does nothing currently but will be useful in the future when descriptor wallets are the default. For the tests that rely on legacy wallet behavior, this option is being set so that we don't forget in the future. Those tests are `feature_segwit.py`, `wallet_watchonly.py`, `wallet_implicitsegwit.py`, `wallet_import_with_label.py`, and `wallet_import_with_label.py`.

  If you invert the `--descriptors`/`--legacy-wallet` default so that descriptor wallets are the default, all tests (besides the legacy wallet specific ones) will pass.

ACKs for top commit:
  MarcoFalke:
    review ACK c7b7e0a69265946aecc885be911c7650911ba2e3 🎿
  laanwj:
    ACK c7b7e0a69265946aecc885be911c7650911ba2e3

Tree-SHA512: 2f4e87815005d1d0a2543ea7947f7cd7593d8cf5312228ef85f8e096f19739b225769961943049cb44f6f07a35b8de988e2246ab9aca5bb5a0b2e62694d5637d
2024-03-09 03:01:24 +07:00
Samuel Dobson
56d9fe9510
Merge #20226: wallet, rpc: add listdescriptors command
647b81b70938dc4dbcf32399c56f78be395c721a wallet, rpc: add listdescriptors command (Ivan Metlushko)

Pull request description:

  Looking for concept ACKs

  **Rationale**: allow users to inspect the contents of their newly created descriptor wallets.

  Currently the command only returns xpubs which is not very useful in itself, but there are multiples ways to extend it:
   * add an option to export xprv
   * with #19136 it'll be possible to return normalised descriptors suitable for a watch-only purposes

  The output is compatible with `importdescriptors` command so it could be easily used for backup/recover purposes.

  **Output example:**
  ```json
  [
    {
      "desc": "wpkh(tpubD6NzVbkrYhZ4WW6E2ZETFyNfq2hfF23SKxqSGFvUpPAY58jmmuBybwqwFihAyQPk9KnwTt5516NDZRJ7k5QPeKjy7wuVd5WvXNxwwAs5tUD/*)#nhavpr5h",
      "timestamp": 1296688602,
      "active": false,
      "range": [
        0,
        999
      ],
      "next": 0
    }
  ]
  ```

ACKs for top commit:
  jonatack:
    re-ACK 647b81b70938dc4dbcf32399c56f78be395c721a rebased to master, debug builds cleanly, reviewed diff since last review, tested with a descriptor wallet (and with a legacy wallet)
  achow101:
    re-ACK 647b81b

Tree-SHA512: 51a3620bb17c836c52cecb066d4fa9d5ff418af56809046eaee0528c4dc240a4e90fff5711ba96e399c6664e00b9ee8194e33852b1b9e75af18061296e19a8a7
2024-03-09 03:00:29 +07:00
Samuel Dobson
9600f9ca35
Merge #19441: walletdb: don't reinitialize desc cache with multiple cache entries
a66a7a1a7060bb422eba3b8c214852416c4280d1 walletdb: don't reinitialize desc cache with multiple cache entries (Andrew Chow)

Pull request description:

  When loading descriptor caches, we would accidentally reinitialize the descriptor cache when seeing that one already exists. This should have only been initializing the cache when one does not exist. However this code itself is unnecessary as the act of looking up the cache to add to it will initialize it if it didn't already exist.

  This issue could be hit by trying to load a wallet that had imported a multisig descriptor. The wallet would fail to load.

  A test has been added to wallet_importdescriptors.py to catch this case. Another test case has also been added to check that loading a wallet with only single key descriptors works.

ACKs for top commit:
  hugohn:
    tACK [a66a7a1](a66a7a1a70)
  jonatack:
    ACK a66a7a1a706
  meshcollider:
    Code review ACK a66a7a1a7060bb422eba3b8c214852416c4280d1

Tree-SHA512: 3df746421a008708eaa3bbbdd12b9ddd3e2ec111d54625a212dca7414b971cc1f6e2b1757b3232c31a2f637d1b1ef43bf3ffa4ac4216646cf1e92db5f79954f1
2024-03-09 03:00:28 +07:00
MarcoFalke
a1e3885a68
Merge #19239: tests: move generate_wif_key to wallet_util.py
3a83a01694160f2e722e1bc90a328bd569b8e109 [tests] move generate_wif_key to wallet_util.py (John Newbery)
b216b0b71f7853d747af8b712fc250c699f3c320 [tests] sort imports in rpc_createmultisig.py (John Newbery)
e38081846d889fcbbbc6ca4a4d3bca26807fde2f Revert "[TESTS] Move base58 to own module to break circular dependency" (John Newbery)

Pull request description:

  generate_wif_key is a wallet utility function. Move it from the EC key module to the wallet util module.

  This fixes the circular dependency issue in #17977

ACKs for top commit:
  MarcoFalke:
    ACK 3a83a01694160f2e722e1bc90a328bd569b8e109 🍪

Tree-SHA512: 24985dffb75202721ccc0c6c5b52f1fa5d1ce7963bccde24389feb913cab4dad0c265274ca67892c46c8b64e6a065a0f23263a89be4fb9134dfefbdbe5c7238a
2024-03-09 03:00:27 +07:00