aboutsummaryrefslogtreecommitdiff
path: root/src/wallet/scriptpubkeyman.cpp
AgeCommit message (Collapse)Author
2024-04-07[clang-tidy] Enable the misc-no-recursion checkdergoegge
Co-authored-by: stickies-v <stickies-v@protonmail.com> Co-authored-by: Gloria Zhao <gloriajzhao@gmail.com>
2024-03-20wallet, descspkm: Refactor wallet descriptor generation to standalone funcAndrew Chow
2024-03-18desc spkm: Add functions to retrieve specific private keysAva Chow
2024-02-20Merge bitcoin/bitcoin#26008: wallet: cache IsMine scriptPubKeys to improve ↵fanquake
performance of descriptor wallets e041ed9b755468d205ed48b29f6c4b9e9df8bc9f wallet: Retrieve ID from loaded DescSPKM directly (Ava Chow) 39640dd34e980e69d13664ddbc2a7612a1888ab4 wallet: Use scriptPubKeyCache in GetSolvingProvider (Ava Chow) b410f68791143800968f4c628beda1c9f898b4f6 wallet: Use scriptPubKey cache in GetScriptPubKeyMans (Ava Chow) edf4e73a163739a64eb9a54cd95413583a0e5c1f wallet: Use scriptPubKey cache in IsMine (Ava Chow) 37232332bd7253422ea92a8c9eeb36b12fc84b56 wallet: Cache scriptPubKeys for all DescriptorSPKMs (Ava Chow) 99a0cddbc04e8bfea3748a6cb4c0107392fab94f wallet: Introduce a callback called after TopUp completes (Ava Chow) b27682593266c99507c720855cb34f5f7d363dd2 bench: Add a benchmark for ismine (Ava Chow) Pull request description: Wallets that have a ton of non-ranged descriptors (such as a migrated non-HD wallet) perform fairly poorly due to looping through all of the wallet's `ScriptPubKeyMan`s. This is done in various places, such as `IsMine`, and helper functions for fetching a `ScriptPubKeyMan` and a `SolvingProvider`. This also has a bit of a performance impact on standard descriptor wallets, although less noticeable due to the small number of SPKMs. As these functions are based on doing `IsMine` for each `ScriptPubKeyMan`, we can improve this performance by caching `IsMine` scriptPubKeys for all descriptors and use that to determine which `ScriptPubKeyMan` to actually use for those things. This cache is used exclusively and we no longer iterate the SPKMs. Also added a benchmark for `IsMine`. ACKs for top commit: ryanofsky: Code review ACK e041ed9b755468d205ed48b29f6c4b9e9df8bc9f. Just suggested changes since last review josibake: ACK https://github.com/bitcoin/bitcoin/pull/26008/commits/e041ed9b755468d205ed48b29f6c4b9e9df8bc9f furszy: Code review ACK e041ed9b Tree-SHA512: 8e7081991a025e682e9dea838b4543b0d179832d1c47397fb9fe7a97fa01eb699c15a5d5a785634926844fc83a46e6ac07ef753119f39d84423220ef8a548894
2024-02-16wallet: Introduce a callback called after TopUp completesAva Chow
After TopUp completes, the wallet containing each SPKM will want to know what new scriptPubKeys were generated. In order for all TopUp calls (including ones internal the the SPKM), we use a callback function in the WalletStorage interface.
2024-02-06Merge bitcoin/bitcoin#28833: wallet: refactor: remove unused `SignatureData` ↵Ava Chow
instances in spkm's `FillPSBT` methods e2ad343f69af4f924b22dccf94a52b6431ef6e3c wallet: remove unused `SignatureData` instances in spkm's `FillPSBT` methods (Sebastian Falbesoner) Pull request description: These are filled with signature data from a PSBT input, but not used anywhere after, hence they can be removed. Note that the same code is in the `SignPSBTInput` function where the `sigdata` result is indeed used. ACKs for top commit: achow101: ACK e2ad343f69af4f924b22dccf94a52b6431ef6e3c brunoerg: crACK e2ad343f69af4f924b22dccf94a52b6431ef6e3c Tree-SHA512: f0cabcc000bcea6bc7d7ec9d3be2e2a8accbdbffbe35252250ea2305b65a5813fde2b8096fbdd2c7cccdf417ea285183dc325fc2d210d88bce62978ce642930e
2024-02-03wallet: remove unused 'accept_no_keys' arg from decryption processfurszy
The wallet decryption process (CheckDecryptionKey() and Unlock()) contains an arg 'accept_no_keys,' introduced in #13926, that has never been used. Additionally, this also removes the unimplemented SplitWalletPath function.
2024-01-22wallet: remove unused `SignatureData` instances in spkm's `FillPSBT` methodsSebastian Falbesoner
These are filled with signature data from a PSBT input, but not used anywhere after, hence they can be removed.
2024-01-18wallet: avoid returning a reference to vMasterKey after releasing the mutex ↵Vasil Dimov
that guards it `CWallet::GetEncryptionKey()` would return a reference to the internal `CWallet::vMasterKey`, guarded by `CWallet::cs_wallet`, which is unsafe. Returning a copy would be a shorter solution, but could have security implications of the master key remaining somewhere in the memory even after `CWallet::Lock()` (the current calls to `CWallet::GetEncryptionKey()` are safe, but that is not future proof). So, instead of `EncryptSecret(m_storage.GetEncryptionKey(), ...)` change the `GetEncryptionKey()` method to provide the encryption key to a given callback: `m_storage.WithEncryptionKey([](const CKeyingMaterial& k) { EncryptSecret(k, ...); })` This silences the following (clang 18): ``` wallet/wallet.cpp:3520:12: error: returning variable 'vMasterKey' by reference requires holding mutex 'cs_wallet' [-Werror,-Wthread-safety-reference-return] 3520 | return vMasterKey; | ^ ```
2024-01-16refactor: Allow std::span construction from CKeyMarcoFalke
2023-12-23refactor: share and use `GenerateRandomKey` helperSebastian Falbesoner
Making the `GenerateRandomKey` helper available to other modules via key.{h.cpp} allows us to create random private keys directly at instantiation of CKey, in contrast to the two-step process of creating the instance and then having to call `MakeNewKey(...)`.
2023-12-14Merge bitcoin/bitcoin#28920: wallet: birth time update during tx scanningAva Chow
1ce45baed7dd2da3f1cb85c9c25110e5537451ae rpc: getwalletinfo, return wallet 'birthtime' (furszy) 83c66444d0604f0a9ec3bc3f89d4f1a810b7cda0 test: coverage for wallet birth time interaction with -reindex (furszy) 6f497377aa17cb8a590fd7717fa8ededf4249999 wallet: fix legacy spkm default birth time (furszy) 75fbf444c1e13c6ba0e79a34871534c845a13849 wallet: birth time update during tx scanning (furszy) b4306e3c8db6cbaedc8845c6d21c750b39f682bf refactor: rename FirstKeyTimeChanged to MaybeUpdateBirthTime (furszy) Pull request description: Fixing #28897. As the user may have imported a descriptor with a timestamp newer than the actual birth time of the first key (by setting 'timestamp=now'), the wallet needs to update the birth time when it detects a transaction older than the oldest descriptor timestamp. Testing Notes: Can cherry-pick the test commit on top of master. It will fail there. ACKs for top commit: Sjors: re-utACK 1ce45baed7dd2da3f1cb85c9c25110e5537451ae achow101: ACK 1ce45baed7dd2da3f1cb85c9c25110e5537451ae Tree-SHA512: 10c2382f87356ae9ea3fcb637d7edc5ed0e51e13cc2729c314c9ffb57c684b9ac3c4b757b85810c0a674020b7287c43d3be8273bcf75e2aff0cc1c037f1159f9
2023-12-05wallet: fix legacy spkm default birth timefurszy
To avoid scanning blocks, as assumed by a wallet with no generated keys or imported scripts, the default value for the birth time needs to be set to the maximum int64_t value. Once the first key is generated or the first script is imported, the legacy SPKM will update the birth time automatically.
2023-11-21wallet: descriptors setup, batch db operationsfurszy
Instead of doing one db transaction per descriptor setup, batch all descriptors' setup writes in a single db txn. Speeding up the process and preventing the wallet from entering an inconsistent state if any of the intermediate transactions fail.
2023-11-21wallet: batch legacy spkm TopUpfurszy
Instead of performing multiple atomic write operations per legacy spkm setup call, batch them all within a single atomic db txn.
2023-11-21wallet: batch descriptor spkm TopUpfurszy
Instead of performing multiple atomic write operations per descriptor setup call, batch them all within a single atomic db txn.
2023-11-05wallet: cache descriptor ID to avoid repeated descriptor string creationSebastian Falbesoner
Right now a wallet descriptor is converted to it's string representation (via `Descriptor::ToString`) repeatedly at different instances: - on finding a `DescriptorScriptPubKeyMan` for a given descriptor (`CWallet::GetDescriptorScriptPubKeyMan`, e.g. used by the `importdescriptors` RPC); the string representation is created once for each spkm in the wallet and at each iteration again for the searched descriptor (`DescriptorScriptPubKeyMan::HasWalletDescriptor`) - whenever `DescriptorScriptPubKeyMan::GetID()` is called, e.g. in `TopUp` or any instances where a descriptor is written to the DB to determine the database key etc. As there is no good reason to calculate a fixed descriptor's string/ID more than once, add the ID as a field to `WalletDescriptor` and calculate it immediately at initialization (or deserialization). `HasWalletDescriptor` is changed to compare the spkm's and searched descriptor's ID instead of the string to take use of that. This speeds up the functional test `wallet_miniscript.py` by a factor of 5-6x on my machine (3m30.95s on master vs. 0m38.02s on PR). The recently introduced "max-size TapMiniscript" test-case introduced a descriptor that takes 2-3 seconds to create a string representation, so the repeated calls to that were significantly hurting the performance.
2023-10-12tidy: modernize-use-emplaceMarcoFalke
2023-09-19Merge bitcoin/bitcoin#28125: wallet: bugfix, disallow migration of invalid ↵Andrew Chow
scripts 8e7e3e614955e60d3bf9e9a481ef8916bf9e22d9 test: wallet, verify migration doesn't crash for an invalid script (furszy) 1de8a2372ab39386e689b27d15c4d029be239319 wallet: disallow migration of invalid or not-watched scripts (furszy) Pull request description: Fixing #28057. The legacy wallet allows to import any raw script (#28126), without checking if it was valid or not. Appending it to the watch-only set. This causes a crash in the migration process because we are only expecting to find valid scripts inside the legacy spkm. These stored scripts internally map to `ISMINE_NO` (same as if they weren't stored at all..). So we need to check for these special case, and take into account that the legacy spkm could be storing invalid not watched scripts. Which, in code words, means `IsMineInner()` returning `IsMineResult::INVALID` for them. Note: To verify this, can run the test commit on top of master. `wallet_migration.py` will crash without the bugfix commit. ACKs for top commit: achow101: ACK 8e7e3e614955e60d3bf9e9a481ef8916bf9e22d9 Tree-SHA512: c2070e8ba78037a8f573b05bf6caa672803188f05429adf5b93f9fc1493faedadecdf018dee9ead27c656710558c849c5da8ca5f6f3bc9c23b3c4275d2fb50c7
2023-08-14Rename script/standard.{cpp/h} to script/solver.{cpp/h}Andrew Chow
Since script/standard only contains things that are used by the Solver and its callers, rename the files to script/solver.
2023-08-14Clean up things that include script/standard.hAndrew Chow
Remove standard.h from files that don't use anything in it, and include it in files that do.
2023-08-14Move CScriptID to script.{h/cpp}Andrew Chow
CScriptID should be next to CScript just as CKeyID is next to CPubKey
2023-08-10wallet: disallow migration of invalid or not-watched scriptsfurszy
The legacy wallet allowed to import any raw script, without checking if it was valid or not. Appending it to the watch-only set. This causes a crash in the migration process because we are only expecting to find valid scripts inside the legacy spkm. These stored scripts internally map to `ISMINE_NO` (same as if they weren't stored at all..). So we need to check for these special case, and take into account that the legacy spkm could be storing invalid not watched scripts. Which, in code words, means IsMineInner() returning IsMineResult::INVALID for them.
2023-06-28refactor: extract descriptor ID calculation from spkm GetID()furszy
This allows us to verify the descriptor ID on the descriptors unit tests in different software versions without requiring to use the entire DescriptorScriptPubKeyMan machinery. Note: The unit test changes are introduced after the bugfix commit but this commit + the unit test commit can be cherry-picked on top of the v25 branch to verify IDs correctness. IDs must be the same for v25 and after the bugfix commit.
2023-06-14Merge bitcoin/bitcoin#25634: wallet, tests: Expand and test when the blank ↵Ryan Ofsky
wallet flag should be un/set cdba23db353a1beff831ff4fc83d01ed64e8c2a9 wallet: Document blank flag use in descriptor wallets (Ryan Ofsky) 43310200dce8d450ae5808824af788cefaa5d6db wallet: Ensure that the blank wallet flag is unset after imports (Andrew Chow) e9379f1ffa7a4eebce397f1150317e840655e021 rpc, wallet: Include information about blank flag (Andrew Chow) Pull request description: The `blank` wallet flag is used to indicate that the wallet intentionally does not have any keys, scripts, or descriptors, and it prevents the automatic generation of those things for such a wallet. Once the wallet contains any of those data, it is unnecessary, and possibly incorrect, to have `blank` set. This PR fixes a few places where this was not properly happening. It also adds a test for this unset behavior. ACKs for top commit: S3RK: reACK cdba23db353a1beff831ff4fc83d01ed64e8c2a9 ryanofsky: Code review ACK cdba23db353a1beff831ff4fc83d01ed64e8c2a9. Only change since last review is dropping the commit which makes createwallet RPC set BLANK flag automatically when DISABLE_PRIVATE_KEYS flag is set Tree-SHA512: 85bc2a9754df0531575d5c8f4ad7e8f38dcd50083dc29b3283dacf56feae842e81f34654c5e1781f2dadb0560ff80e454bbc8ca3b2d1fab1b236499ae9abd7da
2023-06-13wallet: Ensure that the blank wallet flag is unset after importsAndrew Chow
2023-05-25wallet: skip block scan if block was created before wallet birthdayfurszy
To avoid wasting processing power, we can skip blocks that occurred before the wallet's creation time, since these blocks are guaranteed not to contain any relevant wallet data. This has direct implications (an speed improvement) on the underlying blockchain synchronization process as well. The reason is that the validation interface queue is limited to 10 tasks per time. This means that no more than 10 blocks can be waiting for the wallet(s) to be processed while we are synchronizing the chain (activating the best chain to be more precise). Which can be a bottleneck if blocks arrive and are processed faster from the network than what they are processed by the wallet(s).
2023-05-08Merge bitcoin/bitcoin#26076: Switch hardened derivation marker to hAndrew Chow
fe49f06c0e91b96feb8d8f1bd478c3173f14782c doc: clarify PR 26076 release note (Sjors Provoost) bd13dc2f46ea10302a928fcf0f53b7aed77ad260 Switch hardened derivation marker to h in descriptors (Sjors Provoost) Pull request description: This makes it easier to handle descriptor strings manually, especially when importing from another Bitcoin Core wallet. For example the `importdescriptors` RPC call is easiest to use `h` as the marker: `'["desc": ".../0h/..."]'`, avoiding the need for escape characters. With this change `listdescriptors` will use `h`, so you can copy-paste the result, without having to add escape characters or switch `'` to 'h' manually. Both markers can still be parsed. The `hdkeypath` field in `getaddressinfo` is also impacted by this change, except for legacy wallets. The latter is to prevent accidentally breaking ancient software that uses our legacy wallet. See discussion in #15740 ACKs for top commit: achow101: ACK fe49f06c0e91b96feb8d8f1bd478c3173f14782c darosior: re-ACK fe49f06c0e91b96feb8d8f1bd478c3173f14782c Tree-SHA512: f78bc873b24a6f7a2bf38f5dd58f2b723e35e6b10e4d65c36ec300e2d362d475eeca6e5afa04b3037ab4bee0bf8ebc93ea5fc18102a2111d3d88fc873c08dc89
2023-04-04Switch hardened derivation marker to h in descriptorsSjors Provoost
This makes it easier to handle descriptor strings manually. E.g. an RPC call that takes an array of descriptors can now use '["desc": ".../0h/..."]'. Both markers can still be parsed. The default for new descriptors is changed to h. In normalized form h is also used. For private keys the chosen marker is preserved in a round trip. The hdkeypath field in getaddressinfo is also impacted by this change.
2023-03-31Merge bitcoin/bitcoin#27274: refactor: remove unused param from legacy ↵fanquake
pubkey interface 1869310f3cfa4ab26b5090d8a4002eefdc84870e refactor: remove unused param from legacy pubkey (Bushstar) Pull request description: Unused param present in legacy pubkey manager interface. This param will not be used and should be removed to prevent unintended usage. ACKs for top commit: Sjors: ACK 1869310f3cfa4ab26b5090d8a4002eefdc84870e furszy: ACK 1869310f3cfa4ab26b5090d8a4002eefdc84870e Tree-SHA512: 0fb41fc8f481f859262f2e8e9a93c990c1b4637e74fd9191ccc0b3c523d0e7d94217a3074bb357276e1941a10d29326f850f9b27eccc1eca57cf6b549353400c
2023-03-26clang-tidy: Add `performance-inefficient-vector-operation` checkHennadii Stepanov
https://clang.llvm.org/extra/clang-tidy/checks/performance/inefficient-vector-operation.html
2023-03-20refactor: remove unused param from legacy pubkeyBushstar
2023-02-23wallet: annotate bools in descriptor SPKM FillPSBT()Sjors Provoost
2023-02-15wallet: set keypool_size instead of access global args managerfurszy
2023-02-01Fix clang-tidy readability-const-return-type violationsMarcoFalke
2023-01-30Merge bitcoin/bitcoin#15294: refactor: Extract RipeMd160MarcoFalke
6879be691bf636a53208ef058f2ebe18bfa8017c refactor: Extract RIPEMD160 (Ben Woosley) Pull request description: To directly return a CRIPEMD160 hash from data. Simplifies the call sites. ACKs for top commit: achow101: ACK 6879be691bf636a53208ef058f2ebe18bfa8017c theStack: re-ACK 6879be691bf636a53208ef058f2ebe18bfa8017c MarcoFalke: review ACK 6879be691bf636a53208ef058f2ebe18bfa8017c 🏔 Tree-SHA512: 6ead85d8060c2ac6afd43ec716ff5a82d6754c4132fe7df3b898541fa19f1dfd8b301b2b66ae7cb7594b1b1a8c7f68bce3790a8c610d4a1164e995d89bc5ae34
2023-01-26refactor: Extract RIPEMD160Ben Woosley
To directly return a CRIPEMD160 hash from data. Incidentally, decoding this acronym: * RIPEMD -> RIPE Message Digest * RIPE -> RACE Integrity Primitives Evaluation * RACE -> Research and Development in Advanced Communications Technologies in Europe
2023-01-03refactor: use braced init for integer constants instead of c style castsPasta
2022-12-24scripted-diff: Bump copyright headersHennadii Stepanov
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT- Commits of previous years: - 2021: f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7 - 2020: fa0074e2d82928016a43ca408717154a1c70a4db - 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
2022-10-28psbt: Include output pubkey in additional pubkeys to signAndrew Chow
In addition to the pubkeys in hd_keypaths and tap_bip32_keypaths, also see if the descriptor can produce a SigningProvider for the output pubkey. Also slightly refactors this area to reduce code duplication.
2022-10-25wallet: add method for retrieving the end range for a ScriptPubKeyManSebastian Falbesoner
2022-10-25wallet: support fetching scriptPubKeys with minimum descriptor range indexSebastian Falbesoner
This extra method will be needed for updating the filter set for faster wallet rescans; after an internal top-up has happened, we only want to add the newly created scriptPubKeys.
2022-10-13Merge bitcoin/bitcoin#25526: wallet: avoid double keypool TopUp() call on ↵Andrew Chow
descriptor wallets bfb9b94ebefdb95ac7656836975b3d5afc428744 wallet: remove duplicate descriptor type check in GetNewDestination (furszy) 76b982a4a5328c1357dbc5361317f682db160876 wallet: remove unused `nAccountingEntryNumber` field (furszy) 599ff5adfc7e1227c6d97d861d0715aee57611dd wallet: avoid double TopUp() calls on descriptor wallets (furszy) Pull request description: Found it while was digging over a `getnewaddress` timeout on the functional test suite. ### Context: We are calling `TopUp()` twice in the following flows for descriptor wallets: A) `CWallet::GetNewDestination`: 1) Calls spk_man->TopUp() 2) Calls spk_man->GetNewDestination() --> which, after the basic script checks, calls TopUp() again. B) `CWallet::GetReservedDestination`: 1) Calls spk_man->TopUp() 2) Calls spk_man->GetReservedDestination() --> which calls to GetNewDestination (which calls to TopUp again). ### Changes: Move `TopUp()` responsibility from the wallet class to each scriptpubkeyman. So each spkm can decide to call it or not after perform the basic checks for the new destination request. Aside from that, remove the unused `nAccountingEntryNumber` wallet field. And a duplicated descriptor type check in `GetNewDestination` ACKs for top commit: aureleoules: re-ACK bfb9b94ebefdb95ac7656836975b3d5afc428744. achow101: ACK bfb9b94ebefdb95ac7656836975b3d5afc428744 theStack: Code-review ACK bfb9b94ebefdb95ac7656836975b3d5afc428744 Tree-SHA512: 3ab73f37729e50d6c6a4434f676855bc1fb404619d63c03e5b06ce61c292c09c59d64cb1aa3bd9277b06f26988956991d62c90f9d835884f41ed500b43a12058
2022-10-09Fix comment typos in scriptpubkeyman.cpp, wallet.cpp, wallet.hDimitris Tsapakidis
Fix comment typos: sigature -> signature ponter -> pointer it's key -> its key
2022-08-26Implement LegacyScriptPubKeyMan::DeleteRecordsAndrew Chow
2022-08-25Implement LegacyScriptPubKeyMan::MigrateToDescriptorAndrew Chow
2022-08-25scriptpubkeyman: Implement GetScriptPubKeys in LegacyAndrew Chow
2022-08-19Merge bitcoin/bitcoin#25707: refactor: Make const references to avoid ↵MacroFake
unnecessarily copying objects and enable two clang-tidy checks ae7ae36d311a869b3bda41d29dc0e47fade77d28 tidy: Enable two clang-tidy checks (Aurèle Oulès) 081b0e53e3adca7ea57d23e5fcd9db4b86415a72 refactor: Make const refs vars where applicable (Aurèle Oulès) Pull request description: I added const references to some variables to avoid unnecessarily copying objects. Also added two clang-tidy checks : [performance-for-range-copy](https://releases.llvm.org/11.1.0/tools/clang/tools/extra/docs/clang-tidy/checks/performance-for-range-copy.html) and [performance-unnecessary-copy-initialization](https://releases.llvm.org/12.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.html). ACKs for top commit: vasild: ACK ae7ae36d311a869b3bda41d29dc0e47fade77d28 MarcoFalke: review ACK ae7ae36d311a869b3bda41d29dc0e47fade77d28 Tree-SHA512: f6ac6b0cd0eee1e0c34d2f186484bc0f7ec6071451cccb33fa88a67d93d92b304e2fac378b88f087e94657745bca4e966dbc443759587400eb01b1f3061fde8c
2022-08-17Merge bitcoin/bitcoin#25748: refactor: Avoid copies in FlatSigningProvider MergeAndrew Chow
fa3f15f2dd94ae597a66037f5928fe4e90fe099d refactor: Avoid copies in FlatSigningProvider Merge (MacroFake) Pull request description: `Merge` will create several copies unconditionally: * To initialize the args `a`, and `b` * `ret`, which is the merge of the two args So change the code to let the caller decide how many copies they need/want: * `a`, and `b` must be explicitly moved or copied by the caller * `ret` is no longer needed, as `a` can be used for it in place "for free" ACKs for top commit: achow101: ACK fa3f15f2dd94ae597a66037f5928fe4e90fe099d furszy: looks good, ACK fa3f15f2 ryanofsky: Code review ACK fa3f15f2dd94ae597a66037f5928fe4e90fe099d. Confirmed that all the places `std::move` was added the argument actually did seem safe to move from. Compiler enforces that temporary copies are explicitly created in non-move cases. Tree-SHA512: 7c027ccdea1549cd9f37403344ecbb76e008adf545f6ce52996bf95e89eb7dc89af6cb31435a9289d6f2eea1c416961b2fb96348bc8a211d550728f1d99ac49c
2022-08-16Merge bitcoin/bitcoin#25734: wallet, refactor: #24584 follow-upsAndrew Chow
8cd21bb2799d37ed00dc9d0490bb5f5f1375932b refactor: improve readability for AttemptSelection (josibake) f47ff717611182da27461e29b3c23933eb22fbce test: only run test for descriptor wallets (josibake) 0760ce0b9e646b6c86f4cc890c6ab78103a242ab test: add missing BOOST_ASSERT (josibake) db09aec9378c5e8cc49c866fa50bfcb6c567d66c wallet: switch to new shuffle, erase, push_back (josibake) b6b50b0f2b055d81c5d4ff9e21dd88cdc9a88ccb scripted-diff: Uppercase function names (josibake) 3f27a2adce12c6b0e7b43ba7c024331657bcf335 refactor: add new helper methods (josibake) f5649db9d5e984ba7f376ccfd5b0a627f5c42402 refactor: add UNKNOWN OutputType (josibake) Pull request description: This PR is to address follow-ups for #24584, specifically: * Remove redundant, hard-to-read code by adding a new `OutputType` and adding shuffle, erase, and push_back methods for `CoinsResult` * Add missing `BOOST_ASSERT` to unit test * Ensure functional test only runs if using descriptor wallets * Improve readability of `AttemptSelection` by removing triple-nested if statement Note for reviewers: commit `refactor: add new helper methods` should throw an "unused function warning"; the function is used in the next commit. Also, commit `wallet: switch to new shuffle, erase, push_back` will fail to compile, but this is fixed in the next commit with a scripted-diff. the commits are separate like this (code change then scripted-diff) to improve legibility. ACKs for top commit: achow101: ACK 8cd21bb2799d37ed00dc9d0490bb5f5f1375932b aureleoules: ACK 8cd21bb2799d37ed00dc9d0490bb5f5f1375932b. LarryRuane: Concept, code review ACK 8cd21bb2799d37ed00dc9d0490bb5f5f1375932b furszy: utACK 8cd21bb2. Left a small, non-blocking, comment. Tree-SHA512: a1bbc5962833e3df4f01a4895d8bd748cc4c608c3f296fd94e8afd8797b8d2e94e7bd44d598bd76fa5c9f5536864f396fcd097348fa0bb190a49a86b0917d60e
generated by cgit v1.2.3 (git 2.26.2) at 2024-08-25 08:38:36 +0000