aboutsummaryrefslogtreecommitdiff
path: root/src/wallet/spend.cpp
AgeCommit message (Collapse)Author
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.
2023-03-23Merge bitcoin/bitcoin#24845: wallet: return error msg for ↵glozow
"too-long-mempool-chain" f3221d373a8623fe4808e00c7a92fbb6e0d6419b test: add wallet too-long-mempool-chain error coverage (furszy) acf0119d24c9f8fae825093249a46cd38e4a3a91 wallet: return error msg for too-long-mempool-chain failure (furszy) Pull request description: Fixes #23144. We currently return a general "Insufficient funds" from Coin Selection when we actually skipped unconfirmed UTXOs that surpassed the mempool ancestors limit. This PR make the error clearer by returning: "Unconfirmed UTXOs are available, but spending them creates a chain of transactions that will be rejected by the mempool" Also, added an early return from Coin Selection if the sum of the discarded coins decreases the available balance below the target amount. ACKs for top commit: achow101: ACK f3221d373a8623fe4808e00c7a92fbb6e0d6419b S3RK: Code review ACK f3221d373a8623fe4808e00c7a92fbb6e0d6419b Xekyo: ACK f3221d373a8623fe4808e00c7a92fbb6e0d6419b Tree-SHA512: 13e5824b75ac302280ff894560a4ebf32a74f32fe49ef8281f2bc99c0104b92cef33d3b143c6e131f3a07eafe64533af7fc60abff585142c134b9d6e531a6a66
2023-03-22Merge bitcoin/bitcoin#27271: RPC: Fix fund transaction crash when at ↵Andrew Chow
0-value, 0-fee d7cc503843769b789dc5f031b4ddc75d555ba980 Fix fund transaction case at 0-value, 0-fee (Greg Sanders) Pull request description: and when no inputs are pre-selected. triggered via: walletcreatefundedpsbt '[]' '[{"data": "deadbeef"}]' 0 '{"fee_rate": "0"}' ACKs for top commit: achow101: ACK d7cc503843769b789dc5f031b4ddc75d555ba980 josibake: ACK https://github.com/bitcoin/bitcoin/pull/27271/commits/d7cc503843769b789dc5f031b4ddc75d555ba980 furszy: Crashes sucks code ACK d7cc5038 Tree-SHA512: 3f5e10875666aaf52c11d6a38b951aa75d0cbe684cc7f904e199f7a864923bf31d03a654687f8b746cae0eebb886a799bff2c6d200699438480d4c0ff8785f3a
2023-03-19Merge bitcoin/bitcoin#25666: refactor: wallet, do not translate init options ↵fanquake
names e43a547a3674a31504a60ede9b4912e014a54139 refactor: wallet, do not translate init arguments names (furszy) Pull request description: Simple, and not interesting, refactor that someone has to do sooner or later. We are translating some init arguments names when those shouldn't be translated. ACKs for top commit: achow101: ACK e43a547a3674a31504a60ede9b4912e014a54139 MarcoFalke: lgtm ACK e43a547a3674a31504a60ede9b4912e014a54139 ryanofsky: Code review ACK e43a547a3674a31504a60ede9b4912e014a54139. Just rebased since last review. Tree-SHA512: c6eca98fd66d54d5510de03ab4e63c00ba2838af4237d2bb135d01c47f8ad8ca9aa7ae1e45cf668afcfb9dd958b075a1756cc887b3beef2cb494933d4d83eab0
2023-03-16Fix fund transaction case at 0-value, 0-feeGreg Sanders
2023-03-10wallet: return error msg for too-long-mempool-chain failurefurszy
We currently return "Insufficient funds" which doesn't really describe what went wrong; the tx creation failed because of a long-mempool-chain, not because of a lack of funds. Also, return early from Coin Selection if the sum of the discarded coins decreases the available balance below the target amount.
2023-03-08wallet: remove coin control arg from AutomaticCoinSelectionfurszy
we only need the "include unsafe" flag, not all what coin control stores.
2023-03-08wallet: GroupOutput, remove unneeded "spendable" checkfurszy
`AvailableCoins` already filters non-spendable coins.
2023-03-08wallet: APS, don't create empty groupsfurszy
By moving the "positive-only" flag out of the lambda function.
2023-03-08wallet: do not make two COutputs, use shared_ptrfurszy
2023-03-07wallet: refactor coin selection algos to return util::Resultfurszy
so the selection processes can retrieve different errors and not uninformative std::nullopt
2023-03-06wallet: single output groups filtering and grouping processfurszy
Optimizes coin selection by performing the "group outputs" procedure only once, outside the "attempt selection" process. Avoiding the repeated execution of the 'GroupOutputs' operation that occurs on each coin eligibility filters (up to 8 of them); then for every coin vector type plus one for all the coins together. This also let us not perform coin selection over coin eligibility filtered groups that don't add new elements. (because, if the previous round failed, and the subsequent one has the same coins, then this new round will fail again).
2023-03-06wallet: unify outputs grouping processfurszy
The 'GroupOutputs()' function performs the same calculations for only-positive and mixed groups, the only difference is that when we look for only-positive groups, we discard negative utxos. So, instead of wasting resources calling GroupOutputs() for positive-only first, then call it again to include the negative ones in the result, we can execute GroupOutputs() only once, including in the response both group types (positive-only and mixed).
2023-03-06wallet: decouple outputs grouping process from each ChooseSelectionResultfurszy
Another step towards the single OutputGroups calculation goal
2023-03-06refactor: make OutputGroup::m_outputs field a vector of shared_ptrfurszy
Initial steps towards sharing COutput instances across all possible OutputGroups (instead of copying them time after time).
2023-03-03wallet: make OutputGroup "positive_only" filter explicitfurszy
And not hide it inside the `OutputGroup::Insert` method. This method does not return anything if insertion fails. We can know before calling `Insert` whether the coin will be accepted or not.
2023-02-23wallet: skip R-value grinding for external signersSjors Provoost
When producing a dummy signature for the purpose of estimating the transaction fee, do not assume an external signer performs R-value grinding on the signature. In particular, this avoids a scenario where the fee rate is 1 sat / vbyte and a transaction with a 72 byte signature is not accepted into our mempool. This commit also drops the nullptr default for CCoinControl arguments for functions that it touches. This is because having a boolean argument right next to an optional pointer is error prone. Co-Authored-By: S3RK <1466284+S3RK@users.noreply.github.com>
2023-02-20Merge bitcoin/bitcoin#27053: wallet: reuse change dest when re-creating TX ↵fanquake
with avoidpartialspends 14b4921a91920df25b19ff420bfe2bff8c56f71e wallet: reuse change dest when recreating TX with avoidpartialspends (Matthew Zipkin) Pull request description: Closes https://github.com/bitcoin/bitcoin/issues/27051 When the wallet creates a transaction internally, it will also create an alternative that spends using destination groups and see if the fee difference is negligible. If it costs the user the same to send the grouped version, we send it (even if the user has `avoidpartialspends` set to `false` which is default). This patch ensures that the second transaction creation attempt re-uses the change destination selected by the first attempt. Otherwise, the first change address remains reserved, will not be used in the second attempt, and then will never be used by the wallet, leaving gaps in the BIP44 chain. If the user had `avoidpartialspends` set to true, there is no second version of the created transaction and the change addresses are not affected. I believe this behavior was introduced in https://github.com/bitcoin/bitcoin/pull/14582 ACKs for top commit: achow101: ACK 14b4921a91920df25b19ff420bfe2bff8c56f71e Tree-SHA512: a3d56f251ff4b333fc11325f30d05513e34ab0a2eb703fadd0ad98d167ae074493df1a24068298336c6ed2da6b31aa2befa490bc790bbc260ed357c8f2397659
2023-02-16Merge bitcoin/bitcoin#24149: Signing support for Miniscript Descriptorsfanquake
6c7a17a8e0eec377f83ed1399f003ae70b898270 psbt: support externally provided preimages for Miniscript satisfaction (Antoine Poinsot) 840a396029316896beda46600aec3c1af09a899c qa: add a "smart" Miniscript fuzz target (Antoine Poinsot) 17e3547241d593bc92c5c6b36c54284d9d9f3feb qa: add a fuzz target generating random nodes from a binary encoding (Antoine Poinsot) 611e12502a5887ffb751bb92fadaa334d484824b qa: functional test Miniscript signing with key and timelocks (Antoine Poinsot) d57b7f2021d2369f6e88cdf0f562aab27c51beaf refactor: make descriptors in Miniscript functional test more readable (Antoine Poinsot) 0a8fc9e200b5018c1efd6f9126eb405ca0beeea3 wallet: check solvability using descriptor in AvailableCoins (Antoine Poinsot) 560e62b1e221832ae99ff8684559a7b8f9df84a7 script/sign: signing support for Miniscripts with hash preimage challenges (Antoine Poinsot) a2f81b6a8f1ff3b0750711409c7538812a52ef40 script/sign: signing support for Miniscript with timelocks (Antoine Poinsot) 61c6d1a8440db09c44d7fd367a6f2c641ea93d40 script/sign: basic signing support for Miniscript descriptors (Antoine Poinsot) 4242c1c52127df3a24be0c15b88d4fc463af04fc Align 'e' property of or_d and andor with website spec (Pieter Wuille) f5deb417804b9f267830bd40177677987df4526d Various additional explanations of the satisfaction logic from Pieter (Pieter Wuille) 22c5b00345063bdeb8b6d3da8b5692d18f92bfb7 miniscript: satisfaction support (Antoine Poinsot) Pull request description: This makes the Miniscript descriptors solvable. Note this introduces signing support for much more complex scripts than the wallet was previously able to solve, and the whole tooling isn't provided for a complete Miniscript integration in the wallet. Particularly, the PSBT<->Miniscript integration isn't entirely covered in this PR. ACKs for top commit: achow101: ACK 6c7a17a8e0eec377f83ed1399f003ae70b898270 sipa: utACK 6c7a17a8e0eec377f83ed1399f003ae70b898270 (to the extent that it's not my own code). Tree-SHA512: a71ec002aaf66bd429012caa338fc58384067bcd2f453a46e21d381ed1bacc8e57afb9db57c0fb4bf40de43b30808815e9ebc0ae1fbd9e61df0e7b91a17771cc
2023-02-15wallet: reuse change dest when recreating TX with avoidpartialspendsMatthew Zipkin
2023-02-11wallet: check solvability using descriptor in AvailableCoinsAntoine Poinsot
This is a workaround for Miniscript descriptors containing hash challenges. For those we can't mock the signature creator without making OP_EQUAL mockable in the interpreter, so CalculateMaximumInputSize will always return -1 and outputs for these descriptors would appear unsolvable while they actually are.
2023-01-18Merge bitcoin/bitcoin#25659: wallet: simplify ListCoins implementationAndrew Chow
a2ac6f9582c4c996fa36e4801fa0aac756235754 wallet: unify FindNonChangeParentOutput functions (furszy) b3f4e827378e010cd2a5d1b876d01db52c054d26 wallet: simplify ListCoins implementation (furszy) Pull request description: Focused on the following changes: 1) Removed the entire locked coins lookup that was inside `ListCoins` by including them directly on the `AvailableCoins` result (where we were skipping them before). 2) Unified both `FindNonChangeParentOutput` functions (only called from `ListCoins`) ACKs for top commit: achow101: ACK a2ac6f9582c4c996fa36e4801fa0aac756235754 aureleoules: ACK a2ac6f9582c4c996fa36e4801fa0aac756235754, LGTM theStack: Code-review ACK a2ac6f9582c4c996fa36e4801fa0aac756235754 Tree-SHA512: f72b21ee1600c5992559b5dcd8ff260527afac2ec696737f998343a0850b84d439e7f86ea52a14cc0cddabf132a30bf5b52fb34950578ac323083d4375b937f1
2023-01-03refactor: use braced init for integer constants instead of c style castsPasta
2023-01-03Merge bitcoin/bitcoin#26661: wallet: Coin Selection, return accurate error ↵Andrew Chow
messages 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 gui: create tx, launch error dialog if backend throws runtime_error (furszy) f4d79477ff0946b0bd340ade9251fa38e3b95dd7 wallet: coin selection, add duplicated inputs checks (furszy) 0aa065b14e67592d5be8f46ebbe5d59a083ff0a5 wallet: return accurate error messages from Coin Selection (furszy) 7e8340ab1a970a14e180b1fcf420b46a5657b062 wallet: make SelectCoins flow return util::Result (furszy) e5e147fe97f706e82bc51358f8bdc355f355be57 wallet: refactor eight consecutive 'AttemptSelection' calls into a loop (furszy) Pull request description: Work decoupled from #25806, which cleanup and improves the Coin Selection flow further. Adding the capability to propagate specific error messages from the Coin Selection process to the user. Instead of always returning the general "Insufficient funds" message which is not always accurate to what happened internally. Letting us instruct the user how to proceed under certain circumstances. The following error messages were added: 1) If the selection result exceeds the maximum transaction weight, we now will return: -> "The inputs size exceeds the maximum weight. Please try sending a smaller amount or manually consolidating your wallet's UTXOs". 2) If the user pre-selected inputs and disallowed the automatic coin selection process (no other inputs are allowed), we now will return: -> "The preselected coins total amount does not cover the transaction target. Please allow other inputs to be automatically selected or include more coins manually". 3) The double-counted preset inputs during Coin Selection error will now throw an "internal bug detected" message instead of crashing the node. The essence of this work comes from several comments: 1. https://github.com/bitcoin/bitcoin/pull/26560#discussion_r1037395665 2. https://github.com/bitcoin/bitcoin/pull/25729#discussion_r940619491 3. https://github.com/bitcoin/bitcoin/pull/25269#pullrequestreview-1135240825 4. https://github.com/bitcoin/bitcoin/issues/23144 (which is connected to #24845) ACKs for top commit: ishaanam: crACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 achow101: ACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 aureleoules: ACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 theStack: ACK 76dc547ee7b05864e7b1b6c55fc0301d47aa3a15 :city_sunrise: Tree-SHA512: 9de30792d7a5849cae77747aa978e70390b66ee9d082779a56088a024f82e725b0af050e6603aece0ac8229f6d73bc471ba97b4ab69dc7eddf419f5f56ae89a5
2023-01-03wallet: unify FindNonChangeParentOutput functionsfurszy
The function is only used in ListCoins.
2023-01-03wallet: simplify ListCoins implementationfurszy
Can remove the locked coins lookup if we include them directly inside the AvailableCoins result
2023-01-03Merge bitcoin/bitcoin#25789: test: clean and extend availablecoins_tests ↵Andrew Chow
coverage 9622fe64b8785430c71d4abc8637075026dc690c test: move coins result test to wallet_tests.cpp (furszy) f69347d0588647ff9a4e986c7be987827a0417f4 test: extend and simplify availablecoins_tests (furszy) 212ccdf2c2b70d973b18ae78f0158ec5f0c3bbb4 wallet: AvailableCoins, add arg to include/skip locked coins (furszy) Pull request description: Negative PR with extended test coverage :). 1) Cleaned duplicated code and added coverage for the 'AvailableCoins' incremental result. 2) The class `AvailableCoinsTestingSetup` inside `availablecoins_tests.cpp` is a plain copy of `ListCoinsTestingSetup` that is inside `wallet_tests.cpp`. So, deleted the file and moved the `BasicOutputTypesTest` test case to `wallet_tests.cpp`. 3) Added arg to include/skip locked coins from the `AvailableCoins` result. This is needed for point (1) as otherwise the wallet will spend the coins that we recently created due its closeness to the recipient amount. Note: this last point comes from #25659 where I'm using the same functionality to clean/speedup another flow as well. ACKs for top commit: achow101: ACK 9622fe64b8785430c71d4abc8637075026dc690c theStack: ACK 9622fe64b8785430c71d4abc8637075026dc690c aureleoules: reACK 9622fe64b8785430c71d4abc8637075026dc690c, nice cleanup! Tree-SHA512: 1ed9133120bfe8815455d1ad317bb0ff96e11a0cc34ee8098716ab9b001749168fa649212b2fa14b330c1686cb1f29039ff1f88ae306db68881b0428c038f388
2022-12-24scripted-diff: Bump copyright headersHennadii Stepanov
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT- Commits of previous years: - 2021: f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7 - 2020: fa0074e2d82928016a43ca408717154a1c70a4db - 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
2022-12-21wallet: return accurate error messages from Coin Selectionfurszy
and not the general "Insufficient funds" when the wallet actually have funds. Two new error messages: 1) If the selection result exceeds the maximum transaction weight, we now will return: "The inputs size exceeds the maximum weight". 2) If the user preselected inputs and disallowed the automatic coin selection process (no other inputs are allowed), we now will return: "The preselected coins total amount does not cover the transaction target".
2022-12-21wallet: make SelectCoins flow return util::Resultfurszy
2022-12-21wallet: refactor eight consecutive 'AttemptSelection' calls into a loopfurszy
and remove 'CoinEligibilityFilter' default constructor to prevent mistakes.
2022-12-14Merge bitcoin/bitcoin#26668: wallet: if only have one output type, don't ↵Andrew Chow
perform "mixed" coin selection 89c1491d35389eac0c1fecc59333cdfae3b1bd2c wallet: if only have one output type, don't perform "mixed" coin selection (furszy) Pull request description: For wallets that only have one output type, we are currently performing the same selection process over the same coins twice. The "mixed coin selection" doesn't add any value to the result (there is nothing to mix if the available coins struct has only one type). ACKs for top commit: achow101: ACK 89c1491d35389eac0c1fecc59333cdfae3b1bd2c john-moffett: ACK 89c1491d35389eac0c1fecc59333cdfae3b1bd2c kristapsk: cr utACK 89c1491d35389eac0c1fecc59333cdfae3b1bd2c Tree-SHA512: 672eaeed3ba911d13fa61a46f719c8fe1ebe4d2dc7d723040e71937c693659411bc99cdbd9f0014e836b70eebeff1b8ca861f4d81d39e6f79f437364a526edbe
2022-12-09wallet: Sanity check fee paid cannot be negativeAndrew Chow
We need to check that the fee is not negative even before it is finalized. The setting of fees for SFFO may adjust the fee to be "correct" and no longer negative, but erroneously reduce the amounts too far. So we need to check this condition before we do those adjustments.
2022-12-09wallet: Move fee underpayment check to after fee settingAndrew Chow
It doesn't make sense to be checking whether the fee paid is underpaying before we've finished setting the fees. So do that after we have done the reduction for SFFO and change adjustment for fee overpayment.
2022-12-08wallet: if only have one output type, don't perform "mixed" coin selectionfurszy
there is nothing to mix.
2022-12-06wallet: Rename nFeeRet in CreateTransactionInternal to current_feeAndrew Chow
nFeeRet represents the fee that the transaction currently pays. Update it's name to reflect that.
2022-12-06Merge bitcoin/bitcoin#25729: wallet: Check max transaction weight in ↵Andrew Chow
CoinSelection c7c7ee9d0b2d7b303b9300f941e37e09e7d8d8b6 test: Check max transaction weight in CoinSelection (Aurèle Oulès) 6b563cae92957dc30dc35103a7c321fdb0115ef3 wallet: Check max tx weight in coin selector (Aurèle Oulès) Pull request description: This PR is an attempt to fix #5782. I have added 4 test scenarios, 3 of them provided here https://github.com/bitcoin/bitcoin/issues/5782#issuecomment-73819058 (slightly modified to use a segwit wallet). Here are my benchmarks : ## PR | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 1,466,341.00 | 681.97 | 0.6% | 11,176,762.00 | 3,358,752.00 | 3.328 | 1,897,839.00 | 0.3% | 0.02 | `CoinSelection` ## Master | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 1,526,029.00 | 655.30 | 0.5% | 11,142,188.00 | 3,499,200.00 | 3.184 | 1,994,156.00 | 0.2% | 0.02 | `CoinSelection` ACKs for top commit: achow101: reACK c7c7ee9d0b2d7b303b9300f941e37e09e7d8d8b6 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25729/commits/c7c7ee9d0b2d7b303b9300f941e37e09e7d8d8b6 furszy: diff ACK c7c7ee9d Tree-SHA512: ef0b28576ff845174651ba494aa9adee234c96e6f886d0e032eceb7050296e45b099dda0039d1dfb9944469f067627b2101f3ff855c70353cf39d1fc7ee81828
2022-12-06Merge bitcoin/bitcoin#26238: clang-tidy: fixup named argument commentsMarcoFalke
203886c443c4ad76f8a1dba740a286e383e55206 Fixup clang-tidy named argument comments (fanquake) Pull request description: Fix comments so they are checked/consistent. Fix incorrect comments. ACKs for top commit: hebasto: ACK 203886c443c4ad76f8a1dba740a286e383e55206, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: e1257840f91fe39842e2b19299c1633604697b8584fe44b1977ada33cdde5433c877ed0b669fa334e20b04971dc89cd47d58b2783b6f7004521f01d05a1245da
2022-12-06Merge bitcoin/bitcoin#26611: wallet: Change coin selection fee assert to errorMarcoFalke
3eb041f014870954db564369a4be4bd0dea48fbe wallet: Change coin selection fee assert to error (Andrew Chow) c6e7f224c119f47af250a9e0c5b185cb98b30c4c util: Add StrFormatInternalBug and STR_INTERNAL_BUG (MarcoFalke) Pull request description: Returning an error instead of asserting for the low fee check will be better as it does not crash the node and instructs users to report the bug. ACKs for top commit: S3RK: ACK 3eb041f014870954db564369a4be4bd0dea48fbe aureleoules: ACK 3eb041f014870954db564369a4be4bd0dea48fbe furszy: ACK 3eb041f0 Tree-SHA512: 118c13d7cdfce492080edd4cb12e6d960695377b978c7573f9c58b6d918664afd0e8e591eed0605d08ac756fa8eceed456349de5f3a025174069abf369bb5a5f
2022-12-05wallet: Check max tx weight in coin selectorAurèle Oulès
Co-authored-by: Andrew Chow <github@achow101.com>
generated by cgit v1.2.3 (git 2.26.2) at 2024-10-13 05:22:21 +0000