aboutsummaryrefslogtreecommitdiff
path: root/src/wallet
AgeCommit message (Collapse)Author
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-09GetExternalSigner(): fail if multiple signers are foundamadeuszpawlik
If there are multiple external signers, `GetExternalSigner()` will just pick the first one in the list. If the user has two or more hardware wallets connected at the same time, he might not notice this. This PR adds a check and fails with suitable message.
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.
2022-05-26Merge bitcoin/bitcoin#25003: tracing: fix ↵Andrew Chow
`coin_selection:aps_create_tx_internal` calling logic 6b636730f4befee39d57fcfd51298f3015dbf563 tracing: fix `coin_selection:aps_create_tx_internal` calling logic (Sebastian Falbesoner) Pull request description: According to the documentation, the tracepoint `coin_selection:aps_create_tx_internal` "Is called when the second `CreateTransactionInternal` with Avoid Partial Spends enabled completes." Currently it is only called if the second call to `CreateTransactionInternal` succeeds, i.e. the third parameter is always `true` and we don't get notified in the case that it fails. This PR fixes this by moving the tracepoint call and the `use_aps` boolean variable outside the if body. ACKs for top commit: achow101: ACK 6b636730f4befee39d57fcfd51298f3015dbf563 furszy: re-ACK 6b636730 Tree-SHA512: 453825123aa10748642c7dd94324ced2d07df0f4fac478b0947a34820b515ae300f75721679a90a164f3127029739df55c4de035c4567e663893c3c6dbdef216
2022-05-26Merge bitcoin/bitcoin#25202: log: Use severity-based logging for ↵laanwj
leveldb/libevent messages, reverse LogPrintLevel order c4e77177276ea2b79c4675cb2678ee2cc757b743 refactor: Change LogPrintLevel order to category, severity (laanwj) ce920713bf0810614c2c0c994511b50d4f660bce leveldb: Log messages from leveldb with category and debug level (laanwj) 18ec120bb9e1fc9d27d2419da4c580bd3cde7e86 http: Use severity-based logging for messages from libevent (laanwj) bd971bffb02c7b06aac9a479f7e5ed8f71de2bec logging: Unconditionally log levels >= WARN (laanwj) Pull request description: Log messages from leveldb and libevent libraries in the severity+level based log format introduced in bitcoin/bitcoin#24464. Example of messages before: ``` 2022-05-24T18:11:57Z [libevent] libevent: event_add: event: 0x55da963fcc10 (fd 10), EV_READ call 0x7f1c7a254620 2022-05-24T18:11:57Z [libevent] libevent: Epoll ADD(1) on fd 10 okay. Old events were 0; read change was 1 (add); write change was 0 (none); close change was 0 (none) 2022-05-24T18:12:08Z leveldb: Generated table #609127@1: 6445 keys, 312916 bytes 2022-05-24T18:12:08Z leveldb: Generated table #609128@1: 5607 keys, 268548 bytes 2022-05-24T18:12:08Z leveldb: Generated table #609129@1: 189 keys, 9384 bytes 2022-05-24T18:12:08Z leveldb: Generated table #609130@1: 293 keys, 13818 bytes ``` Example of messages after: ``` 2022-05-24T17:59:52Z [libevent:debug] event_add: event: 0x5652f44dac10 (fd 10), EV_READ call 0x7f210f2e6620 2022-05-24T17:59:52Z [libevent:debug] Epoll ADD(1) on fd 10 okay. Old events were 0; read change was 1 (add); write change was 0 (none); close change was 0 (none) 2022-05-24T17:59:53Z [leveldb:debug] Recovering log #1072 2022-05-24T17:59:53Z [leveldb:debug] Level-0 table #1082: started 2022-05-24T17:59:53Z [leveldb:debug] Level-0 table #1082: 193 bytes OK 2022-05-24T17:59:53Z [leveldb:debug] Delete type=3 #1070 2022-05-24T17:59:53Z [leveldb:debug] Delete type=0 #1072 ``` The first commit changes it so that messages with level Warning and Error are always logged independent of the `-debug` setting. I think this is useful to make sure warnings and errors, which tend to be important, are not lost. In the future this should be made more configurable. Last commit changes LogPrintLevel argument order to category, severity: This is more consistent with the other functions, as well as with the logging output itself. If we want to make this change, we should do it before it's all over the place. ACKs for top commit: jonatack: Tested ACK c4e77177276ea2b79c4675cb2678ee2cc757b743 Tree-SHA512: ea48a1517f8c1b23760e59933bbd64ebf09c32eb947e31b8c2696b4630ac1f4303b126720183e2de052bcede3a17e475bbf3fbb6378a12b0fa8f3582d610930d
2022-05-25fuzz: coinselection, add missing fee rate.furszy
Otherwise, 'GroupOutputs' will crash at group insertion time (output.GetEffectiveValue() asserts that the value exists).
2022-05-25logging: Unconditionally log levels >= WARNlaanwj
Messages with level `WARN` or higher should be logged even when the category is not provided with `-debug=`, to make sure important warnings are not lost.
2022-05-23Merge bitcoin/bitcoin#25083: Set effective_value when initializing a COutputAndrew Chow
6fbb0edac22c63f1b723f731c2601b1d46879a58 Set effective_value when initializing a COutput (ishaanam) Pull request description: Previously in COutput, effective_value was initialized as the absolute value of the txout and the fee as 0. effective_value along with the fee was calculated outside of the COutput constructor and set after the object had been initialized. These changes will allow either the fee or the feerate to be passed in a COutput constructor and the fee and effective_value are calculated and set in the constructor. As a result, AvailableCoins also needs to be passed the feerate when utxos are being spent. When balance is calculated or the coins are being listed and feerate is neither available nor required, AvailableCoinsListUnspent is used instead, which runs AvailableCoins while providing the default value for `feerate`. Unit tests for the calculation of effective value have also been added. ACKs for top commit: achow101: re-ACK 6fbb0edac22c63f1b723f731c2601b1d46879a58 Xekyo: re-ACK 6fbb0edac22c63f1b723f731c2601b1d46879a58 w0xlt: Code Review ACK https://github.com/bitcoin/bitcoin/pull/25083/commits/6fbb0edac22c63f1b723f731c2601b1d46879a58 furszy: Looks good, have been touching this area lately, code review ACK 6fbb0eda. Tree-SHA512: 5943ee4f4b0c1dcfe146f2fc22853e607259d6d53156b80a8a8f4baa70760a8b25ab822777b7f5d21ecb02dac08bdee704a9a918d5660276d6994b19a284b256
2022-05-23Merge bitcoin/bitcoin#25122: rpc: getreceivedbylabel, return early if no ↵Andrew Chow
addresses were found in the address book baa3ddc49c46d00e3e0de06e494656f0f00b0ee8 doc: add release notes about `getreceivedbylabel` returning an error if the label is not in the address book. (furszy) 8897a21658ad93f7b628eb2a3411fec2265d73fb rpc: getreceivedbylabel, don't loop over the entire wallet txs map if no destinations were found for the input label. (furszy) Pull request description: Built on top of #23662, coming from comment https://github.com/bitcoin/bitcoin/pull/23662#pullrequestreview-971407999. If `wallet.GetLabelAddresses()` returns an empty vector (the wallet does not have stored destinations with that label in the addressbook) or if none of the returned destinations are from the wallet, we can return the function right away. Otherwise, we are walking through all the wallet txs + outputs for no reason (`output_scripts` is empty). ACKs for top commit: achow101: ACK baa3ddc49c46d00e3e0de06e494656f0f00b0ee8 theStack: re-ACK baa3ddc49c46d00e3e0de06e494656f0f00b0ee8 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25122/commits/baa3ddc49c46d00e3e0de06e494656f0f00b0ee8 Tree-SHA512: 00e10365b179bf008da2f3ef8fbb3ee04a330426374020e3f2d0151b16991baba4ef2b944e4659452f3e4d6cb20f128d0918ddf0453933a25a4d9fd8414a1911
2022-05-21Set effective_value when initializing a COutputishaanam
Previously in COutput, effective_value was initialized as the absolute value of the txout, and fee as 0. effective_value along with fee were calculated outside of the COutput constructor and set after the object had been initialized. These changes will allow either the fee or the feerate to be passed in a COutput constructor. If either are provided, fee and effective_value are calculated and set in the constructor. As a result, AvailableCoins also needs to be passed the feerate when utxos are being spent. When balance is calculated or the coins are being listed and feerate is neither available nor required, AvailableCoinsListUnspent is used instead, which runs AvailableCoins while providing the default value for feerate. Unit tests for the calculation of effective value have also been added.
2022-05-20rpc: getreceivedbylabel, don't loop over the entire wallet txs map if no ↵furszy
destinations were found for the input label. If wallet.GetLabelAddresses() returns an empty vector (the wallet does not have addresses with that label in the addressbook) or if none of the returned destinations are from the wallet, we can return the function right away.
2022-05-20Merge bitcoin/bitcoin#24820: test: 3 new tests for SelectCoins functionAndrew Chow
3f8def51d53a078a5ee71ec675b5e06b784147de add 3 new test cases for SelectCoins() (akankshakashyap) Pull request description: Three new tests have been added. 1. More coins should be selected when effective fee < long term fee. 2. Less coin should be selected when effective fee > long term fee. 3. If a coin is preselected, it should be selected even if disadvantageous. ACKs for top commit: achow101: ACK 3f8def51d53a078a5ee71ec675b5e06b784147de brunoerg: ACK 3f8def51d53a078a5ee71ec675b5e06b784147de Tree-SHA512: 8db6dd942b02a38c99953b801605f98c4c17729768fdfcf7605c5bbdb17509500a39d0a78a4b19aab37812d2994ec7630d2b4e78d1d348f1c27b67588d74e155
2022-05-21scripted-diff: Convert global Mutexes to GlobalMutexesAnthony Towns
-BEGIN VERIFY SCRIPT- sed -i -E -e '/^([a-z]+ )?Mutex [a-z]/ s/Mutex/GlobalMutex/' $(git grep -lE '^([a-z]+ )?Mutex [a-z]') -END VERIFY SCRIPT-
2022-05-20Merge bitcoin/bitcoin#25171: rpc: wallet: remove ↵MacroFake
`-deprecatedrpc=exclude_coinbase` logic a4703ce9d79855ac0bd7dc07b71a51245f9aa5f8 doc: add release notes about removal of the `deprecatedrpc=exclude_coinbase` (Sebastian Falbesoner) ef0aa74836c4339aa7f14fc1c9583d86dd5c388a rpc: wallet: remove `-deprecatedrpc=exclude_coinbase` logic (Sebastian Falbesoner) Pull request description: Including coinbase transactions in `receivedby` RPCs and adding the `-deprecatedrpc=exclude_coinbase` was done in PR #14707 (released in v23.0). For the next release v24.0, this configuration option can be removed. ACKs for top commit: fanquake: ACK a4703ce9d79855ac0bd7dc07b71a51245f9aa5f8 Tree-SHA512: 97cd4e78501e64f678c78d2ebb5be5376688c023e34fced71dd24e432d27aa31a74b5483545f49ba0bdf48656d8b8b7bee74e3db26cf6daf112613f1caa4dfa4
2022-05-19Merge bitcoin/bitcoin#25153: scripted-diff: Use getInt<T> over get_int/get_int64fanquake
fa9af218780b7960d756db80c57222e5bf2137b1 scripted-diff: Use getInt<T> over get_int/get_int64 (MacroFake) Pull request description: Seems better to see the return type directly and be able to modify it easier, as the return type is used for exceptions (in-range checking and parsing feedback). ACKs for top commit: fanquake: ACK fa9af218780b7960d756db80c57222e5bf2137b1 Tree-SHA512: 284aa2527d0f663ca01550115025c9c64c787531d595f866c718f6ad09b9b0cac1e683a7d77f8009b75de990fd37166b44063ffa83fba8a04e9a31600b4c2725
2022-05-19rpc: wallet: remove `-deprecatedrpc=exclude_coinbase` logicSebastian Falbesoner
2022-05-18Merge bitcoin/bitcoin#25108: tidy: add modernize-use-default-member-initMacroFake
ac6fbf2c83578129a0397d0d0dc9b1c6bdb30701 tidy: use modernize-use-default-member-init (fanquake) 7aa40f55636be565441a9e0af8de0a346bfa4da2 refactor: use C++11 default initializers (fanquake) Pull request description: Refactor and then enable [`modernize-use-default-member-init`](https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-default-member-init.html) in our `clang-tidy` job. Top commit has no ACKs. Tree-SHA512: 536b406f20639f8c588fe9e96175ec60c7bb825506b2670b562370b2f572801c24203c483443be3c199e1b958c0765d4532e57c57a4e78689162a1dd422d844f
2022-05-18scripted-diff: Use getInt<T> over get_int/get_int64MacroFake
-BEGIN VERIFY SCRIPT- sed -i 's|\<get_int64\>|getInt<int64_t>|g' $(git grep -l get_int ':(exclude)src/univalue') sed -i 's|\<get_int\>|getInt<int>|g' $(git grep -l get_int ':(exclude)src/univalue') -END VERIFY SCRIPT-
2022-05-18Merge bitcoin/bitcoin#25148: refactor: Remove `NO_THREAD_SAFETY_ANALYSIS` ↵MacroFake
from non-test/benchmarking code a55db4ea1cf10e0ab4a6eb5cd1dd3bd95626fba0 Add more proper thread safety annotations (Hennadii Stepanov) 8cfe93e3fcf263bf059f738d5e7d9c94901a7c5a Add proper thread safety annotation to `CWallet::GetTxConflicts()` (Hennadii Stepanov) ca446f2c59720c1575aeeab9c9d636d98ce8528c Add proper thread safety annotation to `CachedTxGetAvailableCredit()` (Hennadii Stepanov) Pull request description: In non-test/benchmarking code, there are three cases of the `NO_THREAD_SAFETY_ANALYSIS` annotation which are accompanied with `TODO` comments. This PR adds proper thread safety annotations instead of `NO_THREAD_SAFETY_ANALYSIS`. ACKs for top commit: laanwj: Code review ACK a55db4ea1cf10e0ab4a6eb5cd1dd3bd95626fba0 Tree-SHA512: 806d72eebc1edf088bfa435c8cd11465be0de6789798dd92abd008425516768acb864a73d834a49d412bb10f7fccfb47473f998cb72739dab6caeef6bcfaf191
2022-05-18wallet: do not count wallet utxos as externalS3RK
2022-05-17refactor: use C++11 default initializersfanquake
2022-05-17tracing: fix `coin_selection:aps_create_tx_internal` calling logicSebastian Falbesoner
According to the documentation, the tracepoint `coin_selection:aps_create_tx_internal` "Is called when the second `CreateTransactionInternal` with Avoid Partial Spends enabled completes." Currently it is only called if the second call to `CreateTransactionInternal` succeeds, i.e. the third parameter is always `true` and we don't get notified in the case that it fails. Fix this by introducing a boolean variable for the result of the call and moving the tracepoint call outside the if body.
2022-05-17Merge bitcoin/bitcoin#20640: wallet, refactor: return out-params of ↵fanquake
CreateTransaction() as optional struct 4c5ceb040cf50d24201903a9200fb23be88d96fb wallet: CreateTransaction(): return out-params as (optional) struct (Sebastian Falbesoner) c9fdaa5e3ae09b45be6a5c2d4ee6b1e8cef9d8a8 wallet: CreateTransactionInternal(): return out-params as (optional) struct (Sebastian Falbesoner) Pull request description: The method `CWallet::CreateTransaction` currently returns several values in the form of out-parameters: * the actual newly created transaction (`CTransactionRef& tx`) * its required fee (`CAmount& nFeeRate`) * the position of the change output (`int& nChangePosInOut`) -- as the name suggests, this is both an in- and out-param By returning these values in an optional structure (which returns no value a.k.a. `std::nullopt` if an error occured), the interfaces is shorter, cleaner (requested change position is now in-param and can be passed by value) and callers don't have to create dummy variables for results that they are not interested in. Note that the names of the replaced out-variables were kept in `CreateTransactionInternal` to keep the diff minimal. Also, the fee calculation data (`FeeCalculation& fee_calc_out`) would be another candidate to put into the structure, but `FeeCalculation` is currently an opaque data type in the wallet interface and I think it should stay that way. As a potential follow-up, I think it would make sense to also do the same refactoring for `CWallet::FundTransaction`, which has a very similar parameter structure. Suggested by laanwj in https://github.com/bitcoin/bitcoin/pull/20588#issuecomment-739838428. ACKs for top commit: achow101: re-ACK 4c5ceb040cf50d24201903a9200fb23be88d96fb Xekyo: ACK 4c5ceb040cf50d24201903a9200fb23be88d96fb w0xlt: crACK https://github.com/bitcoin/bitcoin/pull/20640/commits/4c5ceb040cf50d24201903a9200fb23be88d96fb Tree-SHA512: 27e5348bbf4f698713002d40c834dcda59c711c93207113e14522fc6d9ae7f4d8edf1ef6d214c5dd62bb52943d342878960ca333728828bf39b645a27d55d524
2022-05-16Merge bitcoin/bitcoin#25088: Wallet: Ensure m_attaching_chain is set before ↵Andrew Chow
registering for signals ba10b90915dae6a802ecb0f80f72a1a9ea5a4c67 Wallet: Ensure m_attaching_chain is set before registering for signals (Luke Dashjr) Pull request description: Avoids a race where chainStateFlushed could be called before rescanning began, yet rescan gets interrupted or fails Followup for #24984 avoiding a race between registering and setting the flag. ACKs for top commit: mzumsande: Code Review ACK ba10b90915dae6a802ecb0f80f72a1a9ea5a4c67 achow101: ACK ba10b90915dae6a802ecb0f80f72a1a9ea5a4c67 Tree-SHA512: 1d2fa2db189d3e87f2d0863cf2ab62166094436483f0da16760b1083a4743bf08e476a3277e0d36564213d65dd6f0a1fc16a4bf68d3338c991a14d1de9fc0fee
2022-05-16Add more proper thread safety annotationsHennadii Stepanov
2022-05-16Add proper thread safety annotation to `CWallet::GetTxConflicts()`Hennadii Stepanov
2022-05-16Add proper thread safety annotation to `CachedTxGetAvailableCredit()`Hennadii Stepanov
2022-05-16Merge bitcoin/bitcoin#23662: rpc: improve `getreceivedby{address,label}` ↵Andrew Chow
performance f336ff7f213564909cf5f9742618cc6ec87600fd rpc: avoid expensive `IsMine` calls in `GetReceived` tally (Sebastian Falbesoner) a7b65af2a4450663d4a20a617c59dac87b34fb36 rpc: avoid scriptPubKey<->CTxDestination conversions in `GetReceived` tally (Sebastian Falbesoner) Pull request description: The RPC calls `getreceivedbyaddress`/`getreceivedbylabel` both use the internal helper function `GetReceived` which was introduced in PR #17579 to deduplicate tallying code. For every wallet-related transaction output, the following unnecessary operations are currently performed in the tally loop, leading to a quite bad performance (as reported in #23645): - converting from CScript -> TxDestination (`ExtractDestination(...)`), converting from TxDestination -> CScript (`CWallet::IsMine(const CTxDestination& dest)`); this can be avoided by directly using output scripts in the search set instead of addresses (first commit) - checking if the iterated output script belongs to the wallet by calling `IsMine`; this can be avoided by only adding addresses to the search set which fulfil `IsMine` in the first place (second commit) ### Benchmark results The functional test [wallet_pr23662.py](https://github.com/theStack/bitcoin/blob/pr23662_benchmarks/test/functional/wallet_pr23662.py) (not part of this PR) creates transactions with 15000 different addresses: - 5000 outputs (500 txs with 10 outputs each) with label set, IsMine set (received) - 5000 outputs (500 txs with 10 outputs each) with label set, IsMine not set (sent) - 5000 outputs (500 txs with 10 outputs each) without label set, IsMine not set (sent) Then, the time is measured for calling `getreceivedbyaddress` and `getreceivedbylabel`, the latter with two variants. Results on my machine: | branch | `getreceivedbyaddress` (single) | `getreceivedbylabel` (single) | `getreceivedbylabel` (10000) | |--------------------|---------------------------------|-------------------------------|------------------------------| | master | 406.13ms | 425.33ms | 446.58ms | | PR (first commit) | 367.18ms | 365.81ms | 426.33ms | | PR (second commit) | 3.96ms | 4.83ms | 339.69ms | Fixes #23645. ACKs for top commit: achow101: ACK f336ff7f213564909cf5f9742618cc6ec87600fd w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/23662/commits/f336ff7f213564909cf5f9742618cc6ec87600fd furszy: Code ACK f336ff7f Tree-SHA512: 9cbf402b9e269713bd3feda9e31719d9dca8a0dfd526de12fd3d561711589195d0c50143432c65dae279c4eab90a4fc3f99e29fbc0452fefe05113e92d129b8f
2022-05-16wallet: CreateTransaction(): return out-params as (optional) structSebastian Falbesoner
2022-05-16wallet: CreateTransactionInternal(): return out-params as (optional) structSebastian Falbesoner
2022-05-12scripted-diff: replace non-standard fixed width integer types (`u_int`...` ↵Sebastian Falbesoner
-> `uint`...) -BEGIN VERIFY SCRIPT- sed -i 's/u_int/uint/g' $(git grep -l u_int) -END VERIFY SCRIPT-
2022-05-10wallet: Change log interval to use `steady_clock`w0xlt
This refactors the log interval variables to use `steady_clock` as it is best suitable for measuring intervals.
2022-05-10sqlite: Use in-memory db instead of temp for mockdbAndrew Chow
The mock db can be in-memory rather than just at temp file.
2022-05-10walletdb: Create a mock database of specific typeAndrew Chow
We may want to make a mock database of either SQLite or BDB, not just whatever the compiled default is.
2022-05-09Wallet: Ensure m_attaching_chain is set before registering for signalsLuke Dashjr
Avoids a race where chainStateFlushed could be called before rescanning began, yet rescan gets interrupted or fails