Age | Commit message (Collapse) | Author |
|
Returns a std::optional<SelectionResult> from KnapsackSolver instead of
using out parameters for the inputs set and selected value.
|
|
Removes coins_out and value_ret has SelectCoinsBnB return a
std::optional<SelectionResult>
|
|
|
|
Replace the CoinSet actual_selection with a SelectionResult
expected_result. We don't use the SelectionResult functions yet, but
will soon.
-BEGIN VERIFY SCRIPT-
sed -i 's/CoinSet actual_selection/SelectionResult expected_result(CAmount(0))/' src/wallet/test/coinselector_tests.cpp
sed -i 's/actual_selection/expected_result.m_selected_inputs/' src/wallet/test/coinselector_tests.cpp
sed -i 's/expected_result.m_selected_inputs.clear/expected_result.Clear/' src/wallet/test/coinselector_tests.cpp
-END VERIFY SCRIPT-
|
|
Introduces a SelectionResult struct which contains the set of selected
inputs and the total transaction fee for the transaction. This will be
used by the various SelectCoins* functions. Additionally helpers are
provided to compute the total input value and result comparisons.
|
|
added address_type of `bech32m` to rpc calls `getnewaddress`/`getrawchangeaddress`
|
|
Wallet RPCs that allow a rescan based on block-timestamp or block-height
need to sync with the active chain first, because the user might assume
the wallet is up-to-date with the latest block they got reported via a
blockchain RPC.
|
|
11daf6ceb1d9ea1f8d638b123eecfe39d162a7c3 More Span simplifications (Pieter Wuille)
568dd2f83900a11a4dbba1250722791a135bf0a9 Replace MakeSpan helper with Span deduction guide (Pieter Wuille)
Pull request description:
C++17 supports [user-defined deduction guides](https://en.cppreference.com/w/cpp/language/class_template_argument_deduction), allowing class constructors to be invoked without specifying class template arguments. Instead, the code can contain rules to infer the template arguments from the constructor argument types.
This alleviates the need for the `MakeSpan` helper. Convert the existing MakeSpan rules into deduction rules for `Span` itself, and replace all invocations of `MakeSpan` with just `Span` ones.
ACKs for top commit:
MarcoFalke:
re-ACK 11daf6ceb1d9ea1f8d638b123eecfe39d162a7c3 Only change is removing a hunk in the tests 🌕
Tree-SHA512: 10f3e82e4338f39d9b7b407cd11aac7ebe1e9191b58e3d7f4e5e338a4636c0e126b4a1d912127c7446f57ba356c8d6544482e47f97901efea6a54fffbfd7895f
|
|
2c35a93b3cc19dc71d5664f9f61c24a04f419e35 Generalize/simplify VectorReader into SpanReader (Pieter Wuille)
Pull request description:
Originally written for #21590 (safegcd-based MuHash inverses), but then found a better way that removed the need for it, so I'm submitting it independently.
ACKs for top commit:
MarcoFalke:
re-ACK 2c35a93b3cc19dc71d5664f9f61c24a04f419e35 🖨
shaavan:
ACK 2c35a93b3cc19dc71d5664f9f61c24a04f419e35
Tree-SHA512: 959e3251e0cfe20e13a50639b617c9dc2a561d613a0884d983c93d15dacb6d2305d760aa933d18ba055cef8a1651a344bcb6b3f93051ecf26d3f2efc5779efa4
|
|
|
|
|
|
|
|
|
|
|
|
destinations to the address book
3d71d16d1eb4173c70d4c294559fc2365e189856 test: listtranscations with externally generated addresses (S3RK)
d04566415e16ae685af066384f346dff522c068f Add to spends only transcations from me (S3RK)
9f3a622b1cea37e452560f2f82d8e82d3b48a73a Automatically add labels to detected receiving addresses (S3RK)
c1b99c088c54eb101c0a28a67237965576ccf5ad Return used destinations from ScriptPubKeyMan::MarkUnusedAddresses (S3RK)
03840c20640685295a65ed8c82456e877f668b9b Add CWallet::IsInternalScriptPubKeyMan (S3RK)
456e350926adde5dabdbc85fc0f017fb29bdadb3 wallet: resolve ambiguity of two ScriptPubKey managers providing same script (S3RK)
Pull request description:
This PR fixes certain use-cases when **send-to-self** transactions are missing from `listtransactions` output.
1. When a receiving address is generated externally to the wallet
(e.g. same wallet running on two nodes, or by 3rd party from xpub)
2. When restoring backup with lost metadata, but keypool gap is not exceeded yet
When the block is connected or tx added to mempool we already mark used keys. This PR extends this logic to determine whether the destination is a receiving one and if yes add it to the address book with empty label.
Works both for legacy and descriptors wallets.
- For legacy it uses the internal flag from the keypool entry. Caveat: because we don't know which script type would be used we add all possible destinations for such keys.
- For descriptor wallets it uses internal flag for the script pub key manager. Caveat: it only works for active descriptors.
fixes #19856
fixes #20293
ACKs for top commit:
laanwj:
Code review ACK 3d71d16d1eb4173c70d4c294559fc2365e189856
Tree-SHA512: 03fafd5548ead0c4ffe9ebcc9eb2849f1d2fa7270fda4166419b86877d4e57dcf04460e465fbb9c90b42031f3c05d1b83f1b67a9f82c2a42980825ed1e7b52e6
|
|
to wallet/rpc/util
ff945e553affbb7e25da1257a0dd47e413ec5164 MOVEONLY: Move utility functions from rpcwallet to wallet/rpc/util (Samuel Dobson)
7b04a064f6e8ee9d93e5a5ad54dab20b769083f2 Introduce wallet/rpc/util (Samuel Dobson)
Pull request description:
This is part one of multiple to split up rpcwallet.cpp into smaller, more logical units.
See #23622 for context and overall plan. I'll open PRs in stages to hopefully minimise conflicts.
Can be reviewed with `--color-moved=dimmed-zebra`
The end goal can be seen here: https://github.com/meshcollider/bitcoin/tree/202111_split_walletrpc
ACKs for top commit:
MarcoFalke:
nice, ACK ff945e553affbb7e25da1257a0dd47e413ec5164 🐰
shaavan:
ACK ff945e553affbb7e25da1257a0dd47e413ec5164
Tree-SHA512: 6e3d1de6db770fe2fca540c8c4f30183ab8258c26e3a1fb46937714d28818a52933eafbfcafe2995f6a6e2551a3f3dd3ec93363b81de7912c0d81f5749d1c86d
|
|
|
|
c771ee8571dc63d27747ef565671eb1dcdc0ff4b doc: use BIP125-replaceable (fanquake)
36dc5bb8cb1d71356d645338b5b49361f3bda956 doc: Extract CreateTxDoc in rawtransaction (fanquake)
Pull request description:
For the fields: inputs, outputs, locktime, replaceable. Similar to #23172.
Can be reviewed with `--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space`.
ACKs for top commit:
MarcoFalke:
ACK c771ee8571dc63d27747ef565671eb1dcdc0ff4b 😸
Tree-SHA512: e6e4211b89bedec472f8381b3cbea5670f82b728955888c794f694164b8d8bdd51a99c64762b625357ac2171005712b82f81ee7c1b8f5c5620bdedeeefa2b9da
|
|
|
|
named arguments (tests only)
fa00447442f22a24e5ca5fc538d0bf7bef575544 scripted-diff: Use clang-tidy syntax for C++ named arguments (MarcoFalke)
fae13c39896898aef2281433af143c22d8b3a3b4 doc: Use clang-tidy comments in crypto_tests (MarcoFalke)
Pull request description:
Incorrect named args are source of bugs, like #22979.
To allow them being checked by `clang-tidy`, use a format it can understand.
ACKs for top commit:
shaavan:
ACK fa00447442f22a24e5ca5fc538d0bf7bef575544
rajarshimaitra:
ACK https://github.com/bitcoin/bitcoin/pull/23546/commits/fa00447442f22a24e5ca5fc538d0bf7bef575544
jonatack:
ACK fa00447442f22a24e5ca5fc538d0bf7bef575544
fanquake:
ACK fa00447442f22a24e5ca5fc538d0bf7bef575544
Tree-SHA512: 4d23a8363da81dfea21a4cd8516ab5e0dc70119e4d503f3f240f38573218b2c2e84083b97e956c62942d78b2f17490f8b3b2e8077d257644fda1d901e2b80507
|
|
|
|
|
|
fae239208d3676452a755f736ee5aaa17adeb493 wallet: Split signmessage from rpcwallet (MarcoFalke)
Pull request description:
rpcwallet is the file that takes longest to compile, especially with sanitizers enabled it can take several 10s of seconds.
Allow faster incremental and parallel builds by starting to split it up. First, split off `signmessage`, which is unrelated to other stuff such as wallet file handling, wallet encryption, tx creation, or wallet status/info.
ACKs for top commit:
ryanofsky:
Code review ACK fae239208d3676452a755f736ee5aaa17adeb493. Confirmed move only
meshcollider:
Code review ACK fae239208d3676452a755f736ee5aaa17adeb493
Tree-SHA512: 250445cd544e39376f225871270cdcae462f16cfd9d25ede4b148e915642bfac9ee7ef3e8eccdd2443dc74dbf794d3bcd5fe5c58b1d05a2dcec70b8e03b37dff
|
|
Based on suggestions by MarcoFalke <falke.marco@gmail.com>
|
|
|
|
finalizing
a99ed8986554fa1ecc854e43ea373d957e598db8 psbt: sign without finalizing (Andrew Chow)
Pull request description:
It can be useful to sign an input with `walletprocesspsbt` but not finalize that input if it is complete. This PR adds another option to `walletprocesspsbt` to be able to do that. We will still finalize by default.
This does not materially change the PSBT workflow since `finalizepsbt` needs to be called in order to extract the tx for broadcast.
ACKs for top commit:
meshcollider:
utACK a99ed8986554fa1ecc854e43ea373d957e598db8
Sjors:
utACK a99ed89
Tree-SHA512: c88e5d3222109c5f4e763b1b9d97ce4655f68f2985a4509caab2d4e7f5bac5047328fd69696e82a330f5c5a333e0312568ae293515689b77a4747ca2f17caca6
|
|
isminefilter
fa2c991ec93bc72d276f0dcd747b3e57c246139b refactor: Use underlying type of isminetype for isminefilter (MarcoFalke)
Pull request description:
This does not change behavior, but it would be good for code clarity and to avoid `-Wimplicit-int-conversion` compiler warnings to use the an int of the same width for both `isminetype` and `isminefilter`.
ACKs for top commit:
laanwj:
Code review ACK fa2c991ec93bc72d276f0dcd747b3e57c246139b
shaavan:
crACK fa2c991ec93bc72d276f0dcd747b3e57c246139b
promag:
Code review ACK fa2c991ec93bc72d276f0dcd747b3e57c246139b.
Tree-SHA512: b3e255de7c9b1dea272bc8cb9386b339fe701f18580e03e997c270cac6453088ca2032e26e39f536d66cd1b6fda3e96bdbdc6e960879030e635338d0916277e6
|
|
f13a22a631efe01e1fbae4ae78a4901d14ebda3c wallet: Call load handlers without cs_wallet locked (João Barbosa)
Pull request description:
Don't have `cs_wallet` locked while calling each `context.wallet_load_fns`. A load handler can always lock `cs_wallet` if needed.
The lock was added in 1c7e25db0c to satisfy TSAN. With 44c430ffac most of the code requiring the lock is in `CWallet::AttachChain`. A comment is added to warn about wallets_mutex and cs_wallet lock ordering.
ACKs for top commit:
meshcollider:
re-utACK f13a22a631efe01e1fbae4ae78a4901d14ebda3c
ryanofsky:
Code review ACK f13a22a631efe01e1fbae4ae78a4901d14ebda3c. Only change since last review is adding a lock order comment
jonatack:
ACK f13a22a631efe01e1fbae4ae78a4901d14ebda3c
Tree-SHA512: d51976c3aae4bebc2d1997c88edff712d21fc5523801f5614062a10f826e164579973aeb1981bb1cbc243ecff6af3250362f544c02a79e5d135cbbca1704be62
|
|
Can be reviewed with --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
|
|
d8ee8f3cd32bbfefec931724f5798cbb088ceb6f refactor: Make CWalletTx sync state type-safe (Russell Yanofsky)
Pull request description:
Current `CWalletTx` state representation makes it possible to set inconsistent states that won't be handled correctly by wallet sync code or serialized & deserialized back into the same form.
For example, it is possible to call `setConflicted` without setting a conflicting block hash, or `setConfirmed` with no transaction index. And it's possible update individual `m_confirm` and `fInMempool` data fields without setting an overall consistent state that can be serialized and handled correctly.
Fix this without changing behavior by using `std::variant`, instead of an enum and collection of fields, to represent sync state, so state tracking code is safer and more legible.
This is a first step to fixing state tracking bugs https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Transaction-Conflict-Tracking, by adding an extra margin of safety that can prevent new bugs from being introduced as existing bugs are fixed.
ACKs for top commit:
laanwj:
re-ACK d8ee8f3cd32bbfefec931724f5798cbb088ceb6f
jonatack:
Code review ACK d8ee8f3cd32bbfefec931724f5798cbb088ceb6f
Tree-SHA512: b9f15e9d99dbdbdd3ef7a76764e11f66949f50e6227e284126f209e4cb106af6d55e9a9e8c7d4aa216ddc92c6d5acc6f4aa4746f209bbd77f03831b51a2841c3
|
|
|
|
fa3e0da06b491b8c0fa2dbae37682a9112c9deb8 policy: Treat taproot as always active (MarcoFalke)
Pull request description:
Now that taproot is active, it can be treated as if it was always active for policy for the next major release. This simplifies the code and changes two things:
* Importing `tr` descriptors can be done before the chain is fully synced. This is fine, because the wallet will already generate `tr` descriptors by default (regardless of the taproot status) after commit 47fe7445e7f54aee10ec6dbc53f1db1adbeb43de.
* Valid taproot spends won't be rejected from the mempool before taproot is active. This is strictly speaking a bugfix after commit 47fe7445e7f54aee10ec6dbc53f1db1adbeb43de, since the wallet may generate taproot spends before the chain is fully synced. For example, a slow node or a purposefully offline node. Currently, the wallet needs the mempool to account for change. See https://github.com/bitcoin/bitcoin/issues/11887.
A similar change was done for segwit v0 in https://github.com/bitcoin/bitcoin/pull/13120 .
This effectively reverts commit c5ec0367d718544caa3a1578d6c730fc92ee4e94.
ACKs for top commit:
mjdietzx:
Code Review ACK fa3e0da06b491b8c0fa2dbae37682a9112c9deb8
achow101:
ACK fa3e0da06b491b8c0fa2dbae37682a9112c9deb8
sipa:
utACK fa3e0da06b491b8c0fa2dbae37682a9112c9deb8
gruve-p:
ACK https://github.com/bitcoin/bitcoin/pull/23512/commits/fa3e0da06b491b8c0fa2dbae37682a9112c9deb8
gunar:
Code Review + tACK fa3e0da06
rajarshimaitra:
code review + tACK https://github.com/bitcoin/bitcoin/pull/23512/commits/fa3e0da06b491b8c0fa2dbae37682a9112c9deb8
Tree-SHA512: c6dc7a4e6c345bdec33f256847dc63906ab1696aa683ab9b32a79e715613950884ac3a1a7a44e95f31bb28e58dd64679a616175f7e152b21f5550f3337c8e622
|
|
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
|
|
4868c9f1b39f03adee0009cd41d96598b43e8b78 Extract Taproot internal keyid with GetKeyFromDestination (Andrew Chow)
d8abbe119c71f917e0fd2e80536c1e5d979b4dc6 Mention bech32m in -addresstype and -changetype help (Andrew Chow)
8fb57845ee3844c9ba854471065109d2e409300f Create a tr() descriptor bech32m DescriptorScriptPubKeyMan by default (Andrew Chow)
54b3699862de687f782c7c52500d6a2372478355 Store pubkeys in TRDescriptor::MakeScripts (Andrew Chow)
Pull request description:
Make a `tr()` descriptor by default in descriptor wallets so that users will be able to make and use segwit v1 bech32m addresses.
ACKs for top commit:
MarcoFalke:
Concept ACK 4868c9f1b39f03adee0009cd41d96598b43e8b78
Sjors:
re-utACK 4868c9f1b39f03adee0009cd41d96598b43e8b78
gruve-p:
ACK https://github.com/bitcoin/bitcoin/pull/22364/commits/4868c9f1b39f03adee0009cd41d96598b43e8b78
meshcollider:
Concept + code review ACK 4868c9f1b39f03adee0009cd41d96598b43e8b78
Tree-SHA512: e5896e665b8d559f1d759b6582d1bb24f70d4698a57307684339d9fdcdac28ae9bc17bc946a7efec9cb35c130a95ffc36e3961a335124ec4535d77b8d00e9631
|
|
blank descriptor wallets
ee03c782ba61993d9e95fa499546cd14cee35445 wallet: Make GetOldestKeyPoolTime return nullopt for blank wallets (Hennadii Stepanov)
3e4f069d23cd2ea5de8fa3c4b1a761ab097ad56f wallet, refactor: Make GetOldestKeyPoolTime return type std::optional (Hennadii Stepanov)
Pull request description:
The "keypoololdest" field in the `getwalletinfo` RPC response should be used for legacy wallets only.
Th current implementation (04437ee721e66a7b76bef5ec2f88dd1efcd03b84) assumes that `CWallet::GetOldestKeyPoolTime()` always return `0` for descriptor wallets. This assumption is wrong for _blank_ descriptor wallets, when `m_spk_managers` is empty. As a result:
```
$ src/bitcoin-cli -signet -rpcwallet=211024-d-DPK getwalletinfo
{
"walletname": "211024-d-DPK",
"walletversion": 169900,
"format": "sqlite",
"balance": 0.00000000,
"unconfirmed_balance": 0.00000000,
"immature_balance": 0.00000000,
"txcount": 0,
"keypoololdest": 9223372036854775807,
"keypoolsize": 0,
"keypoolsize_hd_internal": 0,
"paytxfee": 0.00000000,
"private_keys_enabled": false,
"avoid_reuse": false,
"scanning": false,
"descriptors": true
}
```
This PR fixes this issue with direct checking of the `WALLET_FLAG_DESCRIPTORS` flag.
ACKs for top commit:
lsilva01:
re-ACK ee03c78
stratospher:
ACK ee03c78.
meshcollider:
Code review ACK ee03c782ba61993d9e95fa499546cd14cee35445
Tree-SHA512: 9852f9f8ed5c08c07507274d7714f039bbfda66da6df65cf98f67bf11a600167d0f7f872680c95775399477f4df9ba9fce80ec0cbe0adb7f2bb33c3bd65b15df
|
|
-BEGIN VERIFY SCRIPT-
perl -0777 -pi -e 's:((\(|\{|,)(\n| )*)\/\* ?([^=* ]+) ?\*\/ ?:\1/*\4=*/:g' $( git ls-files ./src/test ./src/wallet/test )
-END VERIFY SCRIPT-
|
|
|
|
|
|
|
|
|
|
with bitcoin-wallet
5b6b5ef5d178bea3c1b2f21e3bd05186312ea191 util: Use FEATURE_LATEST for wallets created with bitcoin-wallet (Hennadii Stepanov)
Pull request description:
Since the 49d2374acf5845c5f760b5fd241482f292164147 commit was athored by **jonasschnelli** in 2016, the wallet version was bumped twice: in 2017 (bitcoin/bitcoin#11250) and in 2018 (bitcoin/bitcoin#12560).
This PR bumps the version of wallets created with `bitcoin-wallet` offline tool.
On master (04437ee721e66a7b76bef5ec2f88dd1efcd03b84) -- `"walletversion": 139900`:
```
$ src/bitcoin-wallet -signet -wallet=211025-test-master create
Topping up keypool...
Wallet info
===========
Name: 211025-test-master
Format: sqlite
Descriptors: yes
Encrypted: no
HD (hd seed available): yes
Keypool Size: 6000
Transactions: 0
Address Book: 0
$ src/bitcoin-cli -signet -rpcwallet=211025-test-master getwalletinfo
{
"walletname": "211025-test-master",
"walletversion": 139900,
"format": "sqlite",
"balance": 0.00000000,
"unconfirmed_balance": 0.00000000,
"immature_balance": 0.00000000,
"txcount": 0,
"keypoolsize": 3000,
"keypoolsize_hd_internal": 3000,
"paytxfee": 0.00000000,
"private_keys_enabled": true,
"avoid_reuse": false,
"scanning": false,
"descriptors": true
}
```
With this PR -- `"walletversion": 169900`:
```
$ src/bitcoin-wallet -signet -wallet=211025-test-pr create
Topping up keypool...
Wallet info
===========
Name: 211025-test-pr
Format: sqlite
Descriptors: yes
Encrypted: no
HD (hd seed available): yes
Keypool Size: 6000
Transactions: 0
Address Book: 0
$ src/bitcoin-cli -signet -rpcwallet=211025-test-pr getwalletinfo
{
"walletname": "211025-test-pr",
"walletversion": 169900,
"format": "sqlite",
"balance": 0.00000000,
"unconfirmed_balance": 0.00000000,
"immature_balance": 0.00000000,
"txcount": 0,
"keypoolsize": 3000,
"keypoolsize_hd_internal": 3000,
"paytxfee": 0.00000000,
"private_keys_enabled": true,
"avoid_reuse": false,
"scanning": false,
"descriptors": true
}
```
ACKs for top commit:
lsilva01:
Code Review ACK 5b6b5ef
stratospher:
ACK 5b6b5ef.
rajarshimaitra:
ACK https://github.com/bitcoin/bitcoin/pull/23349/commits/5b6b5ef5d178bea3c1b2f21e3bd05186312ea191
meshcollider:
Code review ACK 5b6b5ef5d178bea3c1b2f21e3bd05186312ea191
Tree-SHA512: 0221e76fa8f29037920d0a483c742bf270ecaead45f30230943b78775aaea63ac052e43fe712d15c2326e515dea2d2ac82de0924882598421c1874f2e6f442a6
|
|
Current CWalletTx state representation makes it possible to set
inconsistent states that won't be handled correctly by wallet sync code
or serialized & deserialized back into the same form.
For example, it is possible to call setConflicted without setting a
conflicting block hash, or setConfirmed with no transaction index. And
it's possible update individual m_confirm and fInMempool data fields
without setting an overall consistent state that can be serialized and
handled correctly.
Fix this without changing behavior by using std::variant, instead of an
enum and collection of fields, to represent sync state, so state
tracking code is safer and more legible.
This is a first step to fixing state tracking bugs
https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Transaction-Conflict-Tracking,
by adding an extra margin of safety that can prevent new bugs from being
introduced as existing bugs are fixed.
|
|
|
|
`wallet.cpp` (2)
2ec38bdebbdfd3f932eaa85c98b617d7b9326399 Remove `gArgs` from `wallet.h` and `wallet.cpp` (Kiminuo)
Pull request description:
This is a follow-up PR to #22183 and is related to #21005 issue.
ACKs for top commit:
ryanofsky:
Code review ACK 2ec38bdebbdfd3f932eaa85c98b617d7b9326399. No changes since last review, just rebase
Tree-SHA512: ae7fa1927b0a268f25697928ccaf1b3cf10ee1ccef3f9d2344001fbd7e11fe8ce768745c65e76bd7d1632c6c7650612b5b54eaf2be61093854f75a4c4dcb1784
|
|
|
|
dump{privkey,wallet} RPCs
|
|
This also enables working with a const ScriptPubKeyMan which was
previously not possible due to std::map::operator[] not being const.
|
|
bd9c6ade46c8c29bea3fd5df62a35efadb77ac89 wallet: Fixed Grammatical error in bdb.h (zealsham)
Pull request description:
A comment in bdb.h file in the wallet directory contains a grammatical error that makes the underlying code harder to reason about .
The comment which says `/** Indicate the a new database user has began using the database. */` should actually be
`/** indicate that a new database user has began using the database. */`. The former is quite confusing , and leaves you wondering what "the a new database user " is refering to . This pull request thus provides value to the bitcoin codebase by improving readability .
Top commit has no ACKs.
Tree-SHA512: db07cda39f89ac344be3497c884a8c6e4b48276afae18e931a9a8e5732c58eed20645ccd18d6b1212c763f64b1300477c1de26a00b5b3b24e6141ffae3658ca7
|
|
Wallet: Fixed Grammatical error in bdb.h
|
|
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
|