Age | Commit message (Collapse) | Author |
|
|
|
647b81b70938dc4dbcf32399c56f78be395c721a wallet, rpc: add listdescriptors command (Ivan Metlushko)
Pull request description:
Looking for concept ACKs
**Rationale**: allow users to inspect the contents of their newly created descriptor wallets.
Currently the command only returns xpubs which is not very useful in itself, but there are multiples ways to extend it:
* add an option to export xprv
* with #19136 it'll be possible to return normalised descriptors suitable for a watch-only purposes
The output is compatible with `importdescriptors` command so it could be easily used for backup/recover purposes.
**Output example:**
```json
[
{
"desc": "wpkh(tpubD6NzVbkrYhZ4WW6E2ZETFyNfq2hfF23SKxqSGFvUpPAY58jmmuBybwqwFihAyQPk9KnwTt5516NDZRJ7k5QPeKjy7wuVd5WvXNxwwAs5tUD/*)#nhavpr5h",
"timestamp": 1296688602,
"active": false,
"range": [
0,
999
],
"next": 0
}
]
```
ACKs for top commit:
jonatack:
re-ACK 647b81b70938dc4dbcf32399c56f78be395c721a rebased to master, debug builds cleanly, reviewed diff since last review, tested with a descriptor wallet (and with a legacy wallet)
achow101:
re-ACK 647b81b
Tree-SHA512: 51a3620bb17c836c52cecb066d4fa9d5ff418af56809046eaee0528c4dc240a4e90fff5711ba96e399c6664e00b9ee8194e33852b1b9e75af18061296e19a8a7
|
|
|
|
a6739cc86827759c543bf81f5532ec46e40549c3 rpc: Add specific error code for "wallet already loaded" (Wladimir J. van der Laan)
Pull request description:
Add a separate RPC error code for "wallet already loaded" to avoid having to match on message to detect this.
Requested by shesek for rust-bitcoinrpc.
If concept ACKed needs:
- [ ] Release note
- [x] A functional test (updated the existing test to make it pass, I think this is enough)
ACKs for top commit:
jonasschnelli:
Code Review ACK a6739cc86827759c543bf81f5532ec46e40549c3
promag:
Code review ACK a6739cc86827759c543bf81f5532ec46e40549c3.
Tree-SHA512: 9091872e6ea148aec733705d6af330f72a02f23b936b892ac28f9023da7430af6332418048adbee6014305b812316391812039e9180f7f3362d11f206c13b7d0
|
|
|
|
This commit addresses #20809.
We add an additional 'error' property in the result of 'validateaddress' in case the address is not valid that gives a short description of why the address in invalid. We also change the error message returned by 'getaddressinfo' in case the address is invalid.
|
|
ea0a7ec949f0f7e212f0d8819f7a54cad2258bdd Remove deprecated bumpfee behavior (Andrew Chow)
Pull request description:
Removes the deprecation message, behavior, and test.
This was marked for removal in 22.0.
ACKs for top commit:
promag:
ACK ea0a7ec949f0f7e212f0d8819f7a54cad2258bdd, maybe add need release notes tag.
Tree-SHA512: d1626906849f6ee37213c32e5f8c1433ad8fb7beabcd88f5801b1964b322171a2341bdfbd9a3a5ab39b2fd9d9c6a05f73298583423a73cab1275653105c03e8e
|
|
|
|
|
|
|
|
The only use was to work around a compiler warning in an ancient
compiler, which we no longer support.
|
|
f3d870fc2271bf45e0269e5ae135bced1a26f620 wallet: List all wallets in non-SQLite or non-BDB builds (Russell Yanofsky)
d70dc89e78ee6355e0bc37cc36cfc04ef7a86885 refactor: Consolidate redundant wallet database path and exists functions (Russell Yanofsky)
6a7a63644cd2fc56538d323cc0d5c1d7945247fd refactor: Drop call to GetWalletEnv in wallet salvage code (Russell Yanofsky)
6ee9cbdd18a70894f89dd268c276d5eb47a34827 refactor: Replace ListWalletDir() function with ListDatabases() (Russell Yanofsky)
5aaeb6cf877055c47fa2bbd2ea5e8d3d2033933b MOVEONLY: Move IsBDBFile, IsSQLiteFile, and ListWalletDir (Russell Yanofsky)
Pull request description:
This PR does not change behavior when bitcoin is built normally with both the SQLite and BDB libraries. It just makes non-SQLite and non-BDB builds more similar to the normal build. Specifically:
- It makes wallet directory lists always include all wallets so wallets don't appear missing depending on the build.
- It now triggers specific "Build does not support SQLite database format" and "Build does not support Berkeley DB database format" errors if a wallet can't be loaded instead of the more ambiguous and scary "Data is not in recognized format" error.
Both changes are implemented in the last commit. The previous commits are just refactoring cleanups that make the last commit possible and consolidate and reduce code.
ACKs for top commit:
achow101:
ACK f3d870fc2271bf45e0269e5ae135bced1a26f620
promag:
Tested ACK f3d870fc2271bf45e0269e5ae135bced1a26f620. Tested a --without-sqlite build with sqlite wallets.
Tree-SHA512: 029ad21559dbc338b5f351d05113c51bc25bce830f4f4e18bcd82287bc528275347a60249da65b91d252632aeb70b25d057bd59c704bfcaafb9f790bc5b59762
|
|
|
|
No change to behavior. This is just cleanup after previous MOVEONLY commit to
make db.h list function fit conventions of surrounding functions.
|
|
matching RPC endpoint wallet
89bdad5b25ae4ac03a486f729a5b58ae6f21946d RPC/Wallet: unloadwallet: Allow specifying wallet_name param matching RPC endpoint (Luke Dashjr)
Pull request description:
Allow specifying the `wallet_name` param to `unloadwallet` on RPC wallet endpoints, so long as it matches the endpoint wallet.
ACKs for top commit:
jonatack:
ACK 89bdad5b25ae4ac03a486f729a5b58ae6f21946d
MarcoFalke:
review ACK 89bdad5b25ae4ac03a486f729a5b58ae6f21946d
Tree-SHA512: efb399c33f7b5596870a26a8680f453ca47aa7a6db4e550f9435d13044f1c4bad0ae11e8f0205213409d08b75c4188c3be782e54aafab1f65b97eb8cf5c252a9
|
|
3eb6f8b2e61c24a22ea9396d86672307845f35eb wallet (not for backport): improve upgradewallet error messages (Jon Atack)
ca8cd893bb56bf5d455154b0498b1f58f77d20ed wallet: fix and improve upgradewallet error responses (Jon Atack)
99d56e357159c7154f69f28cb5587c5ca20d6594 wallet: fix and improve upgradewallet result responses (Jon Atack)
2498b04ce88696a3216fc38b7d393906b733e8b1 Don't upgrade to HD split if it is already supported (Andrew Chow)
c46c18b788cb0862aafbb116fd37936cbed6a431 wallet: refactor GetClosestWalletFeature() (Jon Atack)
Pull request description:
This follows up on #18836 and #20282 to fix and improve the as-yet unreleased `upgradewallet` feature and also implement review follow-up in https://github.com/bitcoin/bitcoin/pull/18836#discussion_r519328607.
This PR fixes 4 upgradewallet issues:
- this bug: https://github.com/bitcoin/bitcoin/pull/20403#discussion_r526063920
- it returns nothing in the absence of an RPC error, which isn't reassuring for users
- it returns the same thing both in the case of a successful upgrade and when no upgrade took place
- the error message object is currently dead code
This PR fixes the above and provides:
...user feedback to not silently return without upgrading
```
{
"wallet_name": "disable private keys",
"previous_version": 169900,
"current_version": 169900,
"result": "Already at latest version. Wallet version unchanged."
}
```
...better feedback after successfully upgrading
```
{
"wallet_name": "watch-only",
"previous_version": 159900,
"current_version": 169900,
"result": "Wallet upgraded successfully from version 159900 to version 169900."
}
```
...helpful error responses
```
{
"wallet_name": "blank",
"previous_version": 169900,
"current_version": 169900,
"error": "Cannot downgrade wallet from version 169900 to version 159900. Wallet version unchanged."
}
{
"wallet_name": "blank",
"previous_version": 130000,
"current_version": 130000,
"error": "Cannot upgrade a non HD split wallet from version 130000 to version 169899 without upgrading to support pre-split keypool. Please use version 169900 or no version specified."
}
```
updated help:
```
upgradewallet ( version )
Upgrade the wallet. Upgrades to the latest version if no version number is specified.
New keys may be generated and a new wallet backup will need to be made.
Arguments:
1. version (numeric, optional, default=169900) The version number to upgrade to. Default is the latest wallet version.
Result:
{ (json object)
"wallet_name" : "str", (string) Name of wallet this operation was performed on
"previous_version" : n, (numeric) Version of wallet before this operation
"current_version" : n, (numeric) Version of wallet after this operation
"result" : "str", (string, optional) Description of result, if no error
"error" : "str" (string, optional) Error message (if there is one)
}
```
ACKs for top commit:
achow101:
ACK 3eb6f8b
MarcoFalke:
review ACK 3eb6f8b2e61c24a22ea9396d86672307845f35eb 🛡
Tree-SHA512: b767314069e26b5933b123acfea6aa40708507f504bdb22884da020a4ca1332af38a7072b061e36281533af9f4e236d94d3c129daf6fe5b55241127537038eed
|
|
fa69c2c78455fd0dc436018fece9ff7fc83a180d wallet: Do not treat default constructed types as None-type (MarcoFalke)
fac4e136fa3d0fab7fde900a6be921313e16e7a6 refactor: Change pointer to reference because it can not be null (MarcoFalke)
Pull request description:
Equating `0==None` and `""==None` is confusing, unneeded and undocumented
ACKs for top commit:
jonatack:
ACK fa69c2c78455fd0dc436018fece9ff7fc83a180d
achow101:
ACK fa69c2c78455fd0dc436018fece9ff7fc83a180d
Sjors:
tACK fa69c2c78455fd0dc436018fece9ff7fc83a180d modulo `unset`
Tree-SHA512: c4c8d0ad80c6697621d356a9545caf28ca2facc82bb2fa8e70eceb52372d25f0685237c73688c4b01da0e75d213c77c0d45011a8bdfe81ea783d85f045786dac
|
|
request and wallet_name parameter specify a wallet
b1f59d55d920d2b35269b474762f94fec87bfb16 RPC/Wallet: unloadwallet: Clarify docs/error when both the RPC endpoint and wallet_name parameter specify a wallet (Luke Dashjr)
Pull request description:
Just documentation clarifications from #20448
ACKs for top commit:
MarcoFalke:
review ACK b1f59d55d920d2b35269b474762f94fec87bfb16
jonatack:
re-ACK b1f59d55d920d2b35269b474762f94fec87bfb16 per `git diff e8303a0 b1f59d5`
Tree-SHA512: ac068b0aa7ceed49496367fdd9425b59dbba18b56e89b26afc22a6c8ece51f0b92a169cacd55740b1cadab2b32f4f8e8700e609066ab7e59d3b53c7891da585e
|
|
endpoint
|
|
wallet_name parameter specify a wallet
|
|
d52f502b1ea1cafa7d58c5517f01dba26ecb7269 Fix mock SQLiteDatabases (Andrew Chow)
99309ab3e96a290359b84f9b657c5115aa3470dd Allow disabling BDB in configure with --without-bdb (Andrew Chow)
ee47f11f7399ec3a4330ea1f2fc388c7e32959d6 GUI: Force descriptor wallets when BDB is not compiled (Andrew Chow)
71e40b33bd1e72ccf5d82e1d3f8b481f8e965492 RPC: Require descriptors=True for createwallet when BDB is not compiled (Andrew Chow)
6ebc41bf9cb0184554923e84e1935195d356f2b3 Enforce salvage is only for BDB wallets (Andrew Chow)
a58b719cf75e2d97205ec260bcff0d4780fe4fb8 Do not compile BDB things when USE_BDB is defined (Andrew Chow)
b33af48210c117a734fc3e1bebeb1c2057645775 Include wallet/bdb.h where it is actually being used (Andrew Chow)
Pull request description:
Adds a `--without-bdb` option to `configure` which disables the compilation of the BDB stuff. Legacy wallets will not be created when BDB is not compiled. A legacy-sqlite wallet can be loaded, but we will not create them.
Based on #20156 to resolve the situation where both `--without-sqlite` and `--without-bdb` are provided. In that case, the wallet is disabled and `--disable-wallet` is effectively set.
ACKs for top commit:
laanwj:
Code review ACK d52f502b1ea1cafa7d58c5517f01dba26ecb7269
Tree-SHA512: 5a92ba7a542acc2e27003e9d4e5940e0d02d5c1f110db06cdcab831372bfd83e8d89c269caff31dd5bff062c1cf5f04683becff12bd23a33be731676f346553d
|
|
and remove redundant units ("Must be at least 1.000 sat/vB sat/vB" -> "1.00 sat vB")
|
|
A check to raise an error on zero-fee txns was mistakenly extended in commit
a0d4957 from the bumpfee and send{toaddress, many} RPCs to also include
fundrawtransaction and walletcreatefundedpsbt.
This commit overrides zero fee rate checking for these two RPCs, not only for
the feeRate (BTC/kvB) arg to return to previous behavior, but also for the new
fee_rate (sat/vB) arg.
|
|
|
|
|
|
|
|
|
|
|
|
05e82d86b09d914ebce05dbc92a7299cb026847b wallet: override minfee checks (fOverrideFeeRate) for fee_rate (Jon Atack)
9a670b4f07a6140de809d73cbd7f3e614eb6ea74 wallet: update sendtoaddress, send RPC examples with fee_rate (Jon Atack)
be481b72e24fb6834bd674cd8daee67c6938b42d wallet: use MIN_RELAY_TX_FEE in bumpfee help (Jon Atack)
449b730579566459e350703611629e63e54657ed wallet: provide valid values if invalid estimate mode passed (Jon Atack)
6da3afbaee5809ebf6d88efaa3958c505c2d71c7 wallet: update remaining rpcwallet fee rate units to BTC/kvB (Jon Atack)
173b5b5fe07d45be5a1e5bc7a5df996f20ab1e85 wallet: update fee rate units, use sat/vB for fee_rate error messages (Jon Atack)
7f9835a05abf3e168ad93e7195cbaa4bf61b9b07 wallet: remove fee rates from conf_target helps (Jon Atack)
b7994c01e9a3251536fe6538a22f614774eec82d wallet: add fee_rate unit warnings to bumpfee (Jon Atack)
410e471fa42d3db04e8879c71f8c824dcc151a83 wallet: remove redundant bumpfee fee_rate checks (Jon Atack)
a0d495747320c79b27a83c216dcc526ac8df8f24 wallet: introduce fee_rate (sat/vB) param/option (Jon Atack)
e21212f01b7c41eba13b0479b252053cf482bc1f wallet: remove unneeded WALLET_BTC_KB_TO_SAT_B constant (Jon Atack)
6112cf20d43b0be34fe0edce2ac3e6b27cae1bbe wallet: add CFeeRate ctor doxygen documentation (Jon Atack)
3f7279161347543ce4e997d78ea89a4043491145 wallet: fix bug in RPC send options (Jon Atack)
Pull request description:
This PR builds on #11413 and #20220 to address #19543.
- replace overloading the conf_target and estimate_mode params with `fee_rate` in sat/vB in the sendtoaddress, sendmany, send, fundrawtransaction, walletcreatefundedpsbt, and bumpfee RPCs
- allow non-actionable conf_target value of `0` and estimate_mode value of `""` to be passed to use `fee_rate` as a positional argument, in addition to as a named argument
- fix a bug in the experimental send RPC described in https://github.com/bitcoin/bitcoin/pull/20220#discussion_r513789526 where args were not being passed correctly into the options values
- update the feerate error message units for these RPCs from BTC/kB to sat/vB
- update the test coverage, help docs, doxygen docs, and some of the RPC examples
- other changes to address the excellent review feedback
See this wallet meeting log for more context: http://www.erisian.com.au/bitcoin-core-dev/log-2020-11-06.html#l-309
ACKs for top commit:
achow101:
re-ACK 05e82d8
MarcoFalke:
review ACK 05e82d86b0 did not test and found a few style nits, which can be fixed later 🍯
Xekyo:
tACK 05e82d86b09d914ebce05dbc92a7299cb026847b
Sjors:
utACK 05e82d86b09d914ebce05dbc92a7299cb026847b
Tree-SHA512: a4ee5f184ada53f1840b2923d25873bda88c5a2ae48e67eeea2417a0b35154798cfdb3c147b05dd56bd6608a784e1b91623bb985ee2ab9ef2baaec22206d0a9c
|
|
963696288955dc31b3a4fd136bfb791a9d99755b [upgradewallet] removed unused warning param (Sishir Giri)
Pull request description:
The `warning` variable was unused in `upgradewallet` so I removed it
ACKs for top commit:
practicalswift:
ACK 963696288955dc31b3a4fd136bfb791a9d99755b: diff looks correct
MarcoFalke:
review ACK 963696288955dc31b3a4fd136bfb791a9d99755b
jonatack:
ACK 963696288955dc31b3a4fd136bfb791a9d99755b
Tree-SHA512: 1d63186ce1e05e86a778340f2d7986c2cee1523de0a11cea39e8d148ac7ee26c49741dfa302b5c1cd1c8d74e67c1f9baee2763720c2d850b57da9a3fdce24565
|
|
|
|
|
|
|
|
in RPCs fundrawtransaction and walletcreatefundedpsbt only.
This provides the existing feeRate (BTC/kvB) behavior in these two RPCs to the
new fee_rate (sat/vB) param also.
See these two GitHub review discussions for more info:
https://github.com/bitcoin/bitcoin/pull/10706/#discussion_r126560525
https://github.com/bitcoin/bitcoin/pull/20305#discussion_r520032533
|
|
|
|
Co-authored-by: Murch <murch@murch.one>
|
|
Co-authored-by: Murch <murch@murch.one>
|
|
|
|
and BTC/kvB for feeRate error messages.
|
|
|
|
|
|
SetFeeEstimateMode() handles these checks now and provides a more actionable
error message.
|
|
Create a fee_rate (sat/vB) RPC param and replace overloading the conf_target and
estimate_mode params in the following 6 RPCs with it:
- sendtoaddress
- sendmany
- send
- fundrawtransaction
- walletcreatefundedpsbt
- bumpfee
In RPC bumpfee, the previously existing fee_rate remains but the unit is changed
from BTC/kvB to sat/vB. This is a breaking change, but it should not be an
overly risky one, as the units change by a factor of 1e5 and any fees specified
in BTC/kvB after this commit will either be too low and raise an error or be 1
sat/vB and can be RBFed.
Update the test coverage for each RPC.
Co-authored-by: Murch <murch@murch.one>
|
|
|
|
when empty, options were not being populated by arguments of the same name
found while adding test coverage in 603c0050
|
|
0be29000c011dec0722481dbebb159873da6fa54 rpc: update conf_target helps for correctness/consistency (Jon Atack)
778b9be40667876c705e586849ea9c9e44cf451c wallet, rpc: fix send subtract_fee_from_outputs help (Jon Atack)
603c0050837ec65765208dd54dde354145fbe098 wallet: add rpc send explicit fee rate coverage (Jon Atack)
dd341e602d5160fc621c0299179b91403756b61d wallet: add sendtoaddress/sendmany explicit fee rate coverage (Jon Atack)
44e7bfa60313e4ae67da49e5ba4535038b71b453 wallet: add walletcreatefundedpsbt explicit fee rate coverage (Jon Atack)
6e1ea4273e52fdcd86c87628aa595c03a071ca8c test: refactor for walletcreatefundedpsbt fee rate coverage (Jon Atack)
3ac7b0c6f1c68e74a84d868a454f508bada6b09d wallet: fundrawtx fee rate coverage, fixup ParseConfirmTarget() (Jon Atack)
2d8eba8f8425a2515022d51f1f5b4911329fbf55 wallet: combine redundant bumpfee invalid params and args tests (Jon Atack)
1697a40b6f841a54ee0d9744ed7fd09034b0ddad wallet: improve bumpfee error/help, add explicit fee rate coverage (Jon Atack)
fc5721723d34f76f9e1ffd2e31f274ea6b22f894 wallet: fix SetFeeEstimateMode() error message (Jon Atack)
052427eef1c9da84c474c5161b1910d3328ef0da wallet, bugfix: fix bumpfee with explicit fee rate modes (Jon Atack)
Pull request description:
Follow-up to #11413 providing a base to build on for #19543:
- bugfix for `bumpfee` raising a JSON error with explicit feerates, fixes issue #20219
- adds explicit feerate test coverage for `bumpfee`, `fundrawtransaction`, `walletcreatefundedpsbt`, `send`, `sendtoaddress`, and `sendmany`
- improves a few related RPC error messages and `ParseConfirmTarget()` / error message
- fixes/improves the explicit fee rate information in the 6 RPC helps, of which 2 were also missing `conf_target` sat/B units
This provides a spec and regression coverage for the potential next step of a universal `sat/vB` feerate argument (see #19543), as well as immediate coverage and minimum fixes for 0.21.
ACKs for top commit:
kallewoof:
Concept/Tested ACK 0be29000c011dec0722481dbebb159873da6fa54
meshcollider:
Code review + functional test run ACK 0be29000c011dec0722481dbebb159873da6fa54
Tree-SHA512: efd965003e991cba51d4504e2940f06ab3d742e34022e96a673606b44fad85596aa03a8c1809f06df7ebcf21a38e18a891e54392fe3d6fb4d120bbe4ea0cf5e0
|
|
|
|
bbb42a68961a8ae1e4ef3221ffb1d5ff7272b075 RPC: createwallet: Nicer error message if descriptor wallet requested and sqlite support not compiled in (Luke Dashjr)
6608fec332eac4dfd91138bc4fe2e1b5c7bb758f GUI: Create Wallet: Nicely disable descriptor wallet checkbox if sqlite support not compiled in (Luke Dashjr)
7b54d768e1514b328e1ac108d3db2f1bac3ba7ff Make sqlite support optional (compile-time) (Luke Dashjr)
Pull request description:
As a new requirement, sqlite support should be optional. This PR aims to be only minimum/blocker changes for 0.21.
Potential follow-up PRs after this:
* Make BDB support optional
* Nicer error messages when user tries to load an unsupported wallet
* Don't compile descriptor wallet code if sqlite disabled
ACKs for top commit:
jonasschnelli:
Tested ACK bbb42a68961a8ae1e4ef3221ffb1d5ff7272b075
achow101:
ACK bbb42a68961a8ae1e4ef3221ffb1d5ff7272b075
Sjors:
re-utACK bbb42a68961a8ae1e4ef3221ffb1d5ff7272b075
hebasto:
ACK bbb42a68961a8ae1e4ef3221ffb1d5ff7272b075, tested on Linux Mint 20 (x86_64, Qt 5.12.8).
Tree-SHA512: 500209dd1971310fab8ae51543343ce0ba91f088ccccff6109b4cc27547cd5532289dca6cb7dac2a7d7c59cdf3c8f5aacc31e9b0f912e38cea52ec26b97100bd
|
|
for sendtoaddress, sendmany, send, fundrawtransaction, walletcreatefundedpsbt, bumpfee
|
|
|