backport: bitcoin#9381, #17219, #18671, #18853, #20230, partial #11403 (descriptor wallets preparation) #5580
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
knst
changed the title
knst
changed the title
knst
force-pushed
the
bp-descriptors-pre
branch
2 times, most recently
from
a4dc356
to
d432ec7
Compare
knst
changed the title
knst
force-pushed
the
bp-descriptors-pre
branch
2 times, most recently
from
ffe0e46
to
09781ea
Compare
UdjinM6
force-pushed
the
bp-descriptors-pre
branch
from
09781ea
to
9d13c53
Compare
|
rebased from GH GUI to fix |
knst
changed the title
PastaPastaPasta
approved these changes
Dec 6, 2023
…r cache and cache xpubs
…SegWit wallet support
…LegacyScriptPubKeyMan Some changes are missing or incorrectly backported during CWallet refactoring in bitcoin#17260, bitcoin#17261 such as: - Missing changes for CWallet::GetOldestKeyPoolTime - useless check of spk_man existance in getnewaddress - GetHDChain is used assuming it exists only legacy keymanager - using internal spk_man API instead wallet's in getwalletinfo
…mpwallet fa60afc wallet: Add BlockUntilSyncedToCurrentChain to dumpwallet (MarcoFalke) Pull request description: dumpwallet includes the block hash in the output, so this method depends on the chainstate. According to the developer notes https://github.com/bitcoin/bitcoin/blame/e84a5f000493fe39adb2a5f22b43c3848dcd0a4f/doc/developer-notes.md#L1095 it must include a `BlockUntilSyncedToCurrentChain`. This is a minor fix and does not need backport, I think. It fixes test failures such as https://travis-ci.org/github/bitcoin/bitcoin/jobs/675487097#L2657 , which can only happen in master because the test was not backported. ACKs for top commit: promag: Code review ACK fa60afc. ryanofsky: Code review ACK fa60afc meshcollider: utACK fa60afc Tree-SHA512: 8df70b06b226b2cdf880dec9264adb72d66fd81b09b404fd1665a79e5f5236d26122eebf15df00fe71ee292b5c91b2dc23a0a42b2aa50a8d690604b23832723f
…ool is empty 92bcd70 [wallet] allow transaction without change if keypool is empty (Sjors Provoost) 709f868 [wallet] CreateTransaction: simplify change address check (Sjors Provoost) 5efc25f [wallet] translate "Keypool ran out" message (Sjors Provoost) Pull request description: Extracted from bitcoin#16944 First this PR simplifies the check when generating a change address, by dropping `CanGetAddresses` and just letting `reservedest.GetReservedDestination` do this check. Second, when the keypool is empty, instead of immediately giving up, we create a dummy change address and pass that to coin selection. If we didn't need the change address (e.g. when spending the entire balance), then it's all good. If we did need a change address, we throw the original error. ACKs for top commit: fjahr: Code review ACK 92bcd70 jonasschnelli: utACK 92bcd70 achow101: ACK 92bcd70 meshcollider: Code review ACK 92bcd70 Tree-SHA512: 07b8c8251f57061c58a85ebf0359be63583c23bac7a2c4cefdc14820c0cdebcc90a2bb218e5ede0db11d1e204cda149e056dfd18614642070b3d56efe2735006
…true fa47cf9 wallet: Fix typo in assert that is compile-time true (MarcoFalke) Pull request description: Commit 92bcd70 presumably added a check that a `dest` of type `CNoDestination` implies an empty `scriptChange`. However, it accidentally checked for `boost::variant::empty`, which always returns false: https://www.boost.org/doc/libs/1_72_0/doc/html/boost/variant.html#id-1_3_46_5_4_1_1_16_2-bb ACKs for top commit: Sjors: utACK fa47cf9 Tree-SHA512: 9626b1e2947039853703932a362c2ee204e002d3344856eb93eef0e0f833401336f2dfa80fd43b83c8ec6eac624e6302aee771fb67aec436ba6483be02b8d615
28b112e Get rid of BindWallet (Russell Yanofsky) d002f9d Disable CWalletTx copy constructor (Russell Yanofsky) 65b9d8f Avoid copying CWalletTx in LoadToWallet (Russell Yanofsky) bd2fbc7 Get rid of unneeded CWalletTx::Init parameter (Russell Yanofsky) 2b9cba2 Remove CWalletTx merging logic from AddToWallet (Russell Yanofsky) Pull request description: This is a pure refactoring, no behavior is changing. Instead of AddToWallet taking a temporary CWalletTx object and then potentially merging it with a pre-existing CWalletTx, have it take a callback so callers can update the pre-existing CWalletTx directly. This makes AddToWallet simpler because now it is only has to be concerned with saving CWalletTx objects and not merging them. This makes AddToWallet calls clearer because they can now make direct updates to CWalletTx entries without having to make temporary objects and then worry about how they will be merged. Motivation for this change came from the bumpfee PR bitcoin#8456 where we wanted to be able to call AddToWallet to make a simple update to an existing transaction, but were reluctant to, because the existing CWalletTx merging logic did not apply and seemed dangerous try to update as part of that PR. After this refactoring, the bumpfee PR could call AddToWallet safely instead of implementing a duplicate AddToWallet function. This also allows getting rid of the CWalletTx copy constructor to prevent unintentional copying. ACKs for top commit: MarcoFalke: Anyway, re-ACK 28b112e Tree-SHA512: 528dd088714472a237500b200f4433db850bdb7fc29c5e5d81cae48072061dfb967f7c37edd90b33f24901239f9be982988547c1f8c80abc25fb243fbf7330ef
…dds missing changes for wallet_multiwallet.py test 4dca7d0 appveyor: Enable multiwallet test (Chun Kuan Lee) Pull request description: Based on bitcoin#14320 This PR enable multiwallet test on appveyor. Also re-enable symlink tests on Windows which is available after Windows Vista. I disable these tests in bitcoin#13964 because I suppose that Windows does not support symlink, but I was wrong. Tree-SHA512: 852cd4dedf36ec9c34aff8926cb34e6a560aea0bb9170c7a2264fc292dbb605622d561568d8df39aeb90d3d2bb700901d218ea7e7c5e21d84827c40d6370b369
…et cannot get address bf6855a wallet: Fix bug when just created encrypted wallet cannot get address (Hennadii Stepanov) Pull request description: Fix bitcoin-core/gui#105 ACKs for top commit: achow101: Tested ACK bf6855a kristapsk: ACK bf6855a meshcollider: Tested ACK bf6855a Tree-SHA512: eca0ab306d7206f2e5db568e83217bd854caac104379f4d8fb261db832d4d6310cbb1eab44ce9b05a5ac2eb5879a623b729752a88810f8370c24518a8d81292d
…now it's possible to remove workaround
PastaPastaPasta
force-pushed
the
bp-descriptors-pre
branch
from
9d13c53
to
da212ad
Compare
PastaPastaPasta
added a commit
that referenced
this pull request
Mar 6, 2024
…itcoin#18787, bitcoin#18805, bitcoin#18888, bitcoin#19502, bitcoin#19077, bitcoin#20125, bitcoin#20153, bitcoin#20198, bitcoin#20262, bitcoin#20266, bitcoin#23608, bitcoin#29510 - native descriptor wallets 6b71f27 Merge bitcoin#29510: wallet: `getrawchangeaddress` and `getnewaddress` failures should not affect keypools for descriptor wallets (Ava Chow) 85fa370 refactor: use Params().ExtCoinType() for descriptor wallets (Konstantin Akimov) da8e563 fix: skip functional tests which requires BDB if no bdb (see 20267) (Konstantin Akimov) 4ba44fa fix: skip interface_zmq.py which is not ready to work without bdb (Konstantin Akimov) 45fc8a4 fix: autobackup influences an exclusive locks made by SQLite (Konstantin Akimov) e542cd2 fix: missing changes from bitcoin#21634 (Konstantin Akimov) 2de7aec Merge bitcoin#19502: Bugfix: Wallet: Soft-fail exceptions within ListWalletDir file checks (Samuel Dobson) c172605 Merge bitcoin#19077: wallet: Add sqlite as an alternative wallet database and use it for new descriptor wallets (Samuel Dobson) 2439247 Merge bitcoin#23608: test: fix `feature_rbf.py --descriptors` and add to test runner (fanquake) f6b3614 fix: descriptor wallets follow-up to merge bitcoin#20202: Make BDB support optional (Konstantin Akimov) a340ad6 Merge bitcoin#20262: tests: Skip --descriptor tests if sqlite is not compiled (Samuel Dobson) 7d55046 Merge bitcoin#20125: rpc, wallet: Expose database format in getwalletinfo (Samuel Dobson) 343d4b0 fix: descriptor wallets follow-up for bitcoin#20156: Make sqlite support optional (compile-time) (Konstantin Akimov) fa30777 Merge bitcoin#20198: Show name, format and if uses descriptors in bitcoin-wallet tool (MarcoFalke) 14121ec Merge bitcoin#18888: test: Remove RPCOverloadWrapper boilerplate (MarcoFalke) b18351e Merge bitcoin#20153: wallet: do not import a descriptor with hardened derivations into a watch-only wallet (Wladimir J. van der Laan) c995e5d Merge bitcoin#20266: wallet: fix change detection of imported internal descriptors (Wladimir J. van der Laan) c864582 Merge bitcoin#18787: wallet: descriptor wallet release notes and cleanups (Samuel Dobson) 0949c08 Merge bitcoin#18782: wallet: Make sure no DescriptorScriptPubKeyMan or WalletDescriptor members are left uninitialized after construction (Samuel Dobson) baa6959 Merge bitcoin#18805: tests: Add missing sync_all to wallet_importdescriptors.py (MarcoFalke) 76e08f9 Merge bitcoin#18027: "PSBT Operations" dialog (Samuel Dobson) c1b94b6 fix: wallet should be unlocked before generating keys for Descriptor wallet (Konstantin Akimov) f293c04 Merge bitcoin#16528: Native Descriptor Wallets using DescriptorScriptPubKeyMan (Andrew Chow) 4064334 fix: get receiving address for Descriptor Wallets (Konstantin Akimov) bdbd0b1 chore: dashification of descriptor implementation in dash (UdjinM6) b02fc0b fix: counting calculation of internal keys for Descriptor Wallets (Konstantin Akimov) Pull request description: ## 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: - #5580 - #5807 ## What was done? backports: - bitcoin#16528 - bitcoin#18027 - bitcoin#18805 - bitcoin#18782 - bitcoin#18787 - bitcoin#20266 - bitcoin#20153 - bitcoin#18888 - bitcoin#20198 - bitcoin#20125 - bitcoin#20262 - bitcoin#23608 - bitcoin#19077 - bitcoin#19502 - bitcoin#29510 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: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone ACKs for top commit: PastaPastaPasta: Rebase looks good; utACK 6b71f27 PastaPastaPasta: utACK 6b71f27 UdjinM6: utACK 6b71f27 kwvg: utACK 6b71f27 Tree-SHA512: 776c5dfe1eec2b5bebc8d606476cd981c810ac81965b348e78c13e96fff23be500c495ae68c93f669403941c96eccdd3775f2b96572163c34175900e15549b5d
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
This PR is preparation for native descriptors PR #5579
What was done?
Applied fixes that have been missing or incorrectly backported from CWallet refactoring in bitcoin#17260, bitcoin#17261:
Fixed logic of parsing parsed Pub Key from bitcoin#18204
Plus related backports:
How Has This Been Tested?
Run unit functional test including code from #5579 (native descriptor wallets)
Breaking Changes
Checklist: