Since the only remaining isminetypes are ISMINE_NO and ISMINE_SPENDABLE,
this enum is now just a bool and can be removed. IsMine is changed to
return a bool and any usage of isminetypes and isminefilters are changed
to be the remaining ISMINE_SPENDABLE case.
This isminetype is not a real isminetype as it is never returned by
IsMine. This is only used for isminefilters in one function, which can
be better represented with a bool parameter avoid_reuse.
In descriptor wallets, we consider all outputs to be spendable as we no
longer have mixed mine and watchonly in a wallet. As such,
COutput::spendable is meaningless and can be removed.
Furthermore, CoinFilterParams::only_spendable can be removed as that was
essentially checking for COutput::spendable.
Lastly, AvailableCoinsListUnspent can also be removed as the wrapper is
now only setting the feerate to std::nullopt which is trivial enough that
a dedicated wrapper is not needed.
5c8bf7b39e9bffba7c4d5778b56b1ebe72f5ea1b doc: add release notes for version 3 transactions (ishaanam)
4ef8065a5e3d2fe9cd3d7a71224ef2ca2e7b495a test: add truc wallet tests (ishaanam)
5d932e14dbe41c349ab41f88088398e0ab10d335 test: extract `bulk_vout` from `bulk_tx` so it can be used by wallet tests (ishaanam)
2cb473d9f2152e33bab2c3c626801deb7841aa20 rpc: Support version 3 transaction creation (Bue-von-hon)
4c20343b4d318be62086676e0898e56221500de1 rpc: Add transaction min standard version parameter (Bue-von-hon)
c5a2d080116270ecd0414c14eb412fa30eaaedaf wallet: don't return utxos from multiple truc txs in AvailableCoins (ishaanam)
da8748ad626fc5813eb06244630e12c8ceb3cedf wallet: limit v3 tx weight in coin selection (ishaanam)
85c54106156f5bbac87f4442a0a27f1b9187125b wallet: mark unconfirmed v3 siblings as mempool conflicts (ishaanam)
0804fc3cb11089000d3b0e8bed41df0b0bf5fff1 wallet: throw error at conflicting tx versions in pre-selected inputs (ishaanam)
cc155226fee1f5c9a40ec37f6276e45c9c42b26a wallet: set m_version in coin control to default value (ishaanam)
2e9617664e70b5e586c485e7c65ce342ffd66cdf wallet: don't include unconfirmed v3 txs with children in available coins (ishaanam)
ec2676becdf488f7a1151345a019c05dec926308 wallet: unconfirmed ancestors and descendants are always truc (ishaanam)
Pull request description:
This PR Implements the following:
- If creating a v3 transaction, `AvailableCoins` doesn't return unconfirmed v2 utxos (and vice versa)
- `AvailableCoins` doesn't return an unconfirmed v3 utxo if its transaction already has a child
- If a v3 transaction is kicked out of the mempool by a sibling, mark the sibling as a mempool conflict
- Throw an error if pre-selected inputs are of the wrong transaction version
- Allow setting version to 3 manually in `createrawtransaction` (uses commits from #31936)
- Limits a v3 transaction weight in coin selection
Closes#31348
To-Do:
- [x] Test a v3 sibling conflict kicking out one of our transactions from the mempool
- [x] Implement separate size limit for TRUC children
- [x] Test that we can't fund a v2 transaction when everything is v3 unconfirmed
- [x] Test a v3 sibling conflict being removed from the mempool
- [x] Test limiting v3 transaction weight in coin selection
- [x] Simplify tests
- [x] Add documentation
- [x] Test that user-input max weight is not overwritten by truc max weight
- [x] Test v3 in RPCs other than `createrawtransaction`
ACKs for top commit:
glozow:
reACK 5c8bf7b39e9
achow101:
ACK 5c8bf7b39e9bffba7c4d5778b56b1ebe72f5ea1b
rkrux:
ACK 5c8bf7b39e9bffba7c4d5778b56b1ebe72f5ea1b
Tree-SHA512: da8aea51c113e193dd0b442eff765bd6b8dc0e5066272d3e52190a223c903f48788795f32c554f268af0d2607b5b8c3985c648879cb176c65540837c05d0abb5
60d1042b9a4db8daf9fffdc29053652e99b7126e wallet: Remove unused `WalletFeature` enums (woltx)
66de58208a713e16f0d48bceed4d7496eae4b05b wallet: Remove `CWallet::nWalletVersion` and related functions (woltx)
7cda3d0f5bdca64b11f966a60167cde5451071a3 wallet: Remove `IsFeatureSupported()` and `CanSupportFeature()` (woltx)
ba0158522981287f2fde83f38392baac0216b0b4 wallet: `MigrateToDescriptor` no longer calls `CanSupportFeature` (woltx)
63acee279756e72f96fda14a9963281860bf318b wallet: Remove `GetClosestWalletFeature()` (woltx)
e27da3150b48ccf106ba93044bd28c6d1f505421 wallet: Remove `GetVersion()` (woltx)
Pull request description:
This PR incorporates the suggestion provided by PRabahy and pablomartin4btc in https://github.com/bitcoin/bitcoin/pull/32944 of removing `CWallet::nWalletVersion` and several related functions, such as `SetMinVersion()`, `GetVersion()`, `GetClosestWalletFeature()`, `IsFeatureSupported()`, `CanSupportFeature()`, etc ...
This field is no longer used in the descriptor wallet and there is still a lot of code related to it, so the changes here provide a good cleanup in the wallet code.
Built on top of https://github.com/bitcoin/bitcoin/pull/32944
ACKs for top commit:
maflcko:
review ACK 60d1042b9a4db8daf9fffdc29053652e99b7126e 🐾
achow101:
ACK 60d1042b9a4db8daf9fffdc29053652e99b7126e
pablomartin4btc:
ACK 60d1042b9a4db8daf9fffdc29053652e99b7126e
Tree-SHA512: 1a7ad8e15d57df8f66545776e7d178a2cd5312c87769a29770588375e3de5f24247aab9919acf004ed3eca16d08ba595b5f1c7b2b3eef7752e89d9c295624583
Moves the file from `src/util` to `src/primitives`. Now that the
refactor is complete, Txid and Wtxid are fundamental types, so it
makes sense for them to reside in `src/primitives`.
aac0b6dd79b0db1e9d42a6f466709a61cfd1f69f test: test sendall and send do anti-fee-sniping (ishaanam)
20802c7b65f4b196a3ed0aca876974153db55d9d wallet, rpc: add anti-fee-sniping to `send` and `sendall` (ishaanam)
Pull request description:
Currently, `send` and `sendall` don't do anti-fee-sniping because they don't use `CreateTransaction`. This PR adds anti-fee-sniping to these RPCs by calling `DiscourageFeeSniping` from `FinishTransaction` when the user does not specify a locktime.
ACKs for top commit:
achow101:
ACK aac0b6dd79b0db1e9d42a6f466709a61cfd1f69f
murchandamus:
ACK aac0b6dd79b0db1e9d42a6f466709a61cfd1f69f
glozow:
ACK aac0b6dd79b0db1e9d42a6f466709a61cfd1f69f
Tree-SHA512: d4f1b43b5bda489bdba46b0af60e50bff0de604a35670e6ea6e1de2b539f16b3f68805492f51d6d2078d421b63432ca22a561a5721d1a37686f2e48284e1e646
251d02084688c67523e9ec92ec79ee657454ab93 init, wallet: replace hardcoded output types with `FormatAllOutputTypes` (Sebastian Falbesoner)
e3ba0757a9410336e904a1b108d86165f347fc52 rpc, wallet: replace remaining hardcoded output types with `FormatAllOutputTypes` (Sebastian Falbesoner)
Pull request description:
This PR takes use of the `FormatAllOutputTypes` helper (introduced in PR #32432, commit 8cc9845b8ddf4f93a02c622e7df8d1095dc1a640) to get rid of the remaining hardcoded output types in wallet RPC and command line arguments documentation [1]. Note that it can't be used in the [`createmultisig` RPC](fc162299f0/src/rpc/output_script.cpp (L100)), as this one is only for pre-taproot output types and hence doesn't contain "bech32m" in the list.
[1] instances were found via `$ git grep legacy.*p2sh-segwit ./src/rpc/ ./src/wallet/`
ACKs for top commit:
nervana21:
tACK [251d020](251d020846)
maflcko:
review ACK 251d02084688c67523e9ec92ec79ee657454ab93 🌨
pablomartin4btc:
re-utACK 251d02084688c67523e9ec92ec79ee657454ab93
rkrux:
crACK 251d02084688c67523e9ec92ec79ee657454ab93
Tree-SHA512: 23d1025d068f3a44b115a34b217b808fcae59fc574e35a899f0d43a85512935c90675d2e98c621287e02adc3a9f4a08289a26596c66e2122262af0cd2dfbde72
c5c1960f9350d6315cadbdc95fface5f85f25806 doc: Add release notes for changes in RPCs (pablomartin4btc)
90fd5acbe57edb219a5dcbdc1095e11ae5398da5 rpc, test: Fix error message in getdescriptoractivity (pablomartin4btc)
39fef1d203678291020aa1adb2e420a117f86169 test: Add missing logging info for each test (pablomartin4btc)
53ac704efd668f7d4ad74158e628023c9a34141f rpc, test: Fix error message in unloadwallet (pablomartin4btc)
1fc3a8e8e7ae4698ac5cd5292a7e7e37097d37ce rpc, test: Add EnsureUniqueWalletName tests (pablomartin4btc)
b635bc0896294af5afa1b18a35f307dfae441bb8 rpc, util: Add EnsureUniqueWalletName (pablomartin4btc)
Pull request description:
Currently, `unloadwallet` RPC call fails with a JSON parsing error when no `wallet_name` argument is provided. This behavior is misleading because the error originates from a low-level JSON type mismatch, rather than clearly indicating that the wallet name or RPC endpoint (`-rpcwallet=...`) is missing. Also, found out that the [issue](https://github.com/bitcoin/bitcoin/pull/13111#issuecomment-398831543) was noticed during its implementation but never addressed.
In addition, I've verified all RPC commands calls finding that `getdescriptoractivity` had the same problem, but related to the array input types (blockhashes & descriptors), so I've corrected that RPC as well. For consistency I've added the missing logging info for each test case in `test/functional/rpc_getdescriptoractivity.py` in preparation for the new test.
**_-Before_**
```
./build/bin/bitcoin-cli -regtest -datadir=/tmp/btc unloadwallet
error code: -3
error message:
JSON value of type number is not of expected type string
```
```
./build/bin/bitcoin-cli -regtest -datadir=/tmp/btc getdescriptoractivity
error code: -3
error message:
JSON value of type null is not of expected type array
```
```
./build/bin/bitcoin-cli -regtest -datadir=/tmp/btc getdescriptoractivity '[]'
error code: -3
error message:
JSON value of type null is not of expected type array
```
**_-After_**
```
./build/bin/bitcoin-cli -regtest -datadir=/tmp/btc unloadwallet
error code: -8
error message:
Either the RPC endpoint wallet or the wallet name parameter must be provided
```
```
./build/bin/bitcoin-cli -regtest -datadir=/tmp/btc getdescriptoractivity
error code: -1
error message:
getdescriptoractivity ["blockhash",...] [scanobjects,...] ( include_mempool )
Get spend and receive activity associated with a set of descriptors for a set of blocks. This command pairs well with the `relevant_blocks` output of `scanblocks()`.
This call may take several minutes. If you encounter timeouts, try specifying no RPC timeout (bitcoin-cli -rpcclienttimeout=0)
Arguments:
1. blockhashes (json array, required) The list of blockhashes to examine for activity. Order doesn't matter. Must be along main chain or an error is thrown.
[
"blockhash", (string) A valid blockhash
...
]
2. scanobjects (json array, required) Array of scan objects. Every scan object is either a string descriptor or an object:
[
"descriptor", (string) An output descriptor
{ (json object) An object with output descriptor and metadata
"desc": "str", (string, required) An output descriptor
"range": n or [n,n], (numeric or array, optional, default=1000) The range of HD chain indexes to explore (either end or [begin,end])
},
...
]
3. include_mempool (boolean, optional, default=true) Whether to include unconfirmed activity
...
```
```
./build/bin/bitcoin-cli -regtest -datadir=/tmp/btc getdescriptoractivity '[]'
error code: -1
error message:
getdescriptoractivity ["blockhash",...] [scanobjects,...] ( include_mempool )
...
```
ACKs for top commit:
achow101:
ACK c5c1960f9350d6315cadbdc95fface5f85f25806
stickies-v:
re-ACK c5c1960f9350d6315cadbdc95fface5f85f25806
furszy:
ACK c5c1960f9350d6315cadbdc95fface5f85f25806
Tree-SHA512: e831ff1acbfd15d2ce3a69bb408cce94664c0b63b2aa2f4627a05c6c052241ae3b5cc238219ef1b30afb489a4a3f4c3030e2168b0c8f08b4d20805d050d810f5
This commit takes use of the `FormatAllOutputTypes` helper
(introduced in PR #32432, commit 8cc9845b8ddf4f93a02c622e7df8d1095dc1a640)
to get rid of the hardcoded output types in wallet RPC documentation.
Note that it can't be used in the `createmultisig` RPC, as this one is
only for pre-taproot output types and hence doesn't contain "bech32m" in the list.
6135e0553e6e58fcf506700991fa178f2c50a266 wallet, rpc: Move (Un)LockCoin WalletBatch creation out of RPC (Ava Chow)
Pull request description:
If the locked coin needs to be persisted to the wallet database, insteead of having the RPC figure out when to create a WalletBatch and having LockCoin's behavior depend on it, have LockCoin take whether to persist as a parameter so it makes the batch.
Since unlocking a persisted locked coin requires a database write as well, we need to track whether the locked coin was persisted to the wallet database so that it can erase the locked coin when necessary.
Keeping track of whether a locked coin was persisted is also useful information for future PRs.
Split from #32489
ACKs for top commit:
rkrux:
ACK 6135e05
Sjors:
ACK 6135e0553e6e58fcf506700991fa178f2c50a266
w0xlt:
ACK 6135e0553e
Tree-SHA512: 0e2367fc4d50c62ec41443374b64c4c5ecf679998677df47fb8776cfb44704713bc45547e32e96cd30d1dbed766f5d333efb6f10eb0e71271606638e07e61a01
The unloadwallet RPC previously failed with a low-level JSON parsing error
when called without any arguments (wallet_name).
Although this issue was first identified during review of the original unloadwallet
implementation in #13111, it was never addressed.
Raise RPC_INVALID_PARAMETER instead describing that either the RPC endpoint or wallet
name must be provided.
Adding a new functional test for this use case.
Refactor migratewallet to use the same logic as the wallet_name argument handling
is identical.
Co-authored-by: maflcko <maflcko@users.noreply.github.com>
Add a new function called EnsureUniqueWalletNamet that returns the
selected wallet name across the RPC request endpoint and wallet_name.
Supports the case where the wallet_name argument may be omitted—either
when using a wallet endpoint, or when not provided at all. In the latter
case, if no wallet endpoint is used, an error is raised.
Internally reuses the existing implementation to avoid redundant URL
decoding and logic duplication.
This is a preparatory change for upcoming refactoring of unloadwallet
and migratewallet, which will adopt EnsureUniqueWalletName for improved
clarity and consistency.
fcfd3db563e89fd79820a4cdfa102d624d801de1 remove RPCTimerInterface and RPCRunLater (Matthew Zipkin)
8a1765795fd3bff79d790102ca7cefa8fd9b204c use WalletContext scheduler for walletpassphrase callback (Matthew Zipkin)
Pull request description:
This removes the dependency on libevent for events scheduled by RPC commands, like re-locking a wallet some time after decryption with walletpassphrase. Since walletpassphrase is currently the only RPC that does this, `RPCRunLater`, `RPCTimerInterface` and all related methods are left unused, and deleted in the second commit. Any future RPC that needs to execute a callback in the future can follow the pattern in this PR and just use a scheduler from node or wallet context.
This is an alternative approach to #32796, described in https://github.com/bitcoin/bitcoin/pull/32796#issuecomment-3014309449
ACKs for top commit:
fjahr:
Code Review ACK fcfd3db563e89fd79820a4cdfa102d624d801de1
achow101:
ACK fcfd3db563e89fd79820a4cdfa102d624d801de1
furszy:
ACK fcfd3db563e89fd79820a4cdfa102d624d801de1
Tree-SHA512: 04f5e9c3f73f598c3d41d6e35bb59c64c7b93b03ad9fce3c40901733147ce7764f41f475fef1527d44af18f722759996a31ca83b48cb52153795d5022fecfd14
b1a8ac07e91dd1d305fcbc16ea931d60e46c0055 doc: Release note for removed watchonly parameters and results (Ava Chow)
15710869e19e707ef03492c55030dcefa16269d8 wallet: Remove ISMINE_WATCH_ONLY (Ava Chow)
4439bf4b41a6997d4d965f00a8c40efa9cf6895b wallet, spend: Remove fWatchOnly from CCoinControl (Ava Chow)
1337c72198a7d32935431d64e9e58c12f9003abc wallet, rpc: Remove watchonly from RPCs (Ava Chow)
e81d95d435744e48615973dc22acce1a291bd20d wallet: Remove watchonly balances (Ava Chow)
d20dc9c6aae089ab926fd135febd69a8f0744a18 wallet: Wallets without private keys cannot grind R (Ava Chow)
9991f49c38c084967ca66791d838c99b42f000eb test: Watchonly wallets should estimate larger size (Ava Chow)
Pull request description:
Descriptor wallets do not use the watchonly behavior as it is not possible to mix watchonly and non-watchonly in a descriptor wallet. With legacy wallets now removed, all of the watchonly handling and reporting code is no longer needed. This PR removes watchonly options and results from the RPCs and the handling of watchonly things from the wallet's internals.
With all of the watchonly things removed, ISMINE_WATCH_ONLY is removed as well.
Split from #32523
Depends on #32594 for tests that are easier to read
ACKs for top commit:
Eunovo:
ACK b1a8ac07e9
maflcko:
re-ACK b1a8ac07e91dd1d305fcbc16ea931d60e46c0055 🌈
rkrux:
ACK b1a8ac07e91dd1d305fcbc16ea931d60e46c0055
furszy:
light code review ACK b1a8ac07e91dd1d305fcbc16ea931d60e46c0055
Tree-SHA512: bc87f37a13294f7208991be8f93899b49e5bdf87c70e0f66d9c4cb09c03be6c202320406f27e9a35aa2f57319d19a3f0c07d5e5ddbc97c7edab165b1656d6612
8cc9845b8ddf4f93a02c622e7df8d1095dc1a640 wallet, rpc: Use `OUTPUT_TYPES` to describe the output types instead of hardcoding them (w0xlt)
Pull request description:
Follow-up to https://github.com/bitcoin/bitcoin/pull/32429, built on top of it.
This PR addresses the https://github.com/bitcoin/bitcoin/pull/32429#discussion_r2076251627 that the RPC documentation does not use `OUTPUT_TYPES`, but rather hardcodes them, as is already the case for the `getnewaddress` command.
So here the output types are changed from `std::string` to `std::string_view` so that the values are known at compile time or during the early stages of program startup, before main() execution.
It also updates `wallet/rpc/addresses.cpp` to write the RPC docs according to `OUTPUT_TYPES` instead of using hardcoded version.
It also updates the documentation in outputtypes.h, adding Doxygen comments,
ACKs for top commit:
maflcko:
lgtm ACK 8cc9845b8ddf4f93a02c622e7df8d1095dc1a640
achow101:
ACK 8cc9845b8ddf4f93a02c622e7df8d1095dc1a640
Tree-SHA512: e86d813d6d158dd2f6c62519a7ecaa878f2e4f686b5bae82028a106bd6671a13b10fb366f9bb7b94974777217e1852f38e8aa05bba00cd27f94f4412167a3562
c3fe85e2d6dd4f251a62a99fd891b0fa370f9712 wallet, rpc, test: Remove deprecated getunconfirmedbalance (Ava Chow)
0ec255139be3745a135386e9db957fe81bc3d833 wallet, rpc: Remove deprecated balances from getwalletinfo (Ava Chow)
Pull request description:
`getwalletinfo` result fields `balance`, `immature_balance`, and `unconfirmed_balance`, and the `getunconfirmedbalance` RPC have all been deprecated since 0.19.0. It's been long enough that they should either be removed or undeprecated. The functionality provided by these RPCs is provided by `getbalances`.
ACKs for top commit:
davidgumberg:
ACK c3fe85e2d6
rkrux:
ACK c3fe85e2d6dd4f251a62a99fd891b0fa370f9712
BrandonOdiwuor:
ACK c3fe85e2d6dd4f251a62a99fd891b0fa370f9712 removing the deprecated `balance, unconfirmed_balance, immature_balance` fields from `getwalletinfo` and `getunconfirmedbalance` RPCs, as this infomation can be found on the `getbalances` RPC
w0xlt:
reACK c3fe85e2d6
Tree-SHA512: c7c4acfd9cabc7517ba813b95281a6c6a717a417312afd9346298669b4f7bd37724ad977148ce42db7fd47fc3d1f5a8482d8ff2e7b9cb74756b171a5b8b91ef2
47237cd1938058b29fdec242c3a37611e255fda0 wallet, rpc: Output wallet flags in getwalletinfo (Ava Chow)
bc2a26b296238cbead6012c071bc7741c40fbd02 wallet: Add GetWalletFlags (Ava Chow)
69f588a99a7a79d1d72300bc0f2c8475f95f6c6a wallet: Set upgraded descriptor cache flag for newly created wallets (Ava Chow)
Pull request description:
Newly created wallets will always have an upgraded descriptor cache, so set those.
Also, to verify this behavior, add a new `flags` field to `getwalletinfo` and check that in the functional tests.
Split from #32489
ACKs for top commit:
Sjors:
ACK 47237cd1938058b29fdec242c3a37611e255fda0
w0xlt:
ACK 47237cd193
rkrux:
ACK 47237cd1938058b29fdec242c3a37611e255fda0
Tree-SHA512: 97c7f85b858efe5ced9b8aafb6cd7c1a547de6f8013b82bfc75bc567cf73c9db5e168e3980355756541305520022fd776b8d4d240d3fb34ed86c27d2acaf4863
ce90f0c99fded22dd24f08757d6f48b5c6b52990 rpc, wallet, refactor: Remove non-descriptor errors (pablomartin4btc)
573bcd75d7b65ff02aaeea40d6f870a9c0bc7490 wallet, refactor: Remove unused SetupGeneration (pablomartin4btc)
5431f2dc2159f55e0fbe89d07deb97fe2a73fb43 wallet, refactor: Remove Legacy warnings and errors (pablomartin4btc)
Pull request description:
Remove dead code due to legacy wallet support removal.
These changes have no impact on functionality. They are transparent to the end user, as legacy wallets can't be created or loaded anymore, so these checks are no longer reached. The legacy-to-descriptor wallet migration flow is not affected either, as these removals are not part of its process.
ACKs for top commit:
achow101:
ACK ce90f0c99fded22dd24f08757d6f48b5c6b52990
rkrux:
utACK ce90f0c99fded22dd24f08757d6f48b5c6b52990
Tree-SHA512: 9229ad9dda9ff1dece73b5b15a20d69c6ab1ff2c75b2ec430ddbbaeb3467f6a850f53df527bcb4a8114ccbf1aa9c794462d71a8d516aed6f9a9da74edae16feb
0def84d407facd319b52826d013cad0d5fc8dbf5 test: Verify parent_desc in RPCs (Ava Chow)
2554cee988fb2ddf65428b354a238f1a4efc1aca test: Enable default wallet for wallet_descriptor.py (Ava Chow)
3fc9d9f241a44ab64774aa9ddc3ded4bb589ed5a wallet, rpc: Push the normalized parent descriptor (Ava Chow)
Pull request description:
Instead of prividing the descriptor string as stored in the db, use the normalized descriptor as is done for getaddressinfo's parent_desc field.
Split from #32489
ACKs for top commit:
Sjors:
re-utACK 0def84d407
rkrux:
ACK 0def84d407facd319b52826d013cad0d5fc8dbf5
w0xlt:
reACK 0def84d407
Tree-SHA512: 575c5b545d6f0aa7e135696b7a55c004e754fca4dd35dd9cf71b0b45b49a2e86e7b20570e768534d587005953bb893645379ec1ba4f98cfd26811f9c2f17de2d
It is not possible to load a legacy/ non-descriptor wallet anymore
so no need to check for WALLET_FLAG_DESCRIPTORS in RPC calls, even when
passing -rpcwallet/ JSON `/wallet/<walletname>/` endpoint, that searches
for the wallets loaded already in the context.
This RPC lists all the descriptors present in the wallet, not only
the ones that were imported, but also the ones generated when a
new wallet is created.
It can be verified by creating a new wallet and calling the
`listdescriptors` RPC, which will contain 8 ranged descriptors that
are created for every new wallet.
Also, update the description to get rid of "descriptor-enabled"
because this is the only wallet type available now after removal of
legacy wallets.
f98e1aaf34e347088caa54403521e3b5cb55dd40 rpc: Note in fundrawtransaction doc, fee rate is for package (benthecarman)
Pull request description:
Accidentally made some transactions with a much higher fee rate than I wanted because I did not know this would do it for the package rather than the individual tx.
ACKs for top commit:
achow101:
ACK f98e1aaf34e347088caa54403521e3b5cb55dd40
rkrux:
re-ACK f98e1aaf34e347088caa54403521e3b5cb55dd40
danielabrozzoni:
ACK f98e1aaf34e347088caa54403521e3b5cb55dd40
Tree-SHA512: 9f961de1200803ec4d1c6901fd606bb6cf707ffd03942d9dc0d4b6554c827075f99d693b93e892f728679d67e63e12c71da4426dab091b3311d1605bc37251a2
86e1111239cdb39dd32cfb5178653c608fa30515 test: verify node skips loading legacy wallets during startup (furszy)
9f94de5bb54ff683bd4d3a7723617b34a4706bb6 wallet: init, don't error out when loading legacy wallets (furszy)
Pull request description:
Instead of failing during initialization and shutting down the app when encountering a legacy wallet, skip loading the wallet and notify the user accordingly.
This allows users to access migration functionalities without needing to manually remove the wallet from settings.json or resort to using the bitcoin-wallet utility.
This means that GUI users will be able to use the migration button, and bitcoin-cli users will be able to call the migratewallet RPC directly after init.
ACKs for top commit:
achow101:
ACK 86e1111239cdb39dd32cfb5178653c608fa30515
w0xlt:
ACK 86e1111239
Tree-SHA512: 85d594a503ee7a833a23754b71b6ba4869ca34ed802c9ac0cd7b2fa56978f5fcad84ee4bd3acdcc61cf8e7f08f0789336febc5d76beae1eebf7bd51462512b78
If the locked coin needs to be persisted to the wallet database,
insteead of having the RPC figure out when to create a WalletBatch and
having LockCoin's behavior depend on it, have LockCoin take whether to
persist as a parameter so it makes the batch.
Since unlocking a persisted locked coin requires a database write as
well, we need to track whether the locked coin was persisted to the
wallet database so that it can erase the locked coin when necessary.
Keeping track of whether a locked coin was persisted is also useful
information for future PRs.
This is the RPC example counterpart to commit
86de8c1668005304b2c630ca2ad4a8ca8e348e90 (PR #32544).
Since the recent legacy wallet removal this parameter *must* be
true, so providing it in the examples doesn't contain valuable
information anymore and it seems best to remove them.
Now that we only ever operate on descriptor wallets, mentioning
that a faster rescan is only available for them is redundant and
can be removed.
These texts were originally introduced in commit
ca48a4694f73e5be8f971ae482ebc2cce4caef44 (PR #25957).
This `getwalletinfo()` result field was only ever returned for
legacy wallets and is hence not relevant anymore, so we can
delete it and the corresponding CWallet/ScriptPubKeyMan code
behind it.
785e1407b0a39fef81a7b25554aab88d4cecd66b wallet: Use util::Error throughout AddWalletDescriptor (Ava Chow)
Pull request description:
#32023 changed `AddWalletDescriptor` to return `util::Error`, but did not change all of the failure cases to do so. This may result in some callers continuing when there was actually an error. Unify all of the failure cases to use `util::Error` so that all callers handle `AddWalletDescriptor` errors in the same way.
The encapsulated return type is changed from `ScriptPubKeyMan*` to `std::reference_wrapper<DescriptorScriptPubKeyMan>`. This avoids having a value that can be interpreted as a bool, and also removes the need to constantly dynamic_cast the returned value. The only kind of `ScriptPubKeyMan` that can come out of `AddWalletDescriptor` is a `DescriptorScriptPubKeyMan` anyways.
ACKs for top commit:
Sjors:
utACK 785e1407b0a39fef81a7b25554aab88d4cecd66b
ryanofsky:
Code review ACK 785e1407b0a39fef81a7b25554aab88d4cecd66b
furszy:
Code review ACK 785e1407b0a39fef81a7b25554aab88d4cecd66b
Tree-SHA512: 52a48263c8d4161a8c0419b7289c25b0986f8e3bcd10b639eeeb0b6862d08b6c5e70998d20070ab26b39ecd90ab83dc8b71c65d85f70626282cf8cc6abff50e7
ee045b61efc1479c1866b786661ef39a863677d0 rpc, psbt: Require sighashes match for descriptorprocesspsbt (Ava Chow)
2b7682c3729d4e054ac4260b344a75ad4b7239b3 psbt: use sighash type field to determine whether to remove non-witness utxos (Ava Chow)
28781b5f06709212934c521c513bb2e1a521a31f psbt: Add sighash types to PSBT when not DEFAULT or ALL (Ava Chow)
15ce1bd73f80e998f7402433572b695f589f7f42 psbt: Enforce sighash type of signatures matches psbt (Ava Chow)
1f71cd337ad75390a1f8810d6715f3634ed07e98 wallet: Remove sighash type enforcement from FillPSBT (Ava Chow)
4c7d767e49b2e709a2b00af92ca76e9f30e47aec psbt: Check sighash types in SignPSBTInput and take sighash as optional (Ava Chow)
a11825694856a2643e9600fa537182fbb597c107 script: Add IsPayToTaproot() (Ava Chow)
d6001dcd4ada5b64c8113450ed736a2581c97518 wallet: change FillPSBT to take sighash as optional (Ava Chow)
e58b680923b10f0690de9dcd34f17fbb8d6de5eb psbt: Return PSBTError from SignPSBTInput (Ava Chow)
2adfd815325713d64b9daa61c2f93061d27bd47d tests: Test PSBT sighash type mismatch (Ava Chow)
5a5d26d6123e0056656e406cd9f35aac6f71df4b psbt: Require ECDSA signatures to be validly encoded (Ava Chow)
Pull request description:
Currently, we do not add the sighash field to PSBTs at all, even when we have signed with a non-default sighash. This PR changes the behavior such that when we (attempt to) sign with a sighash other than DEFAULT or ALL, the sighash type field will be added to the PSBT to inform the later signers that a different sighash type was used by a signer. Notably, this is necessary for MuSig2 support as all signers must sign using the same sighash type, but the sighash is not provided in partial signatures.
Furthermore, because the sighash type can also be provided on the command line, we require that if both a command line sighash type and the sighash field is present, they must specify the same sighash type. However, this was being checked by the wallet, rather than the signing code, so the `descriptorprocesspsbt` RPC was not enforcing this restriction at all, and in fact ignored the sighash field entirely. This PR refactors the checking code so that the underlying PSBT signing function `SignPSBTInput` does the check.
ACKs for top commit:
theStack:
re-ACK ee045b61efc1479c1866b786661ef39a863677d0
rkrux:
re-ACK ee045b61efc1479c1866b786661ef39a863677d0
fjahr:
Code review ACK ee045b61efc1479c1866b786661ef39a863677d0
Tree-SHA512: 4ead5be1ef6756251b827f594beba868a145d75bf7f4ef6f15ad21f0ae4b8d71b38c83494e5a6b75f37fadd097178cddd93d614b962a2c72fc134f00ba2f74ae
