aboutsummaryrefslogtreecommitdiff
path: root/src/wallet/spend.cpp
AgeCommit message (Collapse)Author
2024-04-25refactor: Avoid copying util::Result valuesRyan Ofsky
Copying util::Result values is less efficient than moving them because they allocate memory and contain strings. Also this is needed to avoid compile errors in https://github.com/bitcoin/bitcoin/pull/25722 which adds a std::unique_ptr member to util::Result which implicity disables copying.
2024-02-09Merge bitcoin/bitcoin#27877: wallet: Add CoinGrinder coin selection algorithmAva Chow
13161ecf032b7a850686e5942c12222c8f3d0d52 opt: Skip over barren combinations of tiny UTXOs (Murch) b7672c7cdd87acb105639f475744094b53cc9891 opt: Skip checking max_weight separately (Murch) 1edd2baa37a69ff69c2eaeceaad1028f1968cbab opt: Cut if last addition was minimal weight (Murch) 5248e2a60d243b3d5c77ecd9e4c335daca399a48 opt: Skip heavier UTXOs with same effective value (Murch) 9124c73742287b06dfe6e8a94be56ace25f07b2c opt: Tiebreak UTXOs by weight for CoinGrinder (Murch) 451be19dc10381122f705bbb2127b083f1d598c6 opt: Skip evaluation of equivalent input sets (Murch) 407b1e3432b77511900b77be84d5d7450352f462 opt: Track remaining effective_value in lookahead (Murch) 5f84f3cc043c5fb15072f5072fee752eaa01a2ec opt: Skip branches with worse weight (Murch) d68bc74fb2e3ae4ae775ab544fe5b4ab46025abb fuzz: Test optimality of CoinGrinder (Murch) 67df6c629a2e9878b06c1903e90b67087eaa3688 fuzz: Add CoinGrinder fuzz target (Murch) 1502231229dbc32c94e80a2fc2959275c5d246ef coinselection: Track whether CG completed (Murch) 7488acc64685ae16e20b78e7ad018137f27fe404 test: Add coin_grinder_tests (Murch) 6cc9a46cd0f4ed80d4523bbef1508142e0c80d27 coinselection: Add CoinGrinder algorithm (Murch) 89d09566431f57034d9a7df32547ceb13d79c62c opt: Tie-break UTXO sort by waste for BnB (Murch) aaee65823c6e620bef5cc96d8026567e64d822fe doc: Document max_weight on BnB (Murch) Pull request description: ***Please refer to the [topic on Delving Bitcoin](https://delvingbitcoin.org/t/gutterguard-and-coingrinder-simulation-results/279) discussing Gutter Guard/Coingrinder simulation results.*** Adds a coin selection algorithm that minimizes the weight of the input set while creating change. Motivations --- - At high feerates, using unnecessary inputs can significantly increase the fees - Users are upset when fees are relatively large compared to the amount sent - Some users struggle to maintain a sufficient count of UTXOs in their wallet Approach --- So far, Bitcoin Core has used a balanced approach to coin selection, where it will generate multiple input set candidates using various coin selection algorithms and pick the least wasteful among their results, but not explicitly minimize the input set weight. Under some circumstances, we _do_ want to minimize the weight of the input set. Sometimes changeless solutions require many or heavy inputs, and there is not always a changeless solution for Branch and Bound to find in the first place. This can cause expensive transactions unnecessarily. Given a wallet with sufficient funds, `CoinGrinder` will pick the minimal-waste input set for a transaction with a change output. The current implementation only runs `CoinGrinder` at feerates over 3×long-term-feerate-estimate (by default 30 ṩ/vB), which may be a decent compromise between our goal to reduce costs for the users, but still permit transactions at lower feerates to naturally reduce the wallet’s UTXO pool to curb bloat. Trade-offs --- Simulations for my thesis on coin selection ([see Section 6.3.2.1 [PDF]](https://murch.one/erhardt2016coinselection.pdf)) suggest that minimizing the input set for all transactions tends to grind a wallet’s UTXO pool to dust (pun intended): an approach selecting inputs per coin-age-priority (in effect similar to “largest first selection”) on average produced a UTXO pool with 15× the UTXO count as Bitcoin Core’s Knapsack-based Coin Selection then (in 2016). Therefore, I do not recommend running `CoinGrinder` under all circumstances, but only at extreme feerates or when we have another good reason to minimize the input set for other reasons. In the long-term, we should introduce additional metrics to score different input set candidates, e.g. on basis of their privacy and wallet health impact, to pick from all our coin selection results, but until then, we may want to limit use of `CoinGrinder` in other ways. ACKs for top commit: achow101: ACK 13161ecf032b7a850686e5942c12222c8f3d0d52 sr-gi: ACK [13161ec](https://github.com/bitcoin/bitcoin/pull/27877/commits/13161ecf032b7a850686e5942c12222c8f3d0d52) sipa: ACK 13161ecf032b7a850686e5942c12222c8f3d0d52 Tree-SHA512: 895b08b2ebfd0b71127949b7dba27146a6d10700bf8590402b14f261e7b937f4e2e1b24ca46de440c35f19349043ed2eba4159dc2aa3edae57721384186dae40
2024-02-09coinselection: Add CoinGrinder algorithmMurch
CoinGrinder is a DFS-based coin selection algorithm that deterministically finds the input set with the lowest weight creating a change output.
2024-01-23Merge bitcoin/bitcoin#28560: wallet, rpc: `FundTransaction` refactorAva Chow
18ad1b9142e91cef2f5c6a693eeb2d0fbb8c517d refactor: pass CRecipient to FundTransaction (josibake) 5ad19668dbcc47486d1c18f711cea3d8a9d2e7e2 refactor: simplify `CreateRecipients` (josibake) 47353a608dc6e20e5fd2ca53850d6f9aa3240d4a refactor: remove out param from `ParseRecipients` (josibake) f7384b921c3460c7a3cc7827a68b2c613bd98f8e refactor: move parsing to new function (josibake) 6f569ac903e5ddaac275996a5d0c31b2220b7b81 refactor: move normalization to new function (josibake) 435fe5cd96599c518e26efe444c9d94d1277996b test: add tests for fundrawtx and sendmany rpcs (josibake) Pull request description: ## Motivation The primary motivation for this PR is to enable `FundTransaction` to take a vector of `CRecipient` objects to allow passing BIP352 silent payment addresses to RPCs that use `FundTransaction` (e.g. `send`, `walletcreatefundedpsbt`). To do that, SFFO logic needs to be moved out of `FundTransaction` so the `CRecipient` objects with the correct SFFO information can be created and then passed to `FundTransaction`. As a secondary motivation, this PR moves the SFFO stuff closer to the caller, making the code cleaner and easier to understand. This is done by having a single function which parses RPC inputs for SFFO and consistently using the `set<int>` method for communicating SFFO. I'm also not convinced we need to pass a full `CMutableTx` object to `FundTransaction`, but I'm leaving that for a follow-up PR/discussion, as its not a blocker for silent payments. ACKs for top commit: S3RK: reACK 18ad1b9142e91cef2f5c6a693eeb2d0fbb8c517d josibake: > According to my `range-diff` nothing changed. reACK [18ad1b9](https://github.com/bitcoin/bitcoin/commit/18ad1b9142e91cef2f5c6a693eeb2d0fbb8c517d) achow101: ACK 18ad1b9142e91cef2f5c6a693eeb2d0fbb8c517d Tree-SHA512: d61f017cf7d98489ef216475b68693fd77e7b53a26a6477dcd73e7e5ceff5036b2d21476e377839e710bb73644759d42c4f9f4b14ed96b3e56ed87b07aa6d1a7
2024-01-20Move TRACEx parameters to seperate linesRichard Myers
2024-01-20wallet: fix coin selection tracing to return -1 when no change posRichard Myers
2024-01-19refactor: pass CRecipient to FundTransactionjosibake
Instead turning tx.vout into a vector of `CRecipient`, make `FundTransaction` take a `CRecipient` vector directly. This allows us to remove SFFO logic from the wrapper RPC `FundTransaction` since the `CRecipient` objects have already been created with the correct SFFO values. This also allows us to remove SFFO from both `FundTransaction` function signatures. This sets us up in a future PR to be able to use these RPCs with BIP352 static payment codes.
2023-12-17wallet, mempool: propagete `checkChainLimits` error message to walletismaelsadeeq
Update CheckPackageLimits to use util::Result to pass the error message instead of out parameter. Also update test to reflect the error message from `CTxMempool` `CheckPackageLimits` output.
2023-12-12Merge bitcoin/bitcoin#28994: wallet: skip BnB when SFFO is enabledAndrew Chow
576bee88fd36e207b7288077626947a1fce0fc33 fuzz: disable BnB when SFFO is enabled (furszy) 05e5ff194c7722b4ebc2b9309fc0bf47b3cf1df7 test: add coverage for BnB-SFFO restriction (furszy) 0c5755761c3e544547899ad096121585dffa73df wallet: create tx, log resulting coin selection info (furszy) 5cea25ba795d6eb9ccc721d01560783ae576af34 wallet: skip BnB when SFFO is active (Murch) Pull request description: Solves #28918. Coming from https://github.com/bitcoin/bitcoin/issues/28918#issuecomment-1838626406 discussion. The intention is to decouple only the bugfix relevant commits from #28985, allowing them to be included in the 26.x release. This way, we can avoid disabling the coin selection fuzzing test for an entire release. Note: Have introduced few changes to the bug fix commit so that the unit tests pass without the additional burden introduced in #28985. ACKs for top commit: josibake: ACK https://github.com/bitcoin/bitcoin/commit/576bee88fd36e207b7288077626947a1fce0fc33 murchandamus: ACK https://github.com/bitcoin/bitcoin/commit/576bee88fd36e207b7288077626947a1fce0fc33 achow101: ACK 576bee88fd36e207b7288077626947a1fce0fc33 Tree-SHA512: f5d90eb3f3f524265afe4719495c9bf30f98b9af26cf039f7df5a7db977abae72caa7a3478cdd0ab10cd143bc1662e8fc5286b5bc10fc10f0dd582a45b45c31a
2023-12-08wallet: return CreatedTransactionResult from FundTransactionAndrew Chow
Instead of using the output parameters, return CreatedTransactionResult from FundTransaction in the same way that CreateTransaction does. Additionally, instead of modifying the original CMutableTransaction, the result from CreateTransactionInternal is used.
2023-12-08wallet: use optional for change position as an optional in CreateTransactionAndrew Chow
Instead of making -1 a magic number meaning no change or random change position, use an optional to have that meaning.
2023-12-08wallet: Explicitly preserve scriptSig and scriptWitness in CreateTransactionAndrew Chow
When creating a transaction with preset inputs, also preserve the scriptSig and scriptWitness for those preset inputs if they are provided (e.g. in fundrawtransaction).
2023-12-08wallet: Explicitly preserve transaction version in CreateTransactionAndrew Chow
We provide the preset nVersion to CCoinControl so that CreateTransactionInternal can be aware of it and set it in the produced transaction.
2023-12-08wallet: Explicitly preserve transaction locktime in CreateTransactionAndrew Chow
We provide the preset nLockTime to CCoinControl so that CreateTransactionInternal can be aware of it and set it in the produced transaction.
2023-12-08wallet: Set preset input sequence through coin controlAndrew Chow
2023-12-08wallet: Replace SelectExternal with SetTxOutAndrew Chow
Instead of having a separate CCoinControl::SelectExternal function, we can use the normal CCoinControl::Select function and explicitly use PreselectedInput::SetTxOut in the caller. The semantics of what an external input is remains.
2023-12-08coincontrol: Replace HasInputWeight with returning optional from GetAndrew Chow
2023-12-07wallet: create tx, log resulting coin selection infofurszy
Useful for understanding what is going on internally when the software is running. Debug issues, and provide more accurate feedback to users.
2023-12-07wallet: skip BnB when SFFO is activeMurch
Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
2023-11-21Use Txid in COutpointdergoegge
2023-11-17Merge bitcoin/bitcoin#28878: Remove version field from GetSerializeSizefanquake
83986f464c59a6517f790a960a72574e167f3f72 Include version.h in fewer places (Anthony Towns) c7b61fd61b199cbefda660c9d394bb4035a49528 Convert some CDataStream to DataStream (Anthony Towns) 1410d300df7e57a895f2697d9849a2201021c973 serialize: Drop useless version param from GetSerializeSize() (Anthony Towns) bf574a75016123309b894da895ab1c7a81731933 serialize: drop GetSerializeSizeMany (Anthony Towns) efa9eb6d7c8012fe4ed85699d81c8fe5dd18da1e serialize: Drop nVersion from [C]SizeComputer (Anthony Towns) Pull request description: Drops the version field from `GetSerializeSize()`, simplifying the code in various places. Also drop `GetSerializeSizeMany()` (as just removing the version parameter could result in silent bugs) and remove unnecessary instances of `#include <version.h>`. ACKs for top commit: maflcko: ACK 83986f464c59a6517f790a960a72574e167f3f72 📒 theuni: ACK 83986f464c59a6517f790a960a72574e167f3f72. Tree-SHA512: 36617b6dfbb1b4b0afbf673e905525fc6d623d3f568d3f86e3b9d4f69820db97d099e83a88007bfff881f731ddca6755ebf1549e8d8a7762437dfadbf434c62e
2023-11-16Merge bitcoin/bitcoin#28605: Fix typosfanquake
43de4d3630274e1287179c86896ed4c2d8b9eff4 doc: fix typos (Sjors Provoost) Pull request description: This PR fixes typos found by lint-spelling.py using codespell 2.2.6. Our CI linter job uses codespell 2.2.5 and found fewer typos that I did locally. In any case it's happy now. ACKs for top commit: pablomartin4btc: re ACK 43de4d3630274e1287179c86896ed4c2d8b9eff4 Tree-SHA512: c032fe86cb49c924a468385653b31f309a9db68c478d70335bba3e65a1ff3826abe80284fe00a090ab5a509e1edbf17e476f6922fb15d055e50f1103dad2ccb0
2023-11-16serialize: Drop useless version param from GetSerializeSize()Anthony Towns
2023-11-07doc: fix typosSjors Provoost
As found by lint-spelling.py using codespell 2.2.6.
2023-10-20interfaces: Rename CalculateBumpFees methods to be compatible with capn'protoRyan Ofsky
2023-10-12tidy: modernize-use-emplaceMarcoFalke
2023-09-19Merge bitcoin/bitcoin#28246: wallet: Use CTxDestination in CRecipient ↵fanquake
instead of just scriptPubKey ad0c469d98c51931b98b7fd937c6ac3eeaed024e wallet: Use CTxDestination in CRecipient rather than scriptPubKey (Andrew Chow) 07d3bdf4ebc06825ea24ab6f7c87aef6a22238c6 Add PubKeyDestination for P2PK scripts (Andrew Chow) 1a98a51c666e9ae77364115775ec2e0ba984e8e0 Allow CNoDestination to represent a raw script (Andrew Chow) 8dd067088d41f021b357d7db5fa5f0a9f61edddc Make WitnessUnknown members private (Andrew Chow) Pull request description: For silent payments, we want to provide a `SilentPaymentsDestination` to be used as the recipient, which requires `CRecipient` to use something other than just the `scriptPubKey` as we cannot know the output script for a silent payment prior to transaction creation. `CTxDestination` seems like the obvious place to add a `SilentPaymentsDestination` as it is our internal representation of an address. In order to still allow paying to arbitrary scriptPubKeys (e.g. for data carrier outputs, or the user hand crafted a raw transaction that they have given to `fundrawtransaction`), `CNoDestination` is changed to contain raw scripts. Additionally, P2PK scripts are now interpreted as a new `PubKeyDestination` rather than `PKHash`. This results in some things that would have given an address for P2PK scripts to no longer do so. This is arguably more correct. `ExtractDestination`'s behavior is slightly changed for the above. It now returns `true` for those destinations that have addresses, so P2PK scripts now result in `false`. Even though it returns false for `CNoDestination`, the script will now be included in that `CNoDestination`. Builds on #28244 ACKs for top commit: josibake: ACK https://github.com/bitcoin/bitcoin/pull/28246/commits/ad0c469d98c51931b98b7fd937c6ac3eeaed024e Tree-SHA512: ef3f8f3c7284779d9806c77c85b21caf910a79a1f7e7f1b51abcc0d7e074f14e00abf30f625a13075e41d94dad6202c10ddff462c0ee74c2ca4aab585b145a52
2023-09-14Merge bitcoin/bitcoin#26152: Bump unconfirmed ancestor transactions to ↵Andrew Chow
target feerate f18f9ef4d31c70e2d71ab90a24511692821418c3 Amend bumpfee for inputs with overlapping ancestry (Murch) 2e35e944dab09eff30952233f8dfc0b12c4553d5 Bump unconfirmed parent txs to target feerate (Murch) 3e3e05241128f68cf12f73ee06ff997395643885 coinselection: Move GetSelectionWaste into SelectionResult (Andrew Chow) c57889da6650715f3e1153b6104bbdae15fcac90 [node] interface to get bump fees (glozow) c24851be945b2a633ee44ed3c8a501eee5580b62 Make MiniMinerMempoolEntry fields private (Murch) ac6030e4d8f7d578cd4a8593f41189efca548064 Remove unused imports (Murch) d2f90c31ef3b8dee5a3e0804ecc62fa1cfec7cd5 Fix calculation of ancestor set feerates in test (Murch) a1f7d986e0211e54e21a1d4a570e5f15294dca72 Match tx names to index in miniminer overlap test (Murch) Pull request description: Includes some commits to address follow-ups from #27021: https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1554675156 Reduces the effective value of unconfirmed UTXOs by the fees necessary to bump their ancestor transactions to the same feerate. While the individual UTXOs always account for their full ancestry before coin-selection, we can correct potential overestimates with a second pass where we establish the ancestry and bump fee for the whole input set collectively. Fixes #9645 Fixes #9864 Fixes #15553 ACKs for top commit: S3RK: ACK f18f9ef4d31c70e2d71ab90a24511692821418c3 ismaelsadeeq: ACK f18f9ef4d31c70e2d71ab90a24511692821418c3 achow101: ACK f18f9ef4d31c70e2d71ab90a24511692821418c3 brunoerg: crACK f18f9ef4d31c70e2d71ab90a24511692821418c3 t-bast: ACK https://github.com/bitcoin/bitcoin/pull/26152/commits/f18f9ef4d31c70e2d71ab90a24511692821418c3, I reviewed the latest changes and run e2e tests against eclair, everything looks good :+1: Tree-SHA512: b65180c4243b1f9d13c311ada7a1c9f2f055d530d6c533b78c2068b50b8c29ac1321e89e85675b15515760d4f1b653ebd9da77b37c7be52d9bc565a3538f0aa6
2023-09-13Amend bumpfee for inputs with overlapping ancestryMurch
At the end of coin selection reduce the fees by the difference between the individual bump fee estimates and the collective bump fee estimate.
2023-09-13Bump unconfirmed parent txs to target feerateMurch
When a transaction uses an unconfirmed input, preceding this commit it would not consider the feerate of the parent transaction. Given a parent transaction with a lower ancestor feerate, this resulted in the new transaction's ancestor feerate undershooting the target feerate. This commit changes how we calculate the effective value of unconfirmed UTXOs. The effective value of unconfirmed UTXOs is decreased by the fee necessary to bump its ancestry to the target feerate. This also impacts the calculation of the waste metric: since the estimate for the current fee is increased by the bump fees, unconfirmed UTXOs current fees appear less favorable compared to their unchanged long term fees. This has one caveat: if multiple UTXOs have overlapping ancestries, each of their individual estimates will account for bumping all ancestors.
2023-09-12wallet: Use CTxDestination in CRecipient rather than scriptPubKeyAndrew Chow
2023-09-12Add PubKeyDestination for P2PK scriptsAndrew Chow
P2PK scripts are not PKHash destinations, they should have their own type. This also results in no longer showing a p2pkh address for p2pk outputs. However for backwards compatibility, ListCoinst will still do this conversion.
2023-09-08consensus/validation.h: remove needless GetTransactionOutputWeight helperAntoine Poinsot
Introduced in 9b7ec393b82ca9d7ada77d06e0835df0386a8b85. This copied the format of the other Get.*Weight helpers but it's useless for a CTxOut.
2023-08-25wallet: accurately account for the size of the witness stackAntoine Poinsot
When estimating the maximum size of an input, we were assuming the number of elements on the witness stack could be encode in a single byte. This is a valid approximation for all the descriptors we support (including P2WSH Miniscript ones), but may not hold anymore once we support Miniscript within Taproot descriptors (since the max standard witness stack size of 100 gets lifted). It's a low-hanging fruit to account for it correctly, so just do it now.
2023-08-25wallet: use descriptor satisfaction size to estimate inputs sizeAntoine Poinsot
Instead of using the dummysigner to compute a placeholder satisfaction, infer a descriptor on the scriptPubKey of the coin being spent and use the estimation of the satisfaction size given by the descriptor directly. Note this (almost, see next paragraph) exactly conserves the previous behaviour. For instance CalculateMaximumSignedInputSize was previously assuming the input to be spent in a transaction that spends at least one Segwit coin, since it was always accounting for the serialization of the number of witness elements. In this commit we use a placeholder for the size of the serialization of the witness stack size (1 byte). Since the logic in this commit is already tricky enough to review, and that it is only a very tiny approximation not observable through the existing tests, it is addressed in the next commit.
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-06-21[bug] Increase SRD target by change_feeMurch
I discovered via fuzzing of another coin selection approach that at extremely high feerates SRD may find input sets that lead to transactions without change outputs. This is an unintended outcome since SRD is meant to always produce a transaction with a change output—we use other algorithms to specifically search for changeless solutions. The issue occures when the flat allowance of 50,000 ṩ for change is insufficient to pay for the creation of a change output with a non-dust amount, at and above 1,613 ṩ/vB. Increasing the change budget by change_fees makes SRD behave as expected at any feerates.
2023-05-20refactor: Move system from util to common libraryTheCharlatan
Since the kernel library no longer depends on the system file, move it to the common library instead in accordance to the diagram in doc/design/libraries.md.
2023-04-26refactor: Make ListSelected return vectorSebastian Falbesoner
2023-04-26wallet: Use std::optional for GetExternalOutput and fixupsAurèle Oulès
2023-04-21Merge bitcoin/bitcoin#27419: move-only: Extract common/args from util/systemfanquake
be55f545d53d44fdcf2d8ae802e9eae551d120c6 move-only: Extract common/args and common/config.cpp from util/system (TheCharlatan) Pull request description: This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". It is part of a series of patches splitting up the `util/system` files. Its preceding pull request is https://github.com/bitcoin/bitcoin/pull/27254. The pull request contains an extraction of ArgsManager related functions from util/system into their own common/ file. The background of this commit is an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager. The ArgsManager belongs into the common library, since the kernel library should not depend on it. See [doc/design/libraries.md](https://github.com/bitcoin/bitcoin/blob/master/doc/design/libraries.md) for more information on this rationale. ACKs for top commit: MarcoFalke: re-ACK be55f545d53d44fdcf2d8ae802e9eae551d120c6 🚲 ryanofsky: Code review ACK be55f545d53d44fdcf2d8ae802e9eae551d120c6. Just small cleanups since the last review. hebasto: ACK be55f545d53d44fdcf2d8ae802e9eae551d120c6, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 90eb03334af0155b823030b4f2ecf286d35058d700ee2ddbbaa445be19e31eb0fe982656f35bd14ecee3ad2c3d0db3746855cb8f3777eff7253713e42873e111
2023-04-20Merge bitcoin/bitcoin#26720: wallet: coin selection, don't return results ↵Andrew Chow
that exceed the max allowed weight 25ab14712b9d80276016f9fc9bff7fb9c1d09635 refactor: coinselector_tests, unify wallet creation code (furszy) ba9431c505e1590db6103b9632134985cd4704dc test: coverage for bnb max weight (furszy) 5a2bc45ee0b123e461c5191322ed0b43524c3d82 wallet: clean post coin selection max weight filter (furszy) 2d112584e384de10021c64e4700455d71326824e coin selection: BnB, don't return selection if exceeds max allowed tx weight (furszy) d3a1c098e4b5df2ebbae20c6e390c3d783950e93 test: coin selection, add coverage for SRD (furszy) 9d9689e5a657956db8a30829c994600ec7d3098b coin selection: heap-ify SRD, don't return selection if exceeds max tx weight (furszy) 6107ec2229c5f5b4e944a6b10d38010c850094ac coin selection: knapsack, select closest UTXO above target if result exceeds max tx size (furszy) 1284223691127e76135a46d251c52416104f0ff1 wallet: refactor coin selection algos to return util::Result (furszy) Pull request description: Coming from the following comment https://github.com/bitcoin/bitcoin/pull/25729#discussion_r1029324367. The reason why we are adding hundreds of UTXO from different sources when the target amount is covered only by one of them is because only SRD returns a usable result. Context: In the test, we create 1515 UTXOs with 0.033 BTC each, and 1 UTXO with 50 BTC. Then perform Coin Selection to fund 49.5 BTC. As the selection of the 1515 small UTXOs exceeds the max allowed tx size, the expectation here is to receive a selection result that only contain the big UTXO. Which is not happening for the following reason: Knapsack returns a result that exceeds the max allowed transaction size, when it should return the closest utxo above the target, so we fallback to SRD who selects coins randomly up until the target is met. So we end up with a selection result with lot more coins than what is needed. ACKs for top commit: S3RK: ACK 25ab14712b9d80276016f9fc9bff7fb9c1d09635 achow101: ACK 25ab14712b9d80276016f9fc9bff7fb9c1d09635 Xekyo: reACK 25ab14712b9d80276016f9fc9bff7fb9c1d09635 theStack: Code-review ACK 25ab14712b9d80276016f9fc9bff7fb9c1d09635 Tree-SHA512: 2425de4cc479b4db999b3b2e02eb522a2130a06379cca0418672a51c4076971a1d427191173820db76a0f85a8edfff100114e1c38fb3b5dc51598d07cabe1a60
2023-04-19move-only: Extract common/args and common/config.cpp from util/systemTheCharlatan
This is an extraction of ArgsManager related functions from util/system into their own common file. Config file related functions are moved to common/config.cpp. The background of this commit is an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager. The ArgsManager belongs into the common library, since the kernel library should not depend on it. See doc/design/libraries.md for more information on this rationale.
2023-04-05wallet: clean post coin selection max weight filterfurszy
Now the coin selection algorithms contemplate the maximum allowed weight internally and return std::nullopt if their result exceeds it.
2023-04-05coin selection: BnB, don't return selection if exceeds max allowed tx weightfurszy
2023-04-05coin selection: heap-ify SRD, don't return selection if exceeds max tx weightfurszy
Uses a min-effective-value heap, so we can remove the least valuable input/s while the selected weight exceeds the maximum allowed weight. Co-authored-by: Murch <murch@murch.one>
2023-04-05coin selection: knapsack, select closest UTXO above target if result exceeds ↵furszy
max tx size The simplest scenario where this is useful is on the 'check_max_weight' unit test already: We create 1515 UTXOs with 0.033 BTC each, and 1 UTXO with 50 BTC. Then perform Coin Selection. As the selection of the 1515 small UTXOs exceeds the max allowed tx size, the expectation here is to receive a selection result that only contain the big UTXO (which is not happening for the reasons stated below). As knapsack returns a result that exceeds the max allowed transaction size, we fallback to SRD, which selects coins randomly up until the target is met. So we end up with a selection result with lot more coins than what is needed.
2023-04-03gui: bugfix, getAvailableBalance skips selected coinsfurszy
The previous behavior for getAvailableBalance when coin control has selected coins was to return the sum of them. Instead, we are currently returning the wallet's available total balance minus the selected coins total amount. This turns into a GUI-only issue for the "use available balance" button when the user manually select coins in the send screen. Reason: We missed to update the GetAvailableBalance function to include the coin control selected coins on #25685. Context: Since #25685 we skip the selected coins inside `AvailableCoins`, the reason is that there is no need to traverse the wallet's txes map just to get coins that can directly be fetched by their id.
generated by cgit v1.2.3 (git 2.26.2) at 2024-08-24 21:44:09 +0000