aboutsummaryrefslogtreecommitdiff
path: root/src/wallet
AgeCommit message (Collapse)Author
2022-07-08Merge bitcoin/bitcoin#25481: wallet: unify max signature logicAndrew Chow
d54c5c8b1b1a38b5b38e6878aea0fa8d6c1ad7e9 wallet: use CCoinControl to estimate signature size (S3RK) a94659c84ee10ac5915eb5a6b654435183d88521 wallet: replace GetTxSpendSize with CalculateMaximumSignedInputSize (S3RK) Pull request description: Currently `DummySignTx` and `DummySignInput` use different ways to determine signature size. This PR unifies the way wallet estimates signature size for various inputs. Instead of passing boolean flags from calling code the `use_max_sig` is now calculated at the place of signature creation using information available in `CCoinControl` ACKs for top commit: achow101: ACK d54c5c8b1b1a38b5b38e6878aea0fa8d6c1ad7e9 theStack: Code-review ACK d54c5c8b1b1a38b5b38e6878aea0fa8d6c1ad7e9 Tree-SHA512: e790903ad4683067070aa7dbf7434a1bd142282a5bc425112e64d88d27559f1a2cd60c68d6022feaf6b845237035cb18ece10f6243d719ba28173b69bd99110a
2022-07-08Merge bitcoin/bitcoin#25337: refactor: encapsulate wallet's address book accessAndrew Chow
d69045e291e32e02d105d1b5ff1c8b86db0ae69e test: add coverage for 'listreceivedbyaddress' no change addrs return (furszy) 324f00a6420bbd64c67c264e50632e6fa36ae732 refactor: 'ListReceived' use optional for filtered address (furszy) b459fc122feace9e9a738c48aab21961cf15dddc refactor: RPC 'ListReceived', encapsulate m_address_book access (furszy) fa9f2ab8fd53075d2a3ec93ddac4908e73525c46 refactor: RPC 'listlabels', encapsulate 'CWallet::ListAddrBookLabels' functionality (furszy) 83e42c4b94e376a19d3eb0a2379769b8b8ac5fc8 refactor: use 'ForEachAddrBookEntry' in RPC 'getaddressesbylabel' (furszy) 2b48642499016cb357e4bcec32481cd50361194e refactor: use ForEachAddrBookEntry in interfaces::getAddresses (furszy) 032842ae4196aaed5ea3567ea01a61ed75ab2edd wallet: implement ForEachAddrBookEntry method (furszy) 09649bc95d5f2855a54a8cf02e65215a3b333c92 refactor: implement general 'ListAddrBookAddresses' for addressbook destinations lookup (furszy) 192eb1e61c3c43baec7f32c498ab0ce0656a58f7 refactor: getAddress don't access m_address_book, use FindAddressEntry function (furszy) Pull request description: ### Context The wallet's `m_address_book` field is being accessed directly from several places across the sources. ### Problem Code structure wise, we shouldn't be accessing it directly. It could end up being modified by mistake (from a place that has nothing to do with the wallet like an RPC command or the GUI) and cause a bigger issue: like an address book entry 'purpose' string change, which if done badly (from 'send' to 'receive'), could end up in a user sharing a "receive" address that he/she doesn't own. ### Solution Encapsulate `m_address_book` access inside the wallet. ------------------------------------------------------- Extra Note: This is the initial step towards decoupling the address book functionality from the wallet's sources. In other words, the creation of the `AddressBookManager` (which will be coming in a follow-up PR). ACKs for top commit: achow101: ACK d69045e291e32e02d105d1b5ff1c8b86db0ae69e theStack: ACK d69045e291e32e02d105d1b5ff1c8b86db0ae69e ✅ w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25337/commits/d69045e291e32e02d105d1b5ff1c8b86db0ae69e Tree-SHA512: dba17acd86f171b4e9af0223bbbcad380048570f6a2f6a92732a51f01abe8806debaf65c9e9e5569fa76a541903cbb50adcb5f56ef77858151c698ae6b218e2a
2022-07-06Merge bitcoin/bitcoin#25507: wallet: don't add change fee to target if ↵Andrew Chow
subtracting fees from output 140d942634f9f1bba191aafa948df57812c0f3fe wallet: don't add change fee to target if subtracting fees from output (S3RK) Pull request description: Change fee is payed by the recipient, so we don't need to add it to our target for coin selection. ACKs for top commit: achow101: ACK 140d942634f9f1bba191aafa948df57812c0f3fe ishaanam: ACK 140d942634f9f1bba191aafa948df57812c0f3fe furszy: Code review ACK 140d9426 Tree-SHA512: b5efd0264c47ecee9204a3fd039bad24c69f9e614c6e1d9bb240ee5be6356b175aa074f3be123e6cfb8becd4d7bd1028eebe18801662cc69d19413d8d5a9dd5c
2022-06-30Merge bitcoin/bitcoin#25383: wallet: don't read db every time that a new ↵MacroFake
'WalletBatch' is created c318211ddd48d44dd81dded553afeee3bc41c89e walletdb: fix last client version update (furszy) bda8ebe608e6572eaaf40cd28dab6954241c9b0d wallet: don't read db every time that a new WalletBatch is created (furszy) Pull request description: Found it while was working on #25297. We are performing a db read operation every time that a new `WalletBatch` is created, inside the constructor, just to check if the client version field is inside the db or not. As the client version field does not change in the entire db lifecycle, this operation can be done only once: The first time that the db is accessed/opened and the client version value can be cached. ACKs for top commit: achow101: ACK c318211ddd48d44dd81dded553afeee3bc41c89e w0xlt: reACK https://github.com/bitcoin/bitcoin/pull/25383/commits/c318211ddd48d44dd81dded553afeee3bc41c89e Tree-SHA512: 7fb780c656e169e8eb21e7212242494a647f6506d6da2cca828703713d440d29c82bec9e7d2c410f37b49361226ccd80846d3eeb8168383d0c2a11d85d73bee2
2022-06-30wallet: don't add change fee to target if subtracting fees from outputS3RK
2022-06-29Merge bitcoin/bitcoin#25497: wallet: more accurate target for large transactionsAndrew Chow
25e4762ae7a09906020e62cd947e9f71300439cf wallet: more accurate tx_noinputs_size (S3RK) Pull request description: Rationale: more accurate non-input fee estimation for txs with >=253 inputs ACKs for top commit: laanwj: Concept and code review ACK 25e4762ae7a09906020e62cd947e9f71300439cf achow101: ACK 25e4762ae7a09906020e62cd947e9f71300439cf furszy: Code review ACK 25e4762a. left a small nit. Tree-SHA512: bda8fad725d32ad3e13c007fa56ddb6679ac1a32098ddb08d9a114054acfa681cb66cd703ac675297f731cb381b09067a99a4efa31320140bbdd03f0cfdc81af
2022-06-29wallet: more accurate tx_noinputs_sizeS3RK
2022-06-28Revert "bnb: exit selection when best_waste is 0"Murch
This reverts commit 9b5950db8683f9b4be03f79ee0aae8a780b01a4b. Waste can be negative. At feerates lower than long_term_feerate this means that a waste of 0 may be a suboptimal solution and this causes the search to exit prematurely. Only when the feerate is equal to the long_term_feerate would achieving a waste of 0 indicate that we have achieved an optimal solution, because it would mean that the excess is 0. It seems unlikely that this would ever occur outside of test cases, and even then we should prefer solutions with more inputs over solutions with fewer according to previous decisions—but solutions with more inputs are found later in the branch exploration. The "optimization" described in #18257 and implemented in #18262 is therefore a premature exit on a suboptimal solution and should be reverted.
2022-06-28Merge bitcoin/bitcoin#24924: bench: Make WalletLoading benchmark run fasterfanquake
e673d8b475995075b696208386c9e45ae7ca3e20 bench: Enable loading benchmarks depending on what's compiled (Andrew Chow) 4af3547ebac672a2d516e8696fd3580a766c27eb bench: Use mock wallet database for wallet loading benchmark (Andrew Chow) 49910f255f77e14fccf189353d188efac00d1445 sqlite: Use in-memory db instead of temp for mockdb (Andrew Chow) a1080802f8d7c3d1251ec6f2be33031f568deafa walletdb: Create a mock database of specific type (Andrew Chow) 7c0d34476df446e3825198b27c6f62bba4c0b974 bench: reduce the number of txs in wallet for wallet loading bench (Andrew Chow) f85b54ed27bd6eddb1e7035db02d542575b3ab24 bench: Add transactions directly instead of mining blocks (Andrew Chow) d94244c4bf37365272a16eb2ce6517605b4c8a47 bench: reduce number of epochs for wallet loading benchmark (Andrew Chow) 817c051364208d3f9e7e2af5700bd2bee5c9f303 bench: use unsafesqlitesync in wallet loading benchmark (Andrew Chow) 9e404a98312d73c969adf4f8e87aad1ac4b3029d bench: Remove minEpochIterations from wallet loading benchmark (Andrew Chow) Pull request description: `minEpochIterations` is probably unnecessary to set, so removing it makes the runtime much faster. ACKs for top commit: Rspigler: tACK e673d8b475995075b696208386c9e45ae7ca3e20 furszy: Code review ACK e673d8b4, nice PR. glozow: Concept ACK e673d8b475995075b696208386c9e45ae7ca3e20. For each commit, verified that there was a performance improvement without negating the purpose of the bench, and made some effort to verify that the code is correct. Tree-SHA512: 9337352ef846cf18642d5c14546c5abc1674b4975adb5dc961a1a276ca91f046b83b7a5e27ea6cd26264b96ae71151e14055579baf36afae7692ef4029800877
2022-06-28wallet: use CCoinControl to estimate signature sizeS3RK
2022-06-28wallet: replace GetTxSpendSize with CalculateMaximumSignedInputSizeS3RK
2022-06-27wallet: also search taproot pubkeys in FillPSBTAndrew Chow
When filling a PSBT, we search the listed pubkeys in order to determine whether the current DescriptorScriptPubKeyMan could sign the transaction even if it is not watching the scripts. With Taproot, the taproot pubkeys need to be searched as well.
2022-06-27psbt: Remove non_witness_utxo for segwit v1+Andrew Chow
If all inputs are segwit v1+, the non_witness_utxos can be removed.
2022-06-22refactor: 'ListReceived' use optional for filtered addressfurszy
Plus remove open bracket jump line
2022-06-22refactor: RPC 'ListReceived', encapsulate m_address_book accessfurszy
2022-06-22refactor: RPC 'listlabels', encapsulate 'CWallet::ListAddrBookLabels' ↵furszy
functionality Mainly to not access 'm_address_book' externally.
2022-06-21Merge bitcoin/bitcoin#25427: wallet: remove extra wtx lookup in AddToSpendsAndrew Chow
32e5edc0f454c59c8e0d8d86a9abfa9a3f25ca28 wallet: avoid extra wtx lookup in AddToSpends (furszy) Pull request description: As `AddToSpends` is only called from `AddToWallet` and `LoadToWallet`, places where we insert the wtx into the wallet map, we can directly feed `AddToSpends` with the `wtx` and remove another extra lookup. ACKs for top commit: laanwj: Code review ACK 32e5edc0f454c59c8e0d8d86a9abfa9a3f25ca28 achow101: ACK 32e5edc0f454c59c8e0d8d86a9abfa9a3f25ca28 theStack: Code-review ACK 32e5edc0f454c59c8e0d8d86a9abfa9a3f25ca28 w0xlt: Code Review ACK https://github.com/bitcoin/bitcoin/pull/25427/commits/32e5edc0f454c59c8e0d8d86a9abfa9a3f25ca28 brunoerg: crACK 32e5edc0f454c59c8e0d8d86a9abfa9a3f25ca28 Tree-SHA512: e9fb8df44c3e3fa26c107d261bf78e45014b4755890a64817f2be62ee6b7751f5dd2813a18dcb103a21ddba1422f9d2d59c4bf186f08314e634365d36b01be8f
2022-06-21refactor: use 'ForEachAddrBookEntry' in RPC 'getaddressesbylabel'furszy
2022-06-21refactor: use ForEachAddrBookEntry in interfaces::getAddressesfurszy
2022-06-21wallet: implement ForEachAddrBookEntry methodfurszy
2022-06-21refactor: implement general 'ListAddrBookAddresses' for addressbook ↵furszy
destinations lookup
2022-06-20wallet: avoid extra wtx lookup in AddToSpendsfurszy
This method is only called from AddToWallet and LoadToWallet, places where we already have the wtx.
2022-06-19scripted-diff: rename fAllowOtherInputs -> m_allow_other_inputsfurszy
-BEGIN VERIFY SCRIPT- sed -i 's/fAllowOtherInputs/m_allow_other_inputs/g' -- $(git grep --files-with-matches 'fAllowOtherInputs') -END VERIFY SCRIPT-
2022-06-19refactor: use GetWalletTx in SelectCoins instead of access mapWalletfurszy
2022-06-19wallet: move "use-only coinControl inputs" below the selected inputs lookupfurszy
Otherwise, RPC commands such as `walletcreatefundedpsbt` will not support the manual selection of locked, spent and externally added coins. Full explanation is inside #25118 comments but brief summary is: `vCoins` at `SelectCoins` time could not be containing the manually selected input because, even when they were selected by the user, the current `AvailableCoins` flow skips locked and spent coins. Extra note: this is an intermediate step to unify the `fAllowOtherInputs`/`m_add_inputs` concepts. It will not be a problem anymore in the future when we finally decouple the wtx-outputs lookup process from `SelectCoins` and don't skip the user's manually selected coins in `AvailableCoins`.
2022-06-19wallet: unify “allow/block other inputs“ conceptfurszy
Seeking to make the `CoinControl` option less confusing/redundant. In #16377 the `CoinControl` flag ‘m_add_inputs’ was added to tell the coin filtering and selection process two things: - Coin Filtering: Only use the provided inputs. Skip the Rest. - Coin Selection: Search the wtxs-outputs and append all the `CoinControl` selected outpoints to the selection result (skipping all the available output checks). Nothing else. Meanwhile, in `CoinControl` we already have a flag ‘fAllowOtherInputs’ which is already saying: - Coin Filtering: Only use the provided inputs. Skip the Rest. - Coin Selection: If false, no selection process -> append all the `CoinControl` selected outpoints to the selection result (while they passed all the `AvailableCoins` checks and are available in the 'vCoins' vector). As can notice, the first point in the coin filtering process is duplicated in the two option flags. And the second one, is slightly different merely because it takes into account whether the coin is on the `AvailableCoins` vector or not. So it makes sense to merge ‘m_add_inputs’ and ‘fAllowOtherInputs’ into a single field for the coin filtering process while introduce other changes to add the missing/skipped coins into 'vCoins' vector if they were manually selected by the user (follow-up commits).
2022-06-18wallet: fix warning: "argument name 'feerate' in comment does not match ↵furszy
parameter name" Happened because the "feerate=" comment was after the comma.
2022-06-17Merge bitcoin/bitcoin#25005: wallet: remove extra wtx lookup in ↵Andrew Chow
'AvailableCoins' + several code cleanups. fd5c996d1609e6f88769f6f3ef0c322e3435b3aa wallet: GetAvailableBalance, remove double walk-through every available coin (furszy) 162d4ad10f28c5fa38551d69ce9b296ab3933c77 wallet: add 'only_spendable' filter to AvailableCoins (furszy) cdf185ccfb2085e5a4bf82d833392d74b748aeff wallet: remove unused IsSpentKey(hash, index) method (furszy) 4b83bf8dbcf6b8b1c1293575391e90ac7e21b0e0 wallet: avoid extra IsSpentKey -> GetWalletTx lookups (furszy) 3d8a2822570e3cf4d1bc4f9d59b5dcb0145920ad wallet: decouple IsSpentKey(scriptPubKey) from IsSpentKey(hash, n) (furszy) a06fa94ff81e2bccef0316ea5ec4eca0f4de5071 wallet: IsSpent, 'COutPoint' arg instead of (hash, index) (furszy) 91902b77202fc636edb3db587cb6e87d9fb9b60a wallet: IsLockedCoin, 'COutPoint' arg instead of (hash, index) (furszy) 9472ca0a65396206b3078bddf98f4c1807be2d82 wallet: AvailableCoins, don't call 'wtx.tx->vout[i]' multiple times (furszy) 4ce235ef8f9a9dddc52d7ab60c8f71bda1d38873 wallet: return 'CoinsResult' struct in `AvailableCoins` (furszy) Pull request description: This started in #24845 but grew out of scope of it. So, points tackled: 1) Avoid extra `GetWalletTx` lookups inside `AvailableCoins -> IsSpentKey`. `IsSpentKey` was receiving the tx hash and index to internally lookup the tx inside the wallet's map. As all the `IsSpentKey` function callers already have the wtx available, them can provide the `scriptPubKey` directly. 2) Most of the time, we call `Wallet::AvailableCoins`, and later on the process, skip the non-spendable coins from the result in subsequent for-loops. So to speedup the process: introduced the ability to filter by "only_spendable" coins inside `Wallet::AvailableCoins` directly. (the non-spendable coins skip examples are inside `AttemptSelection->GroupOutputs` and `GetAvailableBalance`). 4) Refactored `AvailableCoins` in several ways: a) Now it will return a new struct `CoinsResult` instead of receiving the vCoins vector reference (which was being cleared at the beginning of the method anyway). --> this is coming from #24845 but cherry-picked it here too to make the following commits look nicer. b) Unified all the 'wtx.tx->vout[I]' calls into a single call (coming from this comment https://github.com/bitcoin/bitcoin/pull/24699#discussion_r854163032). 5) The wallet `IsLockedCoin` and `IsSpent` methods now accept an `OutPoint` instead of a hash:index. Which let me cleanup a bunch of extra code. 6) Speeded up the wallet 'GetAvailableBalance': filtering `AvailableCoins` by spendable outputs only and using the 'AvailableCoins' retrieved `total_amount` instead of looping over all the retrieved coins once more. ------------------------------------------------------- Side topic, all this process will look even nicer with #25218 ACKs for top commit: achow101: ACK fd5c996d1609e6f88769f6f3ef0c322e3435b3aa brunoerg: crACK fd5c996d1609e6f88769f6f3ef0c322e3435b3aa w0xlt: Code Review ACK https://github.com/bitcoin/bitcoin/pull/25005/commits/fd5c996d1609e6f88769f6f3ef0c322e3435b3aa Tree-SHA512: 376a85476f907f4f7d1fc3de74b3dbe159b8cc24687374d8739711ad202ea07a33e86f4e66dece836da3ae6985147119fe584f6e672f11d0450ba6bd165b3220
2022-06-16walletdb: fix last client version updatefurszy
The value was only being updated launching releases with higher version numbers and not if the user launched a previous release. Co-authored-by: MacroFake <falke.marco@gmail.com>
2022-06-16Merge bitcoin/bitcoin#24649: wallet: do not count wallet utxos as externalAndrew Chow
7832e9438f5c66b88f60676d14e1e11d669eb109 test: fundrawtransaction preset input weight calculation (S3RK) c3981e379fa088aa7aa03b2f505342a5b3bc3436 wallet: do not count wallet utxos as external (S3RK) Pull request description: Correctly differentiating between external vs non-external utxos in coin control produces more accurate weight and fee estimations. Weight for external utxos is estimated based on the maximum signature size, while for the wallet utxos we expect minimal signature due to signature grinding. ACKs for top commit: achow101: re-ACK 7832e9438f5c66b88f60676d14e1e11d669eb109 Xekyo: re-ACK 7832e9438f5c66b88f60676d14e1e11d669eb109 furszy: ACK 7832e943 Tree-SHA512: bb5635b0bd85fa9a76922a53ad3fa062286424c06a695a0e87407c665713e80a33555b644fbb13bcc1ab503dcd7f53aacbdc368d69ac0ecff8005603623ac94f
2022-06-16wallet: don't read db every time that a new WalletBatch is createdfurszy
Better to perform the action only one time (during 'LoadWallet'). Where the value is being used.
2022-06-15Merge bitcoin/bitcoin#25368: doc: Update importaddress mention ↵Andrew Chow
incompatibility with descriptor wallet e3609cdc01cf992800f28b20b0107b7fdc1f880e doc: Update importaddress mention incompatibility with descriptor wallet (BrokenProgrammer) Pull request description: This is related to #25363 and offers a small update to the error messages from `EnsureLegacyScriptPubKeyMan` and `EnsureConstLegacyScriptPubKeyMan` to mention that they only are compatible with legacy wallets. The RPC documentation for `importaddress` is also updated to mention this as well as guide the user to the alternative `importdescriptors` for cases when using descriptor wallets. I'm thinking that we can introduce a "porting guide" document mentioned in #25363 in a separate PR since I would have to make myself more familiar with the subject before being able to tackle that. ACKs for top commit: laanwj: Code review ACK e3609cdc01cf992800f28b20b0107b7fdc1f880e achow101: ACK e3609cdc01cf992800f28b20b0107b7fdc1f880e Tree-SHA512: c7a924a7283fe59dc4e04c8c8fa034c15601f0b25eff09d975e98e2e8db5268ff470336b2d978d6916af9f782f9257b840d64bd15485b1742b4a8b8bfd0bb50f
2022-06-14doc: Update importaddress mention incompatibility with descriptor walletBrokenProgrammer
2022-06-14refactor: getAddress don't access m_address_book, use FindAddressEntry functionfurszy
2022-06-14scripted-diff: Avoid incompatibility with CMake AUTOUIC featureHennadii Stepanov
-BEGIN VERIFY SCRIPT- sed -i "s|node/ui_interface|node/interface_ui|g" $(git grep -l "node/ui_interface" ./src) git mv src/node/ui_interface.cpp src/node/interface_ui.cpp git mv src/node/ui_interface.h src/node/interface_ui.h sed -i "s|BITCOIN_NODE_UI_INTERFACE_H|BITCOIN_NODE_INTERFACE_UI_H|g" src/node/interface_ui.h -END VERIFY SCRIPT-
2022-06-10Merge bitcoin/bitcoin#24931: Strengthen thread safety assertionsMacroFake
ce893c0497fc9b8ab9752153dfcc77c9f427545e doc: Update developer notes (Anthony Towns) d2852917eecad6ab422a7b2c9892d351a7f0cc96 sync.h: Imply negative assertions when calling LOCK (Anthony Towns) bba87c0553780eacf0317fbfec7330ea27aa02f8 scripted-diff: Convert global Mutexes to GlobalMutexes (Anthony Towns) a559509a0b8cade27199740212d7b589f71a0e3b sync.h: Add GlobalMutex type (Anthony Towns) be6aa72f9f8d50b6b5b19b319a74abe7ab4099ff qt/clientmodel: thread safety annotation for m_cached_tip_mutex (Anthony Towns) f24bd45b37e1b2d19e5a053dbfefa30306c1d41a net_processing: thread safety annotation for m_tx_relay_mutex (Anthony Towns) Pull request description: This changes `LOCK(mutex)` for non-global, non-recursive mutexes to be annotated with the negative capability for the mutex it refers to, to prevent . clang applies negative capabilities recursively, so this helps avoid forgetting to annotate functions. This can't reasonably be used for globals, because clang would require every function to be annotated with `EXCLUSIVE_LOCKS_REQUIRED(!g_mutex)` for each global mutex; so this introduces a trivial `GlobalMutex` subclass of `Mutex`, and reduces the annotations for both `GlobalMutex` to `LOCKS_EXCLUDED` which only catches trivial errors (eg (`LOCK(x); LOCK(x);`). ACKs for top commit: MarcoFalke: review ACK ce893c0497fc9b8ab9752153dfcc77c9f427545e 🐦 hebasto: ACK ce893c0497fc9b8ab9752153dfcc77c9f427545e Tree-SHA512: 5c35e8c7677ce3d994a7e3774f4344adad496223a51b3a1d1d3b5f20684b2e1d5cff688eb3fbc8d33e1b9940dfa76e515f9434e21de6f3ce3c935e29a319f529
2022-06-08wallet: GetAvailableBalance, remove double walk-through every available coinfurszy
Filtering `AvailableCoins` by spendable outputs only and using the retrieved total_amount.
2022-06-08wallet: add 'only_spendable' filter to AvailableCoinsfurszy
We are skipping the non-spendable coins that appear in vCoins ('AvailableCoins' result) later, in several parts of the CreateTransaction and GetBalance flows: GetAvailableBalance (1) gets all the available coins calling AvailableCoins and, right away, walk through the entire vector, skipping the non-spendable coins, to calculate the total balance. Inside CreateTransactionInternal —> SelectCoins(vCoins,...), we have several calls to AttemptSelection which, on each of them internally, we call twice to GroupOutputs which internally has two for-loops over the entire vCoins vector that skip the non-spendable coins. So, Purpose is not add the non-spendable coins into the AvailableCoins result (vCoins) in the first place for the processes that aren’t using them at all, so we don’t waste resources skipping them later so many times. Note: this speedup is for all the processes that call to CreateTransaction and GetBalance* internally.
2022-06-08wallet: remove unused IsSpentKey(hash, index) methodfurszy
2022-06-08wallet: avoid extra IsSpentKey -> GetWalletTx lookupsfurszy
2022-06-08wallet: decouple IsSpentKey(scriptPubKey) from IsSpentKey(hash, n)furszy
This will be used in a follow-up commit to prevent extra 'GetWalletTx' lookups if the function caller already have the wtx and can just provide the scriptPubKey directly.
2022-06-08wallet: IsSpent, 'COutPoint' arg instead of (hash, index)furszy
2022-06-08wallet: IsLockedCoin, 'COutPoint' arg instead of (hash, index)furszy
2022-06-08wallet: AvailableCoins, don't call 'wtx.tx->vout[i]' multiple timesfurszy
2022-06-08wallet: return 'CoinsResult' struct in `AvailableCoins`furszy
Instead of accepting a `vCoins` reference that is cleared at the beginning of the method. Note: This new struct, down the commits line, will contain other `AvailableCoins` useful results.
2022-06-07Merge bitcoin/bitcoin#25239: wallet: 'CommitTransaction', remove extra wtx ↵Andrew Chow
lookup and add exception for db write error 57fb37c27599fc865f20b42a27bb9c227f384de3 wallet: CommitTransaction, remove extra wtx lookup and add exception for a possible db write error. (furszy) Pull request description: Two points for `CWallet::CommitTransaction`: 1) The extra wtx lookup: As we are calling to `AddToWallet` first, which returns the recently added/updated wtx pointer, there is no need to look up the wtx again few lines later. We can just use it. 2) The db write error: `AddToWallet` can only return a nullptr if the db write fails, which inside `CommitTransaction` translates to an exception throw cause. We expect everywhere that `CommitTransaction` always succeed. ------------------------------------------------ Extra note: This finding generated another working path for me :) It starts with the following question: why are we returning a nullptr from `AddToWallet` if the db write failed without removing the recently added transaction from the wallet's map?.. Can led to a wallet invalid state where the inputs of this new transaction are not marked dirty, while the transaction that spends them still exist on the in-memory wallet tx map. -- I'm writing it here to gather some feedback first and not forget it, will create a follow-up PR in the coming days 🚜 -- ACKs for top commit: achow101: ACK 57fb37c27599fc865f20b42a27bb9c227f384de3 jonatack: ACK 57fb37c ryanofsky: Code review ACK 57fb37c27599fc865f20b42a27bb9c227f384de3. Seems like a clear improvement. Better to fail earlier with a better error message if the failure is going to happen anyway Tree-SHA512: 80e59c01852cfbbc70a5de1a1c2c59b5e572f9eaa08c2175112cb515256e63fa04c7942f92a513b620d6b06e66392029ebe8902287c456efdbee58a7a5ae42da
2022-06-06Merge bitcoin/bitcoin#25220: rpc: fix incorrect warning for address type ↵laanwj
p2sh-segwit in createmultisig 3a9b9bb38e653c8ff7220b9af6e337a90c2c22dc test: ensure createmultisig and addmultisigaddress are not returning any warning for expected cases (brunoerg) eaf6f630c0190c634b5f1c85f749437f4209cc36 rpc: fix inappropriate warning for address type p2sh-segwit in createmultisig and addmultisigaddress (brunoerg) Pull request description: Fixes #25127 If there are any uncompressed keys when calling `AddAndGetMultisigDestination`, it will just default to a legacy address regardless of the chosen `address_type`. So, #23113 added a warnings field which will warn the user why their address format is different. However, when creating a multisig (p2sh-segwit), it is returning an inappropriate warning, because when getting the output type from destination (`OutputTypeFromDestination`), it returns `ScriptHash` for both legacy and `P2SH_SEGWIT`. So, since `P2SH_SEGWIT` is different from `ScriptHash`, it returns the warning: https://github.com/bitcoin/bitcoin/blob/192d639a6b1bd0feaa52e6ea4e63e33982704c32/src/rpc/output_script.cpp#L166-L169 So, to avoid this mistake I changed `OutputTypeFromDestination` to `descriptor->GetOutputType()` to get the appropriate output type. ACKs for top commit: jonatack: ACK 3a9b9bb38e653c8ff7220b9af6e337a90c2c22dc laanwj: Code review ACK 3a9b9bb38e653c8ff7220b9af6e337a90c2c22dc Tree-SHA512: 49f717479c2b8906277e7591ddd4747f7961c2d5c77494b5124045de9036a4277d46b9ad99279d51f0c4484284c445f1e1d3c55c49bbf0716741bad426a89369
2022-06-06rpc: fix inappropriate warning for address type p2sh-segwit in ↵brunoerg
createmultisig and addmultisigaddress
2022-06-04doc: Fix typo in importdescriptorsKolby Moroz Liebl
2022-05-29wallet: CommitTransaction, remove extra wtx lookup and add exception for a ↵furszy
possible db write error. 1) `Wallet::AddToWallet` is already returning the pointer to the inserted `CWalletTx`, so there is no need to look it up in the map again. 2) `Wallet::AddToWallet` can only return a nullptr if the db `writeTx` call failed. Which should be treated as an error.
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-06 09:13:40 +0000