aboutsummaryrefslogtreecommitdiff
path: root/src/wallet
AgeCommit message (Collapse)Author
2021-09-30Merge bitcoin/bitcoin#23104: log: Avoid breaking single log lines over ↵W. J. van der Laan
multiple lines in the log file 2222c04e1b9960030cb590c789a0d2375add4544 log: Adjust coin selection log string (MarcoFalke) fa6c1e850f3a96f884ba8a635b72d3abea1f4e56 test: Fix typos in tests (MarcoFalke) faeae2980fa2493391cdced20950a991e28cf47d log: Avoid broken DEBUG_LOCKORDER log (MarcoFalke) faffaa85cde32b621f598a8ea8dceae34f33f021 log: Avoid broken SELECTCOINS log (MarcoFalke) Pull request description: Follow up to commit d8b4b3077fd20c90b635eff1dd240bdad9725027 ACKs for top commit: laanwj: re-ACK 2222c04e1b9960030cb590c789a0d2375add4544 practicalswift: cr ACK 2222c04e1b9960030cb590c789a0d2375add4544 Tree-SHA512: e0daf76815a1b7c4898ceffedeaf7ede093223abf709874f9a0d78c8e41551c14e8b56d055c8fdf06ec698df64e67dfc168bbd8716131b23648d1d1294fa6636
2021-09-30Merge bitcoin/bitcoin#23112: wallet: enable SQLite extended result codesW. J. van der Laan
90be29c5b52e68b5de8a3282cd83172fbf9acf1b wallet: enable SQLite extended result codes (Sebastian Falbesoner) Pull request description: With this change, we get more fine-grained error messages if something goes wrong in the course of communicating with the SQLite database. To pick some random examples, the error codes SQLITE_IOERR_NOMEM, SQLITE_IOERR_CORRUPTFS or SQLITE_IOERR_FSYNC are way more specific than just a plain SQLITE_IOERR, and the corresponding error messages generated by sqlite3_errstr() will hence give a better hint to the user (or also to the developers, if an error report is sent) what the cause for a failure is. See the SQLite documentation https://www.sqlite.org/c3ref/extended_result_codes.html https://www.sqlite.org/c3ref/c_abort_rollback.html > In its default configuration, SQLite API routines return one of 30 integer result codes. However, experience has shown that many of these result codes are too coarse-grained. They do not provide as much information about problems as programmers might like. In an effort to address this, newer versions of SQLite (version 3.3.8 2006-10-09 and later) include support for additional result codes that provide more detailed information about errors. ACKs for top commit: Sjors: utACK 90be29c achow101: ACK 90be29c5b52e68b5de8a3282cd83172fbf9acf1b laanwj: Code review ACK 90be29c5b52e68b5de8a3282cd83172fbf9acf1b Tree-SHA512: 2b7a60860c206f2b5f8ff9d4a7698efdee897c9ad024621b8fd165b841c20746d9780da3cf46aaf448a777e229a5b3cdf3a4792e8ef82cda9c5d46e354a9a598
2021-09-29log: Adjust coin selection log stringMarcoFalke
Replace the outdated function name with words from the English language. Logging the function name can be toggled with -logsourcelocations.
2021-09-29Merge bitcoin/bitcoin#20591: wallet, bugfix: fix ComputeTimeSmart function ↵Samuel Dobson
during rescanning process. 240ea294d5e899a906f213f039b21e94c24d6018 doc: update doxygen documention of ComputeTimeSmart() and AddToWalletIfInvolvingMe() regarding rescanning_old_block parameter (BitcoinTsunami) d6eb39af21810bf1c3bdce0ef2212c1ad6597bcd test: add functional test to check transaction time determination during block rescanning (BitcoinTsunami) 07b44f16e71b9df10dfac7f32f92997938f7e7aa wallet: fix ComputeTimeSmart algorithm to use blocktime during old block rescanning (BitcoinTsunami) Pull request description: The function ComputeTimeSmart in wallet.cpp assume that transaction are discovered in the right order. Moreover the 'smarttime' determination algorithm is coded with realtime scenario in mind and not rescanning of old block. The functional test demonstrate that if the user import a wallet, then rescan only recent history, and then rescan the entire history, the older transaction discovered would have an incorrect time determination. In the context of rescanning old block, the only time value that as a meaning is the blocktime. That's why I've fixed the problem with a simple separation between rescanning of old block and realtime time determination. The fix is written to have no impact on every realtime scenario and only impact the behaviour during a rescanning process. This PR Fixes #20181. To be fair, I don't think that this bug could be triggered with the wallet GUI, because it always proceed with a proper rescan. But RPC API provide the possibility to trigger it. I've discovered it, because Specter desktop v0.10.0 was impacted. (https://github.com/cryptoadvance/specter-desktop/issues/680). ACKs for top commit: jonatack: ACK 240ea294d5e899a906f213f039b21e94c24d6018 per `git diff b92d552 240ea29`, re-verified rebase to latest master + debug build clean + new test passes on the branch and fails on master, only change since my review a few hours ago is incorporation of latest review suggestions meshcollider: re-utACK 240ea294d5e899a906f213f039b21e94c24d6018 Tree-SHA512: 514b02e41d011ddfa325f5e8080b93800e1ea4ed5853fa420670a6ac700e8b463000dbea65f8ced8565cfb950c7f51b69154034dcb111e67aca3b964a0061494
2021-09-29Merge bitcoin/bitcoin#22650: Remove -deprecatedrpc=addresses flag and ↵Samuel Dobson
corresponding code/logic 43cd6b8af9d613ca033800c5cd8524c3f77e13ec doc: add release notes for removal of the -deprecatedrpc=addresses flag (Michael Dietz) 2b1fdc2c6ce1d0b0e51a3f107e23443c142d57af refactor: minor styling, prefer snake case and same line if (Michael Dietz) d64deac7b823a0eba97ab3a3686054eefe330d3c refactor: share logic between ScriptPubKeyToUniv and ScriptToUniv (Michael Dietz) 8721638daa8502c7f8de5ae24a9393d7290a2ce5 rpc: remove deprecated addresses and reqSigs from rpc outputs (Michael Dietz) Pull request description: Resolves #21797 now that we've branched-off to v23 ("addresses" and "reqSigs" deprecated) "ExtractDestinations" should be removed. `-deprecatedrpc=addresses` was initially added in this PR #20286 (which resolved the original issue #20102). Some chunks of code and logic are no longer used/necessary with the removal of this, and therefore some minor refactoring is done in this PR as well (separated commits) ACKs for top commit: MarcoFalke: re-ACK 43cd6b8af9d613ca033800c5cd8524c3f77e13ec 🐉 meshcollider: Code review ACK 43cd6b8af9d613ca033800c5cd8524c3f77e13ec jonatack: ACK 43cd6b8af9d613ca033800c5cd8524c3f77e13ec per `git range-diff a9d0cec 92dc5e9 43cd6b8`, also rebased to latest master, debug built + quick re-review of each commit to bring back context, and ran tests locally at the final commit Tree-SHA512: fba83495e396d3c06f0dcf49292f14f4aa6b68fa758f0503941fade1a6e7271cda8378e2734af1faea550d1b43c85a36c52ebcc9dec0732936f9233b4b97901c
2021-09-28doc: update doxygen documention of ComputeTimeSmart() and ↵BitcoinTsunami
AddToWalletIfInvolvingMe() regarding rescanning_old_block parameter
2021-09-28wallet: fix ComputeTimeSmart algorithm to use blocktime during old block ↵BitcoinTsunami
rescanning
2021-09-28Merge bitcoin/bitcoin#23106: Ensure wallet is unlocked before signing PSBT ↵MarcoFalke
with walletprocesspsbt and GUI 7e3ee4cdd0f60a2f549ba030fe96b90d61c036c5 GUI: Ask user to unlock wallet before signing psbt (Samuel Dobson) 0f3acecf3372f9da143590bb17d8444564e083f4 Add test that walletprocesspsbt requires unlocked wallet when signing (Samuel Dobson) 0e895212bb571ae0de5580adfd8ee9b3c2137e24 Ensure wallet is unlocked before signing in walletprocesspsbt (Samuel Dobson) Pull request description: If signing a PSBT, we need to ensure the wallet is unlocked. Fixes #22874, fixes bitcoin-core/gui#312 ACKs for top commit: achow101: ACK 7e3ee4cdd0f60a2f549ba030fe96b90d61c036c5 lsilva01: Code Review ACK https://github.com/bitcoin/bitcoin/pull/23106/commits/7e3ee4cdd0f60a2f549ba030fe96b90d61c036c5 benthecarman: ACK 7e3ee4cdd0f60a2f549ba030fe96b90d61c036c5 Tree-SHA512: 6726a873582747900ab454ea21153a92be86808a4c1517dc2856b389876a2da9e8df1ffa9b567b6bd017038342c3544ecf5ca3c97744e7debe0a5ee65563687d
2021-09-28Merge bitcoin/bitcoin#22938: test: Add remaining scenarios of 0 waste, in ↵Samuel Dobson
wallet waste_test efcaefc7b5ffe0495e7c809032342ee5ca4841be test: Add remaining scenarios of 0 waste (rajarshimaitra) Pull request description: As per the [review club](https://bitcoincore.reviews/22009) discussion on #22009 , it was observed that there were other two fee scenarios in which selection waste could be zero. These are: - (LTF - Fee) == Change Cost - (LTF - Fee) == Excess Even though these are obvious by the definition of waste metric, adding tests for them can be helpful in explaining its behavior to new readers of the code base, along with pinning the behavior for future. This PR adds those two cases to waste calculation unit test. Also let me know if I am missing more scenarios. ACKs for top commit: jonatack: Tested re-ACK efcaefc7b5ffe0495e7c809032342ee5ca4841be achow101: ACK efcaefc7b5ffe0495e7c809032342ee5ca4841be meshcollider: ACK efcaefc7b5ffe0495e7c809032342ee5ca4841be Tree-SHA512: 13fe3e2c0ea7bb58d34e16c32908b84705130dec16382ff941e5e60ca5b379f9c5811b33f36c4c72d7a98cfbb6af2f196d0a69e96989afa4b9e49893eaadd7cb
2021-09-28wallet: enable SQLite extended result codesSebastian Falbesoner
With this change, we get more fine-grained error messages if something goes wrong in the course of communicating with the SQLite database. To pick some random examples, the error codes SQLITE_IOERR_NOMEM, SQLITE_IOERR_CORRUPTFS or SQLITE_IOERR_FSYNC are way more specific than just a plain SQLITE_IOERR, and the corresponding error messages generated by sqlite3_errstr() will hence give a better hint to the user (or also to the developers, if an error report is sent) what the cause for a failure is.
2021-09-27Merge bitcoin/bitcoin#22976: scripted-diff: Rename overloaded int GetArg to ↵merge-script
GetIntArg 93b9800fec588fc0c258e754405db1e9f89cfb81 scripted-diff: Rename overloaded int GetArg to GetIntArg (Russell Yanofsky) Pull request description: This is meant to improve readability of code and remove guesswork needed to determine argument types and migrate to [typed arguments (#22978)](https://github.com/bitcoin/bitcoin/issues/22978) by having distinctly named `GetArg` `GetArgs` `GetBoolArg` and `GetIntArg` methods. --- This commit was originally part of #22766 and had some review discussion there. But it was [wisely suggested](https://github.com/bitcoin/bitcoin/pull/22766#issuecomment-910001542) to be split off to make that PR smaller. ACKs for top commit: hebasto: ACK 93b9800fec588fc0c258e754405db1e9f89cfb81. MarcoFalke: re-ACK 93b9800fec588fc0c258e754405db1e9f89cfb81 📨 Tree-SHA512: e034bd938b2c8fbadd90bcd52213a61161965dfddf18c2cb0d68816ecf2438cde8afee6fb7e3418f5c6b35c208338da9deb99e4e418dbf68ca34552e0916a625
2021-09-28Ensure wallet is unlocked before signing in walletprocesspsbtSamuel Dobson
2021-09-27Merge bitcoin/bitcoin#22892: doc: Clarify that change_cost cannot be ↵W. J. van der Laan
negative in GetSelectionWaste d2eccacd188959c5aed661d0d5b39c119e7c7a32 doc: Clarify that change_cost cannot be negative in GetSelectionWaste (benthecarman) Pull request description: We assert that the `change_cost` must be positive so we should document it in the function's docs https://github.com/bitcoin/bitcoin/blob/4f5ad43b1e05cd7b403f87aae4c4d42e5aea810b/src/wallet/coinselection.cpp#L361 ACKs for top commit: jonatack: ACK d2eccacd188959c5aed661d0d5b39c119e7c7a32 jarolrod: ACK d2eccacd188959c5aed661d0d5b39c119e7c7a32 shaavan: ACK d2eccacd188959c5aed661d0d5b39c119e7c7a32 Tree-SHA512: 089bc41d7e212b811455527ce2ac83301eae0edc03933dcbd5229b22078827395c8e3bcfda0ba029b7c60705db3119b4f80e372842a6bd8dae195dbaa0b52833
2021-09-27log: Avoid broken SELECTCOINS logMarcoFalke
Before this patch, the log might be corrupted by other threads logging at the same time. For example, another RPC thread: [httpworker.1] [default wallet] keypool reserve 1296 [httpworker.1] SelectCoins() best subset: Received a POST request for / from 127.0.0.1:53732 [httpworker.3] ThreadRPCServer method=getnetworkinfo user=__cookie__ [httpworker.1] 0.78125 0.1953125 0.02441406 0.00610351 0.00305175 0.00152587 total 1.01025417
2021-09-27scripted-diff: Rename overloaded int GetArg to GetIntArgRussell Yanofsky
Improve readability of code, simplify future scripted diff cleanup PRs, and be more consistent with naming for GetBoolArg. This will also be useful for replacing runtime settings type checking with compile time checking. -BEGIN VERIFY SCRIPT- git grep -l GetArg | xargs sed -i 's/GetArg(\([^)]*\( [0-9]\+\|-1\|port\|BaseParams().RPCPort()\|Params().GetDefaultPort()\|_TIMEOUT\|Height\|_WORKQUEUE\|_THREADS\|_CONNECTIONS\|LIMIT\|SigOp\|Bytes\|_VERSION\|_AGE\|_CHECKS\|Checks() ? 1 : 0\|_BANTIME\|Cache\|BLOCKS\|LEVEL\|Weight\|Version\|BUFFER\|TARGET\|WEIGHT\|TXN\|TRANSACTIONS\|ADJUSTMENT\|i64\|Size\|nDefault\|_EXPIRY\|HEIGHT\|SIZE\|SNDHWM\|_TIME_MS\)\))/GetIntArg(\1)/g' -END VERIFY SCRIPT- Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
2021-09-26Merge bitcoin/bitcoin#23065: Allow UTXO locks to be written to wallet DBW. J. van der Laan
d96b000e94d72d041689c5c47e374df2ebc0e011 Make GUI UTXO lock/unlock persistent (Samuel Dobson) 077154fe698f5556ad6e26ef49c9024c2f07ff68 Add release note for lockunspent change (Samuel Dobson) 719ae927dcdb60c0f9902fa79796256035228c4e Update lockunspent tests for lock persistence (Samuel Dobson) f13fc16295c19a156f2974d2d73fba56d52fc161 Allow lockunspent to store the lock in the wallet DB (Samuel Dobson) c52789365e5dbcb25aa5f1775de4d318da79e5a7 Allow locked UTXOs to be store in the wallet database (Samuel Dobson) Pull request description: Addresses and closes #22368 As per that issue (and its predecessor #14907), there seems to be some interest in allowing unspent outputs to be locked persistently. This PR does so by adding a flag to lockunspent to store the change in the wallet database. Defaults to false, so there is no change in default behaviour. Edit: GUI commit changes default behaviour. UTXOs locked/unlocked via the GUI are now persistent. ACKs for top commit: achow101: ACK d96b000e94d72d041689c5c47e374df2ebc0e011 kristapsk: ACK d96b000e94d72d041689c5c47e374df2ebc0e011 lsilva01: Tested ACK https://github.com/bitcoin/bitcoin/pull/23065/commits/d96b000e94d72d041689c5c47e374df2ebc0e011 on Ubuntu 20.04 prayank23: ACK https://github.com/bitcoin/bitcoin/pull/23065/commits/d96b000e94d72d041689c5c47e374df2ebc0e011 Tree-SHA512: 957a5bbfe7f763036796906ccb1598feb6c14c5975838be1ba24a198840bf59e83233165cb112cebae909b6b25bf27275a4d7fa425923ef6c788ff671d7a89a8
2021-09-25Make GUI UTXO lock/unlock persistentSamuel Dobson
2021-09-25Allow lockunspent to store the lock in the wallet DBSamuel Dobson
2021-09-25Allow locked UTXOs to be store in the wallet databaseSamuel Dobson
2021-09-24rpc: remove deprecated addresses and reqSigs from rpc outputsMichael Dietz
2021-09-24test: Add remaining scenarios of 0 wasterajarshimaitra
There are two more cases where waste can be 0, when: - (Fee - LTF) == -Change Cost - (Fee - LTF) == -Excess Adding these two conditions explicitly in the unit test will help pin the behavior, also demonstrate waste calculation scenarios to new readers.
2021-09-21doc: Fix RPC result documentationMarcoFalke
2021-09-20Merge bitcoin/bitcoin#12677: RPC: Add ancestor{count,size,fees} to ↵W. J. van der Laan
listunspent output 6cb60f3e6d652ffa4cf570426a7cf1f690d15c45 doc/release-notes: Add new listunspent fields (Luke Dashjr) 0be2f17ef5649c2d77efbbbdd9222332b2ebf0d2 QA: Add tests for listunspent ancestor{count,size,fees} to mempool_packages (Luke Dashjr) 6966e80f453c46d5d0a923118205f19ac2f4e336 RPC: Add ancestor{count,size,fees} to listunspent output (Luke Dashjr) 3f77dfdaf0f0bfe0c4662a616d6943f31bdd5bf4 Expose ancestorsize and ancestorfees via getTransactionAncestry (Luke Dashjr) Pull request description: Requested by a user ACKs for top commit: prayank23: reACK https://github.com/bitcoin/bitcoin/commit/6cb60f3e6d652ffa4cf570426a7cf1f690d15c45 fjahr: Code review re-ACK 6cb60f3e6d652ffa4cf570426a7cf1f690d15c45 kiminuo: ACK [6cb60f3](https://github.com/bitcoin/bitcoin/commit/6cb60f3e6d652ffa4cf570426a7cf1f690d15c45) achow101: Code Review ACK 6cb60f3e6d652ffa4cf570426a7cf1f690d15c45 naumenkogs: ACK 6cb60f3e6d652ffa4cf570426a7cf1f690d15c45 darosior: utACK 6cb60f3e6d652ffa4cf570426a7cf1f690d15c45 Tree-SHA512: 5d16e5799558691e5853ab7ea2cc85514cb45da3ce69134d855c71845beef32ec6af5ab28d4462683e9800c8ea126f162773a9d3d5660edac08fd8edbfeda173
2021-09-17Merge bitcoin/bitcoin#22941: wallet: refactor: inline functions ↵fanquake
`{Read,Write}OrderPos` 98cf19ca32785c991628324c313e01349c2986af wallet: refactor: avoid duplicate lookup on `mapValue["timesmart"]` (Sebastian Falbesoner) 973d8ba93d0fa00bed4569287e32696300036ab8 wallet: refactor: inline function WriteOrderPos() (Sebastian Falbesoner) 65ed198295e58cf1bc339aa17349b83490872f70 wallet: refactor: inline function ReadOrderPos() (Sebastian Falbesoner) Pull request description: The functions `ReadOrderPos` and `WriteOrderPos` have been introduced in commit 9c7722b7c5ce49130bd978b932f73b629ce5cebe in 2012. Since accounts have been removed in #13825 (commit c9c32e6b844fc79467b7e24c6c916142a0d08484), they are only called at one place in `CWalletTx::{Serialize,Unserialize}` and thus can be directly inlined instead. Additionally, this PR aims to avoids duplicate lookups on the map `mapValue` (affects keys "n" and "timesmart"). ACKs for top commit: laanwj: Code review ACK 98cf19ca32785c991628324c313e01349c2986af achow101: Code Review ACK 98cf19ca32785c991628324c313e01349c2986af Tree-SHA512: 8af63c174c79e589bd713f04e8e40caba9f93ec2978c805427cac50d48049808a8c23ff5eea9ef589c9bd79fc66087f43ff5ab28e3cda51dd03f37c0164e2e4c
2021-09-16RPC: Add ancestor{count,size,fees} to listunspent outputLuke Dashjr
2021-09-16Merge bitcoin/bitcoin#22979: doc: Fix walletdisplayaddress RPC docfanquake
fa22ddc7d570feda649bedebeacd42a3b67ce24c doc: Fix walletdisplayaddress RPC doc (MarcoFalke) Pull request description: Previously the description was empty ACKs for top commit: fanquake: ACK fa22ddc7d570feda649bedebeacd42a3b67ce24c Tree-SHA512: f8898f9545378dd9f68eba036745776c1bc8d43c4753a19fcd9c3d945a11d9e07cd394d9ed215c2d090964830f785f812f18608f04fc7e2fb73730fc77c5d807
2021-09-16Merge bitcoin/bitcoin#22219: multiprocess: Start using init makeNode, ↵fanquake
makeChain, etc methods e4709c7b56612553fb7cbf16ef2d5099c5b732d0 Start using init makeNode, makeChain, etc methods (Russell Yanofsky) Pull request description: Use `interfaces::Init::make*` methods instead of `interfaces::Make*` functions, so interfaces can be constructed differently in different executable without having to change any code. (So for example `bitcoin-gui` can make an `interfaces::Node` pointer that communicates with a `bitcoin-node` subprocess, while `bitcoin-qt` can make an `interfaces::Node` pointer that controls node code in the same process.) --- This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). The commit was first part of larger PR #10102. ACKs for top commit: jamesob: reACK https://github.com/bitcoin/bitcoin/commit/e4709c7b56612553fb7cbf16ef2d5099c5b732d0 achow101: ACK e4709c7b56612553fb7cbf16ef2d5099c5b732d0 benthecarman: utACK e4709c7b56612553fb7cbf16ef2d5099c5b732d0 Tree-SHA512: 580c1979dbb2ef444157c8e53041e70d15ddeee77e5cbdb34f70b6d228cc2d2fe3843825f172da84e506200c58f7e0932f7cd4c006bb5058c1f4e43259394834
2021-09-15doc: Fix walletdisplayaddress RPC docMarcoFalke
2021-09-12wallet: refactor: avoid duplicate lookup on `mapValue["timesmart"]`Sebastian Falbesoner
Also, use a named cast for converting the atoi64() result into an unsigned int type.
2021-09-12wallet: refactor: inline function WriteOrderPos()Sebastian Falbesoner
Since accounts were removed in commit c9c32e6b844fc79467b7e24c6c916142a0d08484, this function is only called at one place and thus can be as well inlined.
2021-09-10wallet: refactor: inline function ReadOrderPos()Sebastian Falbesoner
Since accounts were removed in commit c9c32e6b844fc79467b7e24c6c916142a0d08484, this function is only called at one place and thus can be as well inlined. Also, avoid a duplicate lookup by using the find() method and dereference, instead of calling count() and operator[].
2021-09-09Merge bitcoin/bitcoin#22903: Enable clang-tidy bugprone-argument-comment and ↵fanquake
fix violations fa57fa1a2e764792b873998ddf38db2ac061dcb6 Enable clang-tidy bugprone-argument-comment and fix violations (MarcoFalke) Pull request description: Named arguments can be dangerous when they are wrong, because they are not enforced by the compiler. Currently there are only minor typos, no actual bugs. Fix the typos and add the `.clang-tidy` file to make it easier to find them in the future. ACKs for top commit: practicalswift: cr ACK fa57fa1a2e764792b873998ddf38db2ac061dcb6 fanquake: ACK fa57fa1a2e764792b873998ddf38db2ac061dcb6 Tree-SHA512: b66f01e0a1e77e56ed8454002176df660cc2cc0947a90785aa33cc5b8003a1f99fd8b2f8f89f2a0bf180ff2c42c031d69e669d127bb557b879c17975275a220b
2021-09-07Enable clang-tidy bugprone-argument-comment and fix violationsMarcoFalke
2021-09-05doc: Clarify that change_cost cannot be negative in GetSelectionWastebenthecarman
2021-09-03wallet: AddWalletDescriptor requires cs_wallet lockJoão Barbosa
No change in behavior, the lock is already held at call sites.
2021-09-03wallet: GetLabelAddresses requires cs_wallet lockJoão Barbosa
No change in behavior, the lock is already held at call sites.
2021-09-03Merge bitcoin/bitcoin#17526: Add Single Random Draw as an additional coin ↵Samuel Dobson
selection algorithm 3633b667ffca5a715d9fb27e977515c1e24f600a Use SelectCoinsSRD if it has less waste (Andrew Chow) 8bf789b4b4b26082aea1d91c4d7aa8b01aedfdcf Add SelectCoinsSRD function (Andrew Chow) 2ad3b5d2ad03f781f564ee697ef11e18b2edcea3 tests: wallet_basic lock needed unspents (Andrew Chow) b77885f13e480304085c654f1b948b20bba63452 tests: wallet_txn explicilty specify inputs (Andrew Chow) 59ba7d2861359f19fa740da9daad1a199409583f tests: rpc_fundrawtx better test for UTXO inclusion with include_unsafe (Andrew Chow) a165bfbe44b4db3a40b8d1ba8095035367c932a7 tests: rpc_fundrawtx use specific inputs for unavailable change test (Andrew Chow) df765a484d84702f066e127813fa45a7d440a957 tests: rpc_fundrawtx lock to UTXO types (Andrew Chow) Pull request description: To ease in the use of SRD as our fallback mechanism, this PR adds it as a secondary fallback algorithm in addition to the knapsack solver. Since #22009, the solution with the least waste will be chosen. This pattern is continued with SRD simply being another solution whose waste is compared. ACKs for top commit: glozow: reACK 3633b66 via `git range-diff 981b9d1...3633b66`, thanks for taking the suggestions laanwj: Concept and code review ACK 3633b667ffca5a715d9fb27e977515c1e24f600a Tree-SHA512: 895659f553fea2230990136565bdf18b1328de8b0ce47f06b64bb4d69301f6dd68cb38debe5c24fb6de1317b735fc020a987c541f00bbea65229de47e53adf92
2021-09-23Use SelectCoinsSRD if it has less wasteAndrew Chow
Try to find a solution with SelectCoinsSRD. If we do have one, add it to the list of solutions from which we choose the one with the least waste as the solution to use.
2021-09-23Add SelectCoinsSRD functionAndrew Chow
2021-09-01refactor: Detach wallet transaction methods (followup for move-only)Russell Yanofsky
Followup to commit "MOVEONLY: CWallet transaction code out of wallet.cpp/.h" that detaches and renames some CWalletTx methods, making into them into standalone functions or CWallet methods instead. There are no changes in behavior and no code changes that aren't purely mechanical. It just gives spend and receive functions more consistent names and removes the circular dependencies added by the earlier MOVEONLY commit. There are also no comment or documentation changes. Removed comments from transaction.h are just migrated to spend.h, receive.h, and wallet.h.
2021-09-01Merge bitcoin/bitcoin#22009: wallet: Decide which coin selection solution to ↵Samuel Dobson
use based on waste metric 86beee05795216738f51fa744539336503c26fd9 Use waste metric for deciding which selection to use (Andrew Chow) b3df0caf7c291a316298e54e73426c765e61c129 tests: Test GetSelectionWaste (Andrew Chow) 4f5ad43b1e05cd7b403f87aae4c4d42e5aea810b Add waste metric calculation function (Andrew Chow) 935b3ddf72aa390087684e03166c707f5b173434 scripted-diff: tests: Use KnapsackSolver directly (Andrew Chow) 6a023a6f904efe38dacd662d919aba74f066b1dc tests: Add KnapsackGroupOutputs helper function (Andrew Chow) d5069fc1aa7d335f3043227f843cbb9d8ba1507b tests: Use SelectCoinsBnB directly instead of AttemptSelection (Andrew Chow) 54de7b47463d98f860167d4e0b7e4ebb3926b59c Allow the long term feerate to be configured, default of 10 sat/vb (Andrew Chow) Pull request description: Branch and Bound introduced a metric that we call waste. This metric is used as part of bounding the search tree, but it can be generalized to all coin selection solutions, including those with change. As such, this PR introduces the waste metric at a higher level so that we can run both of our coin selection algorithms (BnB and KnapsackSolver) and choose the one which has the least waste. In the event that both find a solution with the same change, we choose the one that spends more inputs. Also this PR sets the long term feerate to 10 sat/vb rather than using the 1008 block estimate. This allows the long term feerate to be the feerate that we switch between consolidating and optimizing for fees. This also removes a bug where the long term feerate would incorrectly be set to the fallback fee. While this doesn't matter prior to this PR, it does have an effect following this. The long term feerate can be configured by the user through a new `-consolidatefeerate` option. ACKs for top commit: Xekyo: reACK 86beee0 via git range-diff fe47558...86beee0 meshcollider: re-utACK 86beee05795216738f51fa744539336503c26fd9 Tree-SHA512: 54b154b346538eca68ae2a3b83a033b495c1605c14f842bfc43ded2256b110983ce674c647fe753cf0305b1b178403d8d60d6d4203c7a712bec784be52e90d42
2021-08-27Use waste metric for deciding which selection to useAndrew Chow
Instead of always choosing BnB if it finds a solution, always do both BnB and KnapsackSolver and choose the one which has the least waste.
2021-08-27tests: Test GetSelectionWasteAndrew Chow
Tests for some waste calculation scenarios add_coin is modified to allow fee and long term fee to be set for an added CInputCoin.
2021-08-27Add waste metric calculation functionAndrew Chow
2021-08-27scripted-diff: tests: Use KnapsackSolver directlyAndrew Chow
When doing the coin selector tests for KnapsackSolver, call it directly instead of using AttemptSelection and hoping/instructing it uses KnapsackSolver. -BEGIN VERIFY SCRIPT- sed -i 's/testWallet\.AttemptSelection( /KnapsackSolver(/g' src/wallet/test/coinselector_tests.cpp sed -i 's/testWallet\.AttemptSelection(/KnapsackSolver(/g' src/wallet/test/coinselector_tests.cpp sed -i 's/, filter_standard, vCoins, setCoinsRet, nValueRet, coin_selection_params)/, KnapsackGroupOutputs(filter_standard), setCoinsRet, nValueRet)/g' src/wallet/test/coinselector_tests.cpp sed -i 's/, filter_confirmed, vCoins, setCoinsRet, nValueRet, coin_selection_params)/, KnapsackGroupOutputs(filter_confirmed), setCoinsRet, nValueRet)/g' src/wallet/test/coinselector_tests.cpp sed -i 's/, filter_standard_extra, vCoins, setCoinsRet, nValueRet, coin_selection_params)/, KnapsackGroupOutputs(filter_standard_extra), setCoinsRet, nValueRet)/g' src/wallet/test/coinselector_tests.cpp sed -i 's/BOOST_CHECK( /BOOST_CHECK(/g' src/wallet/test/coinselector_tests.cpp -END VERIFY SCRIPT-
2021-08-27tests: Add KnapsackGroupOutputs helper functionAndrew Chow
In order to change the KnapsackSolver tests to call KnapsackSolver, we need KnapsackGroupOutputs to create the OutputGroups filtered with the filter criteria.
2021-08-27tests: Use SelectCoinsBnB directly instead of AttemptSelectionAndrew Chow
Instead of calling AttemptSelection with the hopes/instruction that it uses BnB, call SelectCoinsBnB directly to test it.
2021-08-27Allow the long term feerate to be configured, default of 10 sat/vbAndrew Chow
The long term feerate is really the highest feerate that the user is comfortable with making consolidatory transactions. This is should thus be something that can be configured by the user via a new startup option -consolidatefeerate. The default value is 10 sat/vbyte, chosen arbitrarily (it seems like a reasonable number).
2021-08-26Merge bitcoin/bitcoin#22713: Fix build with Boost 1.77.0fanquake
acb7aad27ec8a184808aa7905887e3b2c5d54e9c Fix build with Boost 1.77.0 (Rafael Sadowski) Pull request description: BOOST_FILESYSTEM_C_STR changed to accept the path as an argument. ACKs for top commit: hebasto: ACK acb7aad27ec8a184808aa7905887e3b2c5d54e9c benthecarman: ACK acb7aad27ec8a184808aa7905887e3b2c5d54e9c fanquake: ACK acb7aad27ec8a184808aa7905887e3b2c5d54e9c - tested the fix with Boost 1.77.0 and 1.71.0. Tree-SHA512: c25fcb56971ee7a448cfb074f8a13696b32c16c63f81076f8a76911f93aa849c8f3637555b0b4215fa0d8b958641d7e4e60d10e103b833545cbc6b1f4009b526
2021-08-26Merge bitcoin/bitcoin#22183: Remove `gArgs` from `wallet.h` and `wallet.cpp`MarcoFalke
c3c213215b25f3e6f36d46b1d49dfcc3040cee1c Use `context.args` in `src/wallet/load.cpp`. (Kiminuo) 25de4e77feddf9b2f4d134bab5faa26c3e5a764d Use `context.args` in `CWallet::Create` instead of `gArgs`. (Kiminuo) aa5e7c9471c50771bc77b0ec4e0e0929e4a32eae Fix typo in bitcoin-cli.cpp (Kiminuo) Pull request description: The PR attempts to move us an inch towards the [goal](https://github.com/bitcoin/bitcoin/pull/21244#discussion_r615307465) by using `WalletContext` in `wallet.{h|cpp}` code instead of relying on the global state (i.e. `gArgs`). Edit: The PR builds on #19101. ACKs for top commit: ryanofsky: Code review ACK c3c213215b25f3e6f36d46b1d49dfcc3040cee1c. Changes since last review: just rebasing and adding wallet load commit Tree-SHA512: 2b436f5a219e32c2d529f55a89edca086d949396cebf9e089a21aa7b1c180e3c0fb17468f415dfc834f8e1614f8b3914c7e9a0bd33b95e7e0199c0dfe5ca9490