Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

backport: bitcoin#16528, #18027, #18782, #18787, #18805, #18888, #19502, #19077, #20125, #20153, #20198, #20262, #20266, #23608, #29510 - native descriptor wallets #5579

Merged
merged 26 commits into from
Mar 6, 2024

Conversation

knst
Copy link
Collaborator

@knst knst commented Sep 18, 2023

Issue being fixed or feature implemented

This PR is a batch of backports and related fixes to add a support of native descriptor wallets to Dash Core.

There're more related backports, but this PR is a minimal package of backports to get descriptor wallets working and unit/functional tests to succeed. To do: bitcoin#20226, bitcoin#21049, bitcoin#18788, bitcoin#20267, bitcoin#19230, bitcoin#19239, bitcoin#19441, bitcoin#19568, bitcoin#19979, bitcoin-core/gui#96, bitcoin#19136, bitcoin#21277, bitcoin#21063, bitcoin#21302, bitcoin#19651, bitcoin#20191, bitcoin#22446 and other.

Prior work:

What was done?

backports:

and extra fixes and missing changes for bitcoin#20156, bitcoin#20202, bitcoin#20267, bitcoin#21634 + fix of auto-backup for sqlite wallets.

How Has This Been Tested?

There're 2 new functional tests: wallet_importdescriptors.py and wallet_descriptor.py
Beside that many functional tests run twice now: using legacy wallet and descriptor wallets: wallet_hd.py, wallet_basic.py, wallet_labels.py, wallet_keypool_topup.py, wallet_avoidreuse.py, rpc_psbt.py, wallet_keypool_hd.py, rpc_createmultisig.py, wallet_encryption.py.
With bitcoin#18788 expected to more tests run.

Breaking Changes

N/A

Checklist:

  • 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
  • I have assigned this pull request to a milestone

@github-actions
Copy link

This pull request has conflicts, please rebase.

Copy link

github-actions bot commented Dec 6, 2023

This pull request has conflicts, please rebase.

Copy link

This pull request has conflicts, please rebase.

Copy link

This pull request has conflicts, please rebase.

@knst knst changed the title backport: bitcoin#16528 - native descriptor wallets backport: bitcoin#16528, #18027, #18782, #18787, #18805, #18888, #20125, #20153, #20198, #20262, #20266, #23608 - native descriptor wallets Feb 12, 2024
@knst knst force-pushed the bp-descriptors-1 branch 4 times, most recently from 3f06a81 to 058063c Compare February 16, 2024 18:34
@knst knst changed the title backport: bitcoin#16528, #18027, #18782, #18787, #18805, #18888, #20125, #20153, #20198, #20262, #20266, #23608 - native descriptor wallets backport: bitcoin#16528, #18027, #18782, #18787, #18805, #18888, #19502, #19077, #20125, #20153, #20198, #20262, #20266, #23608 - native descriptor wallets Feb 16, 2024
@knst knst marked this pull request as ready for review February 17, 2024 06:34
meshcollider and others added 19 commits March 7, 2024 01:23
…r WalletDescriptor members are left uninitialized after construction

2a78098 wallet: Make sure no WalletDescriptor members are uninitialized after construction (practicalswift)
ff046ae wallet: Make sure no DescriptorScriptPubKeyMan members are uninitialized after construction (practicalswift)

Pull request description:

  This is a small folllow-up to bitcoin#16528 ("Native Descriptor Wallets using DescriptorScriptPubKeyMan") which was merged in to `master` a couple of hours ago.

  Make sure no `DescriptorScriptPubKeyMan` or `WalletDescriptor` members are left uninitialized after construction.

  Before this change `bool m_internal` was left uninitialized when using the `DescriptorScriptPubKeyMan(WalletStorage&, WalletDescriptor&)` ctor.

  The same goes for the now initialized integers which were left uninitialized when using the `WalletDescriptor()` ctor.

ACKs for top commit:
  instagibbs:
    utACK  bitcoin@2a78098
  fjahr:
    Code review ACK 2a78098
  Sjors:
    utACK 2a78098
  achow101:
    ACK 2a78098
  brakmic:
    Code review ACK 2a78098
  meshcollider:
    utACK 2a78098

Tree-SHA512: c98e035268fdc7f65a423b73ac0cf010b0ef7c5e679b3cf170c1813efac8ab5c657dcbaf43c746770bea59e4772bfefe4caa834f1175260c39c7f35d92946ba5
…nups

ca2a096 Change SetType to SetInternal and remove m_address_type (Andrew Chow)
89b1ce1 Remove unimplemented SetCrypted from DescriptorScriptPubKeyMan (Andrew Chow)
b9073c8 rpc: createwallet warning that descriptor wallets are experimental (Andrew Chow)
610030d docs: Add release notes for descriptor wallets (Andrew Chow)

Pull request description:

  Some docs and cleanup following bitcoin#16528.

  * Added release notes to explain a bit of motivation for descriptor wallets, what was changed, and how users will be effected by it. Also mentions the caveats regarding multsigs and watchonly that we have discussed on IRC.
  * Adds a warning to `createwallet` that descriptor wallets are experimental.
  * Removed unused `SetCrypted` as suggestioned: bitcoin#16528 (comment)
  * Removed `m_address_type` as mentioned in bitcoin#18782 (comment)

ACKs for top commit:
  Sjors:
    tACK ca2a096
  instagibbs:
    utACK bitcoin@ca2a096
  meshcollider:
    utACK ca2a096

Tree-SHA512: 987188a912c191430e5d3f89bcef54ba6773692fc2d95b16a3ec11d9007ded210466ed980a3857e8b7196beef6422f07f9c85cc157f996c02d16f4dbde2e7b2a
…l descriptors

bd93fc9 Fix change detection of imported internal descriptors (Andrew Chow)

Pull request description:

  Import internal descriptors were having address book entries added which meant they would be detected as non-change. Fix this and add a test for it.

ACKs for top commit:
  laanwj:
    Code review ACK bd93fc9
  meshcollider:
    utACK bd93fc9
  promag:
    Code review ACK bd93fc9.

Tree-SHA512: 8fa9e364be317627ec171eedffdb505976c0e7f1e55bc7e8cfdffa3aeea5db24d231f55166602cd0e97a5ba621acc871de0a765c75d0c65678f83e93c3b657c5
… derivations into a watch-only wallet

538be42 wallet: fix importdescriptor silent fail (Ivan Metlushko)

Pull request description:

  Currently `importdescriptor` command will successfully import a descriptor with hardened derivations into a watch-only wallet while silently failing to expand the descriptor to fill the cache. This leads to a broken wallet state and failure to load such wallet due to missing cache on subsequent restart.

ACKs for top commit:
  laanwj:
    Code review ACK 538be42
  achow101:
    ACK 538be42
  meshcollider:
    utACK 538be42

Tree-SHA512: 4bdd0ab4437d55b3f1a79c3a300a0b186089155c020fe220a73d0cce274de47d90371d88918d39fd795f9fccf8db328f1e322d29a6062f9ce94a1c254398f004
faa26d3 test: Remove RPCOverloadWrapper boilerplate (MarcoFalke)

Pull request description:

  There are too many wrappers in test_node already, so at least the code that implements the wrappers should be as minimal as possible.

ACKs for top commit:
  laanwj:
    code review ACK faa26d3

Tree-SHA512: 94e593907de22187524e2445afb3101e40b3b599d4b4015aa8c6ca902d7586ff9daf520828759029d199a3af79e61b96b490a822a5a193ac7bf946beacb11a24
…coin-wallet tool

fa4074b Show name, format and if uses descriptors in bitcoin-wallet tool (Jonas Schnelli)

Pull request description:

ACKs for top commit:
  MarcoFalke:
    ACK fa4074b
  jonatack:
    re-ACK fa4074b

Tree-SHA512: cf6ee96ff21532fc4b0ba7a0fdfdc1fa485c9b1495447350fe65cd0bd919e0e0280613933265cdee069b8c29ccf015ac374535a70cac3d4fb89f4d08b3a03519
…info

624bab0 test: add coverage for getwalletinfo format field (Jon Atack)
5e737a0 rpc, wallet: Expose database format in getwalletinfo (João Barbosa)

Pull request description:

  Support for sqlite based wallets was added in bitcoin#19077. This PR adds the `format` key in `getwalletinfo` response, that can be `bdb` or  `sqlite`.

ACKs for top commit:
  jonatack:
    Tested ACK 624bab0
  laanwj:
    Code review ACK 624bab0.
  MarcoFalke:
    doesn't hurt ACK 624bab0
  hebasto:
    ACK 624bab0, tested on Linux Mint 20 (x86_64).
  meshcollider:
    utACK 624bab0

Tree-SHA512: a81f8530f040f6381d33e073a65f281993eccfa717424ab6e651c1203cbaf27794dcb7175570459e7fdaa211565bc060d0a3ecbe70d2b6f9c49b8d5071e4441c
…compiled

7411876 Ensure a legacy wallet for BDB format check (Andrew Chow)
5866403 Skip --descriptor tests if sqlite is not compiled (Andrew Chow)

Pull request description:

  bitcoin#20156 allows sqlite to not be compiled by configuring `--without-sqlite`. However doing so and then running the test runner will result in all of the `--descriptor` tests to fail. We should be skipping those tests if sqlite was not compiled.

ACKs for top commit:
  practicalswift:
    ACK 7411876: patch looks correct
  Sjors:
    tACK 7411876
  ryanofsky:
    Code review ACK 7411876
  hebasto:
    ACK 7411876, tested on Linux Mint 20 (x86_64), tests pass for binaries compiled with:

Tree-SHA512: 1d635a66d2b7bb865300144dfefcfdaf86133aaaa020c8f440a471476ac1205d32f2df704906ce6c2ea48ddf791c3c95055f6291340b4f7b353c1b02cab5cabe
… to test runner

b79dbe8 test: add feature_rbf.py --descriptors to test_runner.py (Sebastian Falbesoner)
166f8ec test: always rescan after importing private keys in `init_wallet` helper (Sebastian Falbesoner)

Pull request description:

  The functional test feature_rbf.py currently fails on master branch, if descriptor wallets are used (argument `--descriptors`). This is due to the fact that in this case, a call to the helper `init_wallet`

  https://github.com/bitcoin/bitcoin/blob/111c3e06b35b7867f2e0f740e988f648ac6c325d/test/functional/test_framework/test_framework.py#L428-L434

  creates a wallet without rescanning the blockchain; the test framework maps the importprivkey RPC calls to the importdescriptors RPC without rescanning by default (timestamp='now'). Fix this by always calling with `rescan=True`, which calls importdescriptors with timestamp=0. Also add `feature_rbf.py --descriptors` to the list of the test runner's calls.

  Fixes bitcoin#23563.

ACKs for top commit:
  mjdietzx:
    ACK b79dbe8

Tree-SHA512: a3f3f7a4077066e3c910919d3b5e04bc6b580c1e0a06e9a2fc258950eaea5e59c0f805c8f00432aea722609f2f7e41eebfab653471b76729c5a316825a3d8c86
…base and use it for new descriptor wallets

c4a29d0 Update wallet_multiwallet.py for descriptor and sqlite wallets (Russell Yanofsky)
310b0fd Run dumpwallet for legacy wallets only in  wallet_backup.py (Andrew Chow)
6c6639a Include sqlite3 in documentation (Andrew Chow)
f023b7c wallet: Enforce sqlite serialized threading mode (Andrew Chow)
6173269 Set and check the sqlite user version (Andrew Chow)
9d3d2d2 Use network magic as sqlite wallet application ID (Andrew Chow)
9af5de3 Use SQLite for descriptor wallets (Andrew Chow)
9b78f3c walletutil: Wallets can also be sqlite (Andrew Chow)
ac38a87 Determine wallet file type based on file magic (Andrew Chow)
6045f77 Implement SQLiteDatabase::MakeBatch (Andrew Chow)
727e6b2 Implement SQLiteDatabase::Verify (Andrew Chow)
b4df8fd Implement SQLiteDatabase::Rewrite (Andrew Chow)
010e365 Implement SQLiteDatabase::TxnBegin, TxnCommit, and TxnAbort (Andrew Chow)
ac5c161 Implement SQLiteDatabase::Backup (Andrew Chow)
f6f9cd6 Implement SQLiteBatch::StartCursor, ReadAtCursor, and CloseCursor (Andrew Chow)
bf90e03 Implement SQLiteBatch::ReadKey, WriteKey, EraseKey, and HasKey (Andrew Chow)
7aa4562 Add SetupSQLStatements (Andrew Chow)
6636a26 Implement SQLiteBatch::Close (Andrew Chow)
9382535 Implement SQLiteDatabase::Close (Andrew Chow)
a0de833 Implement SQLiteDatabase::Open (Andrew Chow)
3bfa0fe Initialize and Shutdown sqlite3 globals (Andrew Chow)
5a488b3 Constructors, destructors, and relevant private fields for SQLiteDatabase/Batch (Andrew Chow)
ca8b7e0 Implement SQLiteDatabaseVersion (Andrew Chow)
7577b6e Add SQLiteDatabase and SQLiteBatch dummy classes (Andrew Chow)
e87df82 Add sqlite to travis and depends (Andrew Chow)
54729f3 Add libsqlite3 (Andrew Chow)

Pull request description:

  This PR adds a new class `SQLiteDatabase` which is a subclass of `WalletDatabase`. This provides access to a SQLite database that is used to store the wallet records. To keep compatibility with BDB and to complexity of the change down, we don't make use of many SQLite's features. We use it strictly as a key-value store. We create a table `main` which has two columns, `key` and `value` both with the type `blob`.

  For new descriptor wallets, we will create a `SQLiteDatabase` instead of a `BerkeleyDatabase`. There is no requirement that all SQLite wallets are descriptor wallets, nor is there a requirement that all descriptor wallets be SQLite wallets. This allows for existing descriptor wallets to work as well as keeping open the option to migrate existing wallets to SQLite.

  We keep the name `wallet.dat` for SQLite wallets. We are able to determine which database type to use by searching for specific magic bytes in the `wallet.dat` file. SQLite begins it's files with a null terminated string `SQLite format 3`. BDB has `0x00053162` at byte 12 (note that the byte order of this integer depends on the system endianness). So when we see that there is a `wallet.dat` file that we want to open, we check for the magic bytes to determine which database system to use.

  I decided to keep the `wallet.dat` naming to keep things like backup script to continue to function as they won't need to be modified to look for a different file name. It also simplifies a couple of things in the implementation and the tests as `wallet.dat` is something that is specifically being looked for. If we don't want this behavior, then I do have another branch which creates `wallet.sqlite` files instead, but I find that this direction is easier.

ACKs for top commit:
  Sjors:
    re-utACK c4a29d0
  promag:
    Tested ACK c4a29d0.
  fjahr:
    reACK c4a29d0
  S3RK:
    Re-review ACK c4a29d0
  meshcollider:
    re-utACK c4a29d0
  hebasto:
    re-ACK c4a29d0, only rebased since my [previous](bitcoin#19077 (review)) review, verified with `git range-diff master d18892d c4a29d0`.
  ryanofsky:
    Code review ACK c4a29d0. I am honestly confused about reasons for locking into `wallet.dat` again when it's so easy now to use a clean format. I assume I'm just very dense, or there's some unstated reason, because the only thing that's been brought up are unrealistic compatibility scenarios (all require actively creating a wallet with non-default descriptor+sqlite option, then trying to using the descriptor+sqlite wallets with old software or scripts and ignoring the results) that we didn't pay attention to with previous PRs like bitcoin#11687, which did not require any active interfaction.
  jonatack:
    ACK c4a29d0, debug builds and test runs after rebase to latest master @ c2c4dba, some manual testing creating, using, unloading and reloading a few different new sqlite descriptor wallets over several node restarts/shutdowns.

Tree-SHA512: 19145732e5001484947352d3175a660b5102bc6e833f227a55bd41b9b2f4d92737bbed7cead64b75b509decf9e1408cd81c185ab1fb4b90561aee427c4f9751c
…WalletDir file checks

24d2d33 QA: wallet_multiwallet: Check that recursive symlink directory and wallet.dat loops are ignored (Luke Dashjr)
69f59af Bugfix: Wallet: Soft-fail exceptions within ListWalletDir file checks (Luke Dashjr)

Pull request description:

  Previously, an exception would be thrown, which could kill the node in some circumstances.

  Includes test changes to cause failure.

  Review with `?w=1`

ACKs for top commit:
  hebasto:
    re-ACK 24d2d33, rebased only since my [previous](bitcoin#19502 (review)) review.
  promag:
    Tested ACK 24d2d33, test change fails on master.
  meshcollider:
    utACK 24d2d33

Tree-SHA512: f701f81b3aa3d3e15cee52ac9e7c31a73c0d8166e56bf077235294507cbcee099829fedc432a1c4b6d8780885f4e37897b44b980b08125771de3c849c000499e
It happens if dash is build with support of both SQLite and BDB
Skipped tests:
 - feature_filelock
 - wallet_descriptors
 - interface_bitcoin_cli
And all functional tests which explitely specify they are using non-descriptor wallets
…` failures should not affect keypools for descriptor wallets

e073f1d test: make sure keypool sizes do not change on `getrawchangeaddress`/`getnewaddress` failures (UdjinM6)
367bb7a wallet: Avoid updating `ReserveDestination::nIndex` when `GetReservedDestination` fails (UdjinM6)

Pull request description:

  I think the expected behaviour of `getrawchangeaddress` and `getnewaddress` RPCs is that their failure should not affect keypool in any way. At least that's how legacy wallets work, you can confirm this behaviour by running `wallet_keypool.py --legacy-wallet` on master with e073f1d applied on top. However running `wallet_keypool.py --descriptors` on the same commit results in the following failure:
  ```
    File "/path/to/bitcoin/test/functional/test_framework/test_framework.py", line 131, in main
      self.run_test()
    File "/path/to/bitcoin/test/functional/wallet_keypool.py", line 114, in run_test
      assert_equal(kp_size_before, kp_size_after)
    File "/path/to/bitcoin/test/functional/test_framework/util.py", line 57, in assert_equal
      raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
  AssertionError: not([18, 24] == [19, 24])
  ```

  This happens because we pass `nIndex` (which is a class member) into `GetReservedDestination` and since it's passed by reference we get an updated value back, so `nIndex` won't be equal `-1` anymore, no matter if the function failed or succeeded. This means that `ReturnDestination` (called by dtor of `ReserveDestination`) will try to return something we did not actually reserve.

  The fix is to simply use a temporary variable instead of a class member and only update `nIndex` when `op_address` actually has value, basically do it the same way we do for other class members (`address` and `fInternal`) already.

ACKs for top commit:
  achow101:
    ACK e073f1d
  josibake:
    ACK bitcoin@e073f1d

Tree-SHA512: 1128288a60dd4d8f306ef6f7ac66cdfeae3c9cc35c66ecada2d78fa61ac759f2a757b70fc3976ba8b5081200942b58dfabc184c01ccf911af40ba8c145344651
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rebase looks good; utACK 6b71f27

 -:  ---------- >  1:  d7482eb8a6 merge bitcoin#27985: Add support for RFC8439 variant of ChaCha20
 -:  ---------- >  2:  ff542199bc merge bitcoin#27993: Make poly1305 support incremental computation + modernize
 -:  ---------- >  3:  1b1924e3d5 merge bitcoin#28008: BIP324 ciphersuite
 -:  ---------- >  4:  7c5edf772a merge bitcoin#28267: BIP324 ciphersuite follow-up
 -:  ---------- >  5:  c2aa01cf1d merge bitcoin#28374: python cryptography required for BIP 324 functional tests
 -:  ---------- >  6:  b60c493265 merge bitcoin#28100: more Span<std::byte> modernization & follow-ups
 1:  2bf7e37f9e =  7:  b02fc0b2ce fix: counting calculation of internal keys for Descriptor Wallets
 2:  52d8a634aa =  8:  bdbd0b14a7 chore: dashification of descriptor implementation in dash
 3:  2c62724448 =  9:  4064334732 fix: get receiving address for Descriptor Wallets
 4:  6e11c0efb5 ! 10:  f293c046f4 Merge #16528: Native Descriptor Wallets using DescriptorScriptPubKeyMan
    @@ test/functional/test_framework/key.py
     @@ test/functional/test_framework/key.py: import random
      import unittest
      
    - from test_framework import secp256k1
    + from test_framework.crypto import secp256k1
     +from .address import byte_to_base58
      
      # Order of the secp256k1 curve
 5:  453ab7a8a3 = 11:  c1b94b6f52 fix: wallet should be unlocked before generating keys for Descriptor wallet
 6:  583eb90513 = 12:  76e08f9b3d Merge #18027: "PSBT Operations" dialog
 7:  bbefc9813b = 13:  baa6959068 Merge #18805: tests: Add missing sync_all to wallet_importdescriptors.py
 8:  82011f72ea = 14:  0949c08996 Merge #18782: wallet: Make sure no DescriptorScriptPubKeyMan or WalletDescriptor members are left uninitialized after construction
 9:  f988bb65f2 = 15:  c86458250c Merge #18787: wallet: descriptor wallet release notes and cleanups
10:  5d31560ae5 = 16:  c995e5d957 Merge #20266: wallet: fix change detection of imported internal descriptors
11:  49edf877ad = 17:  b18351e415 Merge #20153: wallet: do not import a descriptor with hardened derivations into a watch-only wallet
12:  a2bed66ad5 = 18:  14121ec5f3 Merge #18888: test: Remove RPCOverloadWrapper boilerplate
13:  6c975ab937 = 19:  fa30777494 Merge #20198: Show name, format and if uses descriptors in bitcoin-wallet tool
14:  4fd4bee37a = 20:  343d4b07d3 fix: descriptor wallets follow-up for bitcoin#20156: Make sqlite support optional (compile-time)
15:  98b6575a45 = 21:  7d55046dfb Merge #20125: rpc, wallet: Expose database format in getwalletinfo
16:  468463d5db = 22:  a340ad641e Merge #20262: tests: Skip --descriptor tests if sqlite is not compiled
17:  15fa4c0003 = 23:  f6b3614754 fix: descriptor wallets follow-up to merge bitcoin#20202: Make BDB support optional
18:  d033977126 = 24:  2439247e93 Merge bitcoin/bitcoin#23608: test: fix `feature_rbf.py --descriptors` and add to test runner
19:  6b407459bf = 25:  c172605cd7 Merge #19077: wallet: Add sqlite as an alternative wallet database and use it for new descriptor wallets
20:  17516bdd30 = 26:  2de7aecf6f Merge #19502: Bugfix: Wallet: Soft-fail exceptions within ListWalletDir file checks
21:  8f097b1899 = 27:  e542cd2d34 fix: missing changes from bitcoin#21634
22:  06b9a83680 = 28:  45fc8a4863 fix: autobackup influences an exclusive locks made by SQLite
23:  dfff8c9808 = 29:  4ba44fa3c9 fix: skip interface_zmq.py which is not ready to work without bdb
24:  7a41aeb807 = 30:  da8e5639ee fix: skip functional tests which requires BDB if no bdb (see 20267)
25:  764a49d4b1 = 31:  85fa37068f refactor: use Params().ExtCoinType() for descriptor wallets
26:  7462870d35 = 32:  6b71f274ae Merge bitcoin/bitcoin#29510: wallet: `getrawchangeaddress` and `getnewaddress` failures should not affect keypools for descriptor wallets

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 6b71f27

Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 6b71f27

Copy link
Collaborator

@kwvg kwvg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 6b71f27

@PastaPastaPasta PastaPastaPasta merged commit bbd1a54 into dashpay:develop Mar 6, 2024
5 of 11 checks passed
@knst knst deleted the bp-descriptors-1 branch March 8, 2024 11:38
PastaPastaPasta added a commit that referenced this pull request Mar 14, 2024
…itcoin#19239, bitcoin#19441, bitcoin#19568, bitcoin#19668, bitcoin#20226, bitcoin#21049, bitcoin#22446 (descriptor wallets part II)

85fa6e4 Merge bitcoin#22446: test: Fix wallet_listdescriptors.py if bdb is not compiled (fanquake)
9a02f7d fix: drop requirement of HD in CanGetAddresses for watch-only wallets (Konstantin Akimov)
d04c1a7 fix: unify ScriptPubKeyMan implementation with bitcoin's (Konstantin Akimov)
953b670 Merge bitcoin#18788: tests: Update more tests to work with descriptor wallets (Wladimir J. van der Laan)
da2a7ed Merge bitcoin#21049: Add release notes for listdescriptors RPC (MarcoFalke)
56d9fe9 Merge bitcoin#20226: wallet, rpc: add listdescriptors command (Samuel Dobson)
59c30e6 fix: add missing dashification for bitcoin#18067 (Konstantin Akimov)
3575a6c Merge bitcoin#19568: Wallet should not override signing errors (fanquake)
9600f9c Merge bitcoin#19441: walletdb: don't reinitialize desc cache with multiple cache entries (Samuel Dobson)
a1e3885 Merge bitcoin#19239: tests: move generate_wif_key to wallet_util.py (MarcoFalke)
eaf31ad Merge bitcoin#19230: [TESTS] Move base58 to own module to break circular dependency (MarcoFalke)
6668e21 Merge bitcoin#18974: test: Check that invalid witness destinations can not be imported (MarcoFalke)

Pull request description:

  ## Issue being fixed or feature implemented
  This PR continues supporting of Descriptor Wallets, see #5579 for prior work.
  See dashpay/dash-issues#59 for Check-list issue

  ## What was done?

  Particularly:
   - bitcoin#18974
   - bitcoin#19230
   - bitcoin#19239
   - bitcoin#19441
   - bitcoin#19568
   - bitcoin#20226
   - bitcoin#21049
   - bitcoin#18788
   - bitcoin#22446

  Beside that fixed:
   - drop requirement of HD in CanGetAddresses for watch-only wallets
   - unify ScriptPubKeyMan implementation for KeyOrigin with bitcoin's
   - minor dashification for  bitcoin#18067

  ## How Has This Been Tested?
  Run unit + functional test. Backport bitcoin#18788 enables descriptor wallets for multiple functional tests.

  ## Breaking Changes
  N/A

  ## 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 85fa6e4

Tree-SHA512: 4759e834c04d0f66e765399e3941afcaad2b63503d79a5cef2c04393bbeccaa8e85c06ae3633504c74ad7854180d4d7f3d4eb11c1c3a27ab9ac166a9811b90df
PastaPastaPasta added a commit that referenced this pull request Jul 2, 2024
bebea4b fix: auto backup issue with descriptor wallets for CJ (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  Qt CoinJoin session has problems (#5579 (review)):
   - Autobackup problems
   - False keypool depletion reporting

  dashpay/dash-issues#59

  ## What was done?
  Disables check for "remaining keys left" and "auto-backups" for non-legacy wallet.

  ## How Has This Been Tested?
  Run unit/functional test
  Try to run CJ mixing for descriptor wallet.

  ## Breaking Changes
  N/A

  ## 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 bebea4b

Tree-SHA512: 610551001d054c447ddca9451ac6d94f3d063ecf3ccfab437d99324efc5f99ff86e59d80a36f4ff4983d3c8107aa19292c021cb3210fcf51e79919387169c414
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RPC Some notable changes to RPC params/behaviour/descriptions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants