aboutsummaryrefslogtreecommitdiff
path: root/src/wallet/rpc
AgeCommit message (Collapse)Author
2022-06-14doc: Update importaddress mention incompatibility with descriptor walletBrokenProgrammer
2022-06-08wallet: avoid extra IsSpentKey -> GetWalletTx lookupsfurszy
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: 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-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-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#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-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-16Add proper thread safety annotation to `CWallet::GetTxConflicts()`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-04-26Merge bitcoin/bitcoin#24977: rpc: Explain active and internal in listdescriptorsfanquake
4637bbe448ae7370528f40092ce230c32602a6d6 rpc: Explain active and internal in listdescriptors (Andrew Chow) Pull request description: The current help text for active and internal in listdescriptors is not particularly helpful. They require the reader to already know what those terms mean. This help text is updated to actually explain the definitions of those words in context of a descriptor wallet. ACKs for top commit: S3RK: ACK 4637bbe448ae7370528f40092ce230c32602a6d6 jarolrod: ACK 4637bbe448ae7370528f40092ce230c32602a6d6 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/24977/commits/4637bbe448ae7370528f40092ce230c32602a6d6 Tree-SHA512: 0af2c04f3b9920799cf616ad618bde9248eb9f74cc28f443b5b0f6646deba76e9b1415aca0865ad3bcc24aa6af0e9d07ad7b7cd80f0fe80838cf847f1b944426
2022-04-26Merge bitcoin/bitcoin#24959: Remove not needed clang-format off commentslaanwj
fa870e3d4ccd6dfd0a9a8f2c608721a7251114e2 Remove not needed clang-format off comments (MarcoFalke) Pull request description: It seems odd to disable clang-format and force manual formatting when there is no need for it. So remove the clang-format comments and other unneeded comments. Can be reviewed with `--word-diff-regex=. --ignore-all-space` Looks like this was initially added in commit d9d79576f423cd9c5cef4547c7e3648dbb339460 to accommodate a linter that has since been removed and replaced by a functional test. ACKs for top commit: laanwj: Code review ACK fa870e3d4ccd6dfd0a9a8f2c608721a7251114e2 fanquake: ACK fa870e3d4ccd6dfd0a9a8f2c608721a7251114e2 Tree-SHA512: 0f8f97c12f5dbe517dd96c10b10ce1b8772d8daed33e6b41f73ea1040e89888cf3b8c0ad7b20319e366fe30c71e8b181c89098ae7f6a3deb8647e1b4731db815
2022-04-26Merge bitcoin/bitcoin#22953: refactor: introduce single-separator split ↵fanquake
helper (boost::split replacement) a62e84438d27ee6213219fe2c233e58814fcbb5d fuzz: add `SplitString` fuzz target (MarcoFalke) 4fad7e46d94a0fdee4ff917e81360d7ae6bd8110 test: add unit tests for `SplitString` helper (Kiminuo) 9cc8e876e412056ed22d364538f0da3d5d71946d refactor: introduce single-separator split helper `SplitString` (Sebastian Falbesoner) Pull request description: This PR adds a simple string split helper `SplitString` that takes use of the spanparsing `Split` function that was first introduced in #13697 (commit fe8a7dcd78cfeedc9a7c705e91384f793822912b). This enables to replace most calls to `boost::split`, in the cases where only a single separator character is used. Note that while previous attempts to replace `boost::split` were controversial (e.g. #13751), this one has a trivial implementation: it merely uses an internal helper (that is unit tested and in regular use with output descriptiors) and converts its result from spans to strings. As a drawback though, not all `boost::split` instances can be tackled. As a possible optimization, one could return a vector of `std::string_view`s (available since C++17) instead of strings, to avoid copies. This would need more carefulness on the caller sites though, to avoid potential lifetime issues, and it's probably not worth it, considering that none of the places where strings are split are really performance-critical. ACKs for top commit: martinus: Code review ACK a62e84438d27ee6213219fe2c233e58814fcbb5d. Ran all tests. I also like that with `boost::split` it was not obvious that the resulting container was cleared, and with `SplitString` API that's obvious. Tree-SHA512: 10cb22619ebe46831b1f8e83584a89381a036b54c88701484ac00743e2a62cfe52c9f3ecdbb2d0815e536c99034558277cc263600ec3f3588b291c07eef8ed24
2022-04-25rpc: bumpfee signer supportSjors Provoost
2022-04-25rpc: document bools in FillPSBT() callsSjors Provoost
2022-04-25rpc: Explain active and internal in listdescriptorsAndrew Chow
The current help text for active and internal in listdescriptors is not particularly helpful. They require the reader to already know what those terms mean. This help text is updated to actually explain the definitions of those words in context of a descriptor wallet.
2022-04-25Remove not needed clang-format off commentsMarcoFalke
Can be reviewed with --word-diff-regex=. --ignore-all-space
2022-04-24Merge bitcoin/bitcoin#24812: util/check: Add CHECK_NONFATAL identity ↵MarcoFalke
function and NONFATAL_UNREACHABLE macro ee02c8bd9aedad8cfd3c2618035fe275da025fb9 util/check: Add CHECK_NONFATAL identity function, NONFATAL_UNREACHABLE AND UNREACHABLE macros (Aurèle Oulès) Pull request description: This PR replaces the macro `CHECK_NONFATAL` with an identity function. I simplified the usage of `CHECK_NONFATAL` where applicable in `src/rpc`. This function is useful in sanity checks for RPC and command-line interfaces. Context: https://github.com/bitcoin/bitcoin/pull/24804#discussion_r846182474. Also adds `UNREACHABLE_NONFATAL` macro. ACKs for top commit: jonatack: ACK ee02c8bd9aedad8cfd3c2618035fe275da025fb9 MarcoFalke: ACK ee02c8bd9aedad8cfd3c2618035fe275da025fb9 🍨 Tree-SHA512: 3cba09223cd7b22e62fe5d0b46c4a024c1d9957d4268ba6d3fb07fcc0a5854fc0886bb3266184e6a7df5df91373b3e84edd6adf6999c4e934aeef8c043b01aa2
2022-04-16util/check: Add CHECK_NONFATAL identity function, NONFATAL_UNREACHABLE AND ↵Aurèle Oulès
UNREACHABLE macros
2022-04-14rpc, wallet: setwalletflags warnings are optionalAndrew Chow
Without this, trying to disable a wallet flag results in an Internal bug detected.
2022-04-11refactor: introduce single-separator split helper `SplitString`Sebastian Falbesoner
This helper uses spanparsing::Split internally and enables to replace all calls to boost::split where only a single separator is passed. Co-authored-by: Martin Ankerl <Martin.Ankerl@gmail.com> Co-authored-by: MarcoFalke <falke.marco@gmail.com>
2022-04-06doc: Convert remaining comments to clang-tidy formatMarcoFalke
2022-04-04refactor: fix clang-tidy named args usagefanquake
2022-03-30refactor: use named args when ScriptToUniv or TxToUniv are invokedMichael Dietz
2022-03-30Merge bitcoin/bitcoin#24118: Add 'sendall' RPC née sweepMarcoFalke
bb84b7145b31dbfdcb4cf0b9b6e612a57e573993 add tests for no recipient and using send_max while inputs are specified (ishaanam) 49090ec4025152c847be8a5ab6aa6f379e345260 Add sendall RPC née sweep (Murch) 902793c7772e5bdd5aae5b0d20a32c02a1a6dc7c Extract FinishTransaction from send() (Murch) 6d2208a3f6849a3732af6ff010eeea629b9b10d0 Extract interpretation of fee estimation arguments (Murch) a31d75e5fb5c1304445d698595079e29f3cd3a3a Elaborate error messages for outdated options (Murch) 35ed094e4b0e0554e609709f6ca1f7d17096882c Extract prevention of outdated option names (Murch) Pull request description: Add sendall RPC née sweep _Motivation_ Currently, the wallet uses a fSubtractFeeAmount (SFFO) flag on the recipients objects for all forms of sending calls. According to the commit discussion, this flag was chiefly introduced to permit sweeping without manually calculating the fees of transactions. However, the flag leads to unintuitive behavior and makes it more complicated to test many wallet RPCs exhaustively. We proposed to introduce a dedicated `sendall` RPC with the intention to cover this functionality. Since the proposal, it was discovered in further discussion that our proposed `sendall` rpc and SFFO have subtly different scopes of operation. • sendall: Use _given UTXOs_ to pay a destination the remainder after fees. • SFFO: Use a _given budget_ to pay an address the remainder after fees. While `sendall` will simplify cases of spending a given set of UTXOs such as paying the value from one or more specific UTXOs, emptying a wallet, or burning dust, we realized that there are some cases in which SFFO is used to pay other parties from a limited budget, which can often lead to the creation of change outputs. This cannot be easily replicated using `sendall` as it would require manual computation of the appropriate change amount. As such, sendall cannot replace all uses of SFFO, but it still has a different use case and will aid in simplifying some wallet calls and numerous wallet tests. _Sendall call details_ The proposed sendall call builds a transaction from a specific subset of the wallet's UTXO pool (by default all of them) and assigns the funds to one or more receivers. Receivers can either be specified with a given amount or receive an equal share of the remaining unassigned funds. At least one recipient must be provided without assigned amount to collect the remainder. The `sendall` call will never create change. The call has a `send_max` option that changes the default behavior of spending all UTXOs ("no UTXO left behind"), to maximizing the output amount of the transaction by skipping uneconomic UTXOs. The `send_max` option is incompatible with providing a specific set of inputs. --- Edit: Replaced OP with latest commit message to reflect my updated motivation of the proposal. ACKs for top commit: achow101: re-ACK bb84b7145b31dbfdcb4cf0b9b6e612a57e573993 Tree-SHA512: 20aaf75d268cb4b144f5d6437d33ec7b5f989256b3daeeb768ae1e7f39dc6b962af8223c5cb42ecc72dc38cecd921c53c077bc0ec300b994e902412213dd2cc3
2022-03-29Add sendall RPC née sweepMurch
_Motivation_ Currently, the wallet uses a fSubtractFeeAmount (SFFO) flag on the recipients objects for all forms of sending calls. According to the commit discussion, this flag was chiefly introduced to permit sweeping without manually calculating the fees of transactions. However, the flag leads to unintuitive behavior and makes it more complicated to test many wallet RPCs exhaustively. We proposed to introduce a dedicated `sendall` RPC with the intention to cover this functionality. Since the proposal, it was discovered in further discussion that our proposed `sendall` rpc and SFFO have subtly different scopes of operation. • sendall: Use _specific UTXOs_ to pay a destination the remainder after fees. • SFFO: Use a _specific budget_ to pay an address the remainder after fees. While `sendall` will simplify cases of spending from specific UTXOs, emptying a wallet, or burning dust, we realized that there are some cases in which SFFO is used to pay other parties from a limited budget, which can often lead to the creation of change outputs. This cannot be easily replicated using `sendall` as it would require manual computation of the appropriate change amount. As such, sendall cannot replace all uses of SFFO, but it still has a different use case and will aid in simplifying some wallet calls and numerous wallet tests. _Sendall call details_ The proposed sendall call builds a transaction from a specific subset of the wallet's UTXO pool (by default all of them) and assigns the funds to one or more receivers. Receivers can either be specified with a specific amount or receive an equal share of the remaining unassigned funds. At least one recipient must be provided without assigned amount to collect the remainder. The `sendall` call will never create change. The call has a `send_max` option that changes the default behavior of spending all UTXOs ("no UTXO left behind"), to maximizing the output amount of the transaction by skipping uneconomic UTXOs. The `send_max` option is incompatible with providing a specific set of inputs.
2022-03-28Merge bitcoin/bitcoin#23083: rpc: Fail to return undocumented or ↵fanquake
misdocumented JSON fc892c3a80091fbeaa2b5a6ec5bdaa31359b42de rpc: Fail to return undocumented or misdocumented JSON (MarcoFalke) f4bc4a705addea3e60c3b69437913e6571df275d rpc: Add m_skip_type_check to RPCResult (MarcoFalke) Pull request description: This avoids documentation shortcomings such as the ones fixed in commit e7b6272b305386a264adf2c04b7bebfb8499070f, 138d55e6a0241f126916fde6ac9177c7e2a119c4, 577bd51a4b8de066466a445192c1c653872657e2, f8c84e047c61200fae4cc1d85688e113bf270409, 0ee9a00f90e81a6978b30bdb250a37cbfa6da022, 13f41855c5fedf11d4a2525f2f0dd214533e5e62, or faecb2ee0a0ad3eb8303cfc84a87dc02ec553c43 ACKs for top commit: fanquake: ACK fc892c3a80091fbeaa2b5a6ec5bdaa31359b42de - tested that this catches issue, i.e #24691: Tree-SHA512: 9d0d7e6291bfc6f67541a4ff746d374ad8751fefcff6d103d8621c0298b190ab1d209ce96cfc3a0d4a6a5460a9f9bb790eb96027b16e5ff91f2512e40c92ca84
2022-03-25wallet: Prevent wallet unload on GetWalletForJSONRPCRequestJoão Barbosa
Don't extend shared ownership of all wallets to GetWalletForJSONRPCRequest scope.
2022-03-25Extract FinishTransaction from send()Murch
The final step of send either produces a PSBT or the final transaction. We extract these steps to a new helper function `FinishTransaction()` to reuse them in `sendall`.
2022-03-25Extract interpretation of fee estimation argumentsMurch
This will be reused in `sendall`, so we extract a method to prevent duplication.
2022-03-25Elaborate error messages for outdated optionsMurch
2022-03-25Extract prevention of outdated option namesMurch
This will be reused in `sendall` so we extract it to avoid duplication.
2022-03-24Merge bitcoin/bitcoin#24091: wallet: Consolidate CInputCoin and COutputfanquake
049003fe68a4183f6f20da16f58f10079d1e02df coinselection: Remove COutput operators == and != (Andrew Chow) f6c39c6adb6cbf9c87f04d3d667701905ef5c0a0 coinselection: Remove CInputCoin (Andrew Chow) 70f31f1a81710aa59e95770de9a84bf58cbce1e8 coinselection: Use COutput instead of CInputCoin (Andrew Chow) 14fbb57b79c664090f6a4e60d7bdfc9759ff4307 coinselection: Add effective value and fees to COutput (Andrew Chow) f0821230b8de2eec21a869d1edf9e2b9f502de25 moveonly: move COutput to coinselection.h (Andrew Chow) 42e974e15c6deba1d9395a4da9341c9ebec6e8e5 wallet: Remove CWallet and CWalletTx from COutput's constructor (Andrew Chow) 14d04d5ad15ae56df56edee7ca9a202b52037889 wallet: Replace CWalletTx in COutput with COutPoint and CTxOut (Andrew Chow) 0ba4d1916e26e2a5d603edcdb7625463989d25b6 wallet: Provide input bytes to COutput (Andrew Chow) d51f27d3bb0d6e3ca55bcd23ce53e4fe413a9360 wallet: Store whether a COutput is from the wallet (Andrew Chow) b799814bbd53736b79495072f3c9e05989a465e8 wallet: Store tx time in COutput (Andrew Chow) 46022953ee2e8113167bafd1fd48a383a578b13c wallet: Remove use_max_sig default value (Andrew Chow) 10379f007fd2c18f4cd24d0a0783d6d929f45556 scripted-diff: Rename COutput member variables (Andrew Chow) c7c64db41e1718584aa2f30ff27f60ab0966de62 wallet: cleanup COutput constructor (Andrew Chow) Pull request description: While working on coin selection code, it occurred to me that `CInputCoin` is really a subset of `COutput` and the conversion of a `COutput` to a `CInputCoin` does not appear to be all that useful. So this PR adds fields that are present in `CInputCoin` to `COutput` and replaces the usage of `CInputCoin` with `COutput`. `COutput` is also moved to coinselection.h. As part of this move, the usage of `CWalletTx` is removed from `COutput`. It is instead replaced by storing a `COutPoint` and the `CTxOut` rather than the entire `CWalletTx` as coin selection does not really need the full `CWalletTx`. The `CWalletTx` was only used for figuring out whether the transaction containing the output was from the current wallet, and for the transaction's time. These are now parameters to `COutput`'s constructor. ACKs for top commit: ryanofsky: Code review ACK 049003fe68a4183f6f20da16f58f10079d1e02df, just adding comments and removing == operators since last review w0xlt: reACK 049003f Xekyo: reACK 049003fe68a4183f6f20da16f58f10079d1e02df Tree-SHA512: 048b4cd620a0415e1d9fe8597257ee4bc64656566e1d28a9bdd147d6d72dc87c3f34a3339fa9ab6acf42c388df7901fc4ee900ccaabc3de790ffad162b544c15
2022-03-24rpc: Fail to return undocumented or misdocumented JSONMarcoFalke
2022-03-24Merge bitcoin/bitcoin#23732: refactor: Remove `gArgs` from `bdb.h` and ↵MarcoFalke
`sqlite.h` 39b1763730177cd7d6a32fd9321da640b0d65e0e Replace use of `ArgsManager` with `DatabaseOptions` (Kiminuo) Pull request description: Contributes to #21005. The goal of this PR is to remove `gArgs` from database classes (i.e. `bdb.h` and `sqlite.h`) so that they can be tested without relying on `gArgs` in tests. Notes: * My goal is to enable unit-testing without relying on `gArgs` as much as possible. Global variables are hard to reason about which in turn makes it slightly harder to contribute to this codebase. When the compiler does the heavy lifting for us and allows us only to construct an object (or call a method) with valid parameters, we may also save some time in code reviews. The cost for this is passing an argument which is not for free but the cost is very miniscule compared to benefits, I think. * GUI code is an exception because it seems fine to have `gArgs` there so I don't plan to make changes in `src/qt` folder, for example. * My approach to removal of `gArgs` uses is moving from lower levels to upper ones and pass `ArgsManager` as an argument as needed. The approach is very similar to what #20158. ACKs for top commit: achow101: ACK 39b1763730177cd7d6a32fd9321da640b0d65e0e ryanofsky: Code review ACK 39b1763730177cd7d6a32fd9321da640b0d65e0e. Just the two small ReadDatabaseArgs and Berkeley open changes that were discussed since the last review Tree-SHA512: aa066b314db593e46c18698fe8cdd500f558b405dc04e4a9a3ff57b52b5b3a81a6cb090e0e661785d1d02c1bf18958c1f4cd715ff233aab63381e3f80960622d
2022-03-23wallet: Replace CWalletTx in COutput with COutPoint and CTxOutAndrew Chow
Instead of having a pointer to the CWalletTx in COutput, we can just store the COutPoint and the CTxOut as those are the only things we need from the CWalletTx. Other things CWalletTx used to provide were time and fIsFromMe but these are also being stored by COutput.
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-19 09:55:29 +0000